On some pages, I see a lot of picture cache tiles being invalidated when there is no apparent visual change.

One of the causes is that clips are being interned with different IDs because they are being sent in new display lists with different image keys for the mask data.

STR:

1. Apply the attached patch - this logs out information every time a clip is interned.
2. Open https://reddit.com/r/rust
3. Wait for page load
4. Scroll down the page
5. Observe logs similar to below, where the same clip is being interned but with a different rasterized image key for the mask.

An example of logging for one clip (each interning of it occurs on subsequent frames):

ItemUid { uid: 1421 }: ClipItemKey { kind: ImageMask(RectangleKey { x: 513.0, y: 4972.0, w: 577.0, h: 261.0 }, ImageKey(IdNamespace(3), 159), false), spatial_node_index: SpatialNodeIndex(5) }

ItemUid { uid: 1434 }: ClipItemKey { kind: ImageMask(RectangleKey { x: 513.0, y: 4972.0, w: 577.0, h: 261.0 }, ImageKey(IdNamespace(3), 166), false), spatial_node_index: SpatialNodeIndex(5) }

ItemUid { uid: 1453 }: ClipItemKey { kind: ImageMask(RectangleKey { x: 513.0, y: 4972.0, w: 577.0, h: 261.0 }, ImageKey(IdNamespace(3), 173), false), spatial_node_index: SpatialNodeIndex(5) }

In each case, everything about the clip node is the same except for the image key, which WR interprets as new mask data, causing the tile to invalidate.

Each time a frame occurs where the clip masks are re-interned, 7 clip masks get updated:

ItemUid { uid: 1447 }: ClipItemKey { kind: ImageMask(RectangleKey { x: 513.0, y: 371.0, w: 582.0, h: 261.0 }, ImageKey(IdNamespace(3), 167), false), spatial_node_index: SpatialNodeIndex(5) }
ItemUid { uid: 1448 }: ClipItemKey { kind: ImageMask(RectangleKey { x: 513.0, y: 746.0, w: 537.0, h: 22.0 }, ImageKey(IdNamespace(3), 168), false), spatial_node_index: SpatialNodeIndex(5) }
ItemUid { uid: 1449 }: ClipItemKey { kind: ImageMask(RectangleKey { x: 513.0, y: 2463.0, w: 582.0, h: 250.0 }, ImageKey(IdNamespace(3), 169), false), spatial_node_index: SpatialNodeIndex(5) }
ItemUid { uid: 1450 }: ClipItemKey { kind: ImageMask(RectangleKey { x: 513.0, y: 2834.0, w: 582.0, h: 261.0 }, ImageKey(IdNamespace(3), 170), false), spatial_node_index: SpatialNodeIndex(5) }
ItemUid { uid: 1451 }: ClipItemKey { kind: ImageMask(RectangleKey { x: 513.0, y: 4004.0, w: 582.0, h: 261.0 }, ImageKey(IdNamespace(3), 171), false), spatial_node_index: SpatialNodeIndex(5) }
ItemUid { uid: 1452 }: ClipItemKey { kind: ImageMask(RectangleKey { x: 513.0, y: 4375.0, w: 582.0, h: 158.0 }, ImageKey(IdNamespace(3), 172), false), spatial_node_index: SpatialNodeIndex(5) }
ItemUid { uid: 1453 }: ClipItemKey { kind: ImageMask(RectangleKey { x: 513.0, y: 4972.0, w: 577.0, h: 261.0 }, ImageKey(IdNamespace(3), 173), false), spatial_node_index: SpatialNodeIndex(5) }

Nical, any suggestions about this?

Two things:

• (1) the image mask logic in WebRenderCommandBuilder.cpp doesn't have a code path to update the existing image key, which is inefficient for the resource cache (the atlas allocator has more work to do, and we end up keeping several versions of the mask in the cache if it doesn't get evicted right away. Also if only a portion of the mask changes we could use the dirty rect to upload less, but we miss this opportunity here). That doesn't explain why the masks are invalidated in the first place.
• (2) The mask is generated in WebRenderCommandBuilder::BuildWrMaskImage( which contains a branch that will invalidate under certain conditions. The one we are hitting on this reddit page is !itemRect.IsEqualInterior(maskData->mItemRect). I don't know this code (Jeff, halp!) so it's unclear why the position matters. I tried to replace it with itemRect.Size() != maskData->mItemRect.Size(), it doesn't seem to break the page and lets us invalidate a lot less, but that might not be correct.

Changing this itemRect.Size() != maskData->mItemRect.Size() seems reasonable to me. I think the original test was there to match how conservative we were before adding BuildWrMaskImage. Can you put up a patch?

The patch breaks layout/reftests/invalidation/filter-userspace-offset.svg https://treeherder.mozilla.org/#/jobs?repo=try&revision=3f340c846342c7329263165708a920cd6ea9835b

Attempt for pernosco trace: https://treeherder.mozilla.org/#/jobs?repo=try&revision=0d3044f767925770db9377c5f0e6f496ed7a8235

I didn't get a chance to debug this much but I did talk to mstange (the author of the test) about it. The test seems involve a foreignObject element that moves while its mask stays in the same position. Since I'm on PTO next week I'd recommend trying to figure something out with mstange.

There are some r+ patches which didn't land and no activity in this bug for 2 weeks.
:gw, could you have a look please?
For more information, please visit auto_nag documentation.

The patches for this bug that are useful will be Nical's.

Here's the pernosco trace: https://pernos.co/debug/Oikf4yEz6evm-xsL5G8J_Q/index.html

The filterUnits="userSpaceOnUse" is contributing to this not working. This property acts as a sort of position:fixed for masks, but we don't know about it in the WebRenderCommandBuilder code. It's not clear how the non-WebRender code handles this case. I'm going to look at that to hopefully get some inspiration.

The non-WebRender code does not use MaskLayers for this case. But perhaps we can force it to.

So non-WebRender has two different kinds of invalidation it does for MaskLayers:
CSSMaskLayerUserData and MaskLayerUserData

CSSMaskLayerUserData has the full bounds which explains how it manages to work.

When not using an active mask layer, FrameLayerBuilder avoids this problem by keeping things relative to the animated geometry root.

It's not clear what the best way for us to solve this is.

Should be looked at further by someone who knows Gecko well. Miko, want to take a look?

Maybe we can just solve this by instead of comparing items bounds, comparing item bounds relative to the animated geometry root. Does that seem practical Miko?

(In reply to Jeff Muizelaar [:jrmuizel] from comment #17)

Maybe we can just solve this by instead of comparing items bounds, comparing item bounds relative to the animated geometry root. Does that seem practical Miko?

I do not know enough about scrolling and masking to answer that. Markus probably knows better, he also added the reftest that was mentioned in comment 6.

Let me know if this is still an issue.