Bug 1219296 - Scroll snap directly in APZ instead of going through the main thread. r=kats
authorBotond Ballo <botond@mozilla.com>
Mon, 28 Mar 2016 18:36:02 -0400
changeset 291426 28a2a773036181d99e9f314c24ad7cc1612b239b
parent 291425 0b0e5290d011412dfdb21a986a622c00f3f1944e
child 291427 0b4e95151cc44d9dc3912a3027abe34e825567a4
push id19656
push usergwagner@mozilla.com
push dateMon, 04 Apr 2016 13:43:23 +0000
treeherderb2g-inbound@e99061fde28a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskats
bugs1219296
milestone48.0a1
Bug 1219296 - Scroll snap directly in APZ instead of going through the main thread. r=kats MozReview-Commit-ID: 3qAdSWXwOsu
gfx/layers/FrameMetrics.h
gfx/layers/apz/src/APZUtils.h
gfx/layers/apz/src/AsyncPanZoomController.cpp
gfx/layers/apz/src/AsyncPanZoomController.h
layout/base/Units.h
layout/generic/ScrollSnap.h
--- a/gfx/layers/FrameMetrics.h
+++ b/gfx/layers/FrameMetrics.h
@@ -189,16 +189,25 @@ public:
   CSSSize CalculateBoundedCompositedSizeInCssPixels() const
   {
     CSSSize size = CalculateCompositedSizeInCssPixels();
     size.width = std::min(size.width, mRootCompositionSize.width);
     size.height = std::min(size.height, mRootCompositionSize.height);
     return size;
   }
 
+  CSSRect CalculateScrollRange() const
+  {
+    CSSSize scrollPortSize = CalculateCompositedSizeInCssPixels();
+    CSSRect scrollRange = mScrollableRect;
+    scrollRange.width = std::max(scrollRange.width - scrollPortSize.width, 0.0f);
+    scrollRange.height = std::max(scrollRange.height - scrollPortSize.height, 0.0f);
+    return scrollRange;
+  }
+
   void ScrollBy(const CSSPoint& aPoint)
   {
     mScrollOffset += aPoint;
   }
 
   void ZoomBy(float aScale)
   {
     ZoomBy(gfxSize(aScale, aScale));
--- a/gfx/layers/apz/src/APZUtils.h
+++ b/gfx/layers/apz/src/APZUtils.h
@@ -20,17 +20,17 @@ enum HitTestResult {
   HitNothing,
   HitLayer,
   HitDispatchToContentRegion,
 };
 
 enum CancelAnimationFlags : uint32_t {
   Default = 0x0,            /* Cancel all animations */
   ExcludeOverscroll = 0x1,  /* Don't clear overscroll */
-  RequestSnap = 0x2         /* Request snapping to snap points */
+  ScrollSnap = 0x2          /* Snap to snap points */
 };
 
 inline CancelAnimationFlags
 operator|(CancelAnimationFlags a, CancelAnimationFlags b)
 {
   return static_cast<CancelAnimationFlags>(static_cast<int>(a)
                                          | static_cast<int>(b));
 }
--- a/gfx/layers/apz/src/AsyncPanZoomController.cpp
+++ b/gfx/layers/apz/src/AsyncPanZoomController.cpp
@@ -65,16 +65,17 @@
 #include "nsMathUtils.h"                // for NS_hypot
 #include "nsPoint.h"                    // for nsIntPoint
 #include "nsStyleConsts.h"
 #include "nsStyleStruct.h"              // for nsTimingFunction
 #include "nsTArray.h"                   // for nsTArray, nsTArray_Impl, etc
 #include "nsThreadUtils.h"              // for NS_IsMainThread
 #include "prsystem.h"                   // for PR_GetPhysicalMemorySize
 #include "SharedMemoryBasic.h"          // for SharedMemoryBasic
+#include "ScrollSnap.h"                 // for ScrollSnapUtils
 #include "WheelScrollAnimation.h"
 
 #define ENABLE_APZC_LOGGING 0
 // #define ENABLE_APZC_LOGGING 1
 
 #if ENABLE_APZC_LOGGING
 #  define APZC_LOG(...) printf_stderr("APZC: " __VA_ARGS__)
 #  define APZC_LOG_FM(fm, prefix, ...) \
@@ -686,17 +687,23 @@ public:
     if (!continueX && !continueY) {
       // If we got into overscroll from a fling, that fling did not request a
       // fling snap to avoid a resulting scrollTo from cancelling the overscroll
       // animation too early. We do still want to request a fling snap, though,
       // in case the end of the axis at which we're overscrolled is not a valid
       // snap point, so we request one now. If there are no snap points, this will
       // do nothing. If there are snap points, we'll get a scrollTo that snaps us
       // back to the nearest valid snap point.
-      mApzc.RequestSnap();
+      // The scroll snapping is done in a deferred task, otherwise the state
+      // change to NOTHING caused by the overscroll animation ending would
+      // clobber a possible state change to SMOOTH_SCROLL in ScrollSnap().
+      if (!mDeferredTasks.append(NewRunnableMethod(&mApzc,
+                                                   &AsyncPanZoomController::ScrollSnap))) {
+        MOZ_CRASH();
+      }
       return false;
     }
     return true;
   }
 
   virtual bool WantsRepaints() override
   {
     return false;
@@ -919,17 +926,17 @@ AsyncPanZoomController::GetInputQueue() 
   return mInputQueue;
 }
 
 void
 AsyncPanZoomController::Destroy()
 {
   APZThreadUtils::AssertOnCompositorThread();
 
-  CancelAnimation(CancelAnimationFlags::RequestSnap);
+  CancelAnimation(CancelAnimationFlags::ScrollSnap);
 
   { // scope the lock
     MonitorAutoLock lock(mRefPtrMonitor);
     mGeckoContentController = nullptr;
     mGestureEventListener = nullptr;
   }
   mParent = nullptr;
   mTreeManager = nullptr;
@@ -1545,46 +1552,47 @@ nsEventStatus AsyncPanZoomController::On
   if (HasReadyTouchBlock() && !CurrentTouchBlock()->TouchActionAllowsPinchZoom()) {
     return nsEventStatus_eIgnore;
   }
 
   SetState(NOTHING);
 
   {
     ReentrantMonitorAutoEnter lock(mMonitor);
+    ScheduleComposite();
+    RequestContentRepaint();
+    UpdateSharedCompositorFrameMetrics();
+  }
+
+  // Non-negative focus point would indicate that one finger is still down
+  if (aEvent.mFocusPoint.x != -1 && aEvent.mFocusPoint.y != -1) {
+    mPanDirRestricted = false;
+    mX.StartTouch(aEvent.mFocusPoint.x, aEvent.mTime);
+    mY.StartTouch(aEvent.mFocusPoint.y, aEvent.mTime);
+    SetState(TOUCHING);
+  } else {
+    // Otherwise, handle the fingers being lifted.
+    ReentrantMonitorAutoEnter lock(mMonitor);
 
     // We can get into a situation where we are overscrolled at the end of a
     // pinch if we go into overscroll with a two-finger pan, and then turn
     // that into a pinch by increasing the span sufficiently. In such a case,
     // there is no snap-back animation to get us out of overscroll, so we need
     // to get out of it somehow.
     // Moreover, in cases of scroll handoff, the overscroll can be on an APZC
     // further up in the handoff chain rather than on the current APZC, so
     // we need to clear overscroll along the entire handoff chain.
     if (HasReadyTouchBlock()) {
       CurrentTouchBlock()->GetOverscrollHandoffChain()->ClearOverscroll();
     } else {
       ClearOverscroll();
     }
     // Along with clearing the overscroll, we also want to snap to the nearest
-    // snap point as appropriate, so ask the main thread (which knows about such
-    // things) to handle it.
-    RequestSnap();
-
-    ScheduleComposite();
-    RequestContentRepaint();
-    UpdateSharedCompositorFrameMetrics();
-  }
-
-  // Non-negative focus point would indicate that one finger is still down
-  if (aEvent.mFocusPoint.x != -1 && aEvent.mFocusPoint.y != -1) {
-    mPanDirRestricted = false;
-    mX.StartTouch(aEvent.mFocusPoint.x, aEvent.mTime);
-    mY.StartTouch(aEvent.mFocusPoint.y, aEvent.mTime);
-    SetState(TOUCHING);
+    // snap point as appropriate.
+    ScrollSnap();
   }
 
   return nsEventStatus_eConsumeNoDefault;
 }
 
 bool
 AsyncPanZoomController::ConvertToGecko(const ScreenIntPoint& aPoint, CSSPoint* aOut)
 {
@@ -1998,32 +2006,32 @@ nsEventStatus AsyncPanZoomController::On
   if (!overscrollHandoffChain->CanScrollInDirection(this, Layer::VERTICAL)) {
     mY.SetVelocity(0);
   }
 
   SetState(NOTHING);
   RequestContentRepaint();
 
   if (!aEvent.mFollowedByMomentum) {
-    RequestSnap();
+    ScrollSnap();
   }
 
   return nsEventStatus_eConsumeNoDefault;
 }
 
 nsEventStatus AsyncPanZoomController::OnPanMomentumStart(const PanGestureInput& aEvent) {
   APZC_LOG("%p got a pan-momentumstart in state %d\n", this, mState);
 
   if (mState == SMOOTH_SCROLL) {
     // SMOOTH_SCROLL scrolls are cancelled by pan gestures.
     CancelAnimation();
   }
 
   SetState(PAN_MOMENTUM);
-  RequestSnapToDestination();
+  ScrollSnapToDestination();
 
   // Call into OnPan in order to process any delta included in this event.
   OnPan(aEvent, false);
 
   return nsEventStatus_eConsumeNoDefault;
 }
 
 nsEventStatus AsyncPanZoomController::OnPanMomentumEnd(const PanGestureInput& aEvent) {
@@ -2514,61 +2522,28 @@ void AsyncPanZoomController::AcceptFling
   if (mX.CanScroll()) {
     mX.SetVelocity(mX.GetVelocity() + aHandoffState.mVelocity.x);
     aHandoffState.mVelocity.x = 0;
   }
   if (mY.CanScroll()) {
     mY.SetVelocity(mY.GetVelocity() + aHandoffState.mVelocity.y);
     aHandoffState.mVelocity.y = 0;
   }
-  SetState(FLING);
-  FlingAnimation *fling = new FlingAnimation(*this,
-      aHandoffState.mChain,
-      !aHandoffState.mIsHandoff,  // only apply acceleration if this is an initial fling
-      aHandoffState.mScrolledApzc);
-  RequestSnapToDestination();
-  StartAnimation(fling);
-}
-
-void
-AsyncPanZoomController::RequestSnapToDestination()
-{
-  ReentrantMonitorAutoEnter lock(mMonitor);
-
-  float friction = gfxPrefs::APZFlingFriction();
-  ParentLayerPoint velocity(mX.GetVelocity(), mY.GetVelocity());
-  ParentLayerPoint predictedDelta;
-  // "-velocity / log(1.0 - friction)" is the integral of the deceleration
-  // curve modeled for flings in the "Axis" class.
-  if (velocity.x != 0.0f) {
-    predictedDelta.x = -velocity.x / log(1.0 - friction);
-  }
-  if (velocity.y != 0.0f) {
-    predictedDelta.y = -velocity.y / log(1.0 - friction);
-  }
-  CSSPoint predictedDestination = mFrameMetrics.GetScrollOffset() + predictedDelta / mFrameMetrics.GetZoom();
-
-  // If the fling will overscroll, don't request a fling snap, because the
-  // resulting content scrollTo() would unnecessarily cancel the overscroll
-  // animation.
-  bool flingWillOverscroll = IsOverscrolled() && ((velocity.x * mX.GetOverscroll() >= 0) ||
-                                                  (velocity.y * mY.GetOverscroll() >= 0));
-  if (!flingWillOverscroll) {
-    RefPtr<GeckoContentController> controller = GetGeckoContentController();
-    if (controller) {
-      APZC_LOG("%p fling snapping.  friction: %f velocity: %f, %f "
-               "predictedDelta: %f, %f position: %f, %f "
-               "predictedDestination: %f, %f\n",
-               this, friction, velocity.x, velocity.y, (float)predictedDelta.x,
-               (float)predictedDelta.y, (float)mFrameMetrics.GetScrollOffset().x,
-               (float)mFrameMetrics.GetScrollOffset().y,
-               (float)predictedDestination.x, (float)predictedDestination.y);
-      controller->RequestFlingSnap(mFrameMetrics.GetScrollId(),
-                                   predictedDestination);
-    }
+
+  // If there's a scroll snap point near the predicted fling destination,
+  // scroll there using a smooth scroll animation. Otherwise, start a
+  // fling animation.
+  ScrollSnapToDestination();
+  if (mState != SMOOTH_SCROLL) {
+    SetState(FLING);
+    FlingAnimation *fling = new FlingAnimation(*this,
+        aHandoffState.mChain,
+        !aHandoffState.mIsHandoff,  // only apply acceleration if this is an initial fling
+        aHandoffState.mScrolledApzc);
+    StartAnimation(fling);
   }
 }
 
 bool AsyncPanZoomController::AttemptFling(FlingHandoffState& aHandoffState) {
   // If we are pannable, take over the fling ourselves.
   if (IsPannable()) {
     AcceptFling(aHandoffState);
     return true;
@@ -2703,19 +2678,19 @@ void AsyncPanZoomController::CancelAnima
   // Setting the state to nothing and cancelling the animation can
   // preempt normal mechanisms for relieving overscroll, so we need to clear
   // overscroll here.
   if (!(aFlags & ExcludeOverscroll) && IsOverscrolled()) {
     ClearOverscroll();
     repaint = true;
   }
   // Similar to relieving overscroll, we also need to snap to any snap points
-  // if appropriate, so ask the main thread to do that.
-  if (aFlags & CancelAnimationFlags::RequestSnap) {
-    RequestSnap();
+  // if appropriate.
+  if (aFlags & CancelAnimationFlags::ScrollSnap) {
+    ScrollSnap();
   }
   if (repaint) {
     RequestContentRepaint();
     ScheduleComposite();
     UpdateSharedCompositorFrameMetrics();
   }
 }
 
@@ -2905,17 +2880,17 @@ bool AsyncPanZoomController::SnapBackIfO
     APZC_LOG("%p is overscrolled, starting snap-back\n", this);
     StartOverscrollAnimation(ParentLayerPoint(0, 0));
     return true;
   }
   // If we don't kick off an overscroll animation, we still need to ask the
   // main thread to snap to any nearby snap points, assuming we haven't already
   // done so when we started this fling
   if (mState != FLING) {
-    RequestSnap();
+    ScrollSnap();
   }
   return false;
 }
 
 bool AsyncPanZoomController::IsFlingingFast() const {
   ReentrantMonitorAutoEnter lock(mMonitor);
   if (mState == FLING &&
       GetVelocityVector().Length() > gfxPrefs::APZFlingStopOnTapThreshold()) {
@@ -3747,17 +3722,17 @@ AsyncPanZoomController::ResetTouchInputS
   }
 }
 
 void
 AsyncPanZoomController::CancelAnimationAndGestureState()
 {
   mX.CancelGesture();
   mY.CancelGesture();
-  CancelAnimation(CancelAnimationFlags::RequestSnap);
+  CancelAnimation(CancelAnimationFlags::ScrollSnap);
 }
 
 bool
 AsyncPanZoomController::HasReadyTouchBlock() const
 {
   return GetInputQueue()->HasReadyTouchBlock();
 }
 
@@ -3924,19 +3899,69 @@ void AsyncPanZoomController::ShareCompos
       // so the content process know which APZC sent this shared FrameMetrics.
       if (!compositor->SendSharedCompositorFrameMetrics(mem, handle, mLayersId, mAPZCId)) {
         APZC_LOG("%p failed to share FrameMetrics with content process.", this);
       }
     }
   }
 }
 
-void AsyncPanZoomController::RequestSnap() {
-  if (RefPtr<GeckoContentController> controller = GetGeckoContentController()) {
-    APZC_LOG("%p requesting snap near %s\n", this,
-        Stringify(mFrameMetrics.GetScrollOffset()).c_str());
-    controller->RequestFlingSnap(mFrameMetrics.GetScrollId(),
-                                 mFrameMetrics.GetScrollOffset());
+void AsyncPanZoomController::ScrollSnapNear(const CSSPoint& aDestination) {
+  mMonitor.AssertCurrentThreadIn();
+  APZC_LOG("%p scroll snapping near %s\n", this, Stringify(aDestination).c_str());
+  CSSRect scrollRange = mFrameMetrics.CalculateScrollRange();
+  if (Maybe<nsPoint> snapPoint = ScrollSnapUtils::GetSnapPointForDestination(
+          mScrollMetadata.GetSnapInfo(),
+          nsIScrollableFrame::DEVICE_PIXELS,
+          CSSSize::ToAppUnits(mFrameMetrics.CalculateCompositedSizeInCssPixels()),
+          CSSRect::ToAppUnits(scrollRange),
+          CSSPoint::ToAppUnits(mFrameMetrics.GetScrollOffset()),
+          CSSPoint::ToAppUnits(aDestination))) {
+    CSSPoint cssSnapPoint = CSSPoint::FromAppUnits(snapPoint.ref());
+    // GetSnapPointForDestination() can produce a destination that's outside
+    // of the scroll frame's scroll range. Clamp it here (this matches the
+    // behaviour of the main-thread code path, which clamps it in
+    // nsGfxScrollFrame::ScrollTo()).
+    cssSnapPoint = scrollRange.ClampPoint(cssSnapPoint);
+    SmoothScrollTo(cssSnapPoint);
+  }
+}
+
+void AsyncPanZoomController::ScrollSnap() {
+  ReentrantMonitorAutoEnter lock(mMonitor);
+  ScrollSnapNear(mFrameMetrics.GetScrollOffset());
+}
+
+void AsyncPanZoomController::ScrollSnapToDestination() {
+  ReentrantMonitorAutoEnter lock(mMonitor);
+
+  float friction = gfxPrefs::APZFlingFriction();
+  ParentLayerPoint velocity(mX.GetVelocity(), mY.GetVelocity());
+  ParentLayerPoint predictedDelta;
+  // "-velocity / log(1.0 - friction)" is the integral of the deceleration
+  // curve modeled for flings in the "Axis" class.
+  if (velocity.x != 0.0f) {
+    predictedDelta.x = -velocity.x / log(1.0 - friction);
+  }
+  if (velocity.y != 0.0f) {
+    predictedDelta.y = -velocity.y / log(1.0 - friction);
+  }
+  CSSPoint predictedDestination = mFrameMetrics.GetScrollOffset() + predictedDelta / mFrameMetrics.GetZoom();
+
+  // If the fling will overscroll, don't scroll snap, because then the user
+  // user would not see any overscroll animation.
+  bool flingWillOverscroll = IsOverscrolled() && ((velocity.x * mX.GetOverscroll() >= 0) ||
+                                                  (velocity.y * mY.GetOverscroll() >= 0));
+  if (!flingWillOverscroll) {
+    APZC_LOG("%p fling snapping.  friction: %f velocity: %f, %f "
+             "predictedDelta: %f, %f position: %f, %f "
+             "predictedDestination: %f, %f\n",
+             this, friction, velocity.x, velocity.y, (float)predictedDelta.x,
+             (float)predictedDelta.y, (float)mFrameMetrics.GetScrollOffset().x,
+             (float)mFrameMetrics.GetScrollOffset().y,
+             (float)predictedDestination.x, (float)predictedDestination.y);
+
+    ScrollSnapNear(predictedDestination);
   }
 }
 
 } // namespace layers
 } // namespace mozilla
--- a/gfx/layers/apz/src/AsyncPanZoomController.h
+++ b/gfx/layers/apz/src/AsyncPanZoomController.h
@@ -638,23 +638,24 @@ protected:
   static AxisLockMode GetAxisLockMode();
 
   // Helper function for OnSingleTapUp() and OnSingleTapConfirmed().
   nsEventStatus GenerateSingleTap(const ScreenIntPoint& aPoint, mozilla::Modifiers aModifiers);
 
   // Common processing at the end of a touch block.
   void OnTouchEndOrCancel();
 
-  // This is called to request that the main thread snap the scroll position
-  // to a nearby snap position if appropriate. The current scroll position is
-  // used as the final destination.
-  void RequestSnap();
-  // Same as above, but takes into account the current velocity to find a
-  // predicted destination.
-  void RequestSnapToDestination();
+  // Snap to a snap position nearby the current scroll position, if appropriate.
+  void ScrollSnap();
+  // Snap to a snap position nearby the destination predicted based on the
+  // current velocity, if appropriate.
+  void ScrollSnapToDestination();
+
+  // Helper function for ScrollSnap() and ScrollSnapToDestination().
+  void ScrollSnapNear(const CSSPoint& aDestination);
 
   uint64_t mLayersId;
   RefPtr<CompositorBridgeParent> mCompositorBridgeParent;
 
   /* Access to the following two fields is protected by the mRefPtrMonitor,
      since they are accessed on the UI thread but can be cleared on the
      compositor thread. */
   RefPtr<GeckoContentController> mGeckoContentController;
