1702859 - subframe scroll thumbs mispositioned with pinch zoom + webrender

Status: RESOLVED FIXED

(Core :: Panning and Zooming, defect, P2)

Description

[Timothy Nikkel (:tnikkel)]

Attachment: posh3.html

The async transform is most likely the source of the problem.

I checked older builds, the problem seems to exist as far back as I can reasonably test, before that desktop zooming isn't in good enough shape to test it seems.

Comment 1

[Timothy Nikkel (:tnikkel)]

Attachment: Screen Shot 2021-04-03 at 3.56.58 AM.png

Here's one example of the problem, I can scroll the thumbs all the way out of view.

Comment 4

[Botond Ballo [:botond]]

Raising priority as we are seeing users notice this.

Comment 5

[Timothy Nikkel (:tnikkel)]

The work we did with "transform to ancestor" (bug 1732600 and related), mostly the new understandings we got about how resolution works with webrender vs layers, might make this easier to debug/understand.

Comment 6

[Botond Ballo [:botond]]

The async transform of the scrollbars appears to be off by a factor equal to the resolution.

Comment 7

[Botond Ballo [:botond]]

I believe what's happening here is that ComputeTransformForScrollThumb() is computing a transform in the scrollbar's "layer space", that is, a coordinate space such that one unit of translation moves the scrollbar by one rendered pixel.

With Layers, this transform was used as the scrollbar layer's shadow transform. The scrollbar layer was also subject to the shadow transforms of ancestor layers, which in the case of a subframe scrollbar included the zoom container layer. However, the zoom container layer's effective transform scale was 1.0 (the zoom was cancelled out by a pre-scale, to reflect the fact that the zoom container's descendants are already rendered at a higher resolution to account for the zoom).

With WebRender, this transform is used as a transform property associated with the scrollbar's animation id. In this usage, subframe scrollbars are also subject to the zoom transform, but this time there is no pre-scale to cancel that out.

Therefore, for subframe scrollbars, the values expected by Layers and WebRender in this computed transform differ by a factor equal to the zoom level.

Comment 8

[Botond Ballo [:botond]]

Attachment: Bug 1702859 - Add a comment explaining why async-scrollbar-zoom-{1,2}.html are not enabled on desktop yet. r=tnikkel

Comment 9

[Botond Ballo [:botond]]

Attachment: Bug 1702859 - Make APZ scrollbar reftest names more descriptive and more consistent. r=tnikkel

Depends on D130534

Comment 10

[Botond Ballo [:botond]]

Attachment: Bug 1702859 - Add async scrolling versions of the zoomed root scrollbar tests. r=tnikkel

These already pass, just adding them for test coverage.

Depends on D130535

Comment 11

[Botond Ballo [:botond]]

Attachment: Bug 1702859 - Add subframe versions of the zoomed scrollbar async scroll tests. r=tnikkel

For these tests, the reference file also has the resolution (otherwise
the subframe scrollbar thicknesses wouldn't match), the only difference
is async scroll vs. sync scroll.

These tests fail without the fix for this bug.

Depends on D130536

Comment 12

[Botond Ballo [:botond]]

Attachment: Bug 1702859 - Fix async transform computation for subframe scrollbars. r=tnikkel

The computation included the resolution of enclosing scroll frames
in the scrollbar transform. This was correct with Layers (where
the resolution was "pushed down" to descendant layers such that
their layer space represented rendered pixels) but incorrect with
WebRender (where items with transform properties like a subframe
scrollbar are still subject to enclosing transform properties like
the zoom).

This makes the reftests added in the previous patch pass.

Depends on D130537

Comment 13

[Botond Ballo [:botond]]

Attachment: Bug 1702859 - Tweak fuzz values for new tests. r=tnikkel

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/eeea103db0ee
Add a comment explaining why async-scrollbar-zoom-{1,2}.html are not enabled on desktop yet. r=tnikkel
https://hg.mozilla.org/integration/autoland/rev/9fd7371a4616
Make APZ scrollbar reftest names more descriptive and more consistent. r=tnikkel
https://hg.mozilla.org/integration/autoland/rev/448061f3c607
Add async scrolling versions of the zoomed root scrollbar tests. r=tnikkel
https://hg.mozilla.org/integration/autoland/rev/eaa080c9e238
Add subframe versions of the zoomed scrollbar async scroll tests. r=tnikkel
https://hg.mozilla.org/integration/autoland/rev/3bb9cf3fa215
Fix async transform computation for subframe scrollbars. r=tnikkel
https://hg.mozilla.org/integration/autoland/rev/254807479c9d
Tweak fuzz values for new tests. r=tnikkel

Comment 15

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/eeea103db0ee
https://hg.mozilla.org/mozilla-central/rev/9fd7371a4616
https://hg.mozilla.org/mozilla-central/rev/448061f3c607
https://hg.mozilla.org/mozilla-central/rev/eaa080c9e238
https://hg.mozilla.org/mozilla-central/rev/3bb9cf3fa215
https://hg.mozilla.org/mozilla-central/rev/254807479c9d

Comment 16

[Timothy Nikkel (:tnikkel)]

(In reply to Botond Ballo [:botond] from comment #7)

I believe what's happening here is that ComputeTransformForScrollThumb() is computing a transform in the scrollbar's "layer space", that is, a coordinate space such that one unit of translation moves the scrollbar by one rendered pixel.

With Layers, this transform was used as the scrollbar layer's shadow transform. The scrollbar layer was also subject to the shadow transforms of ancestor layers, which in the case of a subframe scrollbar included the zoom container layer. However, the zoom container layer's effective transform scale was 1.0 (the zoom was cancelled out by a pre-scale, to reflect the fact that the zoom container's descendants are already rendered at a higher resolution to account for the zoom).

With WebRender, this transform is used as a transform property associated with the scrollbar's animation id. In this usage, subframe scrollbars are also subject to the zoom transform, but this time there is no pre-scale to cancel that out.

Therefore, for subframe scrollbars, the values expected by Layers and WebRender in this computed transform differ by a factor equal to the zoom level.

I missed this comment when reviewing, it would have been helpful! I need to remind myself to check the bug.