- Revision
- 264856
- Author
- [email protected]
- Date
- 2020-07-24 14:03:23 -0700 (Fri, 24 Jul 2020)
Log Message
steps() timing function on a transform animation triggers a render every frame
https://bugs.webkit.org/show_bug.cgi?id=214712
<rdar://problem/62737868>
Reviewed by Tim Horton.
Source/WebCore:
An animation of transform, with a steps() timing function, would schedule a rendering
update on every frame. This happened because KeyframeEffect::updateAcceleratedActions()
would keep trying, and failing to start the accelerated animation every frame, since
GraphicsLayerCA refuses to run accelerated animations with steps timing functions.
Fix by making m_isRunningAccelerated an Optional<>, which gets a value once we've tried
to run the animation the first time. Also ensure that we re-try if the timing
function changes, via animationDidChangeTimingProperties().
Test: animations/steps-transform-rendering-updates.html
* animation/KeyframeEffect.cpp:
(WebCore::KeyframeEffect::isRunningAcceleratedAnimationForProperty const):
(WebCore::KeyframeEffect::updateAcceleratedActions):
(WebCore::KeyframeEffect::animationDidChangeTimingProperties):
(WebCore::KeyframeEffect::animationWasCanceled):
(WebCore::KeyframeEffect::willChangeRenderer):
(WebCore::KeyframeEffect::animationSuspensionStateDidChange):
(WebCore::KeyframeEffect::applyPendingAcceleratedActions):
* animation/KeyframeEffect.h:
(WebCore::KeyframeEffect::isRunningAccelerated const):
* page/Page.cpp:
(WebCore::Page::startTrackingRenderingUpdates):
(WebCore::Page::renderingUpdateCount const):
(WebCore::Page::updateRendering):
* page/Page.h:
* testing/Internals.cpp:
(WebCore::Internals::startTrackingRenderingUpdates):
(WebCore::Internals::renderingUpdateCount):
* testing/Internals.h:
* testing/Internals.idl:
LayoutTests:
* animations/steps-transform-rendering-updates-expected.txt: Added.
* animations/steps-transform-rendering-updates.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (264855 => 264856)
--- trunk/LayoutTests/ChangeLog 2020-07-24 21:01:59 UTC (rev 264855)
+++ trunk/LayoutTests/ChangeLog 2020-07-24 21:03:23 UTC (rev 264856)
@@ -1,3 +1,14 @@
+2020-07-23 Simon Fraser <[email protected]>
+
+ steps() timing function on a transform animation triggers a render every frame
+ https://bugs.webkit.org/show_bug.cgi?id=214712
+ <rdar://problem/62737868>
+
+ Reviewed by Tim Horton.
+
+ * animations/steps-transform-rendering-updates-expected.txt: Added.
+ * animations/steps-transform-rendering-updates.html: Added.
+
2020-07-24 Mark Lam <[email protected]>
Fix JS bindings code to DECLARE_THROW_SCOPE in the functions that can throw instead of passing a ThrowScope around.
Added: trunk/LayoutTests/animations/steps-transform-rendering-updates-expected.txt (0 => 264856)
--- trunk/LayoutTests/animations/steps-transform-rendering-updates-expected.txt (rev 0)
+++ trunk/LayoutTests/animations/steps-transform-rendering-updates-expected.txt 2020-07-24 21:03:23 UTC (rev 264856)
@@ -0,0 +1,6 @@
+PASS count is 0
+PASS count < 6 is true
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/animations/steps-transform-rendering-updates.html (0 => 264856)
--- trunk/LayoutTests/animations/steps-transform-rendering-updates.html (rev 0)
+++ trunk/LayoutTests/animations/steps-transform-rendering-updates.html 2020-07-24 21:03:23 UTC (rev 264856)
@@ -0,0 +1,51 @@
+<!DOCTYPE HTML>
+<html>
+<head>
+ <script src=""
+ <style>
+ #box {
+ width: 100px;
+ height: 100px;
+ background-color: silver;
+ }
+
+ #box.animating {
+ animation: spinner 0.25s steps(2) 1;
+ }
+
+ @keyframes spinner {
+ from { transform:rotate(0deg); }
+ to { transform:rotate(360deg); }
+ }
+ </style>
+ <script>
+ window.jsTestIsAsync = true;
+
+ let count = 0;
+ window.addEventListener('load', () => {
+ if (!window.internals) {
+ finishJSTest();
+ return;
+ }
+
+ let box = document.getElementById('box');
+ box.addEventListener('animationstart', () => {
+ internals.startTrackingRenderingUpdates();
+ shouldBe('count', '0');
+ }, false);
+ box.addEventListener('animationend', () => {
+ let count = internals.renderingUpdateCount();
+ shouldBeTrue('count < 6');
+ finishJSTest();
+ }, false);
+
+ box.classList.add('animating');
+ }, false);
+ </script>
+</head>
+<body>
+ <div id="box"></div>
+ <div id="console"></div>
+ <script src=""
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (264855 => 264856)
--- trunk/Source/WebCore/ChangeLog 2020-07-24 21:01:59 UTC (rev 264855)
+++ trunk/Source/WebCore/ChangeLog 2020-07-24 21:03:23 UTC (rev 264856)
@@ -1,3 +1,43 @@
+2020-07-23 Simon Fraser <[email protected]>
+
+ steps() timing function on a transform animation triggers a render every frame
+ https://bugs.webkit.org/show_bug.cgi?id=214712
+ <rdar://problem/62737868>
+
+ Reviewed by Tim Horton.
+
+ An animation of transform, with a steps() timing function, would schedule a rendering
+ update on every frame. This happened because KeyframeEffect::updateAcceleratedActions()
+ would keep trying, and failing to start the accelerated animation every frame, since
+ GraphicsLayerCA refuses to run accelerated animations with steps timing functions.
+
+ Fix by making m_isRunningAccelerated a tri-state enum, which gets a new value once we've tried
+ to run the animation the first time. Also ensure that we re-try if the timing
+ function changes, via animationDidChangeTimingProperties().
+
+ Test: animations/steps-transform-rendering-updates.html
+
+ * animation/KeyframeEffect.cpp:
+ (WebCore::KeyframeEffect::isRunningAcceleratedAnimationForProperty const):
+ (WebCore::KeyframeEffect::updateAcceleratedActions):
+ (WebCore::KeyframeEffect::animationDidChangeTimingProperties):
+ (WebCore::KeyframeEffect::animationWasCanceled):
+ (WebCore::KeyframeEffect::willChangeRenderer):
+ (WebCore::KeyframeEffect::animationSuspensionStateDidChange):
+ (WebCore::KeyframeEffect::applyPendingAcceleratedActions):
+ * animation/KeyframeEffect.h:
+ (WebCore::KeyframeEffect::isRunningAccelerated const):
+ * page/Page.cpp:
+ (WebCore::Page::startTrackingRenderingUpdates):
+ (WebCore::Page::renderingUpdateCount const):
+ (WebCore::Page::updateRendering):
+ * page/Page.h:
+ * testing/Internals.cpp:
+ (WebCore::Internals::startTrackingRenderingUpdates):
+ (WebCore::Internals::renderingUpdateCount):
+ * testing/Internals.h:
+ * testing/Internals.idl:
+
2020-07-24 Mark Lam <[email protected]>
Fix JS bindings code to DECLARE_THROW_SCOPE in the functions that can throw instead of passing a ThrowScope around.
Modified: trunk/Source/WebCore/animation/KeyframeEffect.cpp (264855 => 264856)
--- trunk/Source/WebCore/animation/KeyframeEffect.cpp 2020-07-24 21:01:59 UTC (rev 264855)
+++ trunk/Source/WebCore/animation/KeyframeEffect.cpp 2020-07-24 21:03:23 UTC (rev 264856)
@@ -1232,7 +1232,7 @@
bool KeyframeEffect::isRunningAcceleratedAnimationForProperty(CSSPropertyID property) const
{
- return m_isRunningAccelerated && CSSPropertyAnimation::animationOfPropertyIsAccelerated(property) && m_blendingKeyframes.properties().contains(property);
+ return isRunningAccelerated() && CSSPropertyAnimation::animationOfPropertyIsAccelerated(property) && m_blendingKeyframes.properties().contains(property);
}
void KeyframeEffect::invalidate()
@@ -1502,7 +1502,7 @@
// If we're not already running accelerated, the only thing we're interested in is whether we need to start the animation
// which we need to do once we're in the active phase. Otherwise, there's no change in accelerated state to consider.
bool isActive = computedTiming.phase == AnimationEffectPhase::Active;
- if (!m_isRunningAccelerated) {
+ if (m_runningAccelerated == RunningAccelerated::NotStarted) {
if (isActive && animation()->playState() == WebAnimation::PlayState::Running)
addPendingAcceleratedAction(AcceleratedAction::Play);
return;
@@ -1555,27 +1555,31 @@
void KeyframeEffect::animationDidChangeTimingProperties()
{
+ computeSomeKeyframesUseStepsTimingFunction();
+ // The timing function can affect whether the platform can run this as an accelerated animation.
+ m_runningAccelerated = RunningAccelerated::NotStarted;
+
// There is no need to update the animation if we're not playing already. If updating timing
// means we're moving into an active lexicalGlobalObject, we'll pick this up in apply().
- if (m_isRunningAccelerated || isAboutToRunAccelerated())
+ if (isAboutToRunAccelerated())
addPendingAcceleratedAction(AcceleratedAction::UpdateTiming);
}
void KeyframeEffect::animationWasCanceled()
{
- if (m_isRunningAccelerated || isAboutToRunAccelerated())
+ if (isRunningAccelerated() || isAboutToRunAccelerated())
addPendingAcceleratedAction(AcceleratedAction::Stop);
}
void KeyframeEffect::willChangeRenderer()
{
- if (m_isRunningAccelerated || isAboutToRunAccelerated())
+ if (isRunningAccelerated() || isAboutToRunAccelerated())
addPendingAcceleratedAction(AcceleratedAction::Stop);
}
void KeyframeEffect::animationSuspensionStateDidChange(bool animationIsSuspended)
{
- if (m_isRunningAccelerated || isAboutToRunAccelerated())
+ if (isRunningAccelerated() || isAboutToRunAccelerated())
addPendingAcceleratedAction(animationIsSuspended ? AcceleratedAction::Pause : AcceleratedAction::Play);
}
@@ -1595,7 +1599,7 @@
// in which case if we asked for the animation to stop, we can discard the current set of accelerated actions.
if (m_lastRecordedAcceleratedAction == AcceleratedAction::Stop) {
m_pendingAcceleratedActions.clear();
- m_isRunningAccelerated = false;
+ m_runningAccelerated = RunningAccelerated::NotStarted;
}
return;
}
@@ -1606,11 +1610,11 @@
// To simplify the code we use a default of 0s for an unresolved current time since for a Stop action that is acceptable.
auto timeOffset = animation()->currentTime().valueOr(0_s).seconds() - delay().seconds();
- auto startAnimation = [&]() -> bool {
+ auto startAnimation = [&]() -> RunningAccelerated {
renderer->animationFinished(m_blendingKeyframes.animationName());
if (!m_blendingKeyframes.hasImplicitKeyframes())
- return renderer->startAnimation(timeOffset, backingAnimationForCompositedRenderer(), m_blendingKeyframes);
+ return renderer->startAnimation(timeOffset, backingAnimationForCompositedRenderer(), m_blendingKeyframes) ? RunningAccelerated::Yes : RunningAccelerated::No;
ASSERT(m_target);
@@ -1620,14 +1624,15 @@
KeyframeList explicitKeyframes(m_blendingKeyframes.animationName());
explicitKeyframes.copyKeyframes(m_blendingKeyframes);
explicitKeyframes.fillImplicitKeyframes(*m_target, m_target->styleResolver(), lastStyleChangeEventStyle);
- return renderer->startAnimation(timeOffset, backingAnimationForCompositedRenderer(), explicitKeyframes);
+ return renderer->startAnimation(timeOffset, backingAnimationForCompositedRenderer(), explicitKeyframes) ? RunningAccelerated::Yes : RunningAccelerated::No;
};
for (const auto& action : pendingAcceleratedActions) {
switch (action) {
case AcceleratedAction::Play:
- m_isRunningAccelerated = startAnimation();
- if (!m_isRunningAccelerated) {
+ m_runningAccelerated = startAnimation();
+ LOG_WITH_STREAM(Animations, stream << "KeyframeEffect " << this << " applyPendingAcceleratedActions " << m_blendingKeyframes.animationName() << " Play, started accelerated: " << (m_runningAccelerated == RunningAccelerated::Yes));
+ if (m_runningAccelerated == RunningAccelerated::No) {
m_lastRecordedAcceleratedAction = AcceleratedAction::Stop;
return;
}
@@ -1636,7 +1641,8 @@
renderer->animationPaused(timeOffset, m_blendingKeyframes.animationName());
break;
case AcceleratedAction::UpdateTiming:
- startAnimation();
+ m_runningAccelerated = startAnimation();
+ LOG_WITH_STREAM(Animations, stream << "KeyframeEffect " << this << " applyPendingAcceleratedActions " << m_blendingKeyframes.animationName() << " UpdateTiming, started accelerated: " << (m_runningAccelerated == RunningAccelerated::Yes));
if (animation()->playState() == WebAnimation::PlayState::Paused)
renderer->animationPaused(timeOffset, m_blendingKeyframes.animationName());
break;
@@ -1645,7 +1651,7 @@
renderer->animationFinished(m_blendingKeyframes.animationName());
if (!document()->renderTreeBeingDestroyed())
m_target->invalidateStyleAndLayerComposition();
- m_isRunningAccelerated = false;
+ m_runningAccelerated = RunningAccelerated::NotStarted;
break;
}
}
Modified: trunk/Source/WebCore/animation/KeyframeEffect.h (264855 => 264856)
--- trunk/Source/WebCore/animation/KeyframeEffect.h 2020-07-24 21:01:59 UTC (rev 264855)
+++ trunk/Source/WebCore/animation/KeyframeEffect.h 2020-07-24 21:03:23 UTC (rev 264856)
@@ -138,9 +138,12 @@
RenderElement* renderer() const override;
const RenderStyle& currentStyle() const override;
bool triggersStackingContext() const { return m_triggersStackingContext; }
- bool isRunningAccelerated() const { return m_isRunningAccelerated; }
+ bool isRunningAccelerated() const { return m_runningAccelerated == RunningAccelerated::Yes; }
+
+ // FIXME: These ignore the fact that some timing functions can prevent acceleration.
bool isAboutToRunAccelerated() const { return m_acceleratedPropertiesState != AcceleratedProperties::None && m_lastRecordedAcceleratedAction != AcceleratedAction::Stop; }
bool isCompletelyAccelerated() const { return m_acceleratedPropertiesState == AcceleratedProperties::All; }
+
bool filterFunctionListsMatch() const override { return m_filterFunctionListsMatch; }
bool transformFunctionListsMatch() const override { return m_transformFunctionListsMatch; }
#if ENABLE(FILTERS_LEVEL_2)
@@ -170,6 +173,7 @@
enum class AcceleratedAction : uint8_t { Play, Pause, UpdateTiming, Stop };
enum class BlendingKeyframesSource : uint8_t { CSSAnimation, CSSTransition, WebAnimation };
enum class AcceleratedProperties : uint8_t { None, Some, All };
+ enum class RunningAccelerated : uint8_t { NotStarted, Yes, No };
Document* document() const;
void updateEffectStackMembership();
@@ -211,7 +215,7 @@
CompositeOperation m_compositeOperation { CompositeOperation::Replace };
AcceleratedProperties m_acceleratedPropertiesState { AcceleratedProperties::None };
AnimationEffectPhase m_phaseAtLastApplication { AnimationEffectPhase::Idle };
- bool m_isRunningAccelerated { false };
+ RunningAccelerated m_runningAccelerated;
bool m_needsForcedLayout { false };
bool m_triggersStackingContext { false };
bool m_transformFunctionListsMatch { false };
Modified: trunk/Source/WebCore/page/Page.cpp (264855 => 264856)
--- trunk/Source/WebCore/page/Page.cpp 2020-07-24 21:01:59 UTC (rev 264855)
+++ trunk/Source/WebCore/page/Page.cpp 2020-07-24 21:03:23 UTC (rev 264856)
@@ -1415,6 +1415,17 @@
renderingUpdateScheduler().scheduleTimedRenderingUpdate();
}
+void Page::startTrackingRenderingUpdates()
+{
+ m_isTrackingRenderingUpdates = true;
+ m_renderingUpdateCount = 0;
+}
+
+unsigned Page::renderingUpdateCount() const
+{
+ return m_renderingUpdateCount;
+}
+
// https://html.spec.whatwg.org/multipage/webappapis.html#update-the-rendering
void Page::updateRendering()
{
@@ -1493,6 +1504,9 @@
if (document && document->domWindow())
document->domWindow()->unfreezeNowTimestamp();
}
+
+ if (m_isTrackingRenderingUpdates)
+ ++m_renderingUpdateCount;
layoutIfNeeded();
doAfterUpdateRendering();
Modified: trunk/Source/WebCore/page/Page.h (264855 => 264856)
--- trunk/Source/WebCore/page/Page.h 2020-07-24 21:01:59 UTC (rev 264855)
+++ trunk/Source/WebCore/page/Page.h 2020-07-24 21:03:23 UTC (rev 264856)
@@ -493,6 +493,9 @@
WEBCORE_EXPORT void scheduleRenderingUpdate();
void scheduleTimedRenderingUpdate();
+
+ WEBCORE_EXPORT void startTrackingRenderingUpdates();
+ WEBCORE_EXPORT unsigned renderingUpdateCount() const;
WEBCORE_EXPORT void suspendScriptedAnimations();
WEBCORE_EXPORT void resumeScriptedAnimations();
@@ -983,6 +986,9 @@
PAL::SessionID m_sessionID;
+ unsigned m_renderingUpdateCount { 0 };
+ bool m_isTrackingRenderingUpdates { false };
+
bool m_isClosing { false };
bool m_isRestoringCachedPage { false };
Modified: trunk/Source/WebCore/testing/Internals.cpp (264855 => 264856)
--- trunk/Source/WebCore/testing/Internals.cpp 2020-07-24 21:01:59 UTC (rev 264855)
+++ trunk/Source/WebCore/testing/Internals.cpp 2020-07-24 21:03:23 UTC (rev 264856)
@@ -3542,6 +3542,25 @@
return document->renderView()->compositor().compositingUpdateCount();
}
+ExceptionOr<void> Internals::startTrackingRenderingUpdates()
+{
+ Document* document = contextDocument();
+ if (!document)
+ return Exception { InvalidAccessError };
+
+ document->page()->startTrackingRenderingUpdates();
+ return { };
+}
+
+ExceptionOr<unsigned> Internals::renderingUpdateCount()
+{
+ Document* document = contextDocument();
+ if (!document)
+ return Exception { InvalidAccessError };
+
+ return document->page()->renderingUpdateCount();
+}
+
ExceptionOr<void> Internals::setCompositingPolicyOverride(Optional<CompositingPolicy> policyOverride)
{
Document* document = contextDocument();
Modified: trunk/Source/WebCore/testing/Internals.h (264855 => 264856)
--- trunk/Source/WebCore/testing/Internals.h 2020-07-24 21:01:59 UTC (rev 264855)
+++ trunk/Source/WebCore/testing/Internals.h 2020-07-24 21:03:23 UTC (rev 264856)
@@ -541,6 +541,9 @@
ExceptionOr<void> startTrackingCompositingUpdates();
ExceptionOr<unsigned> compositingUpdateCount();
+ ExceptionOr<void> startTrackingRenderingUpdates();
+ ExceptionOr<unsigned> renderingUpdateCount();
+
enum CompositingPolicy { Normal, Conservative };
ExceptionOr<void> setCompositingPolicyOverride(Optional<CompositingPolicy>);
ExceptionOr<Optional<CompositingPolicy>> compositingPolicyOverride() const;
Modified: trunk/Source/WebCore/testing/Internals.idl (264855 => 264856)
--- trunk/Source/WebCore/testing/Internals.idl 2020-07-24 21:01:59 UTC (rev 264855)
+++ trunk/Source/WebCore/testing/Internals.idl 2020-07-24 21:03:23 UTC (rev 264856)
@@ -604,6 +604,9 @@
[MayThrowException] void startTrackingCompositingUpdates();
[MayThrowException] unsigned long compositingUpdateCount();
+ [MayThrowException] void startTrackingRenderingUpdates();
+ [MayThrowException] unsigned long renderingUpdateCount();
+
attribute CompositingPolicy? compositingPolicyOverride;
void updateLayoutAndStyleForAllFrames();
