Bug 1249040 - Allow wheel scrolls to accumulate in the presence of scroll snapping. r=kats

MozReview-Commit-ID: EUyGvkoyu8I

--HG--
extra : rebase_source : b9f9bbb7a11a976ff696c28b026d292a8f90d0e1

gfx/layers/AxisPhysicsMSDModel.cpp

@@ -58,7 +58,7 @@ AxisPhysicsMSDModel::Acceleration(const State &aState)
 
 
 double
-AxisPhysicsMSDModel::GetDestination()
+AxisPhysicsMSDModel::GetDestination() const
 {
   return mDestination;
 }

gfx/layers/AxisPhysicsMSDModel.h

@@ -27,7 +27,7 @@ public:
   /**
    * Gets the raw destination of this axis at this moment.
    */
-  double GetDestination();
+  double GetDestination() const;
 
   /**
    * Sets the raw destination of this axis at this moment.

gfx/layers/apz/src/AsyncPanZoomAnimation.h

@@ -18,6 +18,7 @@ namespace mozilla {
 namespace layers {
 
 class WheelScrollAnimation;
+class SmoothScrollAnimation;
 
 class AsyncPanZoomAnimation {
   NS_INLINE_DECL_THREADSAFE_REFCOUNTING(AsyncPanZoomAnimation)
@@ -52,6 +53,9 @@ public:
   virtual WheelScrollAnimation* AsWheelScrollAnimation() {
     return nullptr;
   }
+  virtual SmoothScrollAnimation* AsSmoothScrollAnimation() {
+    return nullptr;
+  }
 
   virtual bool WantsRepaints() {
     return true;

gfx/layers/apz/src/AsyncPanZoomController.cpp

@@ -742,7 +742,7 @@ public:
    * frame. Returns true if the smooth scroll should be advanced by one frame,
    * or false if the smooth scroll has ended.
    */
