To support Bug 1022818 and Bug 1010538, the CSSOM-View smooth scrolling behavior movement model will be implemented on the main thread for platforms that do no support APZ (possibly within AsyncScroll.cpp). Bug 1022825 tracks implementation for platforms that support APZ. Details on the smooth scrolling movement model is described in Bug 1010538.

- Implemented the AxisPhysicsModel class, which encapsulates interpolation and integration of a 1-dimensional physics model in a frame-rate independent and stable manner. - Implemented the AxisPhysicsMSDModel class, which models a generic 1-dimensional Mass-Spring-Damper simulation. - This physical movement simulation code has been implemented separately from the existing momentum code in Axis.cpp so that it can be utilized by both the compositor (AsyncPanZoomController) and main thread (nsGfxScrollFrame).

Comment on attachment 8451076 [details] [diff] [review] Bug 1026023 - Part 1: Create Preferences - Added layout.css.scroll-behavior.enabled preference to enable and disable scroll behavior and related smooth scrolling. - Added layout.css.scroll-behavior.spring-constant preference to enable tuning of the smooth scroll motion. spring-constant controls the strength of the simulated MSD (Mass-Spring-Damper). - Added layout.css.scroll-behavior.damping-ratio preference to enable tuning of the smooth scroll motion. damping-ratio controls the under, over, or critically damped behavior of the simulated MSD.

- Added nsIScrollableFrame::ScrollMode::SMOOTH_MSD to differentiate existing smooth scrolls used by keyboard and mousewheel events from the CSSOM-View scroll-behavior's MSD motion scrolling. - Implemented ScrollFrameHelper::AsyncSmoothMSDScroll, which takes the role of ScrollFrameHelper::AsyncScroll when SMOOTH_MSD scrolls are requested. - Implemented glue code to handle callbacks from AsyncSmoothMSDScroll and to hand off velocity between the classes when one scroll animation is interrupted by another. Please note: These patches will not have any end-user effect on their own. They provide functionality used by Bug 1022818, Bug 1022825, and Bug 1010538.

- Implemented the AxisPhysicsModel class, which encapsulates interpolation and integration of a 1-dimensional physics model in a frame-rate independent and stable manner. - Implemented the AxisPhysicsMSDModel class, which models a generic 1-dimensional Mass-Spring-Damper simulation. - This physical movement simulation code has been implemented separately from the existing momentum code in Axis.cpp so that it can be utilized by both the compositor (AsyncPanZoomController) and main thread (nsGfxScrollFrame). V2 Patch: Adjusted spacing and wrapping

Added nsIScrollableFrame::ScrollMode::SMOOTH_MSD to differentiate existing smooth scrolls used by keyboard and mousewheel events from the CSSOM-View scroll-behavior's MSD motion scrolling. - Implemented ScrollFrameHelper::AsyncSmoothMSDScroll, which takes the role of ScrollFrameHelper::AsyncScroll when SMOOTH_MSD scrolls are requested. - Implemented glue code to handle callbacks from AsyncSmoothMSDScroll and to hand off velocity between the classes when one scroll animation is interrupted by another. V2 Patch: Adjusted spacing and wrapping

I am looking for someone with the capacity to review these patches as well as the patches in Bug 1022818. Would you have some extra cycles?

I can help review, but I'm not a peer for layout/dom code so my review alone won't be enough.

I have no experience with Gecko/DOM code, so I unfortunately can't help.

I can do the gfx/ and layout/ parts, as could roc.

- Added layout.css.scroll-behavior.enabled preference to enable and disable scroll behavior and related smooth scrolling. - Added layout.css.scroll-behavior.spring-constant preference to enable tuning of the smooth scroll motion. spring-constant controls the strength of the simulated MSD (Mass-Spring-Damper). - Added layout.css.scroll-behavior.damping-ratio preference to enable tuning of the smooth scroll motion. damping-ratio controls the under, over, or critically damped behavior of the simulated MSD. V2 Patch: Un-bitrotted

- Added nsIScrollableFrame::ScrollMode::SMOOTH_MSD to differentiate existing smooth scrolls used by keyboard and mousewheel events from the CSSOM-View scroll-behavior's MSD motion scrolling. - Implemented ScrollFrameHelper::AsyncSmoothMSDScroll, which takes the role of ScrollFrameHelper::AsyncScroll when SMOOTH_MSD scrolls are requested. - Implemented glue code to handle callbacks from AsyncSmoothMSDScroll and to hand off velocity between the classes when one scroll animation is interrupted by another. V3 Patch: Smooth MSD scrolling is now cancelled by mouse wheel and touchpad events, without jumping to MSD scroll destination.

