Consider making picture caching required
Categories
(Core :: Graphics: WebRender, task, P3)
Tracking
()
Tracking | Status | |
---|---|---|
firefox83 | --- | fixed |
People
(Reporter: gw, Assigned: gw)
Details
Attachments
(1 file)
(deleted),
text/x-phabricator-request
|
Details |
Should picture caching become required?
There are only two scenarios in which picture caching is disabled:
- During pinch-zoom on platforms without native OS compositor active.
- As a debug setting.
Picture caching is required even during pinch-zoom on platforms using an OS compositor (DC, CA, Wayland) since we must draw into OS surfaces. It is a performance win to disable picture caching during pinch-zoom on platforms with no OS compositor (e.g. android), however I'm not sure it's worth the extra complexity for that single use case (recent android versions do have a platform compositor API that we can take advantage of in the future, which will further reduce places the non-picture-caching path is active).
If we do require picture caching to be active in all cases, we get:
- Remove an extra code path that requires maintenance and testing.
- Dirty rects and partial present are always available, even during pinch-zoom.
- Simplifies much of the CPU code that deals with 'compositor' clips (a performance win + code simplification)
- Simplifies how we calculate clipped regions of child surfaces (performance win + code simplification)
- Simplifies parts of the partial present handling renderer code paths.
- Simplifies some upcoming optimization work to retain more state (e.g. tile assignments) between scenes / frames.
One scenario where it was previously beneficial to disable picture caching was for full-screen video / canvas. However, now that we promote these to compositor surfaces, there's no penalty to having picture caching enabled in these cases.
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Updated•4 years ago
|
Comment 1•4 years ago
|
||
I think I'm fine with this from an android perspective. Zooming on some devices seems to be either
a) fine, even with picture caching enabled, or
b) poor, even with picture caching disabled.
So I don't think it makes a huge difference. Fewer code paths is a good thing, and I suspect it would fix bug 1668804
Updated•4 years ago
|
Comment 2•4 years ago
|
||
Sounds good to me. I think regressing Android zooming perf would be the only concern, so if that's not an issue (according to comment 1), great!
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Comment 3•4 years ago
|
||
This patch removes the public API and high level logic for
disabling picture caching for debugging and pinch-zoom in
some cases.
Follow up patches will remove and simplify the internal parts
of WR that remain to handle the disabled picture caching
code path.
Comment 4•4 years ago
|
||
I turn off picture caching to get consistent gpu times when looking at slow sites. Glenn suggested a better alternative is to have debug option to consider every tile invalid every frame and I agree that would be better.
Updated•4 years ago
|
Comment 6•4 years ago
|
||
bugherder |
Description
•