Title: [268746] trunk
Revision
268746
Author
grao...@webkit.org
Date
2020-10-20 10:34:38 -0700 (Tue, 20 Oct 2020)

Log Message

REGRESSION(r268615): some accelerated transform tests are failing
https://bugs.webkit.org/show_bug.cgi?id=217851
<rdar://problem/70394402>

Reviewed by Dean Jackson.

Source/WebCore:

When we added support for accelerated individual transform properties animations, we added
the notion of base transform animations which are used to set the base value of any
transform-related property that is not animated.

Those animations were defined to start as early as possible, assuming that a very small value
after 0s was as early as possible. However, it's possible that other animations start with a
negative time if they have a delay or are seeked, if the value returned by CACurrentMediaTime()
is smaller than that delay. This means that if the machine had been booted for less time than
an accelerated animation's delay, the base transform animation wouldn't overlap.

We now ensure that those base transform animations start as early as the earliest animation
that is being committed in a call to GraphicsLayerCA::updateAnimations().

* platform/graphics/ca/GraphicsLayerCA.cpp:
(WebCore::GraphicsLayerCA::updateAnimations):
(WebCore::GraphicsLayerCA::setAnimationOnLayer):
* platform/graphics/ca/GraphicsLayerCA.h:
(WebCore::GraphicsLayerCA::LayerPropertyAnimation::computedBeginTime const):

LayoutTests:

Remove flaky epectations for affected tests and skip tests on Windows where failures remain.

* TestExpectations:
* platform/mac/TestExpectations:
* platform/win/TestExpectations:

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (268745 => 268746)


--- trunk/LayoutTests/ChangeLog	2020-10-20 17:24:38 UTC (rev 268745)
+++ trunk/LayoutTests/ChangeLog	2020-10-20 17:34:38 UTC (rev 268746)
@@ -1,3 +1,17 @@
+2020-10-20  Antoine Quint  <grao...@webkit.org>
+
+        REGRESSION(r268615): some accelerated transform tests are failing
+        https://bugs.webkit.org/show_bug.cgi?id=217851
+        <rdar://problem/70394402>
+
+        Reviewed by Dean Jackson.
+
+        Remove flaky epectations for affected tests and skip tests on Windows where failures remain.
+
+        * TestExpectations:
+        * platform/mac/TestExpectations:
+        * platform/win/TestExpectations:
+
 2020-10-20  Rob Buis  <rb...@igalia.com>
 
         Introduce Selection specific mixin of GlobalEventHandlers

Modified: trunk/LayoutTests/TestExpectations (268745 => 268746)


--- trunk/LayoutTests/TestExpectations	2020-10-20 17:24:38 UTC (rev 268745)
+++ trunk/LayoutTests/TestExpectations	2020-10-20 17:34:38 UTC (rev 268746)
@@ -4518,10 +4518,3 @@
 
 fast/layoutformattingcontext/ [ ImageOnlyFailure ]
 webkit.org/b/217054 fast/layoutformattingcontext/horizontal-sizing-with-trailing-letter-spacing.html [ Skip ]
-
-webkit.org/b/217851 transitions/interrupted-transition-hardware.html [ Pass Failure ]
-webkit.org/b/217851 webanimations/accelerated-transform-related-animation-property-order.html [ Pass Failure ]
-webkit.org/b/217851 webanimations/accelerated-translate-animation-additional-animation-added-in-flight.html [ Pass Failure ]
-webkit.org/b/217851 webanimations/accelerated-translate-animation-underlying-transform-changed-in-flight.html [ Pass Failure ]
-webkit.org/b/217851 webanimations/accelerated-translate-animation-with-transform.html [ Pass Failure ]
-webkit.org/b/217851 webanimations/accelerated-translate-animation.html [ Pass Failure ]

Modified: trunk/LayoutTests/platform/mac/TestExpectations (268745 => 268746)


--- trunk/LayoutTests/platform/mac/TestExpectations	2020-10-20 17:24:38 UTC (rev 268745)
+++ trunk/LayoutTests/platform/mac/TestExpectations	2020-10-20 17:34:38 UTC (rev 268746)
@@ -2246,15 +2246,6 @@
 
 webkit.org/b/217620 inspector/audit/basic-async.htm [ Pass Timeout ]
 
