Web Animations CSS: Implement CSS Transitions backed on Web Animations model

Source/core/animation/css/CSSAnimations.cpp

Index: Source/core/animation/css/CSSAnimations.cpp
diff --git a/Source/core/animation/css/CSSAnimations.cpp b/Source/core/animation/css/CSSAnimations.cpp
index 0e3a97b540249600f8f4856c1c7b2c1147685d97..533aec599ece9d58b10c6255cb2022195b0918a6 100644
--- a/Source/core/animation/css/CSSAnimations.cpp
+++ b/Source/core/animation/css/CSSAnimations.cpp
@@ -31,14 +31,19 @@
 #include "config.h"
 #include "core/animation/css/CSSAnimations.h"
 
+#include "StylePropertyShorthand.h"
 #include "core/animation/ActiveAnimations.h"
 #include "core/animation/DocumentTimeline.h"
 #include "core/animation/KeyframeAnimationEffect.h"
+#include "core/animation/css/CSSAnimatableValueFactory.h"
 #include "core/css/CSSKeyframeRule.h"
 #include "core/css/resolver/StyleResolver.h"
 #include "core/dom/Element.h"
 #include "core/events/EventNames.h"
+#include "core/events/TransitionEvent.h"
 #include "core/events/WebKitAnimationEvent.h"
+// FIXME: Remove this once all properties are supported
+#include "core/page/animation/CSSPropertyAnimation.h"
 #include "core/platform/animation/CSSAnimationDataList.h"
 #include "core/platform/animation/TimingFunction.h"
 #include "wtf/HashSet.h"
@@ -143,18 +148,21 @@ CSSAnimationUpdateScope::~CSSAnimationUpdateScope()
         cssAnimations->maybeApplyPendingUpdate(m_target);
 }
 
-bool CSSAnimations::needsUpdate(const Element* element, const RenderStyle* style)
+PassOwnPtr<CSSAnimationUpdate> CSSAnimations::calculateUpdate(Element* element, const RenderStyle* style, StyleResolver* resolver)
 {
-    ActiveAnimations* activeAnimations = element->activeAnimations();
-    const CSSAnimationDataList* animations = style->animations();
-    const CSSAnimations* cssAnimations = activeAnimations ? activeAnimations->cssAnimations() : 0;
-    EDisplay display = style->display();
-    return (display != NONE && animations && animations->size()) || (cssAnimations && !cssAnimations->isEmpty());

[dstockwell, 2013/10/08 21:02:59]

where did this display != NONE check go?

[Timothy Loh, 2013/10/09 01:33:07]

Looks redundant - if display is none and cssAnimations is null/empty, the
function won't do anything anyway.

+    ASSERT(RuntimeEnabledFeatures::webAnimationsCSSEnabled());
+    OwnPtr<CSSAnimationUpdate> update = adoptPtr(new CSSAnimationUpdate());
+    updateAnimationUpdate(update.get(), element, style, resolver);

[dstockwell, 2013/10/08 21:02:59]

I think calculateAnimationUpdate and calculateTransitionUpdate are better names
here. updateXUpdate is just awkward.

[Timothy Loh, 2013/10/09 01:33:07]

Done.

+    updateTransitionUpdate(update.get(), element, style);
+    return update->isEmpty() ? nullptr : update.release();
 }
 
