REGRESSION (r263506): scale transform transitions won't overshoot

https://bugs.webkit.org/show_bug.cgi?id=215826 <rdar://problem/67759310> Reviewed by Simon Fraser. Source/WebCore: Test: webanimations/accelerated-css-transition-with-easing-y-axis-above-1.html In r263506 we made a change where accelerated animations would set their animation-wide timing functions using PlatformCAAnimation::setTimingFunction() instead of setting it on individual keyframes. This change was required to support the unique ability of JS-originated animations to specify both an animation-wide timing function as well as per-keyframe timing functions. In the case of CSS Transitions, this meant that instead of setting a per-keyframe timing function, this change would set the animation-wide timing function since CSS Transitions should map the transition-timing-function property to the "easing" property of the Animation object exposed by the Web Animations API, not as the timing function of the single keyframe interval. However, this change surfaced a bug in Core Animation on macOS and iOS where a cubic-bezier() timing function with y values above 1 get clipped when applied to a CAKeyframeAnimation using the timingFunction property (singular) instead of the timingFunctions property (plural). To work around this issue, we set Animation::property() on the generated Animation object passed to GraphicsLayerCA to make it possible to detect when we're dealing with a CSS Transition and set the timing function on the keyframe interval rather than on the animation itself. Alas, this does not fix the case where a JS-originated animation will specify an animation-wide timing function with a cubic-bezier() timing function with y values above 1. There is no known workaround at this time for this case and this is covered by bug 215918. * animation/KeyframeEffect.cpp: (WebCore::KeyframeEffect::backingAnimationForCompositedRenderer const): * platform/graphics/ca/GraphicsLayerCA.cpp: (WebCore::GraphicsLayerCA::createTransformAnimationsFromKeyframes): LayoutTests: Add a new test that checks that a CSS Transition using a cubic-bezier() timing function with a y value beyond 1 does not get clipped. * platform/win/TestExpectations: * webanimations/accelerated-css-transition-with-easing-y-axis-above-1-expected.html: Added. * webanimations/accelerated-css-transition-with-easing-y-axis-above-1.html: Added. Canonical link: https://commits.webkit.org/228730@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@266280 268f45cc-cd09-0410-ab3c-d52691b4dbfc
WebKit · Aug 28, 2020 · ffc770b · ffc770b
1 parent f6adfc1
commit ffc770b
Show file tree

Hide file tree

Showing 7 changed files with 127 additions and 3 deletions.
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
@@ -1,3 +1,18 @@
+2020-08-28  Antoine Quint  <graouts@webkit.org>
+
+        REGRESSION (r263506): scale transform transitions won't overshoot
+        https://bugs.webkit.org/show_bug.cgi?id=215826
+        <rdar://problem/67759310>
+
+        Reviewed by Simon Fraser.
+
+        Add a new test that checks that a CSS Transition using a cubic-bezier() timing function with
+        a y value beyond 1 does not get clipped.
+
+        * platform/win/TestExpectations:
+        * webanimations/accelerated-css-transition-with-easing-y-axis-above-1-expected.html: Added.
+        * webanimations/accelerated-css-transition-with-easing-y-axis-above-1.html: Added.
+
 2020-08-28  Hector Lopez  <hector_i_lopez@apple.com>
 
         [ macOS iOS ] compositing/video/poster.html is a flaky failure

diff --git a/LayoutTests/platform/win/TestExpectations b/LayoutTests/platform/win/TestExpectations
@@ -4551,3 +4551,5 @@ webkit.org/b/215636  fast/text/emoji-gender-fe0f-9.html [ ImageOnlyFailure ]
 webkit.org/b/215636  fast/text/emoji-gender.html [ ImageOnlyFailure ]
 
 webkit.org/b/215827 animations/steps-transform-rendering-updates.html [ Failure ]