-# These are failing on Mojave due to an error in the way CA animations are applied.
-# https://bugs.webkit.org/show_bug.cgi?id=217842
-[ Mojave ] transitions/interrupted-transition-hardware.html [ Pass Failure ]
-[ Mojave ] webanimations/accelerated-transform-related-animation-property-order.html [ Pass Failure ]
-[ Mojave ] webanimations/accelerated-translate-animation-additional-animation-added-in-flight.html [ Pass Failure ]
-[ Mojave ] webanimations/accelerated-translate-animation-underlying-transform-changed-in-flight.html [ Pass Failure ]
-[ Mojave ] webanimations/accelerated-translate-animation-with-transform.html [ Pass Failure ]
-[ Mojave ] webanimations/accelerated-translate-animation.html [ Pass Failure ]
-
 webkit.org/b/217931 imported/w3c/web-platform-tests/html/semantics/scripting-1/the-script-element/moving-between-documents/before-prepare-createHTMLDocument-success-empty-src-module.html [ Pass Failure ]
 
 imported/w3c/web-platform-tests/speech-api/SpeechSynthesis-speak-twice.html [ Skip ]

Modified: trunk/LayoutTests/platform/win/TestExpectations (268745 => 268746)


--- trunk/LayoutTests/platform/win/TestExpectations	2020-10-20 17:24:38 UTC (rev 268745)
+++ trunk/LayoutTests/platform/win/TestExpectations	2020-10-20 17:34:38 UTC (rev 268746)
@@ -4586,8 +4586,12 @@
 webkit.org/b/217812 transforms/2d/rotate-change-composited.html [ Skip ]
 webkit.org/b/217812 transforms/2d/translate-change-composited.html [ Skip ]
 webkit.org/b/217812 transforms/2d/rotate-composited.html [ Skip ]
+webkit.org/b/217812 webanimations/accelerated-transform-related-animation-property-order.html [ Skip ]
+webkit.org/b/217812 webanimations/accelerated-translate-animation-additional-animation-added-in-flight.html [ Skip ]
+webkit.org/b/217812 webanimations/accelerated-translate-animation-underlying-transform-changed-in-flight.html [ Skip ]
+webkit.org/b/217812 webanimations/accelerated-translate-animation-with-transform.html [ Skip ]
+webkit.org/b/217812 webanimations/accelerated-translate-animation.html [ Skip ]
 
 webkit.org/b/217922 [ Release ] animations/additive-transform-animations.html [ ImageOnlyFailure ]
 webkit.org/b/217922 animations/needs-layout.html [ ImageOnlyFailure ]
 webkit.org/b/217922 fast/animation/animation-mixed-transform-crash.html [ ImageOnlyFailure ]
-

Modified: trunk/Source/WebCore/ChangeLog (268745 => 268746)


--- trunk/Source/WebCore/ChangeLog	2020-10-20 17:24:38 UTC (rev 268745)
+++ trunk/Source/WebCore/ChangeLog	2020-10-20 17:34:38 UTC (rev 268746)
@@ -1,3 +1,30 @@
+2020-10-20  Antoine Quint  <grao...@webkit.org>
+
+        REGRESSION(r268615): some accelerated transform tests are failing
+        https://bugs.webkit.org/show_bug.cgi?id=217851
+        <rdar://problem/70394402>
+
+        Reviewed by Dean Jackson.
+
+        When we added support for accelerated individual transform properties animations, we added
+        the notion of base transform animations which are used to set the base value of any
+        transform-related property that is not animated.
+
+        Those animations were defined to start as early as possible, assuming that a very small value
+        after 0s was as early as possible. However, it's possible that other animations start with a
+        negative time if they have a delay or are seeked, if the value returned by CACurrentMediaTime()
+        is smaller than that delay. This means that if the machine had been booted for less time than
+        an accelerated animation's delay, the base transform animation wouldn't overlap.
+
+        We now ensure that those base transform animations start as early as the earliest animation
+        that is being committed in a call to GraphicsLayerCA::updateAnimations().
+
+        * platform/graphics/ca/GraphicsLayerCA.cpp:
+        (WebCore::GraphicsLayerCA::updateAnimations):
+        (WebCore::GraphicsLayerCA::setAnimationOnLayer):
+        * platform/graphics/ca/GraphicsLayerCA.h:
+        (WebCore::GraphicsLayerCA::LayerPropertyAnimation::computedBeginTime const):
+
 2020-10-20  Rob Buis  <rb...@igalia.com>
 
         Introduce Selection specific mixin of GlobalEventHandlers

Modified: trunk/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp (268745 => 268746)