-PassOwnPtr<CSSAnimationUpdate> CSSAnimations::calculateUpdate(Element* element, const RenderStyle* style, const CSSAnimations* cssAnimations, const CSSAnimationDataList* animationDataList, StyleResolver* resolver)
+void CSSAnimations::updateAnimationUpdate(CSSAnimationUpdate* update, Element* element, const RenderStyle* style, StyleResolver* resolver)
 {
-    OwnPtr<CSSAnimationUpdate> update;
+    ActiveAnimations* activeAnimations = element->activeAnimations();
+    const CSSAnimationDataList* animationDataList = style->animations();
+    const CSSAnimations* cssAnimations = activeAnimations ? activeAnimations->cssAnimations() : 0;
+
     HashSet<AtomicString> inactive;
     if (cssAnimations)
         for (AnimationMap::const_iterator iter = cssAnimations->m_animations.begin(); iter != cssAnimations->m_animations.end(); ++iter)
@@ -182,8 +190,6 @@ PassOwnPtr<CSSAnimationUpdate> CSSAnimations::calculateUpdate(Element* element,
             Vector<std::pair<KeyframeAnimationEffect::KeyframeVector, RefPtr<TimingFunction> > > keyframesAndTimingFunctions;
             resolver->resolveKeyframes(element, style, animationName, defaultTimingFunction.get(), keyframesAndTimingFunctions);
             if (!keyframesAndTimingFunctions.isEmpty()) {
-                if (!update)
-                    update = adoptPtr(new CSSAnimationUpdate());
                 HashSet<RefPtr<InertAnimation> > animations;
                 for (size_t j = 0; j < keyframesAndTimingFunctions.size(); ++j) {
                     ASSERT(!keyframesAndTimingFunctions[j].first.isEmpty());
@@ -196,12 +202,8 @@ PassOwnPtr<CSSAnimationUpdate> CSSAnimations::calculateUpdate(Element* element,
         }
     }
 
-    if (!inactive.isEmpty() && !update)
-        update = adoptPtr(new CSSAnimationUpdate());
     for (HashSet<AtomicString>::const_iterator iter = inactive.begin(); iter != inactive.end(); ++iter)
         update->cancelAnimation(*iter, cssAnimations->m_animations.get(*iter));
-
-    return update.release();
 }
 
 void CSSAnimations::maybeApplyPendingUpdate(Element* element)
@@ -223,7 +225,7 @@ void CSSAnimations::maybeApplyPendingUpdate(Element* element)
     // FIXME: Apply updates to play-state.
 
     for (Vector<CSSAnimationUpdate::NewAnimation>::const_iterator iter = update->newAnimations().begin(); iter != update->newAnimations().end(); ++iter) {
-        OwnPtr<CSSAnimations::EventDelegate> eventDelegate = adoptPtr(new EventDelegate(element, iter->name));
+        OwnPtr<AnimationEventDelegate> eventDelegate = adoptPtr(new AnimationEventDelegate(element, iter->name));
         HashSet<RefPtr<Player> > players;
         for (HashSet<RefPtr<InertAnimation> >::const_iterator animationsIter = iter->animations.begin(); animationsIter != iter->animations.end(); ++animationsIter) {
             const InertAnimation* inertAnimation = animationsIter->get();
@@ -234,6 +236,205 @@ void CSSAnimations::maybeApplyPendingUpdate(Element* element)
         }
         m_animations.set(iter->name, players);
     }
+
+    Vector<CSSPropertyID> removedTransitions;
+    for (TransitionMap::iterator iter = m_transitions.begin(); iter != m_transitions.end(); ++iter) {

[dstockwell, 2013/10/08 21:02:59]

Why are we iterating over the transition map rather than the cancelled
transitions?

[Timothy Loh, 2013/10/09 01:33:07]

Because I'm silly :(

+        Player* player = iter->value.get();
+        CSSPropertyID id = iter->key;
+        if (update->endedTransitions().contains(id)) {
+            player->cancel();
+            removedTransitions.append(id);
+        }
+    }
+    for (int i = 0; i < removedTransitions.size(); ++i)

[dstockwell, 2013/10/08 21:02:59]

Should this be size_t?

[Timothy Loh, 2013/10/09 01:33:07]

Loop removed (changed to loop over cancelled transitions)

+        m_transitions.remove(removedTransitions[i]);

[Steve Block, 2013/10/09 00:57:12]

Why do you use two passes to remove the cancelled transitions? Can you loop
through update->endedTransitions() and remove them directly from m_transitions?

[Timothy Loh, 2013/10/09 01:33:07]

Doug had the same comment. Fixed.

+
+    HashMap<CSSPropertyID, RefPtr<InertAnimation> >::const_iterator iter;
+    for (iter = update->newTransitions().begin(); iter != update->newTransitions().end(); ++iter) {
+        CSSPropertyID id = iter->key;
+        OwnPtr<TransitionEventDelegate> eventDelegate = adoptPtr(new TransitionEventDelegate(element, id));
+        RefPtr<Animation> transition = Animation::create(element, iter->value->effect(), iter->value->specified(), eventDelegate.release());
+        // FIXME: We'll probably want a separate DocumentTimeline for transitions, or
+        // at least to not have transitions also added to the default AnimationStack.
+        Player* player = element->document().timeline()->play(transition.get()).get();
+        m_transitions.set(id, player);
+    }
+}
+
+namespace {

[Steve Block, 2013/10/09 00:57:12]

You should merge this with the anonymous namespace at the top

[Timothy Loh, 2013/10/09 07:39:23]

Done. Also moved the existing function timingFromAnimationData to the anonymous
namespace.

+
+struct TransitionDetails {
+    TransitionDetails(PassRefPtr<AnimatableValue> from, PassRefPtr<AnimatableValue> to, const CSSAnimationData* anim)
+        : from(from), to(to), anim(anim)

[dstockwell, 2013/10/08 21:02:59]

place to and anim on separate lines for consistency

[Timothy Loh, 2013/10/09 01:33:07]

Done.

+    {
+    }
+    TransitionDetails() { } // The HashMap calls the default ctor
+    RefPtr<AnimatableValue> from;
+    RefPtr<AnimatableValue> to;
+    const CSSAnimationData* anim;
+};
+typedef HashMap<CSSPropertyID, TransitionDetails> StyleChange;
+
+void updateIfPropertyChanged(const CSSAnimationData* anim, CSSPropertyID id, const RenderStyle* oldStyle, const RenderStyle* newStyle, StyleChange& styleChange)
+{
+    RefPtr<AnimatableValue> from = CSSAnimatableValueFactory::create(id, oldStyle);

[dstockwell, 2013/10/08 21:02:59]

If there's already a transition running on this property we have already
constructed this animatable value. We can pull it out of the transition map.

+    RefPtr<AnimatableValue> to = CSSAnimatableValueFactory::create(id, newStyle);
+    // If we have multiple transitions on the same property, we will use the
+    // last one since we iterate over them in order and this will override
+    // a previously set TransitionDetails.
+    if (!from->equals(to.get()))
+        styleChange.add(id, TransitionDetails(from, to, anim));
+}
+
+void computeStyleChange(const RenderStyle* oldStyle, const RenderStyle* newStyle, StyleChange& styleChange, HashSet<CSSPropertyID>& listedProperties)
+{
+    if (!newStyle->transitions())
+        return;
+
+    for (size_t i = 0; i < newStyle->transitions()->size(); ++i) {
+        const CSSAnimationData* anim = newStyle->transitions()->animation(i);
+        CSSAnimationData::AnimationMode mode = anim->animationMode();
+        if (anim->duration() + anim->delay() <= 0 || mode == CSSAnimationData::AnimateNone)
+            continue;
+
+        if (mode == CSSAnimationData::AnimateAll) {
+            const Vector<CSSPropertyID>& animatableProperties = CSSAnimations::animatableProperties();
+            for (unsigned i = 0; i < animatableProperties.size(); ++i) {
+                CSSPropertyID id = animatableProperties[i];
+                listedProperties.add(id);
+                // FIXME: We haven't yet added support for all animatable properties,
+                // so this check lets us transition 'all' without failing to pull
+                // values out of the RenderStyles.
+                if (CSSPropertyAnimation::propertiesEqual(id, oldStyle, newStyle))

[dstockwell, 2013/10/08 21:02:59]

Could we just skip over the specific properties that are not yet implemented?

[Steve Block, 2013/10/09 00:57:12]

I don't see how testing for equality means we skip properties we haven't yet
implemented?

Also, given that we're close to having all values implemented, is there any need
for this work-around? How many LayoutTests does it affect?

[Timothy Loh, 2013/10/09 01:33:07]

Not sure what you mean here. The check is just so that when the virtual test
suite turns on, the tests using all will pass if they only depend on
transitioning properties we've implemented. We don't have the machinery to catch
RELEASE_ASSERTs.

[Timothy Loh, 2013/10/09 07:39:23]

OK. Will take this out for now. This takes it from 68/99 to 58/99.

+                    continue;
+                updateIfPropertyChanged(anim, id, oldStyle, newStyle, styleChange);
+            }
+            continue;
+        }
+

[dstockwell, 2013/10/08 21:02:59]

ASSERT mode == AnimateSingleProperty

[Timothy Loh, 2013/10/09 01:33:07]

Done.

+        const StylePropertyShorthand& propertyList = shorthandForProperty(anim->property());
+        if (!propertyList.length()) {
+            listedProperties.add(anim->property());
+            updateIfPropertyChanged(anim, anim->property(), oldStyle, newStyle, styleChange);
+        } else {
+            for (unsigned i = 0; i < propertyList.length(); ++i) {
+                CSSPropertyID id = propertyList.properties()[i];
+                if (!CSSAnimations::isAnimatableProperty(id))
+                    continue;
+                listedProperties.add(id);
+                updateIfPropertyChanged(anim, id, oldStyle, newStyle, styleChange);

[Steve Block, 2013/10/09 00:57:12]

Can you merge this block with the AnimateAll case to save duplication?

[Timothy Loh, 2013/10/09 07:39:23]

OK, I changed animatableProperties to return a const StylePropertyShorthand&.
Also making this skip the isAnimatableProperty check in the all case in case the
switch statement isn't cheap.

+            }
+        }
+    }
+}
+
+} // namespace
+
+void CSSAnimations::updateTransitionUpdate(CSSAnimationUpdate* update, CSSPropertyID id, const AnimatableValue* from, const AnimatableValue* to, const CSSAnimationData* anim, const TransitionMap* transitions)

[dstockwell, 2013/10/08 21:02:59]

calculateTransitionUpdateForProperty

[Timothy Loh, 2013/10/09 01:33:07]

Done.

+{
+    // FIXME: Skip the rest of this if there is a running animation on this property
+
+    Timing timing;
+    RefPtr<TimingFunction> timingFunction = timingFromAnimationData(anim, timing);
+    timing.timingFunction = timingFunction;
+    timing.fillMode = Timing::FillModeBoth;
+
+    if (transitions) {
+        TransitionMap::const_iterator existingTransitionIter = transitions->find(id);
+
+        if (existingTransitionIter != transitions->end() && !update->endedTransitions().contains(id)) {
+            const Animation* animation = static_cast<const Animation*>(existingTransitionIter->value->source());

[dstockwell, 2013/10/08 21:02:59]

I don't like reaching in here to pull out the target value. I think we should
store the from/to values that we need separately as part of a struct in the
transition map.

[Steve Block, 2013/10/09 00:57:12]

Agreed

[Timothy Loh, 2013/10/09 07:39:23]

OK. Plumbing these explicitly through now. Also changed the HashMap of new
transitions to a Vector.

+            const KeyframeAnimationEffect* existingEffect = static_cast<const KeyframeAnimationEffect*>(animation->effect());
+
+            ASSERT(existingEffect->getFrames().size() == 2);
+            const AnimatableValue* existingTo = existingEffect->getFrames()[1]->propertyValue(id);
+
+            if (existingTo->equals(to))
+                return;
+            update->endTransition(id);
+        }
+    }
+
+    KeyframeAnimationEffect::KeyframeVector keyframes;
+
+    RefPtr<Keyframe> startKeyframe = Keyframe::create();
+    startKeyframe->setPropertyValue(id, from);
+    startKeyframe->setOffset(0);
+    keyframes.append(startKeyframe);
+
+    RefPtr<Keyframe> endKeyframe = Keyframe::create();
+    endKeyframe->setPropertyValue(id, to);
+    endKeyframe->setOffset(1);
+    keyframes.append(endKeyframe);
+
+    RefPtr<KeyframeAnimationEffect> effect = KeyframeAnimationEffect::create(keyframes);
+
+    update->startTransition(id, InertAnimation::create(effect, timing));
+}
+
+void CSSAnimations::updateTransitionUpdate(CSSAnimationUpdate* update, const Element* element, const RenderStyle* style)
+{
+    ActiveAnimations* activeAnimations = element->activeAnimations();
+    const CSSAnimations* cssAnimations = activeAnimations ? activeAnimations->cssAnimations() : 0;
+    const TransitionMap* transitions = cssAnimations ? &cssAnimations->m_transitions : 0;
+
+    HashSet<CSSPropertyID> listedProperties;
+    if (style->display() != NONE && element->renderer() && element->renderer()->style()) {

[dstockwell, 2013/10/08 21:02:59]

if display is none, there's should be nothing to do at all

[Timothy Loh, 2013/10/09 01:33:07]

For animations, it looks like we'll set all the running animations to be
cancelled in the update. This does the same for transitions.

+        StyleChange styleChange;
+        computeStyleChange(element->renderer()->style(), style, styleChange, listedProperties);
+        for (StyleChange::const_iterator iter = styleChange.begin(); iter != styleChange.end(); ++iter) {
+            CSSPropertyID id = iter->key;
+            AnimatableValue* from = iter->value.from.get();
+            AnimatableValue* to = iter->value.to.get();
+            const CSSAnimationData* anim = iter->value.anim;
+            updateTransitionUpdate(update, id, from, to, anim, transitions);
+        }
+    }
+
+    if (transitions) {
+        for (TransitionMap::const_iterator iter = transitions->begin(); iter != transitions->end(); ++iter) {
+            const TimedItem* timedItem = iter->value->source();
+            bool isReversed = timedItem->specified().playbackRate < 0;

[dstockwell, 2013/10/08 21:02:59]

Reversing doesn't seem to be implemented, can we remove all this logic?

[Timothy Loh, 2013/10/09 01:33:07]

Whoops, I pulled most of it out for a separate patch, missed this section.

+            TimedItem::Phase end = isReversed ? TimedItem::PhaseBefore : TimedItem::PhaseAfter;
+            if (timedItem->phase() == end || !listedProperties.contains(iter->key))
+                update->endTransition(iter->key);
+        }
+    }
+}
+
+
+AnimationEffect::CompositableValueMap CSSAnimations::compositableValuesForTransitions(const CSSAnimations* cssAnimations, const CSSAnimationUpdate* update)

[dstockwell, 2013/10/08 21:02:59]

If the transitions are being added to the document timeline does this do
anything? If we're applying transitions twice we can defer adding this for now.
I think we need a different design, reaching into the animation to access the
compositableValues doesn't make sense here.

[Timothy Loh, 2013/10/09 07:39:23]

This mean that frame 0 will show the end state, since applyAnimatedProperties
won't look at the transitions part of the update. But OK, will take this out for
now, since it'll probably end up elsewhere.
We chatted about this in person, details at the top of this reply.

+{
+    // FIXME: Transitions shouldn't apply values for properties with a running CSS Animation.
+    AnimationEffect::CompositableValueMap result;
+
+    if (cssAnimations) {
+        const TransitionMap& transitions = cssAnimations->m_transitions;
+        for (TransitionMap::const_iterator iter = transitions.begin(); iter != transitions.end(); ++iter) {
+            CSSPropertyID id = iter->key;
+            if (update && update->endedTransitions().contains(id))
+                continue;
+            Player* player = iter->value.get();
+            Animation* animation = static_cast<Animation*>(player->source());
+            ASSERT(animation && animation->compositableValues());
+            const AnimationEffect::CompositableValueMap* compositableValues = animation->compositableValues();
+            ASSERT(compositableValues->size() == 1 && compositableValues->contains(id));
+            result.set(id, compositableValues->get(id));
+        }
+    }
+
+    if (update) {
+        HashMap<CSSPropertyID, RefPtr<InertAnimation> >::const_iterator iter;

[dstockwell, 2013/10/08 21:02:59]

Put this on the next line

[Timothy Loh, 2013/10/09 01:33:07]

Makes the line a bit long but OK.

+        for (iter = update->newTransitions().begin(); iter != update->newTransitions().end(); ++iter) {
+            CSSPropertyID id = iter->key;
+            InertAnimation* animation = iter->value.get();
+            OwnPtr<AnimationEffect::CompositableValueMap> compositableValues = animation->sample();
+            ASSERT(compositableValues->size() == 1 && compositableValues->contains(id));
+            result.set(id, compositableValues->get(id));
+        }
+    }
+
+    return result;
 }
 
 void CSSAnimations::cancel()
@@ -244,17 +445,20 @@ void CSSAnimations::cancel()
             (*animationsIter)->cancel();
     }
 
