- Revision
- 265358
- Author
- simon.fra...@apple.com
- Date
- 2020-08-06 17:57:45 -0700 (Thu, 06 Aug 2020)
Log Message
Avoid triggering redundant compositing updates when trying ot run a steps() animation on transform
https://bugs.webkit.org/show_bug.cgi?id=215241
<rdar://problem/62737868>
Reviewed by Zalan Bujtas.
Source/WebCore:
With a steps() timing function and keyframes animating the transform property, KeyframeEffect::applyPendingAcceleratedActions()
tries to restart the animation every time because the GraphicsLayer reports that it didn't start an accelerated animation.
r264856 patched some of this, but we still call animationFinished() every time, and this triggers a compositing update via
the m_owningLayer.setNeeds* calls in RenderLayerBacking::animationFinished().
So don't try to remove the animation if wasn't running. This makes those compositing updates a no-op, which is important
because these animations still invalidate style on every frame (webkit.org/b/215229).
Test: animations/steps-transform-compositing-updates.html
* animation/KeyframeEffect.cpp:
(WebCore::KeyframeEffect::applyPendingAcceleratedActions):
LayoutTests:
animations/steps-transform-rendering-updates.html was landed with a bug; it aliased
the global 'count' variable, and was thus testing the wrong thing. So land a failing
result for the test for now (webkit.org/b/215229 addresses the fix).
* animations/steps-transform-compositing-updates-expected.txt: Copied from LayoutTests/animations/steps-transform-rendering-updates-expected.txt.
* animations/steps-transform-compositing-updates.html: Copied from LayoutTests/animations/steps-transform-rendering-updates.html.
* animations/steps-transform-rendering-updates-expected.txt:
* animations/steps-transform-rendering-updates.html:
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (265357 => 265358)
--- trunk/LayoutTests/ChangeLog 2020-08-07 00:23:50 UTC (rev 265357)
+++ trunk/LayoutTests/ChangeLog 2020-08-07 00:57:45 UTC (rev 265358)
@@ -1,3 +1,20 @@
+2020-08-06 Simon Fraser <simon.fra...@apple.com>
+
+ Avoid triggering redundant compositing updates when trying ot run a steps() animation on transform
+ https://bugs.webkit.org/show_bug.cgi?id=215241
+ <rdar://problem/62737868>
+
+ Reviewed by Zalan Bujtas.
+
+ animations/steps-transform-rendering-updates.html was landed with a bug; it aliased
+ the global 'count' variable, and was thus testing the wrong thing. So land a failing
+ result for the test for now (webkit.org/b/215229 addresses the fix).
+
+ * animations/steps-transform-compositing-updates-expected.txt: Copied from LayoutTests/animations/steps-transform-rendering-updates-expected.txt.
+ * animations/steps-transform-compositing-updates.html: Copied from LayoutTests/animations/steps-transform-rendering-updates.html.
+ * animations/steps-transform-rendering-updates-expected.txt:
+ * animations/steps-transform-rendering-updates.html:
+
2020-08-06 Hector Lopez <hector_i_lo...@apple.com>
[ macOS ] webgpu/whlsl/dont-crash-parsing-enum.html is a flaky failure
Copied: trunk/LayoutTests/animations/steps-transform-compositing-updates-expected.txt (from rev 265357, trunk/LayoutTests/animations/steps-transform-rendering-updates-expected.txt) (0 => 265358)
--- trunk/LayoutTests/animations/steps-transform-compositing-updates-expected.txt (rev 0)
+++ trunk/LayoutTests/animations/steps-transform-compositing-updates-expected.txt 2020-08-07 00:57:45 UTC (rev 265358)
@@ -0,0 +1,6 @@
+PASS count is 0
+PASS count < 6 is true
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Copied: trunk/LayoutTests/animations/steps-transform-compositing-updates.html (from rev 265357, trunk/LayoutTests/animations/steps-transform-rendering-updates.html) (0 => 265358)
--- trunk/LayoutTests/animations/steps-transform-compositing-updates.html (rev 0)
+++ trunk/LayoutTests/animations/steps-transform-compositing-updates.html 2020-08-07 00:57:45 UTC (rev 265358)
@@ -0,0 +1,48 @@
+<!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', () => {
+ let box = document.getElementById('box');
+ box.addEventListener('animationstart', () => {
+ if (window.internals)
+ internals.startTrackingCompositingUpdates();
+ shouldBe('count', '0');
+ }, false);
+ box.addEventListener('animationend', () => {
+ if (window.internals)
+ count = internals.compositingUpdateCount();
+ 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/LayoutTests/animations/steps-transform-rendering-updates-expected.txt (265357 => 265358)
--- trunk/LayoutTests/animations/steps-transform-rendering-updates-expected.txt 2020-08-07 00:23:50 UTC (rev 265357)
+++ trunk/LayoutTests/animations/steps-transform-rendering-updates-expected.txt 2020-08-07 00:57:45 UTC (rev 265358)
@@ -1,5 +1,5 @@
PASS count is 0
-PASS count < 6 is true
+FAIL count < 6 should be true. Was false.
PASS successfullyParsed is true
TEST COMPLETE
Modified: trunk/LayoutTests/animations/steps-transform-rendering-updates.html (265357 => 265358)
--- trunk/LayoutTests/animations/steps-transform-rendering-updates.html 2020-08-07 00:23:50 UTC (rev 265357)
+++ trunk/LayoutTests/animations/steps-transform-rendering-updates.html 2020-08-07 00:57:45 UTC (rev 265358)
@@ -34,7 +34,7 @@
shouldBe('count', '0');
}, false);
box.addEventListener('animationend', () => {
- let count = internals.renderingUpdateCount();
+ count = internals.renderingUpdateCount();
shouldBeTrue('count < 6');
finishJSTest();
}, false);
Copied: trunk/LayoutTests/platform/win/animations/steps-transform-rendering-updates-expected.txt (from rev 265357, trunk/LayoutTests/animations/steps-transform-rendering-updates-expected.txt) (0 => 265358)
--- trunk/LayoutTests/platform/win/animations/steps-transform-rendering-updates-expected.txt (rev 0)
+++ trunk/LayoutTests/platform/win/animations/steps-transform-rendering-updates-expected.txt 2020-08-07 00:57:45 UTC (rev 265358)
@@ -0,0 +1,6 @@
+PASS count is 0
+PASS count < 6 is true
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Modified: trunk/Source/WebCore/ChangeLog (265357 => 265358)
--- trunk/Source/WebCore/ChangeLog 2020-08-07 00:23:50 UTC (rev 265357)
+++ trunk/Source/WebCore/ChangeLog 2020-08-07 00:57:45 UTC (rev 265358)
@@ -1,3 +1,24 @@
+2020-08-06 Simon Fraser <simon.fra...@apple.com>
+
+ Avoid triggering redundant compositing updates when trying ot run a steps() animation on transform
+ https://bugs.webkit.org/show_bug.cgi?id=215241
+ <rdar://problem/62737868>
+
+ Reviewed by Zalan Bujtas.
+
+ With a steps() timing function and keyframes animating the transform property, KeyframeEffect::applyPendingAcceleratedActions()
+ tries to restart the animation every time because the GraphicsLayer reports that it didn't start an accelerated animation.
+ r264856 patched some of this, but we still call animationFinished() every time, and this triggers a compositing update via
+ the m_owningLayer.setNeeds* calls in RenderLayerBacking::animationFinished().
+
+ So don't try to remove the animation if wasn't running. This makes those compositing updates a no-op, which is important
+ because these animations still invalidate style on every frame (webkit.org/b/215229).
+
+ Test: animations/steps-transform-compositing-updates.html
+
+ * animation/KeyframeEffect.cpp:
+ (WebCore::KeyframeEffect::applyPendingAcceleratedActions):
+
2020-08-06 Peng Liu <peng.l...@apple.com>
Web process crashes at WebCore::FullscreenManager::didExitFullscreen
Modified: trunk/Source/WebCore/animation/KeyframeEffect.cpp (265357 => 265358)
--- trunk/Source/WebCore/animation/KeyframeEffect.cpp 2020-08-07 00:23:50 UTC (rev 265357)
+++ trunk/Source/WebCore/animation/KeyframeEffect.cpp 2020-08-07 00:57:45 UTC (rev 265358)
@@ -1612,7 +1612,8 @@
auto timeOffset = animation()->currentTime().valueOr(0_s).seconds() - delay().seconds();
auto startAnimation = [&]() -> RunningAccelerated {
- renderer->animationFinished(m_blendingKeyframes.animationName());
+ if (m_runningAccelerated == RunningAccelerated::Yes)
+ renderer->animationFinished(m_blendingKeyframes.animationName());
if (!m_blendingKeyframes.hasImplicitKeyframes())
return renderer->startAnimation(timeOffset, backingAnimationForCompositedRenderer(), m_blendingKeyframes) ? RunningAccelerated::Yes : RunningAccelerated::No;
