- Revision
- 286991
- Author
- alanc...@apple.com
- Date
- 2021-12-13 15:49:02 -0800 (Mon, 13 Dec 2021)
Log Message
Cherry-pick r286919. rdar://problem/86247557
Momentum Event Dispatcher: Tail frames are the wrong velocity if momentum event dispatch rate doesn't match screen refresh rate
https://bugs.webkit.org/show_bug.cgi?id=234168
<rdar://problem/86247557>
Reviewed by Simon Fraser.
In r286671, I scaled the tail frames into the momentum event disaptch
rate, but they are actually always dispatched at display refresh
frequency. In many cases these things are the same, but in some
cases can differ (most commonly a 120Hz display with 60Hz event dispatch),
so to always have the tail move at the right rate, scale into the display
refresh rate instead).
* UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::windowScreenDidChange):
* WebProcess/WebPage/EventDispatcher.cpp:
(WebKit::EventDispatcher::pageScreenDidChange):
* WebProcess/WebPage/EventDispatcher.h:
* WebProcess/WebPage/EventDispatcher.messages.in:
* WebProcess/WebPage/MomentumEventDispatcher.cpp:
(WebKit::MomentumEventDispatcher::didStartMomentumPhase):
(WebKit::MomentumEventDispatcher::displayProperties const):
(WebKit::MomentumEventDispatcher::startDisplayLink):
(WebKit::MomentumEventDispatcher::stopDisplayLink):
(WebKit::MomentumEventDispatcher::pageScreenDidChange):
(WebKit::MomentumEventDispatcher::displayWasRefreshed):
(WebKit::MomentumEventDispatcher::displayID const): Deleted.
* WebProcess/WebPage/MomentumEventDispatcher.h:
Plumb and store the nominal display refresh rate.
(WebKit::MomentumEventDispatcher::buildOffsetTableWithInitialDelta):
Scale the tail frames from the 60Hz ideal rate into the display refresh
rate, instead of the event dispatch rate.
Incoming events still scale *in* from the event dispatch rate, since
that's... the rate they come at.
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@286919 268f45cc-cd09-0410-ab3c-d52691b4dbfc
Modified Paths
Diff
Modified: branches/safari-612-branch/Source/WebKit/ChangeLog (286990 => 286991)
--- branches/safari-612-branch/Source/WebKit/ChangeLog 2021-12-13 23:45:36 UTC (rev 286990)
+++ branches/safari-612-branch/Source/WebKit/ChangeLog 2021-12-13 23:49:02 UTC (rev 286991)
@@ -1,5 +1,88 @@
2021-12-13 Alan Coon <alanc...@apple.com>
+ Cherry-pick r286919. rdar://problem/86247557
+
+ Momentum Event Dispatcher: Tail frames are the wrong velocity if momentum event dispatch rate doesn't match screen refresh rate
+ https://bugs.webkit.org/show_bug.cgi?id=234168
+ <rdar://problem/86247557>
+
+ Reviewed by Simon Fraser.
+
+ In r286671, I scaled the tail frames into the momentum event disaptch
+ rate, but they are actually always dispatched at display refresh
+ frequency. In many cases these things are the same, but in some
+ cases can differ (most commonly a 120Hz display with 60Hz event dispatch),
+ so to always have the tail move at the right rate, scale into the display
+ refresh rate instead).
+
+ * UIProcess/WebPageProxy.cpp:
+ (WebKit::WebPageProxy::windowScreenDidChange):
+ * WebProcess/WebPage/EventDispatcher.cpp:
+ (WebKit::EventDispatcher::pageScreenDidChange):
+ * WebProcess/WebPage/EventDispatcher.h:
+ * WebProcess/WebPage/EventDispatcher.messages.in:
+ * WebProcess/WebPage/MomentumEventDispatcher.cpp:
+ (WebKit::MomentumEventDispatcher::didStartMomentumPhase):
+ (WebKit::MomentumEventDispatcher::displayProperties const):
+ (WebKit::MomentumEventDispatcher::startDisplayLink):
+ (WebKit::MomentumEventDispatcher::stopDisplayLink):
+ (WebKit::MomentumEventDispatcher::pageScreenDidChange):
+ (WebKit::MomentumEventDispatcher::displayWasRefreshed):
+ (WebKit::MomentumEventDispatcher::displayID const): Deleted.
+ * WebProcess/WebPage/MomentumEventDispatcher.h:
+ Plumb and store the nominal display refresh rate.
+
+ (WebKit::MomentumEventDispatcher::buildOffsetTableWithInitialDelta):
+ Scale the tail frames from the 60Hz ideal rate into the display refresh
+ rate, instead of the event dispatch rate.
+
+ Incoming events still scale *in* from the event dispatch rate, since
+ that's... the rate they come at.
+
+
+ git-svn-id: https://svn.webkit.org/repository/webkit/trunk@286919 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+ 2021-12-11 Tim Horton <timothy_hor...@apple.com>
+
+ Momentum Event Dispatcher: Tail frames are the wrong velocity if momentum event dispatch rate doesn't match screen refresh rate
+ https://bugs.webkit.org/show_bug.cgi?id=234168
+ <rdar://problem/86247557>
+
+ Reviewed by Simon Fraser.
+
+ In r286671, I scaled the tail frames into the momentum event disaptch
+ rate, but they are actually always dispatched at display refresh
+ frequency. In many cases these things are the same, but in some
+ cases can differ (most commonly a 120Hz display with 60Hz event dispatch),
+ so to always have the tail move at the right rate, scale into the display
+ refresh rate instead).
+
+ * UIProcess/WebPageProxy.cpp:
+ (WebKit::WebPageProxy::windowScreenDidChange):
+ * WebProcess/WebPage/EventDispatcher.cpp:
+ (WebKit::EventDispatcher::pageScreenDidChange):
+ * WebProcess/WebPage/EventDispatcher.h:
+ * WebProcess/WebPage/EventDispatcher.messages.in:
+ * WebProcess/WebPage/MomentumEventDispatcher.cpp:
+ (WebKit::MomentumEventDispatcher::didStartMomentumPhase):
+ (WebKit::MomentumEventDispatcher::displayProperties const):
+ (WebKit::MomentumEventDispatcher::startDisplayLink):
+ (WebKit::MomentumEventDispatcher::stopDisplayLink):
+ (WebKit::MomentumEventDispatcher::pageScreenDidChange):
+ (WebKit::MomentumEventDispatcher::displayWasRefreshed):
+ (WebKit::MomentumEventDispatcher::displayID const): Deleted.
+ * WebProcess/WebPage/MomentumEventDispatcher.h:
+ Plumb and store the nominal display refresh rate.
+
+ (WebKit::MomentumEventDispatcher::buildOffsetTableWithInitialDelta):
+ Scale the tail frames from the 60Hz ideal rate into the display refresh
+ rate, instead of the event dispatch rate.
+
+ Incoming events still scale *in* from the event dispatch rate, since
+ that's... the rate they come at.
+
+2021-12-13 Alan Coon <alanc...@apple.com>
+
Cherry-pick r286905. rdar://problem/86235740
Scrolling can drop frames when CoreAnimation commits take a long time
Modified: branches/safari-612-branch/Source/WebKit/UIProcess/WebPageProxy.cpp (286990 => 286991)
--- branches/safari-612-branch/Source/WebKit/UIProcess/WebPageProxy.cpp 2021-12-13 23:45:36 UTC (rev 286990)
+++ branches/safari-612-branch/Source/WebKit/UIProcess/WebPageProxy.cpp 2021-12-13 23:49:02 UTC (rev 286991)
@@ -4016,7 +4016,7 @@
if (!hasRunningProcess())
return;
- send(Messages::EventDispatcher::PageScreenDidChange(m_webPageID, displayID));
+ send(Messages::EventDispatcher::PageScreenDidChange(m_webPageID, displayID, nominalFramesPerSecond));
send(Messages::WebPage::WindowScreenDidChange(displayID, nominalFramesPerSecond));
#if HAVE(CVDISPLAYLINK)
updateDisplayLinkFrequency();
Modified: branches/safari-612-branch/Source/WebKit/WebProcess/WebPage/EventDispatcher.cpp (286990 => 286991)
--- branches/safari-612-branch/Source/WebKit/WebProcess/WebPage/EventDispatcher.cpp 2021-12-13 23:45:36 UTC (rev 286990)
+++ branches/safari-612-branch/Source/WebKit/WebProcess/WebPage/EventDispatcher.cpp 2021-12-13 23:49:02 UTC (rev 286991)
@@ -330,10 +330,10 @@
}
#endif
-void EventDispatcher::pageScreenDidChange(PageIdentifier pageID, PlatformDisplayID displayID)
+void EventDispatcher::pageScreenDidChange(PageIdentifier pageID, PlatformDisplayID displayID, std::optional<unsigned> nominalFramesPerSecond)
{
#if ENABLE(MOMENTUM_EVENT_DISPATCHER)
- m_momentumEventDispatcher->pageScreenDidChange(pageID, displayID);
+ m_momentumEventDispatcher->pageScreenDidChange(pageID, displayID, nominalFramesPerSecond);
#else
UNUSED_PARAM(pageID);
UNUSED_PARAM(displayID);
Modified: branches/safari-612-branch/Source/WebKit/WebProcess/WebPage/EventDispatcher.h (286990 => 286991)
--- branches/safari-612-branch/Source/WebKit/WebProcess/WebPage/EventDispatcher.h 2021-12-13 23:45:36 UTC (rev 286990)
+++ branches/safari-612-branch/Source/WebKit/WebProcess/WebPage/EventDispatcher.h 2021-12-13 23:49:02 UTC (rev 286991)
@@ -126,7 +126,7 @@
void displayDidRefreshOnScrollingThread(WebCore::PlatformDisplayID);
#endif
- void pageScreenDidChange(WebCore::PageIdentifier, WebCore::PlatformDisplayID);
+ void pageScreenDidChange(WebCore::PageIdentifier, WebCore::PlatformDisplayID, std::optional<unsigned> nominalFramesPerSecond);
Ref<WorkQueue> m_queue;
Modified: branches/safari-612-branch/Source/WebKit/WebProcess/WebPage/EventDispatcher.messages.in (286990 => 286991)
--- branches/safari-612-branch/Source/WebKit/WebProcess/WebPage/EventDispatcher.messages.in 2021-12-13 23:45:36 UTC (rev 286990)
+++ branches/safari-612-branch/Source/WebKit/WebProcess/WebPage/EventDispatcher.messages.in 2021-12-13 23:49:02 UTC (rev 286991)
@@ -36,5 +36,5 @@
#if ENABLE(MOMENTUM_EVENT_DISPATCHER)
SetScrollingAccelerationCurve(WebCore::PageIdentifier pageID, std::optional<WebKit::ScrollingAccelerationCurve> curve)
#endif
- PageScreenDidChange(WebCore::PageIdentifier pageID, uint32_t displayID)
+ PageScreenDidChange(WebCore::PageIdentifier pageID, uint32_t displayID, std::optional<unsigned> nominalFramesPerSecond)
}
Modified: branches/safari-612-branch/Source/WebKit/WebProcess/WebPage/MomentumEventDispatcher.cpp (286990 => 286991)
--- branches/safari-612-branch/Source/WebKit/WebProcess/WebPage/MomentumEventDispatcher.cpp 2021-12-13 23:45:36 UTC (rev 286990)
+++ branches/safari-612-branch/Source/WebKit/WebProcess/WebPage/MomentumEventDispatcher.cpp 2021-12-13 23:49:02 UTC (rev 286991)
@@ -40,7 +40,7 @@
static constexpr Seconds deltaHistoryMaximumAge = 500_ms;
static constexpr Seconds deltaHistoryMaximumInterval = 150_ms;
-static constexpr float idealCurveFrameRate = 60;
+static constexpr WebCore::FramesPerSecond idealCurveFrameRate = 60;
static constexpr Seconds idealCurveFrameInterval = 1_s / idealCurveFrameRate;
MomentumEventDispatcher::MomentumEventDispatcher(EventDispatcher& dispatcher)
@@ -184,6 +184,10 @@
void MomentumEventDispatcher::didStartMomentumPhase(WebCore::PageIdentifier pageIdentifier, const WebWheelEvent& event)
{
+ auto displayProperties = this->displayProperties(pageIdentifier);
+ if (!displayProperties)
+ return;
+
tracePoint(SyntheticMomentumStart);
auto momentumStartInterval = event.ioHIDEventTimestamp() - m_lastEndedEventTimestamp;
@@ -193,6 +197,7 @@
m_currentGesture.initiatingEvent = event;
m_currentGesture.currentOffset = { };
m_currentGesture.startTime = MonotonicTime::now() - momentumStartInterval;
+ m_currentGesture.displayNominalFrameRate = displayProperties->nominalFrameRate;
m_currentGesture.accelerationCurve = [&] () -> std::optional<ScrollingAccelerationCurve> {
auto curveIterator = m_accelerationCurves.find(m_currentGesture.pageIdentifier);
if (curveIterator == m_accelerationCurves.end())
@@ -241,51 +246,54 @@
#endif
}
-WebCore::PlatformDisplayID MomentumEventDispatcher::displayID() const
+std::optional<MomentumEventDispatcher::DisplayProperties> MomentumEventDispatcher::displayProperties(WebCore::PageIdentifier pageIdentifier) const
{
- ASSERT(m_currentGesture.pageIdentifier);
- auto displayIDIterator = m_displayIDs.find(m_currentGesture.pageIdentifier);
- if (displayIDIterator == m_displayIDs.end())
- return { };
- return displayIDIterator->value;
+ ASSERT(pageIdentifier);
+ auto displayPropertiesIterator = m_displayProperties.find(pageIdentifier);
+ if (displayPropertiesIterator == m_displayProperties.end())
+ return std::nullopt;
+ return { displayPropertiesIterator->value };
}
void MomentumEventDispatcher::startDisplayLink()
{
- auto displayID = this->displayID();
- if (!displayID) {
+ auto displayProperties = this->displayProperties(m_currentGesture.pageIdentifier);
+ if (!displayProperties) {
RELEASE_LOG(ScrollAnimations, "MomentumEventDispatcher failed to start display link");
return;
}
// FIXME: Switch down to lower-than-full-speed frame rates for the tail end of the curve.
- WebProcess::singleton().parentProcessConnection()->send(Messages::WebProcessProxy::StartDisplayLink(m_observerID, displayID, WebCore::FullSpeedFramesPerSecond), 0);
+ WebProcess::singleton().parentProcessConnection()->send(Messages::WebProcessProxy::StartDisplayLink(m_observerID, displayProperties->displayID, WebCore::FullSpeedFramesPerSecond), 0);
#if ENABLE(MOMENTUM_EVENT_DISPATCHER_TEMPORARY_LOGGING)
- RELEASE_LOG(ScrollAnimations, "MomentumEventDispatcher starting display link for display %d", displayID);
+ RELEASE_LOG(ScrollAnimations, "MomentumEventDispatcher starting display link for display %d", displayProperties->displayID);
#endif
}
void MomentumEventDispatcher::stopDisplayLink()
{
- auto displayID = this->displayID();
- if (!displayID) {
+ auto displayProperties = this->displayProperties(m_currentGesture.pageIdentifier);
+ if (!displayProperties) {
RELEASE_LOG(ScrollAnimations, "MomentumEventDispatcher failed to stop display link");
return;
}
- WebProcess::singleton().parentProcessConnection()->send(Messages::WebProcessProxy::StopDisplayLink(m_observerID, displayID), 0);
+ WebProcess::singleton().parentProcessConnection()->send(Messages::WebProcessProxy::StopDisplayLink(m_observerID, displayProperties->displayID), 0);
#if ENABLE(MOMENTUM_EVENT_DISPATCHER_TEMPORARY_LOGGING)
- RELEASE_LOG(ScrollAnimations, "MomentumEventDispatcher stopping display link for display %d", displayID);
+ RELEASE_LOG(ScrollAnimations, "MomentumEventDispatcher stopping display link for display %d", displayProperties->displayID);
#endif
}
-void MomentumEventDispatcher::pageScreenDidChange(WebCore::PageIdentifier pageID, WebCore::PlatformDisplayID displayID)
+void MomentumEventDispatcher::pageScreenDidChange(WebCore::PageIdentifier pageID, WebCore::PlatformDisplayID displayID, std::optional<unsigned> nominalFramesPerSecond)
{
bool affectsCurrentGesture = (pageID == m_currentGesture.pageIdentifier);
if (affectsCurrentGesture)
stopDisplayLink();
- m_displayIDs.set(pageID, displayID);
+ DisplayProperties properties;
+ properties.displayID = displayID;
+ properties.nominalFrameRate = nominalFramesPerSecond.value_or(WebCore::FullSpeedFramesPerSecond);
+ m_displayProperties.set(pageID, WTFMove(properties));
if (affectsCurrentGesture)
startDisplayLink();
@@ -319,7 +327,8 @@
if (!m_currentGesture.active)
return;
- if (displayID != this->displayID())
+ auto displayProperties = this->displayProperties(m_currentGesture.pageIdentifier);
+ if (!displayProperties || displayID != displayProperties->displayID)
return;
dispatchSyntheticMomentumEvent(WebWheelEvent::PhaseChanged, consumeDeltaForCurrentTime());
@@ -370,7 +379,9 @@
WebCore::FloatSize accumulatedOffset;
WebCore::FloatSize unacceleratedDelta = initialUnacceleratedDelta;
- float physicalCurveMultiplier = idealCurveFrameRate / m_currentGesture.accelerationCurve->frameRate();
+ // Tail deltas will be dispatched at the screen refresh rate, not the momentum
+ // dispatch rate, so we need to scale from 60Hz into screen refresh rate.
+ float tailCurveMultiplier = static_cast<float>(idealCurveFrameRate) / m_currentGesture.displayNominalFrameRate;
bool inTail = false;
WebCore::FloatSize tailCarry;
@@ -385,7 +396,7 @@
}
if (inTail) {
- auto tailDelta = acceleratedDelta * physicalCurveMultiplier;
+ auto tailDelta = acceleratedDelta * tailCurveMultiplier;
auto deltaWithCarry = tailDelta + tailCarry;
auto quantizedDelta = roundedIntSize(deltaWithCarry);
tailCarry = deltaWithCarry - quantizedDelta;
Modified: branches/safari-612-branch/Source/WebKit/WebProcess/WebPage/MomentumEventDispatcher.h (286990 => 286991)
--- branches/safari-612-branch/Source/WebKit/WebProcess/WebPage/MomentumEventDispatcher.h 2021-12-13 23:45:36 UTC (rev 286990)
+++ branches/safari-612-branch/Source/WebKit/WebProcess/WebPage/MomentumEventDispatcher.h 2021-12-13 23:49:02 UTC (rev 286991)
@@ -42,6 +42,7 @@
namespace WebCore {
struct DisplayUpdate;
+using FramesPerSecond = unsigned;
using PlatformDisplayID = uint32_t;
}
@@ -62,7 +63,7 @@
void displayWasRefreshed(WebCore::PlatformDisplayID, const WebCore::DisplayUpdate&);
- void pageScreenDidChange(WebCore::PageIdentifier, WebCore::PlatformDisplayID);
+ void pageScreenDidChange(WebCore::PageIdentifier, WebCore::PlatformDisplayID, std::optional<unsigned> nominalFramesPerSecond);
private:
void didStartMomentumPhase(WebCore::PageIdentifier, const WebWheelEvent&);
@@ -73,7 +74,11 @@
void startDisplayLink();
void stopDisplayLink();
- WebCore::PlatformDisplayID displayID() const;
+ struct DisplayProperties {
+ WebCore::PlatformDisplayID displayID;
+ WebCore::FramesPerSecond nominalFrameRate;
+ };
+ std::optional<DisplayProperties> displayProperties(WebCore::PageIdentifier) const;
void dispatchSyntheticMomentumEvent(WebWheelEvent::Phase, WebCore::FloatSize delta);
@@ -137,6 +142,8 @@
Seconds tailStartDelay;
unsigned currentTailDeltaIndex { 0 };
+ WebCore::FramesPerSecond displayNominalFrameRate { 0 };
+
#if ENABLE(MOMENTUM_EVENT_DISPATCHER_TEMPORARY_LOGGING)
WebCore::FloatSize accumulatedEventOffset;
bool didLogInitialQueueState { false };
@@ -148,7 +155,8 @@
} m_currentGesture;
DisplayLinkObserverID m_observerID;
- HashMap<WebCore::PageIdentifier, WebCore::PlatformDisplayID> m_displayIDs;
+
+ HashMap<WebCore::PageIdentifier, DisplayProperties> m_displayProperties;
HashMap<WebCore::PageIdentifier, std::optional<ScrollingAccelerationCurve>> m_accelerationCurves;
EventDispatcher& m_dispatcher;
};