I have updated the patch so that smooth MSD scrolling is now cancelled by mouse wheel and touchpad events, without jumping to MSD scroll destination. This conforms with the CSSOM-View scroll-behavior spec and removes the sudden jump when it was interrupted by mouse-wheel events. One side-effect of this is that OSX touchpad fling gestures will continuously cancel smooth scrolls while momentum is in effect. This has the effect that "scroll to top" buttons and the like will be non-responsive until the fling-ed content comes to a stop. Would it make sense to cancel (or ignore) the rest of a fling event when a smooth scroll is started? Also, I noticed that when the system is set to underdamped or you rapidly change scroll directions, the overshoot is not visible due to the clamping occurring in nsGFXScrollFrame. I'll add a "part4" patch to cover that scenario and enable us to explore the effect.

- Implemented the AxisPhysicsModel class, which encapsulates interpolation and integration of a 1-dimensional physics model in a frame-rate independent and stable manner. - Implemented the AxisPhysicsMSDModel class, which models a generic 1-dimensional Mass-Spring-Damper simulation. - This physical movement simulation code has been implemented separately from the existing momentum code in Axis.cpp so that it can be utilized by both the compositor (AsyncPanZoomController) and main thread (nsGfxScrollFrame). V3 Patch: Fixed type-o in AxisPhysicsModel::SetPosition method name.

- Added nsIScrollableFrame::ScrollMode::SMOOTH_MSD to differentiate existing smooth scrolls used by keyboard and mousewheel events from the CSSOM-View scroll-behavior's MSD motion scrolling. - Implemented ScrollFrameHelper::AsyncSmoothMSDScroll, which takes the role of ScrollFrameHelper::AsyncScroll when SMOOTH_MSD scrolls are requested. - Implemented glue code to handle callbacks from AsyncSmoothMSDScroll and to hand off velocity between the classes when one scroll animation is interrupted by another. V4 Patch: - Intermediate animation frames are now able to overshoot the destination for under-damped MSD configurations and when changing destinations rapidly. - If overshooting the destination and hitting the extents of the allowed scroll range for a frame (i.e. no overscroll allowed), the velocity of the MSD will be zero'ed out. This effectively absorbs the energy of the impact and prevents bounce-back. - Corrected indentation

(In reply to :kip (Kearwood Gilbert) from comment #13) > Also, I noticed that when the system is set to underdamped or you rapidly > change scroll directions, the overshoot is not visible due to the clamping > occurring in nsGFXScrollFrame. I'll add a "part4" patch to cover that > scenario and enable us to explore the effect. I have incorporated the overshoot changes into an updated "part3" patch, V4: - Intermediate animation frames are now able to overshoot the destination for under-damped MSD configurations and when changing destinations rapidly. - If overshooting the destination and hitting the extents of the allowed scroll range for a frame (i.e. no overscroll allowed), the velocity of the MSD will be zero'ed out. This effectively absorbs the energy of the impact and prevents bounce-back.

(In reply to Matt Woodrow (:mattwoodrow) from comment #10) > I can do the gfx/ and layout/ parts, as could roc. Thanks Matt, I have assigned the patches you for review. Please advise if any should be sent to someone else.

Comment on attachment 8452437 [details] [diff] [review] Bug 1026023 - Part 1: Create Preferences (V2 Patch) Review of attachment 8452437 [details] [diff] [review]: ----------------------------------------------------------------- ::: layout/base/nsLayoutUtils.cpp @@ +545,5 @@ > + "layout.css.scroll-behavior.enabled", > + false); > + } > + > + return sCSSScrollBehaviorEnabled; Unrelated to this bug, but it would be nice if we could copy gfxPrefs to avoid needing all this boilerplate for each new layout pref.

