-
Notifications
You must be signed in to change notification settings - Fork 22.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
SVG feComposite improvements #2353
SVG feComposite improvements #2353
Conversation
@chrisdavidmills @Ryuno-Ki This is ready for review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks OK to me. I gave it a light copy edit and feel like it makes sense. But I'm certainly not an SVG expert...anything to add?
Hrm, I just decided to get this merged. Any further improvements can come later. Thanks for all your investigation on this @hamishwillee ! |
Dang. I just finished work :-) |
<p><code>BackgroundImage</code> is not supported as a filter source in modern browsers (see the <a href="/en-US/docs/Web/SVG/Element/feComposite#browser_compatibility">feComposite compatibility table</a>). We therefore need to import one of the images to blend inside the filter itself, using an <code><feImage></code> element.</p> | ||
|
||
<div class="notecard note"> | ||
<h4>Note</h4> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nitpick: I think, we intend by two spaces.
@@ -9,19 +9,57 @@ | |||
--- | |||
<div>{{SVGRef}}</div> | |||
|
|||
<p>The <strong><code><feComposite></code></strong> <a href="/en-US/docs/Web/SVG">SVG</a> filter primitive performs the combination of two input images pixel-wise in image space using one of the Porter-Duff compositing operations: <code>over</code><em>, </em><code>in</code><em>, </em><code>atop</code><em>, </em><code>out</code><em>, </em><code>xor</code>, and <code>lighter</code>. Additionally, a component-wise <code>arithmetic</code> operation (with the result clamped between [0..1]) can be applied.</p> | |||
<p>The <strong><code><feComposite></code></strong> <a href="/en-US/docs/Web/SVG">SVG</a> filter primitive performs the combination of two input images pixel-wise in image space using one of the Porter-Duff compositing operations: <code>over</code>, <code>in</code>, <code>atop</code><em>, <code>out</code>, <code>xor</code>, <code>lighter</code>, or <code>arithmetic</code>.</p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think, the <em>
element is misplaced here.
<th>Description</th> | ||
</tr> | ||
<tr> | ||
<td><p>over <img src="operation_over.png" title="over operator" width="300px"></p></td> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing alt
attribute (perhaps rename the title
for it?)
</td> | ||
</tr> | ||
<tr> | ||
<td><p>in <img src="operation_in.png" title="in operator"></p></td> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing alt
attribute.
<td>The parts of the source graphic defined by the <code>in</code> attribute that overlap the destination graphic defined in the <code>in2</code> attribute, replace the destination graphic.</td> | ||
</tr> | ||
<tr> | ||
<td><p>over <img src="operation_out.png" title="out operator"> </p></td> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing alt
attribute.
<td>The parts of the source graphic defined by the <code>in</code> attribute that fall outside the destination graphic defined in the <code>in2</code> attribute, are displayed.</td> | ||
</tr> | ||
<tr> | ||
<td><p>atop <img src="operation_atop.png" title="atop operator"></p></td> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing alt
attribute.
<td>The parts of the source graphic defined in the <code>in</code> attribute, which overlap the destination graphic defined in the <code>in2</code> attribute, replace the destination graphic. The parts of the destination graphic that do not overlap with the source graphic stay untouched.</td> | ||
</tr> | ||
<tr> | ||
<td><p>xor <img src="operation_xor.png" title="xor operator"></p></td> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing alt
attribute.
<td>The non-overlapping regions of the source graphic defined in the <code>in</code> attribute and the destination graphic defined in the <code>in2</code> attribute are combined.</td> | ||
</tr> | ||
<tr> | ||
<td><p>lighter <img src="operation_lighter.png" title="lighter operator"> </p></td> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing alt
attribute.
<td>The sum of the source graphic defined in the <code>in</code> attribute and the destination graphic defined in the <code>in2</code> attribute is displayed.</td> | ||
</tr> | ||
<tr> | ||
<td><p>arithmetic <img src="operation_arithmetic.png" title="arithmetic operator"></p></td> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing alt
attribute.
|
||
<p>where:</p> | ||
<pre class="brush: plain">result = k1*i1*i2 + k2*i1 + k3*i2 + k4</pre> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we had some MathML elsewhere. Might be more apt here.
<p>where:</p> | ||
<ul> | ||
<li><code>i1</code> and <code>i2</code> indicate the corresponding pixel channel values of the input image, which map to {{SVGAttr("in")}} and {{SVGAttr("in2")}} respectively</li> | ||
<li>{{SVGAttr("k1")}},{{SVGAttr("k2")}},{{SVGAttr("k3")}},and {{SVGAttr("k4")}} indicate the values of the attributes with the same name.</li> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing spaces after comma.
<li><a href="/en-US/docs/Web/SVG/Attribute#Filter_primitive_attributes">Filter primitive attributes</a> »</li> | ||
<li><a href="/en-US/docs/Web/SVG/Attribute#core_attributes">Core attributes</a> »</li> | ||
<li><a href="/en-US/docs/Web/SVG/Attribute#presentation_attributes">Presentation attributes</a> »</li> | ||
<li><a href="/en-US/docs/Web/SVG/Attribute#filter_primitive_attributes">Filter primitive attributes</a> »</li> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't the opening » go here?
|
||
<div class="notecard note"> | ||
<h4>Note</h4> | ||
<p><code>BackgroundImage</code> cannot be used as a compositing source on modern browsers, so we can't define a filter that composites using whatever pixels happen to be under the filter as one of the sources. The approach above is a <a href="/en-US/docs/Web/SVG/Attribute/in#workaround_for_backgroundimage">Workaround because we can't use <code>BackgroundImage</code></a>.</p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel, like „Workaround” should be lowercase here.
@hamishwillee I found some minor stylistic nitpicks. Are you fine with me filling a PR on addressing them? |
</filter> | ||
<filter id="imageLighter"> | ||
<feImage xlink:href="https://developer.mozilla.org/files/6457/mdn_logo_only_color.png" x="10px" y="10px" width="160px" /> | ||
<feComposite in2="SourceGraphic" operator="lighter"/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sideshowbarker validator is flagging this. Should I file upstream or is this a bug in the docs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<feComposite in2="SourceGraphic" operator="lighter"/>
@sideshowbarker validator is flagging this. Should I file upstream or is this a bug in the docs?
That was a conformance bug in the checker (thanks for the heads-up).
I fixed it just now in validator/validator@970b6a5
This attempts to address some of the issues pointed out in #1450
The example doesn't work because it depends on
BackgroundImage
source. This is not supported by any of the modern browsers (working on getting that into the BCD. There is a workaround to this problem, but the document doesn't explain what the problem is. Further, that workaround also works-around a bug in Firefox that it can't use an FeImage "partial" as a source - ie you have to pull in a remote URL you can't define a shape and reference that. All very ugly.Anyway, this does two things.
Appreciate some review. I know little about SVG other than this :-)