REGRESSION (r260360): easing curves are broken on JS-originated anima…

…tions https://bugs.webkit.org/show_bug.cgi?id=213495 <rdar://problem/64649747> Reviewed by Darin Adler. Source/WebCore: Prior to Web Animations, there was no way for an animation to set an animation-wide timing function while also setting a per-keyframe timing function. As such GraphicsLayerCA would sometimes decide to set the timing function on keyframes or on the entire CAAnimation. However, we can no longer do this with Web Animations where an animation can set an animation-wide timing function and also keyframe-specific timing functions. In this patch we create CAKeyframeAnimation objects for any animation that has at least two keyframes if Web Animations are enabled, whereas the legacy code path requires at least three keyframes. We allow PlatformCAAnimation::setTimingFunction() to be called in the Web Animations code path only under GraphicsLayerCA::setupAnimation() while leaving the only call sites in place only for the legacy code path. Finally, we modify GraphicsLayerCA::timingFunctionForAnimationValue() to only ever return a keyframe- specific timing function or fall back to a default linear timing function in the Web Animations code path, leaving the function to behave the same way as it used to in the legacy code path. Test: webanimations/accelerated-animation-with-easing.html * platform/animation/TimingFunction.h: * platform/graphics/ca/GraphicsLayerCA.cpp: (WebCore::isKeyframe): (WebCore::GraphicsLayerCA::createAnimationFromKeyframes): (WebCore::GraphicsLayerCA::appendToUncommittedAnimations): (WebCore::GraphicsLayerCA::createTransformAnimationsFromKeyframes): LayoutTests: Add a new test that checks that various ways of setting the easing timing function on a JS-originated animation always yield the same visible animation behavior. * webanimations/accelerated-animation-with-easing-expected.html: Added. * webanimations/accelerated-animation-with-easing.html: Added. Canonical link: https://commits.webkit.org/226392@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@263506 268f45cc-cd09-0410-ab3c-d52691b4dbfc
WebKit · Jun 25, 2020 · 38666f1 · 38666f1
1 parent 04928a2
commit 38666f1
Show file tree

Hide file tree

Showing 6 changed files with 165 additions and 10 deletions.
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
@@ -1,3 +1,17 @@
+2020-06-25  Antoine Quint  <graouts@webkit.org>
+
+        REGRESSION (r260360): easing curves are broken on JS-originated animations
+        https://bugs.webkit.org/show_bug.cgi?id=213495
+        <rdar://problem/64649747>
+
+        Reviewed by Darin Adler.
+
+        Add a new test that checks that various ways of setting the easing timing function on a JS-originated
+        animation always yield the same visible animation behavior.
+
+        * webanimations/accelerated-animation-with-easing-expected.html: Added.
+        * webanimations/accelerated-animation-with-easing.html: Added.
+
 2020-06-24  Sergio Villar Senin  <svillar@igalia.com>
 
         Make video-inside-flex-item.html more robust