+    for (TransitionMap::iterator iter = m_transitions.begin(); iter != m_transitions.end(); ++iter)
+        iter->value->cancel();
+
     m_animations.clear();

[dstockwell, 2013/10/08 21:02:59]

m_transitions.clear()

[Timothy Loh, 2013/10/09 01:33:07]

Done.

     m_pendingUpdate = nullptr;
 }
 
-void CSSAnimations::EventDelegate::maybeDispatch(Document::ListenerType listenerType, AtomicString& eventName, double elapsedTime)
+void CSSAnimations::AnimationEventDelegate::maybeDispatch(Document::ListenerType listenerType, AtomicString& eventName, double elapsedTime)
 {
     if (m_target->document().hasListenerType(listenerType))
         m_target->document().timeline()->addEventToDispatch(m_target, WebKitAnimationEvent::create(eventName, m_name, elapsedTime));
 }
 
-void CSSAnimations::EventDelegate::onEventCondition(const TimedItem* timedItem, bool isFirstSample, TimedItem::Phase previousPhase, double previousIteration)
+void CSSAnimations::AnimationEventDelegate::onEventCondition(const TimedItem* timedItem, bool isFirstSample, TimedItem::Phase previousPhase, double previousIteration)
 {
     // Events for a single document are queued and dispatched as a group at
     // the end of DocumentTimeline::serviceAnimations.
@@ -287,6 +491,26 @@ void CSSAnimations::EventDelegate::onEventCondition(const TimedItem* timedItem,
         maybeDispatch(Document::ANIMATIONEND_LISTENER, eventNames().animationendEvent, timedItem->activeDuration());
 }
 
+void CSSAnimations::TransitionEventDelegate::onEventCondition(const TimedItem* timedItem, bool isFirstSample, TimedItem::Phase previousPhase, double previousIteration)
+{
+    // Events for a single document are queued and dispatched as a group at
+    // the end of DocumentTimeline::serviceAnimations.
+    // FIXME: Events which are queued outside of serviceAnimations should
+    // trigger a timer to dispatch when control is released.
+    const TimedItem::Phase currentPhase = timedItem->phase();
+    if (currentPhase == TimedItem::PhaseActive || previousPhase == currentPhase)

[dstockwell, 2013/10/08 21:02:59]

Shouldn't we only be firing when currentPhase becomes PhaseAfter?

[Timothy Loh, 2013/10/09 01:33:07]

Tweaked condition slightly.

+        return;
+    if (m_target->document().hasListenerType(Document::TRANSITIONEND_LISTENER)) {
+        String propertyName = getPropertyNameString(m_id);
+        const Timing& timing = timedItem->specified();
+        double elapsedTime = timing.iterationDuration;
+        const AtomicString& eventType = eventNames().transitionendEvent;
+        String pseudoElement = PseudoElement::pseudoElementNameForEvents(m_target->pseudoId());
+        m_target->document().timeline()->addEventToDispatch(m_target, TransitionEvent::create(eventType, propertyName, elapsedTime, pseudoElement));
+    }
+}
+
+
 bool CSSAnimations::isAnimatableProperty(CSSPropertyID property)
 {
     switch (property) {
@@ -398,4 +622,17 @@ bool CSSAnimations::isAnimatableProperty(CSSPropertyID property)
     }
 }
 
+const Vector<CSSPropertyID>& CSSAnimations::animatableProperties()
+{
+    DEFINE_STATIC_LOCAL(Vector<CSSPropertyID>, properties, ());
+    if (properties.isEmpty()) {
+        for (int i = firstCSSProperty; i < lastCSSProperty; ++i) {
+            CSSPropertyID id = convertToCSSPropertyID(i);
+            if (isAnimatableProperty(id))
+                properties.append(id);
+        }
+    }
+    return properties;
+}
+
 } // namespace WebCore