Changeset 283393 in webkit

Timestamp:

Oct 1, 2021, 1:25:48 PM (5 years ago)

Author:

Simon Fraser

Message:

Further cleanup of macOS rubberbanding code
​https://bugs.webkit.org/show_bug.cgi?id=231087

Reviewed by Alan Bujtas.

Split chunks of rubber-banding logic into their own functions for clarity,
separating the "compute the delta" and "apply the delta" parts.

• platform/ScrollingEffectsController.h:
• platform/mac/ScrollingEffectsController.mm:

(WebCore::ScrollingEffectsController::handleWheelEvent):
(WebCore::ScrollingEffectsController::modifyScrollDeltaForStretching):
(WebCore::ScrollingEffectsController::applyScrollDeltaWithStretching):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• platform/ScrollingEffectsController.h (modified) (1 diff)
• platform/mac/ScrollingEffectsController.mm (modified) (4 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2021-10-01  Simon Fraser  <simon.fraser@apple.com>
+
+        Further cleanup of macOS rubberbanding code
+        https://bugs.webkit.org/show_bug.cgi?id=231087
+
+        Reviewed by Alan Bujtas.
+
+        Split chunks of rubber-banding logic into their own functions for clarity,
+        separating the "compute the delta" and "apply the delta" parts.
+
+        * platform/ScrollingEffectsController.h:
+        * platform/mac/ScrollingEffectsController.mm:
+        (WebCore::ScrollingEffectsController::handleWheelEvent):
+        (WebCore::ScrollingEffectsController::modifyScrollDeltaForStretching):
+        (WebCore::ScrollingEffectsController::applyScrollDeltaWithStretching):
+
 2021-10-01  Alan Bujtas  <zalan@apple.com>
 

trunk/Source/WebCore/platform/ScrollingEffectsController.h

@@ -199 +199 @@
     void stopDeferringWheelEventTestCompletionDueToScrollSnapping();
 
+    bool modifyScrollDeltaForStretching(const PlatformWheelEvent&, FloatSize&, bool isHorizontallyStretched, bool isVerticallyStretched);
+    bool applyScrollDeltaWithStretching(const PlatformWheelEvent&, FloatSize, bool isHorizontallyStretched, bool isVerticallyStretched);
+
     void startRubberbandAnimationIfNecessary();
     void startRubberbandAnimation();

trunk/Source/WebCore/platform/mac/ScrollingEffectsController.mm

@@ -177 +177 @@
     }
 
-    // Reset unapplied overscroll because we may decide to remove delta at various points and put it into this value.
-    auto delta = std::exchange(m_unappliedOverscrollDelta, { });
-
     IntSize stretchAmount = m_client.stretchAmount();
     bool isVerticallyStretched = stretchAmount.height();
     bool isHorizontallyStretched = stretchAmount.width();
 
-    auto eventCoalescedDelta = (isVerticallyStretched || isHorizontallyStretched) ? -wheelEvent.unacceleratedScrollingDelta() : -wheelEvent.delta();
-    delta += eventCoalescedDelta;
-
-    // FIXME: All of the code below could be simplified since predominantAxis is known, and we zero out the delta on the other axis.
-    auto affectedSide = affectedSideOnDominantAxis(delta);
-
-    float deltaX = delta.width();
-    float deltaY = delta.height();
-
-    bool shouldStretch = false;
+    // Much of this code, including this use of unaccelerated deltas when stretched, is based on AppKit behavior.
+    auto eventDelta = (isVerticallyStretched || isHorizontallyStretched) ? -wheelEvent.unacceleratedScrollingDelta() : -wheelEvent.delta();
+
+    // Reset unapplied overscroll because we may decide to remove delta at various points and put it into this value.
+    auto delta = std::exchange(m_unappliedOverscrollDelta, { });
+    delta += eventDelta;
 
     auto momentumPhase = wheelEvent.momentumPhase();
-
     if (!m_momentumScrollInProgress && (momentumPhase == PlatformWheelEventPhase::Began || momentumPhase == PlatformWheelEventPhase::Changed))
         m_momentumScrollInProgress = true;
 
