bug 1249162 - Fix unwanted thumb shifts when starting APZ drag r=botond draft
authorKevin Wern <kevin.m.wern@gmail.com>
Sun, 27 Nov 2016 20:48:08 -0500
changeset 448752 29548fb95ec3e958d903d964753857ee949753ba
parent 445717 a69583d2dbc6fdc18f63761a89cf539c356668be
child 539364 565341efe29fccd9ac982d24376809c7839eb287
push id38421
push userbmo:kevin.m.wern@gmail.com
push dateMon, 12 Dec 2016 20:14:29 +0000
reviewersbotond
bugs1249162
milestone53.0a1
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
@@ -930,45 +930,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;
   }