[css-scroll-snap] Pass the full target point when selecting a snap of…

…fset

https://bugs.webkit.org/show_bug.cgi?id=228023

Reviewed by Frédéric Wang.

Source/WebCore:

Pass the full proposed destination scroll offset when calling closestSnapOffset. For
now, only the component in the scroll direction is used, but eventually the other
component will be used to avoid snapping to snap areas that are entirely off the screen.

No new tests. This change is simply a refactor in preparation for a behavior
change and shouldn't change behavior itself.

* page/scrolling/ScrollSnapOffsetsInfo.cpp:
(WebCore::closestSnapOffsetWithInfoAndAxis):
(WebCore::LayoutScrollSnapOffsetsInfo::closestSnapOffset const):
(WebCore::FloatScrollSnapOffsetsInfo::closestSnapOffset const):
* page/scrolling/ScrollSnapOffsetsInfo.h:
* page/scrolling/nicosia/ScrollingTreeScrollingNodeDelegateNicosia.cpp:
(WebCore::ScrollingTreeScrollingNodeDelegateNicosia::handleWheelEvent):
* platform/ScrollAnimator.cpp:
(WebCore::ScrollAnimator::scroll):
(WebCore::ScrollAnimator::adjustScrollOffsetForSnappingIfNeeded):
* platform/ScrollAnimator.h:
* platform/ScrollController.cpp:
(WebCore::ScrollController::setNearestScrollSnapIndexForAxisAndOffset):
(WebCore::ScrollController::adjustScrollDestination):
(WebCore::ScrollController::updateActiveScrollSnapIndexForClientOffset):
(WebCore::ScrollController::resnapAfterLayout):
* platform/ScrollController.h:
* platform/ScrollSnapAnimatorState.cpp:
(WebCore::ScrollSnapAnimatorState::setupAnimationForState):
(WebCore::ScrollSnapAnimatorState::targetOffsetForStartOffset const):
* platform/ScrollSnapAnimatorState.h:
* platform/ScrollableArea.cpp:
(WebCore::ScrollableArea::doPostThumbMoveSnapping):

Source/WebKit:

* UIProcess/RemoteLayerTree/RemoteScrollingCoordinatorProxy.h:
* UIProcess/RemoteLayerTree/ios/RemoteScrollingCoordinatorProxyIOS.mm:
(WebKit::RemoteScrollingCoordinatorProxy::adjustTargetContentOffsetForSnapping):
(WebKit::RemoteScrollingCoordinatorProxy::closestSnapOffsetForMainFrameScrolling const):
* UIProcess/RemoteLayerTree/ios/ScrollingTreeScrollingNodeDelegateIOS.mm:
(-[WKScrollingNodeScrollViewDelegate scrollViewWillEndDragging:withVelocity:targetContentOffset:]):


Canonical link: https://commits.webkit.org/239866@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@280171 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog

@@ -1,3 +1,41 @@
+2021-07-22  Martin Robinson  <mrobinson@webkit.org>
+
+        [css-scroll-snap] Pass the full target point when selecting a snap offset
+        https://bugs.webkit.org/show_bug.cgi?id=228023
+
+        Reviewed by Frédéric Wang.
+
+        Pass the full proposed destination scroll offset when calling closestSnapOffset. For
+        now, only the component in the scroll direction is used, but eventually the other
+        component will be used to avoid snapping to snap areas that are entirely off the screen.
+
+        No new tests. This change is simply a refactor in preparation for a behavior
+        change and shouldn't change behavior itself.
+
+        * page/scrolling/ScrollSnapOffsetsInfo.cpp:
+        (WebCore::closestSnapOffsetWithInfoAndAxis):
+        (WebCore::LayoutScrollSnapOffsetsInfo::closestSnapOffset const):
+        (WebCore::FloatScrollSnapOffsetsInfo::closestSnapOffset const):
+        * page/scrolling/ScrollSnapOffsetsInfo.h:
+        * page/scrolling/nicosia/ScrollingTreeScrollingNodeDelegateNicosia.cpp:
+        (WebCore::ScrollingTreeScrollingNodeDelegateNicosia::handleWheelEvent):
+        * platform/ScrollAnimator.cpp:
+        (WebCore::ScrollAnimator::scroll):
+        (WebCore::ScrollAnimator::adjustScrollOffsetForSnappingIfNeeded):
+        * platform/ScrollAnimator.h:
+        * platform/ScrollController.cpp:
+        (WebCore::ScrollController::setNearestScrollSnapIndexForAxisAndOffset):
+        (WebCore::ScrollController::adjustScrollDestination):
+        (WebCore::ScrollController::updateActiveScrollSnapIndexForClientOffset):
+        (WebCore::ScrollController::resnapAfterLayout):
+        * platform/ScrollController.h:
+        * platform/ScrollSnapAnimatorState.cpp:
+        (WebCore::ScrollSnapAnimatorState::setupAnimationForState):
+        (WebCore::ScrollSnapAnimatorState::targetOffsetForStartOffset const):
+        * platform/ScrollSnapAnimatorState.h:
+        * platform/ScrollableArea.cpp:
+        (WebCore::ScrollableArea::doPostThumbMoveSnapping):
+
 2021-07-21  Alex Christensen  <achristensen@webkit.org>
 
         Add linkedOnOrAfter check for r269162

