Changeset 273656 in webkit

Timestamp:

Mar 1, 2021, 10:47:02 AM (6 years ago)

Author:

graouts@webkit.org

Message:

REGRESSION(r272004): transform transition with delay doesn't behave correctly
​https://bugs.webkit.org/show_bug.cgi?id=222545
<rdar://problem/74865413>

Reviewed by Dean Jackson.

Source/WebCore:

To support accelerated animations of individual transform properties, we introduced the notion of
non-interpolating animations to apply the underlying value for a given property before applying
the actual animations for this property with additivity set to true.

These non-interpolating animations were meant to last between the time at which animations were
committed and the effective start of the first animation for that property, accounting for any
delay.

However, we neglected to handle the case where that first animation had a fill mode that would
make it fill backwards, such as CSS Transitions. In that situation, the animation would have
its first keyframe applied on top of the underlying value, effectively applying the underlying
value twice with additivity.

We now only add these non-interpolating animations if the first animation has a delay and does
not fill backwards.

Test: webanimations/transform-transition-with-delay-on-forced-layer-with-transform.html

• platform/graphics/ca/GraphicsLayerCA.cpp:

(WebCore::GraphicsLayerCA::updateAnimations):

LayoutTests:

Add a new test where an element with a non-identity transform starts a transform transition with a
long delay. Prior to this patch, this test failed because, while in the delay phase, the transition
would mean the underlying transform was applied twice: once by the non-interpolating animation
generated for the underlying "transform" value, and once by the first keyframe of the transition
since it fills backwards.

• webanimations/transform-transition-with-delay-on-forced-layer-with-transform-expected.html: Added.
• webanimations/transform-transition-with-delay-on-forced-layer-with-transform.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/webanimations/transform-transition-with-delay-on-forced-layer-with-transform-expected.html (added)
• LayoutTests/webanimations/transform-transition-with-delay-on-forced-layer-with-transform.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp (modified) (3 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2021-03-01  Antoine Quint  <graouts@webkit.org>
+
+        REGRESSION(r272004): transform transition with delay doesn't behave correctly
+        https://bugs.webkit.org/show_bug.cgi?id=222545
+        <rdar://problem/74865413>
+
+        Reviewed by Dean Jackson.
+
+        Add a new test where an element with a non-identity transform starts a transform transition with a
+        long delay. Prior to this patch, this test failed because, while in the delay phase, the transition
+        would mean the underlying transform was applied twice: once by the non-interpolating animation
+        generated for the underlying "transform" value, and once by the first keyframe of the transition
+        since it fills backwards.
+
+        * webanimations/transform-transition-with-delay-on-forced-layer-with-transform-expected.html: Added.
+        * webanimations/transform-transition-with-delay-on-forced-layer-with-transform.html: Added.
+
 2021-03-01  Chris Gambrell  <cgambrell@apple.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2021-03-01  Antoine Quint  <graouts@webkit.org>
+
+        REGRESSION(r272004): transform transition with delay doesn't behave correctly
+        https://bugs.webkit.org/show_bug.cgi?id=222545
+        <rdar://problem/74865413>
+
+        Reviewed by Dean Jackson.
+
+        To support accelerated animations of individual transform properties, we introduced the notion of
+        non-interpolating animations to apply the underlying value for a given property before applying
+        the actual animations for this property with additivity set to true.
+
+        These non-interpolating animations were meant to last between the time at which animations were
+        committed and the effective start of the first animation for that property, accounting for any
+        delay.
+
+        However, we neglected to handle the case where that first animation had a fill mode that would
+        make it fill backwards, such as CSS Transitions. In that situation, the animation would have
+        its first keyframe applied on top of the underlying value, effectively applying the underlying
+        value twice with additivity.
+
+        We now only add these non-interpolating animations if the first animation has a delay and does
+        not fill backwards.
+
+        Test: webanimations/transform-transition-with-delay-on-forced-layer-with-transform.html
+
+        * platform/graphics/ca/GraphicsLayerCA.cpp:
+        (WebCore::GraphicsLayerCA::updateAnimations):
+
 2021-03-01  Chris Dumez  <cdumez@apple.com>
 

trunk/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp

@@ -3064 +3064 @@
             }
 
