Bug 1458653 - Don't accelerate a fling if the fingers paused at the beginning of the touch motion. r=botond

This avoids unexpected acceleration after a "fling, fling, touch-pause-fling" sequence.

The touch start timestamp is captured when we go from NOTHING to TOUCHING,
and the pan start timestamp is captured when we go from TOUCHING to PANNING.

Differential Revision: https://phabricator.services.mozilla.com/D95470

gfx/layers/apz/src/AndroidAPZ.cpp

@@ -54,9 +54,8 @@ AsyncPanZoomAnimation* AndroidSpecificState::CreateFlingAnimation(
     AsyncPanZoomController& aApzc, const FlingHandoffState& aHandoffState,
     float aPLPPI) {
   if (StaticPrefs::apz_android_chrome_fling_physics_enabled()) {
-    return new GenericFlingAnimation<AndroidFlingPhysics>(
-        aApzc, aHandoffState.mChain, aHandoffState.mIsHandoff,
-        aHandoffState.mScrolledApzc, aPLPPI);
+    return new GenericFlingAnimation<AndroidFlingPhysics>(aApzc, aHandoffState,
+                                                          aPLPPI);
   } else {
     return new StackScrollerFlingAnimation(aApzc, this, aHandoffState.mChain,
                                            aHandoffState.mIsHandoff,

gfx/layers/apz/src/AsyncPanZoomController.cpp

@@ -264,6 +264,12 @@ typedef PlatformSpecificStateBase
  * be considered for fling acceleration.
  * Units: screen pixels per milliseconds
  *
+ * \li\b apz.fling_accel_max_pause_interval_ms
+ * The maximum time that is allowed to elapse between the touch start event that
+ * interrupts the previous fling, and the touch move that initiates panning for
+ * the current fling, for that fling to be considered for fling acceleration.
+ * Units: milliseconds
+ *
  * \li\b apz.fling_accel_base_mult
  * \li\b apz.fling_accel_supplemental_mult
  * When applying an acceleration on a fling, the new computed velocity is
@@ -524,9 +530,8 @@ static uint32_t sAsyncPanZoomControllerCount = 0;
 AsyncPanZoomAnimation* PlatformSpecificStateBase::CreateFlingAnimation(
     AsyncPanZoomController& aApzc, const FlingHandoffState& aHandoffState,
     float aPLPPI) {
-  return new GenericFlingAnimation<DesktopFlingPhysics>(
-      aApzc, aHandoffState.mChain, aHandoffState.mIsHandoff,
-      aHandoffState.mScrolledApzc, aPLPPI);
+  return new GenericFlingAnimation<DesktopFlingPhysics>(aApzc, aHandoffState,
+                                                        aPLPPI);
 }
 
 UniquePtr<VelocityTracker> PlatformSpecificStateBase::CreateVelocityTracker(
@@ -1258,6 +1263,7 @@ nsEventStatus AsyncPanZoomController::OnTouchStart(
             GetCurrentTouchBlock()->GetOverscrollHandoffChain()->CanBePanned(
                 this));
       }
+      mTouchStartTime = aEvent.mTimeStamp;
       SetState(TOUCHING);
       break;
     }
@@ -1310,11 +1316,11 @@ nsEventStatus AsyncPanZoomController::OnTouchMove(
         // ConsumeNoDefault status immediately to trigger cancel event further.
         // It should happen independent of the parent type (whether it is
         // scrolling or not).
-        StartPanning(extPoint);
+        StartPanning(extPoint, aEvent.mTimeStamp);
         return nsEventStatus_eConsumeNoDefault;
       }
 
-      return StartPanning(extPoint);
+      return StartPanning(extPoint, aEvent.mTimeStamp);
     }
 
     case PANNING:
@@ -1715,7 +1721,8 @@ nsEventStatus AsyncPanZoomController::OnScaleEnd(
     } else {
       // If zooming isn't allowed, StartTouch() was already called
       // in OnScaleBegin().
-      StartPanning(ToExternalPoint(aEvent.mScreenOffset, aEvent.mFocusPoint));
+      StartPanning(ToExternalPoint(aEvent.mScreenOffset, aEvent.mFocusPoint),
+                   aEvent.mTimeStamp);
     }
   } else {
     // Otherwise, handle the gesture being completely done.
@@ -1797,7 +1804,8 @@ nsEventStatus AsyncPanZoomController::HandleEndOfPan() {
   if (APZCTreeManager* treeManagerLocal = GetApzcTreeManager()) {
     const FlingHandoffState handoffState{
         flingVelocity, GetCurrentInputBlock()->GetOverscrollHandoffChain(),
-        false /* not handoff */, GetCurrentInputBlock()->GetScrolledApzc()};
+        Some(mTouchStartRestingTimeBeforePan), false /* not handoff */,
+        GetCurrentInputBlock()->GetScrolledApzc()};
     treeManagerLocal->DispatchFling(this, handoffState);
   }
   return nsEventStatus_eConsumeNoDefault;
@@ -3137,7 +3145,7 @@ void AsyncPanZoomController::HandlePinchLocking(
 }
 
 nsEventStatus AsyncPanZoomController::StartPanning(
-    const ExternalPoint& aStartPoint) {
+    const ExternalPoint& aStartPoint, const TimeStamp& aEventTime) {
   ParentLayerPoint vector =
       ToParentLayerCoordinates(PanVector(aStartPoint), mStartTouch);
   double angle = atan2(vector.y, vector.x);  // range [-pi, pi]
@@ -3157,6 +3165,7 @@ nsEventStatus AsyncPanZoomController::StartPanning(
   if (IsInPanningState()) {
     mozilla::Telemetry::Accumulate(mozilla::Telemetry::SCROLL_INPUT_METHODS,
                                    (uint32_t)ScrollInputMethod::ApzTouch);
+    mTouchStartRestingTimeBeforePan = aEventTime - mTouchStartTime;
 
     if (RefPtr<GeckoContentController> controller =
             GetGeckoContentController()) {
@@ -3479,7 +3488,8 @@ void AsyncPanZoomController::HandleFlingOverscroll(
   APZCTreeManager* treeManagerLocal = GetApzcTreeManager();
   if (treeManagerLocal) {
     const FlingHandoffState handoffState{aVelocity, aOverscrollHandoffChain,
-                                         true /* handoff */, aScrolledApzc};
+                                         Nothing(), true /* handoff */,
+                                         aScrolledApzc};
     ParentLayerPoint residualVelocity =
         treeManagerLocal->DispatchFling(this, handoffState);
     FLING_LOG("APZC %p left with residual velocity %s\n", this,

gfx/layers/apz/src/AsyncPanZoomController.h

@@ -818,7 +818,8 @@ class AsyncPanZoomController {
    * position of the start point so that the pan gesture is calculated
    * regardless of if the window/GeckoView moved during the pan.
    */
-  nsEventStatus StartPanning(const ExternalPoint& aStartPoint);
+  nsEventStatus StartPanning(const ExternalPoint& aStartPoint,
+                             const TimeStamp& aEventTime);
 
   /**
    * Wrapper for Axis::UpdateWithTouchAtDevicePoint(). Calls this function for
@@ -1676,6 +1677,13 @@ class AsyncPanZoomController {
   }
 
  private:
+  // The timestamp of the latest touch start event.
+  TimeStamp mTouchStartTime;
+  // The time duration between mTouchStartTime and the touchmove event that
+  // started the pan (the touchmove event that transitioned this APZC from the
+  // TOUCHING state to one of the PANNING* states). Only valid while this APZC
+  // is in a panning state.
+  TimeDuration mTouchStartRestingTimeBeforePan;
   // Extra offset to add to the async scroll position for testing
   CSSPoint mTestAsyncScrollOffset;
   // Extra zoom to include in the aync zoom for testing

gfx/layers/apz/src/FlingAccelerator.cpp

@@ -8,7 +8,7 @@
 
 #include "mozilla/StaticPrefs_apz.h"
 
-#include "GenericFlingAnimation.h"  // for FLING_LOG
+#include "GenericFlingAnimation.h"  // for FLING_LOG and FlingHandoffState
 
 namespace mozilla {
 namespace layers {
@@ -30,14 +30,15 @@ static float Accelerate(float aBase, float aSupplemental) {
 }
 
 ParentLayerPoint FlingAccelerator::GetFlingStartingVelocity(
-    const SampleTime& aNow, const ParentLayerPoint& aVelocity) {
+    const SampleTime& aNow, const ParentLayerPoint& aVelocity,
+    const FlingHandoffState& aHandoffState) {
   // If the fling should be accelerated and is in the same direction as the
   // previous fling, boost the velocity to be the sum of the two. Check separate
   // axes separately because we could have two vertical flings with small
   // horizontal components on the opposite side of zero, and we still want the
   // y-fling to get accelerated.
   ParentLayerPoint velocity = aVelocity;
-  if (ShouldAccelerate(aNow, aVelocity)) {
+  if (ShouldAccelerate(aNow, aVelocity, aHandoffState)) {
     if (velocity.x != 0 &&
         SameDirection(velocity.x, mPreviousFlingStartingVelocity.x)) {
       velocity.x = Accelerate(velocity.x, mPreviousFlingStartingVelocity.x);
@@ -63,21 +64,31 @@ ParentLayerPoint FlingAccelerator::GetFlingStartingVelocity(
 }
 
 bool FlingAccelerator::ShouldAccelerate(
-    const SampleTime& aNow, const ParentLayerPoint& aVelocity) const {
+    const SampleTime& aNow, const ParentLayerPoint& aVelocity,
+    const FlingHandoffState& aHandoffState) const {
   if (!IsTracking() || mPreviousFlingStartTime.IsNull()) {
     FLING_LOG("%p Fling accelerator was reset, not accelerating.\n", this);
     return false;
   }
 
-  if ((aNow - mPreviousFlingStartTime).ToMilliseconds() >=
-      StaticPrefs::apz_fling_accel_interval_ms()) {
-    FLING_LOG(
-        "%p Too much time (%fms) elapsed since previous fling, not "
-        "accelerating.\n",
-        this, float((aNow - mPreviousFlingStartTime).ToMilliseconds()));
+  if (!aHandoffState.mTouchStartRestingTime) {
+    FLING_LOG("%p Don't have a touch start resting time, not accelerating.\n",
+              this);
     return false;
   }
 
+  double msSincePreviousFling =
+      (aNow - mPreviousFlingStartTime).ToMilliseconds();
+  double msBetweenTouchStartAndPanStart =
+      aHandoffState.mTouchStartRestingTime->ToMilliseconds();
+  FLING_LOG(
+      "%p ShouldAccelerate with pan velocity %f pixels/ms, previous fling "
+      "cancel velocity %f pixels/ms, time elapsed since starting previous "
+      "fling %fms, time between touch start and pan start %fms.\n",
+      this, float(aVelocity.Length()),
+      float(mPreviousFlingCancelVelocity.Length()), float(msSincePreviousFling),
+      float(msBetweenTouchStartAndPanStart));
+
   if (aVelocity.Length() < StaticPrefs::apz_fling_accel_min_velocity()) {
     FLING_LOG("%p Fling velocity too low (%f), not accelerating.\n", this,
               float(aVelocity.Length()));
@@ -93,6 +104,23 @@ bool FlingAccelerator::ShouldAccelerate(
     return false;
   }
 
+  if (msSincePreviousFling >= StaticPrefs::apz_fling_accel_interval_ms()) {
+    FLING_LOG(
+        "%p Too much time (%fms) elapsed since previous fling, not "
+        "accelerating.\n",
+        this, msSincePreviousFling);
+    return false;
+  }
+
+  if (msBetweenTouchStartAndPanStart >=
+      StaticPrefs::apz_fling_accel_max_pause_interval_ms()) {
+    FLING_LOG(
+        "%p Too much time (%fms) elapsed between touch start and pan start, "
+        "not accelerating.\n",
+        this, msBetweenTouchStartAndPanStart);
+    return false;
+  }
+
   return true;
 }
 

gfx/layers/apz/src/FlingAccelerator.h

@@ -13,6 +13,8 @@
 namespace mozilla {
 namespace layers {
 
+struct FlingHandoffState;
+
 /**
  * This class is used to track state that is used when determining whether a
  * fling should be accelerated.
@@ -29,8 +31,9 @@ class FlingAccelerator final {
 
   // Starts a new fling, and returns the (potentially accelerated) velocity that
   // should be used for that fling.
-  ParentLayerPoint GetFlingStartingVelocity(const SampleTime& aNow,
-                                            const ParentLayerPoint& aVelocity);
+  ParentLayerPoint GetFlingStartingVelocity(
+      const SampleTime& aNow, const ParentLayerPoint& aVelocity,
+      const FlingHandoffState& aHandoffState);
 
   void ObserveFlingCanceled(const ParentLayerPoint& aVelocity) {
     mPreviousFlingCancelVelocity = aVelocity;
@@ -38,7 +41,8 @@ class FlingAccelerator final {
 
  protected:
   bool ShouldAccelerate(const SampleTime& aNow,
-                        const ParentLayerPoint& aVelocity) const;
+                        const ParentLayerPoint& aVelocity,
+                        const FlingHandoffState& aHandoffState) const;
 
   // The initial velocity of the most recent fling.
   ParentLayerPoint mPreviousFlingStartingVelocity;

gfx/layers/apz/src/GenericFlingAnimation.h

@@ -55,14 +55,11 @@ template <typename FlingPhysics>
 class GenericFlingAnimation : public AsyncPanZoomAnimation,
                               public FlingPhysics {
  public:
-  GenericFlingAnimation(
-      AsyncPanZoomController& aApzc,
-      const RefPtr<const OverscrollHandoffChain>& aOverscrollHandoffChain,
-      bool aFlingIsHandedOff,
-      const RefPtr<const AsyncPanZoomController>& aScrolledApzc, float aPLPPI)
+  GenericFlingAnimation(AsyncPanZoomController& aApzc,
+                        const FlingHandoffState& aHandoffState, float aPLPPI)
       : mApzc(aApzc),
-        mOverscrollHandoffChain(aOverscrollHandoffChain),
-        mScrolledApzc(aScrolledApzc) {
+        mOverscrollHandoffChain(aHandoffState.mChain),
+        mScrolledApzc(aHandoffState.mScrolledApzc) {
     MOZ_ASSERT(mOverscrollHandoffChain);
 
     // Drop any velocity on axes where we don't have room to scroll anyways
@@ -80,7 +77,7 @@ class GenericFlingAnimation : public AsyncPanZoomAnimation,
       mApzc.mY.SetVelocity(0);
     }
 
-    if (aFlingIsHandedOff) {
+    if (aHandoffState.mIsHandoff) {
       // Only apply acceleration in the APZC that originated the fling, not in
       // APZCs further down the handoff chain during handoff.
       mApzc.mFlingAccelerator.Reset();
@@ -88,7 +85,7 @@ class GenericFlingAnimation : public AsyncPanZoomAnimation,
 
     ParentLayerPoint velocity =
         mApzc.mFlingAccelerator.GetFlingStartingVelocity(
-            aApzc.GetFrameTime(), mApzc.GetVelocityVector());
+            aApzc.GetFrameTime(), mApzc.GetVelocityVector(), aHandoffState);
 
     mApzc.SetVelocityVector(velocity);
 

gfx/layers/apz/src/OverscrollHandoffState.h

@@ -156,6 +156,12 @@ struct FlingHandoffState {
   // otherwise it may not stay alive for the entire handoff.
   RefPtr<const OverscrollHandoffChain> mChain;
 
+  // The time duration between the touch start and the touch move that started
+  // the pan gesture which triggered this fling. In other words, the time it
+  // took for the finger to move enough to cross the touch slop threshold.
+  // Nothing if this fling was not immediately caused by a touch pan.
+  Maybe<TimeDuration> mTouchStartRestingTime;
+
   // Whether handoff has happened by this point, or we're still process
   // the original fling.
   bool mIsHandoff;

gfx/layers/apz/test/gtest/TestFlingAcceleration.cpp

@@ -160,3 +160,21 @@ TEST_F(APZCFlingAccelerationTester,
                          {0, 10, 17, 29, 29, 33, 33, 0, 31, 27, 13});
   CHECK_VELOCITY(Up, 2.3, 2.7);
 }
+
+TEST_F(APZCFlingAccelerationTester, ShouldNotAccelerateWhenPausedAtStartOfPan) {
+  SCOPED_GFX_PREF_INT("apz.fling_accel_interval_ms", 750);
+
+  ExecutePanGesture100Hz(
+      ScreenIntPoint{711, 1468},
+      {0, 0, 0, 0, -8, 0, -18, -32, -50, -57, -66, -68, -63, -60});
+  CHECK_VELOCITY(Down, 6.2, 8.5);
+
+  ExecuteWait(TimeDuration::FromMilliseconds(285));
+  CHECK_VELOCITY(Down, 3.4, 7.3);
+
+  ExecutePanGesture100Hz(
+      ScreenIntPoint{658, 1352},
+      {0, 0, 0, 0, 0, 0,  0,   0,   0,   0,   0,   0,   0,
+       0, 0, 0, 0, 0, -8, -18, -34, -53, -70, -75, -75, -64});
+  CHECK_VELOCITY(Down, 6.7, 9.1);
+}

modules/libpref/init/StaticPrefList.yaml

@@ -397,6 +397,11 @@
   value: 1.5f
   mirror: always
 
+- name: apz.fling_accel_max_pause_interval_ms
+  type: RelaxedAtomicInt32
+  value: 65
+  mirror: always
+
 - name: apz.fling_curve_function_x1
   type: float
   value: 0.0f