Source/WebCore/page/scrolling/ScrollSnapOffsetsInfo.cpp

@@ -90,9 +90,10 @@ static PotentialSnapPointSearchResult<UnitType> searchForPotentialSnapPoints(con
     return { previous, next, snapStop, landedInsideSnapAreaThatConsumesViewport };
 }
 
-template <typename InfoType, typename SizeType, typename LayoutType>
-static std::pair<LayoutType, std::optional<unsigned>> closestSnapOffsetWithInfoAndAxis(const InfoType& info, ScrollEventAxis axis, const SizeType& viewportSize, LayoutType scrollDestinationOffset, float velocity, std::optional<LayoutType> originalOffsetForDirectionalSnapping)
+template <typename InfoType, typename SizeType, typename LayoutType, typename PointType>
+static std::pair<LayoutType, std::optional<unsigned>> closestSnapOffsetWithInfoAndAxis(const InfoType& info, ScrollEventAxis axis, const SizeType& viewportSize, PointType scrollDestinationOffsetPoint, float velocity, std::optional<LayoutType> originalOffsetForDirectionalSnapping)
 {
+    auto scrollDestinationOffset = axis == ScrollEventAxis::Horizontal ? scrollDestinationOffsetPoint.x() : scrollDestinationOffsetPoint.y();
     const auto& snapOffsets = info.offsetsForAxis(axis);
     auto pairForNoSnapping = std::make_pair(scrollDestinationOffset, std::nullopt);
     if (snapOffsets.isEmpty())
@@ -388,13 +389,13 @@ FloatScrollSnapOffsetsInfo LayoutScrollSnapOffsetsInfo::convertUnits(float devic
 }
 
 template <> template <>
-std::pair<LayoutUnit, std::optional<unsigned>> LayoutScrollSnapOffsetsInfo::closestSnapOffset(ScrollEventAxis axis, const LayoutSize& viewportSize, LayoutUnit scrollDestinationOffset, float velocity, std::optional<LayoutUnit> originalPositionForDirectionalSnapping) const
+std::pair<LayoutUnit, std::optional<unsigned>> LayoutScrollSnapOffsetsInfo::closestSnapOffset(ScrollEventAxis axis, const LayoutSize& viewportSize, LayoutPoint scrollDestinationOffset, float velocity, std::optional<LayoutUnit> originalPositionForDirectionalSnapping) const
 {
     return closestSnapOffsetWithInfoAndAxis(*this, axis, viewportSize, scrollDestinationOffset, velocity, originalPositionForDirectionalSnapping);
 }
 
 template <> template<>
-std::pair<float, std::optional<unsigned>> FloatScrollSnapOffsetsInfo::closestSnapOffset(ScrollEventAxis axis, const FloatSize& viewportSize, float scrollDestinationOffset, float velocity, std::optional<float> originalPositionForDirectionalSnapping) const
+std::pair<float, std::optional<unsigned>> FloatScrollSnapOffsetsInfo::closestSnapOffset(ScrollEventAxis axis, const FloatSize& viewportSize, FloatPoint scrollDestinationOffset, float velocity, std::optional<float> originalPositionForDirectionalSnapping) const
 {
     return closestSnapOffsetWithInfoAndAxis(*this, axis, viewportSize, scrollDestinationOffset, velocity, originalPositionForDirectionalSnapping);
 }

Source/WebCore/page/scrolling/ScrollSnapOffsetsInfo.h

@@ -70,8 +70,8 @@ struct ScrollSnapOffsetsInfo {
     }
 
     template<typename OutputType> OutputType convertUnits(float deviceScaleFactor = 0.0) const;
-    template<typename SizeType>
-    WEBCORE_EXPORT std::pair<UnitType, std::optional<unsigned>> closestSnapOffset(ScrollEventAxis, const SizeType& viewportSize, UnitType scrollDestinationOffset, float velocity, std::optional<UnitType> originalPositionForDirectionalSnapping = std::nullopt) const;
+    template<typename SizeType, typename PointType>
+    WEBCORE_EXPORT std::pair<UnitType, std::optional<unsigned>> closestSnapOffset(ScrollEventAxis, const SizeType& viewportSize, PointType scrollDestinationOffset, float velocity, std::optional<UnitType> originalPositionForDirectionalSnapping = std::nullopt) const;
 };
 
 using LayoutScrollSnapOffsetsInfo = ScrollSnapOffsetsInfo<LayoutUnit, LayoutRect>;
