- Revision
- 285953
- Author
- timothy_hor...@apple.com
- Date
- 2021-11-17 14:21:59 -0800 (Wed, 17 Nov 2021)
Log Message
Momentum animator sometimes starts the animation at a very high velocity
https://bugs.webkit.org/show_bug.cgi?id=233245
<rdar://problem/85307115>
Reviewed by Simon Fraser.
Source/WebCore:
* page/WheelEventDeltaFilter.cpp:
(WebCore::WheelEventDeltaFilter::eventCopyWithVelocity const):
* page/WheelEventDeltaFilter.h:
* platform/PlatformWheelEvent.h:
(WebCore::PlatformWheelEvent::copyWithVelocity const):
Make it possible to ask the delta filter to only add velocity data without filtering deltas.
We should later make ScrollingEffectsController talk directly to the
WheelEventDeltaFilter to get the velocity, but that requires a great
deal of scrolling thread plumbing.
* page/mac/WheelEventDeltaFilterMac.h:
* page/mac/WheelEventDeltaFilterMac.mm:
(WebCore::WheelEventDeltaFilterMac::updateFromEvent):
Factor updateCurrentVelocityFromEvent out.
Update the current velocity for momentum begin phase events as well.
(WebCore::WheelEventDeltaFilterMac::updateCurrentVelocityFromEvent):
* platform/ScrollingEffectsController.h:
* platform/mac/ScrollingEffectsController.mm:
(WebCore::ScrollingEffectsController::handleWheelEvent):
Start the momentum animation with the velocity provided by the momentum
begin phase, instead of a potentially incorrect delta from an earlier change phase.
Source/WebKit:
* WebProcess/WebPage/EventDispatcher.cpp:
(WebKit::EventDispatcher::wheelEvent):
Ensure that wheel events always have velocities attached, even for
events that don't go through the delta filter.
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (285952 => 285953)
--- trunk/Source/WebCore/ChangeLog 2021-11-17 21:35:01 UTC (rev 285952)
+++ trunk/Source/WebCore/ChangeLog 2021-11-17 22:21:59 UTC (rev 285953)
@@ -1,3 +1,35 @@
+2021-11-17 Tim Horton <timothy_hor...@apple.com>
+
+ Momentum animator sometimes starts the animation at a very high velocity
+ https://bugs.webkit.org/show_bug.cgi?id=233245
+ <rdar://problem/85307115>
+
+ Reviewed by Simon Fraser.
+
+ * page/WheelEventDeltaFilter.cpp:
+ (WebCore::WheelEventDeltaFilter::eventCopyWithVelocity const):
+ * page/WheelEventDeltaFilter.h:
+ * platform/PlatformWheelEvent.h:
+ (WebCore::PlatformWheelEvent::copyWithVelocity const):
+ Make it possible to ask the delta filter to only add velocity data without filtering deltas.
+ We should later make ScrollingEffectsController talk directly to the
+ WheelEventDeltaFilter to get the velocity, but that requires a great
+ deal of scrolling thread plumbing.
+
+ * page/mac/WheelEventDeltaFilterMac.h:
+ * page/mac/WheelEventDeltaFilterMac.mm:
+ (WebCore::WheelEventDeltaFilterMac::updateFromEvent):
+ Factor updateCurrentVelocityFromEvent out.
+ Update the current velocity for momentum begin phase events as well.
+
+ (WebCore::WheelEventDeltaFilterMac::updateCurrentVelocityFromEvent):
+
+ * platform/ScrollingEffectsController.h:
+ * platform/mac/ScrollingEffectsController.mm:
+ (WebCore::ScrollingEffectsController::handleWheelEvent):
+ Start the momentum animation with the velocity provided by the momentum
+ begin phase, instead of a potentially incorrect delta from an earlier change phase.
+
2021-11-17 Wenson Hsieh <wenson_hs...@apple.com>
Add a helper class to coordinate batch analysis of images
Modified: trunk/Source/WebCore/page/WheelEventDeltaFilter.cpp (285952 => 285953)
--- trunk/Source/WebCore/page/WheelEventDeltaFilter.cpp 2021-11-17 21:35:01 UTC (rev 285952)
+++ trunk/Source/WebCore/page/WheelEventDeltaFilter.cpp 2021-11-17 22:21:59 UTC (rev 285953)
@@ -62,11 +62,26 @@
#endif
}
+bool WheelEventDeltaFilter::shouldIncludeVelocityForEvent(const PlatformWheelEvent& event)
+{
+#if ENABLE(KINETIC_SCROLLING)
+ // Include velocity on momentum-began phases so that the momentum animator can use it.
+ if (event.momentumPhase() == PlatformWheelEventPhase::Began)
+ return true;
+#endif
+ return shouldApplyFilteringForEvent(event);
+}
+
PlatformWheelEvent WheelEventDeltaFilter::eventCopyWithFilteredDeltas(const PlatformWheelEvent& event) const
{
return event.copyWithDeltaAndVelocity(m_currentFilteredDelta, m_currentFilteredVelocity);
}
+PlatformWheelEvent WheelEventDeltaFilter::eventCopyWithVelocity(const PlatformWheelEvent& event) const
+{
+ return event.copyWithVelocity(m_currentFilteredVelocity);
+}
+
FloatSize WheelEventDeltaFilter::filteredDelta() const
{
return m_currentFilteredDelta;
Modified: trunk/Source/WebCore/page/WheelEventDeltaFilter.h (285952 => 285953)
--- trunk/Source/WebCore/page/WheelEventDeltaFilter.h 2021-11-17 21:35:01 UTC (rev 285952)
+++ trunk/Source/WebCore/page/WheelEventDeltaFilter.h 2021-11-17 22:21:59 UTC (rev 285953)
@@ -43,11 +43,13 @@
WEBCORE_EXPORT virtual void updateFromEvent(const PlatformWheelEvent&) = 0;
WEBCORE_EXPORT PlatformWheelEvent eventCopyWithFilteredDeltas(const PlatformWheelEvent&) const;
+ WEBCORE_EXPORT PlatformWheelEvent eventCopyWithVelocity(const PlatformWheelEvent&) const;
WEBCORE_EXPORT FloatSize filteredVelocity() const;
WEBCORE_EXPORT FloatSize filteredDelta() const;
WEBCORE_EXPORT static bool shouldApplyFilteringForEvent(const PlatformWheelEvent&);
+ WEBCORE_EXPORT static bool shouldIncludeVelocityForEvent(const PlatformWheelEvent&);
protected:
FloatSize m_currentFilteredDelta;
Modified: trunk/Source/WebCore/page/mac/WheelEventDeltaFilterMac.h (285952 => 285953)
--- trunk/Source/WebCore/page/mac/WheelEventDeltaFilterMac.h 2021-11-17 21:35:01 UTC (rev 285952)
+++ trunk/Source/WebCore/page/mac/WheelEventDeltaFilterMac.h 2021-11-17 22:21:59 UTC (rev 285953)
@@ -45,6 +45,8 @@
private:
void reset();
+ void updateCurrentVelocityFromEvent(const PlatformWheelEvent&);
+
RetainPtr<_NSScrollingPredominantAxisFilter> m_predominantAxisFilter;
WallTime m_initialWallTime;
WallTime m_lastIOHIDEventTimestamp;
Modified: trunk/Source/WebCore/page/mac/WheelEventDeltaFilterMac.mm (285952 => 285953)
--- trunk/Source/WebCore/page/mac/WheelEventDeltaFilterMac.mm 2021-11-17 21:35:01 UTC (rev 285952)
+++ trunk/Source/WebCore/page/mac/WheelEventDeltaFilterMac.mm 2021-11-17 22:21:59 UTC (rev 285953)
@@ -45,13 +45,12 @@
void WheelEventDeltaFilterMac::updateFromEvent(const PlatformWheelEvent& event)
{
if (event.momentumPhase() != PlatformWheelEventPhase::None) {
+ if (event.momentumPhase() == PlatformWheelEventPhase::Began)
+ updateCurrentVelocityFromEvent(event);
m_lastIOHIDEventTimestamp = event.ioHIDEventTimestamp();
return;
}
- // The absolute value of timestamp doesn't matter; the filter looks at deltas from the previous event.
- auto timestamp = event.timestamp() - m_initialWallTime;
-
switch (event.phase()) {
case PlatformWheelEventPhase::None:
case PlatformWheelEventPhase::Ended:
@@ -59,29 +58,13 @@
case PlatformWheelEventPhase::Began:
reset();
- FALLTHROUGH;
- case PlatformWheelEventPhase::Changed: {
- NSPoint filteredDeltaResult;
- NSPoint filteredVelocityResult;
+ updateCurrentVelocityFromEvent(event);
+ break;
- [m_predominantAxisFilter filterInputDelta:toFloatPoint(event.delta()) timestamp:timestamp.seconds() outputDelta:&filteredDeltaResult velocity:&filteredVelocityResult];
- auto axisFilteredVelocity = toFloatSize(filteredVelocityResult);
- m_currentFilteredDelta = toFloatSize(filteredDeltaResult);
+ case PlatformWheelEventPhase::Changed:
+ updateCurrentVelocityFromEvent(event);
+ break;
- // Use a 1ms minimum to avoid divide by zero. The usual cadence of these events matches screen refresh rate.
- auto deltaFromLastEvent = std::max(event.ioHIDEventTimestamp() - m_lastIOHIDEventTimestamp, 1_ms);
- m_currentFilteredVelocity = event.delta() / deltaFromLastEvent.seconds();
-
- // Apply the axis-locking that m_predominantAxisFilter does.
- if (!axisFilteredVelocity.width())
- m_currentFilteredVelocity.setWidth(0);
- if (!axisFilteredVelocity.height())
- m_currentFilteredVelocity.setHeight(0);
-
- LOG(ScrollAnimations, "WheelEventDeltaFilterMac::updateFromEvent: _NSScrollingPredominantAxisFilter velocity %.2f, %2f, IOHIDEvent velocity %.2f,%.2f",
- axisFilteredVelocity.width(), axisFilteredVelocity.height(), m_currentFilteredVelocity.width(), m_currentFilteredVelocity.height());
- break;
- }
case PlatformWheelEventPhase::MayBegin:
case PlatformWheelEventPhase::Cancelled:
case PlatformWheelEventPhase::Stationary:
@@ -92,6 +75,32 @@
m_lastIOHIDEventTimestamp = event.ioHIDEventTimestamp();
}
+void WheelEventDeltaFilterMac::updateCurrentVelocityFromEvent(const PlatformWheelEvent& event)
+{
+ // The absolute value of timestamp doesn't matter; the filter looks at deltas from the previous event.
+ auto timestamp = event.timestamp() - m_initialWallTime;
+
+ NSPoint filteredDeltaResult;
+ NSPoint filteredVelocityResult;
+
+ [m_predominantAxisFilter filterInputDelta:toFloatPoint(event.delta()) timestamp:timestamp.seconds() outputDelta:&filteredDeltaResult velocity:&filteredVelocityResult];
+ auto axisFilteredVelocity = toFloatSize(filteredVelocityResult);
+ m_currentFilteredDelta = toFloatSize(filteredDeltaResult);
+
+ // Use a 1ms minimum to avoid divide by zero. The usual cadence of these events matches screen refresh rate.
+ auto deltaFromLastEvent = std::max(event.ioHIDEventTimestamp() - m_lastIOHIDEventTimestamp, 1_ms);
+ m_currentFilteredVelocity = event.delta() / deltaFromLastEvent.seconds();
+
+ // Apply the axis-locking that m_predominantAxisFilter does.
+ if (!axisFilteredVelocity.width())
+ m_currentFilteredVelocity.setWidth(0);
+ if (!axisFilteredVelocity.height())
+ m_currentFilteredVelocity.setHeight(0);
+
+ LOG(ScrollAnimations, "WheelEventDeltaFilterMac::updateFromEvent: _NSScrollingPredominantAxisFilter velocity %.2f, %2f, IOHIDEvent velocity %.2f,%.2f",
+ axisFilteredVelocity.width(), axisFilteredVelocity.height(), m_currentFilteredVelocity.width(), m_currentFilteredVelocity.height());
+}
+
void WheelEventDeltaFilterMac::reset()
{
[m_predominantAxisFilter reset];
Modified: trunk/Source/WebCore/platform/PlatformWheelEvent.h (285952 => 285953)
--- trunk/Source/WebCore/platform/PlatformWheelEvent.h 2021-11-17 21:35:01 UTC (rev 285952)
+++ trunk/Source/WebCore/platform/PlatformWheelEvent.h 2021-11-17 22:21:59 UTC (rev 285953)
@@ -129,6 +129,13 @@
return copy;
}
+ PlatformWheelEvent copyWithVelocity(FloatSize velocity) const
+ {
+ PlatformWheelEvent copy = *this;
+ copy.m_scrollingVelocity = velocity;
+ return copy;
+ }
+
const IntPoint& position() const { return m_position; } // PlatformWindow coordinates.
const IntPoint& globalPosition() const { return m_globalPosition; } // Screen coordinates.
Modified: trunk/Source/WebCore/platform/ScrollingEffectsController.h (285952 => 285953)
--- trunk/Source/WebCore/platform/ScrollingEffectsController.h 2021-11-17 21:35:01 UTC (rev 285952)
+++ trunk/Source/WebCore/platform/ScrollingEffectsController.h 2021-11-17 22:21:59 UTC (rev 285953)
@@ -260,7 +260,6 @@
FloatSize m_stretchScrollForce;
FloatSize m_momentumVelocity;
- FloatSize m_scrollingVelocityForMomentumAnimation; // Do we need both this, m_scrollingVelocityForScrollSnap and m_momentumVelocity?
FloatSize m_scrollingVelocityForScrollSnap;
#if !LOG_DISABLED
FloatPoint m_eventDrivenScrollMomentumStartOffset;
Modified: trunk/Source/WebCore/platform/mac/ScrollingEffectsController.mm (285952 => 285953)
--- trunk/Source/WebCore/platform/mac/ScrollingEffectsController.mm 2021-11-17 21:35:01 UTC (rev 285952)
+++ trunk/Source/WebCore/platform/mac/ScrollingEffectsController.mm 2021-11-17 22:21:59 UTC (rev 285953)
@@ -129,9 +129,6 @@
bool ScrollingEffectsController::handleWheelEvent(const PlatformWheelEvent& wheelEvent)
{
- if (WheelEventDeltaFilter::shouldApplyFilteringForEvent(wheelEvent))
- m_scrollingVelocityForMomentumAnimation = -wheelEvent.scrollingVelocity(); // Note that event delta is reversed from scroll direction.
-
if (processWheelEventForScrollSnap(wheelEvent))
return true;
@@ -228,7 +225,7 @@
if (!m_momentumScrollInProgress && (momentumPhase == PlatformWheelEventPhase::Began || momentumPhase == PlatformWheelEventPhase::Changed)) {
m_momentumScrollInProgress = true;
if (momentumScrollingAnimatorEnabled()) {
- startMomentumScrollWithInitialVelocity(m_client.scrollOffset(), m_scrollingVelocityForMomentumAnimation, -wheelEvent.delta(), [](const FloatPoint& targetOffset) { return targetOffset; });
+ startMomentumScrollWithInitialVelocity(m_client.scrollOffset(), -wheelEvent.scrollingVelocity(), -wheelEvent.delta(), [](const FloatPoint& targetOffset) { return targetOffset; });
#if !LOG_DISABLED
m_eventDrivenScrollOffset = m_client.scrollOffset();
m_eventDrivenScrollMomentumStartOffset = m_eventDrivenScrollOffset;
Modified: trunk/Source/WebKit/ChangeLog (285952 => 285953)
--- trunk/Source/WebKit/ChangeLog 2021-11-17 21:35:01 UTC (rev 285952)
+++ trunk/Source/WebKit/ChangeLog 2021-11-17 22:21:59 UTC (rev 285953)
@@ -1,3 +1,16 @@
+2021-11-17 Tim Horton <timothy_hor...@apple.com>
+
+ Momentum animator sometimes starts the animation at a very high velocity
+ https://bugs.webkit.org/show_bug.cgi?id=233245
+ <rdar://problem/85307115>
+
+ Reviewed by Simon Fraser.
+
+ * WebProcess/WebPage/EventDispatcher.cpp:
+ (WebKit::EventDispatcher::wheelEvent):
+ Ensure that wheel events always have velocities attached, even for
+ events that don't go through the delta filter.
+
2021-11-17 Wenson Hsieh <wenson_hs...@apple.com>
Add a helper class to coordinate batch analysis of images
Modified: trunk/Source/WebKit/WebProcess/WebPage/EventDispatcher.cpp (285952 => 285953)
--- trunk/Source/WebKit/WebProcess/WebPage/EventDispatcher.cpp 2021-11-17 21:35:01 UTC (rev 285952)
+++ trunk/Source/WebKit/WebProcess/WebPage/EventDispatcher.cpp 2021-11-17 22:21:59 UTC (rev 285953)
@@ -106,6 +106,8 @@
m_recentWheelEventDeltaFilter->updateFromEvent(platformWheelEvent);
if (WheelEventDeltaFilter::shouldApplyFilteringForEvent(platformWheelEvent))
platformWheelEvent = m_recentWheelEventDeltaFilter->eventCopyWithFilteredDeltas(platformWheelEvent);
+ else if (WheelEventDeltaFilter::shouldIncludeVelocityForEvent(platformWheelEvent))
+ platformWheelEvent = m_recentWheelEventDeltaFilter->eventCopyWithVelocity(platformWheelEvent);
#endif
Locker locker { m_scrollingTreesLock };
