1209964 - Async adjustment of ImageLayers gets confused by ImageLayer transform (fixed background-image jiggles)

Status: RESOLVED FIXED

(Core :: Panning and Zooming, defect)

Description

[Markus Stange [:mstange]]

Attachment: testcase

STR:
 1. Load the testcase.
 2. Ensure the image is completely visible (not clipped), resize your window if necessary.
 3. Scroll.

The image should stay fixed, but it jiggles up and down.

The 256x256 image gets turned into an ImageLayer which has a transform to scale it up to 512x512 CSS pixels, as required by the background-size value.

Comment 1

[Botond Ballo [:botond]]

The problem is with the calculations inside AlignedFixedAndStickyLayers(). They assume that the calculated cumulative transforms (|oldCumulativeTransform| and |newCumulativeTransform|) apply on top of the fixed layer's own transform, but in the case where the fixed layer is the same as the subtree root (as here), the cumulative transforms already include the layer's own transform.

Comment 2

[Botond Ballo [:botond]]

(In reply to Botond Ballo [:botond] from comment #1)
> but in the case where the fixed layer is the
> same as the subtree root (as here), the cumulative transforms already
> include the layer's own transform.

(because they are part of the incoming |aPreviousTransformForRoot| and |aCurrentTransformForRoot|).

Comment 3

[Botond Ballo [:botond]]

I took this opportunity to rework the code that calculates |translation| in AlignFixedAndStickyLayers(). It still does the same thing (modulo the fix for this bug), but I think it's expressed a bit more clearly now.

Comment 4

[Botond Ballo [:botond]]

Attachment: MozReview Request: Bug 1209964 - In AlignFixedAndStickyLayers(), properly handle the case where a fixed or sticky layer is its own subtree root and has a local transform. r=kats

Bug 1209964 - In AlignFixedAndStickyLayers(), properly handle the case where a fixed or sticky layer is its own subtree root and has a local transform. r=kats

Comment 5

[Botond Ballo [:botond]]

Attachment: MozReview Request: Bug 1209964 - Reftest. r=mstange

Bug 1209964 - Reftest. r=mstange

Comment 7

[Botond Ballo [:botond]]

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

Comment 8

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

Comment on attachment 8669244 [details]
MozReview Request: Bug 1209964 - In AlignFixedAndStickyLayers(), properly handle the case where a fixed or sticky layer is its own subtree root and has a local transform. r=kats

https://reviewboard.mozilla.org/r/21173/#review19059

Thanks! This code is much easier to follow than the old code (which I'm not ashamed to admit I fall asleep trying to understand).

::: gfx/layers/composite/AsyncCompositionManager.cpp:401
(Diff revision 1)
> -
> +  // Offset the layer's anchor point to make sure to make sure fixed position

"to make sure" is repeated

Comment 9

[Markus Stange [:mstange]]

Comment on attachment 8669245 [details]
MozReview Request: Bug 1209964 - Reftest. r=mstange

https://reviewboard.mozilla.org/r/21175/#review19103

Comment 10

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d6d8b1f21461
https://hg.mozilla.org/integration/mozilla-inbound/rev/5b5ebeeb2979

Comment 11

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/6899c815160c

Comment 12

[Botond Ballo [:botond]]

(In reply to Pulsebot from comment #11)
> https://hg.mozilla.org/integration/mozilla-inbound/rev/6899c815160c

This was a follow-up to fuzz the reftest on B2G, where it was failing due to some one-pixel differences in the enlarged image.

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/d6d8b1f21461
https://hg.mozilla.org/mozilla-central/rev/5b5ebeeb2979
https://hg.mozilla.org/mozilla-central/rev/6899c815160c