+
+webkit.org/b/215930 webanimations/accelerated-css-transition-with-easing-y-axis-above-1.html [ Skip ]
diff --git a/...utTests/webanimations/accelerated-css-transition-with-easing-y-axis-above-1-expected.html b/...utTests/webanimations/accelerated-css-transition-with-easing-y-axis-above-1-expected.html
@@ -0,0 +1,2 @@
+<body style="background-color: green">
+</body>
diff --git a/LayoutTests/webanimations/accelerated-css-transition-with-easing-y-axis-above-1.html b/LayoutTests/webanimations/accelerated-css-transition-with-easing-y-axis-above-1.html
@@ -0,0 +1,45 @@
+<body>
+<style>
+
+    body {
+        background-color: red;
+    }
+
+    div {
+        position: absolute;
+        top: 0;
+        left: 0;
+        width: 400px;
+        height: 400px;
+        background-color: green;
+        transform-origin: top left;
+        transform: scale(0.5);
+    }
+
+    div.scaled-up {
+        transform: scale(1);
+        transition: 2s transform cubic-bezier(0, 200, 1, 200);
+    }
+
+</style>
+<div></div>
+<script src="../resources/ui-helper.js"></script>
+<script>
+
+(async function() {
+    if (window.testRunner)
+        testRunner.waitUntilDone();
+
+    await UIHelper.renderingUpdate();
+    document.querySelector("div").classList.add("scaled-up");
+
+    await Promise.all(document.getAnimations().map(animation => animation.ready));
+    await UIHelper.ensurePresentationUpdate();
+    await UIHelper.renderingUpdate();
+
+    if (window.testRunner)
+        testRunner.notifyDone();
+})();
+
+</script>
+</body>
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
@@ -1,3 +1,42 @@
+2020-08-28  Antoine Quint  <graouts@webkit.org>
+
+        REGRESSION (r263506): scale transform transitions won't overshoot
+        https://bugs.webkit.org/show_bug.cgi?id=215826
+        <rdar://problem/67759310>
+
+        Reviewed by Simon Fraser.
+
+        Test: webanimations/accelerated-css-transition-with-easing-y-axis-above-1.html
+
+        In r263506 we made a change where accelerated animations would set their animation-wide
+        timing functions using PlatformCAAnimation::setTimingFunction() instead of setting it
+        on individual keyframes. This change was required to support the unique ability of
+        JS-originated animations to specify both an animation-wide timing function as well as
+        per-keyframe timing functions.
+
+        In the case of CSS Transitions, this meant that instead of setting a per-keyframe timing
+        function, this change would set the animation-wide timing function since CSS Transitions
+        should map the transition-timing-function property to the "easing" property of the Animation
+        object exposed by the Web Animations API, not as the timing function of the single keyframe
+        interval.
+
+        However, this change surfaced a bug in Core Animation on macOS and iOS where a cubic-bezier()
+        timing function with y values above 1 get clipped when applied to a CAKeyframeAnimation using
+        the timingFunction property (singular) instead of the timingFunctions property (plural).
+
+        To work around this issue, we set Animation::property() on the generated Animation object passed
+        to GraphicsLayerCA to make it possible to detect when we're dealing with a CSS Transition and
+        set the timing function on the keyframe interval rather than on the animation itself.
+
+        Alas, this does not fix the case where a JS-originated animation will specify an animation-wide
+        timing function with a cubic-bezier() timing function with y values above 1. There is no known
+        workaround at this time for this case and this is covered by bug 215918.
+
+        * animation/KeyframeEffect.cpp:
+        (WebCore::KeyframeEffect::backingAnimationForCompositedRenderer const):
+        * platform/graphics/ca/GraphicsLayerCA.cpp:
+        (WebCore::GraphicsLayerCA::createTransformAnimationsFromKeyframes):
+
 2020-08-28  Zalan Bujtas  <zalan@apple.com>
 
         [LFC][IFC] Move inline alignment code to LineBox

diff --git a/Source/WebCore/animation/KeyframeEffect.cpp b/Source/WebCore/animation/KeyframeEffect.cpp
@@ -1672,6 +1672,13 @@ Ref<const Animation> KeyframeEffect::backingAnimationForCompositedRenderer() con
     animation->setTimingFunction(timingFunction()->clone());
     animation->setPlaybackRate(effectAnimation->playbackRate());
 
+    if (m_blendingKeyframesSource == BlendingKeyframesSource::CSSTransition && is<CubicBezierTimingFunction>(timingFunction())) {
+        // FIXME: https://bugs.webkit.org/show_bug.cgi?id=215918
+        // If we are dealing with a CSS Transition and using a cubic bezier timing function, we set up
+        // the Animation object so that GraphicsLayerCA can work around a Core Animation limitation.
+        animation->setProperty({ Animation::TransitionMode::SingleProperty, *m_blendingKeyframes.properties().begin() });
+    }
+
     switch (fill()) {
     case FillMode::None:
     case FillMode::Auto:

diff --git a/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp b/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp
@@ -3331,16 +3331,30 @@ void GraphicsLayerCA::setupAnimation(PlatformCAAnimation* propertyAnim, const An
     propertyAnim->setAdditive(additive);
     propertyAnim->setFillMode(fillMode);
 
-    if (RuntimeEnabledFeatures::sharedFeatures().webAnimationsCSSIntegrationEnabled())
-        propertyAnim->setTimingFunction(anim->timingFunction());
+    if (RuntimeEnabledFeatures::sharedFeatures().webAnimationsCSSIntegrationEnabled()) {
+        // FIXME: https://bugs.webkit.org/show_bug.cgi?id=215918
+        // A CSS Transition is the only scenario where Animation::property() will have
+        // its mode set to SingleProperty. In this case, we don't set the animation-wide
+        // timing function to work around a Core Animation limitation.
+        if (anim->property().mode != Animation::TransitionMode::SingleProperty)
+            propertyAnim->setTimingFunction(anim->timingFunction());
+    }
 }
 
 const TimingFunction& GraphicsLayerCA::timingFunctionForAnimationValue(const AnimationValue& animValue, const Animation& anim)
 {
     if (RuntimeEnabledFeatures::sharedFeatures().webAnimationsCSSIntegrationEnabled()) {
+        if (anim.property().mode == Animation::TransitionMode::SingleProperty && anim.timingFunction()) {
+            // FIXME: https://bugs.webkit.org/show_bug.cgi?id=215918
+            // A CSS Transition is the only scenario where Animation::property() will have
+            // its mode set to SingleProperty. In this case, we chose not to set set the
+            // animation-wide timing function, so we set it on the single keyframe interval
+            // to work around a Core Animation limitation.
+            return *anim.timingFunction();
+        }
         if (animValue.timingFunction())
             return *animValue.timingFunction();
-        else if (anim.defaultTimingFunctionForKeyframes())
+        if (anim.defaultTimingFunctionForKeyframes())
             return *anim.defaultTimingFunctionForKeyframes();
         return LinearTimingFunction::sharedLinearTimingFunction();
     }