@@ -80,13 +80,13 @@ using FloatScrollSnapOffsetsInfo = ScrollSnapOffsetsInfo<float, FloatRect>;
 template <> template <>
 LayoutScrollSnapOffsetsInfo FloatScrollSnapOffsetsInfo::convertUnits(float /* unusedScaleFactor */) const;
 template <> template <>
-WEBCORE_EXPORT std::pair<float, std::optional<unsigned>> FloatScrollSnapOffsetsInfo::closestSnapOffset(ScrollEventAxis, const FloatSize& viewportSize, float scrollDestinationOffset, float velocity, std::optional<float> originalPositionForDirectionalSnapping) const;
+WEBCORE_EXPORT std::pair<float, std::optional<unsigned>> FloatScrollSnapOffsetsInfo::closestSnapOffset(ScrollEventAxis, const FloatSize& viewportSize, FloatPoint scrollDestinationOffset, float velocity, std::optional<float> originalPositionForDirectionalSnapping) const;
 
 
 template <> template <>
 FloatScrollSnapOffsetsInfo LayoutScrollSnapOffsetsInfo::convertUnits(float deviceScaleFactor) const;
 template <> template <>
-WEBCORE_EXPORT std::pair<LayoutUnit, std::optional<unsigned>> LayoutScrollSnapOffsetsInfo::closestSnapOffset(ScrollEventAxis, const LayoutSize& viewportSize, LayoutUnit scrollDestinationOffset, float velocity, std::optional<LayoutUnit> originalPositionForDirectionalSnapping) const;
+WEBCORE_EXPORT std::pair<LayoutUnit, std::optional<unsigned>> LayoutScrollSnapOffsetsInfo::closestSnapOffset(ScrollEventAxis, const LayoutSize& viewportSize, LayoutPoint scrollDestinationOffset, float velocity, std::optional<LayoutUnit> originalPositionForDirectionalSnapping) const;
 
 // Update the snap offsets for this scrollable area, given the RenderBox of the scroll container, the RenderStyle
 // which defines the scroll-snap properties, and the viewport rectangle with the origin at the top left of

Source/WebCore/page/scrolling/nicosia/ScrollingTreeScrollingNodeDelegateNicosia.cpp

