Title: [286671] trunk/Source/WebKit
Revision
286671
Author
timothy_hor...@apple.com
Date
2021-12-08 11:58:13 -0800 (Wed, 08 Dec 2021)

Log Message

Momentum Event Dispatcher: Momentum tail should have montonically decreasing deltas and tail gaps
https://bugs.webkit.org/show_bug.cgi?id=233993
<rdar://problem/86118367>

Reviewed by Simon Fraser.

* WebProcess/WebPage/MomentumEventDispatcher.cpp:
(WebKit::MomentumEventDispatcher::consumeDeltaForCurrentTime):
(WebKit::MomentumEventDispatcher::buildOffsetTableWithInitialDelta):
(WebKit::MomentumEventDispatcher::computeNextDelta):
In order to avoid visual stutter due to integral rounding of the
deltas we dispatch in the momentum phase, switch to a table of
deltas instead of interpolating along the offset curve during the
tail end of the animation (when the rounding makes up a large
proportion of each delta).

(WebKit::MomentumEventDispatcher::equalizeTailGaps):
Sort the deltas up until the first zero to ensure a lack of unexpected
perceptual acceleration, and then inject skipped frames in order to
ensure that frames that have effective scroll movement (non-zero deltas)
are always spaced equally-or-further apart than earlier ones, but
never closer together (which, again, would be percieved as acceleration).

(WebKit::MomentumEventDispatcher::handleWheelEvent):
(WebKit::MomentumEventDispatcher::pushLogEntry):
(WebKit::MomentumEventDispatcher::flushLog):
Lastly, adjust some logging to make it easier to tell which row in
the output corresponds to an event delta or generated delta, so it's
easier to find skipped frames.
* WebProcess/WebPage/MomentumEventDispatcher.h:

Modified Paths

Diff

Modified: trunk/Source/WebKit/ChangeLog (286670 => 286671)


--- trunk/Source/WebKit/ChangeLog	2021-12-08 19:46:15 UTC (rev 286670)
+++ trunk/Source/WebKit/ChangeLog	2021-12-08 19:58:13 UTC (rev 286671)
@@ -1,3 +1,36 @@
+2021-12-08  Tim Horton  <timothy_hor...@apple.com>
+
+        Momentum Event Dispatcher: Momentum tail should have montonically decreasing deltas and tail gaps
+        https://bugs.webkit.org/show_bug.cgi?id=233993
+        <rdar://problem/86118367>
+
+        Reviewed by Simon Fraser.
+
+        * WebProcess/WebPage/MomentumEventDispatcher.cpp:
+        (WebKit::MomentumEventDispatcher::consumeDeltaForCurrentTime):
+        (WebKit::MomentumEventDispatcher::buildOffsetTableWithInitialDelta):
+        (WebKit::MomentumEventDispatcher::computeNextDelta):
+        In order to avoid visual stutter due to integral rounding of the
+        deltas we dispatch in the momentum phase, switch to a table of
+        deltas instead of interpolating along the offset curve during the
+        tail end of the animation (when the rounding makes up a large
+        proportion of each delta).
+
+        (WebKit::MomentumEventDispatcher::equalizeTailGaps):
+        Sort the deltas up until the first zero to ensure a lack of unexpected
+        perceptual acceleration, and then inject skipped frames in order to
+        ensure that frames that have effective scroll movement (non-zero deltas)
+        are always spaced equally-or-further apart than earlier ones, but
+        never closer together (which, again, would be percieved as acceleration).
+
+        (WebKit::MomentumEventDispatcher::handleWheelEvent):
+        (WebKit::MomentumEventDispatcher::pushLogEntry):
+        (WebKit::MomentumEventDispatcher::flushLog):
+        Lastly, adjust some logging to make it easier to tell which row in
+        the output corresponds to an event delta or generated delta, so it's
+        easier to find skipped frames.
+        * WebProcess/WebPage/MomentumEventDispatcher.h:
+
 2021-12-08  Chris Dumez  <cdu...@apple.com>
 
         Make KVO work for WKWebpagePreferences._captivePortalModeEnabled

Modified: trunk/Source/WebKit/WebProcess/WebPage/MomentumEventDispatcher.cpp (286670 => 286671)


--- trunk/Source/WebKit/WebProcess/WebPage/MomentumEventDispatcher.cpp	2021-12-08 19:46:15 UTC (rev 286670)
+++ trunk/Source/WebKit/WebProcess/WebPage/MomentumEventDispatcher.cpp	2021-12-08 19:58:13 UTC (rev 286671)
@@ -113,15 +113,15 @@
         m_currentGesture.accumulatedEventOffset += event.delta();
 
     auto combinedPhase = (event.phase() << 8) | (event.momentumPhase());
