Closed Bug 1518099 Opened 6 years ago Closed 4 years ago

Implement <feComposite operator="lighter"> (SVG filters)

Categories

(Core :: SVG, enhancement, P3)

Product:
Core
Component:
SVG
Type:
enhancement

Tracking

()

Status:
RESOLVED FIXED
Milestone:
86 Branch
Tracking Flags:
Tracking Status
firefox86 --- fixed

People

(Reporter: jwatt, Assigned: longsonr)

Details

(Keywords: dev-doc-needed)

Attachments

(1 file)

Bug 1518099 - Add support for lighter operator in feComposite
48 bytes, text/x-phabricator-request
Details | Review
There is now a 'lighter' value for feComposite's 'operator' attribute: https://drafts.fxtf.org/filter-effects/#attr-valuedef-operator-lighter We appear to already implement this behavior for canvas. Chrome implements this.
Assignee: nobody → longsonr
Status: NEW → ASSIGNED
Pushed by longsonr@gmail.com: https://hg.mozilla.org/integration/autoland/rev/12325181f43a Add support for lighter operator in feComposite r=jrmuizel,emilio
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/27061 for changes under testing/web-platform/tests

https://hg.mozilla.org/mozilla-central/rev/12325181f43a

Status: ASSIGNED → RESOLVED
Closed: 4 years ago
status-firefox86: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → 86 Branch
Upstream PR merged by moz-wptsync-bot

https://developer.mozilla.org/en-US/docs/Web/SVG/Element/feComposite is incorrect as the table at the bottom indicates that Firefox already supports lighter. I can confirm Chrome does support lighter despite the table saying that it doesn't.

Hi Robert,

I'm looking to document this for Firefox 86 (see mdn/content/1727). But there is a lot more to do with the docs before we can really think about this. I think place to start will be BCD, but in parallel I'd like to look at the example.

