Bug 1499941 - Fix spatial vs. scroll coordinate confusion around APZCTreeManager::ProcessDynamicToolbarMovement() and helpers. r=kats
authorBotond Ballo <botond@mozilla.com>
Thu, 25 Oct 2018 00:14:58 +0000
changeset 491238 74050b668ada1a9f3ddabfed1a98cc2b969f067f
parent 491237 8e850a230bfcfb52e5dc15d146a402c7d8521ab8
child 491239 ccfeb561645b499025c28c26287ee0b8d96cc7ad
push id247
push userfmarier@mozilla.com
push dateSat, 27 Oct 2018 01:06:44 +0000
reviewerskats
bugs1499941
milestone65.0a1
Bug 1499941 - Fix spatial vs. scroll coordinate confusion around APZCTreeManager::ProcessDynamicToolbarMovement() and helpers. r=kats Differential Revision: https://phabricator.services.mozilla.com/D9715
gfx/layers/apz/src/APZCTreeManager.h
gfx/layers/apz/src/AndroidDynamicToolbarAnimator.cpp
gfx/layers/apz/src/SimpleVelocityTracker.cpp
--- a/gfx/layers/apz/src/APZCTreeManager.h
+++ b/gfx/layers/apz/src/APZCTreeManager.h
@@ -588,16 +588,18 @@ public:
   ScreenPoint GetCurrentMousePosition() const;
 
   /**
    * Process a movement of the dynamic toolbar by |aDeltaY| over the time
    * period from |aStartTimestampMs| to |aEndTimestampMs|.
    * This is used to track velocities accurately in the presence of movement
    * of the dynamic toolbar, since in such cases the finger can be moving
    * relative to the screen even though no scrolling is occurring.
+   * Note that this function expects "spatial coordinates" (i.e. toolbar
+   * moves up --> negative delta).
    */
   void ProcessDynamicToolbarMovement(uint32_t aStartTimestampMs,
                                      uint32_t aEndTimestampMs,
                                      ScreenCoord aDeltaY);
 private:
   typedef bool (*GuidComparator)(const ScrollableLayerGuid&, const ScrollableLayerGuid&);
 
   /* Helpers */
--- a/gfx/layers/apz/src/AndroidDynamicToolbarAnimator.cpp
+++ b/gfx/layers/apz/src/AndroidDynamicToolbarAnimator.cpp
@@ -623,17 +623,17 @@ AndroidDynamicToolbarAnimator::ProcessTo
 
     uint32_t timeDelta = aTimeStamp - mControllerLastEventTimeStamp;
     if (mControllerLastEventTimeStamp && timeDelta && aDelta) {
       // we can't use mApz because we're on the controller thread, so we have
       // the caller provide a RefPtr to the same underlying object, which should
       // be safe to use.
       aApz->ProcessDynamicToolbarMovement(mControllerLastEventTimeStamp,
                                           aTimeStamp,
-                                          -(float)aDelta);
+                                          (float)aDelta);
     }
   }
 
   return status;
 }
 
 void
 AndroidDynamicToolbarAnimator::HandleTouchEnd(StaticToolbarState aCurrentToolbarState, ScreenIntCoord aCurrentTouch)
--- a/gfx/layers/apz/src/SimpleVelocityTracker.cpp
+++ b/gfx/layers/apz/src/SimpleVelocityTracker.cpp
@@ -73,17 +73,20 @@ SimpleVelocityTracker::AddPosition(Paren
 
 float
 SimpleVelocityTracker::HandleDynamicToolbarMovement(uint32_t aStartTimestampMs,
                                                     uint32_t aEndTimestampMs,
                                                     ParentLayerCoord aDelta)
 {
   float timeDelta = aEndTimestampMs - aStartTimestampMs;
   MOZ_ASSERT(timeDelta != 0);
-  float velocity = aDelta / timeDelta;
+  // Negate the delta to convert from spatial coordinates (e.g. toolbar
+  // has moved up --> negative delta) to scroll coordinates (e.g. toolbar
+  // has moved up --> scroll offset is increasing).
+  float velocity = -aDelta / timeDelta;
   velocity = ApplyFlingCurveToVelocity(velocity);
   mVelocitySampleTimeMs = aEndTimestampMs;
 
   AddVelocityToQueue(aEndTimestampMs, velocity);
   return velocity;
 }
 
 Maybe<float>