-    m_currentLogState.latestEventPhase = combinedPhase;
     m_currentLogState.totalEventOffset += event.delta().height();
     if (!isMomentumEventDuringSyntheticGesture) {
         // Log events that we don't block to the generated offsets log as well,
         // even though we didn't technically generate them, just passed them through.
-        m_currentLogState.latestGeneratedPhase = combinedPhase;
         m_currentLogState.totalGeneratedOffset += event.delta().height();
-    }
-    pushLogEntry();
+        pushLogEntry(combinedPhase, combinedPhase);
+    } else
+        pushLogEntry(0, combinedPhase);
+    
 #endif
 
     // Consume any normal momentum events while we're inside a synthetic momentum gesture.
@@ -177,9 +177,8 @@
     m_dispatcher.internalWheelEvent(m_currentGesture.pageIdentifier, syntheticEvent, m_lastRubberBandableEdges, EventDispatcher::WheelEventOrigin::MomentumEventDispatcher);
 
 #if ENABLE(MOMENTUM_EVENT_DISPATCHER_TEMPORARY_LOGGING)
-    m_currentLogState.latestGeneratedPhase = phase;
     m_currentLogState.totalGeneratedOffset += appKitAcceleratedDelta.height();
-    pushLogEntry();
+    pushLogEntry(phase, 0);
 #endif
 }
 
@@ -219,7 +218,7 @@
     dispatchSyntheticMomentumEvent(WebWheelEvent::PhaseEnded, { });
 
 #if ENABLE(MOMENTUM_EVENT_DISPATCHER_TEMPORARY_LOGGING)
-    RELEASE_LOG(ScrollAnimations, "MomentumEventDispatcher saw momentum end phase with total offset %.1f %.1f, duration %f (event offset would have been %.1f %.1f)", m_currentGesture.currentOffset.width(), m_currentGesture.currentOffset.height(), (MonotonicTime::now() - m_currentGesture.startTime).seconds(), m_currentGesture.accumulatedEventOffset.width(), m_currentGesture.accumulatedEventOffset.height());
+    RELEASE_LOG(ScrollAnimations, "MomentumEventDispatcher saw momentum end phase with total offset %.1f %.1f, duration %f (event offset would have been %.1f %.1f) (tail index %d of %zu)", m_currentGesture.currentOffset.width(), m_currentGesture.currentOffset.height(), (MonotonicTime::now() - m_currentGesture.startTime).seconds(), m_currentGesture.accumulatedEventOffset.width(), m_currentGesture.accumulatedEventOffset.height(), m_currentGesture.currentTailDeltaIndex, m_currentGesture.tailDeltaTable.size());
     m_dispatcher.queue().dispatchAfter(1_s, [this] {
         flushLog();
     });
