Closed Bug 1658168 Opened 4 years ago Closed 4 years ago

apzc smooth scroll animations end immediately when other axis is at end of range and it's not scrolling

Categories

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

defect

Tracking

()

RESOLVED FIXED
81 Branch
Tracking Status
firefox81 --- fixed

People

(Reporter: tnikkel, Assigned: tnikkel)

References

Details

Attachments

(1 file)

When we do a smooth scroll animation on the apzc side we use AxisPhysicsMSDModel. AxisPhysicsMSDModel is also used on the main thread to do smooth scroll animations. AxisPhysicsMSDModel uses doubles to represent all coordinates. On the main thread we pass in appunits, so the output of AxisPhysicsMSDModel is also in appunits. On the apzc side we follow that to.

A problem comes up when we do a smooth scroll animation in one axis, and the other axis is at the end of its range (and not animating). In the non-moving axis we convert the current scroll position (on the frame metrics) to appunits and pass it into AxisPhysicsMSDModel. Then when it comes time to call DoSample for the first time we ask for the position from AxisPhysicsMSDModel (in appunits) and then convert it to CSSPoint. And then we compare it with the current scroll position (on the frame metrics). Especially when we have zoom, the inaccuracy of this computation is enough for the difference between the CSSPoint for the current position from AxisPhysicsMSDModel and the current scroll offset (on the frame metrics) to be bigger than the epsilon that we use here:

https://searchfox.org/mozilla-central/rev/491612fa0be0f809069ea62c6316bf452cacc816/gfx/layers/apz/src/AsyncPanZoomController.cpp#747

and we determine that we have overscrolled the non-moving axis, and therefore the animation must be over.

Since AxisPhysicsMSDModel already operates purely on doubles if we just avoid converting to (integer) app units we avoid this problem. We could have AxisPhysicsMSDModel operate on float appunits, but that's a little inelegant because there aren't functions for converting to float appunits. We could also just have AxisPhysicsMSDModel operate on CSS pixels. That assumes that the physics calculations of AxisPhysicsMSDModel are scale independent, which I don't know for sure but they seem to be visually the same with CSS pixels.

This avoids a problem where apz side smooth scroll animations end immediately because the inaccuracy of converting to (integer) app units and back wrongly tells us that we have overscroll the non-moving axis if that axis is at the end of its range.

When we do a smooth scroll animation on the apzc side we use AxisPhysicsMSDModel. AxisPhysicsMSDModel is also used on the main thread to do smooth scroll animations. AxisPhysicsMSDModel uses doubles to represent all coordinates. On the main thread we pass in appunits, so the output of AxisPhysicsMSDModel is also in appunits. On the apzc side we follow that to.

A problem comes up when we do a smooth scroll animation in one axis, and the other axis is at the end of its range (and not animating). In the non-moving axis we convert the current scroll position (on the frame metrics) to appunits and pass it into AxisPhysicsMSDModel. Then when it comes time to call DoSample for the first time we ask for the position from AxisPhysicsMSDModel (in appunits) and then convert it to CSSPoint. And then we compare it with the current scroll position (on the frame metrics). Especially when we have zoom, the inaccuracy of this computation is enough for the difference between the CSSPoint for the current position from AxisPhysicsMSDModel and the current scroll offset (on the frame metrics) to be bigger than the epsilon that we use here:

https://searchfox.org/mozilla-central/rev/491612fa0be0f809069ea62c6316bf452cacc816/gfx/layers/apz/src/AsyncPanZoomController.cpp#747

and we determine that we have overscrolled the non-moving axis, and therefore the animation must be over.

Since AxisPhysicsMSDModel already operates purely on doubles if we just avoid converting to (integer) app units we avoid this problem. We could have AxisPhysicsMSDModel operate on float appunits, but that's a little inelegant because there aren't functions for converting to float appunits. We could also just have AxisPhysicsMSDModel operate on CSS pixels. That assumes that the physics calculations of AxisPhysicsMSDModel are scale independent, which I don't know for sure but they seem to be visually the same with CSS pixels.

Blocks: 1657822
Severity: -- → S3
Priority: -- → P3
Pushed by tnikkel@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f2f72d4877cc
Make SmoothScrollAnimation pass float CSS pixels to AxisPhysicsMSDModel instead of app units. r=kats
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → 81 Branch
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: