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 442876 74050b668ada1a9f3ddabfed1a98cc2b969f067f
parent 442875 8e850a230bfcfb52e5dc15d146a402c7d8521ab8
child 442877 ccfeb561645b499025c28c26287ee0b8d96cc7ad
push id34926
push userncsoregi@mozilla.com
push dateThu, 25 Oct 2018 04:44:20 +0000
treeherdermozilla-central@58b7faab3b8d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskats
bugs1499941
milestone65.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 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>