1335958 - Simplify APZ overscroll physics

Status: VERIFIED FIXED

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

Description

[Botond Ballo [:botond]]

APZ currently supports an oscillation spring-based overscroll effect.

This was used by B2G, and now that we no longer support B2G, we don't need it anymore.

However, we will want an overscroll effect for macOS (tracked by bug 1124108). So rather than removing APZ's overscroll animation, I propose that we just modify its physics to be what we want for macOS.

Markus' WIP patch in bug 1124108 already does this, but I'd like to land the parts that concern the overscroll physics now, because it will make the refactoring we're planning to do in bug 1282245 easier. I will do so in this bug.

Comment 1

[Botond Ballo [:botond]]

Attachment: Bug 1335958 - Simplify APZ overscroll physics.

Replace the oscillating spring physics used for B2G with simpler physics
where we just snap back without oscillating, using an MSD model.

This is not currently used, but will be once we enable overscroll for
macOS.

Review commit: https://reviewboard.mozilla.org/r/108888/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/108888/

Comment 2

[Botond Ballo [:botond]]

One thing this patch does not do is add prefs for the various constants used by the new physics. I'll leave it to Markus to decide which constants it makes sense to add prefs for, in bug 1124108.

Comment 3

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

Comment on attachment 8832701 [details]
Bug 1335958 - Simplify APZ overscroll physics.

https://reviewboard.mozilla.org/r/108888/#review110306

Looks fairly straightforward. Nice to see lots of code deletion :)

::: gfx/layers/apz/src/Axis.h:285
(Diff revision 1)
> +  // The mass-spring-damper model for overscroll physics.
> +  AxisPhysicsMSDModel mMSDModel;
> +

nit: move this up so it's next to mOverscroll (i.e. group overscroll-related fields together)

::: gfx/layers/apz/src/Axis.cpp:259
(Diff revision 1)
> -             mLastOverscrollPeak == 0 &&
> -             mOverscrollScale == 1);
> -
>    SetVelocity(aVelocity);
> +  mMSDModel.SetPosition(mOverscroll);
> +  mMSDModel.SetVelocity(mVelocity * 1000.0);

Presumably this 1000 is a units conversion, can you add a comment that specifies what units we're converting from/to?

Comment 4

[Botond Ballo [:botond]]

Comment on attachment 8832701 [details]
Bug 1335958 - Simplify APZ overscroll physics.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/108888/diff/1-2/

Comment 5

[Botond Ballo [:botond]]

Addressed review comments.

Comment 6

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/integration/autoland/rev/d66ec372454c676e487da5ba47167a79dcfea001

Comment 7

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/d66ec372454c

Comment 8

[Andrei Purice]

Verified as fixed, since the overscroll physics have been applied, in beta 89.0b2 (20210420191345) and Nightly 90.0a1 (2021-04-20) (20210420213949) on MacOS 11.2.3. The prefs functionality is still debatable.