-  bool DoSample(FrameMetrics& aFrameMetrics, const TimeDuration& aDelta) {
+  bool DoSample(FrameMetrics& aFrameMetrics, const TimeDuration& aDelta) override {
     nsPoint oneParentLayerPixel =
       CSSPoint::ToAppUnits(ParentLayerPoint(1, 1) / aFrameMetrics.GetZoom());
     if (mXAxisModel.IsFinished(oneParentLayerPixel.x) &&
@@ -839,6 +839,15 @@ public:
     mYAxisModel.SetDestination(static_cast<int32_t>(aNewDestination.y));
   }
 
+  CSSPoint GetDestination() const {
+    return CSSPoint::FromAppUnits(
+        nsPoint(mXAxisModel.GetDestination(), mYAxisModel.GetDestination()));
+  }
+
+  SmoothScrollAnimation* AsSmoothScrollAnimation() override {
+    return this;
+  }
+
 private:
   AsyncPanZoomController& mApzc;
   AxisPhysicsMSDModel mXAxisModel, mYAxisModel;
@@ -1827,13 +1836,16 @@ nsEventStatus AsyncPanZoomController::OnScrollWheel(const ScrollWheelInput& aEve
   mozilla::Telemetry::Accumulate(mozilla::Telemetry::SCROLL_INPUT_METHODS,
       (uint32_t) ScrollInputMethodForWheelDeltaType(aEvent.mDeltaType));
 
-  // Wheel events from "clicky" mouse wheels trigger scroll snapping to the
-  // next snap point. Check for this, and adjust the delta to take into
-  // account the snap point.
-  bool scrollSnapping = MaybeAdjustDeltaForScrollSnapping(delta, aEvent);
 
   switch (aEvent.mScrollMode) {
     case ScrollWheelInput::SCROLLMODE_INSTANT: {
+
+      // Wheel events from "clicky" mouse wheels trigger scroll snapping to the
+      // next snap point. Check for this, and adjust the delta to take into
+      // account the snap point.
+      CSSPoint startPosition = mFrameMetrics.GetScrollOffset();
+      MaybeAdjustDeltaForScrollSnapping(aEvent, delta, startPosition);
+
       ScreenPoint distance = ToScreenCoordinates(
         ParentLayerPoint(fabs(delta.x), fabs(delta.y)), aEvent.mLocalOrigin);
 
@@ -1863,33 +1875,43 @@ nsEventStatus AsyncPanZoomController::OnScrollWheel(const ScrollWheelInput& aEve
       // update it.
       ReentrantMonitorAutoEnter lock(mMonitor);
 
-      if (scrollSnapping) {
+      // Perform scroll snapping if appropriate.
-        // If we're scroll snapping use a smooth scroll animation to get
+      CSSPoint startPosition = mFrameMetrics.GetScrollOffset();
+      // If we're already in a wheel scroll or smooth scroll animation,
+      // the delta is applied to its destination, not to the current
+      // scroll position. Take this into account when finding a snap point.
+      if (mState == WHEEL_SCROLL) {
+        startPosition = mAnimation->AsWheelScrollAnimation()->GetDestination();
+      } else if (mState == SMOOTH_SCROLL) {
+        startPosition = mAnimation->AsSmoothScrollAnimation()->GetDestination();
+      }
+      if (MaybeAdjustDeltaForScrollSnapping(aEvent, delta, startPosition)) {
+        // If we're scroll snapping, use a smooth scroll animation to get
         // the desired physics. Note that SmoothScrollTo() will re-use an
         // existing smooth scroll animation if there is one.
-        CSSPoint snapPoint = mFrameMetrics.GetScrollOffset() + (delta / mFrameMetrics.GetZoom());
+        SmoothScrollTo(startPosition);
-        SmoothScrollTo(snapPoint);
+        break;
-      } else {
-        // Otherwise, use a wheel scroll animation, also reusing one if possible.
-        if (mState != WHEEL_SCROLL) {
-          CancelAnimation();
-          SetState(WHEEL_SCROLL);
-
-          nsPoint initialPosition = CSSPoint::ToAppUnits(mFrameMetrics.GetScrollOffset());
-          StartAnimation(new WheelScrollAnimation(
-            *this, initialPosition, aEvent.mDeltaType));
-        }
-
-        nsPoint deltaInAppUnits =
-          CSSPoint::ToAppUnits(delta / mFrameMetrics.GetZoom());
-        // Cast velocity from ParentLayerPoints/ms to CSSPoints/ms then convert to
-        // appunits/second
-        nsPoint velocity =
-          CSSPoint::ToAppUnits(CSSPoint(mX.GetVelocity(), mY.GetVelocity())) * 1000.0f;
-
-        WheelScrollAnimation* animation = mAnimation->AsWheelScrollAnimation();
-        animation->Update(aEvent.mTimeStamp, deltaInAppUnits, nsSize(velocity.x, velocity.y));
       }
+
+      // Otherwise, use a wheel scroll animation, also reusing one if possible.
+      if (mState != WHEEL_SCROLL) {
+        CancelAnimation();
+        SetState(WHEEL_SCROLL);
+
+        nsPoint initialPosition = CSSPoint::ToAppUnits(mFrameMetrics.GetScrollOffset());
+        StartAnimation(new WheelScrollAnimation(
+          *this, initialPosition, aEvent.mDeltaType));
+      }
+
+      nsPoint deltaInAppUnits =
+        CSSPoint::ToAppUnits(delta / mFrameMetrics.GetZoom());
+      // Cast velocity from ParentLayerPoints/ms to CSSPoints/ms then convert to
+      // appunits/second
+      nsPoint velocity =
+        CSSPoint::ToAppUnits(CSSPoint(mX.GetVelocity(), mY.GetVelocity())) * 1000.0f;
+
+      WheelScrollAnimation* animation = mAnimation->AsWheelScrollAnimation();
+      animation->Update(aEvent.mTimeStamp, deltaInAppUnits, nsSize(velocity.x, velocity.y));
       break;
     }
   }
@@ -4027,7 +4049,9 @@ void AsyncPanZoomController::ScrollSnapToDestination() {
 }
 
 bool AsyncPanZoomController::MaybeAdjustDeltaForScrollSnapping(
-    ParentLayerPoint& aDelta, const ScrollWheelInput& aEvent)
+    const ScrollWheelInput& aEvent,
+    ParentLayerPoint& aDelta,
+    CSSPoint& aStartPosition)
 {
   // Don't scroll snap for pixel scrolls. This matches the main thread
   // behaviour in EventStateManager::DoScrollText().
@@ -4036,15 +4060,15 @@ bool AsyncPanZoomController::MaybeAdjustDeltaForScrollSnapping(
   }
 
   ReentrantMonitorAutoEnter lock(mMonitor);
-  CSSPoint scrollOffset = mFrameMetrics.GetScrollOffset();
   CSSToParentLayerScale2D zoom = mFrameMetrics.GetZoom();
   CSSPoint destination = mFrameMetrics.CalculateScrollRange().ClampPoint(
-      scrollOffset + (aDelta / zoom));
+      aStartPosition + (aDelta / zoom));
   nsIScrollableFrame::ScrollUnit unit =
       ScrollWheelInput::ScrollUnitForDeltaType(aEvent.mDeltaType);
 
   if (Maybe<CSSPoint> snapPoint = FindSnapPointNear(destination, unit)) {
-    aDelta = (*snapPoint - scrollOffset) * zoom;
+    aDelta = (*snapPoint - aStartPosition) * zoom;
+    aStartPosition = *snapPoint;
     return true;
   }
   return false;

gfx/layers/apz/src/AsyncPanZoomController.h

@@ -1155,9 +1155,13 @@ private:
 
   // If |aEvent| should trigger scroll snapping, adjust |aDelta| to reflect
   // the snapping (that is, make it a delta that will take us to the desired
-  // snap point). Returns true iff. the delta was so adjusted.
+  // snap point). The delta is interpreted as being relative to
-  bool MaybeAdjustDeltaForScrollSnapping(ParentLayerPoint& aDelta,
+  // |aStartPosition|, and if a target snap point is found, |aStartPosition|
-                                         const ScrollWheelInput& aEvent);
+  // is also updated, to the value of the snap point.
+  // Returns true iff. a target snap point was found.
+  bool MaybeAdjustDeltaForScrollSnapping(const ScrollWheelInput& aEvent,
+                                         ParentLayerPoint& aDelta,
+                                         CSSPoint& aStartPosition);
 
   // Snap to a snap position nearby the current scroll position, if appropriate.
   void ScrollSnap();

gfx/layers/apz/src/WheelScrollAnimation.h

@@ -32,6 +32,10 @@ public:
     return this;
   }
 
+  CSSPoint GetDestination() const {
+    return CSSPoint::FromAppUnits(mFinalDestination);
+  }
+
 private:
   void InitPreferences(TimeStamp aTime);