Details

Reviewers

kvark
nical

Commits

rMOZILLACENTRAL96075609026a: Bug 1572646 - Detect and optimize picture cache tiles that are solid colors.

Bugzilla Bug ID

1572646

Summary

With this patch, tiles that are covered only by the opaque backdrop
primitive are detected and noted as solid colors.

Solid color tiles save memory and performance, because:

No texture slice is allocated as a render target for them.
No need to rasterize this tile.
Drawing the tile is done with the faster rectangle shader.

This already saves performance and GPU memory on quite a few
real world sites (esp. when running at 4k). However, the main
benefit of this will be once we enable picture caching on
multiple content slices and the UI layer. When this occurs, it's
important to avoid allocating tile buffers for all the solid
rectangle tiles that the UI layer typically contains.

Diff Detail

Repository

rMOZILLACENTRAL mozilla-central

Lint

Automatic diff as part of commit; lint not applicable.

Unit

Automatic diff as part of commit; unit tests not applicable.

Event Timeline

gw created this revision.Aug 9 2019, 7:51 AM

Herald added a project: Restricted Project. · View Herald TranscriptAug 9 2019, 7:51 AM

Harbormaster completed remote builds in B101726: Diff 144908.Aug 9 2019, 7:51 AM

phab-bot requested review of this revision.Aug 9 2019, 7:51 AM

phab-bot changed the visibility from "Custom Policy" to "Public (No Login Required)".

phab-bot changed the edit policy from "Custom Policy" to "Restricted Project (Project)".

phab-bot removed a project: Restricted Project.

Pending try looks good - a couple android failures, which I suspect to be emulator bugs (need to investigate though):
https://treeherder.mozilla.org/#/jobs?repo=try&revision=7a94affb795821a339b90139acb1e350f9af175a&selectedJob=260743541

Would it make sense to generalize this and have a notion of "single primitive tile"? I suspect that there are real-world sites where some picture cached tiles are fully covered by a large image.

In D41342#1246192, @nical wrote:

Would it make sense to generalize this and have a notion of "single primitive tile"? I suspect that there are real-world sites where some picture cached tiles are fully covered by a large image.

Yep, that is definitely something we should consider / handle in future. Right now, the TileSurface enum only handles a texture and a solid color, but it should be feasible to expand that to cover other simple tile types too.

gw updated this revision to Diff 145545.Aug 12 2019, 12:40 AM

Harbormaster completed remote builds in B102228: Diff 145545.Aug 12 2019, 12:40 AM

kvark accepted this revision.Aug 12 2019, 1:25 AM

kvark added inline comments.

gfx/wr/webrender/src/picture.rs
402	Where there are "null"-like values, it's too tempting to just use them. Since you are switching to a new enum, perhaps we could store it in `Option` or `Result` where appropriate instead of adding `Invalid` variant?
548	if there is only one opaque primitive, why does this imply a solid rect? could it be an opaque image or something else?
704	should the visibility mask become a member of `TileSurface::Texture`?
1053–1054	comment needs to be updated

This revision is now accepted and ready to land.Aug 12 2019, 1:25 AM

gw marked an inline comment as done.Aug 12 2019, 3:56 AM

gw added inline comments.

gfx/wr/webrender/src/picture.rs
402	Done.
548	We know in this case since only rectangle primitives currently set the opaque_rect field. I'll add a comment clarifying this and that it may change in future.
704	Yup, done.