--- trunk/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp	2020-10-20 17:24:38 UTC (rev 268745)
+++ trunk/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp	2020-10-20 17:34:38 UTC (rev 268746)
@@ -2861,6 +2861,25 @@
 
 void GraphicsLayerCA::updateAnimations()
 {
+    auto baseTransformAnimationBeginTime = 1_s;
+    auto currentTime = Seconds(CACurrentMediaTime());
+    auto updateBeginTimes = [&](LayerPropertyAnimation& animation)
+    {
+        if (animation.m_pendingRemoval)
+            return;
+
+        // In case we have an offset, and we haven't set an explicit begin time previously,
+        // we need to record the beginTime now.
+        if (animation.m_timeOffset && !animation.m_beginTime)
+            animation.m_beginTime = currentTime;
+
+        // Now check if we have a resolved begin time and ensure the begin time we'll use for
+        // base transform animations matches the smallest known begin time to guarantee that
+        // such animations can combine with other explicit transform animations correctly.
+        if (auto computedBeginTime = animation.computedBeginTime())
+            baseTransformAnimationBeginTime = std::min(baseTransformAnimationBeginTime, *computedBeginTime);
+    };
+
     enum class Additive { Yes, No };
     auto addAnimation = [&](LayerPropertyAnimation& animation, Additive additive = Additive::Yes) {
         animation.m_animation->setAdditive(additive == Additive::Yes);
@@ -2890,7 +2909,7 @@
         auto animation = LayerPropertyAnimation(WTFMove(caAnimation), "base-transform-" + createCanonicalUUIDString(), property, 0, 0, 0_s);
         // To ensure the base value transform is applied along with all the interpolating animations, we set it to have started
         // as early as possible, which combined with the infinite duration ensures it's current for any given CA media time.
-        animation.m_beginTime = Seconds::fromNanoseconds(1);
+        animation.m_beginTime = baseTransformAnimationBeginTime;
 
         // Additivity will depend on the source of the matrix, if it was explicitly provided as an identity matrix, it
         // is the initial base value transform animation and must override the current transform value for this layer.
@@ -2900,8 +2919,10 @@
         m_baseValueTransformAnimations.append(WTFMove(animation));
     };
 
-    // Remove all running CA animations.
+    // Iterate through all animations to update each animation's begin time, if necessary,
+    // compute the base transform animation begin times and remove all running CA animations.
     for (auto& animation : m_animations) {
+        updateBeginTimes(animation);
         if (animation.m_playState == PlayState::Playing || animation.m_playState == PlayState::Paused)
             removeCAAnimationFromLayer(animation);
     }
@@ -3021,18 +3042,8 @@
 
     auto& caAnim = *animation.m_animation;
 
-    if (animation.m_timeOffset) {
-        // In case we have an offset, we need to record the beginTime now since we have to pass in an explicit
-        // value in the first place.
-        if (!animation.m_beginTime)
-            animation.m_beginTime = Seconds(CACurrentMediaTime());
-        caAnim.setBeginTime((animation.m_beginTime - animation.m_timeOffset).seconds());
-    } else if (animation.m_beginTime) {
-        // If we already have a begin time, then we already started in the past and must ensure we use that same
-        // begin time. Any other case will get use the CA transaction's time as its begin time and will be recorded
-        // in platformCALayerAnimationStarted().
-        caAnim.setBeginTime(animation.m_beginTime.seconds());
-    }
+    if (auto beginTime = animation.computedBeginTime())
+        caAnim.setBeginTime(beginTime->seconds());
 
     String animationID = animation.animationIdentifier();
 

Modified: trunk/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.h (268745 => 268746)


--- trunk/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.h	2020-10-20 17:24:38 UTC (rev 268745)
+++ trunk/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.h	2020-10-20 17:34:38 UTC (rev 268746)
@@ -30,6 +30,7 @@
 #include "PlatformCALayer.h"
 #include "PlatformCALayerClient.h"
 #include <wtf/HashMap.h>
+#include <wtf/Optional.h>
 #include <wtf/RetainPtr.h>
 #include <wtf/text/StringHash.h>
 
@@ -462,6 +463,12 @@
         { }
 
         String animationIdentifier() const { return makeString(m_name, '_', static_cast<unsigned>(m_property), '_', m_index, '_', m_subIndex); }
+        Optional<Seconds> computedBeginTime() const
+        {
+            if (m_beginTime)
+                return m_beginTime - m_timeOffset;
+            return WTF::nullopt;
+        }
 
         RefPtr<PlatformCAAnimation> m_animation;
         String m_name;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to