Changeset 283296 in webkit

Timestamp:

Sep 29, 2021, 7:33:01 PM (5 years ago)

Author:

Simon Fraser

Message:

Minor cleanup of some rubber-banding code in ScrollingEffectsController
​https://bugs.webkit.org/show_bug.cgi?id=230981

Reviewed by Tim Horton.

As a precursor to unraveling some of the complexity of the rubber-banding code,
tidy up naming and code in ScrollingEffectsController::handleWheelEvent(). No
behavior change.

• platform/PlatformWheelEvent.h:

(WebCore::PlatformWheelEvent::unacceleratedScrollingDelta const):
(WebCore::PlatformWheelEvent::unacceleratedScrollingDeltaX const): Deleted.
(WebCore::PlatformWheelEvent::unacceleratedScrollingDeltaY const): Deleted.

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

(WebCore::convertToProminentAxisFavoringVertical):
(WebCore::ScrollingEffectsController::handleWheelEvent):
(WebCore::ScrollingEffectsController::wheelDeltaBiasingTowardsVertical):

Location:

trunk/Source/WebCore

Files:

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

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2021-09-29  Simon Fraser  <simon.fraser@apple.com>
+
+        Minor cleanup of some rubber-banding code in ScrollingEffectsController
+        https://bugs.webkit.org/show_bug.cgi?id=230981
+
+        Reviewed by Tim Horton.
+
+        As a precursor to unraveling some of the complexity of the rubber-banding code,
+        tidy up naming and code in ScrollingEffectsController::handleWheelEvent(). No
+        behavior change.
+
+        * platform/PlatformWheelEvent.h:
+        (WebCore::PlatformWheelEvent::unacceleratedScrollingDelta const):
+        (WebCore::PlatformWheelEvent::unacceleratedScrollingDeltaX const): Deleted.
+        (WebCore::PlatformWheelEvent::unacceleratedScrollingDeltaY const): Deleted.
+        * platform/ScrollingEffectsController.h:
+        * platform/mac/ScrollingEffectsController.mm:
+        (WebCore::convertToProminentAxisFavoringVertical):
+        (WebCore::ScrollingEffectsController::handleWheelEvent):
+        (WebCore::ScrollingEffectsController::wheelDeltaBiasingTowardsVertical):
+
 2021-09-29  Chris Dumez  <cdumez@apple.com>
 

trunk/Source/WebCore/platform/PlatformWheelEvent.h

@@ -151 +151 @@
 #if PLATFORM(COCOA)
     unsigned scrollCount() const { return m_scrollCount; }
-    float unacceleratedScrollingDeltaX() const { return m_unacceleratedScrollingDeltaX; }
-    float unacceleratedScrollingDeltaY() const { return m_unacceleratedScrollingDeltaY; }
+    FloatSize unacceleratedScrollingDelta() const { return { m_unacceleratedScrollingDeltaX, m_unacceleratedScrollingDeltaY }; }
 #endif
 

trunk/Source/WebCore/platform/ScrollingEffectsController.h

@@ -235 +235 @@
 #if PLATFORM(MAC)
     WallTime m_lastMomentumScrollTimestamp;
-    FloatSize m_overflowScrollDelta;
+    FloatSize m_unappliedOverscrollDelta;
     FloatSize m_stretchScrollForce;
     FloatSize m_momentumVelocity;

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

@@ -84 +84 @@
 }
 
+
+static FloatSize convertToProminentAxisFavoringVertical(FloatSize delta)
+{
+    if (fabsf(delta.height()) >= fabsf(delta.width()))
+        return { 0, delta.height() };
+
+    return { delta.width(), 0 };
+}
+
 bool ScrollingEffectsController::handleWheelEvent(const PlatformWheelEvent& wheelEvent)
 {
@@ -110 +119 @@
         m_stretchScrollForce.setWidth(reboundDeltaForElasticDelta(stretchAmount.width()));
         m_stretchScrollForce.setHeight(reboundDeltaForElasticDelta(stretchAmount.height()));
-        m_overflowScrollDelta = { };
+        m_unappliedOverscrollDelta = { };
 
         stopSnapRubberbandAnimation();
@@ -137 +146 @@
     }
 