@@ -169,11 +169,11 @@ WheelEventHandlingResult ScrollingTreeScrollingNodeDelegateNicosia::handleWheelE
     if (!scrollingNode().snapOffsetsInfo().isEmpty()) {
         float scale = pageScaleFactor();
         FloatPoint originalOffset = LayoutPoint(scrollingNode().currentScrollOffset().x() / scale, scrollingNode().currentScrollOffset().y() / scale);
-        FloatPoint newFloatOffset = scrollingNode().currentScrollOffset() + FloatSize(deltaX, deltaY);
-        auto newOffset = LayoutPoint(newFloatOffset.x() / scale, newFloatOffset.y() / scale);
+        auto newOffset = (scrollingNode().currentScrollOffset() + FloatSize(deltaX, deltaY));
+        newOffset.scale(1.0 / scale);
 
-        auto offsetX = scrollingNode().snapOffsetsInfo().closestSnapOffset(ScrollEventAxis::Horizontal, scrollableAreaSize(), newOffset.x(), deltaX, originalOffset.x()).first;
-        auto offsetY = scrollingNode().snapOffsetsInfo().closestSnapOffset(ScrollEventAxis::Vertical, scrollableAreaSize(), newOffset.y(), deltaY, originalOffset.y()).first;
+        auto offsetX = scrollingNode().snapOffsetsInfo().closestSnapOffset(ScrollEventAxis::Horizontal, scrollableAreaSize(), newOffset, deltaX, originalOffset.x()).first;
+        auto offsetY = scrollingNode().snapOffsetsInfo().closestSnapOffset(ScrollEventAxis::Vertical, scrollableAreaSize(), newOffset, deltaY, originalOffset.y()).first;
 
         deltaX = (offsetX - originalOffset.x()) * scale;
         deltaY = (offsetY - originalOffset.y()) * scale;

Source/WebCore/platform/ScrollAnimator.cpp

@@ -86,9 +86,9 @@ bool ScrollAnimator::scroll(ScrollbarOrientation orientation, ScrollGranularity
         auto currentOffset = offsetFromPosition(currentPosition());
         auto newOffset = currentOffset + delta;
         if (orientation == HorizontalScrollbar)
-            newOffset.setX(m_scrollController.adjustScrollDestination(ScrollEventAxis::Horizontal, newOffset.x(), multiplier, currentOffset.x()));
+            newOffset.setX(m_scrollController.adjustScrollDestination(ScrollEventAxis::Horizontal, newOffset, multiplier, currentOffset.x()));
         else
-            newOffset.setY(m_scrollController.adjustScrollDestination(ScrollEventAxis::Vertical, newOffset.y(), multiplier, currentOffset.y()));
+            newOffset.setY(m_scrollController.adjustScrollDestination(ScrollEventAxis::Vertical, newOffset, multiplier, currentOffset.y()));
         auto newDelta = newOffset - currentOffset;
 
         if (orientation == HorizontalScrollbar)
@@ -397,26 +397,27 @@ FloatPoint ScrollAnimator::adjustScrollOffsetForSnappingIfNeeded(const FloatPoin
         return offset;
 
     FloatPoint newOffset = offset;
-    newOffset.setX(adjustScrollOffsetForSnappingIfNeeded(ScrollEventAxis::Horizontal, newOffset.x(), method));
-    newOffset.setY(adjustScrollOffsetForSnappingIfNeeded(ScrollEventAxis::Vertical, newOffset.y(), method));
+    newOffset.setX(adjustScrollOffsetForSnappingIfNeeded(ScrollEventAxis::Horizontal, newOffset, method));
+    newOffset.setY(adjustScrollOffsetForSnappingIfNeeded(ScrollEventAxis::Vertical, newOffset, method));
     return newOffset;
 }
 
-float ScrollAnimator::adjustScrollOffsetForSnappingIfNeeded(ScrollEventAxis axis, float newOffset, ScrollSnapPointSelectionMethod method)
+float ScrollAnimator::adjustScrollOffsetForSnappingIfNeeded(ScrollEventAxis axis, const FloatPoint& newOffset, ScrollSnapPointSelectionMethod method)
 {
     if (!m_scrollController.usesScrollSnap())
-        return newOffset;
+        return axis == ScrollEventAxis::Horizontal ? newOffset.x() : newOffset.y();
 
     std::optional<float> originalOffset;
-    float velocity = 0.;
+    float velocityInScrollAxis = 0.;
     if (method == ScrollSnapPointSelectionMethod::Directional) {
         FloatSize scrollOrigin = toFloatSize(m_scrollableArea.scrollOrigin());
         auto currentOffset = ScrollableArea::scrollOffsetFromPosition(this->currentPosition(), scrollOrigin);
+        auto velocity = newOffset - currentOffset;
         originalOffset = axis == ScrollEventAxis::Horizontal ? currentOffset.x() : currentOffset.y();
-        velocity = newOffset - (axis == ScrollEventAxis::Horizontal ? currentOffset.x() : currentOffset.y());
+        velocityInScrollAxis = axis == ScrollEventAxis::Horizontal ? velocity.width() : velocity.height();
     }
 
-    return m_scrollController.adjustScrollDestination(axis, newOffset, velocity, originalOffset);
+    return m_scrollController.adjustScrollDestination(axis, newOffset, velocityInScrollAxis, originalOffset);
 }
 
 

Source/WebCore/platform/ScrollAnimator.h

@@ -134,7 +134,7 @@ class ScrollAnimator : private ScrollControllerClient {
     void setWheelEventTestMonitor(RefPtr<WheelEventTestMonitor>&& testMonitor) { m_wheelEventTestMonitor = testMonitor; }
 
     FloatPoint adjustScrollOffsetForSnappingIfNeeded(const FloatPoint& offset, ScrollSnapPointSelectionMethod);
-    float adjustScrollOffsetForSnappingIfNeeded(ScrollEventAxis, float newOffset, ScrollSnapPointSelectionMethod);
+    float adjustScrollOffsetForSnappingIfNeeded(ScrollEventAxis, const FloatPoint& newOffset, ScrollSnapPointSelectionMethod);
 
     std::unique_ptr<ScrollControllerTimer> createTimer(Function<void()>&&) final;
 

Source/WebCore/platform/ScrollController.cpp

@@ -127,19 +127,20 @@ void ScrollController::setActiveScrollSnapIndexForAxis(ScrollEventAxis axis, std
     m_scrollSnapState->setActiveSnapIndexForAxis(axis, index);
 }
 
-void ScrollController::setNearestScrollSnapIndexForAxisAndOffset(ScrollEventAxis axis, int offset)
+void ScrollController::setNearestScrollSnapIndexForAxisAndOffset(ScrollEventAxis axis, ScrollOffset scrollOffset)
 {
     if (!usesScrollSnap())
         return;
 
     float scaleFactor = m_client.pageScaleFactor();
+    LayoutPoint layoutScrollOffset(scrollOffset.x() / scaleFactor, scrollOffset.y() / scaleFactor);
     ScrollSnapAnimatorState& snapState = *m_scrollSnapState;
 
     auto snapOffsets = snapState.snapOffsetsForAxis(axis);
     LayoutSize viewportSize(m_client.viewportSize().width(), m_client.viewportSize().height());
     std::optional<unsigned> activeIndex;
     if (snapOffsets.size())
-        activeIndex = snapState.snapOffsetInfo().closestSnapOffset(axis, viewportSize, LayoutUnit(offset / scaleFactor), 0).second;
+        activeIndex = snapState.snapOffsetInfo().closestSnapOffset(axis, viewportSize, layoutScrollOffset, 0).second;
 
     if (activeIndex == activeScrollSnapIndexForAxis(axis))
         return;
@@ -148,22 +149,24 @@ void ScrollController::setNearestScrollSnapIndexForAxisAndOffset(ScrollEventAxis
     setActiveScrollSnapIndexForAxis(axis, activeIndex);
 }
 
-float ScrollController::adjustScrollDestination(ScrollEventAxis axis, float destinationOffset, float velocity, std::optional<float> originalOffset)
+float ScrollController::adjustScrollDestination(ScrollEventAxis axis, FloatPoint destinationOffset, float velocity, std::optional<float> originalOffset)
 {
     if (!usesScrollSnap())
-        return destinationOffset;
+        return axis == ScrollEventAxis::Horizontal ? destinationOffset.x() : destinationOffset.y();
 
     ScrollSnapAnimatorState& snapState = *m_scrollSnapState;
     auto snapOffsets = snapState.snapOffsetsForAxis(axis);
     if (!snapOffsets.size())
-        return destinationOffset;
+        return axis == ScrollEventAxis::Horizontal ? destinationOffset.x() : destinationOffset.y();
 
+    float scaleFactor = m_client.pageScaleFactor();
     std::optional<LayoutUnit> originalOffsetInLayoutUnits;
     if (originalOffset)
-        originalOffsetInLayoutUnits = LayoutUnit(*originalOffset / m_client.pageScaleFactor());
+        originalOffsetInLayoutUnits = LayoutUnit(*originalOffset / scaleFactor);
     LayoutSize viewportSize(m_client.viewportSize().width(), m_client.viewportSize().height());
-    LayoutUnit offset = snapState.snapOffsetInfo().closestSnapOffset(axis, viewportSize, LayoutUnit(destinationOffset / m_client.pageScaleFactor()), velocity, originalOffsetInLayoutUnits).first;
-    return offset * m_client.pageScaleFactor();
+    LayoutPoint layoutDestinationOffset(destinationOffset.x() / scaleFactor, destinationOffset.y() / scaleFactor);
+    LayoutUnit offset = snapState.snapOffsetInfo().closestSnapOffset(axis, viewportSize, layoutDestinationOffset, velocity, originalOffsetInLayoutUnits).first;
+    return offset * scaleFactor;
 }
 
 
@@ -173,8 +176,8 @@ void ScrollController::updateActiveScrollSnapIndexForClientOffset()
         return;
 
     ScrollOffset offset = roundedIntPoint(m_client.scrollOffset());
-    setNearestScrollSnapIndexForAxisAndOffset(ScrollEventAxis::Horizontal, offset.x());
-    setNearestScrollSnapIndexForAxisAndOffset(ScrollEventAxis::Vertical, offset.y());
+    setNearestScrollSnapIndexForAxisAndOffset(ScrollEventAxis::Horizontal, offset);
+    setNearestScrollSnapIndexForAxisAndOffset(ScrollEventAxis::Vertical, offset);
 }
 
 void ScrollController::resnapAfterLayout()
@@ -188,11 +191,11 @@ void ScrollController::resnapAfterLayout()
 
     auto activeHorizontalIndex = m_scrollSnapState->activeSnapIndexForAxis(ScrollEventAxis::Horizontal);
     if (!activeHorizontalIndex || *activeHorizontalIndex >= snapState.snapOffsetsForAxis(ScrollEventAxis::Horizontal).size())
-        setNearestScrollSnapIndexForAxisAndOffset(ScrollEventAxis::Horizontal, offset.x());
+        setNearestScrollSnapIndexForAxisAndOffset(ScrollEventAxis::Horizontal, offset);
 
     auto activeVerticalIndex = m_scrollSnapState->activeSnapIndexForAxis(ScrollEventAxis::Vertical);
     if (!activeVerticalIndex || *activeVerticalIndex >= snapState.snapOffsetsForAxis(ScrollEventAxis::Vertical).size())
-        setNearestScrollSnapIndexForAxisAndOffset(ScrollEventAxis::Vertical, offset.y());
+        setNearestScrollSnapIndexForAxisAndOffset(ScrollEventAxis::Vertical, offset);
 
 }
 // Currently, only Mac supports momentum srolling-based scrollsnapping and rubber banding

Source/WebCore/platform/ScrollController.h

@@ -135,7 +135,7 @@ class ScrollController {
     std::optional<unsigned> activeScrollSnapIndexForAxis(ScrollEventAxis) const;
     void updateScrollSnapState(const ScrollableArea&);
     void updateGestureInProgressState(const PlatformWheelEvent&);
-    float adjustScrollDestination(ScrollEventAxis, float destinationOffset, float velocity, std::optional<float> originalOffset);
+    float adjustScrollDestination(ScrollEventAxis, FloatPoint destinationOffset, float velocity, std::optional<float> originalOffset);
 
 #if PLATFORM(MAC)
     // Returns true if handled.
@@ -159,7 +159,7 @@ class ScrollController {
 #endif
 
 private:
-    void setNearestScrollSnapIndexForAxisAndOffset(ScrollEventAxis, int);
+    void setNearestScrollSnapIndexForAxisAndOffset(ScrollEventAxis, ScrollOffset);
 
     void updateScrollSnapAnimatingState(MonotonicTime);
     void updateRubberBandAnimatingState(MonotonicTime);