Changeset 268874 in webkit

Timestamp:

Oct 22, 2020, 11:26:31 AM (6 years ago)

Author:

Alan Coon

Message:

Revert r268746. rdar://problem/70578639

Location:

branches/safari-611.1.4-branch

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/TestExpectations (modified) (1 diff)
• LayoutTests/platform/mac/TestExpectations (modified) (1 diff)
• LayoutTests/platform/win/TestExpectations (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp (modified) (4 diffs)
• Source/WebCore/platform/graphics/ca/GraphicsLayerCA.h (modified) (2 diffs)

branches/safari-611.1.4-branch/LayoutTests/ChangeLog

@@ +1 @@
+2020-10-22  Alan Coon  <alancoon@apple.com>
+
+        Revert r268746. rdar://problem/70578639
+
 2020-10-22  Alan Coon  <alancoon@apple.com>
 

branches/safari-611.1.4-branch/LayoutTests/TestExpectations

@@ -4519 +4519 @@
 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 ]

branches/safari-611.1.4-branch/LayoutTests/platform/mac/TestExpectations

@@ -2247 +2247 @@
 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 ]

branches/safari-611.1.4-branch/LayoutTests/platform/win/TestExpectations

@@ -4587 +4587 @@
 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 ]

branches/safari-611.1.4-branch/Source/WebCore/ChangeLog

@@ +1 @@
+2020-10-22  Alan Coon  <alancoon@apple.com>
+
+        Revert r268746. rdar://problem/70578639
+
 2020-10-22  Alan Coon  <alancoon@apple.com>
 

branches/safari-611.1.4-branch/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp

@@ -2862 +2862 @@
 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) {
@@ -2910 +2891 @@
         // 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 = baseTransformAnimationBeginTime;
+        animation.m_beginTime = Seconds::fromNanoseconds(1);
 
         // Additivity will depend on the source of the matrix, if it was explicitly provided as an identity matrix, it
@@ -2920 +2901 @@
     };
 
-    // 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.
+    // 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);
@@ -3043 +3022 @@
     auto& caAnim = *animation.m_animation;
 
-    if (auto beginTime = animation.computedBeginTime())
-        caAnim.setBeginTime(beginTime->seconds());
+    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());
+    }
 
     String animationID = animation.animationIdentifier();

branches/safari-611.1.4-branch/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.h

@@ -31 +31 @@
 #include "PlatformCALayerClient.h"
 #include <wtf/HashMap.h>
-#include <wtf/Optional.h>
 #include <wtf/RetainPtr.h>
 #include <wtf/text/StringHash.h>
@@ -464 +463 @@
 
         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;