1039020 - Layerize background-attachment:fixed for all elements

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Matt Woodrow (:mattwoodrow)]

Attachment: Layerize background-attachment:fixed for desktop

Currently we don't do this because it's hard to support correctly when we have async scrolling. We can trivially enable it for platforms that don't have async scrolling though, for a good performance win. The linked URL is a good example of where this helps.

We also have the question of what to do for async scrolling (either now or in the future).

We currently don't detect these, so async scrolling moves them as if they weren't fixed, and then they jump back into place when we get main thread updates. It appears that Chrome on android renders them correctly when scrolling (unsure if it's async or not), and safari on iOS doesn't render them at all.

Some options:

* Keep things the way they are (broken but apparently that's not a big deal).

* Layerize them and don't change anything else. Should perform better (main thread updates will be cheaper) but we change to a different (and probably worse) sort of broken. We keep the backgrounds fixed in place, but we don't update the draw region so you end up with white gaps being exposed.

* Disable async scrolling when we detect fixed background images. Pretty simple correctness/performance trade-off.

* Implement proper APZ handling for background-attachment:fixed images. Easily the nicest option, but probably a bit of work.

Comment 1

[Matt Woodrow (:mattwoodrow)]

Attachment: Layerize background-attachment:fixed for desktop v2

Comment 2

[Kartikaya Gupta (email:[e-mail])]

How are background-attachment:fixed layers different from fixed-pos or stick-pos layers? Can we reuse that code to keep these layers in place while scrolling?

Comment 3

[Matt Woodrow (:mattwoodrow)]

The 'viewport' created by the element scrolls normally, and the background image is fixed in place behind it. So we need the layer for the background to be fixed (which works just fine using fixed-pos code), but the painted area of the layer (probably expressed via a clip rect) needs to be scrolled. The latter piece is what isn't implemented currently,

Comment 4

[Markus Stange [:mstange]]

Attachment: testcase with border radius

When I tried this last time, it led to painting errors when border-radius was used. I think you need to turn off layerization when that's the case, or add a way to build the right DisplayItemClip for the background clip area.

Comment 5

[Matt Woodrow (:mattwoodrow)]

Thanks Markus, good catch.

This turns out to be a huge hassle, and maybe we should just disable it when we have border-radius.


The problem is that nsDisplayBackgroundImage doesn't clip rounded corners using DisplayItemClip, it instead just handles them internally. This means that even when we layerize the background, we still don't get a mask layer.

When we scroll the clip scrolls with the page and moves relative to the background image. nsDisplayBackgroundImage::ComputeInvalidationRegion doesn't check for this and we fail to invalidate things that we need to.

I think the best way to handle this is to create the correct DisplayItemClip items for nsDisplayBackgroundImage/Color, and add a way to ask nsCSSRendering not to add a clip when drawing the item.

Unfortunately this still doesn't work correctly because of bug 1039926 and we still invalidate on scroll.

We could probably do a quicker fix by improving ComputeInvalidationRegion for backgrounds, or by making DLBI's clip comparison compute the real changed area in the presence of rounded rects (currently it gives up and takes the bounds union).

Comment 6

[Matt Woodrow (:mattwoodrow)]

With the dependent bugs all fixed, then we're down to only repainting the scrolled in areas with Markus' test case.

We could further improve this by getting the page background converted to a ColorLayer (we have a rounded rect clipped white background on top of the page white background which doesn't get optimized) and by getting the fixed background image converted to an ImageLayer (it uses tiling which we don't support with ImageLayer yet). That would get us to 0 repainting, which would be somewhat nice.

Comment 7

[Matt Woodrow (:mattwoodrow)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/bd4287c14070

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/bd4287c14070