diff --git a/LayoutTests/webanimations/accelerated-animation-with-easing-expected.html b/LayoutTests/webanimations/accelerated-animation-with-easing-expected.html
@@ -0,0 +1,2 @@
+<body style="background-color: green">
+</body>
diff --git a/LayoutTests/webanimations/accelerated-animation-with-easing.html b/LayoutTests/webanimations/accelerated-animation-with-easing.html
@@ -0,0 +1,84 @@
+<body>
+<style>
+
+    body {
+        background-color: green;
+    }
+
+    div {
+        position: absolute;
+        top: 0;
+        left: 0;
+        width: 100px;
+        height: 100px;
+        background-color: red;
+    }
+
+    /* Allow for a 1px error on either side */
+    #easing-on-keyframe {
+        left: -1px;
+        width: 102px;
+        background-color: green;
+    }
+
+</style>
+<div id="easing-on-animation-one-keyframe"></div>
+<div id="easing-on-animation-two-keyframes"></div>
+<div id="easing-on-animation-three-keyframes"></div>
+<div id="easing-on-keyframe"></div>
+<script>
+
+(async function() {
+    if (window.testRunner)
+        testRunner.waitUntilDone();
+
+    const animations = [];
+
+    animations.push(document.getElementById("easing-on-animation-one-keyframe").animate({ transform: "translateX(100px)" }, {
+        duration: 500,
+        easing: "ease",
+        direction: "alternate",
+        iterations: Infinity
+    }));
+
+    animations.push(document.getElementById("easing-on-animation-two-keyframes").animate([
+        { transform: "translateX(0)" },
+        { transform: "translateX(100px)" }
+    ], {
+        duration: 500,
+        easing: "ease",
+        direction: "alternate",
+        iterations: Infinity
+    }));
+
+    animations.push(document.getElementById("easing-on-animation-three-keyframes").animate([
+        { transform: "translateX(0)" },
+        { transform: "translateX(50px)" },
+        { transform: "translateX(100px)" }
+    ], {
+        duration: 500,
+        easing: "ease",
+        direction: "alternate",
+        iterations: Infinity
+    }));
+
+    animations.push(document.getElementById("easing-on-keyframe").animate([
+        { transform: "translateX(0)", easing: "ease" },
+        { transform: "translateX(100px)" }
+    ], {
+        duration: 500,
+        direction: "alternate",
+        iterations: Infinity
+    }));
+
+    await Promise.all(animations.map(animation => animation.ready));
+    await new Promise(requestAnimationFrame);
+    await new Promise(requestAnimationFrame);
+    await new Promise(requestAnimationFrame);
+
+    if (window.testRunner)
+        testRunner.notifyDone();
+})();
+
+</script>
+</body>
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
@@ -1,3 +1,36 @@
+2020-06-25  Antoine Quint  <graouts@webkit.org>
+
+        REGRESSION (r260360): easing curves are broken on JS-originated animations
+        https://bugs.webkit.org/show_bug.cgi?id=213495
+        <rdar://problem/64649747>
+
+        Reviewed by Darin Adler.
+
+        Prior to Web Animations, there was no way for an animation to set an animation-wide timing function while
+        also setting a per-keyframe timing function. As such GraphicsLayerCA would sometimes decide to set the
+        timing function on keyframes or on the entire CAAnimation. However, we can no longer do this with Web
+        Animations where an animation can set an animation-wide timing function and also keyframe-specific
+        timing functions.
+
+        In this patch we create CAKeyframeAnimation objects for any animation that has at least two keyframes
+        if Web Animations are enabled, whereas the legacy code path requires at least three keyframes. We allow
+        PlatformCAAnimation::setTimingFunction() to be called in the Web Animations code path only under
+        GraphicsLayerCA::setupAnimation() while leaving the only call sites in place only for the legacy code
+        path.
+
+        Finally, we modify GraphicsLayerCA::timingFunctionForAnimationValue() to only ever return a keyframe-
+        specific timing function or fall back to a default linear timing function in the Web Animations code
+        path, leaving the function to behave the same way as it used to in the legacy code path. 
+
+        Test: webanimations/accelerated-animation-with-easing.html
+
+        * platform/animation/TimingFunction.h:
+        * platform/graphics/ca/GraphicsLayerCA.cpp:
+        (WebCore::isKeyframe):
+        (WebCore::GraphicsLayerCA::createAnimationFromKeyframes):
+        (WebCore::GraphicsLayerCA::appendToUncommittedAnimations):
+        (WebCore::GraphicsLayerCA::createTransformAnimationsFromKeyframes):
+
 2020-06-25  Sergio Villar Senin  <svillar@igalia.com>
 
         Unreviewed, WPE Debug build fix after r263503.

diff --git a/Source/WebCore/platform/animation/TimingFunction.h b/Source/WebCore/platform/animation/TimingFunction.h
@@ -80,6 +80,12 @@ class LinearTimingFunction final : public TimingFunction {
         return is<LinearTimingFunction>(other);
     }
 
+    static const LinearTimingFunction& sharedLinearTimingFunction()
+    {
+        static const LinearTimingFunction& function = create().leakRef();
+        return function;
+    }
+
 private:
     LinearTimingFunction()
         : TimingFunction(LinearFunction)

diff --git a/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp b/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp
@@ -41,6 +41,7 @@
 #include "PlatformScreen.h"
 #include "Region.h"
 #include "RotateTransformOperation.h"
+#include "RuntimeEnabledFeatures.h"
 #include "ScaleTransformOperation.h"
 #include "TiledBacking.h"
 #include "TransformState.h"
@@ -3073,19 +3074,25 @@ void GraphicsLayerCA::updateContentsNeedsDisplay()
         m_contentsLayer->setNeedsDisplay();
 }
 