Comment on attachment 8452570 [details] [diff] [review] Bug 1026023 - Part 2: Implement Physically Based Movement Model (V3 Patch) Review of attachment 8452570 [details] [diff] [review]: ----------------------------------------------------------------- Awesome! ::: gfx/layers/AxisPhysicsMSDModel.cpp @@ +58,5 @@ > + // kFinishDistance while the speed is simultaneously less than > + // kFinishVelocity. This enables an under-damped system to overshoot the > + // destination when desired without prematurely triggering the finished state. > + const double kFinishDistance = 30.0; > + const double kFinishVelocity = 60.0; Comment about how these values were chosen in case someone wants to modify them in the future. ::: gfx/layers/AxisPhysicsMSDModel.h @@ +16,5 @@ > + * AxisPhysicsModel encapsulates a 1-dimensional MSD (Mass-Spring-Damper) model. > + */ > +class AxisPhysicsMSDModel : public AxisPhysicsModel { > +public: > + AxisPhysicsMSDModel(double aPos, double aVelocity, double aSpringConstant, I'd prefer aDestination and aInitialVelocity. Or a comment explaining the parameters. ::: gfx/layers/AxisPhysicsModel.cpp @@ +8,5 @@ > + > +namespace mozilla { > +namespace layers { > + > +const double AxisPhysicsModel::kFixedTimestep = 1.0 / 120.0; // 120hz Any specific reason to choose 120hz here? We use 60hz for our refresh rate. Please add a comment explaining why we use this rate, even if it was chosen fairly arbitrarily. @@ +34,5 @@ > + > +double > +AxisPhysicsModel::GetPosition() > +{ > + return mPrevState.p * (1.0 - mProgress) + mNextState.p * mProgress; Might be nice to add a static LinearInterpolate/lerp helper and implement GetVelocity/GetPosition on top of that. Just to make it super clear why we're doing this. ::: gfx/layers/moz.build @@ +112,5 @@ > 'apz/util/ActiveElementManager.h', > 'apz/util/APZCCallbackHelper.h', > 'AtomicRefCountedWithFinalize.h', > + 'AxisPhysicsModel.h', > + 'AxisPhysicsMSDModel.h', You shouldn't need to export these headers twice. Putting them under EXPORTS copies them to <objdir>/dist/include so you can include them via #include <header.h>, putting them under EXPORTS.mozilla.layers copies them to <objdir>/dist/include/mozilla/layers so you need to #include <mozilla/layers/header.h> (or #include "header.h" when in the same directory obviously). Since your classes are within the mozilla/layers namespace then EXPORTS.mozilla.layers makes the most sense I think.

