If an animation's keyframes affect stacking context properties, create stacking conte...
authorsimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 28 Oct 2016 03:20:32 +0000 (03:20 +0000)
committersimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 28 Oct 2016 03:20:32 +0000 (03:20 +0000)
https://bugs.webkit.org/show_bug.cgi?id=164094

Reviewed by Dean Jackson.

Source/WebCore:

The CSS animations spec <https://drafts.csswg.org/css-animations-1/> now makes it
clear that a keyframe animation animating properties which can affect stacking context
should establish stacking context while it's running, or filling-forwards. This is part
of the "the user agent must act as if the will-change property...includes all the properties
animated by the animation" clause.

Implement by having CompositeAnimation::animate() track whether running animations should
create stacking context, replacing existing code in AnimationController::updateAnimations()
which only looked at opacity and transform. Transitions are also checked to see if they need
to trigger stacking context.

This allows for the removal of a 0.9999 hack when blending opacity.

Tests: animations/stacking-context-fill-forwards.html
       animations/stacking-context-not-fill-forwards.html
       animations/stacking-context-unchanged-while-running.html

* page/animation/AnimationController.cpp:
(WebCore::AnimationController::updateAnimations):
* page/animation/CSSPropertyAnimation.cpp:
* page/animation/CompositeAnimation.cpp:
(WebCore::CompositeAnimation::animate):
* page/animation/KeyframeAnimation.cpp:
(WebCore::KeyframeAnimation::KeyframeAnimation):
(WebCore::KeyframeAnimation::computeStackingContextImpact):
(WebCore::KeyframeAnimation::animate):
* page/animation/KeyframeAnimation.h:
* rendering/RenderLayer.cpp:
(WebCore::RenderLayer::currentTransform):
* rendering/style/WillChangeData.cpp:
(WebCore::WillChangeData::propertyCreatesStackingContext):
(WebCore::propertyCreatesStackingContext): Deleted.
* rendering/style/WillChangeData.h:

LayoutTests:

* animations/stacking-context-fill-forwards-expected.html: Added.
* animations/stacking-context-fill-forwards.html: Added.
* animations/stacking-context-not-fill-forwards-expected.html: Added.
* animations/stacking-context-not-fill-forwards.html: Added.
* animations/stacking-context-unchanged-while-running-expected.html: Added.
* animations/stacking-context-unchanged-while-running.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@208025 268f45cc-cd09-0410-ab3c-d52691b4dbfc

16 files changed:
LayoutTests/ChangeLog
LayoutTests/animations/stacking-context-fill-forwards-expected.html [new file with mode: 0644]
LayoutTests/animations/stacking-context-fill-forwards.html [new file with mode: 0644]
LayoutTests/animations/stacking-context-not-fill-forwards-expected.html [new file with mode: 0644]
LayoutTests/animations/stacking-context-not-fill-forwards.html [new file with mode: 0644]
LayoutTests/animations/stacking-context-unchanged-while-running-expected.html [new file with mode: 0644]
LayoutTests/animations/stacking-context-unchanged-while-running.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/page/animation/AnimationController.cpp
Source/WebCore/page/animation/CSSPropertyAnimation.cpp
Source/WebCore/page/animation/CompositeAnimation.cpp
Source/WebCore/page/animation/KeyframeAnimation.cpp
Source/WebCore/page/animation/KeyframeAnimation.h
Source/WebCore/rendering/RenderLayer.cpp
Source/WebCore/rendering/style/WillChangeData.cpp
Source/WebCore/rendering/style/WillChangeData.h