+    bool shouldStretch = false;
     auto timeDelta = wheelEvent.timestamp() - m_lastMomentumScrollTimestamp;
     if (m_inScrollGesture || m_momentumScrollInProgress) {
         if (m_lastMomentumScrollTimestamp && timeDelta > 0_s && timeDelta < scrollVelocityZeroingTimeout) {
-            m_momentumVelocity = eventCoalescedDelta / timeDelta.seconds();
+            m_momentumVelocity = eventDelta / timeDelta.seconds();
             m_lastMomentumScrollTimestamp = wheelEvent.timestamp();
         } else {
@@ -210 +203 @@
         }
 
-        if (isVerticallyStretched) {
-            if (!isHorizontallyStretched && isHorizontalSide(affectedSide) && m_client.isPinnedOnSide(*affectedSide)) {
-                // Stretching only in the vertical.
-                if (deltaY && (fabsf(deltaX / deltaY) < rubberbandDirectionLockStretchRatio))
-                    deltaX = 0;
-                else if (fabsf(deltaX) < rubberbandMinimumRequiredDeltaBeforeStretch) {
-                    m_unappliedOverscrollDelta.expand(deltaX, 0);
-                    deltaX = 0;
-                } else
-                    m_unappliedOverscrollDelta.expand(deltaX, 0);
-            }
-        } else if (isHorizontallyStretched) {
-            // Stretching only in the horizontal.
-            if (isVerticalSide(affectedSide) && m_client.isPinnedOnSide(*affectedSide)) {
-                if (deltaX && (fabsf(deltaY / deltaX) < rubberbandDirectionLockStretchRatio))
-                    deltaY = 0;
-                else if (fabsf(deltaY) < rubberbandMinimumRequiredDeltaBeforeStretch) {
-                    m_unappliedOverscrollDelta.expand(0, deltaY);
-                    deltaY = 0;
-                } else
-                    m_unappliedOverscrollDelta.expand(0, deltaY);
-            }
-        } else {
-            // Not stretching at all yet.
-            if (affectedSide && m_client.isPinnedOnSide(*affectedSide)) {
-                if (fabsf(deltaY) >= fabsf(deltaX)) {
-                    if (fabsf(deltaX) < rubberbandMinimumRequiredDeltaBeforeStretch) {
-                        m_unappliedOverscrollDelta.expand(deltaX, 0);
-                        deltaX = 0;
-                    } else
-                        m_unappliedOverscrollDelta.expand(deltaX, 0);
-                }
-
-                if (!m_client.allowsHorizontalStretching(wheelEvent))
-                    deltaX = 0;
-
-                if (!m_client.allowsVerticalStretching(wheelEvent))
-                    deltaY = 0;
-
-                shouldStretch = deltaX || deltaY;
-            }
-        }
+        shouldStretch = modifyScrollDeltaForStretching(wheelEvent, delta, isHorizontallyStretched, isVerticallyStretched);
     }
 
     bool handled = true;
 
