2008-09-19 Chris Marrin <cmarrin@apple.com>
authordino@apple.com <dino@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 20 Sep 2008 00:53:49 +0000 (00:53 +0000)
committerdino@apple.com <dino@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 20 Sep 2008 00:53:49 +0000 (00:53 +0000)
        Reviewed by Dave Hyatt.

        Transition starts running when it shouldn't
        https://bugs.webkit.org/show_bug.cgi?id=20892

        When there is a transition and an animation on the
        same element, make sure the animation wins.

        The fix is to save the unanimated style when an animation is started.
        Then, when starting a transition, check to see if there is a current
        animation on the same prop. If so, use the unanimated style as the
        fromStyle rather than the current style.

        Test: animations/transition-and-animation-1.html

        * page/animation/CompositeAnimation.cpp:
        (WebCore::CompositeAnimation::updateTransitions):
        (WebCore::CompositeAnimation::updateKeyframeAnimations):
        (WebCore::CompositeAnimation::animate):
        (WebCore::CompositeAnimation::getAnimationForProperty):
        * page/animation/CompositeAnimation.h:
        * page/animation/ImplicitAnimation.cpp:
        (WebCore::ImplicitAnimation::reset):
        * page/animation/ImplicitAnimation.h:
        * page/animation/KeyframeAnimation.cpp:
        (WebCore::KeyframeAnimation::hasAnimationForProperty):
        * page/animation/KeyframeAnimation.h:
        (WebCore::KeyframeAnimation::KeyframeAnimation):
        (WebCore::KeyframeAnimation::unanimatedStyle):

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

LayoutTests/ChangeLog
LayoutTests/animations/transition-and-animation-1-expected.txt [new file with mode: 0644]
LayoutTests/animations/transition-and-animation-1.html [new file with mode: 0644]
WebCore/ChangeLog
WebCore/page/animation/CompositeAnimation.cpp
WebCore/page/animation/CompositeAnimation.h
WebCore/page/animation/ImplicitAnimation.cpp
WebCore/page/animation/ImplicitAnimation.h
WebCore/page/animation/KeyframeAnimation.cpp
WebCore/page/animation/KeyframeAnimation.h

index f76317e..5e5182a 100644 (file)
@@ -1,3 +1,16 @@
+2008-09-19  Chris Marrin  <cmarrin@apple.com>
+
+        Reviewed by Dave Hyatt.
+
+        Transition starts running when it shouldn't
+        https://bugs.webkit.org/show_bug.cgi?id=20892
+
+        When there is a transition and an animation on the
+        same element, make sure the animation wins.
+
+        * animations/transition-and-animation-1-expected.txt: Added.
+        * animations/transition-and-animation-1.html: Added.
+
 2008-09-19  Beth Dakin  <bdakin@apple.com>
 
         Reviewed by Dave Hyatt.
diff --git a/LayoutTests/animations/transition-and-animation-1-expected.txt b/LayoutTests/animations/transition-and-animation-1-expected.txt
new file mode 100644 (file)
index 0000000..4cd89e5
--- /dev/null
@@ -0,0 +1,2 @@
+This test has a transition and animation on the same property (-webkit-transform). But the transition is never triggered, so nothing should be moving when the animation finishes.
+PASS
diff --git a/LayoutTests/animations/transition-and-animation-1.html b/LayoutTests/animations/transition-and-animation-1.html
new file mode 100644 (file)
index 0000000..0a929ef
--- /dev/null
@@ -0,0 +1,74 @@
+<!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN"
+   "http://www.w3.org/TR/html4/loose.dtd">
+
+<html lang="en">
+<head>
+  <meta http-equiv="Content-Type" content="text/html; charset=utf-8">
+  <title>Transition/Animation Test #1</title>
+  <style type="text/css" media="screen">
+    #box {
+        position: absolute;
+        left: 0;
+        top: 100px;
+        height: 100px;
+        width: 100px;
+        background-color: blue;
+        -webkit-animation-duration: 0.5s;
+        -webkit-animation-timing-function: linear;
+        -webkit-animation-name: "anim";
+        -webkit-transition-property: -webkit-transform;
+        -webkit-transition-duration: 10s;
+    }
+    @-webkit-keyframes "anim" {
+        from { -webkit-transform: translateX(200px); }
+        to   { -webkit-transform: translateX(300px); }
+    }
+    </style>
+    <script type="text/javascript" charset="utf-8">
+    if (window.layoutTestController) {
+        layoutTestController.dumpAsText();
+        layoutTestController.waitUntilDone();
+    }
+    
+    result = "PASS";
+    const defaultTolerance = 0.2;
+    
+    function isEqual(actual, desired, tolerance)
+    {
+        if (tolerance == undefined || tolerance == 0)
+            tolerance = defaultTolerance;
+        var diff = Math.abs(actual - desired);
+        return diff < tolerance;
+    }
+    
+    function snapshot(which)
+    {
+        var m = window.getComputedStyle(document.getElementById('box')).webkitTransform;
+        if (m != "none")
+            result = "FAIL(was:"+m+", expected:none)";
+    }
+
+    function start()
+    {
+        setTimeout("snapshot()", 550);
+        
+        window.setTimeout(function() {
+            document.getElementById('result').innerHTML = result;
+            if (window.layoutTestController)
+                layoutTestController.notifyDone();
+        }, 600);
+    }
+    
+    document.addEventListener('webkitAnimationStart', start, false);
+    
+  </script>
+</head>
+<body>
+This test has a transition and animation on the same property (-webkit-transform). But the transition is never triggered,
+so nothing should be moving when the animation finishes.
+<div id="box">
+</div>
+<div id="result">
+</div>
+</body>
+</html>
index 746a27d..0988ec1 100644 (file)
@@ -1,3 +1,35 @@
+2008-09-19  Chris Marrin  <cmarrin@apple.com>
+
+        Reviewed by Dave Hyatt.
+
+        Transition starts running when it shouldn't
+        https://bugs.webkit.org/show_bug.cgi?id=20892
+
+        When there is a transition and an animation on the
+        same element, make sure the animation wins.
+
+        The fix is to save the unanimated style when an animation is started.
+        Then, when starting a transition, check to see if there is a current
+        animation on the same prop. If so, use the unanimated style as the
+        fromStyle rather than the current style.
+
+        Test: animations/transition-and-animation-1.html
+
+        * page/animation/CompositeAnimation.cpp:
+        (WebCore::CompositeAnimation::updateTransitions):
+        (WebCore::CompositeAnimation::updateKeyframeAnimations):
+        (WebCore::CompositeAnimation::animate):
+        (WebCore::CompositeAnimation::getAnimationForProperty):
+        * page/animation/CompositeAnimation.h:
+        * page/animation/ImplicitAnimation.cpp:
+        (WebCore::ImplicitAnimation::reset):
+        * page/animation/ImplicitAnimation.h:
+        * page/animation/KeyframeAnimation.cpp:
+        (WebCore::KeyframeAnimation::hasAnimationForProperty):
+        * page/animation/KeyframeAnimation.h:
+        (WebCore::KeyframeAnimation::KeyframeAnimation):
+        (WebCore::KeyframeAnimation::unanimatedStyle):
+
 2008-09-19  David Hyatt  <hyatt@apple.com>
 
         Add support for painting/hit testing of four possible scrollbar buttons.
