1601865 - Too many "horizontal scrollbar"-sized surfaces on about:config

Status: RESOLVED FIXED

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

Description

[Markus Stange [:mstange]]

Attachment: Screen Shot 2019-12-06 at 12.36.47 AM.png

In the screenshot, which was taken with gfx.webrender.debug.picture-caching = true, you can see that there are quite a number of 512x16 sized surfaces in places where they shouldn't be: some are in the chrome (active tab + some in the toolbar), and some make up the vertical scrollbar.

This is happening on macOS with overlay scrollbars enabled, and seemingly only while the scrollbars are showing, and I've also only seen it on about:config so far.

I'm guessing that this is a regression from bug 1599327.

Comment 1

[Bert Peers [:bpeers]]

That's pretty strange. Thanks for logging that. It seems related to Bug 1601924. I don't have a mac, but looking at talos/tests/scroll/tiled-fixed.html as a test case, I can see the scrollbar flag being set on primitives of 3000x2000, ie the entire viewport. I'm trying to figure out if layout/drawlists are setting PrimitiveFlags::IS_SCROLLBAR_CONTAINER on primitives where it shouldn't be, or if something got lost in translation.

Comment 2

[Bert Peers [:bpeers]]

In testing/talos/talos/tests/scroll/tiled-fixed.html the problem seems to go away if I remove background-attachment: fixed;.
Perhaps some kind of stacking context propagates primitive flags, turning the entire parent into a "scrollbar"?

Comment 3

[Bert Peers [:bpeers]]

Here's my analysis of Bug 1601924, hoping that this bug is the same. Basically, our assumption that scrollbar clusters go into a slice all by themselves is not holding up.

In talos/talos/tests/scroll/tiled-fixed.html the background-attachment: fixed will cause two new clusters to appear after the scrollbar cluster, and neither of them have the CREATE_PICTURE_CACHE_PRE or IS_CLEAR_PRIMITIVE flags set.
Their contents are PushClipChain for the first and Image, PopClipChain for the latter cluster. The Image is some giant full screen thing, so by the end of PicturePrimitive post_update, the surface has union()ed itself to a huge rectangle, which goes into self.estimated_local_rect and on to tile_cache.pre_update. Hence we have a IS_SCROLLBAR slice of 4Kx3K size, wider than tall, so it's chopped up into 512x16 strips.

Without background-attachment: fixed the huge image still shows up, but it's now in the next cluster which does have CREATE_PICTURE_CACHE_PRE set.

So now I don't know enough context on where the actual bug is. It could be a DisplayList bug, where CREATE_PICTURE_CACHE_PRE should be set on whatever comes after a scrollbar, or perhaps CREATE_PICTURE_CACHE_POST should always be set on a cluster that is also SCROLLBAR_CONTAINER. Along these lines, I can "fix" the bug by doing this:

create_slice |= cluster.flags.intersects(
    ClusterFlags::CREATE_PICTURE_CACHE_POST | ClusterFlags::IS_CLEAR_PRIMITIVE | ClusterFlags::SCROLLBAR_CONTAINER
)

but who knows what might break :P

On the other hand if there is a reason why those subsequent clusters must go into the same slice as the scrollbar, then we might have to rethink the optimization :/

Markus or Glenn, if you have some ideas on what is the right answer (assuming the above makes sense), please comment :) Thanks!

Comment 4

[Bert Peers [:bpeers]]

Attachment: cluster_compare.png

The extra elements that get dragged in to the slice when using a fixed background (right) versus without it (left).

Comment 5

[Bert Peers [:bpeers]]

Assigning to nobody pending further investigation. Issue is "resolved" for now as the original patch is backed out.

Comment 6

[BugBot [:suhaib / :marco/ :calixte]]

The priority flag is not set for this bug.
:jbonisteel, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 7

[Glenn Watson [:gw]]

Attachment: Bug 1601865 - Fix scrollbar cache slices having extra primitives.

There's two potential cases handled by this patch:

(1) A scrollbar container followed by another scrollbar container.
In this case, we need to ensure these are placed into separate
clusters, even though the cluster flags otherwise match, to
ensure that slice creation will see the two clusters.

(2) If a fixed position scroll root trails a scrollbar container.
In this case, ensure that a scrollbar contains marks the
cluster flags to create a slice straight after the scrollbar,
to avoid other primitives with the same scroll root sneaking
into the scrollbar container.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e1a4153cfb39
Fix scrollbar cache slices having extra primitives. r=nical

Comment 9

[Noemi Erli[:noemi_erli]]

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