Bug 1499941 - Fix spatial vs. scroll coordinate confusion around APZCTreeManager::ProcessDynamicToolbarMovement() and helpers. r=kats, a=RyanVM
authorBotond Ballo <botond@mozilla.com>
Thu, 25 Oct 2018 00:14:58 +0000
changeset 500930 b5f86619d08561677cfab972b281e4c06fa6c8a6
parent 500929 c2d642cb5eb28fedc0b8edb46e9fd02d2f689e2b
child 500931 2bb12341b0fd82417cddd5caf19fd4a25b854680
push id1864
push userffxbld-merge
push dateMon, 03 Dec 2018 15:51:40 +0000
treeherdermozilla-release@f040763d99ad [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskats, RyanVM
bugs1499941
milestone64.0
Bug 1499941 - Fix spatial vs. scroll coordinate confusion around APZCTreeManager::ProcessDynamicToolbarMovement() and helpers. r=kats, a=RyanVM 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>