Animation: Cancel same-property animations on compositor

Source/core/animation/CompositorAnimations.cpp

Index: Source/core/animation/CompositorAnimations.cpp
diff --git a/Source/core/animation/CompositorAnimations.cpp b/Source/core/animation/CompositorAnimations.cpp
index 16aa2904cb069e1fd2b64a5d80eea28198202ed2..66b4dfe3fbb6d073ac586e1f15ae977301d3abd7 100644
--- a/Source/core/animation/CompositorAnimations.cpp
+++ b/Source/core/animation/CompositorAnimations.cpp
@@ -31,6 +31,7 @@
 #include "config.h"
 #include "core/animation/CompositorAnimations.h"
 
+#include "core/animation/ActiveAnimations.h"
 #include "core/animation/AnimationNode.h"
 #include "core/animation/AnimationTranslationUtil.h"
 #include "core/animation/CompositorAnimationsImpl.h"
@@ -70,6 +71,37 @@ void getKeyframeValuesForProperty(const KeyframeEffectModelBase* effect, CSSProp
     }
 }
 
+bool alreadyActiveAnimationsBlockProperties(const Element& targetElement, const AnimationPlayer& playerToAdd, const AnimationEffect& effectToAdd)

[dstockwell, 2015/01/28 05:44:05]

I think this would be better written as:

bool affectsOpacity = effectToAdd.affects(CSSPropertyOpacity);

... for ...
  if (affectsOpacity && attachedPlayer->affects(...Opacity))
    return true;

return false;


Also the name is hard, if we went with 'incompatible' it could be just
'hasIncompatatibleAnimations'

[loyso (OOO), 2015/01/28 06:33:31]

Good point. That's also will be more efficient. 
That's more generic but good. Thanks!

+{
+    bool propertyOpacityBlocked = false;
+    bool propertyTransformBlocked = false;
+    bool propertyFilterBlocked = false;
+
+    if (targetElement.hasActiveAnimations()) {
+        ActiveAnimations* activeAnimations = targetElement.activeAnimations();
+        ASSERT(activeAnimations);
+
+        for (const auto& entry : activeAnimations->players()) {
+            const AnimationPlayer* attachedPlayer = entry.key;

[dstockwell, 2015/01/28 05:44:05]

Is it possible to skip consideration of earlier players which are filling?

[loyso (OOO), 2015/01/29 01:08:54]

What do you mean? Players with fill=forwards will be in
activeAnimations().players(), I suppose.

[dstockwell, 2015/01/29 01:14:07]

Yes. Hmm, also players which are idle or finished will be in the list.

[loyso (OOO), 2015/01/29 04:35:59]

Is that a problem? Should we address it?

+            if (attachedPlayer == &playerToAdd)
+                continue;
+
+            if (attachedPlayer->affects(CSSPropertyOpacity))
+                propertyOpacityBlocked = true;
+            else if (attachedPlayer->affects(CSSPropertyTransform))
+                propertyTransformBlocked = true;
+            else if (attachedPlayer->affects(CSSPropertyWebkitFilter))
+                propertyFilterBlocked = true;
+        }
+    }
+
+    if ((propertyOpacityBlocked && effectToAdd.affects(CSSPropertyOpacity))
+        || (propertyTransformBlocked && effectToAdd.affects(CSSPropertyTransform))
+        || (propertyFilterBlocked && effectToAdd.affects(CSSPropertyWebkitFilter)))
+        return true;
+
+    return false;
+}
 }
 
 bool CompositorAnimations::getAnimatedBoundingBox(FloatBox& box, const AnimationEffect& effect, double minValue, double maxValue) const