index b5cf574..ddfd8d2 100644 (file)
@@ -79,6 +79,11 @@ void CompositeAnimation::updateTransitions(RenderObject* renderer, const RenderS
             // ImplicitAnimations are always hashed by actual properties, never cAnimateAll
             ASSERT(prop > firstCSSProperty && prop < (firstCSSProperty + numCSSProperties));
 
+            // If there is a running animation for this property, the transition is overridden
+            // and we have to use the unanimatedStyle from the animation. We do the test
+            // against the unanimated style here, but we "override" the transition later.
+            const KeyframeAnimation* kfAnim = getAnimationForProperty(prop);
+
             // See if there is a current transition for this prop
             ImplicitAnimation* implAnim = m_transitions.get(prop);
             bool equal = true;
@@ -93,7 +98,7 @@ void CompositeAnimation::updateTransitions(RenderObject* renderer, const RenderS
                 }
             } else {
                 // See if we need to start a new transition
-                equal = AnimationBase::propertiesEqual(prop, currentStyle, targetStyle);
+                equal = AnimationBase::propertiesEqual(prop, kfAnim ? kfAnim->unanimatedStyle() : currentStyle, targetStyle);
             }
 
             if (!equal) {
@@ -147,7 +152,7 @@ void CompositeAnimation::updateKeyframeAnimations(RenderObject* renderer, const
                 kfAnim->setIndex(i);
             } else if ((anim->duration() || anim->delay()) && anim->iterationCount()
                         && anim->keyframeList() && !anim->keyframeList()->isEmpty()) {
-                kfAnim = new KeyframeAnimation(const_cast<Animation*>(anim), renderer, i, this);
+                kfAnim = new KeyframeAnimation(const_cast<Animation*>(anim), renderer, i, this, currentStyle ? currentStyle : targetStyle);
                 m_keyframeAnimations.set(kfAnim->name().impl(), kfAnim);
             }
         }