Comment on attachment 8452570 [details] [diff] [review] Bug 1026023 - Part 2: Implement Physically Based Movement Model (V3 Patch) Review of attachment 8452570 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/layers/AxisPhysicsMSDModel.h @@ +16,5 @@ > + * AxisPhysicsModel encapsulates a 1-dimensional MSD (Mass-Spring-Damper) model. > + */ > +class AxisPhysicsMSDModel : public AxisPhysicsModel { > +public: > + AxisPhysicsMSDModel(double aPos, double aVelocity, double aSpringConstant, Also note that we're assuming a unit mass for this class.

Comment on attachment 8452574 [details] [diff] [review] Bug 1026023 - Part 3: Integrate MSD movement with nsGfxScrollFrame (V4 Patch) Review of attachment 8452574 [details] [diff] [review]: ----------------------------------------------------------------- ::: layout/generic/nsGfxScrollFrame.cpp @@ +1258,5 @@ > + , mLastRefreshTime(aStartTime) > + , mCallee(nullptr) > + {} > + > + NS_INLINE_DECL_REFCOUNTING(AsyncScroll) AsyncSmoothMSDScroll @@ +1758,5 @@ > +ScrollFrameHelper::AsyncSmoothMSDScrollCallback(void* anInstance, > + mozilla::TimeDuration aDeltaTime) > +{ > + ScrollFrameHelper* self = static_cast<ScrollFrameHelper*>(anInstance); > + if (!self || !self->mAsyncSmoothMSDScroll) Is it actually possible to hit this? These seem like they should be assertions. @@ +1919,5 @@ > + Preferences::GetFloat("layout.css.scroll-behavior.spring-constant", > + 250.0)); > + double dampingRatio = static_cast<double>( > + Preferences::GetFloat("layout.css.scroll-behavior.damping-ratio", > + 1.0)); Move these pref checks into the AsyncSmoothMSDScroll ctor, they're not all that relevant to this function. @@ +1937,5 @@ > + } > + // We are done scrolling, set our destination to wherever we actually > + // ended up scrolling to. > + mDestination = GetScrollPosition(); > + return; This block of code is repeated 3 times in this function now, probably justifies being moved into a helper. ::: layout/generic/nsGfxScrollFrame.h @@ +173,5 @@ > nsRect GetScrollRangeForClamping() const; > > public: > static void AsyncScrollCallback(void* anInstance, mozilla::TimeStamp aTime); > + static void AsyncSmoothMSDScrollCallback(void* anInstance, Why is this parameter void*? Seems like we should be able to pass ScrollFrameHelper* without problems, or this could not be static. ::: layout/generic/nsIScrollableFrame.h @@ +158,5 @@ > * scrolling. SMOOTH will only be smooth if smooth scrolling is actually > * enabled. INSTANT is always synchronous, NORMAL can be asynchronous. > * If an INSTANT request happens while a smooth or async scroll is already in > * progress, the async scroll is interrupted and we instantly scroll to the > * destination. Briefly define SMOOTH_MSD in this comment (and maybe the difference between it and SMOOTH). @@ +181,5 @@ > * aScrollPosition.x/y is different from the current CSS pixel position, > * makes sure we only move in the direction given by the difference. > * Ensures that GetScrollPositionCSSPixels (the scroll position after > * rounding to CSS pixels) will be exactly aScrollPosition. > * The scroll mode is INSTANT. This last bit of the comment is wrong now :)

- Added layout.css.scroll-behavior.enabled preference to enable and disable scroll behavior and related smooth scrolling. - Added layout.css.scroll-behavior.spring-constant preference to enable tuning of the smooth scroll motion. spring-constant controls the strength of the simulated MSD (Mass-Spring-Damper). - Added layout.css.scroll-behavior.damping-ratio preference to enable tuning of the smooth scroll motion. damping-ratio controls the under, over, or critically damped behavior of the simulated MSD. V3 Patch: Updated to match code review in Comment 18 - Comment 21

- Implemented the AxisPhysicsModel class, which encapsulates interpolation and integration of a 1-dimensional physics model in a frame-rate independent and stable manner. - Implemented the AxisPhysicsMSDModel class, which models a generic 1-dimensional Mass-Spring-Damper simulation. - This physical movement simulation code has been implemented separately from the existing momentum code in Axis.cpp so that it can be utilized by both the compositor (AsyncPanZoomController) and main thread (nsGfxScrollFrame). V4 Patch: Updated to match code review in Comment 18 - Comment 21

- Added nsIScrollableFrame::ScrollMode::SMOOTH_MSD to differentiate existing smooth scrolls used by keyboard and mousewheel events from the CSSOM-View scroll-behavior's MSD motion scrolling. - Implemented ScrollFrameHelper::AsyncSmoothMSDScroll, which takes the role of ScrollFrameHelper::AsyncScroll when SMOOTH_MSD scrolls are requested. - Implemented glue code to handle callbacks from AsyncSmoothMSDScroll and to hand off velocity between the classes when one scroll animation is interrupted by another. V5 Patch: Updated to match code review in Comment 18 - Comment 21

- When a smooth scroll is being processed on a frame, mouse wheel and trackpad momentum scroll event updates will no longer cancel the SMOOTH or SMOOTH_MSD scroll animations, enabling scripts that depend on them to be responsive without forcing the user to wait for the fling animations to completely stop.

- Implemented the AxisPhysicsModel class, which encapsulates interpolation and integration of a 1-dimensional physics model in a frame-rate independent and stable manner. - Implemented the AxisPhysicsMSDModel class, which models a generic 1-dimensional Mass-Spring-Damper simulation. - This physical movement simulation code has been implemented separately from the existing momentum code in Axis.cpp so that it can be utilized by both the compositor (AsyncPanZoomController) and main thread (nsGfxScrollFrame). V5 Patch: Un-bitrotted

- Added layout.css.scroll-behavior.enabled preference to enable and disable scroll behavior and related smooth scrolling. - Added layout.css.scroll-behavior.spring-constant preference to enable tuning of the smooth scroll motion. spring-constant controls the strength of the simulated MSD (Mass-Spring-Damper). - Added layout.css.scroll-behavior.damping-ratio preference to enable tuning of the smooth scroll motion. damping-ratio controls the under, over, or critically damped behavior of the simulated MSD. V4 Patch: Changed gfxPrefs from "Once" to "Live" so that mochitests can set the preferences.

As this bug has no effect on its own, tests utilizing DOM API methods to access this functionality will be included in Bug 1022818.

Pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=a9fefa38e74a

(In reply to :kip (Kearwood Gilbert) from comment #29) > Pushed to try: > > https://tbpl.mozilla.org/?tree=Try&rev=a9fefa38e74a Step 4 of this patchset was missed in this try push. It has been pushed again: https://tbpl.mozilla.org/?tree=Try&rev=2dd893edda2b

https://hg.mozilla.org/integration/mozilla-inbound/rev/68dbb23d5a13 https://hg.mozilla.org/integration/mozilla-inbound/rev/619bf9ab665a https://hg.mozilla.org/integration/mozilla-inbound/rev/1290a934987f https://hg.mozilla.org/integration/mozilla-inbound/rev/91ec42d7d9e8

https://hg.mozilla.org/mozilla-central/rev/68dbb23d5a13 https://hg.mozilla.org/mozilla-central/rev/619bf9ab665a https://hg.mozilla.org/mozilla-central/rev/1290a934987f https://hg.mozilla.org/mozilla-central/rev/91ec42d7d9e8