-    float deltaX = m_overflowScrollDelta.width();
-    float deltaY = m_overflowScrollDelta.height();
-
-    // Reset overflow values because we may decide to remove delta at various points and put it into overflow.
-    m_overflowScrollDelta = { };
+    // 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();
@@ -147 +153 @@
     bool isHorizontallyStretched = stretchAmount.width();
 
-    float eventCoalescedDeltaX;
-    float eventCoalescedDeltaY;
-
-    if (isVerticallyStretched || isHorizontallyStretched) {
-        eventCoalescedDeltaX = -wheelEvent.unacceleratedScrollingDeltaX();
-        eventCoalescedDeltaY = -wheelEvent.unacceleratedScrollingDeltaY();
-    } else {
-        eventCoalescedDeltaX = -wheelEvent.deltaX();
-        eventCoalescedDeltaY = -wheelEvent.deltaY();
-    }
-
-    deltaX += eventCoalescedDeltaX;
-    deltaY += eventCoalescedDeltaY;
-
-    // Slightly prefer scrolling vertically by applying the = case to deltaY
-    // FIXME: Use wheelDeltaBiasingTowardsVertical().
-    if (fabsf(deltaY) >= fabsf(deltaX))
-        deltaX = 0;
-    else
-        deltaY = 0;
+    auto eventCoalescedDelta = (isVerticallyStretched || isHorizontallyStretched) ? -wheelEvent.unacceleratedScrollingDelta() : -wheelEvent.delta();
+    delta += eventCoalescedDelta;
+
+    delta = convertToProminentAxisFavoringVertical(delta);
+    float deltaX = delta.width();
+    float deltaY = delta.height();
 
     bool shouldStretch = false;
 
-    PlatformWheelEventPhase momentumPhase = wheelEvent.momentumPhase();
-
-    // If we are starting momentum scrolling then do some setup.
+    auto momentumPhase = wheelEvent.momentumPhase();
+
     if (!m_momentumScrollInProgress && (momentumPhase == PlatformWheelEventPhase::Began || momentumPhase == PlatformWheelEventPhase::Changed))
         m_momentumScrollInProgress = true;