+static bool isKeyframe(const KeyframeValueList& list)
+{
+    if (RuntimeEnabledFeatures::sharedFeatures().webAnimationsCSSIntegrationEnabled())
+        return list.size() > 1;
+    return list.size() > 2;
+}
+
 bool GraphicsLayerCA::createAnimationFromKeyframes(const KeyframeValueList& valueList, const Animation* animation, const String& animationName, Seconds timeOffset)
 {
     ASSERT(valueList.property() != AnimatedPropertyTransform && (!supportsAcceleratedFilterAnimations() || valueList.property() != AnimatedPropertyFilter));
 
-    bool isKeyframe = valueList.size() > 2;
     bool valuesOK;
 
     bool additive = false;
     int animationIndex = 0;
 
     RefPtr<PlatformCAAnimation> caAnimation;
 
-    if (isKeyframe) {
+    if (isKeyframe(valueList)) {
         caAnimation = createKeyframeAnimation(animation, propertyIdToString(valueList.property()), additive);
         valuesOK = setAnimationKeyframes(valueList, animation, caAnimation.get());
     } else {
@@ -3113,11 +3120,10 @@ bool GraphicsLayerCA::appendToUncommittedAnimations(const KeyframeValueList& val
     int numAnimations = isMatrixAnimation ? 1 : operations->size();
     bool additive = animationIndex < numAnimations - 1;
 #endif
-    bool isKeyframe = valueList.size() > 2;
 
     RefPtr<PlatformCAAnimation> caAnimation;
     bool validMatrices = true;
-    if (isKeyframe) {
+    if (isKeyframe(valueList)) {
         caAnimation = createKeyframeAnimation(animation, propertyIdToString(valueList.property()), additive);
         validMatrices = setTransformAnimationKeyframes(valueList, animation, caAnimation.get(), animationIndex, transformOp, isMatrixAnimation, boxSize);
     } else {
@@ -3166,8 +3172,6 @@ bool GraphicsLayerCA::createTransformAnimationsFromKeyframes(const KeyframeValue
 
 bool GraphicsLayerCA::appendToUncommittedAnimations(const KeyframeValueList& valueList, const FilterOperation* operation, const Animation* animation, const String& animationName, int animationIndex, Seconds timeOffset)
 {
-    bool isKeyframe = valueList.size() > 2;
-
     FilterOperation::OperationType filterOp = operation->type();
     int numAnimatedProperties = PlatformCAFilters::numAnimatedFilterProperties(filterOp);
 
@@ -3183,7 +3187,7 @@ bool GraphicsLayerCA::appendToUncommittedAnimations(const KeyframeValueList& val
         RefPtr<PlatformCAAnimation> caAnimation;
         String keyPath = makeString("filters.filter_", animationIndex, '.', PlatformCAFilters::animatedFilterPropertyName(filterOp, internalFilterPropertyIndex));
 
-        if (isKeyframe) {
+        if (isKeyframe(valueList)) {
             caAnimation = createKeyframeAnimation(animation, keyPath, false);
             valuesOK = setFilterAnimationKeyframes(valueList, animation, caAnimation.get(), animationIndex, internalFilterPropertyIndex, filterOp);
         } else {
@@ -3324,10 +3328,19 @@ void GraphicsLayerCA::setupAnimation(PlatformCAAnimation* propertyAnim, const An
     propertyAnim->setRemovedOnCompletion(false);
     propertyAnim->setAdditive(additive);
     propertyAnim->setFillMode(fillMode);
+
+    if (RuntimeEnabledFeatures::sharedFeatures().webAnimationsCSSIntegrationEnabled())
+        propertyAnim->setTimingFunction(anim->timingFunction());
 }
 
 const TimingFunction& GraphicsLayerCA::timingFunctionForAnimationValue(const AnimationValue& animValue, const Animation& anim)
 {
+    if (RuntimeEnabledFeatures::sharedFeatures().webAnimationsCSSIntegrationEnabled()) {
+        if (animValue.timingFunction())
+            return *animValue.timingFunction();
+        return LinearTimingFunction::sharedLinearTimingFunction();
+    }
+
     if (animValue.timingFunction())
         return *animValue.timingFunction();
     if (anim.isTimingFunctionSet())
@@ -3355,7 +3368,8 @@ bool GraphicsLayerCA::setAnimationEndpoints(const KeyframeValueList& valueList,
 
     // This codepath is used for 2-keyframe animations, so we still need to look in the start
     // for a timing function. Even in the reversing animation case, the first keyframe provides the timing function.
-    basicAnim->setTimingFunction(&timingFunctionForAnimationValue(valueList.at(0), *animation), !forwards);
+    if (!RuntimeEnabledFeatures::sharedFeatures().webAnimationsCSSIntegrationEnabled())
+        basicAnim->setTimingFunction(&timingFunctionForAnimationValue(valueList.at(0), *animation), !forwards);
 
     return true;
 }
@@ -3448,7 +3462,8 @@ bool GraphicsLayerCA::setTransformAnimationEndpoints(const KeyframeValueList& va
 
     // This codepath is used for 2-keyframe animations, so we still need to look in the start
     // for a timing function. Even in the reversing animation case, the first keyframe provides the timing function.
-    basicAnim->setTimingFunction(&timingFunctionForAnimationValue(valueList.at(0), *animation), !forwards);
+    if (!RuntimeEnabledFeatures::sharedFeatures().webAnimationsCSSIntegrationEnabled())
+        basicAnim->setTimingFunction(&timingFunctionForAnimationValue(valueList.at(0), *animation), !forwards);
 
     auto valueFunction = getValueFunctionNameForTransformOperation(transformOpType);
     if (valueFunction != PlatformCAAnimation::NoValueFunction)
@@ -3558,7 +3573,8 @@ bool GraphicsLayerCA::setFilterAnimationEndpoints(const KeyframeValueList& value
 
     // This codepath is used for 2-keyframe animations, so we still need to look in the start
     // for a timing function. Even in the reversing animation case, the first keyframe provides the timing function.
-    basicAnim->setTimingFunction(&timingFunctionForAnimationValue(valueList.at(0), *animation), !forwards);
+    if (!RuntimeEnabledFeatures::sharedFeatures().webAnimationsCSSIntegrationEnabled())
+        basicAnim->setTimingFunction(&timingFunctionForAnimationValue(valueList.at(0), *animation), !forwards);
 
     return true;
 }