Title: [258394] releases/WebKitGTK/webkit-2.26/Source/WebCore
Revision
258394
Author
ape...@igalia.com
Date
2020-03-13 05:01:23 -0700 (Fri, 13 Mar 2020)

Log Message

Merge r254680 - Do not detect the stopped animations in Nicosia::Animation to avoid flashback
https://bugs.webkit.org/show_bug.cgi?id=206280

Patch by Tomoki Imai <tomoki.i...@sony.com> on 2020-01-16
Reviewed by Carlos Garcia Campos.

This fixes the animation flashback issue found in https://webkit.org/blog-files/3d-transforms/morphing-cubes.html.
The flashback was caused by using the old layer transform matrix saved when the animation has been started.

The root cause is an inconsistency of animation state in Nicosia::Animation and CoordinatedGraphicsLayer.
For Nicosia::Animation, ThreadedCompositor increases MonitonicTime for animation every frame, and calls Nicosia::Animation::apply.
For CoordinatedGraphicsLayer, CSSAnimationController updates animations list and if the animation has been finished it updates CSS value.
There is a chance to use old layer state while the Nicosia::Animation stopped, but CoordinatedGraphicsLayer still obtains old CSS value and animations.

In this patch, all the Nicosia::Animation is considered to have "AnimationFillMode::Forwards" or "AnimationFillMode::Both",
which means they are active and use the last position when the animation is stopped.
Stopping and removing animations should be only done by CSSAnimationController and CoordinatedGraphicsScene
as they can remove the animation from the list and update the CSS value at the same time.

Mac implementation GraphicsLayerCA has a similar logic, it replaces AnimationFillMode with Forwards or Both.
https://trac.webkit.org/browser/webkit/trunk/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp?rev=254502#L3248

Tested manually with https://webkit.org/blog-files/3d-transforms/morphing-cubes.html

* platform/graphics/nicosia/NicosiaAnimation.cpp:
(Nicosia::Animation::apply): Return the last value for stopped animations to avoid flickering
(Nicosia::Animation::isActive const): Removed. It should always return true because
all the animations are considered as fillsForwards in Nicosia::Animation to avoid flashback.
(Nicosia::Animations::hasActiveAnimationsOfType const): Remove isActive check.
(Nicosia::Animations::getActiveAnimations() const): Removed. It returns whole animations list because all the animations are active.
* platform/graphics/nicosia/NicosiaAnimation.h:
* platform/graphics/texmap/coordinated/CoordinatedGraphicsLayer.cpp:
(WebCore::CoordinatedGraphicsLayer::flushCompositingStateForThisLayerOnly):

Modified Paths

Diff

Modified: releases/WebKitGTK/webkit-2.26/Source/WebCore/ChangeLog (258393 => 258394)


--- releases/WebKitGTK/webkit-2.26/Source/WebCore/ChangeLog	2020-03-13 10:37:53 UTC (rev 258393)
+++ releases/WebKitGTK/webkit-2.26/Source/WebCore/ChangeLog	2020-03-13 12:01:23 UTC (rev 258394)
@@ -1,3 +1,38 @@
+2020-01-16  Tomoki Imai  <tomoki.i...@sony.com>
+
+        Do not detect the stopped animations in Nicosia::Animation to avoid flashback
+        https://bugs.webkit.org/show_bug.cgi?id=206280
+
+        Reviewed by Carlos Garcia Campos.
+
+        This fixes the animation flashback issue found in https://webkit.org/blog-files/3d-transforms/morphing-cubes.html.
+        The flashback was caused by using the old layer transform matrix saved when the animation has been started.
+
+        The root cause is an inconsistency of animation state in Nicosia::Animation and CoordinatedGraphicsLayer.
+        For Nicosia::Animation, ThreadedCompositor increases MonitonicTime for animation every frame, and calls Nicosia::Animation::apply.
+        For CoordinatedGraphicsLayer, CSSAnimationController updates animations list and if the animation has been finished it updates CSS value.
+        There is a chance to use old layer state while the Nicosia::Animation stopped, but CoordinatedGraphicsLayer still obtains old CSS value and animations.
+
+        In this patch, all the Nicosia::Animation is considered to have "AnimationFillMode::Forwards" or "AnimationFillMode::Both",
+        which means they are active and use the last position when the animation is stopped.
+        Stopping and removing animations should be only done by CSSAnimationController and CoordinatedGraphicsScene
+        as they can remove the animation from the list and update the CSS value at the same time.
+
+        Mac implementation GraphicsLayerCA has a similar logic, it replaces AnimationFillMode with Forwards or Both.
+        https://trac.webkit.org/browser/webkit/trunk/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp?rev=254502#L3248
+
+        Tested manually with https://webkit.org/blog-files/3d-transforms/morphing-cubes.html
+
+        * platform/graphics/nicosia/NicosiaAnimation.cpp:
+        (Nicosia::Animation::apply): Return the last value for stopped animations to avoid flickering
+        (Nicosia::Animation::isActive const): Removed. It should always return true because
+        all the animations are considered as fillsForwards in Nicosia::Animation to avoid flashback.
+        (Nicosia::Animations::hasActiveAnimationsOfType const): Remove isActive check.
+        (Nicosia::Animations::getActiveAnimations() const): Removed. It returns whole animations list because all the animations are active.
+        * platform/graphics/nicosia/NicosiaAnimation.h:
+        * platform/graphics/texmap/coordinated/CoordinatedGraphicsLayer.cpp:
+        (WebCore::CoordinatedGraphicsLayer::flushCompositingStateForThisLayerOnly):
+
 2020-01-09  Brent Fulgham  <bfulg...@apple.com>
 
         REGRESSION (r253662): Large Data URLs are not being handled properly

