Implement some subset of SVG filters for WebRender

RESOLVED FIXED in Firefox 65

Status

()

enhancement
P2
normal
RESOLVED FIXED
8 months ago
5 months ago

People

(Reporter: mstange, Assigned: nical)

Tracking

(Blocks 2 bugs)

Trunk
mozilla65
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox63 wontfix, firefox64 wontfix, firefox65 fixed)

Details

Attachments

(2 attachments)

(Reporter)

Description

8 months ago
With WebRender we currently support all CSS filters, but no SVG filters. However, there are many SVG filter primitives that we *could* support with the current WR functionality. For example, we support
  filter: blur(2px);
but we do not support
  filter: url(#blur2px);
<filter id="blur2px" ...>
  <feGaussianBlur .../>
</filter>

It shouldn't be too hard for us to handle SVG filters as follows:
 1. Build the FilterDescription for the filter.
 2. Iterate through it and detecting filters that WR supports.
 3. Convert those to WR filters, or if we encounter an unsupported filter, go down the old filter painting path.

Special attention will need to be paid to clips, "primitive subregions". Primitive subregions act as a post-filter clip, per primitive. WR doesn't allow us to insert clips into the filter list, but it supports setting a clip on the stacking context that we create for the filter. So if the "primitive subregion"-generated clips can be expressed as just one clip rect that gets applied to the result, we should convert it to a clip on the stacking context, and otherwise treat the filter as not supported.
(Reporter)

Comment 1

8 months ago
The other thing that needs to be paid attention to is color models: SVG filters have, per primitive, the ability to specify whether that primitive should apply in sRGB or linearRGB space. We'd either need to add support to WR to convert between the two, or bail out for any filters that don't use sRGB everywhere.
Priority: P2 → P3
This should be an easy thing for someone to get started on.
Priority: P3 → P2
(Assignee)

Updated

7 months ago
Assignee: nobody → nical.bugzilla
QA Contact: mreavy
QA Contact: mreavy
(Assignee)

Comment 3

7 months ago
I have a prototype that kinda works with sRGB filters (I haven't looked into clipping correctly yet). However the svg spec mandates that the default color space for filter is linear which means that unless the author opts into sRGB, we don't hit the happy path. I looked for svg filter examples and they rarely specify the color space.

Relevant spec: https://www.w3.org/TR/SVG11/painting.html#ColorInterpolationProperties

So I guess we won't get any real benefit until WebRender has some from of support for linear color space filters.
(Assignee)

Comment 5

7 months ago
I added linear/srgb conversion filters in https://github.com/servo/webrender/pull/3182
(Assignee)

Comment 6

7 months ago
This try push has the webrender parts plus the extra gecko glue to support linear space filters: https://treeherder.mozilla.org/#/jobs?repo=try&revision=c11b87b9cee080d89cf9b0edaba26c5d4acf8d14
(Assignee)

Comment 7

6 months ago
I think that my patch isn't quite correct with respect to applying scaling to the filters. The filter code has the concept of filter-space which looks equivalent to the LayoutDevice space with an extra scaling applied. This scale is extracted from the gfxContext's current transform.

My patch currently doesn't apply this scale. Since we don't have a gfxContext I'm guessing that I can use StackingContextHelper::GetInheritedScale instead but that's really a wild guess.
(In reply to Nicolas Silva [:nical] from comment #7)
> My patch currently doesn't apply this scale. Since we don't have a
> gfxContext I'm guessing that I can use
> StackingContextHelper::GetInheritedScale instead but that's really a wild
> guess.

That's the scale that we end up passing to the fallback painting code:

  https://searchfox.org/mozilla-central/rev/a7f4d3ba4fbfe3efbde832869f1d672fce7122f6/gfx/layers/wr/WebRenderCommandBuilder.cpp#1845
(Assignee)

Comment 9

6 months ago
> That's the scale that we end up passing to the fallback painting code:

Yeah, this scale is there to be able to go from layout space to a space that aligns with the pixel grid (blob images being rasterized in device space -ish). The more I think about it the more I convince myself that in the case of passing filters to webrender we want to stay in layout space since webrender already internally tracks transforms and deals with rasterizing things at the right resolution. So the nsFilterInstance code in the case of WebRender shouldn't need to work in raster space and stick to layout space which would mean I can keep just passing an identity matrix like the patch currently does.

FWIW I tried passing the inherited scale and it doesn't change the reftest results at all: https://treeherder.mozilla.org/#/jobs?repo=try&selectedJob=207506345&revision=1883ef1bd4fa9f0e8e2e9e962691b642a12c322f
I'll stick to not passing the and hope that my assumption that layout space and filter space are the same for the purposes of WebRender.

Most of the remaining reftest failures are unexpected passes and some fuzzable differences, except for /layout/reftests/bugs/1059498-3.html. I think that the latter is caused by the incorrect ordering of filters and transforms on wr stacking contexts which is a known bug, so I think we can mark it as failing until the other bug is fixed.
(Assignee)

Comment 11

6 months ago
Here is the try push for latest patches in phabricator: https://treeherder.mozilla.org/#/jobs?repo=try&revision=1c2e05b88e6b639672326b14d5baa0b7efa4539a
(Assignee)

Comment 12

6 months ago
I rebased everything, this try push has the rebase + patch from bug 1468183 (color matrix filter).

Comment 13

6 months ago
Pushed by nsilva@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d6df6b97822a
Try to express SVG filters as CSS filters when possible. r=mstange
https://hg.mozilla.org/integration/mozilla-inbound/rev/eac3a6a42941
Adjust reftest expectations. r=mstange

Comment 14

6 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/d6df6b97822a
https://hg.mozilla.org/mozilla-central/rev/eac3a6a42941
Status: NEW → RESOLVED
Last Resolved: 6 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla65
You need to log in before you can comment on or make changes to this bug.