@@ -294,16 +293,19 @@
 
 WebCore::FloatSize MomentumEventDispatcher::consumeDeltaForCurrentTime()
 {
+    WebCore::FloatSize delta;
+
     auto animationTime = MonotonicTime::now() - m_currentGesture.startTime;
-    auto desiredOffset = offsetAtTime(animationTime);
+    if (animationTime < m_currentGesture.tailStartDelay) {
+        auto desiredOffset = offsetAtTime(animationTime);
+        delta = roundedIntSize(desiredOffset - m_currentGesture.currentOffset);
+    } else {
+        if (m_currentGesture.currentTailDeltaIndex < m_currentGesture.tailDeltaTable.size())
+            delta = -m_currentGesture.tailDeltaTable[m_currentGesture.currentTailDeltaIndex++];
+        else
+            delta = { };
+    }
 
-#if !ENABLE(MOMENTUM_EVENT_DISPATCHER_PREMATURE_ROUNDING)
-    // Intentional delta rounding (but at the end!).
-    WebCore::FloatSize delta = roundedIntSize(desiredOffset - m_currentGesture.currentOffset);
-#else
-    WebCore::FloatSize delta = desiredOffset - m_currentGesture.currentOffset;
-#endif
-
     m_currentGesture.currentOffset += delta;
 
     if (m_currentGesture.initiatingEvent->directionInvertedFromDevice())
@@ -363,27 +365,126 @@
 
 void MomentumEventDispatcher::buildOffsetTableWithInitialDelta(WebCore::FloatSize initialUnacceleratedDelta)
 {
-#if ENABLE(MOMENTUM_EVENT_DISPATCHER_PREMATURE_ROUNDING)
-    m_currentGesture.carryOffset = { };
-#endif
     m_currentGesture.offsetTable.clear();
 
     WebCore::FloatSize accumulatedOffset;
     WebCore::FloatSize unacceleratedDelta = initialUnacceleratedDelta;
 
+    float physicalCurveMultiplier = idealCurveFrameRate / m_currentGesture.accelerationCurve->frameRate();
+    bool inTail = false;
+    WebCore::FloatSize tailCarry;
+
     do {
         WebCore::FloatSize acceleratedDelta;
         std::tie(unacceleratedDelta, acceleratedDelta) = computeNextDelta(unacceleratedDelta);
 
+        const float tailStartUnacceleratedDelta = 6.f;
+        if (!inTail && std::abs(unacceleratedDelta.width()) < tailStartUnacceleratedDelta && std::abs(unacceleratedDelta.height()) < tailStartUnacceleratedDelta) {
+            inTail = true;
+            m_currentGesture.tailStartDelay = idealCurveFrameInterval * m_currentGesture.offsetTable.size();
+        }
+
+        if (inTail) {
+            auto tailDelta = acceleratedDelta * physicalCurveMultiplier;
+            auto deltaWithCarry = tailDelta + tailCarry;
+            auto quantizedDelta = roundedIntSize(deltaWithCarry);
+            tailCarry = deltaWithCarry - quantizedDelta;
+            m_currentGesture.tailDeltaTable.append(quantizedDelta);
+        }
+
         accumulatedOffset += acceleratedDelta;
         m_currentGesture.offsetTable.append(accumulatedOffset);
+
     } while (std::abs(unacceleratedDelta.width()) > 0.5 || std::abs(unacceleratedDelta.height()) > 0.5);
 
+    equalizeTailGaps();
+
 #if ENABLE(MOMENTUM_EVENT_DISPATCHER_TEMPORARY_LOGGING)
-    RELEASE_LOG(ScrollAnimations, "MomentumEventDispatcher built table with %ld frames, initial delta %f %f, distance %f %f (initial delta from last changed event %f %f)", m_currentGesture.offsetTable.size(), initialUnacceleratedDelta.width(), initialUnacceleratedDelta.height(), accumulatedOffset.width(), accumulatedOffset.height(), m_lastActivePhaseDelta.width(), m_lastActivePhaseDelta.height());
+    RELEASE_LOG(ScrollAnimations, "MomentumEventDispatcher built table with %ld frames, %.1f seconds (%ld tail frames, %.1f seconds, starting at %.1f), initial delta %.1f %.1f, distance %.1f %.1f (initial delta from last changed event %.1f %.1f)", m_currentGesture.offsetTable.size(), idealCurveFrameInterval.seconds() * m_currentGesture.offsetTable.size(), m_currentGesture.tailDeltaTable.size(), m_currentGesture.tailDeltaTable.size() * (1.f / m_currentGesture.accelerationCurve->frameRate()), m_currentGesture.tailStartDelay.seconds(), initialUnacceleratedDelta.width(), initialUnacceleratedDelta.height(), accumulatedOffset.width(), accumulatedOffset.height(), m_lastActivePhaseDelta.width(), m_lastActivePhaseDelta.height());
 #endif
 }
 
+void MomentumEventDispatcher::equalizeTailGaps()
+{
+    // Sort the deltas up until the first zero to ensure a lack of unexpected
+    // perceptual acceleration, and then inject skipped frames in order to
+    // ensure that frames that have effective scroll movement (non-zero deltas)
+    // are always spaced equally-or-further apart than earlier ones, but
+    // never closer together.
+
+    auto& table = m_currentGesture.tailDeltaTable;
+    size_t initialTableSize = table.size();
+
+    enum Axis { Horizontal, Vertical };
+    Vector<float> deltas[2];
+    unsigned firstZeroIndex[2] = { 0, 0 };
+    deltas[Horizontal].reserveInitialCapacity(initialTableSize);
+    deltas[Vertical].reserveInitialCapacity(initialTableSize);
+    for (unsigned i = 0; i < initialTableSize; i++) {
+        deltas[Horizontal].uncheckedAppend(table[i].width());
+        if (!firstZeroIndex[Horizontal] && !table[i].width())
+            firstZeroIndex[Horizontal] = i;
+
+        deltas[Vertical].uncheckedAppend(table[i].height());
+        if (!firstZeroIndex[Vertical] && !table[i].height())
+            firstZeroIndex[Vertical] = i;
+    }
+    
+    if (auto index = firstZeroIndex[Horizontal])
+        std::sort(deltas[Horizontal].begin(), std::next(deltas[Horizontal].begin(), index));
+    if (auto index = firstZeroIndex[Vertical])
+        std::sort(deltas[Vertical].begin(), std::next(deltas[Vertical].begin(), index));
+
+    // GapSize is a count of contiguous frames with zero deltas.
+    typedef unsigned GapSize[2];
+    GapSize minimumGap = { 0, 0 };
+    GapSize currentGap = { 0, 0 };
+    GapSize remainingGapToGenerate = { 0, 0 };
+    unsigned originalTableIndex[2] = { 0, 0 };
+
+    auto takeNextDelta = [&] (uint8_t axis) -> float {
+        if (originalTableIndex[axis] >= initialTableSize)
+            return 0.f;
+
+        if (remainingGapToGenerate[axis]) {
+            --remainingGapToGenerate[axis];
+            ++currentGap[axis];
+            return 0.f;
+        }
+
+        auto value = deltas[axis][originalTableIndex[axis]];
+        if (value) {
+            minimumGap[axis] = std::max(minimumGap[axis], currentGap[axis]);
+            remainingGapToGenerate[axis] = minimumGap[axis] - currentGap[axis];
+            if (remainingGapToGenerate[axis]) {
+                --remainingGapToGenerate[axis];
+                ++currentGap[axis];
+                return 0.f;
+            }
+
+            currentGap[axis] = 0;
+        } else
+            ++currentGap[axis];
+
+        ++originalTableIndex[axis];
+
+        return value;
+    };
+
+    size_t finalTableSize = 0;
+    table.shrink(0);
+
+    while (originalTableIndex[Horizontal] < initialTableSize || originalTableIndex[Vertical] < initialTableSize) {
+        WebCore::FloatSize delta(takeNextDelta(Horizontal), takeNextDelta(Vertical));
+        table.append(delta);
+
+        if (!delta.isZero())
+            finalTableSize = table.size();
+    }
+
+    table.shrink(finalTableSize);
+}
+
 static float interpolate(float a, float b, float t)
 {
     return a + t * (b - a);
@@ -433,28 +534,8 @@
     float decayRate = momentumDecayRate(unacceleratedDelta, idealCurveFrameInterval);
     unacceleratedDelta.scale(decayRate);
 
-    auto quantizedUnacceleratedDelta = unacceleratedDelta;
-
-#if ENABLE(MOMENTUM_EVENT_DISPATCHER_PREMATURE_ROUNDING)
-    // Round and carry.
-    int32_t quantizedX = std::round(quantizedUnacceleratedDelta.width());
-    int32_t quantizedY = std::round(quantizedUnacceleratedDelta.height());
-
-    if (std::abs(quantizedUnacceleratedDelta.width()) < 1 && std::abs(quantizedUnacceleratedDelta.height()) < 1) {
-        float deltaXIncludingCarry = quantizedUnacceleratedDelta.width() + m_currentGesture.carryOffset.width();
-        float deltaYIncludingCarry = quantizedUnacceleratedDelta.height() + m_currentGesture.carryOffset.height();
-
-        // Intentional truncation.
-        quantizedX = deltaXIncludingCarry;
-        quantizedY = deltaYIncludingCarry;
-        m_currentGesture.carryOffset = { deltaXIncludingCarry - quantizedX, deltaYIncludingCarry - quantizedY };
-    }
-
-    quantizedUnacceleratedDelta = { static_cast<float>(quantizedX), static_cast<float>(quantizedY) };
-#endif
-
     // The delta queue operates on pre-acceleration deltas, so insert the new event *before* accelerating.
-    didReceiveScrollEventWithInterval(quantizedUnacceleratedDelta, idealCurveFrameInterval);
+    didReceiveScrollEventWithInterval(unacceleratedDelta, idealCurveFrameInterval);
 
     auto accelerateAxis = [&] (HistoricalDeltas& deltas, float value) {
         float totalDelta = 0;
@@ -499,8 +580,8 @@
     };
 
     WebCore::FloatSize acceleratedDelta(
-        accelerateAxis(m_deltaHistoryX, quantizedUnacceleratedDelta.width()),
-        accelerateAxis(m_deltaHistoryY, quantizedUnacceleratedDelta.height())
+        accelerateAxis(m_deltaHistoryX, unacceleratedDelta.width()),
+        accelerateAxis(m_deltaHistoryY, unacceleratedDelta.height())
     );
 
 #if ENABLE(MOMENTUM_EVENT_DISPATCHER_TEMPORARY_LOGGING)
@@ -512,9 +593,11 @@
 
 #if ENABLE(MOMENTUM_EVENT_DISPATCHER_TEMPORARY_LOGGING)
 
-void MomentumEventDispatcher::pushLogEntry()
+void MomentumEventDispatcher::pushLogEntry(uint32_t generatedPhase, uint32_t eventPhase)
 {
     m_currentLogState.time = MonotonicTime::now();
+    m_currentLogState.generatedPhase = generatedPhase;
+    m_currentLogState.eventPhase = eventPhase;
     m_log.append(m_currentLogState);
 }
 
@@ -529,7 +612,7 @@
     auto startTime = m_log[0].time;
     RELEASE_LOG(ScrollAnimations, "MomentumEventDispatcher event log: time,generatedOffset,generatedPhase,eventOffset,eventPhase");
     for (const auto& entry : m_log)
-        RELEASE_LOG(ScrollAnimations, "MomentumEventDispatcher event log: %f,%f,%d,%f,%d", (entry.time - startTime).seconds(), entry.totalGeneratedOffset, entry.latestGeneratedPhase, entry.totalEventOffset, entry.latestEventPhase);
+        RELEASE_LOG(ScrollAnimations, "MomentumEventDispatcher event log: %f,%f,%d,%f,%d", (entry.time - startTime).seconds(), entry.totalGeneratedOffset, entry.generatedPhase, entry.totalEventOffset, entry.eventPhase);
 
     m_log.clear();
     m_currentLogState = { };

Modified: trunk/Source/WebKit/WebProcess/WebPage/MomentumEventDispatcher.h (286670 => 286671)


--- trunk/Source/WebKit/WebProcess/WebPage/MomentumEventDispatcher.h	2021-12-08 19:46:15 UTC (rev 286670)
+++ trunk/Source/WebKit/WebProcess/WebPage/MomentumEventDispatcher.h	2021-12-08 19:58:13 UTC (rev 286671)
@@ -27,8 +27,6 @@
 
 #if ENABLE(MOMENTUM_EVENT_DISPATCHER)
 
-// FIXME: Remove this once we decide which version we want.
-#define ENABLE_MOMENTUM_EVENT_DISPATCHER_PREMATURE_ROUNDING 0
 #define ENABLE_MOMENTUM_EVENT_DISPATCHER_TEMPORARY_LOGGING 1
 
 #include "DisplayLinkObserverID.h"
@@ -80,6 +78,7 @@
     void dispatchSyntheticMomentumEvent(WebWheelEvent::Phase, WebCore::FloatSize delta);
 
     void buildOffsetTableWithInitialDelta(WebCore::FloatSize);
+    void equalizeTailGaps();
 
     // Once consumed, this delta *must* be dispatched in an event.
     WebCore::FloatSize consumeDeltaForCurrentTime();
@@ -91,7 +90,7 @@
     void didReceiveScrollEvent(const WebWheelEvent&);
 
 #if ENABLE(MOMENTUM_EVENT_DISPATCHER_TEMPORARY_LOGGING)
-    void pushLogEntry();
+    void pushLogEntry(uint32_t generatedPhase, uint32_t eventPhase);
     void flushLog();
 
     WebCore::FloatSize m_lastActivePhaseDelta;
@@ -102,8 +101,8 @@
         float totalGeneratedOffset { 0 };
         float totalEventOffset { 0 };
 
-        uint32_t latestGeneratedPhase { 0 };
-        uint32_t latestEventPhase { 0 };
+        uint32_t generatedPhase { 0 };
+        uint32_t eventPhase { 0 };
     };
     LogEntry m_currentLogState;
     Vector<LogEntry> m_log;
@@ -133,7 +132,10 @@
         WebCore::FloatSize currentOffset;
         MonotonicTime startTime;
 
-        Vector<WebCore::FloatSize> offsetTable;
+        Vector<WebCore::FloatSize> offsetTable; // Always at 60Hz intervals.
+        Vector<WebCore::FloatSize> tailDeltaTable; // Always at event dispatch intervals.
+        Seconds tailStartDelay;
+        unsigned currentTailDeltaIndex { 0 };
 
 #if ENABLE(MOMENTUM_EVENT_DISPATCHER_TEMPORARY_LOGGING)
         WebCore::FloatSize accumulatedEventOffset;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to