1501195 - Snapping problem (and a jiggling image) on postbank.de

Status: RESOLVED FIXED

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

Description

[Darkspirit]

Attachment: 2018-10-23_09-57-59.mp4

Screencast: Left=last good, right=first bad.

bad = There is a flickering line on the left side of a growing image. (The image was already jiggling before.)

mozregression --good 2018-06-01 --bad 2018-10-22 --pref gfx.webrender.all:true -a https://www.postbank.de/privatkunden/girokonten.html
> 12:34.36 INFO: Last good revision: 10b2b242b9bdf9daa261c66f269f622d51cb6a12
> 12:34.36 INFO: First bad revision: f5063652dacb667a451348c37c983cf962eaf324
> 12:34.36 INFO: Pushlog:
> https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=10b2b242b9bdf9daa261c66f269f622d51cb6a12&tochange=f5063652dacb667a451348c37c983cf962eaf324

> f5063652dacb	Jeff Muizelaar — Bug 1490640. Mark intermittent tests as random.
> 969f3b484bbb	Jeff Muizelaar — Bug 1490640. Update webrender to 70edb5f8a75ea1e1440ba7984cc42df9eb05ae69

Regression range: https://github.com/servo/webrender/compare/02f14d0f333ef125d1abff7b1146039a0ba75f43...70edb5f8a75ea1e1440ba7984cc42df9eb05ae69

Comment 1

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

This doesn't just happen when it loads. After it finishes, I can find zoom levels where it contains a bad line on either the left and/or the right side.

Comment 2

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

Attachment: Bug 1501195 - Fix snapping issues caused by translation only transforms.

This is a reduced test case distilled from the original page where the
problem was first observed.

Comment 3

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

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=8b6e5a3713e6dd578f8ac74b96aec378ef747e06

Comment 4

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

The transform: translate3d(0px, 0px, 0px) seems to make us create an extra stacking context. This splits off the translate (since the content starts a few pixels down from the top edge and over from the left edge) and the scaling for the background image. However the clips are slightly different and we end up rendering an extra row/column of green pixels as a result. I was thinking this was because of the image snapping issue in bug 1453747 but I think the clip in question was not generated/related to the image itself (since it lives between the translate and scaling stacking contexts, and is applied to the latter in its clip/scroll info), so something else may have gone wrong.

Comment 5

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

From comment 4, I confirmed the patch for bug 1453747 has no impact on this.

Comment 6

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

The latest clipping changes fixed my new reftest, but I can still reproduce the issue on the original link while it is animating/scaling the image, if I adjust the initial zooming.

Comment 7

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

The updated reftest doesn't look the same yet (closer, but now the red rect is too small), but the original problem and the green rect spilling past the red rect in the upper left corner in the reftest can be solved by rounding the bounds rect of the transform: https://searchfox.org/mozilla-central/rev/c43240cef5829b8a2dec118faff8a5e1fec6ae1b/gfx/layers/wr/StackingContextHelper.cpp#71

Comment 8

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

For a container frame, if there is no transform or the transform is only a translation, we appear to round the offset we get from the reference frame:

https://searchfox.org/mozilla-central/source/layout/painting/FrameLayerBuilder.cpp#6112-6135

If we make the same decision in nsDisplayTransform::CreateWebRenderCommands, this seems to fix this bug, as well as bug 1498962. Reftests seem unaffected if we do this:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=c541d386025b77f28e1b57f19196593a6f06f3f5

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/524a6613a2ce
Fix snapping issues caused by translation only transforms. r=jrmuizel

Comment 10

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/524a6613a2ce