Attachment 339582 Details for Bug 185299 – Patch

[patch] Patch

bug-185299-20180504211605.patch (text/plain), 7.22 KB, created by Antoine Quint on 2018-05-04 12:16:06 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Antoine Quint

Created: 2018-05-04 12:16:06 PDT

Size: 7.22 KB

patch

obsolete

>Subversion Revision: 231223
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 68751546a07533ee398537506088ee2cba6beb67..03d74d865ab55a7319f7351a2f4ed4e0782e784b 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,28 @@
>+2018-05-04  Antoine Quint  <graouts@apple.com>
>+
>+        REGRESSION (r230574): Interrupted hardware transitions don't behave correctly
>+        https://bugs.webkit.org/show_bug.cgi?id=185299
>+        <rdar://problem/39630230>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        In r230574, the fix for webkit.org/b/184518, we changed the processing order in GraphicsLayerCA::updateAnimations() to first
>+        process m_uncomittedAnimations and then m_animationsToProcess, so we are guaranteed animations exist before we attempt to pause
>+        or seek them. This broke interrupting and resuming hardware animations (such as an interrupted CSS Transition or an animation
>+        running in a non-visible tab) since a pause operation recorded _before_ an animation was added would be paused anyway since
>+        the animation was now first added, and then paused. The fix is simply to clear any pending AnimationProcessingAction for a
>+        newly-uncommitted animation.
>+
>+        No new tests since there is no existing mechanism that allows to test this behavior since this is about the internal state of
>+        an animation performed by Core Animation.
>+
>+        * platform/graphics/ca/GraphicsLayerCA.cpp:
>+        (WebCore::GraphicsLayerCA::createAnimationFromKeyframes):
>+        (WebCore::GraphicsLayerCA::appendToUncommittedAnimations):
>+        (WebCore::GraphicsLayerCA::createTransformAnimationsFromKeyframes):
>+        * platform/graphics/ca/GraphicsLayerCA.h:
>+        (WebCore::GraphicsLayerCA::LayerPropertyAnimation::LayerPropertyAnimation):
>+
> 2018-05-01  Yusuke Suzuki  <utatane.tea@gmail.com>
> 
>         Use default std::optional if it is provided
>diff --git a/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp b/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp
>index 121001a7c0cc51a1084c4fe6703400d4cd785eb4..42dc7b7e7f140a1ba7479b5a35f43b571835c2d9 100644
>--- a/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp
>+++ b/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp
>@@ -3015,7 +3015,7 @@ bool GraphicsLayerCA::createAnimationFromKeyframes(const KeyframeValueList& valu
>     if (!valuesOK)
>         return false;
> 
>-    m_uncomittedAnimations.append(LayerPropertyAnimation(caAnimation.releaseNonNull(), animationName, valueList.property(), animationIndex, 0, timeOffset));
>+    appendToUncommittedAnimations(LayerPropertyAnimation(caAnimation.releaseNonNull(), animationName, valueList.property(), animationIndex, 0, timeOffset));
> 
>     return true;
> }
>@@ -3042,7 +3042,7 @@ bool GraphicsLayerCA::appendToUncommittedAnimations(const KeyframeValueList& val
>     if (!validMatrices)
>         return false;
> 
>-    m_uncomittedAnimations.append(LayerPropertyAnimation(caAnimation.releaseNonNull(), animationName, valueList.property(), animationIndex, 0, timeOffset));
>+    appendToUncommittedAnimations(LayerPropertyAnimation(caAnimation.releaseNonNull(), animationName, valueList.property(), animationIndex, 0, timeOffset));
>     return true;
> }
> 
>@@ -3104,12 +3104,21 @@ bool GraphicsLayerCA::appendToUncommittedAnimations(const KeyframeValueList& val
>         
>         ASSERT(valuesOK);
> 
>-        m_uncomittedAnimations.append(LayerPropertyAnimation(caAnimation.releaseNonNull(), animationName, valueList.property(), animationIndex, internalFilterPropertyIndex, timeOffset));
>+        appendToUncommittedAnimations(LayerPropertyAnimation(caAnimation.releaseNonNull(), animationName, valueList.property(), animationIndex, internalFilterPropertyIndex, timeOffset));
>     }
> 
>     return true;
> }
> 
>+void GraphicsLayerCA::appendToUncommittedAnimations(LayerPropertyAnimation&& animation)
>+{
>+    // Since we're adding a new animation, make sure we clear any pending AnimationProcessingAction for this animation
>+    // as these are applied after we've committed new animations.
>+    m_animationsToProcess.remove(animation.m_name);
>+
>+    m_uncomittedAnimations.append(WTFMove(animation));
>+}
>+
> bool GraphicsLayerCA::createFilterAnimationsFromKeyframes(const KeyframeValueList& valueList, const Animation* animation, const String& animationName, Seconds timeOffset)
> {
> #if ENABLE(FILTERS_LEVEL_2)
>diff --git a/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.h b/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.h
>index 5011ccd71840bd0a6f32530d2c3aad31b9a8675c..ee41cd43d468ad62bc4130ca9b74b62027f29de9 100644
>--- a/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.h
>+++ b/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.h
>@@ -459,8 +459,29 @@ private:
>         moveOrCopyAnimations(Copy, fromLayer, toLayer);
>     }
> 
>+    // This represents the animation of a single property. There may be multiple transform animations for
>+    // a single transition or keyframe animation, so index is used to distinguish these.
>+    struct LayerPropertyAnimation {
>+        LayerPropertyAnimation(Ref<PlatformCAAnimation>&& caAnimation, const String& animationName, AnimatedPropertyID property, int index, int subIndex, Seconds timeOffset)
>+            : m_animation(WTFMove(caAnimation))
>+            , m_name(animationName)
>+            , m_property(property)
>+            , m_index(index)
>+            , m_subIndex(subIndex)
>+            , m_timeOffset(timeOffset)
>+        { }
>+
>+        RefPtr<PlatformCAAnimation> m_animation;
>+        String m_name;
>+        AnimatedPropertyID m_property;
>+        int m_index;
>+        int m_subIndex;
>+        Seconds m_timeOffset;
>+    };
>+
>     bool appendToUncommittedAnimations(const KeyframeValueList&, const TransformOperations*, const Animation*, const String& animationName, const FloatSize& boxSize, int animationIndex, Seconds timeOffset, bool isMatrixAnimation);
>     bool appendToUncommittedAnimations(const KeyframeValueList&, const FilterOperation*, const Animation*, const String& animationName, int animationIndex, Seconds timeOffset);
>+    void appendToUncommittedAnimations(LayerPropertyAnimation&&);
> 
>     enum LayerChange : uint64_t {
>         NoChange                                = 0,
>@@ -572,26 +593,6 @@ private:
>     RetainPtr<CGImageRef> m_uncorrectedContentsImage;
>     RetainPtr<CGImageRef> m_pendingContentsImage;
>     
>-    // This represents the animation of a single property. There may be multiple transform animations for
>-    // a single transition or keyframe animation, so index is used to distinguish these.
>-    struct LayerPropertyAnimation {
>-        LayerPropertyAnimation(Ref<PlatformCAAnimation>&& caAnimation, const String& animationName, AnimatedPropertyID property, int index, int subIndex, Seconds timeOffset)
>-            : m_animation(WTFMove(caAnimation))
>-            , m_name(animationName)
>-            , m_property(property)
>-            , m_index(index)
>-            , m_subIndex(subIndex)
>-            , m_timeOffset(timeOffset)
>-        { }
>-
>-        RefPtr<PlatformCAAnimation> m_animation;
>-        String m_name;
>-        AnimatedPropertyID m_property;
>-        int m_index;
>-        int m_subIndex;
>-        Seconds m_timeOffset;
>-    };
>-    
>     // Uncommitted transitions and animations.
>     Vector<LayerPropertyAnimation> m_uncomittedAnimations;
>

Actions: View | Formatted Diff | Diff

Attachments on bug 185299: 339541 | 339582 | 340306