@@ -179,6 +184,9 @@ RenderStyle* CompositeAnimation::animate(RenderObject* renderer, const RenderSty
 {
     RenderStyle* resultStyle = 0;
 
+    // Update animations first so we can see if any transitions are overridden
+    updateKeyframeAnimations(renderer, currentStyle, targetStyle);
+
     // We don't do any transitions if we don't have a currentStyle (on startup)
     updateTransitions(renderer, currentStyle, targetStyle);
 
@@ -192,8 +200,6 @@ RenderStyle* CompositeAnimation::animate(RenderObject* renderer, const RenderSty
         }
     }
 
-    updateKeyframeAnimations(renderer, currentStyle, targetStyle);
-
     // Now that we have animation objects ready, let them know about the new goal state.  We want them
     // to fill in a RenderStyle*& only if needed.
     if (targetStyle->hasAnimations()) {
@@ -247,6 +253,22 @@ bool CompositeAnimation::animating()
     return false;
 }
 
+const KeyframeAnimation* CompositeAnimation::getAnimationForProperty(int property) const
+{
+    const KeyframeAnimation* retval = 0;
+    
+    // We want to send back the last animation with the property if there are multiples.
+    // So we need to iterate through all animations
+    AnimationNameMap::const_iterator animationsEnd = m_keyframeAnimations.end();
+    for (AnimationNameMap::const_iterator it = m_keyframeAnimations.begin(); it != animationsEnd; ++it) {
+        const KeyframeAnimation* anim = it->second;
+        if (anim->hasAnimationForProperty(property))
+            retval = anim;
+    }
+    
+    return retval;
+}
+
 void CompositeAnimation::resetTransitions(RenderObject* renderer)
 {
     CSSPropertyTransitionsMap::const_iterator end = m_transitions.end();
index 05ed744..400ba35 100644 (file)
@@ -59,8 +59,8 @@ public:
 
     void setAnimating(bool);
     bool animating();
-
-    bool hasAnimationForProperty(int prop) const { return m_transitions.contains(prop); }
+    
+    const KeyframeAnimation* getAnimationForProperty(int property) const;
 
     void resetTransitions(RenderObject*);
     void resetAnimations(RenderObject*);
index 1a15c2b..9a0d29e 100644 (file)
@@ -116,17 +116,17 @@ void ImplicitAnimation::reset(RenderObject* renderer, const RenderStyle* from /*
     ASSERT((!m_toStyle && !to) || m_toStyle != to);
     ASSERT((!m_fromStyle && !from) || m_fromStyle != from);
     if (m_fromStyle)
-        m_fromStyle->deref(renderer->renderArena());
+        const_cast<RenderStyle*>(m_fromStyle)->deref(renderer->renderArena());
     if (m_toStyle)
-        m_toStyle->deref(renderer->renderArena());
+        const_cast<RenderStyle*>(m_toStyle)->deref(renderer->renderArena());
 
     m_fromStyle = const_cast<RenderStyle*>(from);   // it is read-only, other than the ref
     if (m_fromStyle)
-        m_fromStyle->ref();
+        const_cast<RenderStyle*>(m_fromStyle)->ref();
 
     m_toStyle = const_cast<RenderStyle*>(to);       // it is read-only, other than the ref
     if (m_toStyle)
-        m_toStyle->ref();
+        const_cast<RenderStyle*>(m_toStyle)->ref();
 
     // Restart the transition
     if (from && to)
index e49163c..9464a99 100644 (file)
@@ -74,8 +74,8 @@ private:
     bool m_overridden;          // true when there is a keyframe animation that overrides the transitioning property
 
     // The two styles that we are blending.
-    RenderStyle* m_fromStyle;
-    RenderStyle* m_toStyle;
+    const RenderStyle* m_fromStyle;
+    const RenderStyle* m_toStyle;
 };
 
 } // namespace WebCore
index 5528a03..77a82b7 100644 (file)
@@ -120,6 +120,17 @@ void KeyframeAnimation::animate(CompositeAnimation* animation, RenderObject* ren
     }
 }
 
+bool KeyframeAnimation::hasAnimationForProperty(int property) const
+{
+    HashSet<int>::const_iterator end = m_keyframes->endProperties();
+    for (HashSet<int>::const_iterator it = m_keyframes->beginProperties(); it != end; ++it) {
+        if (*it == property)
+            return true;
+    }
+    
+    return false;
+}
+
 void KeyframeAnimation::endAnimation(bool)
 {
     // Restore the original (unanimated) style
index c251613..70a65f7 100644 (file)
@@ -40,14 +40,20 @@ namespace WebCore {
 // for a single RenderObject.
 class KeyframeAnimation : public AnimationBase {
 public:
-    KeyframeAnimation(const Animation* animation, RenderObject* renderer, int index, CompositeAnimation* compAnim)
+    KeyframeAnimation(const Animation* animation, RenderObject* renderer, int index, CompositeAnimation* compAnim, const RenderStyle* unanimatedStyle)
         : AnimationBase(animation, renderer, compAnim)
         , m_keyframes(animation->keyframeList())
         , m_name(animation->name())
         , m_index(index)
+        , m_unanimatedStyle(0)
     {
         // Set the transform animation list
         validateTransformFunctionList();
+        
+        if (unanimatedStyle) {
+            const_cast<RenderStyle*>(unanimatedStyle)->ref();
+            m_unanimatedStyle = unanimatedStyle;
+        }
     }
 
     virtual ~KeyframeAnimation();
@@ -60,6 +66,10 @@ public:
     void setIndex(int i) { m_index = i; }
 
     virtual bool shouldFireEvents() const { return true; }
+    
+    bool hasAnimationForProperty(int property) const;
+    
+    const RenderStyle* unanimatedStyle() const { return m_unanimatedStyle; }
 
 protected:
     virtual void onAnimationStart(double elapsedTime);
@@ -83,6 +93,9 @@ private:
     AtomicString m_name;
     // The order in which this animation appears in the animation-name style.
     int m_index;
+
+    // The style just before we started animation
+    const RenderStyle* m_unanimatedStyle;
 };
 
 } // namespace WebCore