index f9bc233..1b73153 100644 (file)
@@ -1,3 +1,17 @@
+2016-10-27  Simon Fraser  <simon.fraser@apple.com>
+
+        If an animation's keyframes affect stacking context properties, create stacking context while the animation is running
+        https://bugs.webkit.org/show_bug.cgi?id=164094
+
+        Reviewed by Dean Jackson.
+
+        * animations/stacking-context-fill-forwards-expected.html: Added.
+        * animations/stacking-context-fill-forwards.html: Added.
+        * animations/stacking-context-not-fill-forwards-expected.html: Added.
+        * animations/stacking-context-not-fill-forwards.html: Added.
+        * animations/stacking-context-unchanged-while-running-expected.html: Added.
+        * animations/stacking-context-unchanged-while-running.html: Added.
+
 2016-10-27  Myles C. Maxfield  <mmaxfield@apple.com>
 
         [macOS] [WebGL2] Temporarily upgrade WebGL 2's internal OpenGL context from version 2.1 to 3.2
diff --git a/LayoutTests/animations/stacking-context-fill-forwards-expected.html b/LayoutTests/animations/stacking-context-fill-forwards-expected.html
new file mode 100644 (file)
index 0000000..bfde8ec
--- /dev/null
@@ -0,0 +1,42 @@
+<!DOCTYPE html>
+
+<html>
+<head>
+    <style>
+        .box {
+            position: absolute;
+            top: 0;
+            left: 0;
+            height: 100px;
+            width: 100px;
+        }
+
+        #animating {
+            background-color: orange;
+            z-index: 0;
+        }
+
+        .behind {
+            background-color: blue;
+            top: 50px;
+            left: 50px;
+            z-index: 1;
+        }
+
+        .front {
+            top: 25px;
+            left: 25px;
+            background-color: green;
+            z-index: 2;
+        }
+    </style>
+</head>
+<body>
+
+<div class="behind box"></div>
+<div id="animating" class="box">
+    <div class="front box"></div>
+</div>
+
+</body>
+</html>
diff --git a/LayoutTests/animations/stacking-context-fill-forwards.html b/LayoutTests/animations/stacking-context-fill-forwards.html
new file mode 100644 (file)
index 0000000..d944712
--- /dev/null
@@ -0,0 +1,62 @@
+<!DOCTYPE html>
+
+<html>
+<head>
+    <title>Tests that a fill-forwards animation causes stacking context when finished.</title>
+    <style>
+        .box {
+            position: absolute;
+            top: 0;
+            left: 0;
+            height: 100px;
+            width: 100px;
+        }
+
+        #animating {
+            animation: move 0.1s 1 forwards;
+            background-color: orange;
+        }
+
+        @keyframes move {
+            from { transform: translateX(400px); }
+            to   { transform: none }
+        }
+
+        .behind {
+            background-color: blue;
+            top: 50px;
+            left: 50px;
+            z-index: 1;
+        }
+
+        .front {
+            top: 25px;
+            left: 25px;
+            background-color: green;
+            z-index: 2;
+        }
+    </style>
+    <script>
+        if (window.testRunner)
+            testRunner.waitUntilDone();
+        
+        window.addEventListener('load', function() {
+            document.getElementById('animating').addEventListener('animationend', function() {
+                // Wait until filling forwards.
+                window.setTimeout(function() {
+                    if (window.testRunner)
+                        testRunner.notifyDone();
+                }, 0);
+            })
+        }, false);
+    </script>
+</head>
+<body>
+
+<div class="behind box"></div>
+<div id="animating" class="box">
+    <div class="front box"></div>
+</div>
+
+</body>
+</html>
diff --git a/LayoutTests/animations/stacking-context-not-fill-forwards-expected.html b/LayoutTests/animations/stacking-context-not-fill-forwards-expected.html
new file mode 100644 (file)
index 0000000..e9dfe88
--- /dev/null
@@ -0,0 +1,41 @@
+<!DOCTYPE html>
+
+<html>
+<head>
+    <style>
+        .box {
+            position: absolute;
+            top: 0;
+            left: 0;
+            height: 100px;
+            width: 100px;
+        }
+
+        #animating {
+            background-color: orange;
+        }
+
+        .behind {
+            background-color: blue;
+            top: 50px;
+            left: 50px;
+            z-index: 1;
+        }
+
+        .front {
+            top: 25px;
+            left: 25px;
+            background-color: green;
+            z-index: 2;
+        }
+    </style>
+</head>
+<body>
+
+<div class="behind box"></div>
+<div id="animating" class="box">
+    <div class="front box"></div>
+</div>
+
+</body>
+</html>
diff --git a/LayoutTests/animations/stacking-context-not-fill-forwards.html b/LayoutTests/animations/stacking-context-not-fill-forwards.html
new file mode 100644 (file)
index 0000000..627173a
--- /dev/null
@@ -0,0 +1,62 @@
+<!DOCTYPE html>
+
+<html>
+<head>
+    <title>Tests that a non-fill-forwards animation doesn't cause stacking context once finished.</title>
+    <style>
+        .box {
+            position: absolute;
+            top: 0;
+            left: 0;
+            height: 100px;
+            width: 100px;
+        }
+        
+        #animating {
+            animation: move 0.1s 1;
+            background-color: orange;
+        }
+
+        @keyframes move {
+            from { transform: translateX(400px); }
+            to   { transform: none }
+        }
+
+        .behind {
+            background-color: blue;
+            top: 50px;
+            left: 50px;
+            z-index: 1;
+        }
+
+        .front {
+            top: 25px;
+            left: 25px;
+            background-color: green;
+            z-index: 2;
+        }
+    </style>
+    <script>
+        if (window.testRunner)
+            testRunner.waitUntilDone();
+        
+        window.addEventListener('load', function() {
+            document.getElementById('animating').addEventListener('animationend', function() {
+                // Wait until filling forwards.
+                window.setTimeout(function() {
+                    if (window.testRunner)
+                        testRunner.notifyDone();
+                }, 0);
+            })
+        }, false);
+    </script>
+</head>
+<body>
+
+<div class="behind box"></div>
+<div id="animating" class="box">
+    <div class="front box"></div>
+</div>
+
+</body>
+</html>
diff --git a/LayoutTests/animations/stacking-context-unchanged-while-running-expected.html b/LayoutTests/animations/stacking-context-unchanged-while-running-expected.html
new file mode 100644 (file)
index 0000000..65cf917
--- /dev/null
@@ -0,0 +1,41 @@
+<!DOCTYPE html>
+
+<html>
+<head>
+    <style>
+        .box {
+            position: absolute;
+            top: 0;
+            left: 0;
+            height: 100px;
+            width: 100px;
+        }
+        
+        #animating {
+            background-color: orange;
+             z-index: 0;
+        }
+
+        .behind {
+            background-color: blue;
+            top: 25px;
+            left: 50px;
+            z-index: 1;
+        }
+
+        .front {
+            top: 50px;
+            left: 25px;
+            background-color: green;
+            z-index: 2;
+        }
+    </style>
+</head>
+<body>
+
+<div class="behind box"></div>
+<div id="animating" class="box">
+    <div class="front box"></div>
+</div>
+</body>
+</html>
diff --git a/LayoutTests/animations/stacking-context-unchanged-while-running.html b/LayoutTests/animations/stacking-context-unchanged-while-running.html
new file mode 100644 (file)
index 0000000..3f17be9
--- /dev/null
@@ -0,0 +1,65 @@
+<!DOCTYPE html>
+
+<html>
+<head>
+    <title>Tests that an element with an animation whose keyframes affect stacking context creates stacking context throughout the animation.</title>
+    <style>
+        .box {
+            position: absolute;
+            top: 0;
+            left: 0;
+            height: 100px;
+            width: 100px;
+        }
+
+        #animating {
+            animation: move 2s 1;
+            background-color: orange;
+        }
+
+        @keyframes move {
+            from { z-index: 1; }
+            10%  { z-index: auto; }
+            90%  { z-index: auto; }
+            to   { z-index: 2; }
+        }
+
+        .behind {
+            background-color: blue;
+            top: 25px;
+            left: 50px;
+            z-index: 1;
+        }
+
+        .front {
+            top: 50px;
+            left: 25px;
+            background-color: green;
+            z-index: 2;
+        }
+        
+        #result {
+            opacity: 0;
+        }
+    </style>
+    <script src="resources/animation-test-helpers.js" type="text/javascript"></script>
+    <script type="text/javascript">
+        const expectedValues = [
+            // [animation-name, time, element-id, property, expected-value, tolerance]
+            ["move", 0.5, "animating", "z-index", 0, 0],
+        ];
+
+        var doPixelTest = true;
+        var disablePauseAPI = false;
+        runAnimationTest(expectedValues, null, undefined, disablePauseAPI, doPixelTest);
+    </script>
+</head>
+<body>
+
+<div class="behind box"></div>
+<div id="animating" class="box">
+    <div class="front box"></div>
+</div>
+<div id="result"></div>
+</body>
+</html>
index 11dd634..11eea39 100644 (file)
@@ -1,3 +1,44 @@
+2016-10-27  Simon Fraser  <simon.fraser@apple.com>
+
+        If an animation's keyframes affect stacking context properties, create stacking context while the animation is running
+        https://bugs.webkit.org/show_bug.cgi?id=164094
+
+        Reviewed by Dean Jackson.
+
+        The CSS animations spec <https://drafts.csswg.org/css-animations-1/> now makes it
+        clear that a keyframe animation animating properties which can affect stacking context
+        should establish stacking context while it's running, or filling-forwards. This is part
+        of the "the user agent must act as if the will-change property...includes all the properties
+        animated by the animation" clause.
+
+        Implement by having CompositeAnimation::animate() track whether running animations should
+        create stacking context, replacing existing code in AnimationController::updateAnimations()
+        which only looked at opacity and transform. Transitions are also checked to see if they need
+        to trigger stacking context.
+
+        This allows for the removal of a 0.9999 hack when blending opacity.
+
+        Tests: animations/stacking-context-fill-forwards.html
+               animations/stacking-context-not-fill-forwards.html
+               animations/stacking-context-unchanged-while-running.html
+
+        * page/animation/AnimationController.cpp:
+        (WebCore::AnimationController::updateAnimations):
+        * page/animation/CSSPropertyAnimation.cpp:
+        * page/animation/CompositeAnimation.cpp:
+        (WebCore::CompositeAnimation::animate):
+        * page/animation/KeyframeAnimation.cpp:
+        (WebCore::KeyframeAnimation::KeyframeAnimation):
+        (WebCore::KeyframeAnimation::computeStackingContextImpact):
+        (WebCore::KeyframeAnimation::animate):
+        * page/animation/KeyframeAnimation.h:
+        * rendering/RenderLayer.cpp:
+        (WebCore::RenderLayer::currentTransform):
+        * rendering/style/WillChangeData.cpp:
+        (WebCore::WillChangeData::propertyCreatesStackingContext):
+        (WebCore::propertyCreatesStackingContext): Deleted.
+        * rendering/style/WillChangeData.h:
+
 2016-10-27  Sam Weinig  <sam@webkit.org>
 
         [WebIDL] Move code generators off of domSignature::type and onto domSignature::idlType