Modified: releases/WebKitGTK/webkit-2.26/Source/WebCore/platform/graphics/nicosia/NicosiaAnimation.cpp (258393 => 258394)


--- releases/WebKitGTK/webkit-2.26/Source/WebCore/platform/graphics/nicosia/NicosiaAnimation.cpp	2020-03-13 10:37:53 UTC (rev 258393)
+++ releases/WebKitGTK/webkit-2.26/Source/WebCore/platform/graphics/nicosia/NicosiaAnimation.cpp	2020-03-13 12:01:23 UTC (rev 258394)
@@ -204,8 +204,8 @@
 
 void Animation::apply(ApplicationResult& applicationResults, MonotonicTime time)
 {
-    if (!isActive())
-        return;
+    // Even when m_state == AnimationState::Stopped && !m_fillsForwards, we should calculate the last value to avoid a flash.
+    // CoordinatedGraphicsScene will soon remove the stopped animation and update the value instead of this function.
 
     Seconds totalRunningTime = computeTotalRunningTime(time);
     double normalizedValue = normalizedAnimationValue(totalRunningTime.seconds(), m_duration, m_direction, m_iterationCount);
@@ -213,8 +213,7 @@
     if (m_iterationCount != WebCore::Animation::IterationCountInfinite && totalRunningTime.seconds() >= m_duration * m_iterationCount) {
         m_state = AnimationState::Stopped;
         m_pauseTime = 0_s;
-        if (m_fillsForwards)
-            normalizedValue = normalizedAnimationValueForFillsForwards(m_iterationCount, m_direction);
+        normalizedValue = normalizedAnimationValueForFillsForwards(m_iterationCount, m_direction);
     }
 
     applicationResults.hasRunningAnimations |= (m_state == AnimationState::Playing);
@@ -289,11 +288,6 @@
     return m_totalRunningTime;
 }
 
-bool Animation::isActive() const
-{
-    return m_state != AnimationState::Stopped || m_fillsForwards;
-}
-
 void Animation::applyInternal(ApplicationResult& applicationResults, const AnimationValue& from, const AnimationValue& to, float progress)
 {
     switch (m_keyframes.property()) {
@@ -371,7 +365,7 @@
 {
     return std::any_of(m_animations.begin(), m_animations.end(),
         [&type](const Animation& animation) {
-            return animation.isActive() && animation.keyframes().property() == type;
+            return animation.keyframes().property() == type;
         });
 }
 
@@ -383,14 +377,4 @@
         });
 }
 
-Animations Animations::getActiveAnimations() const
-{
-    Animations active;
-    for (auto& animation : m_animations) {
-        if (animation.isActive())
-            active.add(animation);
-    }
-    return active;
-}
-
 } // namespace Nicosia

Modified: releases/WebKitGTK/webkit-2.26/Source/WebCore/platform/graphics/nicosia/NicosiaAnimation.h (258393 => 258394)


--- releases/WebKitGTK/webkit-2.26/Source/WebCore/platform/graphics/nicosia/NicosiaAnimation.h	2020-03-13 10:37:53 UTC (rev 258393)
+++ releases/WebKitGTK/webkit-2.26/Source/WebCore/platform/graphics/nicosia/NicosiaAnimation.h	2020-03-13 12:01:23 UTC (rev 258394)
@@ -48,7 +48,6 @@
     void applyKeepingInternalState(ApplicationResult&, MonotonicTime);
     void pause(Seconds);
     void resume();
-    bool isActive() const;
 
     const String& name() const { return m_name; }
     const WebCore::KeyframeValueList& keyframes() const { return m_keyframes; }
@@ -96,7 +95,6 @@
 
     bool hasRunningAnimations() const;
     bool hasActiveAnimationsOfType(WebCore::AnimatedPropertyID type) const;
-    Animations getActiveAnimations() const;
 
 private:
     Vector<Animation> m_animations;

Modified: releases/WebKitGTK/webkit-2.26/Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsLayer.cpp (258393 => 258394)


--- releases/WebKitGTK/webkit-2.26/Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsLayer.cpp	2020-03-13 10:37:53 UTC (rev 258393)
+++ releases/WebKitGTK/webkit-2.26/Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsLayer.cpp	2020-03-13 12:01:23 UTC (rev 258394)
@@ -843,7 +843,7 @@
                 if (localDelta.filtersChanged)
                     state.filters = filters();
                 if (localDelta.animationsChanged)
-                    state.animations = m_animations.getActiveAnimations();
+                    state.animations = m_animations;
 
                 if (localDelta.childrenChanged) {
                     state.children = WTF::map(children(),
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to