1612750 - Support force-disabling APZ properly on Android

Status: RESOLVED FIXED

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

Description

[Botond Ballo [:botond]]

The currently implementation of the "force-disable APZ" mechanism is to fall back to the main thread's scroll offset when deciding what scroll offset to use for composition.

This doesn't work on Android because the main thread's scroll offset is the layout viewport offset, and therefore this breaks pinch zooming.

I think what we need to do instead if use the main thread's view of the visual viewport offset at the time of the last paint.

Comment 1

[Botond Ballo [:botond]]

Bug 1519285 would make fixing this easier (but it's not a hard dependency).

Comment 2

[Botond Ballo [:botond]]

(In reply to Botond Ballo [:botond] [spotty responses until Feb 19] from comment #0)

I think what we need to do instead if use the main thread's view of the visual viewport offset at the time of the last paint.

I tried this, and it makes bug 1610960 better, but it's still not great; switching over from APZ enabled to APZ force-disabled still causes a jarring jump in the view as we jump from the async scroll offset to the main-thread's (visual) scroll offset. We may need to take some additional measures, like only having force-disablement take effect at the end of a gesture rather than in the middle of one.

Comment 3

[Botond Ballo [:botond]]

Given that bug 1612983 is changing AccessibleCaret to perform force-disabling in fewer scenarios (and in particular, not the in the original STR in bug 1610960), I think what I have so far should be good enough for now.

Comment 4

[Botond Ballo [:botond]]

Attachment: Bug 1612750 - Simplify a misleading calculation in AsyncPanZoomController::GetCurrentAsyncTransform(). r=tnikkel

Comment 5

[Botond Ballo [:botond]]

Attachment: Bug 1612750 - Move handling of IsApzForceDisabled() into the GetEffective*() functions where possible. r=tnikkel

As part of this change, IsApzForceDisabled() no longer affects async zoom.
This is deliberate, as we don't repaint at intermediate resolutions during
a pinch-zoom gesture, so the alternative (also the previous behaviour) is
that the view does not update until the conclusion of the pinch-zoom gesture,
which is arguably worse than any out-of-sync issues that force-disabling
aims to address.

Depends on D62090

Comment 6

[Botond Ballo [:botond]]

Attachment: Bug 1612750 - Have the main thread always populate FrameMetrics::mVisualViewportOffset. r=tnikkel

APZ may want to know what the main thread's view of the visual viewport offset
was at the time of the last paint even if the main thread does not want APZ
to scroll to that visual viewport offset.

Depends on D62091

Comment 7

[Botond Ballo [:botond]]

Attachment: Bug 1612750 - Use the main thread's visual viewport offset as the fallback scroll offset when APZ is force-disabled. r=tnikkel

This ensures that even if APZ is force-disabled, the permanent offset between
the visual and layout viewports is preserved during rendering.

Depends on D62093

Comment 8

[Botond Ballo [:botond]]

Patches are causing some gtest failures:

https://treeherder.mozilla.org/#/jobs?repo=try&tier=1&revision=fb20051751a76aaac1e6d275a7be80e0fb3d6cd9

Comment 9

[Botond Ballo [:botond]]

(In reply to Botond Ballo [:botond] [spotty responses until Feb 19] from comment #8)

Patches are causing some gtest failures:

Looks like this was just a test issue. This is looking better:

https://treeherder.mozilla.org/#/jobs?repo=try&tier=1&revision=dd11fa9d3b7d48ac6941cfe3b2540cad9d3d67ca

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b0b9c7a4a1d5
Simplify a misleading calculation in AsyncPanZoomController::GetCurrentAsyncTransform(). r=tnikkel
https://hg.mozilla.org/integration/autoland/rev/a18ca1103448
Move handling of IsApzForceDisabled() into the GetEffective*() functions where possible. r=tnikkel
https://hg.mozilla.org/integration/autoland/rev/31a3f433f328
Have the main thread always populate FrameMetrics::mVisualViewportOffset. r=tnikkel
https://hg.mozilla.org/integration/autoland/rev/fa22f9fce567
Use the main thread's visual viewport offset as the fallback scroll offset when APZ is force-disabled. r=tnikkel

Comment 11

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/b0b9c7a4a1d5
https://hg.mozilla.org/mozilla-central/rev/a18ca1103448
https://hg.mozilla.org/mozilla-central/rev/31a3f433f328
https://hg.mozilla.org/mozilla-central/rev/fa22f9fce567