[webkit-changes] [282777] trunk/Source/WebCore

Mon, 20 Sep 2021 13:44:39 -0700

Title: [282777] trunk/Source/WebCore
Revision
282777
Author
simon.fra...@apple.com
Date
2021-09-20 13:44:29 -0700 (Mon, 20 Sep 2021)

Log Message

ScrollSnapAnimatorState should be explicit about when it starts animations
https://bugs.webkit.org/show_bug.cgi?id=230497

Reviewed by Wenson Hsieh.

ScrollSnapAnimatorState::transitionTo* functions may not actually trigger an animation
if the target offset ends up as the initial offset. This currently happens to work
because ScrollingMomentumCalculator reports a duration of zero in this case, but
it's better to just make it clear that no animation was started.

Exercised by tiled-drawing/scrolling/scroll-snap/scroll-snap-mandatory-mainframe-slow-vertical.html

* platform/ScrollSnapAnimatorState.cpp:
(WebCore::ScrollSnapAnimatorState::transitionToSnapAnimationState):
(WebCore::ScrollSnapAnimatorState::transitionToGlideAnimationState):
(WebCore::ScrollSnapAnimatorState::setupAnimationForState):
* platform/ScrollSnapAnimatorState.h:
* platform/mac/ScrollingEffectsController.mm:
(WebCore::ScrollingEffectsController::processWheelEventForScrollSnap):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (282776 => 282777)


--- trunk/Source/WebCore/ChangeLog	2021-09-20 20:41:29 UTC (rev 282776)
+++ trunk/Source/WebCore/ChangeLog	2021-09-20 20:44:29 UTC (rev 282777)
@@ -1,3 +1,25 @@
+2021-09-20  Simon Fraser  <simon.fra...@apple.com>
+
+        ScrollSnapAnimatorState should be explicit about when it starts animations
+        https://bugs.webkit.org/show_bug.cgi?id=230497
+
+        Reviewed by Wenson Hsieh.
+
+        ScrollSnapAnimatorState::transitionTo* functions may not actually trigger an animation
+        if the target offset ends up as the initial offset. This currently happens to work
+        because ScrollingMomentumCalculator reports a duration of zero in this case, but
+        it's better to just make it clear that no animation was started.
+
+        Exercised by tiled-drawing/scrolling/scroll-snap/scroll-snap-mandatory-mainframe-slow-vertical.html
+
+        * platform/ScrollSnapAnimatorState.cpp:
+        (WebCore::ScrollSnapAnimatorState::transitionToSnapAnimationState):
+        (WebCore::ScrollSnapAnimatorState::transitionToGlideAnimationState):
+        (WebCore::ScrollSnapAnimatorState::setupAnimationForState):
+        * platform/ScrollSnapAnimatorState.h:
+        * platform/mac/ScrollingEffectsController.mm:
+        (WebCore::ScrollingEffectsController::processWheelEventForScrollSnap):
+
 2021-09-20  Darin Adler  <da...@apple.com>
 
         Clean up overrides of DisplayCaptureSourceMac::Capturer::generateFrame()

Modified: trunk/Source/WebCore/platform/ScrollSnapAnimatorState.cpp (282776 => 282777)


