Closed Bug 1477366 Opened Last year Closed 9 months ago

https://www.quantifiedplanet.org/ is slow because of slow blur in blob image

Categories

(Core :: Graphics: WebRender, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
mozilla66
Tracking Status
firefox66 --- affected
firefox67 --- affected
firefox68 --- affected

People

(Reporter: jrmuizel, Assigned: mattwoodrow, NeedInfo)

References

(Blocks 2 open bugs)

Details

Attachments

(6 files)

Blocks: wr-perf
Priority: -- → P2
Priority: P2 → P3
Need to see how bad this still is.
Flags: needinfo?(jmuizelaar)
This seems okish now.
Flags: needinfo?(jmuizelaar)
Priority: P3 → P4
This is still not great: https://perfht.ml/2SzFAO5. We could experiment with making sure our svg blurs always end up going through webrender.
Priority: P4 → P3
Can you elaborate on comment 3? Sounds like this would involve modifying the blob grouping code to not include svg blurs in blobs but instead split blobs that contain blurs into two (stuff before blur/stuff after blur), with the blur item going to WR directly. Is that right?
Flags: needinfo?(jmuizelaar)

Yes. That's what I meant.

Flags: needinfo?(jmuizelaar)
Assignee: nobody → matt.woodrow

I looked into one of the failures, feGaussianBlur-4.svg.

The issue is the coordinate conversion comparison here [1], as mentioned in [2].

Isn't the correct thing to do:

  • Take the input bounds, and convert them to post-primitive (not post filter chain) bounds using PostFilterExtentsForPrimitive.
  • Compare the post-filter primitive bounds for that primitive to the primitive subregion, decide if we'd need a clip to represent it, and fail if not.
  • Use the post-filter primitive bounds as the input bounds for the next primitive in the chain

Or

  • Implement a clipping filter primitive within WR.

Does that sound right Markus/nical?

I can probably do the gecko side version easily enough if that's what we want, and nobody else wants to.

Looks like SVG filters are fairly buggy right now, but we just don't have test coverage.

[1] https://searchfox.org/mozilla-central/rev/b4ebbe90ae4d0468fe6232bb4ce90699738c8125/layout/svg/nsFilterInstance.cpp#159
[2] https://phabricator.services.mozilla.com/D8085

Flags: needinfo?(nical.bugzilla)
Flags: needinfo?(mstange)

My head isn't quite wrapped around this anymore. I thought my intent was to make it conservative when the initial svg filter subset for webrender stuff landed, but I'm not sure I really understood the coordinate spaces at play so looks like I got it wrong.

Adding a clipping filter looks like it would enable more uses of the GPU for blurs so it has my vote. If you have cycles to do it, it'd be great!

Flags: needinfo?(nical.bugzilla)

Bug 1514384 changed the behaviour here, previously we clipped to the PrimitiveSubregion on the last primitive, and now we don't even do that.

The issue there was that the clip computed depended on the bounds of the content within, which can change (due to async animations/scrolling), and then the clip becomes incorrect.

As far as I can tell, this is true for all CSS PrimitiveSubregions, so we can't ever clip to them.

Do CSS filters ever compute a PrimtiveSubregion that we need to clip to? The SVG ones seem like they compute real clips (and don't seem to be dependent on the bounds of the inner content).

Markus pointed me to https://github.com/servo/webrender/issues/1880 where he has written up a proposal for a new filter API that covers the SVG use cases.

I'm concerned that trying to fix bugs in the current implementation will be tricky, and wasted work given that we need to rewrite most of the API to support a lot of the use cases.

In particular, the code to detect whether we need to clip to a given PrimitiveSubregion depends on the current content bounds, which we can't rely on. Fixing that would make the code return false for basically every case (except maybe a single filter, where we can use a clip on the result?).

Spending time on getting a few specific cases to work seems like a bad idea for the MVP, so I'm going to unblock this, and we should instead start on the API rewrite as soon as it can be prioritised.

Assignee: matt.woodrow → nobody
Blocks: stage-wr-next
No longer blocks: stage-wr-trains

I'm also concerned that we currently allow SVG filters on HTML content, and now that we're not clipping them at all, these are likely incorrect.

Should we disable SVG filters entirely for the MVP?

See Also: → 1520652

Bug 1520652 takes care of the immediate clipping issue here. Answering the remaining question from comment 10:

(In reply to Matt Woodrow (:mattwoodrow) from comment #10)

Do CSS filters ever compute a PrimtiveSubregion that we need to clip to? The SVG ones seem like they compute real clips (and don't seem to be dependent on the bounds of the inner content).

No, they don't. In the non-WebRender implementation of CSS filters, we compute a "no clip" primitive subregion for CSS filter "primitives" just because it was easier to take advantage of the existing SVG filter code that way. But that code isn't used in the WebRender path anyway. So CSS filters never need to be clipped.

Flags: needinfo?(mstange)
Pushed by mwoodrow@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/3d23cfc9df06
Always make SVG/CSS filters active within blobs when possible. r=jrmuizel
https://hg.mozilla.org/integration/autoland/rev/4e1eb0b0b422
Only support SVG filters with a single input with WebRender. r=mstange
https://hg.mozilla.org/integration/autoland/rev/719acee85b9a
Handle color space conversion for drop shadow filters with WebRender. r=mstange
https://hg.mozilla.org/integration/autoland/rev/f218594acb92
Fuzz some tests for differences between WR and the fallback path. r=mstange
Assignee: nobody → matt.woodrow

Hello,

I can still reproduce this issue using Fx 66.0.2 Build Id:20190326175229, Fx Beta 67.0b5 Build Id:2019032512512 and on Nightly 68.0a1 20190327175114 on Windows 10 x64.

Can you post a profile from a Nightly build? Make sure you include these threads "RenderBackend,Renderer,WebRender,Wr" in the custom threads settings of the profiler.

Flags: needinfo?(daniel.cicas)

I added the Gecko profile (using the Gecko profile add-on) this is the first time when I used this so if I forgot anything please needinfo me.

Flags: needinfo?(daniel.cicas) → needinfo?(jmuizelaar)

That profile shows us spending a bunch of time waiting on the GPU which looks like a different issue than the original one here. Can you file a new bug with that profile and information about your GPU and screen resolution?

Flags: needinfo?(jmuizelaar) → needinfo?(daniel.cicas)

I took a another look at this issue and it might be a mistake on my part. I have attached a new profile using a better GPU (Nvidia GTX 1050, screen resolution 1920x1080) as opposed to the previous profile that used ( NVIDIA GeForce 210, screen resolution 1920x1080).

Can you take a look at this new profile and see if this was the problem? Also I cant find any system requirements for Webrender maybe you can point me in the right direction for this.

Flags: needinfo?(daniel.cicas) → needinfo?(jmuizelaar)
You need to log in before you can comment on or make changes to this bug.