Open Bug 1698766 Opened 3 years ago Updated 3 months ago

Sub-optimal picture cache slicing on dobreprogramy.pl causes extra compositing of Color tiles

Categories

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

defect

Tracking

()

People

(Reporter: jnicol, Assigned: jnicol)

References

(Blocks 2 open bugs)

Details

While investigating slow performance on dobreprogramy.pl for bug 1697663 I noticed a lot of time was spent in the composite shader. This lead to implementing bug 1697899 to speed up the composite shader. However, I have also noticed that we are doing a poor job of choosing picture cache slices, which is leading to more pixels being composited than we'd like.

This occurs on desktop as well as android, in fact it might be even worse on desktop.

The main issue appears to be that we have a full screenful of Color tiles on the bottom slice. These aren't getting occluded, so we end up compositing an extra full screen of color. Then there is a scrollable slice above it which is non-opaque. The black fixed-position header bar does not get its own slice, so the top tiles of the scrollable slice constantly invalidate as you scroll. Occasionally I also see extra slices pop up and I have no idea where they come from.

On Android if you scroll far enough down the page then the scrollable slice becomes opaque and the extra slice of color tiles disappears. But this doesn't occur on desktop.

Glenn, do you have any idea how we can improve the slicing on this page?

Another thought is that perhaps we can skip using the composite shader for the Color tiles on the background slice, and rely on the main framebuffer's clear color instead?

Flags: needinfo?(gwatson)

From playing around in the devtools, there appears to be a border-top-left-radius and border-top-right-radius property on the <div id="slide-menu"> item. Removing those properties makes the scrollable slice opaque and removes the background slice full of color tiles.

You might like to try with the WIP patch applied in https://bugzilla.mozilla.org/show_bug.cgi?id=1689746#c12 and see if that helps invalidation related to the fixed position navbar, just to confirm if that's what causes no slice to be created there.

It seems like there's a couple of options here we could look into:

  • See if those border-radii are causing us not to make the slice opaque for a silly / simple reason that we could improve, so we detect as opaque and occlude tiles below.
  • I think implementing the detection of a full color first cache slice and collapsing that into the clear is probably doable without too much complexity.
Flags: needinfo?(gwatson)

I've had a chance to look at this in a wee bit more depth.

For the header bar, it's actually position: sticky rather than position: fixed. Is it expected that sticky items do not get their own slice? Changing it to fixed gives it it's own slice and avoids the invalidation.

The border radii mean that some pixels outside the curve need to be transparent, preventing us making the slice opaque and occluding the tiles below. I'm not sure if there's any way around that in general? Also, on desktop (or just wider screens?) this element doesn't take up the full width of this page, so even without the border radii we hit this issue.

However, on this page the background color of the element with the border radii is white, and the background color beneath it is also white. So it's silly here. I think the root cause of our issue comes from the fact that the root html node of the document has background: white; background-attachment: fixed;.

Fixed position backgrounds clearly get their own slice, and therefore the scrollable content above it gets its own slice. Due to the scrollable layer not taking up the full width, or due to the border radii on mobile, the scrollable slice cannot be opaque.

However, do fixed position solid color backgrounds actually need their own slice? Clearly images and gradients do, but for colours can we merge the colour in to the scrollable slice in some way?

Flags: needinfo?(gwatson)

I don't think sticky items currently get their own slice - but they should! It should be a fairly simple change to make, and it makes sense to me to do this.

We could probably detect a full screen size (or at least tile covering) fixed color background, and promote that in to the clear color for a tile (we already do this for simple cases, e.g. if the content in a tile contains a backdrop color primitive covering it, we use that for the tile clear color). I wonder if it might make sense to use the new sub-slice functionality I recently added for non-opaque compositor surfaces to handle these cases - I will ponder this a bit more.

Flags: needinfo?(gwatson)
See Also: → 1886739
You need to log in before you can comment on or make changes to this bug.