- Revision
- 289211
- Author
- [email protected]
- Date
- 2022-02-07 05:57:38 -0800 (Mon, 07 Feb 2022)
Log Message
[Web Animations] Starting a transform animation with a 1ms delay doesn't run it accelerated
https://bugs.webkit.org/show_bug.cgi?id=236080
<rdar://problem/88432373>
Reviewed by Dean Jackson.
Source/WebCore:
Accelerated animations can only be started if the keyframe effect's renderer is composited.
Keyframe effects enqueue a list of accelerated actions (play, pause, seek, stop, etc.) when
they can be accelerated, and these actions are performed in sync as the document timeline
is finishing up its "update animations and send events" procedure as we update the page
rendering.
When an animation is started _without_ a delay, the machinery to consider making a renderer
composited happens _before_ we update animations and send events, and since at this stage the
new animation's effect is already in its target's effect stack because it is immediately
"relevant" (per the Web Animations terminology) and targets a property that can be accelerated,
its renderer is indeed composited.
Thus by the time we consider starting this animation's effect with acceleration, it is in a
condition to do so since its renderer is already composited.
However, when an animation is started _with_ a delay, it is not immediately "relevant" and thus
won't appear in its target's effect stack. That will happen once we run the "update animations
and send events procedure" the next time we update the page rendering. But before that, the
effect's renderer will be considered to be made composited and _will not_ be because at this
point there is no effect animating a property that can be accelerated.
As we eventually run the "update animations and send events" procedure, we'll enqueue an
accelerated action to play the animation now that it became relevant, and when that procedure
completes and we try to apply that accelerated action, we'll fail to start an accelerated
animation since the renderer is not composited.
To address this, we only enqueue accelerated actions when the renderer is composited, thus
matching the condition to actually be able to apply this action.
This means we no longer need the animationDidPlay() on effects since we wouldn't be able to
honor enqueuing a "play" accelerated action since the renderer won't be composited yet at this
time in the vast majority of cases.
Test: webanimations/transform-animation-with-delay-yields-accelerated-animation.html
* animation/AnimationEffect.h:
(WebCore::AnimationEffect::animationDidTick):
(WebCore::AnimationEffect::animationDidPlay): Deleted.
* animation/KeyframeEffect.cpp:
(WebCore::KeyframeEffect::updateAcceleratedActions):
(WebCore::KeyframeEffect::animationDidPlay): Deleted.
* animation/KeyframeEffect.h:
* animation/WebAnimation.cpp:
(WebCore::WebAnimation::play):
* animation/AnimationEffect.h:
(WebCore::AnimationEffect::animationDidTick):
(WebCore::AnimationEffect::animationDidPlay): Deleted.
* animation/KeyframeEffect.cpp:
(WebCore::KeyframeEffect::updateAcceleratedActions):
(WebCore::KeyframeEffect::animationDidPlay): Deleted.
* animation/KeyframeEffect.h:
(WebCore::KeyframeEffect::isAboutToRunAccelerated const):
* animation/WebAnimation.cpp:
(WebCore::WebAnimation::play):
LayoutTests:
Add a test that runs a "transform" animation with a small delay and checks that it yielded
an accelerated animation. We also rewrite some existing tests to use internals.acceleratedAnimationsForElement()
since relying on a layer tree dump was not reliable, which also resolves some platform-specific
issues.
* platform/glib/TestExpectations:
* platform/gtk/webanimations/partly-accelerated-transition-by-removing-property-expected.txt: Removed.
* platform/mac-wk1/webanimations/partly-accelerated-transition-by-removing-property-expected.txt: Removed.
* platform/win/TestExpectations:
* webanimations/accelerated-css-transition-with-easing-y-axis-above-1.html: Make sure overflow doesn't cause an image failure.
* webanimations/partly-accelerated-transition-by-removing-property-expected.txt:
* webanimations/partly-accelerated-transition-by-removing-property.html: Rewrite test to check on the accelerated
animation count and ensure the element is already composited to make the test work on WK1 and GTK.
* webanimations/resources/request-frames-until-true.js: Added.
(const.requestFramesUntilTrue.async resolveCondition):
* webanimations/transform-animation-with-delay-yields-accelerated-animation-expected.txt: Added.
* webanimations/transform-animation-with-delay-yields-accelerated-animation.html: Added.
Modified Paths
Added Paths
Removed Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (289210 => 289211)
--- trunk/LayoutTests/ChangeLog 2022-02-07 13:53:35 UTC (rev 289210)
+++ trunk/LayoutTests/ChangeLog 2022-02-07 13:57:38 UTC (rev 289211)
@@ -1,3 +1,29 @@
+2022-02-07 Antoine Quint <[email protected]>
+
+ [Web Animations] Starting a transform animation with a 1ms delay doesn't run it accelerated
+ https://bugs.webkit.org/show_bug.cgi?id=236080
+ <rdar://problem/88432373>
+
+ Reviewed by Dean Jackson.
+
+ Add a test that runs a "transform" animation with a small delay and checks that it yielded
+ an accelerated animation. We also rewrite some existing tests to use internals.acceleratedAnimationsForElement()
+ since relying on a layer tree dump was not reliable, which also resolves some platform-specific
+ issues.
+
+ * platform/glib/TestExpectations:
+ * platform/gtk/webanimations/partly-accelerated-transition-by-removing-property-expected.txt: Removed.
+ * platform/mac-wk1/webanimations/partly-accelerated-transition-by-removing-property-expected.txt: Removed.
+ * platform/win/TestExpectations:
+ * webanimations/accelerated-css-transition-with-easing-y-axis-above-1.html: Make sure overflow doesn't cause an image failure.
+ * webanimations/partly-accelerated-transition-by-removing-property-expected.txt:
+ * webanimations/partly-accelerated-transition-by-removing-property.html: Rewrite test to check on the accelerated
+ animation count and ensure the element is already composited to make the test work on WK1 and GTK.
+ * webanimations/resources/request-frames-until-true.js: Added.
+ (const.requestFramesUntilTrue.async resolveCondition):
+ * webanimations/transform-animation-with-delay-yields-accelerated-animation-expected.txt: Added.
+ * webanimations/transform-animation-with-delay-yields-accelerated-animation.html: Added.
+
2022-02-07 Rob Buis <[email protected]>
Bail out early in stopForUserCancel
Modified: trunk/LayoutTests/platform/glib/TestExpectations (289210 => 289211)
--- trunk/LayoutTests/platform/glib/TestExpectations 2022-02-07 13:53:35 UTC (rev 289210)
+++ trunk/LayoutTests/platform/glib/TestExpectations 2022-02-07 13:57:38 UTC (rev 289211)
@@ -1941,6 +1941,10 @@
# The DocumentTimeline.maximumFrameRate property which this test is about is not available on GTK/WPE.
webanimations/frame-rate/document-timeline-maximum-frame-rate.html [ Skip ]
+# This test requires an internal API which only works on Cocoa ports.
+webanimations/transform-animation-with-delay-yields-accelerated-animation.html [ Skip ]
+webanimations/partly-accelerated-transition-by-removing-property.html [ Skip ]
+
# OT-SVG is not implemented on GTK/WPE
fast/text/otsvg-canvas.html [ ImageOnlyFailure ]
Deleted: trunk/LayoutTests/platform/gtk/webanimations/partly-accelerated-transition-by-removing-property-expected.txt (289210 => 289211)
--- trunk/LayoutTests/platform/gtk/webanimations/partly-accelerated-transition-by-removing-property-expected.txt 2022-02-07 13:53:35 UTC (rev 289210)
+++ trunk/LayoutTests/platform/gtk/webanimations/partly-accelerated-transition-by-removing-property-expected.txt 2022-02-07 13:57:38 UTC (rev 289211)
@@ -1 +0,0 @@
-
Deleted: trunk/LayoutTests/platform/mac-wk1/webanimations/partly-accelerated-transition-by-removing-property-expected.txt (289210 => 289211)
--- trunk/LayoutTests/platform/mac-wk1/webanimations/partly-accelerated-transition-by-removing-property-expected.txt 2022-02-07 13:53:35 UTC (rev 289210)
+++ trunk/LayoutTests/platform/mac-wk1/webanimations/partly-accelerated-transition-by-removing-property-expected.txt 2022-02-07 13:57:38 UTC (rev 289211)
@@ -1 +0,0 @@
-
Modified: trunk/LayoutTests/platform/win/TestExpectations (289210 => 289211)
--- trunk/LayoutTests/platform/win/TestExpectations 2022-02-07 13:53:35 UTC (rev 289210)
+++ trunk/LayoutTests/platform/win/TestExpectations 2022-02-07 13:57:38 UTC (rev 289211)
@@ -4348,7 +4348,6 @@
[ Win10 ] http/tests/security/local-video-poster-from-remote.html [ Failure ]
[ Win10 ] storage/indexeddb/modern/idbindex-getallkeys-1.html [ Failure ]
[ Win10 ] transforms/2d/hindi-rotated.html [ Failure ]
-[ Win10 ] webanimations/partly-accelerated-transition-by-removing-property.html [ Failure ]
[ Win10 ] fast/repaint/block-no-inflow-children.html [ ImageOnlyFailure ]
[ Win10 ] imported/blink/fast/borders/border-radius-with-layer.html [ ImageOnlyFailure ]
@@ -4957,6 +4956,10 @@
# The DocumentTimeline.maximumFrameRate property which this test is about is not available on Windows.
webanimations/frame-rate/document-timeline-maximum-frame-rate.html [ Skip ]
+# This test requires an internal API which only works on Cocoa ports.
+webanimations/transform-animation-with-delay-yields-accelerated-animation.html [ Skip ]
+webanimations/partly-accelerated-transition-by-removing-property.html [ Skip ]
+
pointerevents/mouse/pointer-button-and-buttons.html [ Failure ]
pointerevents/mouse/pointer-capture.html [ Failure ]
Modified: trunk/LayoutTests/webanimations/accelerated-css-transition-with-easing-y-axis-above-1.html (289210 => 289211)
--- trunk/LayoutTests/webanimations/accelerated-css-transition-with-easing-y-axis-above-1.html 2022-02-07 13:53:35 UTC (rev 289210)
+++ trunk/LayoutTests/webanimations/accelerated-css-transition-with-easing-y-axis-above-1.html 2022-02-07 13:57:38 UTC (rev 289211)
@@ -3,6 +3,7 @@
body {
background-color: red;
+ overflow: hidden;
}
div {
Modified: trunk/LayoutTests/webanimations/partly-accelerated-transition-by-removing-property-expected.txt (289210 => 289211)
--- trunk/LayoutTests/webanimations/partly-accelerated-transition-by-removing-property-expected.txt 2022-02-07 13:53:35 UTC (rev 289210)
+++ trunk/LayoutTests/webanimations/partly-accelerated-transition-by-removing-property-expected.txt 2022-02-07 13:57:38 UTC (rev 289211)
@@ -1,19 +1,3 @@
-(GraphicsLayer
- (anchor 0.00 0.00)
- (bounds 800.00 600.00)
- (children 1
- (GraphicsLayer
- (bounds 800.00 600.00)
- (contentsOpaque 1)
- (children 1
- (GraphicsLayer
- (position 100.00 0.00)
- (bounds 100.00 100.00)
- (opacity 0.50)
- (contentsOpaque 1)
- )
- )
- )
- )
-)
+PASS Transtioning two properties, one of which can be accelerated while the other cannot, yields only one accelerated animation.
+
Modified: trunk/LayoutTests/webanimations/partly-accelerated-transition-by-removing-property.html (289210 => 289211)
--- trunk/LayoutTests/webanimations/partly-accelerated-transition-by-removing-property.html 2022-02-07 13:53:35 UTC (rev 289210)
+++ trunk/LayoutTests/webanimations/partly-accelerated-transition-by-removing-property.html 2022-02-07 13:57:38 UTC (rev 289211)
@@ -1,29 +1,48 @@
<!DOCTYPE html>
<body>
-<pre id="results"></pre>
-<div id="target" style="position: absolute; top: 0; left: 0; width: 100px; height: 100px; background-color: black;"></div>
+<script src=""
+<script src=""
+<script src=""
+<style>
+
+ #target {
+ position: absolute;
+ left: 0;
+ top: 0;
+ width: 100px;
+ height: 100px;
+ background-color: black;
+ will-change: opacity;
+ }
+
+</style>
+<div id="target"></div>
<script>
-if (window.testRunner) {
- testRunner.waitUntilDone();
- testRunner.dumpAsText();
-}
+promise_test(async t => {
+ const target = document.getElementById("target");
-const target = document.getElementById("target");
-target.style.opacity = "0.5";
-target.style.marginLeft = "100px";
-setTimeout(() => {
+ const acceleratedAnimationsStarted = async numberOfAnimations => {
+ return requestFramesUntilTrue(
+ () => internals.acceleratedAnimationsForElement(target).length == numberOfAnimations,
+ () => !"acceleratedAnimationsForElement" in window.internals);
+ };
+
+ // Set initial styles.
+ target.style.opacity = "0.5";
+ target.style.marginLeft = "100px";
+
+ await new Promise(setTimeout);
+
+ // Now update styles to trigger a transition for properties, only one of which should yield an accelerated animation.
target.style.removeProperty("opacity");
target.style.removeProperty("margin-left");
target.style.transitionProperty = "margin-left opacity";
target.style.transitionDuration = "1s";
- requestAnimationFrame(() => {
- if (window.internals)
- document.getElementById("results").innerText = internals.layerTreeAsText(document);
- if (window.testRunner)
- testRunner.notifyDone();
- });
-});
+ // Wait a few frames for the opacity animation to be commited.
+ await acceleratedAnimationsStarted(1);
+}, "Transtioning two properties, one of which can be accelerated while the other cannot, yields only one accelerated animation.");
+
</script>
</body>
Added: trunk/LayoutTests/webanimations/resources/request-frames-until-true.js (0 => 289211)
--- trunk/LayoutTests/webanimations/resources/request-frames-until-true.js (rev 0)
+++ trunk/LayoutTests/webanimations/resources/request-frames-until-true.js 2022-02-07 13:57:38 UTC (rev 289211)
@@ -0,0 +1,16 @@
+
+const requestFramesUntilTrue = async (resolveCondition, rejectCondition) => {
+ return new Promise((resolve, reject) => {
+ if (rejectCondition && rejectCondition()) {
+ reject();
+ return;
+ }
+
+ (function tryFrame () {
+ if (resolveCondition())
+ resolve();
+ else
+ requestAnimationFrame(tryFrame);
+ })();
+ });
+};
Added: trunk/LayoutTests/webanimations/transform-animation-with-delay-yields-accelerated-animation-expected.txt (0 => 289211)
--- trunk/LayoutTests/webanimations/transform-animation-with-delay-yields-accelerated-animation-expected.txt (rev 0)
+++ trunk/LayoutTests/webanimations/transform-animation-with-delay-yields-accelerated-animation-expected.txt 2022-02-07 13:57:38 UTC (rev 289211)
@@ -0,0 +1,3 @@
+
+PASS A transform animation with a delay should run accelerated.
+
Added: trunk/LayoutTests/webanimations/transform-animation-with-delay-yields-accelerated-animation.html (0 => 289211)
--- trunk/LayoutTests/webanimations/transform-animation-with-delay-yields-accelerated-animation.html (rev 0)
+++ trunk/LayoutTests/webanimations/transform-animation-with-delay-yields-accelerated-animation.html 2022-02-07 13:57:38 UTC (rev 289211)
@@ -0,0 +1,35 @@
+<!DOCTYPE html>
+<body>
+<script src=""
+<script src=""
+<style>
+
+ #target {
+ position: absolute;
+ left: 0;
+ top: 0;
+ width: 100px;
+ height: 100px;
+ background-color: black;
+ }
+
+</style>
+<div id="target"></div>
+<script>
+
+promise_test(async t => {
+ // Start an accelerated "transform" animation with a tiny delay.
+ const animation = document.getElementById("target").animate({ transform: "translateX(600px)" }, { duration: 1000 * 1000, delay: 1 });
+
+ // Wait two frames for the accelerated animation to be committed.
+ await animation.ready;
+ await new Promise(requestAnimationFrame);
+ await new Promise(requestAnimationFrame);
+ // And another one since the delay means it will take another frame until the animation enters its active phase.
+ await new Promise(requestAnimationFrame);
+
+ assert_equals(internals.acceleratedAnimationsForElement(target).length, 1, "There should be an accelerated animation.");
+}, "A transform animation with a delay should run accelerated.");
+
+</script>
+</body>
Modified: trunk/Source/WebCore/ChangeLog (289210 => 289211)
--- trunk/Source/WebCore/ChangeLog 2022-02-07 13:53:35 UTC (rev 289210)
+++ trunk/Source/WebCore/ChangeLog 2022-02-07 13:57:38 UTC (rev 289211)
@@ -1,3 +1,67 @@
+2022-02-07 Antoine Quint <[email protected]>
+
+ [Web Animations] Starting a transform animation with a 1ms delay doesn't run it accelerated
+ https://bugs.webkit.org/show_bug.cgi?id=236080
+ <rdar://problem/88432373>
+
+ Reviewed by Dean Jackson.
+
+ Accelerated animations can only be started if the keyframe effect's renderer is composited.
+ Keyframe effects enqueue a list of accelerated actions (play, pause, seek, stop, etc.) when
+ they can be accelerated, and these actions are performed in sync as the document timeline
+ is finishing up its "update animations and send events" procedure as we update the page
+ rendering.
+
+ When an animation is started _without_ a delay, the machinery to consider making a renderer
+ composited happens _before_ we update animations and send events, and since at this stage the
+ new animation's effect is already in its target's effect stack because it is immediately
+ "relevant" (per the Web Animations terminology) and targets a property that can be accelerated,
+ its renderer is indeed composited.
+
+ Thus by the time we consider starting this animation's effect with acceleration, it is in a
+ condition to do so since its renderer is already composited.
+
+ However, when an animation is started _with_ a delay, it is not immediately "relevant" and thus
+ won't appear in its target's effect stack. That will happen once we run the "update animations
+ and send events procedure" the next time we update the page rendering. But before that, the
+ effect's renderer will be considered to be made composited and _will not_ be because at this
+ point there is no effect animating a property that can be accelerated.
+
+ As we eventually run the "update animations and send events" procedure, we'll enqueue an
+ accelerated action to play the animation now that it became relevant, and when that procedure
+ completes and we try to apply that accelerated action, we'll fail to start an accelerated
+ animation since the renderer is not composited.
+
+ To address this, we only enqueue accelerated actions when the renderer is composited, thus
+ matching the condition to actually be able to apply this action.
+
+ This means we no longer need the animationDidPlay() on effects since we wouldn't be able to
+ honor enqueuing a "play" accelerated action since the renderer won't be composited yet at this
+ time in the vast majority of cases.
+
+ Test: webanimations/transform-animation-with-delay-yields-accelerated-animation.html
+
+ * animation/AnimationEffect.h:
+ (WebCore::AnimationEffect::animationDidTick):
+ (WebCore::AnimationEffect::animationDidPlay): Deleted.
+ * animation/KeyframeEffect.cpp:
+ (WebCore::KeyframeEffect::updateAcceleratedActions):
+ (WebCore::KeyframeEffect::animationDidPlay): Deleted.
+ * animation/KeyframeEffect.h:
+ * animation/WebAnimation.cpp:
+ (WebCore::WebAnimation::play):
+
+ * animation/AnimationEffect.h:
+ (WebCore::AnimationEffect::animationDidTick):
+ (WebCore::AnimationEffect::animationDidPlay): Deleted.
+ * animation/KeyframeEffect.cpp:
+ (WebCore::KeyframeEffect::updateAcceleratedActions):
+ (WebCore::KeyframeEffect::animationDidPlay): Deleted.
+ * animation/KeyframeEffect.h:
+ (WebCore::KeyframeEffect::isAboutToRunAccelerated const):
+ * animation/WebAnimation.cpp:
+ (WebCore::WebAnimation::play):
+
2022-02-07 Nikolas Zimmermann <[email protected]>
[LBSE] Generalize RenderLayer::renderBoxLocation(), adding support for SVG layers
Modified: trunk/Source/WebCore/animation/AnimationEffect.h (289210 => 289211)
--- trunk/Source/WebCore/animation/AnimationEffect.h 2022-02-07 13:53:35 UTC (rev 289210)
+++ trunk/Source/WebCore/animation/AnimationEffect.h 2022-02-07 13:57:38 UTC (rev 289211)
@@ -63,7 +63,6 @@
ExceptionOr<void> updateTiming(std::optional<OptionalEffectTiming>);
virtual void animationDidTick() { };
- virtual void animationDidPlay() { };
virtual void animationDidChangeTimingProperties() { };
virtual void animationWasCanceled() { };
virtual void animationSuspensionStateDidChange(bool) { };
Modified: trunk/Source/WebCore/animation/KeyframeEffect.cpp (289210 => 289211)
--- trunk/Source/WebCore/animation/KeyframeEffect.cpp 2022-02-07 13:53:35 UTC (rev 289210)
+++ trunk/Source/WebCore/animation/KeyframeEffect.cpp 2022-02-07 13:57:38 UTC (rev 289211)
@@ -1616,6 +1616,10 @@
void KeyframeEffect::updateAcceleratedActions()
{
+ auto* renderer = this->renderer();
+ if (!renderer || !renderer->isComposited())
+ return;
+
if (!canBeAccelerated()) {
// In the case where this animation is actively targeting a transform-related property and yet
// cannot be accelerated, we must notify the effect stack such that any running accelerated
@@ -1680,12 +1684,6 @@
updateAcceleratedActions();
}
-void KeyframeEffect::animationDidPlay()
-{
- if (m_acceleratedPropertiesState != AcceleratedProperties::None)
- addPendingAcceleratedAction(AcceleratedAction::Play);
-}
-
void KeyframeEffect::animationDidChangeTimingProperties()
{
computeSomeKeyframesUseStepsTimingFunction();
Modified: trunk/Source/WebCore/animation/KeyframeEffect.h (289210 => 289211)
--- trunk/Source/WebCore/animation/KeyframeEffect.h 2022-02-07 13:53:35 UTC (rev 289210)
+++ trunk/Source/WebCore/animation/KeyframeEffect.h 2022-02-07 13:57:38 UTC (rev 289211)
@@ -135,7 +135,8 @@
bool triggersStackingContext() const { return m_triggersStackingContext; }
bool isRunningAccelerated() const { return m_runningAccelerated == RunningAccelerated::Yes; }
- bool isAboutToRunAccelerated() const { return canBeAccelerated() && m_lastRecordedAcceleratedAction != AcceleratedAction::Stop; }
+ // 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 filterFunctionListsMatch() const override { return m_filterFunctionListsMatch; }
bool transformFunctionListsMatch() const override { return m_transformFunctionListsMatch; }
@@ -210,7 +211,6 @@
// AnimationEffect
bool isKeyframeEffect() const final { return true; }
void animationDidTick() final;
- void animationDidPlay() final;
void animationDidChangeTimingProperties() final;
void animationWasCanceled() final;
void animationSuspensionStateDidChange(bool) final;
Modified: trunk/Source/WebCore/animation/WebAnimation.cpp (289210 => 289211)
--- trunk/Source/WebCore/animation/WebAnimation.cpp 2022-02-07 13:53:35 UTC (rev 289210)
+++ trunk/Source/WebCore/animation/WebAnimation.cpp 2022-02-07 13:57:38 UTC (rev 289211)
@@ -1047,9 +1047,6 @@
invalidateEffect();
- if (m_effect)
- m_effect->animationDidPlay();
-
return { };
}
