Bug 1402498 - Compute precise velocities and apply the zoom factor correctly. r?botond draft
authorMarkus Stange <mstange@themasta.com>
Sun, 17 Sep 2017 13:45:07 +0200
changeset 672905 55eeebb91408a86b5f43647e4dbf7e36addae23b
parent 672904 4da81c2f12538a316a4238b80ff434a82a7e3811
child 672906 2879bfab982715010c6e65483b8a049b309cdada
push id82420
push userbmo:mstange@themasta.com
push dateFri, 29 Sep 2017 22:38:50 +0000
reviewersbotond
bugs1402498
milestone58.0a1
Bug 1402498 - Compute precise velocities and apply the zoom factor correctly. r?botond This may make the smooth scroll animation feel differently, based on zoom. I'm not sure if the old workaround for that problem even worked; in any case, it gave wrong values. MozReview-Commit-ID: KXD1DPGfbgA
gfx/layers/apz/src/AsyncPanZoomController.cpp
gfx/layers/apz/src/GenericScrollAnimation.cpp
--- a/gfx/layers/apz/src/AsyncPanZoomController.cpp
+++ b/gfx/layers/apz/src/AsyncPanZoomController.cpp
@@ -1819,21 +1819,22 @@ AsyncPanZoomController::OnKeyboard(const
   if (mState != KEYBOARD_SCROLL) {
     CancelAnimation();
     SetState(KEYBOARD_SCROLL);
 
     nsPoint initialPosition = CSSPoint::ToAppUnits(mFrameMetrics.GetScrollOffset());
     StartAnimation(new KeyboardScrollAnimation(*this, initialPosition, aEvent.mAction.mType));
   }
 
-  // Cast velocity from ParentLayerPoints/ms to CSSPoints/ms then convert to
-  // appunits/second. We perform a cast to ParentLayerPoints/ms without a
-  // conversion so that the scroll duration isn't affected by zoom
+  // Convert velocity from ParentLayerPoints/ms to ParentLayerPoints/s and then
+  // to appunits/second.
   nsPoint velocity =
-    CSSPoint::ToAppUnits(CSSPoint(mX.GetVelocity(), mY.GetVelocity())) * 1000.0f;
+    CSSPoint::ToAppUnits(
+      ParentLayerPoint(mX.GetVelocity() * 1000.0f, mY.GetVelocity() * 1000.0f) /
+      mFrameMetrics.GetZoom());
 
   KeyboardScrollAnimation* animation = mAnimation->AsKeyboardScrollAnimation();
   MOZ_ASSERT(animation);
 
   animation->UpdateDestination(aEvent.mTimeStamp,
                                CSSPixel::ToAppUnits(destination),
                                nsSize(velocity.x, velocity.y));
 
@@ -2097,21 +2098,24 @@ nsEventStatus AsyncPanZoomController::On
 
         nsPoint initialPosition = CSSPoint::ToAppUnits(mFrameMetrics.GetScrollOffset());
         StartAnimation(new WheelScrollAnimation(
           *this, initialPosition, aEvent.mDeltaType));
       }
 
       nsPoint deltaInAppUnits =
         CSSPoint::ToAppUnits(delta / mFrameMetrics.GetZoom());
-      // Cast velocity from ParentLayerPoints/ms to CSSPoints/ms then convert to
-      // appunits/second. We perform a cast to ParentLayerPoints/ms without a
-      // conversion so that the scroll duration isn't affected by zoom
+
+      // Convert velocity from ParentLayerPoints/ms to ParentLayerPoints/s and
+      // then to appunits/second.
       nsPoint velocity =
