1572340 - In some cases, picture caching fails (invalidates too much) on some tiles due to clips not being filtered.

Status: RESOLVED WONTFIX

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

Description

[Glenn Watson [:gw]]

Occurs only on some pages. Still need to investigate to find out what's going on, but opening this as a placeholder to add notes.

Comment 1

[Glenn Watson [:gw]]

This is because the clip_rect of the scroll frame inside an iframe is sometimes 1 pixel higher than the clip_rect of the iframe display item, which trips up the WR logic that tries to filter out irrelevant clips.

I suspect this is a bug in Gecko, but it might be simplest to work around it in WR by including the iframe clip rect.

Comment 2

[Glenn Watson [:gw]]

An example of this occurring is on https://news.ycombinator.com/item?id=20640148.

Reproducing it may well depend on a number of factors such as window size, dpi ratio etc.

In the case I'm looking at, the height of the clip_rect supplied to the Iframe display item is 1637.0, while the height of the content_rect of the scroll frame that gets added is 1638.0.

This trips up the clip filtering logic in WR, since it can't filter out that (fixed position) iframe clip, since it does have an effect, clipping out the last row of pixels in the scroll frame that is being cached.

I suspect this is probably a Gecko bug, but I don't know much of the Gecko side. Jeff or Alexis, do you have much knowledge of this part of the Gecko code, or know who might be a good person to ask about it?

I'm going to ponder overnight if we can just work around this in WR, but if it is a bug in Gecko, we should probably fix it there. Alternatively, it'd be good to know if it's deliberate and/or there's a good reason for it to be this way.

Comment 3

[Aria Beingessner [:Gankra]]

Yeah I have no clue about this viewport stuff, I would just end up asking botond about it.

Comment 4

[Glenn Watson [:gw]]

I think this is a Gecko bug.

It appears that whenever layout.css.devPixelsPerPx is fractional, the layout rect and the clip rect of the iframe display item may be off by 1px in size. This trips up the clip filtering in WR, which makes picture caching invalidate much more than it needs to.

It seems like the problem is somewhere around https://searchfox.org/mozilla-central/rev/9ae20497229225cb3fa729a787791f424ff8087b/gfx/webrender_bindings/WebRenderAPI.cpp#1130. The results of rounding the bounds rect to a layout rect are different than the results of IntersectLayoutRect which is called by MergeClipLeaf.

It'd be good to get this investigated / resolved, since it means that any time layout.css.devPixelsPerPx is not an integer, picture caching ends up doing much more work than required (due to extra invalidations every frame). Andrew, any chance you have some time available to look into this?

Comment 5

[Andrew Osmond [:aosmond] (he/him)]

Certainly my massive snapping rewrite in the works (just shadows holding it up now) will have an impact on this. I removed the rounding in gecko which will allow us to snap properly and likely avoid these sorts of 1px issues. There are a number I've managed to fix as part of this work. I will test and report back.

Comment 6

[Andrew Osmond [:aosmond] (he/him)]

We may be able to resolve temporarily by snapping an iframe's display size using the StackingContextHelper::mSnappingSurfaceTransform, assuming it is axis aligned; layout size * mSnappingSurfaceTransform -> round -> / mSnappingSurfaceTransform => true layout size of an iframe. That would probably get the correct result you want most of the time although it might poorly interact with everything else snapping differently.

Comment 7

[Glenn Watson [:gw]]

What causes the issue for WR is that the iframe bounds are off-by-one pixels from the iframe clip rect.

Is it possible to make them consistent in the short term by applying the same logic to both to get both the clip rect and bounds?

That wouldn't necessarily be the correct snapped value, but it would be consistent which would fix this bug and avoid all the extra invalidation work that WR is currently doing in these cases.

Feel free to ping me in IRC if easier to discuss synchronously.

Comment 8

[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 9

[Glenn Watson [:gw]]

Although this is not fixed in Gecko, it's no longer causing a problem since WR picture caching now filters clips in a different way.