Diff
Modified: trunk/Source/WebKit/ChangeLog (286482 => 286483)
--- trunk/Source/WebKit/ChangeLog 2021-12-03 06:39:14 UTC (rev 286482)
+++ trunk/Source/WebKit/ChangeLog 2021-12-03 07:25:20 UTC (rev 286483)
@@ -1,3 +1,66 @@
+2021-12-02 Tim Horton <timothy_hor...@apple.com>
+
+ MomentumEventDispatcher curve sometimes doesn't match the system curve
+ https://bugs.webkit.org/show_bug.cgi?id=233801
+ <rdar://problem/85870287>
+
+ Reviewed by Simon Fraser.
+
+ Three small changes to get us closer to the system curve:
+
+ 1) Fetch the momentum event dispatch interval from the system, and use
+ it to scale the momentum start event's delta (which we use as our
+ initial velocity) into the "ideal" curve's frame rate (which is always
+ 60fps regardless).
+
+ 2) Back-date the animation start time to the fingers-down phase end event,
+ so that the momentum start phase has a delta. This seems to match what
+ the system does.
+
+ 3) Switch to MonotonicTime for the animation time, since it doesn't need
+ to be in the same timebase as events (which are oddly in WallTime), and
+ certainly should be monotonic.
+
+ * Shared/ScrollingAccelerationCurve.cpp:
+ (WebKit::ScrollingAccelerationCurve::ScrollingAccelerationCurve):
+ (WebKit::ScrollingAccelerationCurve::interpolate):
+ (WebKit::ScrollingAccelerationCurve::encode const):
+ (WebKit::ScrollingAccelerationCurve::decode):
+ (WebKit::operator<<):
+ * Shared/ScrollingAccelerationCurve.h:
+ (WebKit::ScrollingAccelerationCurve::frameRate):
+ (WebKit::ScrollingAccelerationCurve::operator== const):
+ * Shared/mac/ScrollingAccelerationCurveMac.mm:
+ (WebKit::fromIOHIDCurve):
+ (WebKit::fromIOHIDCurveArrayWithAcceleration):
+ (WebKit::fromIOHIDDevice):
+ Fetch and propagate the dispatch frame rate. This isn't *really* a
+ ScrollingAccelerationCurve property, but neither is `resolution`;
+ this is just currently the only place we look up HID properties
+ and push them to the Web Content process; some re-architecture is
+ warranted here in the future.
+
+ * WebProcess/WebPage/MomentumEventDispatcher.cpp:
+ (WebKit::MomentumEventDispatcher::handleWheelEvent):
+ Keep track of the last fingers-down phase ended event timestamp.
+
+ (WebKit::MomentumEventDispatcher::didStartMomentumPhase):
+ Backdate the start time so that the first event has the appropriate delta.
+ Divide out the dispatch frame rate so that the initial velocity is
+ as if the curve were running at 60fps (since we *will* run it at 60fps and interpolate).
+
+ (WebKit::MomentumEventDispatcher::didEndMomentumPhase):
+ (WebKit::MomentumEventDispatcher::consumeDeltaForTime):
+ (WebKit::MomentumEventDispatcher::displayWasRefreshed):
+ Factor the consume-a-delta-from-the-ideal-curve code out from displayWasRefreshed
+ so we can use it in the start phase too.
+
+ (WebKit::MomentumEventDispatcher::offsetAtTime):
+ (WebKit::MomentumEventDispatcher::computeNextDelta):
+ Rename idealCurveFrameRate->idealCurveFrameInterval for accuracy.
+
+ * WebProcess/WebPage/MomentumEventDispatcher.h:
+
2021-12-02 Simon Fraser <simon.fra...@apple.com>
A Safari tab can rarely get stuck in a state where rendering updates stop happening
Modified: trunk/Source/WebKit/Shared/ScrollingAccelerationCurve.cpp (286482 => 286483)
--- trunk/Source/WebKit/Shared/ScrollingAccelerationCurve.cpp 2021-12-03 06:39:14 UTC (rev 286482)
+++ trunk/Source/WebKit/Shared/ScrollingAccelerationCurve.cpp 2021-12-03 07:25:20 UTC (rev 286483)
@@ -34,8 +34,8 @@
namespace WebKit {
-ScrollingAccelerationCurve::ScrollingAccelerationCurve(float gainLinear, float gainParabolic, float gainCubic, float gainQuartic, float tangentSpeedLinear, float tangentSpeedParabolicRoot, float resolution)
- : m_parameters { gainLinear, gainParabolic, gainCubic, gainQuartic, tangentSpeedLinear, tangentSpeedParabolicRoot, resolution }
+ScrollingAccelerationCurve::ScrollingAccelerationCurve(float gainLinear, float gainParabolic, float gainCubic, float gainQuartic, float tangentSpeedLinear, float tangentSpeedParabolicRoot, float resolution, float frameRate)
+ : m_parameters { gainLinear, gainParabolic, gainCubic, gainQuartic, tangentSpeedLinear, tangentSpeedParabolicRoot, resolution, frameRate }
{
}
@@ -53,7 +53,7 @@
auto tangentSpeedLinear = interpolate(from.m_parameters.tangentSpeedLinear, to.m_parameters.tangentSpeedLinear);
auto tangentSpeedParabolicRoot = interpolate(from.m_parameters.tangentSpeedParabolicRoot, to.m_parameters.tangentSpeedParabolicRoot);
- return { gainLinear, gainParabolic, gainCubic, gainQuartic, tangentSpeedLinear, tangentSpeedParabolicRoot, from.m_parameters.resolution };
+ return { gainLinear, gainParabolic, gainCubic, gainQuartic, tangentSpeedLinear, tangentSpeedParabolicRoot, from.m_parameters.resolution, from.m_parameters.frameRate };
}
void ScrollingAccelerationCurve::computeIntermediateValuesIfNeeded()
@@ -127,6 +127,7 @@
encoder << m_parameters.tangentSpeedParabolicRoot;
encoder << m_parameters.resolution;
+ encoder << m_parameters.frameRate;
}
std::optional<ScrollingAccelerationCurve> ScrollingAccelerationCurve::decode(IPC::Decoder& decoder)
@@ -154,8 +155,11 @@
float resolution;
if (!decoder.decode(resolution))
return std::nullopt;
+ float frameRate;
+ if (!decoder.decode(frameRate))
+ return std::nullopt;
- return { { gainLinear, gainParabolic, gainCubic, gainQuartic, tangentSpeedLinear, tangentSpeedParabolicRoot, resolution } };
+ return { { gainLinear, gainParabolic, gainCubic, gainQuartic, tangentSpeedLinear, tangentSpeedParabolicRoot, resolution, frameRate } };
}
TextStream& operator<<(TextStream& ts, const ScrollingAccelerationCurve& curve)
@@ -171,6 +175,7 @@
ts.dumpProperty("tangentSpeedLinear", curve.m_parameters.tangentSpeedLinear);
ts.dumpProperty("tangentSpeedParabolicRoot", curve.m_parameters.tangentSpeedParabolicRoot);
ts.dumpProperty("resolution", curve.m_parameters.resolution);
+ ts.dumpProperty("frameRate", curve.m_parameters.frameRate);
return ts;
}
Modified: trunk/Source/WebKit/Shared/ScrollingAccelerationCurve.h (286482 => 286483)
--- trunk/Source/WebKit/Shared/ScrollingAccelerationCurve.h 2021-12-03 06:39:14 UTC (rev 286482)
+++ trunk/Source/WebKit/Shared/ScrollingAccelerationCurve.h 2021-12-03 07:25:20 UTC (rev 286483)
@@ -41,7 +41,7 @@
class ScrollingAccelerationCurve {
public:
- ScrollingAccelerationCurve(float gainLinear, float gainParabolic, float gainCubic, float gainQuartic, float tangentSpeedLinear, float tangentSpeedParabolicRoot, float resolution);
+ ScrollingAccelerationCurve(float gainLinear, float gainParabolic, float gainCubic, float gainQuartic, float tangentSpeedLinear, float tangentSpeedParabolicRoot, float resolution, float frameRate);
static std::optional<ScrollingAccelerationCurve> fromNativeWheelEvent(const NativeWebWheelEvent&);
@@ -48,6 +48,7 @@
static ScrollingAccelerationCurve interpolate(const ScrollingAccelerationCurve& from, const ScrollingAccelerationCurve& to, float amount);
float accelerationFactor(float);
+ float frameRate() const { return m_parameters.frameRate; }
void encode(IPC::Encoder&) const;
static std::optional<ScrollingAccelerationCurve> decode(IPC::Decoder&);
@@ -60,7 +61,8 @@
&& m_parameters.gainQuartic == other.m_parameters.gainQuartic
&& m_parameters.tangentSpeedLinear == other.m_parameters.tangentSpeedLinear
&& m_parameters.tangentSpeedParabolicRoot == other.m_parameters.tangentSpeedParabolicRoot
- && m_parameters.resolution == other.m_parameters.resolution;
+ && m_parameters.resolution == other.m_parameters.resolution
+ && m_parameters.frameRate == other.m_parameters.frameRate;
}
bool operator!=(const ScrollingAccelerationCurve& other) const { return !(*this == other); }
@@ -79,7 +81,11 @@
float gainQuartic { 0 };
float tangentSpeedLinear { 0 };
float tangentSpeedParabolicRoot { 0 };
+
+ // FIXME: Resolution and frame rate are not technically properties
+ // of the curve, just required to use it; they should be plumbed separately.
float resolution { 0 };
+ float frameRate { 0 };
} m_parameters;
struct ComputedIntermediateValues {
Modified: trunk/Source/WebKit/Shared/mac/ScrollingAccelerationCurveMac.mm (286482 => 286483)
--- trunk/Source/WebKit/Shared/mac/ScrollingAccelerationCurveMac.mm 2021-12-03 06:39:14 UTC (rev 286482)
+++ trunk/Source/WebKit/Shared/mac/ScrollingAccelerationCurveMac.mm 2021-12-03 07:25:20 UTC (rev 286483)
@@ -39,12 +39,19 @@
return value / 65536.0f;
}
+static float fromCFNumber(CFNumberRef number)
+{
+ float value;
+ CFNumberGetValue(number, kCFNumberFloatType, &value);
+ return value;
+}
+
static float readFixedPointParameter(NSDictionary *parameters, const char *key)
{
return fromFixedPoint([[parameters objectForKey:@(key)] floatValue]);
}
-static ScrollingAccelerationCurve fromIOHIDCurve(NSDictionary *parameters, float resolution)
+static ScrollingAccelerationCurve fromIOHIDCurve(NSDictionary *parameters, float resolution, float frameRate)
{
auto gainLinear = readFixedPointParameter(parameters, kHIDAccelGainLinearKey);
auto gainParabolic = readFixedPointParameter(parameters, kHIDAccelGainParabolicKey);
@@ -54,10 +61,10 @@
auto tangentSpeedLinear = readFixedPointParameter(parameters, kHIDAccelTangentSpeedLinearKey);
auto tangentSpeedParabolicRoot = readFixedPointParameter(parameters, kHIDAccelTangentSpeedParabolicRootKey);
- return { gainLinear, gainParabolic, gainCubic, gainQuartic, tangentSpeedLinear, tangentSpeedParabolicRoot, resolution };
+ return { gainLinear, gainParabolic, gainCubic, gainQuartic, tangentSpeedLinear, tangentSpeedParabolicRoot, resolution, frameRate };
}
-static ScrollingAccelerationCurve fromIOHIDCurveArrayWithAcceleration(NSArray<NSDictionary *> *ioHIDCurves, float desiredAcceleration, float resolution)
+static ScrollingAccelerationCurve fromIOHIDCurveArrayWithAcceleration(NSArray<NSDictionary *> *ioHIDCurves, float desiredAcceleration, float resolution, float frameRate)
{
__block size_t currentIndex = 0;
__block Vector<std::pair<float, ScrollingAccelerationCurve>> curves;
@@ -64,7 +71,7 @@
[ioHIDCurves enumerateObjectsUsingBlock:^(NSDictionary *parameters, NSUInteger i, BOOL *) {
auto curveAcceleration = readFixedPointParameter(parameters, kHIDAccelIndexKey);
- auto curve = fromIOHIDCurve(parameters, resolution);
+ auto curve = fromIOHIDCurve(parameters, resolution, frameRate);
if (desiredAcceleration > curveAcceleration)
currentIndex = i;
@@ -116,19 +123,26 @@
return std::nullopt;
}
- auto scrollAcceleration = adoptCF(dynamic_cf_cast<CFNumberRef>(IOHIDServiceClientCopyProperty(ioHIDService.get(), scrollAccelerationType.get())));
- if (!scrollAcceleration) {
+ auto scrollAccelerationCF = adoptCF(dynamic_cf_cast<CFNumberRef>(IOHIDServiceClientCopyProperty(ioHIDService.get(), scrollAccelerationType.get())));
+ if (!scrollAccelerationCF) {
RELEASE_LOG(ScrollAnimations, "ScrollingAccelerationCurve::fromIOHIDDevice failed to look up acceleration value");
return std::nullopt;
}
+ auto scrollAcceleration = fromFixedPoint(fromCFNumber(scrollAccelerationCF.get()));
- auto resolution = adoptCF(dynamic_cf_cast<CFNumberRef>(IOHIDServiceClientCopyProperty(ioHIDService.get(), CFSTR(kIOHIDScrollResolutionKey))));
- if (!resolution) {
+ auto resolutionCF = adoptCF(dynamic_cf_cast<CFNumberRef>(IOHIDServiceClientCopyProperty(ioHIDService.get(), CFSTR(kIOHIDScrollResolutionKey))));
+ if (!resolutionCF) {
RELEASE_LOG(ScrollAnimations, "ScrollingAccelerationCurve::fromIOHIDDevice failed to look up resolution");
return std::nullopt;
}
+ auto resolution = fromFixedPoint([(NSNumber *)resolutionCF.get() floatValue]);
- return fromIOHIDCurveArrayWithAcceleration((NSArray *)curves.get(), fromFixedPoint([(NSNumber *)scrollAcceleration.get() floatValue]), fromFixedPoint([(NSNumber *)resolution.get() floatValue]));
+ static CFStringRef dispatchFrameRateKey = CFSTR("ScrollMomentumDispatchRate");
+ static constexpr float defaultDispatchFrameRate = 60;
+ auto frameRateCF = adoptCF(dynamic_cf_cast<CFNumberRef>(IOHIDServiceClientCopyProperty(ioHIDService.get(), dispatchFrameRateKey)));
+ float frameRate = frameRateCF ? fromCFNumber(frameRateCF.get()) : defaultDispatchFrameRate;
+
+ return fromIOHIDCurveArrayWithAcceleration((NSArray *)curves.get(), scrollAcceleration, resolution, frameRate);
}
std::optional<ScrollingAccelerationCurve> ScrollingAccelerationCurve::fromNativeWheelEvent(const NativeWebWheelEvent& nativeWebWheelEvent)
Modified: trunk/Source/WebKit/WebProcess/WebPage/MomentumEventDispatcher.cpp (286482 => 286483)
--- trunk/Source/WebKit/WebProcess/WebPage/MomentumEventDispatcher.cpp 2021-12-03 06:39:14 UTC (rev 286482)
+++ trunk/Source/WebKit/WebProcess/WebPage/MomentumEventDispatcher.cpp 2021-12-03 07:25:20 UTC (rev 286483)
@@ -39,7 +39,8 @@
static constexpr Seconds deltaHistoryMaximumAge = 500_ms;
static constexpr Seconds deltaHistoryMaximumInterval = 150_ms;
-static constexpr Seconds idealCurveFrameRate = 1_s / 60;
+static constexpr float idealCurveFrameRate = 60;
+static constexpr Seconds idealCurveFrameInterval = 1_s / idealCurveFrameRate;
MomentumEventDispatcher::MomentumEventDispatcher(EventDispatcher& dispatcher)
: m_observerID(DisplayLinkObserverID::generate())
@@ -96,6 +97,9 @@
m_lastActivePhaseDelta = *lastActivePhaseDelta;
}
+ if (event.phase() == WebWheelEvent::PhaseEnded)
+ m_lastEndedEventTimestamp = event.ioHIDEventTimestamp();
+
if (eventShouldStartSyntheticMomentumPhase(pageIdentifier, event))
didStartMomentumPhase(pageIdentifier, event);
@@ -159,11 +163,13 @@
void MomentumEventDispatcher::didStartMomentumPhase(WebCore::PageIdentifier pageIdentifier, const WebWheelEvent& event)
{
+ auto momentumStartInterval = event.ioHIDEventTimestamp() - m_lastEndedEventTimestamp;
+
m_currentGesture.active = true;
m_currentGesture.pageIdentifier = pageIdentifier;
m_currentGesture.initiatingEvent = event;
m_currentGesture.currentOffset = { };
- m_currentGesture.startTime = WallTime::now();
+ m_currentGesture.startTime = MonotonicTime::now() - momentumStartInterval;
m_currentGesture.accelerationCurve = [&] () -> std::optional<ScrollingAccelerationCurve> {
auto curveIterator = m_accelerationCurves.find(m_currentGesture.pageIdentifier);
if (curveIterator == m_accelerationCurves.end())
@@ -176,9 +182,10 @@
// FIXME: The system falls back from the table to just generating deltas
// directly when the frame interval is within 20fps of idealCurveFrameRate;
// we should perhaps do the same.
- buildOffsetTableWithInitialDelta(*event.rawPlatformDelta());
+ float idealCurveMultiplier = m_currentGesture.accelerationCurve->frameRate() / idealCurveFrameRate;
+ buildOffsetTableWithInitialDelta(*event.rawPlatformDelta() * idealCurveMultiplier);
- dispatchSyntheticMomentumEvent(WebWheelEvent::PhaseBegan, { });
+ dispatchSyntheticMomentumEvent(WebWheelEvent::PhaseBegan, consumeDeltaForCurrentTime());
}
void MomentumEventDispatcher::didEndMomentumPhase()
@@ -187,7 +194,7 @@
dispatchSyntheticMomentumEvent(WebWheelEvent::PhaseEnded, { });
- 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(), (WallTime::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)", m_currentGesture.currentOffset.width(), m_currentGesture.currentOffset.height(), (MonotonicTime::now() - m_currentGesture.startTime).seconds(), m_currentGesture.accumulatedEventOffset.width(), m_currentGesture.accumulatedEventOffset.height());
stopDisplayLink();
@@ -251,15 +258,9 @@
startDisplayLink();
}
-void MomentumEventDispatcher::displayWasRefreshed(WebCore::PlatformDisplayID displayID, const WebCore::DisplayUpdate&)
+WebCore::FloatSize MomentumEventDispatcher::consumeDeltaForCurrentTime()
{
- if (!m_currentGesture.active)
- return;
-
- if (displayID != this->displayID())
- return;
-
- auto animationTime = WallTime::now() - m_currentGesture.startTime;
+ auto animationTime = MonotonicTime::now() - m_currentGesture.startTime;
auto desiredOffset = offsetAtTime(animationTime);
#if !USE(MOMENTUM_EVENT_DISPATCHER_PREMATURE_ROUNDING)
@@ -269,11 +270,22 @@
WebCore::FloatSize delta = desiredOffset - m_currentGesture.currentOffset;
#endif
- dispatchSyntheticMomentumEvent(WebWheelEvent::PhaseChanged, -delta);
+ m_currentGesture.currentOffset += delta;
- m_currentGesture.currentOffset += delta;
+ return -delta;
}
+void MomentumEventDispatcher::displayWasRefreshed(WebCore::PlatformDisplayID displayID, const WebCore::DisplayUpdate&)
+{
+ if (!m_currentGesture.active)
+ return;
+
+ if (displayID != this->displayID())
+ return;
+
+ dispatchSyntheticMomentumEvent(WebWheelEvent::PhaseChanged, consumeDeltaForCurrentTime());
+}
+
void MomentumEventDispatcher::didReceiveScrollEventWithInterval(WebCore::FloatSize size, Seconds frameInterval)
{
auto push = [](HistoricalDeltas& deltas, Delta newDelta) {
@@ -343,7 +355,7 @@
if (!m_currentGesture.offsetTable.size())
return { };
- float fractionalFrameNumber = time.seconds() / idealCurveFrameRate.seconds();
+ float fractionalFrameNumber = time.seconds() / idealCurveFrameInterval.seconds();
unsigned long lowerFrameNumber = std::min<unsigned long>(m_currentGesture.offsetTable.size() - 1, floor(fractionalFrameNumber));
unsigned long upperFrameNumber = std::min<unsigned long>(m_currentGesture.offsetTable.size() - 1, lowerFrameNumber + 1);
float amount = fractionalFrameNumber - lowerFrameNumber;
@@ -379,7 +391,7 @@
{
WebCore::FloatSize unacceleratedDelta = currentUnacceleratedDelta;
- float decayRate = momentumDecayRate(unacceleratedDelta, idealCurveFrameRate);
+ float decayRate = momentumDecayRate(unacceleratedDelta, idealCurveFrameInterval);
unacceleratedDelta.scale(decayRate);
auto quantizedUnacceleratedDelta = unacceleratedDelta;
@@ -403,7 +415,7 @@
#endif
// The delta queue operates on pre-acceleration deltas, so insert the new event *before* accelerating.
- didReceiveScrollEventWithInterval(quantizedUnacceleratedDelta, idealCurveFrameRate);
+ didReceiveScrollEventWithInterval(quantizedUnacceleratedDelta, idealCurveFrameInterval);
auto accelerateAxis = [&] (HistoricalDeltas& deltas, float value) {
float totalDelta = 0;
Modified: trunk/Source/WebKit/WebProcess/WebPage/MomentumEventDispatcher.h (286482 => 286483)
--- trunk/Source/WebKit/WebProcess/WebPage/MomentumEventDispatcher.h 2021-12-03 06:39:14 UTC (rev 286482)
+++ trunk/Source/WebKit/WebProcess/WebPage/MomentumEventDispatcher.h 2021-12-03 07:25:20 UTC (rev 286483)
@@ -39,6 +39,7 @@
#include <WebCore/RectEdges.h>
#include <memory>
#include <wtf/Deque.h>
+#include <wtf/MonotonicTime.h>
#include <wtf/Noncopyable.h>
namespace WebCore {
@@ -80,6 +81,9 @@
void buildOffsetTableWithInitialDelta(WebCore::FloatSize);
+ // Once consumed, this delta *must* be dispatched in an event.
+ WebCore::FloatSize consumeDeltaForCurrentTime();
+
WebCore::FloatSize offsetAtTime(Seconds);
std::pair<WebCore::FloatSize, WebCore::FloatSize> computeNextDelta(WebCore::FloatSize currentUnacceleratedDelta);
@@ -96,6 +100,7 @@
HistoricalDeltas m_deltaHistoryY;
std::optional<WallTime> m_lastScrollTimestamp;
+ WallTime m_lastEndedEventTimestamp;
std::optional<WebWheelEvent> m_lastIncomingEvent;
WebCore::RectEdges<bool> m_lastRubberBandableEdges;
#if !RELEASE_LOG_DISABLED
@@ -110,7 +115,7 @@
std::optional<WebWheelEvent> initiatingEvent;
WebCore::FloatSize currentOffset;
- WallTime startTime;
+ MonotonicTime startTime;
Vector<WebCore::FloatSize> offsetTable;