-        CSSPoint::ToAppUnits(CSSPoint(mX.GetVelocity(), mY.GetVelocity())) * 1000.0f;
+        CSSPoint::ToAppUnits(
+          ParentLayerPoint(mX.GetVelocity() * 1000.0f,
+                           mY.GetVelocity() * 1000.0f) /
+          mFrameMetrics.GetZoom());
 
       WheelScrollAnimation* animation = mAnimation->AsWheelScrollAnimation();
       animation->UpdateDelta(aEvent.mTimeStamp, deltaInAppUnits, nsSize(velocity.x, velocity.y));
       break;
     }
   }
 
   return nsEventStatus_eConsumeNoDefault;
@@ -2856,21 +2860,24 @@ void AsyncPanZoomController::SmoothScrol
     APZC_LOG("%p updating destination on existing animation\n", this);
     RefPtr<SmoothScrollAnimation> animation(
       static_cast<SmoothScrollAnimation*>(mAnimation.get()));
     animation->SetDestination(CSSPoint::ToAppUnits(aDestination));
   } else {
     CancelAnimation();
     SetState(SMOOTH_SCROLL);
     nsPoint initialPosition = CSSPoint::ToAppUnits(mFrameMetrics.GetScrollOffset());
-    // Cast velocity from ParentLayerPoints/ms to CSSPoints/ms then convert to
-    // appunits/second. We perform a cast to ParentLayerPoints/ms without a
-    // conversion so that the scroll duration isn't affected by zoom
-    nsPoint initialVelocity = CSSPoint::ToAppUnits(CSSPoint(mX.GetVelocity(),
-                                                            mY.GetVelocity())) * 1000.0f;
+    // Convert velocity from ParentLayerPoints/ms to ParentLayerPoints/s and
+    // then to appunits/second.
+    nsPoint initialVelocity =
+      CSSPoint::ToAppUnits(
+        ParentLayerPoint(mX.GetVelocity() * 1000.0f,
+                          mY.GetVelocity() * 1000.0f) /
+        mFrameMetrics.GetZoom());
+
     nsPoint destination = CSSPoint::ToAppUnits(aDestination);
 
     StartAnimation(new SmoothScrollAnimation(*this,
                                              initialPosition, initialVelocity,
                                              destination,
                                              gfxPrefs::ScrollBehaviorSpringConstant(),
                                              gfxPrefs::ScrollBehaviorDampingRatio()));
   }
--- a/gfx/layers/apz/src/GenericScrollAnimation.cpp
+++ b/gfx/layers/apz/src/GenericScrollAnimation.cpp
@@ -69,21 +69,22 @@ GenericScrollAnimation::DoSample(FrameMe
                         : PositionAt(now);
   ParentLayerPoint displacement =
     (CSSPoint::FromAppUnits(sampledDest) - aFrameMetrics.GetScrollOffset()) * zoom;
 
   if (finished) {
     mApzc.mX.SetVelocity(0);
     mApzc.mY.SetVelocity(0);
   } else if (!IsZero(displacement)) {
-    // Velocity is measured in ParentLayerCoords / Milliseconds
-    float xVelocity = displacement.x / aDelta.ToMilliseconds();
-    float yVelocity = displacement.y / aDelta.ToMilliseconds();
-    mApzc.mX.SetVelocity(xVelocity);
-    mApzc.mY.SetVelocity(yVelocity);
+    // Convert velocity from AppUnits/Seconds to ParentLayerCoords/Milliseconds
+    nsSize velocity = VelocityAt(now);
+    ParentLayerPoint velocityPL =
+      CSSPoint::FromAppUnits(nsPoint(velocity.width, velocity.height)) * zoom;
+    mApzc.mX.SetVelocity(velocityPL.x / 1000.0);
+    mApzc.mY.SetVelocity(velocityPL.y / 1000.0);
   }
 
   // Note: we ignore overscroll for generic animations.
   ParentLayerPoint adjustedOffset, overscroll;
   mApzc.mX.AdjustDisplacement(displacement.x, adjustedOffset.x, overscroll.x);
   mApzc.mY.AdjustDisplacement(displacement.y, adjustedOffset.y, overscroll.y,
                               mForceVerticalOverscroll);