From e4d8f7fbe89c94cc8b4f9c8b70c646ae1e36bd8d Mon Sep 17 00:00:00 2001 From: Kevin Wern Date: Sun, 27 Nov 2016 20:48:08 -0500 Subject: [PATCH] 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 --HG-- extra : rebase_source : 29548fb95ec3e958d903d964753857ee949753ba --- gfx/layers/LayerMetricsWrapper.h | 2 +- gfx/layers/apz/src/APZCTreeManager.cpp | 4 ++-- gfx/layers/apz/src/AsyncDragMetrics.h | 8 +++---- gfx/layers/apz/src/AsyncPanZoomController.cpp | 20 +++++++++++------- gfx/layers/apz/src/HitTestingTreeNode.cpp | 12 +++++------ gfx/layers/apz/src/HitTestingTreeNode.h | 6 +++--- layout/xul/nsSliderFrame.cpp | 21 +++++++++++++------ 7 files changed, 43 insertions(+), 30 deletions(-) diff --git a/gfx/layers/LayerMetricsWrapper.h b/gfx/layers/LayerMetricsWrapper.h index d2691563360a..bfe3fd3a8987 100644 --- a/gfx/layers/LayerMetricsWrapper.h +++ b/gfx/layers/LayerMetricsWrapper.h @@ -423,7 +423,7 @@ public: return mLayer->GetScrollbarTargetContainerId(); } - int32_t GetScrollbarSize() const + int32_t GetScrollThumbLength() const { if (GetScrollbarDirection() == Layer::VERTICAL) { return mLayer->GetVisibleRegion().GetBounds().height; diff --git a/gfx/layers/apz/src/APZCTreeManager.cpp b/gfx/layers/apz/src/APZCTreeManager.cpp index 857ae5958235..4f75ae034869 100644 --- a/gfx/layers/apz/src/APZCTreeManager.cpp +++ b/gfx/layers/apz/src/APZCTreeManager.cpp @@ -478,7 +478,7 @@ APZCTreeManager::PrepareNodeForLayer(const LayerMetricsWrapper& aLayer, GetEventRegionsOverride(aParent, aLayer)); node->SetScrollbarData(aLayer.GetScrollbarTargetContainerId(), aLayer.GetScrollbarDirection(), - aLayer.GetScrollbarSize(), + aLayer.GetScrollThumbLength(), aLayer.IsScrollbarContainer()); node->SetFixedPosData(aLayer.GetFixedPositionScrollContainerId()); return node; @@ -665,7 +665,7 @@ APZCTreeManager::PrepareNodeForLayer(const LayerMetricsWrapper& aLayer, node->SetScrollbarData(aLayer.GetScrollbarTargetContainerId(), aLayer.GetScrollbarDirection(), - aLayer.GetScrollbarSize(), + aLayer.GetScrollThumbLength(), aLayer.IsScrollbarContainer()); node->SetFixedPosData(aLayer.GetFixedPositionScrollContainerId()); return node; diff --git a/gfx/layers/apz/src/AsyncDragMetrics.h b/gfx/layers/apz/src/AsyncDragMetrics.h index 54b60f823074..cd763a243e55 100644 --- a/gfx/layers/apz/src/AsyncDragMetrics.h +++ b/gfx/layers/apz/src/AsyncDragMetrics.h @@ -40,8 +40,8 @@ public: 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) @@ -54,8 +54,8 @@ public: FrameMetrics::ViewID mViewId; uint32_t mPresShellId; uint64_t mDragStartSequenceNumber; - CSSIntCoord mScrollbarDragOffset; - CSSIntRect mScrollTrack; + CSSCoord mScrollbarDragOffset; + CSSRect mScrollTrack; DragDirection mDirection; }; diff --git a/gfx/layers/apz/src/AsyncPanZoomController.cpp b/gfx/layers/apz/src/AsyncPanZoomController.cpp index 8b07bb2a5ab6..a72a3fa69713 100644 --- a/gfx/layers/apz/src/AsyncPanZoomController.cpp +++ b/gfx/layers/apz/src/AsyncPanZoomController.cpp @@ -861,7 +861,7 @@ static IntCoordTyped GetAxisStart(AsyncDragMetrics::DragDirection aDir, c } template -static IntCoordTyped GetAxisEnd(AsyncDragMetrics::DragDirection aDir, const IntRectTyped& aValue) { +static CoordTyped GetAxisEnd(AsyncDragMetrics::DragDirection aDir, const RectTyped& aValue) { if (aDir == AsyncDragMetrics::HORIZONTAL) { return aValue.x + aValue.width; } else { @@ -870,7 +870,7 @@ static IntCoordTyped GetAxisEnd(AsyncDragMetrics::DragDirection aDir, con } template -static CoordTyped GetAxisSize(AsyncDragMetrics::DragDirection aDir, const RectTyped& aValue) { +static CoordTyped GetAxisLength(AsyncDragMetrics::DragDirection aDir, const RectTyped& aValue) { if (aDir == AsyncDragMetrics::HORIZONTAL) { return aValue.width; } else { @@ -898,6 +898,10 @@ nsEventStatus AsyncPanZoomController::HandleDragEvent(const MouseInput& aEvent, return nsEventStatus_eConsumeNoDefault; } + if (aEvent.mType != MouseInput::MouseType::MOUSE_MOVE) { + return nsEventStatus_eConsumeNoDefault; + } + RefPtr node = GetApzcTreeManager()->FindScrollNode(aDragMetrics); if (!node) { @@ -915,12 +919,12 @@ nsEventStatus AsyncPanZoomController::HandleDragEvent(const MouseInput& aEvent, 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)); + GetAxisStart(aDragMetrics.mDirection, aDragMetrics.mScrollTrack); - CSSCoord scrollMax = CSSCoord(GetAxisEnd(aDragMetrics.mDirection, aDragMetrics.mScrollTrack)); - scrollMax -= node->GetScrollSize() / + CSSCoord scrollMax = GetAxisLength(aDragMetrics.mDirection, aDragMetrics.mScrollTrack); + scrollMax -= node->GetScrollThumbLength() / GetAxisScale(aDragMetrics.mDirection, mFrameMetrics.GetZoom()) * mFrameMetrics.GetPresShellResolution(); @@ -929,8 +933,8 @@ nsEventStatus AsyncPanZoomController::HandleDragEvent(const MouseInput& aEvent, 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); diff --git a/gfx/layers/apz/src/HitTestingTreeNode.cpp b/gfx/layers/apz/src/HitTestingTreeNode.cpp index acedcde5d77f..a5a5435017ba 100644 --- a/gfx/layers/apz/src/HitTestingTreeNode.cpp +++ b/gfx/layers/apz/src/HitTestingTreeNode.cpp @@ -27,7 +27,7 @@ HitTestingTreeNode::HitTestingTreeNode(AsyncPanZoomController* aApzc, , mLayersId(aLayersId) , mScrollViewId(FrameMetrics::NULL_SCROLL_ID) , mScrollDir(Layer::NONE) - , mScrollSize(0) + , mScrollThumbLength(0) , mIsScrollbarContainer(false) , mFixedPosTarget(FrameMetrics::NULL_SCROLL_ID) , mOverride(EventRegionsOverride::NoOverride) @@ -96,12 +96,12 @@ HitTestingTreeNode::SetLastChild(HitTestingTreeNode* aChild) 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; } @@ -115,10 +115,10 @@ HitTestingTreeNode::MatchesScrollDragMetrics(const AsyncDragMetrics& aDragMetric mScrollViewId == aDragMetrics.mViewId; } -int32_t -HitTestingTreeNode::GetScrollSize() const +LayerIntCoord +HitTestingTreeNode::GetScrollThumbLength() const { - return mScrollSize; + return mScrollThumbLength; } bool diff --git a/gfx/layers/apz/src/HitTestingTreeNode.h b/gfx/layers/apz/src/HitTestingTreeNode.h index 442751a8d6ec..e994c18b0956 100644 --- a/gfx/layers/apz/src/HitTestingTreeNode.h +++ b/gfx/layers/apz/src/HitTestingTreeNode.h @@ -93,10 +93,10 @@ public: 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 */ @@ -130,7 +130,7 @@ private: FrameMetrics::ViewID mScrollViewId; Layer::ScrollDirection mScrollDir; - int32_t mScrollSize; + int32_t mScrollThumbLength; bool mIsScrollbarContainer; FrameMetrics::ViewID mFixedPosTarget; diff --git a/layout/xul/nsSliderFrame.cpp b/layout/xul/nsSliderFrame.cpp index 5c90fe18b008..4185d695ae90 100644 --- a/layout/xul/nsSliderFrame.cpp +++ b/layout/xul/nsSliderFrame.cpp @@ -936,16 +936,21 @@ nsSliderFrame::StartAPZDrag() 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); @@ -957,14 +962,18 @@ nsSliderFrame::StartAPZDrag() nsIFrame* scrollbarBox = GetScrollbar(); nsCOMPtr 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 :