REGRESSION (r232186): Hardware-accelerated CSS animations using steps() timing functi...
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 3 Jul 2018 15:01:55 +0000 (15:01 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 3 Jul 2018 15:01:55 +0000 (15:01 +0000)
https://bugs.webkit.org/show_bug.cgi?id=186129

Patch by Frederic Wang <fwang@igalia.com> on 2018-07-03
Reviewed by Antoine Quint.

Source/WebCore:

When the WebAnimationsCSSIntegration flag is enabled, animating the transform property with
a steps() timing function no longer works. This is because the WebAnimation code wrongly
assumes that the transform property can always be accelerated (for counterexamples, see
GraphicsLayerCA::animationCanBeAccelerated). For consistency with AnimationBase, we make
WebAnimation fallback to non-accelerated mode when RenderBoxModelObject::startAnimation
fails. This addresses the regression previously mentioned.

Test: http/wpt/css/css-animations/start-animation-001.html

* animation/KeyframeEffectReadOnly.cpp:
(WebCore::KeyframeEffectReadOnly::applyPendingAcceleratedActions): Fallback to
non-accelerated mode if startAnimation failed.

LayoutTests:

Add a test to ensure that accelerated and non-accelerated animations are properly started
when WebAnimationsCSSIntegration is enabled. In particular, consider the case of animated
transform using steps() timing function.

* http/wpt/css/css-animations/start-animation-001-expected.html: Added.
* http/wpt/css/css-animations/start-animation-001.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@233462 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/http/wpt/css/css-animations/start-animation-001-expected.html [new file with mode: 0644]
LayoutTests/http/wpt/css/css-animations/start-animation-001.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/animation/KeyframeEffectReadOnly.cpp

index b127a09..38b41e9 100644 (file)
@@ -1,3 +1,17 @@
+2018-07-03  Frederic Wang  <fwang@igalia.com>
+
+        REGRESSION (r232186): Hardware-accelerated CSS animations using steps() timing function no longer work
+        https://bugs.webkit.org/show_bug.cgi?id=186129
+
+        Reviewed by Antoine Quint.
+
+        Add a test to ensure that accelerated and non-accelerated animations are properly started
+        when WebAnimationsCSSIntegration is enabled. In particular, consider the case of animated
+        transform using steps() timing function.
+
+        * http/wpt/css/css-animations/start-animation-001-expected.html: Added.
+        * http/wpt/css/css-animations/start-animation-001.html: Added.
+
 2018-07-03  Frederic Wang  <fred.wang@free.fr>
 
         [iOS] Animations with B├ęzier timing function not suspended on UI process when animation-play-state is set to "paused"
diff --git a/LayoutTests/http/wpt/css/css-animations/start-animation-001-expected.html b/LayoutTests/http/wpt/css/css-animations/start-animation-001-expected.html
new file mode 100644 (file)
index 0000000..2edac8f
--- /dev/null
@@ -0,0 +1,28 @@
+<!DOCTYPE html>
+<html>
+  <head>
+    <meta charset="utf-8">
+    <title>Verify that animations properly start (reference)</title>
+    <style>
+      #container {
+        position: absolute;
+        left: 0;
+        top: 3em;
+      }
+      #coveringRect {
+        background: blue;
+        position: absolute;
+        top: -10px;
+        left: 90px;
+        width: 5000px;
+        height: 500px;
+      }
+    </style>
+  </head>
+  <body>
+    <p>This test passes if green squares are moved behind the blue rectangle.</p>
+    <div id="container">
+      <div id="coveringRect"></div>
+    </div>
+  </body>
+</html>
diff --git a/LayoutTests/http/wpt/css/css-animations/start-animation-001.html b/LayoutTests/http/wpt/css/css-animations/start-animation-001.html
new file mode 100644 (file)
index 0000000..14f19fe
--- /dev/null
@@ -0,0 +1,76 @@
+<!DOCTYPE html><!-- webkit-test-runner [ enableWebAnimationsCSSIntegration=true ] -->
+<html>
+  <head>
+    <meta charset="utf-8">
+    <title>Verify that animations properly start</title>
+    <style>
+      #container {
+        position: absolute;
+        left: 0;
+        top: 3em;
+      }
+      #squareLinear, #squareSteps, #squareNonAccelerated  {
+        width: 100px;
+        height: 100px;
+        background: lightblue;
+        position: absolute;
+      }
+      /* Avoid animation-fill-mode, to make sure that the move is done by the animation code. */
+      #squareLinear {
+        top: 0px;
+        animation: moveByTransform 10s linear;
+      }
+      #squareSteps {
+        top: 150px;
+        animation: moveByTransform 10s steps(1000);
+      }
+      #squareNonAccelerated  {
+        top: 300px;
+        animation: moveByLeft 10s steps(100);
+      }
+      @keyframes moveByTransform
+      {
+        100% {
+          transform: translate(5000px);
+        }
+      }
+      @keyframes moveByLeft
+      {
+        0% {
+          left: 0px;
+        }
+        100% {
+          left: 5000px;
+        }
+      }
+      #coveringRect {
+        background: blue;
+        position: absolute;
+        top: -10px;
+        left: 90px;
+        width: 5000px;
+        height: 500px;
+      }
+    </style>
+    <script>
+      function runTest() {
+        if (!window.testRunner)
+          return;
+        // We wait a bit after the squares are moved behind the blue rectangle. For discontinuous
+        // transforms we have 5000px / 100 steps = 5px/step. Hence this happens after 20 steps i.e.
+        // 20/100*10s = 200ms.
+        testRunner.waitUntilDone();
+        setTimeout(() => { testRunner.notifyDone(); }, 300);
+      }
+    </script>
+  </head>
+  <body onload="runTest()">
+    <p>This test passes if green squares are moved behind the blue rectangle.</p>
+    <div id="container">
+      <div id="squareLinear"><tt>transform</tt> (linear)</div>
+      <div id="squareSteps"><tt>transform</tt> (steps)</div>
+      <div id="squareNonAccelerated"><tt>left</tt> (steps)</div>
+      <div id="coveringRect"></div>
+    </div>
+  </body>
+</html>
index 44aef0c..b9a2435 100644 (file)
@@ -1,3 +1,23 @@
+2018-07-03  Frederic Wang  <fwang@igalia.com>
+
+        REGRESSION (r232186): Hardware-accelerated CSS animations using steps() timing function no longer work
+        https://bugs.webkit.org/show_bug.cgi?id=186129
+
+        Reviewed by Antoine Quint.
+
+        When the WebAnimationsCSSIntegration flag is enabled, animating the transform property with
+        a steps() timing function no longer works. This is because the WebAnimation code wrongly
+        assumes that the transform property can always be accelerated (for counterexamples, see
+        GraphicsLayerCA::animationCanBeAccelerated). For consistency with AnimationBase, we make
+        WebAnimation fallback to non-accelerated mode when RenderBoxModelObject::startAnimation
+        fails. This addresses the regression previously mentioned.
+
+        Test: http/wpt/css/css-animations/start-animation-001.html
+
+        * animation/KeyframeEffectReadOnly.cpp:
+        (WebCore::KeyframeEffectReadOnly::applyPendingAcceleratedActions): Fallback to
+        non-accelerated mode if startAnimation failed.
+
 2018-07-03  David Kilzer  <ddkilzer@apple.com>
 
         [iOS] Add assert to catch improper use of WebCore::Timer in UI Process
index 9e421df..590b402 100644 (file)
@@ -1254,7 +1254,11 @@ void KeyframeEffectReadOnly::applyPendingAcceleratedActions()
     for (const auto& action : pendingAccelerationActions) {
         switch (action) {
         case AcceleratedAction::Play:
-            compositedRenderer->startAnimation(timeOffset, backingAnimationForCompositedRenderer().ptr(), m_blendingKeyframes);
+            if (!compositedRenderer->startAnimation(timeOffset, backingAnimationForCompositedRenderer().ptr(), m_blendingKeyframes)) {
+                m_shouldRunAccelerated = false;
+                m_lastRecordedAcceleratedAction = AcceleratedAction::Stop;
+                return;
+            }
             break;
         case AcceleratedAction::Pause:
             compositedRenderer->animationPaused(timeOffset, m_blendingKeyframes.animationName());