--- trunk/Source/WebCore/platform/ScrollSnapAnimatorState.cpp	2021-09-20 20:41:29 UTC (rev 282776)
+++ trunk/Source/WebCore/platform/ScrollSnapAnimatorState.cpp	2021-09-20 20:44:29 UTC (rev 282777)
@@ -31,21 +31,21 @@
 
 namespace WebCore {
 
-void ScrollSnapAnimatorState::transitionToSnapAnimationState(const ScrollExtents& scrollExtents, float pageScale, const FloatPoint& initialOffset)
+bool ScrollSnapAnimatorState::transitionToSnapAnimationState(const ScrollExtents& scrollExtents, float pageScale, const FloatPoint& initialOffset)
 {
-    setupAnimationForState(ScrollSnapState::Snapping, scrollExtents, pageScale, initialOffset, { }, { });
+    return setupAnimationForState(ScrollSnapState::Snapping, scrollExtents, pageScale, initialOffset, { }, { });
 }
 
-void ScrollSnapAnimatorState::transitionToGlideAnimationState(const ScrollExtents& scrollExtents, float pageScale, const FloatPoint& initialOffset, const FloatSize& initialVelocity, const FloatSize& initialDelta)
+bool ScrollSnapAnimatorState::transitionToGlideAnimationState(const ScrollExtents& scrollExtents, float pageScale, const FloatPoint& initialOffset, const FloatSize& initialVelocity, const FloatSize& initialDelta)
 {
-    setupAnimationForState(ScrollSnapState::Gliding, scrollExtents, pageScale, initialOffset, initialVelocity, initialDelta);
+    return setupAnimationForState(ScrollSnapState::Gliding, scrollExtents, pageScale, initialOffset, initialVelocity, initialDelta);
 }
 
-void ScrollSnapAnimatorState::setupAnimationForState(ScrollSnapState state, const ScrollExtents& scrollExtents, float pageScale, const FloatPoint& initialOffset, const FloatSize& initialVelocity, const FloatSize& initialDelta)
+bool ScrollSnapAnimatorState::setupAnimationForState(ScrollSnapState state, const ScrollExtents& scrollExtents, float pageScale, const FloatPoint& initialOffset, const FloatSize& initialVelocity, const FloatSize& initialDelta)
 {
     ASSERT(state == ScrollSnapState::Snapping || state == ScrollSnapState::Gliding);
     if (m_currentState == state)
-        return;
+        return false;
 
     m_momentumCalculator = ScrollingMomentumCalculator::create(scrollExtents, initialOffset, initialDelta, initialVelocity);
     FloatPoint predictedScrollTarget { m_momentumCalculator->predictedDestinationOffset() };
@@ -53,9 +53,15 @@
     float targetOffsetX, targetOffsetY;
     std::tie(targetOffsetX, m_activeSnapIndexX) = targetOffsetForStartOffset(ScrollEventAxis::Horizontal, scrollExtents, initialOffset.x(), predictedScrollTarget, pageScale, initialDelta.width());
     std::tie(targetOffsetY, m_activeSnapIndexY) = targetOffsetForStartOffset(ScrollEventAxis::Vertical, scrollExtents, initialOffset.y(), predictedScrollTarget, pageScale, initialDelta.height());
-    m_momentumCalculator->setRetargetedScrollOffset({ targetOffsetX, targetOffsetY });
+    auto targetOffset = FloatPoint { targetOffsetX, targetOffsetY };
+    m_momentumCalculator->setRetargetedScrollOffset(targetOffset);
+
+    if (targetOffset == initialOffset)
+        return false;
+
     m_startTime = MonotonicTime::now();
     m_currentState = state;
+    return true;
 }
 
 void ScrollSnapAnimatorState::transitionToUserInteractionState()

Modified: trunk/Source/WebCore/platform/ScrollSnapAnimatorState.h (282776 => 282777)


--- trunk/Source/WebCore/platform/ScrollSnapAnimatorState.h	2021-09-20 20:41:29 UTC (rev 282776)
+++ trunk/Source/WebCore/platform/ScrollSnapAnimatorState.h	2021-09-20 20:44:29 UTC (rev 282777)
@@ -78,15 +78,17 @@
     FloatPoint currentAnimatedScrollOffset(MonotonicTime, bool& isAnimationComplete) const;
 
     // State transition helpers.
-    void transitionToSnapAnimationState(const ScrollExtents&, float pageScale, const FloatPoint& initialOffset);
-    void transitionToGlideAnimationState(const ScrollExtents&, float pageScale, const FloatPoint& initialOffset, const FloatSize& initialVelocity, const FloatSize& initialDelta);
+    // These return true if they start a new animation.
+    bool transitionToSnapAnimationState(const ScrollExtents&, float pageScale, const FloatPoint& initialOffset);
+    bool transitionToGlideAnimationState(const ScrollExtents&, float pageScale, const FloatPoint& initialOffset, const FloatSize& initialVelocity, const FloatSize& initialDelta);
+
     void transitionToUserInteractionState();
     void transitionToDestinationReachedState();
 
 private:
     std::pair<float, std::optional<unsigned>> targetOffsetForStartOffset(ScrollEventAxis, const ScrollExtents&, float startOffset, FloatPoint predictedOffset, float pageScale, float initialDelta) const;
+    bool setupAnimationForState(ScrollSnapState, const ScrollExtents&, float pageScale, const FloatPoint& initialOffset, const FloatSize& initialVelocity, const FloatSize& initialDelta);
     void teardownAnimationForState(ScrollSnapState);
-    void setupAnimationForState(ScrollSnapState, const ScrollExtents&, float pageScale, const FloatPoint& initialOffset, const FloatSize& initialVelocity, const FloatSize& initialDelta);
 
     ScrollSnapState m_currentState { ScrollSnapState::UserInteraction };

Modified: trunk/Source/WebCore/platform/mac/ScrollingEffectsController.mm (282776 => 282777)


--- trunk/Source/WebCore/platform/mac/ScrollingEffectsController.mm	2021-09-20 20:41:29 UTC (rev 282776)
+++ trunk/Source/WebCore/platform/mac/ScrollingEffectsController.mm	2021-09-20 20:44:29 UTC (rev 282777)
@@ -701,13 +701,13 @@
         m_dragEndedScrollingVelocity = -wheelEvent.scrollingVelocity();
         break;
     case WheelEventStatus::UserScrollEnd:
-        m_scrollSnapState->transitionToSnapAnimationState(m_client.scrollExtents(), m_client.pageScaleFactor(), m_client.scrollOffset());
-        startScrollSnapAnimation();
+        if (m_scrollSnapState->transitionToSnapAnimationState(m_client.scrollExtents(), m_client.pageScaleFactor(), m_client.scrollOffset()))
+            startScrollSnapAnimation();
         break;
     case WheelEventStatus::MomentumScrollBegin:
-        m_scrollSnapState->transitionToGlideAnimationState(m_client.scrollExtents(), m_client.pageScaleFactor(), m_client.scrollOffset(), m_dragEndedScrollingVelocity, FloatSize(-wheelEvent.deltaX(), -wheelEvent.deltaY()));
+        if (m_scrollSnapState->transitionToGlideAnimationState(m_client.scrollExtents(), m_client.pageScaleFactor(), m_client.scrollOffset(), m_dragEndedScrollingVelocity, FloatSize(-wheelEvent.deltaX(), -wheelEvent.deltaY())))
+            isMomentumScrolling = true;
         m_dragEndedScrollingVelocity = { };
-        isMomentumScrolling = true;
         break;
     case WheelEventStatus::MomentumScrolling:
     case WheelEventStatus::MomentumScrollEnd:

_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to