As you can see here our example is copied from the spec. It doesn't work "as expected" on FireFox or any of the other main browsers. I found your comment on stackoverflow that this is because we don't support BackgroundImage source (another thing not in the BCD :-( ).

Anyway, I think the "idea" behind the existing example is fine - showing what the each of the filters does by comparing the two inputs. We need something that works on most browsers/isn't reliant on something that doesn't work on any of them.

Can you help create a simple example that works?

Flags: needinfo?(longsonr)

... and is there anything else related to the usage of feComposite (other than BackgroundImage/BackgroundAlpha) that you know are not supported by Firefox (for BCD)?

The example in the spec uses the BackgroundImage pseudo input that only non-Blink Edge supports. That's bug 437554

The other missing feature people come across is related to feImage only i.e. it doesn't work on fragments - bug 455986

Generally everything else is implemented modulo minor bugs.

The patch contains a reftest: https://searchfox.org/mozilla-central/source/layout/reftests/svg/filters/feComposite-operator-lighter.svg

https://developer.mozilla.org/en-US/docs/Web/API/SVGFECompositeElement doe not seem to have a lighter for operator row in the table.

Flags: needinfo?(longsonr)

Thanks Robert,

  1. So you are saying that SVGFECompositeElement IS affected by this change and that we need to add "lighter for operator row?
  2. I'd like to fix up the BCD table for feComposite with whatever you "know". Please confirm:
    • For Firefox support for k1, k2, k3, k4 should be "yes" right?
    • For Chrome the FF test case for lighter works, which tells me that this should be "true". On the other hand this bug seems to indicate it is not fixed. Thoughts?
    • Opera indicates not supported, but that browser is now dependent on Blink too. Tested the lighter for testcase and that works on Opera and Opera Mobile. So my thinking is that Opera supports feComposite too
      • Webview for Android, Chrome for Android and Samsung Internet all have "no" for lighter for.
    • Safari and Safair for iOS uses Webkit. Do you know if that supports FeComposite, and if so, if it supports lighter for operator?

The other missing feature people come across is related to feImage only i.e. it doesn't work on fragments - bug 455986

OK, so presumably that is something to document as a note against https://developer.mozilla.org/en-US/docs/Web/SVG/Element/feImage#browser_compatibility and applies to both the href and xlink:href attributes?

I'm not sure what is meant by a fragment. I thought it meant something referenced using a # that is defined elsewhere in the SVG. But this uses that and seems to work?

<svg width="250" height="250" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink"> 
        <rect id="Img" width="100%" height="80%"  stroke="black" fill="gold" /> 
  
        <filter id="id_2"  primitiveUnits="objectBoundingBox"> 
            <feImage xlink:href="#Img" x="25%"  y="30%" width="50%" height="50%"  result="waves" /> 
            <feComposite operator="atop"  in="waves" in2="SourceAlpha" /> 
        </filter> 
  
        <g> 
            <rect x="1" y="1" width="300" height="200" fill="red"  stroke="blue" /> 
            <rect x="50" y="25" width="150" height="150"  filter="url(#id_2)" /> 
        </g> 
    </svg>
Flags: needinfo?(longsonr)

(In reply to Hamish Willee from comment #12)

Thanks Robert,

  1. So you are saying that SVGFECompositeElement IS affected by this change and that we need to add "lighter for operator row?

That would make sense, yes.

  1. I'd like to fix up the BCD table for feComposite with whatever you "know". Please confirm:
    • For Firefox support for k1, k2, k3, k4 should be "yes" right?

Everyone supports that and always has.

The specification changed after Chrome implemented lighter. We implement the latest specification, Chrome implement the earlier version. The new version makes it easier to set lighter in javascript but when you do use lighter in a supported way the effect is the same. This specification change is documented here: https://github.com/w3c/svgwg/issues/424

  • Opera indicates not supported, but that browser is now dependent on Blink too. Tested the lighter for testcase and that works on Opera and Opera Mobile. So my thinking is that Opera supports feComposite too

Opera uses Blink underneath so if Chrome supports lighter then so does Opera.

 -   Webview for Android, Chrome for Android and Samsung Internet all have "no" for lighter for.

I don't know you'd have to test but I imagine not.

  • Safari and Safair for iOS uses Webkit. Do you know if that supports FeComposite, and if so, if it supports lighter for operator?

Safari supports feComposite. The safari on my lapop (14) also supports lighter. I don't know what version ligher was introduced though.

I'm not sure what is meant by a fragment. I thought it meant something referenced using a # that is defined elsewhere in the SVG. But this uses that and seems to work?

<svg width="250" height="250" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink"> 
        <rect id="Img" width="100%" height="80%"  stroke="black" fill="gold" /> 
  
        <filter id="id_2"  primitiveUnits="objectBoundingBox"> 
            <feImage xlink:href="#Img" x="25%"  y="30%" width="50%" height="50%"  result="waves" /> 
            <feComposite operator="atop"  in="waves" in2="SourceAlpha" /> 
        </filter> 
  
        <g> 
            <rect x="1" y="1" width="300" height="200" fill="red"  stroke="blue" /> 
            <rect x="50" y="25" width="150" height="150"  filter="url(#id_2)" /> 
        </g> 
    </svg>

The above is not something that works in Firefox but it does in Chome. #img points to a fragment i.e. something that isn't a complete document.

Flags: needinfo?(longsonr)

Thanks very much! Regarding the FEImage, I'll add a note for this re BCD (and have added to the other issue). But since I'm here, BCD for FeImage indicates Null for preserveAspectRatio attribute on all browsers. Do we support it on feImage. Do you know if chromium and webkit do?

Flags: needinfo?(longsonr)

We support preserveAspectRatio on feImage. I imagine Chrome and Safari probably do too.

Flags: needinfo?(longsonr)

FYI Documentation for this is now complete - details can be found here: https://github.com/mdn/content/issues/1727 - main changes being to

Thanks for all your help Robert - really appreciate it.

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: