bug 1249162 - Fix unwanted thumb shifts when starting APZ drag r=botond
authorKevin Wern <kevin.m.wern@gmail.com>
Sun, 27 Nov 2016 20:48:08 -0500
changeset 325660 fc1a3d4fd03c44ee9473556c2086b6f5bc722558
parent 325659 e728e6be59dc74e61e48e636aa0d4bedb5c9be67
child 325661 4d6f11e2834346a28d02d139cdbdb489357f1fa7
push id31069
push usercbook@mozilla.com
push dateTue, 13 Dec 2016 14:53:36 +0000
treeherdermozilla-central@fee42adb860e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbotond
bugs1249162
milestone53.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 1249162 - Fix unwanted thumb shifts when starting APZ drag r=botond Fix thumb position determination in these places: - nsSliderFrame::StartAPZDrag() -- Constrain sliderTrack using GetXULClientRect() to match dimensions used in SetCurrentThumbPosition() and DoXULLayout(). This is what caused the scaling/offset mismatch. - nsSliderFrame::StartAPZDrag() -- Adjust nonsensical calculation of cssSliderTrack. Should be sliderFrame + scrollbarFramePosition - compositionBoundsPosition, to get coordinates relative to the same region as that of APZ event coordinates. - AsyncDragMetrics -- Make mScrollTrack and mScrollbarDragOffset float instead of int coordinates. - AsyncPanZoomController::HandleDragEvent() -- Use GetAxisLength(scrollTrack) instead of GetAxisEnd(scrollTrack) in calculation of scrollMax. - AsyncPanZoomController::HandleDragEvent() -- Only change position on MOUSE_MOVE. Additional refactors: - Rename HitTestingTreeNode::GetScrollSize() to GetScrollThumbLength(), along with related functions/variables. - Rename AsyncPanZoomController::GetAxisSize() to GetAxisLength(). - Rename cf to scrollFrame in nsSliderFrame::StartAPZDrag(). MozReview-Commit-ID: CIsU8Pj6qfa
gfx/layers/LayerMetricsWrapper.h
gfx/layers/apz/src/APZCTreeManager.cpp
gfx/layers/apz/src/AsyncDragMetrics.h
gfx/layers/apz/src/AsyncPanZoomController.cpp
gfx/layers/apz/src/HitTestingTreeNode.cpp
gfx/layers/apz/src/HitTestingTreeNode.h
layout/xul/nsSliderFrame.cpp
--- a/gfx/layers/LayerMetricsWrapper.h
+++ b/gfx/layers/LayerMetricsWrapper.h
@@ -418,17 +418,17 @@ public:
 
   FrameMetrics::ViewID GetScrollbarTargetContainerId() const
   {
     MOZ_ASSERT(IsValid());
 
     return mLayer->GetScrollbarTargetContainerId();
   }
 
-  int32_t GetScrollbarSize() const
+  int32_t GetScrollThumbLength() const
   {
     if (GetScrollbarDirection() == Layer::VERTICAL) {
       return mLayer->GetVisibleRegion().GetBounds().height;
     } else {
       return mLayer->GetVisibleRegion().GetBounds().width;
     }
   }
 
--- a/gfx/layers/apz/src/APZCTreeManager.cpp
+++ b/gfx/layers/apz/src/APZCTreeManager.cpp
@@ -473,17 +473,17 @@ APZCTreeManager::PrepareNodeForLayer(con
     AttachNodeToTree(node, aParent, aNextSibling);
     node->SetHitTestData(
         GetEventRegions(aLayer),
         aLayer.GetTransformTyped(),
         aLayer.GetClipRect() ? Some(ParentLayerIntRegion(*aLayer.GetClipRect())) : Nothing(),
         GetEventRegionsOverride(aParent, aLayer));
     node->SetScrollbarData(aLayer.GetScrollbarTargetContainerId(),
                            aLayer.GetScrollbarDirection(),
-                           aLayer.GetScrollbarSize(),
+                           aLayer.GetScrollThumbLength(),
                            aLayer.IsScrollbarContainer());
     node->SetFixedPosData(aLayer.GetFixedPositionScrollContainerId());
     return node;
   }
 
   AsyncPanZoomController* apzc = nullptr;
   // If we get here, aLayer is a scrollable layer and somebody
   // has registered a GeckoContentController for it, so we need to ensure
@@ -660,17 +660,17 @@ APZCTreeManager::PrepareNodeForLayer(con
         GetEventRegions(aLayer),
         aLayer.GetTransformTyped(),
         Some(clipRegion),
         GetEventRegionsOverride(aParent, aLayer));
   }
 
   node->SetScrollbarData(aLayer.GetScrollbarTargetContainerId(),
                          aLayer.GetScrollbarDirection(),
-                         aLayer.GetScrollbarSize(),
+                         aLayer.GetScrollThumbLength(),
                          aLayer.IsScrollbarContainer());
   node->SetFixedPosData(aLayer.GetFixedPositionScrollContainerId());
   return node;
 }
 
 template<typename PanGestureOrScrollWheelInput>
 static bool
 WillHandleInput(const PanGestureOrScrollWheelInput& aPanInput)
--- a/gfx/layers/apz/src/AsyncDragMetrics.h
+++ b/gfx/layers/apz/src/AsyncDragMetrics.h
@@ -35,31 +35,31 @@ public:
     , mDragStartSequenceNumber(0)
     , mScrollbarDragOffset(0)
     , mDirection(NONE)
   {}
 
   AsyncDragMetrics(const FrameMetrics::ViewID& aViewId,
                    uint32_t aPresShellId,
                    uint64_t aDragStartSequenceNumber,
-                   CSSIntCoord aScrollbarDragOffset,
-                   const CSSIntRect& aScrollTrack,
+                   CSSCoord aScrollbarDragOffset,
+                   const CSSRect& aScrollTrack,
                    DragDirection aDirection)
     : mViewId(aViewId)
     , mPresShellId(aPresShellId)
     , mDragStartSequenceNumber(aDragStartSequenceNumber)
     , mScrollbarDragOffset(aScrollbarDragOffset)
     , mScrollTrack(aScrollTrack)
     , mDirection(aDirection)
   {}
 
   FrameMetrics::ViewID mViewId;
   uint32_t mPresShellId;
   uint64_t mDragStartSequenceNumber;
-  CSSIntCoord mScrollbarDragOffset;
-  CSSIntRect mScrollTrack;
+  CSSCoord mScrollbarDragOffset;
+  CSSRect mScrollTrack;
   DragDirection mDirection;
 };
 
 }
 }
 
 #endif
--- a/gfx/layers/apz/src/AsyncPanZoomController.cpp
+++ b/gfx/layers/apz/src/AsyncPanZoomController.cpp
@@ -856,26 +856,26 @@ static IntCoordTyped<Units> GetAxisStart
   if (aDir == AsyncDragMetrics::HORIZONTAL) {
     return aValue.x;
   } else {
     return aValue.y;
   }
 }
 
 template <typename Units>
-static IntCoordTyped<Units> GetAxisEnd(AsyncDragMetrics::DragDirection aDir, const IntRectTyped<Units>& aValue) {
+static CoordTyped<Units> GetAxisEnd(AsyncDragMetrics::DragDirection aDir, const RectTyped<Units>& aValue) {
   if (aDir == AsyncDragMetrics::HORIZONTAL) {
     return aValue.x + aValue.width;
   } else {
     return aValue.y + aValue.height;
   }
 }
 
 template <typename Units>
-static CoordTyped<Units> GetAxisSize(AsyncDragMetrics::DragDirection aDir, const RectTyped<Units>& aValue) {
+static CoordTyped<Units> GetAxisLength(AsyncDragMetrics::DragDirection aDir, const RectTyped<Units>& aValue) {
   if (aDir == AsyncDragMetrics::HORIZONTAL) {
     return aValue.width;
   } else {
     return aValue.height;
   }
 }
 
 template <typename FromUnits, typename ToUnits>
@@ -893,16 +893,20 @@ nsEventStatus AsyncPanZoomController::Ha
   if (!gfxPrefs::APZDragEnabled()) {
     return nsEventStatus_eIgnore;
   }
 
   if (!GetApzcTreeManager()) {
     return nsEventStatus_eConsumeNoDefault;
   }
 
+  if (aEvent.mType != MouseInput::MouseType::MOUSE_MOVE) {
+    return nsEventStatus_eConsumeNoDefault;
+  }
+
   RefPtr<HitTestingTreeNode> node =
     GetApzcTreeManager()->FindScrollNode(aDragMetrics);
   if (!node) {
     return nsEventStatus_eConsumeNoDefault;
   }
 
   mozilla::Telemetry::Accumulate(mozilla::Telemetry::SCROLL_INPUT_METHODS,
       (uint32_t) ScrollInputMethod::ApzScrollbarDrag);
@@ -910,32 +914,32 @@ nsEventStatus AsyncPanZoomController::Ha
   ReentrantMonitorAutoEnter lock(mMonitor);
   CSSPoint scrollFramePoint = aEvent.mLocalOrigin / GetFrameMetrics().GetZoom();
   // The scrollbar can be transformed with the frame but the pres shell
   // resolution is only applied to the scroll frame.
   CSSPoint scrollbarPoint = scrollFramePoint * mFrameMetrics.GetPresShellResolution();
   CSSRect cssCompositionBound = mFrameMetrics.CalculateCompositedRectInCssPixels();
 
   CSSCoord mousePosition = GetAxisStart(aDragMetrics.mDirection, scrollbarPoint) -
-                        CSSCoord(aDragMetrics.mScrollbarDragOffset) -
+                        aDragMetrics.mScrollbarDragOffset -
                         GetAxisStart(aDragMetrics.mDirection, cssCompositionBound) -
-                        CSSCoord(GetAxisStart(aDragMetrics.mDirection, aDragMetrics.mScrollTrack));
-
-  CSSCoord scrollMax = CSSCoord(GetAxisEnd(aDragMetrics.mDirection, aDragMetrics.mScrollTrack));
-  scrollMax -= node->GetScrollSize() /
+                        GetAxisStart(aDragMetrics.mDirection, aDragMetrics.mScrollTrack);
+
+  CSSCoord scrollMax = GetAxisLength(aDragMetrics.mDirection, aDragMetrics.mScrollTrack);
+  scrollMax -= node->GetScrollThumbLength() /
                GetAxisScale(aDragMetrics.mDirection, mFrameMetrics.GetZoom()) *
                mFrameMetrics.GetPresShellResolution();
 
   float scrollPercent = mousePosition / scrollMax;
 
   CSSCoord minScrollPosition =
     GetAxisStart(aDragMetrics.mDirection, mFrameMetrics.GetScrollableRect().TopLeft());
   CSSCoord maxScrollPosition =
-    GetAxisSize(aDragMetrics.mDirection, mFrameMetrics.GetScrollableRect()) -
-    GetAxisSize(aDragMetrics.mDirection, cssCompositionBound);
+    GetAxisLength(aDragMetrics.mDirection, mFrameMetrics.GetScrollableRect()) -
+    GetAxisLength(aDragMetrics.mDirection, cssCompositionBound);
   CSSCoord scrollPosition = scrollPercent * maxScrollPosition;
 
   scrollPosition = std::max(scrollPosition, minScrollPosition);
   scrollPosition = std::min(scrollPosition, maxScrollPosition);
 
   CSSPoint scrollOffset = mFrameMetrics.GetScrollOffset();
   if (aDragMetrics.mDirection == AsyncDragMetrics::HORIZONTAL) {
     scrollOffset.x = scrollPosition;
--- a/gfx/layers/apz/src/HitTestingTreeNode.cpp
+++ b/gfx/layers/apz/src/HitTestingTreeNode.cpp
@@ -22,17 +22,17 @@ namespace layers {
 HitTestingTreeNode::HitTestingTreeNode(AsyncPanZoomController* aApzc,
                                        bool aIsPrimaryHolder,
                                        uint64_t aLayersId)
   : mApzc(aApzc)
   , mIsPrimaryApzcHolder(aIsPrimaryHolder)
   , mLayersId(aLayersId)
   , mScrollViewId(FrameMetrics::NULL_SCROLL_ID)
   , mScrollDir(Layer::NONE)
-  , mScrollSize(0)
+  , mScrollThumbLength(0)
   , mIsScrollbarContainer(false)
   , mFixedPosTarget(FrameMetrics::NULL_SCROLL_ID)
   , mOverride(EventRegionsOverride::NoOverride)
 {
 if (mIsPrimaryApzcHolder) {
     MOZ_ASSERT(mApzc);
   }
   MOZ_ASSERT(!mApzc || mApzc->GetLayersId() == mLayersId);
@@ -91,39 +91,39 @@ HitTestingTreeNode::SetLastChild(HitTest
       aChild->SetApzcParent(parent);
     }
   }
 }
 
 void
 HitTestingTreeNode::SetScrollbarData(FrameMetrics::ViewID aScrollViewId,
                                      Layer::ScrollDirection aDir,
-                                     int32_t aScrollSize,
+                                     int32_t aScrollThumbLength,
                                      bool aIsScrollContainer)
 {
   mScrollViewId = aScrollViewId;
   mScrollDir = aDir;
-  mScrollSize = aScrollSize;;
+  mScrollThumbLength = aScrollThumbLength;
   mIsScrollbarContainer = aIsScrollContainer;
 }
 
 bool
 HitTestingTreeNode::MatchesScrollDragMetrics(const AsyncDragMetrics& aDragMetrics) const
 {
   return ((mScrollDir == Layer::HORIZONTAL &&
            aDragMetrics.mDirection == AsyncDragMetrics::HORIZONTAL) ||
           (mScrollDir == Layer::VERTICAL &&
            aDragMetrics.mDirection == AsyncDragMetrics::VERTICAL)) &&
          mScrollViewId == aDragMetrics.mViewId;
 }
 
-int32_t
-HitTestingTreeNode::GetScrollSize() const
+LayerIntCoord
+HitTestingTreeNode::GetScrollThumbLength() const
 {
-  return mScrollSize;
+  return mScrollThumbLength;
 }
 
 bool
 HitTestingTreeNode::IsScrollbarNode() const
 {
   return mIsScrollbarContainer || (mScrollDir != Layer::NONE);
 }
 
--- a/gfx/layers/apz/src/HitTestingTreeNode.h
+++ b/gfx/layers/apz/src/HitTestingTreeNode.h
@@ -88,20 +88,20 @@ public:
                       const Maybe<ParentLayerIntRegion>& aClipRegion,
                       const EventRegionsOverride& aOverride);
   bool IsOutsideClip(const ParentLayerPoint& aPoint) const;
 
   /* Scrollbar info */
 
   void SetScrollbarData(FrameMetrics::ViewID aScrollViewId,
                         Layer::ScrollDirection aDir,
-                        int32_t aScrollSize,
+                        int32_t aScrollThumbLength,
                         bool aIsScrollContainer);
   bool MatchesScrollDragMetrics(const AsyncDragMetrics& aDragMetrics) const;
-  int32_t GetScrollSize() const;
+  LayerIntCoord GetScrollThumbLength() const;
   bool IsScrollbarNode() const;
 
   /* Fixed pos info */
 
   void SetFixedPosData(FrameMetrics::ViewID aFixedPosTarget);
   FrameMetrics::ViewID GetFixedPosTarget() const;
 
   /* Convert aPoint into the LayerPixel space for the layer corresponding to
@@ -125,17 +125,17 @@ private:
 
   RefPtr<AsyncPanZoomController> mApzc;
   bool mIsPrimaryApzcHolder;
 
   uint64_t mLayersId;
 
   FrameMetrics::ViewID mScrollViewId;
   Layer::ScrollDirection mScrollDir;
-  int32_t mScrollSize;
+  int32_t mScrollThumbLength;
   bool mIsScrollbarContainer;
 
   FrameMetrics::ViewID mFixedPosTarget;
 
   /* Let {L,M} be the {layer, scrollable metrics} pair that this node
    * corresponds to in the layer tree. mEventRegions contains the event regions
    * from L, in the case where event-regions are enabled. If event-regions are
    * disabled, it will contain the visible region of L, which we use as an
--- a/layout/xul/nsSliderFrame.cpp
+++ b/layout/xul/nsSliderFrame.cpp
@@ -931,45 +931,54 @@ nsSliderMediator::HandleEvent(nsIDOMEven
 
 bool
 nsSliderFrame::StartAPZDrag()
 {
   if (!gfxPlatform::GetPlatform()->SupportsApzDragInput()) {
     return false;
   }
 
-  nsContainerFrame* cf = GetScrollbar()->GetParent();
-  if (!cf) {
+  nsContainerFrame* scrollFrame = GetScrollbar()->GetParent();
+  if (!scrollFrame) {
     return false;
   }
 
-  nsIContent* scrollableContent = cf->GetContent();
+  nsIContent* scrollableContent = scrollFrame->GetContent();
   if (!scrollableContent) {
     return false;
   }
 
+  nsIScrollableFrame* scrollFrameAsScrollable = do_QueryFrame(scrollFrame);
+  if (!scrollFrameAsScrollable) {
+    return false;
+  }
+
   mozilla::layers::FrameMetrics::ViewID scrollTargetId;
   bool hasID = nsLayoutUtils::FindIDFor(scrollableContent, &scrollTargetId);
   bool hasAPZView = hasID && (scrollTargetId != layers::FrameMetrics::NULL_SCROLL_ID);
 
   if (!hasAPZView) {
     return false;
   }
 
   nsIFrame* scrollbarBox = GetScrollbar();
   nsCOMPtr<nsIContent> scrollbar = GetContentOfBox(scrollbarBox);
 
+  nsRect sliderTrack;
+  GetXULClientRect(sliderTrack);
+
   // This rect is the range in which the scroll thumb can slide in.
-  nsRect sliderTrack = GetRect() - scrollbarBox->GetPosition();
-  CSSIntRect sliderTrackCSS = CSSIntRect::FromAppUnitsRounded(sliderTrack);
+  sliderTrack = sliderTrack + GetRect().TopLeft() + scrollbarBox->GetPosition() -
+                scrollFrameAsScrollable->GetScrollPortRect().TopLeft();
+  CSSRect sliderTrackCSS = CSSRect::FromAppUnits(sliderTrack);
 
   uint64_t inputblockId = InputAPZContext::GetInputBlockId();
   uint32_t presShellId = PresContext()->PresShell()->GetPresShellId();
   AsyncDragMetrics dragMetrics(scrollTargetId, presShellId, inputblockId,
-                               NSAppUnitsToIntPixels(mDragStart,
+                               NSAppUnitsToFloatPixels(mDragStart,
                                  float(AppUnitsPerCSSPixel())),
                                sliderTrackCSS,
                                IsXULHorizontal() ? AsyncDragMetrics::HORIZONTAL :
                                                    AsyncDragMetrics::VERTICAL);
 
   if (!nsLayoutUtils::HasDisplayPort(scrollableContent)) {
     return false;
   }