@@ -133,12 +165,14 @@ bool CompositorAnimations::getAnimatedBoundingBox(FloatBox& box, const Animation
 // CompositorAnimations public API
 // -----------------------------------------------------------------------
 
-bool CompositorAnimations::isCandidateForAnimationOnCompositor(const Timing& timing, const AnimationEffect& effect, double playerPlaybackRate)
+bool CompositorAnimations::isCandidateForAnimationOnCompositor(const Timing& timing, const Element* targetElement, const AnimationPlayer* playerToAdd, const AnimationEffect& effect, double playerPlaybackRate)

[dstockwell, 2015/01/28 05:44:05]

When can targetElement be null?

[loyso (OOO), 2015/01/28 06:33:31]

Unit tests. I could setup a fake one there if we need the reference type badly.

[loyso (OOO), 2015/01/28 23:18:34]

Do we need it?

[dstockwell, 2015/01/29 01:14:07]

Unless it's non-trivial to create an Element in the test I think we should
prefer targetElement to be non-nullable.

 {
+    if (targetElement && playerToAdd && alreadyActiveAnimationsBlockProperties(*targetElement, *playerToAdd, effect))
+        return false;
+
     const KeyframeEffectModelBase& keyframeEffect = toKeyframeEffectModelBase(effect);
 
     PropertySet properties = keyframeEffect.properties();
-
     if (properties.isEmpty())
         return false;
 
@@ -164,6 +198,7 @@ bool CompositorAnimations::isCandidateForAnimationOnCompositor(const Timing& tim
                 break;
             }
             default:
+                // any other types are not allowed to run on compositor.
                 return false;
             }
         }
@@ -176,15 +211,39 @@ bool CompositorAnimations::isCandidateForAnimationOnCompositor(const Timing& tim
     return true;
 }
 
+void CompositorAnimations::cancelAffectedAnimationsOnCompositor(const Element& targetElement, const AnimationPlayer& playerToAdd, const AnimationEffect& effectToAdd)
+{
+    const bool propertyOpacityCancel = effectToAdd.affects(CSSPropertyOpacity);
+    const bool propertyTransformCancel = effectToAdd.affects(CSSPropertyTransform);
+    const bool propertyFilterCancel = effectToAdd.affects(CSSPropertyWebkitFilter);
+
+    if (!targetElement.hasActiveAnimations())
+        return;
+
+    ActiveAnimations* activeAnimations = targetElement.activeAnimations();
+    ASSERT(activeAnimations);
+
+    for (const auto& entry : activeAnimations->players()) {
+        AnimationPlayer* attachedPlayer = entry.key;
+        if (attachedPlayer == &playerToAdd)
+            continue;
+
+        if ((propertyOpacityCancel && attachedPlayer->affects(CSSPropertyOpacity))
+            || (propertyTransformCancel && attachedPlayer->affects(CSSPropertyTransform))
+            || (propertyFilterCancel && attachedPlayer->affects(CSSPropertyWebkitFilter)))
+            attachedPlayer->cancelAnimationOnCompositor();
+    }
+}
+
 bool CompositorAnimations::canStartAnimationOnCompositor(const Element& element)
 {
     return element.renderer() && element.renderer()->compositingState() == PaintsIntoOwnBacking;
 }
 
-bool CompositorAnimations::startAnimationOnCompositor(const Element& element, int group, double startTime, double timeOffset, const Timing& timing, const AnimationEffect& effect, Vector<int>& startedAnimationIds, double playerPlaybackRate)
+bool CompositorAnimations::startAnimationOnCompositor(const Element& element, int group, double startTime, double timeOffset, const Timing& timing, const AnimationPlayer* player, const AnimationEffect& effect, Vector<int>& startedAnimationIds, double playerPlaybackRate)
 {
     ASSERT(startedAnimationIds.isEmpty());
-    ASSERT(isCandidateForAnimationOnCompositor(timing, effect, playerPlaybackRate));
+    ASSERT(isCandidateForAnimationOnCompositor(timing, &element, player, effect, playerPlaybackRate));
     ASSERT(canStartAnimationOnCompositor(element));
 
     const KeyframeEffectModelBase& keyframeEffect = toKeyframeEffectModelBase(effect);