@@ -179 +170 @@
     if (m_inScrollGesture || m_momentumScrollInProgress) {
         if (m_lastMomentumScrollTimestamp && timeDelta > 0_s && timeDelta < scrollVelocityZeroingTimeout) {
-            m_momentumVelocity.setWidth(eventCoalescedDeltaX / timeDelta.seconds());
-            m_momentumVelocity.setHeight(eventCoalescedDeltaY / timeDelta.seconds());
+            m_momentumVelocity = eventCoalescedDelta / timeDelta.seconds();
             m_lastMomentumScrollTimestamp = wheelEvent.timestamp();
         } else {
             m_lastMomentumScrollTimestamp = wheelEvent.timestamp();
-            m_momentumVelocity = FloatSize();
+            m_momentumVelocity = { };
         }
 
@@ -193 +183 @@
                     deltaX = 0;
                 else if (fabsf(deltaX) < rubberbandMinimumRequiredDeltaBeforeStretch) {
-                    m_overflowScrollDelta.setWidth(m_overflowScrollDelta.width() + deltaX);
+                    m_unappliedOverscrollDelta.expand(deltaX, 0);
                     deltaX = 0;
                 } else
-                    m_overflowScrollDelta.setWidth(m_overflowScrollDelta.width() + deltaX);
+                    m_unappliedOverscrollDelta.expand(deltaX, 0);
             }
         } else if (isHorizontallyStretched) {
@@ -204 +194 @@
                     deltaY = 0;
                 else if (fabsf(deltaY) < rubberbandMinimumRequiredDeltaBeforeStretch) {
-                    m_overflowScrollDelta.setHeight(m_overflowScrollDelta.height() + deltaY);
+                    m_unappliedOverscrollDelta.expand(0, deltaY);
                     deltaY = 0;
                 } else
-                    m_overflowScrollDelta.setHeight(m_overflowScrollDelta.height() + deltaY);
+                    m_unappliedOverscrollDelta.expand(0, deltaY);
             }
         } else {
@@ -214 +204 @@
                 if (fabsf(deltaY) >= fabsf(deltaX)) {
                     if (fabsf(deltaX) < rubberbandMinimumRequiredDeltaBeforeStretch) {
-                        m_overflowScrollDelta.setWidth(m_overflowScrollDelta.width() + deltaX);
+                        m_unappliedOverscrollDelta.expand(deltaX, 0);
                         deltaX = 0;
                     } else
-                        m_overflowScrollDelta.setWidth(m_overflowScrollDelta.width() + deltaX);
+                        m_unappliedOverscrollDelta.expand(deltaX, 0);
                 }
 
@@ -247 +237 @@
                 if (!m_client.allowsHorizontalStretching(wheelEvent)) {
                     deltaX = 0;
-                    eventCoalescedDeltaX = 0;
+                    eventCoalescedDelta.setWidth(0);
                     handled = false;
                 } else if (!isHorizontallyStretched && !m_client.isPinnedForScrollDelta(FloatSize(deltaX, 0))) {
@@ -260 +250 @@
                 if (!m_client.allowsVerticalStretching(wheelEvent)) {
                     deltaY = 0;
-                    eventCoalescedDeltaY = 0;
+                    eventCoalescedDelta.setHeight(0);
                     handled = false;
                 } else if (!isVerticallyStretched && !m_client.isPinnedForScrollDelta(FloatSize(0, deltaY))) {
@@ -273 +263 @@
 
             if (m_momentumScrollInProgress) {
-                if ((m_client.isPinnedForScrollDelta(FloatSize(eventCoalescedDeltaX, eventCoalescedDeltaY)) || (fabsf(eventCoalescedDeltaX) + fabsf(eventCoalescedDeltaY) <= 0)) && m_lastMomentumScrollTimestamp) {
+                if ((m_client.isPinnedForScrollDelta(eventCoalescedDelta) || eventCoalescedDelta.isZero()) && m_lastMomentumScrollTimestamp) {
                     m_ignoreMomentumScrolls = true;
                     m_momentumScrollInProgress = false;
@@ -285 +275 @@
             FloatSize dampedDelta(ceilf(elasticDeltaForReboundDelta(m_stretchScrollForce.width())), ceilf(elasticDeltaForReboundDelta(m_stretchScrollForce.height())));
 
-            LOG_WITH_STREAM(ScrollAnimations, stream << "ScrollingEffectsController::handleWheelEvent() - overscrolled by " << m_overflowScrollDelta << " stretchScrollForce " << m_stretchScrollForce << " move delta " << FloatSize(deltaX, deltaY) << " dampedDelta " << dampedDelta);
+            LOG_WITH_STREAM(ScrollAnimations, stream << "ScrollingEffectsController::handleWheelEvent() - stretchScrollForce " << m_stretchScrollForce << " move delta " << FloatSize(deltaX, deltaY) << " dampedDelta " << dampedDelta);
 
             m_client.immediateScrollByWithoutContentEdgeConstraints(dampedDelta - stretchAmount);
@@ -304 +294 @@
 FloatSize ScrollingEffectsController::wheelDeltaBiasingTowardsVertical(const PlatformWheelEvent& wheelEvent)
 {
-    auto deltaX = wheelEvent.deltaX();
-    auto deltaY = wheelEvent.deltaY();
-
-    if (fabsf(deltaY) >= fabsf(deltaX))
-        deltaX = 0;
-    else
-        deltaY = 0;
-
-    return { deltaX, deltaY };
+    return convertToProminentAxisFavoringVertical(wheelEvent.delta());
 }