index 7f4b46a..9a46deb 100644 (file)
@@ -624,13 +624,6 @@ bool AnimationController::updateAnimations(RenderElement& renderer, const Render
 #endif
     }
 
-    if (animatedStyle) {
-        // If the animations/transitions change opacity or transform, we need to update
-        // the style to impose the stacking rules. Note that this is also
-        // done in StyleResolver::adjustRenderStyle().
-        if (animatedStyle->hasAutoZIndex() && (animatedStyle->opacity() < 1.0f || animatedStyle->hasTransform()))
-            animatedStyle->setZIndex(0);
-    }
     return animationStateChanged;
 }
 
index 7e1d834..1fda298 100644 (file)
@@ -638,10 +638,7 @@ public:
 
     void blend(const AnimationBase* anim, RenderStyle* dst, const RenderStyle* a, const RenderStyle* b, double progress) const override
     {
-        float fromOpacity = a->opacity();
-
-        // This makes sure we put the object being animated into a RenderLayer during the animation
-        dst->setOpacity(blendFunc(anim, (fromOpacity == 1) ? 0.999999f : fromOpacity, b->opacity(), progress));
+        dst->setOpacity(blendFunc(anim, a->opacity(), b->opacity(), progress));
     }
 };
 
index 2f7b1ad..b203d3c 100644 (file)
@@ -292,13 +292,33 @@ bool CompositeAnimation::animate(RenderElement& renderer, const RenderStyle* cur
     m_keyframeAnimations.checkConsistency();
 
     bool animationStateChanged = false;
+    bool forceStackingContext = false;
 
     if (currentStyle) {
         // Now that we have transition objects ready, let them know about the new goal state.  We want them
         // to fill in a RenderStyle*& only if needed.
+        bool checkForStackingContext = false;
         for (auto& transition : m_transitions.values()) {
             if (transition->animate(this, &renderer, currentStyle, &targetStyle, blendedStyle))
                 animationStateChanged = true;
+
+            checkForStackingContext |= WillChangeData::propertyCreatesStackingContext(transition->animatingProperty());
+        }
+
+        if (blendedStyle && checkForStackingContext) {
+            // Note that this is similar to code in StyleResolver::adjustRenderStyle() but only needs to consult
+            // animatable properties that can trigger stacking context.
+            if (blendedStyle->opacity() < 1.0f
+                || blendedStyle->hasTransformRelatedProperty()
+                || blendedStyle->hasMask()
+                || blendedStyle->clipPath()
+                || blendedStyle->boxReflect()
+                || blendedStyle->hasFilter()
+#if ENABLE(FILTERS_LEVEL_2)
+                || blendedStyle->hasBackdropFilter()
+#endif
+                )
+            forceStackingContext = true;
         }
     }
 
@@ -306,8 +326,22 @@ bool CompositeAnimation::animate(RenderElement& renderer, const RenderStyle* cur
     // to fill in a RenderStyle*& only if needed.
     for (auto& name : m_keyframeAnimationOrderMap) {
         RefPtr<KeyframeAnimation> keyframeAnim = m_keyframeAnimations.get(name);
-        if (keyframeAnim && keyframeAnim->animate(this, &renderer, currentStyle, &targetStyle, blendedStyle))
-            animationStateChanged = true;
+        if (keyframeAnim) {
+            if (keyframeAnim->animate(this, &renderer, currentStyle, &targetStyle, blendedStyle))
+                animationStateChanged = true;
+
+            bool runningOrFillingForwards = !keyframeAnim->waitingToStart() && !keyframeAnim->postActive();
+            forceStackingContext |= runningOrFillingForwards && keyframeAnim->triggersStackingContext();
+        }
+    }
+
+    // https://drafts.csswg.org/css-animations-1/
+    // While an animation is applied but has not finished, or has finished but has an animation-fill-mode of forwards or both,
+    // the user agent must act as if the will-change property ([css-will-change-1]) on the element additionally
+    // includes all the properties animated by the animation.
+    if (forceStackingContext && blendedStyle) {
+        if (blendedStyle->hasAutoZIndex())
+            blendedStyle->setZIndex(0);
     }
 
     return animationStateChanged;
index ac8ab01..9f52bc0 100644 (file)
@@ -39,6 +39,7 @@
 #include "RenderStyle.h"
 #include "StylePendingResources.h"
 #include "StyleResolver.h"
+#include "WillChangeData.h"
 
 namespace WebCore {
 
@@ -55,6 +56,7 @@ KeyframeAnimation::KeyframeAnimation(const Animation& animation, RenderElement*
 #if ENABLE(FILTERS_LEVEL_2)
     checkForMatchingBackdropFilterFunctionLists();
 #endif
+    computeStackingContextImpact();
 }
 
 KeyframeAnimation::~KeyframeAnimation()
@@ -64,6 +66,16 @@ KeyframeAnimation::~KeyframeAnimation()
         endAnimation();
 }
 
+void KeyframeAnimation::computeStackingContextImpact()
+{
+    for (auto propertyID : m_keyframes.properties()) {
+        if (WillChangeData::propertyCreatesStackingContext(propertyID)) {
+            m_triggersStackingContext = true;
+            break;
+        }
+    }
+}
+
 void KeyframeAnimation::fetchIntervalEndpointsForProperty(CSSPropertyID property, const RenderStyle*& fromStyle, const RenderStyle*& toStyle, double& prog) const
 {
     size_t numKeyframes = m_keyframes.size();
@@ -147,6 +159,7 @@ bool KeyframeAnimation::animate(CompositeAnimation* compositeAnimation, RenderEl
         return false;
     }
 
+    // FIXME: the code below never changes the state, so this function always returns false.
     AnimationState oldState = state();
 
     // Run a cycle of animation.
index 848dd40..499c027 100644 (file)
@@ -55,6 +55,8 @@ public:
     const AtomicString& name() const { return m_keyframes.animationName(); }
 
     bool hasAnimationForProperty(CSSPropertyID) const;
+
+    bool triggersStackingContext() const { return m_triggersStackingContext; }
     
     void setUnanimatedStyle(std::unique_ptr<RenderStyle> style) { m_unanimatedStyle = WTFMove(style); }
     RenderStyle* unanimatedStyle() const { return m_unanimatedStyle.get(); }
@@ -81,6 +83,7 @@ protected:
 
     bool computeExtentOfAnimationForMatchingTransformLists(const FloatRect& rendererBox, LayoutRect&) const;
 
+    void computeStackingContextImpact();
     void resolveKeyframeStyles();
     void validateTransformFunctionList();
     void checkForMatchingFilterFunctionLists();
@@ -99,6 +102,7 @@ private:
     std::unique_ptr<RenderStyle> m_unanimatedStyle; // The style just before we started animation
 
     bool m_startEventDispatched { false };
+    bool m_triggersStackingContext { false };
 };
 
 } // namespace WebCore
index 58443bf..bdf0d7d 100644 (file)
@@ -996,6 +996,7 @@ TransformationMatrix RenderLayer::currentTransform(RenderStyle::ApplyTransformOr
     
     RenderBox* box = renderBox();
     ASSERT(box);
+    // FIXME: replace with call to AnimationController::isRunningAcceleratedAnimationOnRenderer() and remove RenderStyle::isRunningAcceleratedAnimation().
     if (renderer().style().isRunningAcceleratedAnimation()) {
         TransformationMatrix currTransform;
         FloatRect pixelSnappedBorderRect = snapRectToDevicePixels(box->borderBoxRect(), box->document().deviceScaleFactor());
index 77d4aa8..6763ea2 100644 (file)
@@ -62,7 +62,7 @@ bool WillChangeData::containsProperty(CSSPropertyID property) const
 
 // "If any non-initial value of a property would create a stacking context on the element,
 // specifying that property in will-change must create a stacking context on the element."
-static bool propertyCreatesStackingContext(CSSPropertyID property)
+bool WillChangeData::propertyCreatesStackingContext(CSSPropertyID property)
 {
     switch (property) {
     case CSSPropertyPerspective:
index 49e7608..bb5c604 100644 (file)
@@ -70,6 +70,8 @@ public:
     typedef std::pair<Feature, CSSPropertyID> FeaturePropertyPair;
     FeaturePropertyPair featureAt(size_t) const;
 
+    static bool propertyCreatesStackingContext(CSSPropertyID);
+
 private:
     WillChangeData()
     {