-            Seconds earliestBeginTime = 0_s;
+            LayerPropertyAnimation* earliestAnimation = nullptr;
             for (auto* animation : animations) {
                 if (auto beginTime = animation->computedBeginTime()) {
-                    if (!earliestBeginTime || earliestBeginTime > *beginTime)
-                        earliestBeginTime = *beginTime;
+                    if (!earliestAnimation || *earliestAnimation->computedBeginTime() > *beginTime)
+                        earliestAnimation = animation;
                 }
             }
 
-            if (earliestBeginTime)
-                earliestBeginTime += animationGroupBeginTime;
-
             Vector<RefPtr<PlatformCAAnimation>> caAnimations;
-            if (earliestBeginTime > currentTime) {
-                if (auto* baseValueTransformAnimation = makeBaseValueTransformAnimation(property, TransformationMatrixSource::AskClient, earliestBeginTime)) {
-                    prepareAnimationForAddition(*baseValueTransformAnimation);
-                    caAnimations.append(baseValueTransformAnimation->m_animation);
+
+            // If we have an animation with an explicit begin time that does not fill backwards and starts with a delay,
+            // we must create a non-interpolating animation to set the current value for this transform-related property
+            // until that animation begins.
+            if (earliestAnimation) {
+                auto fillMode = earliestAnimation->m_animation->fillMode();
+                if (fillMode != PlatformCAAnimation::Backwards && fillMode != PlatformCAAnimation::Both) {
+                    Seconds earliestBeginTime = *earliestAnimation->computedBeginTime() + animationGroupBeginTime;
+                    if (earliestBeginTime > currentTime) {
+                        if (auto* baseValueTransformAnimation = makeBaseValueTransformAnimation(property, TransformationMatrixSource::AskClient, earliestBeginTime)) {
+                            prepareAnimationForAddition(*baseValueTransformAnimation);
+                            caAnimations.append(baseValueTransformAnimation->m_animation);
+                        }
+                    }
                 }
             }
@@ -3102 +3109 @@
             }
 
-            Seconds earliestBeginTime = 0_s;
+            LayerPropertyAnimation* earliestAnimation = nullptr;
             Vector<RefPtr<PlatformCAAnimation>> caAnimations;
             for (auto* animation : WTF::makeReversedRange(animations)) {
                 if (auto beginTime = animation->computedBeginTime()) {
-                    if (!earliestBeginTime || earliestBeginTime > *beginTime)
-                        earliestBeginTime = *beginTime;
+                    if (!earliestAnimation || *earliestAnimation->computedBeginTime() > *beginTime)
+                        earliestAnimation = animation;
                 }
                 prepareAnimationForAddition(*animation);
@@ -3113 +3120 @@
             }
 
-            if (earliestBeginTime)
-                earliestBeginTime += animationGroupBeginTime;
-
-            if (earliestBeginTime > currentTime) {
-                if (auto* baseValueTransformAnimation = makeBaseValueTransformAnimation(property, TransformationMatrixSource::AskClient, earliestBeginTime)) {
-                    prepareAnimationForAddition(*baseValueTransformAnimation);
-                    caAnimations.append(baseValueTransformAnimation->m_animation);
+            // If we have an animation with an explicit begin time that does not fill backwards and starts with a delay,
+            // we must create a non-interpolating animation to set the current value for this transform-related property
+            // until that animation begins.
+            if (earliestAnimation) {
+                auto fillMode = earliestAnimation->m_animation->fillMode();
+                if (fillMode != PlatformCAAnimation::Backwards && fillMode != PlatformCAAnimation::Both) {
+                    Seconds earliestBeginTime = *earliestAnimation->computedBeginTime() + animationGroupBeginTime;
+                    if (earliestBeginTime > currentTime) {
+                        if (auto* baseValueTransformAnimation = makeBaseValueTransformAnimation(property, TransformationMatrixSource::AskClient, earliestBeginTime)) {
+                            prepareAnimationForAddition(*baseValueTransformAnimation);
+                            caAnimations.append(baseValueTransformAnimation->m_animation);
+                        }
+                    }
                 }
             }