Attachment 422047 Details for Bug 222594 – Patch

[patch] Patch

bug-222594-20210303071630.patch (text/plain), 16.34 KB, created by Martin Robinson on 2021-03-02 22:16:33 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Martin Robinson

Created: 2021-03-02 22:16:33 PST

Size: 16.34 KB

patch

obsolete

>Subversion Revision: 273733
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index c11645d019f2035bf12244da86c73988ff6bcb57..018cf752c3bcd80d9ead99ec5463130a2a57224b 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,30 @@
>+2021-03-02  Martin Robinson  <mrobinson@webkit.org>
>+
>+        Add basic (non-momentum) wheel event handling for scroll snap
>+        https://bugs.webkit.org/show_bug.cgi?id=222594
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Test: css3/scroll-snap/scroll-snap-wheel-event.html
>+
>+        Enable scroll snapping for basic wheel events on GTK+ and WPE. The Mac port
>+        has special wheel handling due to momentum scrolling. Other scroll-snap-enabled
>+        ports can just use a basic version.
>+
>+        * platform/ScrollAnimator.cpp:
>+        (WebCore::ScrollAnimator::scroll): Accept a bitmask of options now. This
>+        will allow using this method when handling wheel events that do not animate.
>+        (WebCore::ScrollAnimator::handleWheelEvent): Trigger ::scroll with
>+        scroll snapping enabled and pass the appropriate option to disable animations.
>+        (WebCore::ScrollAnimator::processWheelEventForScrollSnap): Deleted.
>+        * platform/ScrollAnimator.h:
>+        (WebCore::ScrollAnimator::ScrollAnimator::processWheelEventForScrollSnap): Made
>+        this a method that can be overridden by subclasses.
>+        * platform/mac/ScrollAnimatorMac.h: Added processWheelEventForScrollSnap.
>+        * platform/mac/ScrollAnimatorMac.mm:
>+        (WebCore::ScrollAnimatorMac::scroll): Pay attention to the NeverAnimate bitmask now.
>+        (WebCore::ScrollAnimatorMac::processWheelEventForScrollSnap): Added.
>+
> 2021-03-02  Martin Robinson  <mrobinson@igalia.com>
> 
>         Eliminate ScrollAnimatorGeneric::m_smoothAnimation
>diff --git a/Source/WebCore/platform/ScrollAnimator.cpp b/Source/WebCore/platform/ScrollAnimator.cpp
>index 64c843b59752464acf7f744b5bbe578b27d48f84..69b8b546f39ff39f93051768bb472cca74f1eabe 100644
>--- a/Source/WebCore/platform/ScrollAnimator.cpp
>+++ b/Source/WebCore/platform/ScrollAnimator.cpp
>@@ -84,15 +84,16 @@ ScrollAnimator::~ScrollAnimator()
> bool ScrollAnimator::scroll(ScrollbarOrientation orientation, ScrollGranularity granularity, float step, float multiplier, ScrollBehavior behavior)
> {
> #if ENABLE(CSS_SCROLL_SNAP)
>-    if (behavior == ScrollBehavior::DoDirectionalSnapping) {
>+    if (behavior & ScrollBehavior::DoDirectionalSnapping) {
>         auto newOffset = ScrollableArea::scrollOffsetFromPosition(positionFromStep(orientation, step, multiplier), toFloatSize(m_scrollableArea.scrollOrigin()));
>         auto currentOffset = m_scrollableArea.scrollOffset();
>+        behavior = static_cast<ScrollBehavior>(behavior & ~ScrollBehavior::DoDirectionalSnapping);
>         if (orientation == HorizontalScrollbar) {
>             newOffset.setX(m_scrollController.adjustScrollDestination(ScrollEventAxis::Horizontal, newOffset.x(), multiplier, currentOffset.x()));
>-            return scroll(HorizontalScrollbar, granularity, newOffset.x() - currentOffset.x(), 1.0);
>+            return scroll(HorizontalScrollbar, granularity, newOffset.x() - currentOffset.x(), 1.0, behavior);
>         }
>         newOffset.setY(m_scrollController.adjustScrollDestination(ScrollEventAxis::Vertical, newOffset.y(), multiplier, currentOffset.y()));
>-        return scroll(VerticalScrollbar, granularity, newOffset.y() - currentOffset.y(), 1.0);
>+        return scroll(VerticalScrollbar, granularity, newOffset.y() - currentOffset.y(), 1.0, behavior);
>     }
> #else
>     UNUSED_PARAM(granularity);
>@@ -100,7 +101,7 @@ bool ScrollAnimator::scroll(ScrollbarOrientation orientation, ScrollGranularity
> #endif
> 
> #if ENABLE(SMOOTH_SCROLLING) && !PLATFORM(IOS_FAMILY)
>-    if (m_scrollableArea.scrollAnimatorEnabled()) {
>+    if (m_scrollableArea.scrollAnimatorEnabled() && !(behavior & ScrollBehavior::NeverAnimate)) {
>         m_scrollAnimation->setCurrentPosition(m_currentPosition);
>         return m_scrollAnimation->scroll(orientation, granularity, step, multiplier);
>     }
>@@ -160,10 +161,6 @@ FloatPoint ScrollAnimator::positionFromStep(ScrollbarOrientation orientation, fl
> 
> #if ENABLE(CSS_SCROLL_SNAP)
> #if PLATFORM(MAC)
>-bool ScrollAnimator::processWheelEventForScrollSnap(const PlatformWheelEvent& wheelEvent)
>-{
>-    return m_scrollController.processWheelEventForScrollSnap(wheelEvent);
>-}
> #endif
> 
> bool ScrollAnimator::activeScrollSnapIndexDidChange() const
>@@ -179,10 +176,11 @@ unsigned ScrollAnimator::activeScrollSnapIndexForAxis(ScrollEventAxis axis) cons
> 
> bool ScrollAnimator::handleWheelEvent(const PlatformWheelEvent& e)
> {
>-#if ENABLE(CSS_SCROLL_SNAP) && PLATFORM(MAC)
>-    if (m_scrollController.processWheelEventForScrollSnap(e))
>-        return false;
>+#if ENABLE(CSS_SCROLL_SNAP)
>+        if (processWheelEventForScrollSnap(e))
>+            return false;
> #endif
>+
> #if PLATFORM(COCOA)
>     // Events in the PlatformWheelEventPhase::MayBegin phase have no deltas, and therefore never passes through the scroll handling logic below.
>     // This causes us to return with an 'unhandled' return state, even though this event was successfully processed.
>@@ -203,7 +201,6 @@ bool ScrollAnimator::handleWheelEvent(const PlatformWheelEvent& e)
> 
>     bool handled = false;
> 
>-    ScrollGranularity granularity = ScrollByPixel;
>     IntSize maxForwardScrollDelta = m_scrollableArea.maximumScrollPosition() - m_scrollableArea.scrollPosition();
>     IntSize maxBackwardScrollDelta = m_scrollableArea.scrollPosition() - m_scrollableArea.minimumScrollPosition();
>     if ((deltaX < 0 && maxForwardScrollDelta.width() > 0)
>@@ -212,6 +209,10 @@ bool ScrollAnimator::handleWheelEvent(const PlatformWheelEvent& e)
>         || (deltaY > 0 && maxBackwardScrollDelta.height() > 0)) {
>         handled = true;
> 
>+        ScrollBehavior behavior = ScrollBehavior::DoDirectionalSnapping;
>+        if (e.hasPreciseScrollingDeltas())
>+            behavior = static_cast<ScrollBehavior>(behavior & ScrollBehavior::NeverAnimate);
>+
>         if (deltaY) {
>             if (e.granularity() == ScrollByPageWheelEvent) {
>                 bool negative = deltaY < 0;
>@@ -219,10 +220,7 @@ bool ScrollAnimator::handleWheelEvent(const PlatformWheelEvent& e)
>                 if (negative)
>                     deltaY = -deltaY;
>             }
>-            if (e.hasPreciseScrollingDeltas())
>-                scrollToPositionWithoutAnimation(positionFromStep(VerticalScrollbar, verticalScrollbar->pixelStep(), -deltaY));
>-            else
>-                scroll(VerticalScrollbar, granularity, verticalScrollbar->pixelStep(), -deltaY);
>+            scroll(VerticalScrollbar, ScrollByPixel, verticalScrollbar->pixelStep(), -deltaY, behavior);
>         }
> 
>         if (deltaX) {
>@@ -232,10 +230,7 @@ bool ScrollAnimator::handleWheelEvent(const PlatformWheelEvent& e)
>                 if (negative)
>                     deltaX = -deltaX;
>             }
>-            if (e.hasPreciseScrollingDeltas())
>-                scrollToPositionWithoutAnimation(positionFromStep(HorizontalScrollbar, horizontalScrollbar->pixelStep(), -deltaX));
>-            else
>-                scroll(HorizontalScrollbar, granularity, horizontalScrollbar->pixelStep(), -deltaX);
>+            scroll(HorizontalScrollbar, ScrollByPixel, horizontalScrollbar->pixelStep(), -deltaX, behavior);
>         }
>     }
>     return handled;
>diff --git a/Source/WebCore/platform/ScrollAnimator.h b/Source/WebCore/platform/ScrollAnimator.h
>index 6eb140677d138bed054e6e70f10b3f18685b88d8..552561696f80230f73dfb3bea0194c0708195c5d 100644
>--- a/Source/WebCore/platform/ScrollAnimator.h
>+++ b/Source/WebCore/platform/ScrollAnimator.h
>@@ -69,8 +69,9 @@ public:
>     virtual ~ScrollAnimator();
> 
>     enum ScrollBehavior {
>-        Default,
>-        DoDirectionalSnapping,
>+        Default = 0,
>+        DoDirectionalSnapping = 1 << 1,
>+        NeverAnimate = 1 << 2,
>     };
> 
>     // Computes a scroll destination for the given parameters.  Returns false if
>@@ -149,9 +150,7 @@ public:
> #endif
> 
> #if ENABLE(CSS_SCROLL_SNAP)
>-#if PLATFORM(MAC)
>-    bool processWheelEventForScrollSnap(const PlatformWheelEvent&);
>-#endif
>+    virtual bool processWheelEventForScrollSnap(const PlatformWheelEvent&) { return false; }
>     void updateScrollSnapState();
>     bool activeScrollSnapIndexDidChange() const;
>     unsigned activeScrollSnapIndexForAxis(ScrollEventAxis) const;
>diff --git a/Source/WebCore/platform/mac/ScrollAnimatorMac.h b/Source/WebCore/platform/mac/ScrollAnimatorMac.h
>index 649b4cf5445098a3312283850b5172ca1cd3278b..7b296dc3e81ffe1db1de4727b8145510eb32a86b 100644
>--- a/Source/WebCore/platform/mac/ScrollAnimatorMac.h
>+++ b/Source/WebCore/platform/mac/ScrollAnimatorMac.h
>@@ -140,6 +140,8 @@ private:
>     String horizontalScrollbarStateForTesting() const final;
>     String verticalScrollbarStateForTesting() const final;
> 
>+    bool processWheelEventForScrollSnap(const PlatformWheelEvent&) override;
>+
>     // ScrollControllerClient.
> #if ENABLE(RUBBER_BANDING)
>     IntSize stretchAmount() const final;
>diff --git a/Source/WebCore/platform/mac/ScrollAnimatorMac.mm b/Source/WebCore/platform/mac/ScrollAnimatorMac.mm
>index 8e00bd718bf50b634d8b296668ccdeb5bc8429ee..2cf96f5242198fcaedb0a92b04152022a9fc4632 100644
>--- a/Source/WebCore/platform/mac/ScrollAnimatorMac.mm
>+++ b/Source/WebCore/platform/mac/ScrollAnimatorMac.mm
>@@ -757,10 +757,11 @@ bool ScrollAnimatorMac::scroll(ScrollbarOrientation orientation, ScrollGranulari
> 
>     // This method doesn't do directional snapping, but our base class does. It will call into
>     // ScrollAnimatorMac::scroll again with the snapped positions and ScrollBehavior::Default.
>-    if (behavior == ScrollBehavior::DoDirectionalSnapping)
>+    if (behavior & ScrollBehavior::DoDirectionalSnapping)
>         return ScrollAnimator::scroll(orientation, granularity, step, multiplier, behavior);
> 
>-    bool shouldAnimate = scrollAnimationEnabledForSystem() && m_scrollableArea.scrollAnimatorEnabled() && granularity != ScrollByPixel;
>+    bool shouldAnimate = scrollAnimationEnabledForSystem() && m_scrollableArea.scrollAnimatorEnabled() && granularity != ScrollByPixel
>+        && !(behavior & ScrollBehavior::NeverAnimate);
>     FloatPoint newPosition = positionFromStep(orientation, step, multiplier);
>     newPosition = newPosition.constrainedBetween(scrollableArea().minimumScrollPosition(), scrollableArea().maximumScrollPosition());
> 
>@@ -1500,6 +1501,11 @@ void ScrollAnimatorMac::setVisibleScrollerThumbRect(const IntRect& scrollerThumb
>     m_visibleScrollerThumbRect = rectInViewCoordinates;
> }
> 
>+bool ScrollAnimatorMac::processWheelEventForScrollSnap(const PlatformWheelEvent& wheelEvent)
>+{
>+    return m_scrollController.processWheelEventForScrollSnap(wheelEvent);
>+}
>+
> } // namespace WebCore
> 
> #endif // ENABLE(SMOOTH_SCROLLING)
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index fe4d36c59a917c10452927747210405d37589a83..f5b117a3480bfe86fc5f0ac18c05a6609720ea57 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,14 @@
>+2021-03-02  Martin Robinson  <mrobinson@webkit.org>
>+
>+        Add basic (non-momentum) wheel event handling for scroll snap
>+        https://bugs.webkit.org/show_bug.cgi?id=222594
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * css3/scroll-snap/scroll-snap-wheel-event-expected.txt: Added.
>+        * css3/scroll-snap/scroll-snap-wheel-event.html: Added.
>+        * platform/ios-wk2/TestExpectations: Skip new test because it uses mouse event simulation.
>+        Move existing classification to better section as well.
>+
> 2021-03-02  Youenn Fablet  <youenn@apple.com>
> 
>         Enable MEDIA_SOURCE in IOS Simulator
>diff --git a/LayoutTests/css3/scroll-snap/scroll-snap-wheel-event-expected.txt b/LayoutTests/css3/scroll-snap/scroll-snap-wheel-event-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..c9e61ca2207141eee0188b7b1c72c1c3c2fa9f0c
>--- /dev/null
>+++ b/LayoutTests/css3/scroll-snap/scroll-snap-wheel-event-expected.txt
>@@ -0,0 +1,6 @@
>+PASS horizontal mouse wheel event snapped
>+PASS vertical mouse wheel event snapped
>+PASS successfullyParsed is true
>+
>+TEST COMPLETE
>+
>diff --git a/LayoutTests/css3/scroll-snap/scroll-snap-wheel-event.html b/LayoutTests/css3/scroll-snap/scroll-snap-wheel-event.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..10e23dab3d2ef7975bd27d492f205afa62597ab6
>--- /dev/null
>+++ b/LayoutTests/css3/scroll-snap/scroll-snap-wheel-event.html
>@@ -0,0 +1,80 @@
>+<!DOCTYPE html>
>+<html>
>+    <head>
>+        <title>Simple wheel events should trigger scroll snapping</title>
>+        <style type="text/css">
>+            .container {
>+                height: 200px;
>+                width: 200px;
>+                overflow: auto;
>+                scroll-snap-type: both mandatory;
>+            }
>+
>+            .horizontal-drawer {
>+                height: 100%;
>+                width: 500px;
>+            }
>+
>+            .block {
>+                height: 100%;
>+                width: 200px;
>+                scroll-snap-align: start;
>+            }
>+        </style>
>+        <script src="../../resources/js-test.js"></script>
>+        <script src="../../resources/ui-helper.js"></script>
>+        <script>
>+        window.jsTestIsAsync = true;
>+
>+        async function onLoad()
>+        {
>+            if (window.eventSender == undefined) {
>+                document.getElementById('console').innerText = "Simple wheel events should trigger scroll snapping";
>+                return;
>+            }
>+            try {
>+                eventSender.mouseMoveTo(20, 190);
>+                eventSender.mouseDown();
>+                eventSender.mouseMoveTo(80, 190);
>+                eventSender.mouseUp();
>+
>+                eventSender.mouseScrollBy(-1, 0);
>+
>+                let horizontalContainer = document.getElementById("horizontal-container");
>+                await UIHelper.waitForTargetScrollAnimationToSettle(horizontalContainer);
>+                expectTrue(horizontalContainer.scrollLeft == 200, "horizontal mouse wheel event snapped");
>+
>+                eventSender.mouseMoveTo(190, 220);
>+                eventSender.mouseDown();
>+                eventSender.mouseMoveTo(190, 270);
>+                eventSender.mouseUp();
>+
>+                eventSender.mouseScrollBy(0, -1);
>+
>+                let verticalContainer = document.getElementById("vertical-container");
>+                await UIHelper.waitForTargetScrollAnimationToSettle(verticalContainer);
>+                expectTrue(verticalContainer.scrollTop == 185, "vertical mouse wheel event snapped");
>+            } catch (e) {
>+                console.log(e);
>+            } finally {
>+                finishJSTest();
>+            }
>+        }
>+        </script>
>+    </head>
>+    <body onload="onLoad();">
>+        <div id="horizontal-container" class="container">
>+            <div class="horizontal-drawer">
>+                <div class="block" style="float: left; background: #80475E"></div>
>+                <div class="block" style="float: left; background: #CC5A71"></div>
>+            </div>
>+        </div>
>+        <div id="vertical-container" class="container">
>+            <div class="block" style="background: #80475E"></div>
>+            <div class="block" style="background: #CC5A71"></div>
>+        </div>
>+        <p id="console"></p>
>+        <script>
>+        </script>
>+    </body>
>+</html>
>diff --git a/LayoutTests/platform/ios-wk2/TestExpectations b/LayoutTests/platform/ios-wk2/TestExpectations
>index 522ec22a8b5e2fb619e0c3f6a161b1d75b89554a..84a047c6da83e8fde622fec1d943c47c934d5ae1 100644
>--- a/LayoutTests/platform/ios-wk2/TestExpectations
>+++ b/LayoutTests/platform/ios-wk2/TestExpectations
>@@ -137,7 +137,6 @@ scrollbars/scrolling-by-page-on-keyboard-spacebar.html [ Failure ]
> scrollbars/scrollbars-on-positioned-content.html [ Failure ]
> css3/scroll-snap/scroll-padding-mainframe-paging.html [ Failure ]
> css3/scroll-snap/scroll-padding-overflow-paging.html [ Failure ]
>-css3/scroll-snap/scroll-snap-click-scrollbar-gutter.html [ Failure ]
> 
> # SVG tests that time out (these require EventSender)
> svg/animations/animVal-basics.html
>@@ -933,6 +932,8 @@ editing/selection/select-out-of-floated-non-editable-09.html [ Skip ]
> editing/selection/select-out-of-floated-non-editable-10.html [ Skip ]
> editing/selection/select-out-of-floated-non-editable-11.html [ Skip ]
> editing/selection/select-out-of-floated-non-editable-12.html [ Skip ]
>+css3/scroll-snap/scroll-snap-click-scrollbar-gutter.html [ Skip ]
>+css3/scroll-snap/scroll-snap-wheel-event.html [ Skip ]
> 
> webkit.org/b/192088 media/no-fullscreen-when-hidden.html [ Failure Pass ]
>

Actions: View | Formatted Diff | Diff

Attachments on bug 222594: 421961 | 422047 | 422686 | 422701 | 423064 | 423099 | 425148 | 426538 | 426659