-    if (deltaX || deltaY) {
-        if (!(shouldStretch || isVerticallyStretched || isHorizontallyStretched)) {
-            if (deltaY) {
-                deltaY *= scrollWheelMultiplier();
-                m_client.immediateScrollBy(FloatSize(0, deltaY));
-            }
-            if (deltaX) {
-                deltaX *= scrollWheelMultiplier();
-                m_client.immediateScrollBy(FloatSize(deltaX, 0));
-            }
-        } else {
-            affectedSide = affectedSideOnDominantAxis({ deltaX, deltaY });
-            if (deltaX) {
-                if (!m_client.allowsHorizontalStretching(wheelEvent)) {
-                    deltaX = 0;
-                    eventCoalescedDelta.setWidth(0);
-                    handled = false;
-                } else if (!isHorizontallyStretched && !m_client.isPinnedOnSide(*affectedSide)) {
-                    deltaX *= scrollWheelMultiplier();
-
-                    m_client.immediateScrollByWithoutContentEdgeConstraints(FloatSize(deltaX, 0));
-                    deltaX = 0;
-                }
-            }
-
-            if (deltaY) {
-                if (!m_client.allowsVerticalStretching(wheelEvent)) {
-                    deltaY = 0;
-                    eventCoalescedDelta.setHeight(0);
-                    handled = false;
-                } else if (!isVerticallyStretched && !m_client.isPinnedOnSide(*affectedSide)) {
-                    deltaY *= scrollWheelMultiplier();
-
-                    m_client.immediateScrollByWithoutContentEdgeConstraints(FloatSize(0, deltaY));
-                    deltaY = 0;
-                }
-            }
-
-            IntSize stretchAmount = m_client.stretchAmount();
-
+    if (!delta.isZero()) {
+        if (shouldStretch || isVerticallyStretched || isHorizontallyStretched) {
+            if (delta.width() && !m_client.allowsHorizontalStretching(wheelEvent))
+                handled = false;
+
+            if (delta.height() && !m_client.allowsVerticalStretching(wheelEvent))
+                handled = false;
+
+            bool canStartAnimation = applyScrollDeltaWithStretching(wheelEvent, delta, isHorizontallyStretched, isVerticallyStretched);
             if (m_momentumScrollInProgress) {
-                auto sideAffectedByEventDelta = affectedSideOnDominantAxis(eventCoalescedDelta);
-                if ((!sideAffectedByEventDelta || m_client.isPinnedOnSide(*sideAffectedByEventDelta)) && m_lastMomentumScrollTimestamp) {
+                if (canStartAnimation && m_lastMomentumScrollTimestamp) {
                     m_ignoreMomentumScrolls = true;
                     m_momentumScrollInProgress = false;
@@ -304 +224 @@
                 }
             }
-
-            m_stretchScrollForce.setWidth(m_stretchScrollForce.width() + deltaX);
-            m_stretchScrollForce.setHeight(m_stretchScrollForce.height() + deltaY);
-
-            FloatSize dampedDelta(ceilf(elasticDeltaForReboundDelta(m_stretchScrollForce.width())), ceilf(elasticDeltaForReboundDelta(m_stretchScrollForce.height())));
-
-            LOG_WITH_STREAM(ScrollAnimations, stream << "ScrollingEffectsController::handleWheelEvent() - stretchScrollForce " << m_stretchScrollForce << " move delta " << FloatSize(deltaX, deltaY) << " dampedDelta " << dampedDelta);
-
-            m_client.immediateScrollByWithoutContentEdgeConstraints(dampedDelta - stretchAmount);
+        } else {
+            delta.scale(scrollWheelMultiplier());
+            m_client.immediateScrollBy(delta);
         }
     }
@@ -325 +239 @@
 
     return handled;
+}
+
+bool ScrollingEffectsController::modifyScrollDeltaForStretching(const PlatformWheelEvent& wheelEvent, FloatSize& delta, bool isHorizontallyStretched, bool isVerticallyStretched)
+{
+    auto affectedSide = affectedSideOnDominantAxis(delta);
+    if (isVerticallyStretched) {
+        if (!isHorizontallyStretched && isHorizontalSide(affectedSide) && m_client.isPinnedOnSide(*affectedSide)) {
+            // Stretching only in the vertical.
+            if (delta.height() && (fabsf(delta.width() / delta.height()) < rubberbandDirectionLockStretchRatio))
+                delta.setWidth(0);
+            else if (fabsf(delta.width()) < rubberbandMinimumRequiredDeltaBeforeStretch) {
+                m_unappliedOverscrollDelta.expand(delta.width(), 0);
+                delta.setWidth(0);
+            } else
+                m_unappliedOverscrollDelta.expand(delta.width(), 0);
+        }
+
+        return false;
+    }
+
+    if (isHorizontallyStretched) {
+        // Stretching only in the horizontal.
+        if (isVerticalSide(affectedSide) && m_client.isPinnedOnSide(*affectedSide)) {
+            if (delta.width() && (fabsf(delta.height() / delta.width()) < rubberbandDirectionLockStretchRatio))
+                delta.setHeight(0);
+            else if (fabsf(delta.height()) < rubberbandMinimumRequiredDeltaBeforeStretch) {
+                m_unappliedOverscrollDelta.expand(0, delta.height());
+                delta.setHeight(0);
+            } else
+                m_unappliedOverscrollDelta.expand(0, delta.height());
+        }
+
+        return false;
+    }
+
+    // Not stretching at all yet.
+    if (affectedSide && m_client.isPinnedOnSide(*affectedSide)) {
+        if (fabsf(delta.height()) >= fabsf(delta.width())) {
+            if (fabsf(delta.width()) < rubberbandMinimumRequiredDeltaBeforeStretch) {
+                m_unappliedOverscrollDelta.expand(delta.width(), 0);
+                delta.setWidth(0);
+            } else
+                m_unappliedOverscrollDelta.expand(delta.width(), 0);
+        }
+
+        if (!m_client.allowsHorizontalStretching(wheelEvent))
+            delta.setWidth(0);
+
+        if (!m_client.allowsVerticalStretching(wheelEvent))
+            delta.setHeight(0);
+
+        return !delta.isZero();
+    }
+    
+    return false;
+}
+
+bool ScrollingEffectsController::applyScrollDeltaWithStretching(const PlatformWheelEvent& wheelEvent, FloatSize delta, bool isHorizontallyStretched, bool isVerticallyStretched)
+{
+    auto eventDelta = (isVerticallyStretched || isHorizontallyStretched) ? -wheelEvent.unacceleratedScrollingDelta() : -wheelEvent.delta();
+    auto affectedSide = affectedSideOnDominantAxis(delta);
+
+    FloatSize deltaToScroll;
+
+    if (delta.width()) {
+        if (!m_client.allowsHorizontalStretching(wheelEvent)) {
+            delta.setWidth(0);
+            eventDelta.setWidth(0);
+        } else if (!isHorizontallyStretched && !m_client.isPinnedOnSide(*affectedSide)) {
+            delta.scale(scrollWheelMultiplier(), 1);
+            deltaToScroll += FloatSize { delta.width(), 0 };
+            delta.setWidth(0);
+        }
+    }
+
+    if (delta.height()) {
+        if (!m_client.allowsVerticalStretching(wheelEvent)) {
+            delta.setHeight(0);
+            eventDelta.setHeight(0);
+        } else if (!isVerticallyStretched && !m_client.isPinnedOnSide(*affectedSide)) {
+            delta.scale(1, scrollWheelMultiplier());
+            deltaToScroll += FloatSize { 0, delta.height() };
+            delta.setHeight(0);
+        }
+    }
+
+    if (!deltaToScroll.isZero())
+        m_client.immediateScrollByWithoutContentEdgeConstraints(deltaToScroll);
+
+    bool canStartAnimation = false;
+    if (m_momentumScrollInProgress) {
+        // Compute canStartAnimation, which looks at isPinnedOnSide(), before applying the stretch delta.
+        auto sideAffectedByEventDelta = affectedSideOnDominantAxis(eventDelta);
+        canStartAnimation = !sideAffectedByEventDelta || m_client.isPinnedOnSide(*sideAffectedByEventDelta);
+    }
+
+    m_stretchScrollForce += delta;
+    auto dampedDelta = FloatSize {
+        ceilf(elasticDeltaForReboundDelta(m_stretchScrollForce.width())),
+        ceilf(elasticDeltaForReboundDelta(m_stretchScrollForce.height()))
+    };
+
+    LOG_WITH_STREAM(ScrollAnimations, stream << "ScrollingEffectsController::applyScrollDeltaWithStretching() - stretchScrollForce " << m_stretchScrollForce << " move delta " << delta << " dampedDelta " << dampedDelta);
+
+    auto stretchAmount = m_client.stretchAmount();
+    m_client.immediateScrollByWithoutContentEdgeConstraints(dampedDelta - stretchAmount);
+
+    return canStartAnimation;
 }