--- a/layout/base/Units.h
+++ b/layout/base/Units.h
@@ -243,16 +243,21 @@ struct CSSPixel {
                    NSToCoordRoundWithClamp(aPoint.y * float(AppUnitsPerCSSPixel())));
   }
 
   static nsPoint ToAppUnits(const CSSIntPoint& aPoint) {
     return nsPoint(NSToCoordRoundWithClamp(float(aPoint.x) * float(AppUnitsPerCSSPixel())),
                    NSToCoordRoundWithClamp(float(aPoint.y) * float(AppUnitsPerCSSPixel())));
   }
 
+  static nsSize ToAppUnits(const CSSSize& aSize) {
+    return nsSize(NSToCoordRoundWithClamp(aSize.width * float(AppUnitsPerCSSPixel())),
+                  NSToCoordRoundWithClamp(aSize.height * float(AppUnitsPerCSSPixel())));
+  }
+
   static nsSize ToAppUnits(const CSSIntSize& aSize) {
     return nsSize(NSToCoordRoundWithClamp(float(aSize.width)  * float(AppUnitsPerCSSPixel())),
                   NSToCoordRoundWithClamp(float(aSize.height) * float(AppUnitsPerCSSPixel())));
   }
 
   static nsRect ToAppUnits(const CSSRect& aRect) {
     return nsRect(NSToCoordRoundWithClamp(aRect.x * float(AppUnitsPerCSSPixel())),
                   NSToCoordRoundWithClamp(aRect.y * float(AppUnitsPerCSSPixel())),
--- a/layout/generic/ScrollSnap.h
+++ b/layout/generic/ScrollSnap.h
@@ -17,16 +17,20 @@ struct ScrollSnapUtils {
    * GetSnapPointForDestination determines which point to snap to after
    * scrolling. |aStartPos| gives the position before scrolling and
    * |aDestination| gives the position after scrolling, with no snapping.
    * Behaviour is dependent on the value of |aUnit|.
    * |aSnapInfo|, |aScrollPortSize|, and |aScrollRange| are characteristics
    * of the scroll frame for which snapping is being performed.
    * If a suitable snap point could be found, it is returned. Otherwise, an
    * empty Maybe is returned.
+   * IMPORTANT NOTE: This function is designed to be called both on and off
+   *                 the main thread. If modifying its implementation, be sure
+   *                 not to touch main-thread-only data structures without
+   *                 appropriate locking.
    */
   static Maybe<nsPoint> GetSnapPointForDestination(
       const layers::ScrollSnapInfo& aSnapInfo,
       nsIScrollableFrame::ScrollUnit aUnit,
       const nsSize& aScrollPortSize,
       const nsRect& aScrollRange,
       const nsPoint& aStartPos,
       const nsPoint& aDestination);