Bug 1509295 - Apply axis locking for flings at the start of the fling. r=botond
authorKartikaya Gupta <kgupta@mozilla.com>
Fri, 22 Mar 2019 16:44:45 +0000
changeset 465739 2b7718ac0493f482bad0ddc0255428520696bcff
parent 465738 989c26cee55799444632810744affc2f8228252b
child 465740 c3d4eb5b3b2223aa6734bb8c2d39457ee768fc3c
push id35746
push usershindli@mozilla.com
push dateSat, 23 Mar 2019 09:46:24 +0000
treeherdermozilla-central@02b7484f316b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbotond
bugs1509295
milestone68.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1509295 - Apply axis locking for flings at the start of the fling. r=botond Instead of having special code in the velocity tracker to deal with axis locking and cancel out the position delta, we can apply axis locking when we transition from the pan to the fling and simply cancel the velocity on locked axes. This avoids the problem where we start a fling from an axis lock but velocity information from prior to the axis lock is used to compute the (perhaps unexpected) fling trajectory. Differential Revision: https://phabricator.services.mozilla.com/D24060
gfx/layers/apz/src/AndroidVelocityTracker.cpp
gfx/layers/apz/src/AndroidVelocityTracker.h
gfx/layers/apz/src/Axis.cpp
gfx/layers/apz/src/Axis.h
gfx/layers/apz/src/SimpleVelocityTracker.cpp
gfx/layers/apz/src/SimpleVelocityTracker.h
--- a/gfx/layers/apz/src/AndroidVelocityTracker.cpp
+++ b/gfx/layers/apz/src/AndroidVelocityTracker.cpp
@@ -37,28 +37,21 @@ AndroidVelocityTracker::AndroidVelocityT
 void AndroidVelocityTracker::StartTracking(ParentLayerCoord aPos,
                                            uint32_t aTimestampMs) {
   Clear();
   mHistory.AppendElement(std::make_pair(aTimestampMs, aPos));
   mLastEventTime = aTimestampMs;
 }
 
 Maybe<float> AndroidVelocityTracker::AddPosition(ParentLayerCoord aPos,
-                                                 uint32_t aTimestampMs,
-                                                 bool aIsAxisLocked) {
+                                                 uint32_t aTimestampMs) {
   if ((aTimestampMs - mLastEventTime) >= kAssumePointerMoveStoppedTimeMs) {
     Clear();
   }
 
-  // If we are axis-locked, adjust the position to reflect the fact that
-  // no movement is happening.
-  if (aIsAxisLocked && !mHistory.IsEmpty()) {
-    aPos = mHistory[mHistory.Length() - 1].second - mAdditionalDelta;
-  }
-
   aPos += mAdditionalDelta;
 
   if (aTimestampMs == mLastEventTime) {
     // If we get a sample with the same timestamp as the previous one,
     // just update its position. Two samples in the history with the
     // same timestamp can lead to things like infinite velocities.
     if (mHistory.Length() > 0) {
       mHistory[mHistory.Length() - 1].second = aPos;
--- a/gfx/layers/apz/src/AndroidVelocityTracker.h
+++ b/gfx/layers/apz/src/AndroidVelocityTracker.h
@@ -17,18 +17,18 @@
 
 namespace mozilla {
 namespace layers {
 
 class AndroidVelocityTracker : public VelocityTracker {
  public:
   explicit AndroidVelocityTracker();
   void StartTracking(ParentLayerCoord aPos, uint32_t aTimestamp) override;
-  Maybe<float> AddPosition(ParentLayerCoord aPos, uint32_t aTimestampMs,
-                           bool aIsAxisLocked) override;
+  Maybe<float> AddPosition(ParentLayerCoord aPos,
+                           uint32_t aTimestampMs) override;
   float HandleDynamicToolbarMovement(uint32_t aStartTimestampMs,
                                      uint32_t aEndTimestampMs,
                                      ParentLayerCoord aDelta) override;
   Maybe<float> ComputeVelocity(uint32_t aTimestampMs) override;
   void Clear() override;
 
  private:
   // A queue of (timestamp, position) pairs; these are the historical
--- a/gfx/layers/apz/src/Axis.cpp
+++ b/gfx/layers/apz/src/Axis.cpp
@@ -61,18 +61,18 @@ float Axis::ToLocalVelocity(float aVeloc
 void Axis::UpdateWithTouchAtDevicePoint(ParentLayerCoord aPos,
                                         uint32_t aTimestampMs) {
   // mVelocityTracker is controller-thread only
   APZThreadUtils::AssertOnControllerThread();
 
   mPos = aPos;
 
   if (Maybe<float> newVelocity =
-          mVelocityTracker->AddPosition(aPos, aTimestampMs, mAxisLocked)) {
-    mVelocity = *newVelocity;
+          mVelocityTracker->AddPosition(aPos, aTimestampMs)) {
+    mVelocity = mAxisLocked ? 0 : *newVelocity;
   }
 }
 
 void Axis::HandleDynamicToolbarMovement(uint32_t aStartTimestampMs,
                                         uint32_t aEndTimestampMs,
                                         ParentLayerCoord aDelta) {
   // mVelocityTracker is controller-thread only
   APZThreadUtils::AssertOnControllerThread();
@@ -232,25 +232,30 @@ ParentLayerCoord Axis::PanDistance() con
 ParentLayerCoord Axis::PanDistance(ParentLayerCoord aPos) const {
   return fabs(aPos - mStartPos);
 }
 
 void Axis::EndTouch(uint32_t aTimestampMs) {
   // mVelocityQueue is controller-thread only
   APZThreadUtils::AssertOnControllerThread();
 
-  mAxisLocked = false;
   // If the velocity tracker wasn't able to compute a velocity, zero out
   // the velocity to make sure we don't get a fling based on some old and
-  // no-longer-relevant value of mVelocity.
-  if (Maybe<float> velocity = mVelocityTracker->ComputeVelocity(aTimestampMs)) {
+  // no-longer-relevant value of mVelocity. Also if the axis is locked then
+  // just reset the velocity to 0 since we don't need any velocity to carry
+  // into the fling.
+  if (mAxisLocked) {
+    mVelocity = 0;
+  } else if (Maybe<float> velocity =
+                 mVelocityTracker->ComputeVelocity(aTimestampMs)) {
     mVelocity = *velocity;
   } else {
     mVelocity = 0;
   }
+  mAxisLocked = false;
   AXIS_LOG("%p|%s ending touch, computed velocity %f\n",
            mAsyncPanZoomController, Name(), mVelocity);
 }
 
 void Axis::CancelGesture() {
   // mVelocityQueue is controller-thread only
   APZThreadUtils::AssertOnControllerThread();
 
--- a/gfx/layers/apz/src/Axis.h
+++ b/gfx/layers/apz/src/Axis.h
@@ -46,22 +46,19 @@ class VelocityTracker {
    * Start tracking velocity along this axis, starting with the given
    * initial position and corresponding timestamp.
    */
   virtual void StartTracking(ParentLayerCoord aPos, uint32_t aTimestamp) = 0;
   /**
    * Record a new position along this axis, at the given timestamp.
    * Returns the average velocity between the last sample and this one, or
    * or Nothing() if a reasonable average cannot be computed.
-   * If |aIsAxisLocked| is true, no movement is happening along this axis,
-   * and this should be reflected both in the returned instantaneous velocity,
-   * and the internal state maintained for calling ComputeVelocity() later.
    */
-  virtual Maybe<float> AddPosition(ParentLayerCoord aPos, uint32_t aTimestampMs,
-                                   bool aIsAxisLocked) = 0;
+  virtual Maybe<float> AddPosition(ParentLayerCoord aPos,
+                                   uint32_t aTimestampMs) = 0;
   /**
    * Record movement of the dynamic toolbar along this axis by |aDelta|
    * over the given time range. Movement of the dynamic toolbar means
    * that physical movement by |aDelta| has occurred, but this will not
    * be reflected in future positions passed to AddPosition().
    * Returns the velocity of the dynamic toolbar movement.
    */
   virtual float HandleDynamicToolbarMovement(uint32_t aStartTimestampMs,
--- a/gfx/layers/apz/src/SimpleVelocityTracker.cpp
+++ b/gfx/layers/apz/src/SimpleVelocityTracker.cpp
@@ -31,35 +31,32 @@ SimpleVelocityTracker::SimpleVelocityTra
 void SimpleVelocityTracker::StartTracking(ParentLayerCoord aPos,
                                           uint32_t aTimestampMs) {
   Clear();
   mVelocitySampleTimeMs = aTimestampMs;
   mVelocitySamplePos = aPos;
 }
 
 Maybe<float> SimpleVelocityTracker::AddPosition(ParentLayerCoord aPos,
-                                                uint32_t aTimestampMs,
-                                                bool aIsAxisLocked) {
+                                                uint32_t aTimestampMs) {
   if (aTimestampMs <= mVelocitySampleTimeMs + MIN_VELOCITY_SAMPLE_TIME_MS) {
     // See also the comment on MIN_VELOCITY_SAMPLE_TIME_MS.
     // We still update mPos so that the positioning is correct (and we don't run
     // into problems like bug 1042734) but the velocity will remain where it
     // was. In particular we don't update either mVelocitySampleTimeMs or
     // mVelocitySamplePos so that eventually when we do get an event with the
     // required time delta we use the corresponding distance delta as well.
     SVT_LOG("%p|%s skipping velocity computation for small time delta %dms\n",
             mAxis->mAsyncPanZoomController, mAxis->Name(),
             (aTimestampMs - mVelocitySampleTimeMs));
     return Nothing();
   }
 
-  float newVelocity = aIsAxisLocked
-                          ? 0.0f
-                          : (float)(mVelocitySamplePos - aPos) /
-                                (float)(aTimestampMs - mVelocitySampleTimeMs);
+  float newVelocity = (float)(mVelocitySamplePos - aPos) /
+                      (float)(aTimestampMs - mVelocitySampleTimeMs);
 
   newVelocity = ApplyFlingCurveToVelocity(newVelocity);
 
   SVT_LOG("%p|%s updating velocity to %f with touch\n",
           mAxis->mAsyncPanZoomController, mAxis->Name(), newVelocity);
   mVelocitySampleTimeMs = aTimestampMs;
   mVelocitySamplePos = aPos;
 
--- a/gfx/layers/apz/src/SimpleVelocityTracker.h
+++ b/gfx/layers/apz/src/SimpleVelocityTracker.h
@@ -16,18 +16,18 @@
 
 namespace mozilla {
 namespace layers {
 
 class SimpleVelocityTracker : public VelocityTracker {
  public:
   explicit SimpleVelocityTracker(Axis* aAxis);
   void StartTracking(ParentLayerCoord aPos, uint32_t aTimestamp) override;
-  Maybe<float> AddPosition(ParentLayerCoord aPos, uint32_t aTimestampMs,
-                           bool aIsAxisLocked) override;
+  Maybe<float> AddPosition(ParentLayerCoord aPos,
+                           uint32_t aTimestampMs) override;
   float HandleDynamicToolbarMovement(uint32_t aStartTimestampMs,
                                      uint32_t aEndTimestampMs,
                                      ParentLayerCoord aDelta) override;
   Maybe<float> ComputeVelocity(uint32_t aTimestampMs) override;
   void Clear() override;
 
  private:
   void AddVelocityToQueue(uint32_t aTimestampMs, float aVelocity);