Disable picture caching when async zooming
Categories
(Core :: Graphics: WebRender, defect, P3)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox72 | --- | fixed |
People
(Reporter: jnicol, Assigned: jnicol)
References
Details
Attachments
(1 file)
When async zooming, picture caching isn't much use because every picture cache tile will be fully invalidated every frame. Zooming can on some frames require a lot of texture upload (due to re-rasterizing glyphs in a new raster space), so rendering in to a picture cache tile, then blitting that tile to the screen, seems like a bad use of precious memory bandwidth.
Note that currently disabling picture caching (by setting gfx.webrender.picture-caching to false) makes async zoom much slower because it breaks the raster space trick and we rasterize glyphs every frame (bug 1566069). Perhaps the solution here would "turn off" picture caching at a much lower level so that won't be an issue, or perhaps that needs fixed. Something to be aware of anyway.
Glenn, could you point me towards where the best place to "turn off" picture caching would be?
|
||
Comment 2•5 years ago
|
||
If feasible, the ideal place to disable picture caching when zooming is:
https://searchfox.org/mozilla-central/source/gfx/wr/webrender/src/picture.rs#3636
This is near the start of each frame where we determine from the 'requested' composite mode of a picture what the 'actual' composite mode of the picture is for this frame (so we can dynamically enable/disable picture caching at this point). We already use this to disable picture caching in the case where the transform of the picture cache root has a perspective transform.
One caveat - it's not common that we dynamically switch from picture caching on to off and vice versa, so this code path may not be that well tested / buggy. If you do encounter weird glitches between enabling and disabling picture caching, we should fix them!
|
|
||
Comment 3•5 years ago
|
||
For unrelated reasons, I was just testing something out with this code path today and realized it is broken - so I guess nothing currently relies on that working. I'll try to investigate it further tomorrow.
|
|
||
Comment 4•5 years ago
|
||
I think, instead, the best option will be to change the enable_picture_caching field in both Renderer and FrameBuilderConfig.
You'll have to plumb some code to do this (probably via setting the value in the FrameBuilderConfig based on whether zoom is enabled, and propagating that to the renderer in the Frame struct). It's probably also necessary to set the picture cache composite above (mentioned in the comments above) based on the value in FrameBuilderConfig.
I haven't tested that at all, but from a quick look at the code, that would be enough to allow you to dynamically enable / disable picture caching per-frame. I can probably take a look at implementing this tomorrow, unless you want to look at implementing that yourself?
|
||
Comment 5•5 years ago
|
||
Is there anything left we need do here or was what we landed in 1592810 enough?
|
Assignee | |
Comment 6•5 years ago
|
||
I believe I still need to hook the zooming code up to Glenn's changes. Will try to get round to that tomorrow!
|
|
Assignee | |
Comment 7•5 years ago
|
||
Whilst pinch zooming, every picture cache tile gets completely
invalidated every frame. It is therefore a waste of memory bandwidth
to render in to picture cache tiles then composite those to the
screen. This change dynamically disables picture caching for frames in
which we are pinch zooming. The exception is if we are using a native
compositor, in which case picture caching will remain enabled, because
it relies on picture caching to work, and does not waste memory
bandwidth.
Pushed by jnicol@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/dd0e1c0d6acc Disable picture caching whilst pinch-zooming. r=gw
|
||
Comment 9•5 years ago
|
||
| bugherder | ||
Description
•