Modified: trunk/Source/WebCore/ChangeLog (283392 => 283393)
--- trunk/Source/WebCore/ChangeLog 2021-10-01 20:20:58 UTC (rev 283392)
+++ trunk/Source/WebCore/ChangeLog 2021-10-01 20:25:48 UTC (rev 283393)
@@ -1,3 +1,19 @@
+2021-10-01 Simon Fraser <simon.fra...@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 <za...@apple.com>
REGRESSION(r283047): PerformanceTests/Layout/line-layout-simple.html regressed by ~10%
Modified: trunk/Source/WebCore/platform/ScrollingEffectsController.h (283392 => 283393)
--- trunk/Source/WebCore/platform/ScrollingEffectsController.h 2021-10-01 20:20:58 UTC (rev 283392)
+++ trunk/Source/WebCore/platform/ScrollingEffectsController.h 2021-10-01 20:25:48 UTC (rev 283393)
@@ -198,6 +198,9 @@
void startDeferringWheelEventTestCompletionDueToScrollSnapping();
void stopDeferringWheelEventTestCompletionDueToScrollSnapping();
+ bool modifyScrollDeltaForStretching(const PlatformWheelEvent&, FloatSize&, bool isHorizontallyStretched, bool isVerticallyStretched);
+ bool applyScrollDeltaWithStretching(const PlatformWheelEvent&, FloatSize, bool isHorizontallyStretched, bool isVerticallyStretched);
+
void startRubberbandAnimationIfNecessary();
void startRubberbandAnimation();
void stopRubberbandAnimation();
Modified: trunk/Source/WebCore/platform/mac/ScrollingEffectsController.mm (283392 => 283393)
--- trunk/Source/WebCore/platform/mac/ScrollingEffectsController.mm 2021-10-01 20:20:58 UTC (rev 283392)
+++ trunk/Source/WebCore/platform/mac/ScrollingEffectsController.mm 2021-10-01 20:25:48 UTC (rev 283393)
@@ -176,33 +176,26 @@
return false;
}
- // 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;
+ // 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();
- // 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);
+ // 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;
- float deltaX = delta.width();
- float deltaY = delta.height();
-
- bool shouldStretch = false;
-
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 {
m_lastMomentumScrollTimestamp = wheelEvent.timestamp();
@@ -209,109 +202,30 @@
m_momentumVelocity = { };
}
- 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();
+ if (!delta.isZero()) {
+ if (shouldStretch || isVerticallyStretched || isHorizontallyStretched) {
+ if (delta.width() && !m_client.allowsHorizontalStretching(wheelEvent))
+ handled = false;
- m_client.immediateScrollByWithoutContentEdgeConstraints(FloatSize(deltaX, 0));
- deltaX = 0;
- }
- }
+ if (delta.height() && !m_client.allowsVerticalStretching(wheelEvent))
+ handled = false;
- 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();
-
+ 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;
startRubberbandAnimationIfNecessary();
}
}
-
- 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);
}
}
@@ -326,6 +240,114 @@
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;
+}
+
FloatSize ScrollingEffectsController::wheelDeltaBiasingTowardsVertical(const PlatformWheelEvent& wheelEvent)
{
return deltaAlignedToDominantAxis(wheelEvent.delta());
