Bug 1303233 - Part 2: Reverse composite order of effects. r?birtles draft
authorHiroyuki Ikezoe <hiikezoe@mozilla-japan.org>
Fri, 23 Sep 2016 13:57:52 +0900
changeset 416838 e043cfc30baa8284bc56929fd31cf169f0e18808
parent 416837 75073bd4e454f06660f87e93dee0c97c3f3bac4f
child 531964 13b31807bf8f8f25aeadf0b092aa24670d92e6a9
push id30262
push userbmo:hiikezoe@mozilla-japan.org
push dateFri, 23 Sep 2016 04:58:45 +0000
reviewersbirtles
bugs1303233
milestone52.0a1
Bug 1303233 - Part 2: Reverse composite order of effects. r?birtles MozReview-Commit-ID: FPGB07KCvE3
dom/animation/Animation.cpp
dom/animation/Animation.h
dom/animation/EffectCompositor.cpp
dom/animation/KeyframeEffectReadOnly.cpp
dom/animation/KeyframeEffectReadOnly.h
--- a/dom/animation/Animation.cpp
+++ b/dom/animation/Animation.cpp
@@ -830,18 +830,17 @@ Animation::HasLowerCompositeOrderThan(co
              "Animation indices should be unique");
 
   // 3. Finally, generic animations sort by their position in the global
   // animation array.
   return mAnimationIndex < aOther.mAnimationIndex;
 }
 
 void
-Animation::ComposeStyle(RefPtr<AnimValuesStyleRule>& aStyleRule,
-                        nsCSSPropertyIDSet& aSetProperties)
+Animation::ComposeStyle(RefPtr<AnimValuesStyleRule>& aStyleRule)
 {
   if (!mEffect) {
     return;
   }
 
   if (!IsInEffect()) {
     return;
   }
@@ -896,17 +895,17 @@ Animation::ComposeStyle(RefPtr<AnimValue
       if (!timeToUse.IsNull()) {
         mHoldTime.SetValue((timeToUse.Value() - mStartTime.Value())
                             .MultDouble(mPlaybackRate));
       }
     }
 
     KeyframeEffectReadOnly* keyframeEffect = mEffect->AsKeyframeEffect();
     if (keyframeEffect) {
-      keyframeEffect->ComposeStyle(aStyleRule, aSetProperties);
+      keyframeEffect->ComposeStyle(aStyleRule);
     }
   }
 
   MOZ_ASSERT(playState == PlayState(),
              "Play state should not change during the course of compositing");
   mFinishedAtLastComposeStyle = (playState == AnimationPlayState::Finished);
 }
 
--- a/dom/animation/Animation.h
+++ b/dom/animation/Animation.h
@@ -305,21 +305,18 @@ public:
    * Returns true if this animation does not currently need to update
    * style on the main thread (e.g. because it is empty, or is
    * running on the compositor).
    */
   bool CanThrottle() const;
   /**
    * Updates |aStyleRule| with the animation values of this animation's effect,
    * if any.
-   * Any properties already contained in |aSetProperties| are not changed. Any
-   * properties that are changed are added to |aSetProperties|.
    */
-  void ComposeStyle(RefPtr<AnimValuesStyleRule>& aStyleRule,
-                    nsCSSPropertyIDSet& aSetProperties);
+  void ComposeStyle(RefPtr<AnimValuesStyleRule>& aStyleRule);
 
   void NotifyEffectTimingUpdated();
 
 protected:
   void SilentlySetCurrentTime(const TimeDuration& aNewCurrentTime);
   void SilentlySetPlaybackRate(double aPlaybackRate);
   void CancelNoUpdate();
   void PlayNoUpdate(ErrorResult& aRv, LimitBehavior aLimitBehavior);
--- a/dom/animation/EffectCompositor.cpp
+++ b/dom/animation/EffectCompositor.cpp
@@ -581,24 +581,21 @@ EffectCompositor::ComposeAnimationRule(d
     }
   }
   sortedEffectList.Sort(EffectCompositeOrderComparator());
 
   RefPtr<AnimValuesStyleRule>& animationRule =
     effects->AnimationRule(aCascadeLevel);
   animationRule = nullptr;
 
-  // If multiple animations specify behavior for the same property the
-  // animation with the *highest* composite order wins.
-  // As a result, we iterate from last animation to first and, if a
-  // property has already been set, we don't change it.
-  nsCSSPropertyIDSet properties;
-
-  for (KeyframeEffectReadOnly* effect : Reversed(sortedEffectList)) {
-    effect->GetAnimation()->ComposeStyle(animationRule, properties);
+  // If multiple animations affect the same property, animations with higher
+  // composite order (priority) override or add or animations with lower
+  // priority.
+  for (KeyframeEffectReadOnly* effect : sortedEffectList) {
+    effect->GetAnimation()->ComposeStyle(animationRule);
   }
 
   MOZ_ASSERT(effects == EffectSet::GetEffectSet(aElement, aPseudoType),
              "EffectSet should not change while composing style");
 
   effects->UpdateAnimationRuleRefreshTime(aCascadeLevel, aRefreshTime);
 }
 
--- a/dom/animation/KeyframeEffectReadOnly.cpp
+++ b/dom/animation/KeyframeEffectReadOnly.cpp
@@ -299,18 +299,17 @@ KeyframeEffectReadOnly::UpdateProperties
   CalculateCumulativeChangeHint(aStyleContext);
 
   MarkCascadeNeedsUpdate();
 
   RequestRestyle(EffectCompositor::RestyleType::Layer);
 }
 
 void
-KeyframeEffectReadOnly::ComposeStyle(RefPtr<AnimValuesStyleRule>& aStyleRule,
-                                     nsCSSPropertyIDSet& aSetProperties)
+KeyframeEffectReadOnly::ComposeStyle(RefPtr<AnimValuesStyleRule>& aStyleRule)
 {
   // Basically this function is called only while the effect is in-effect,
   // but there is a race condition that animation's current time gets back to
   // before phase or forward to after phase due to temporary hold time tweaks in
   // Animation::ComposeStyle().
   if (!IsInEffect()) {
     return;
   }
@@ -333,24 +332,18 @@ KeyframeEffectReadOnly::ComposeStyle(Ref
   for (size_t propIdx = 0, propEnd = mProperties.Length();
        propIdx != propEnd; ++propIdx)
   {
     const AnimationProperty& prop = mProperties[propIdx];
 
     MOZ_ASSERT(prop.mSegments[0].mFromKey == 0.0, "incorrect first from key");
     MOZ_ASSERT(prop.mSegments[prop.mSegments.Length() - 1].mToKey == 1.0,
                "incorrect last to key");
-
-    if (aSetProperties.HasProperty(prop.mProperty)) {
-      // Animations are composed by EffectCompositor by iterating
-      // from the last animation to first. For animations targetting the
-      // same property, the later one wins. So if this property is already set,
-      // we should not override it.
-      continue;
-    }
+    MOZ_ASSERT(prop.mSegments.Length() > 0,
+               "property should not be in animations if it has no segments");
 
     if (effectSet->WinnerInCascade(prop.mProperty) ==
           EffectSet::WinnerCascadeLevel::None ||
         (cascadeLevel == EffectCompositor::CascadeLevel::Transitions &&
          effectSet->WinnerInCascade(prop.mProperty) !=
            EffectSet::WinnerCascadeLevel::TransitionsOnly)) {
       // This isn't the winning declaration, so don't add it to style.
       // For transitions, this is important, because it's how we
@@ -360,21 +353,16 @@ KeyframeEffectReadOnly::ComposeStyle(Ref
       // overridden.
       // NOTE: In the case when the winner level is 'Both', transition level
       // effect has to be composed just right before composing animation level
       // effect so that we can add or accumulate animation effect onto
       // transition effect.
       continue;
     }
 
-    aSetProperties.AddProperty(prop.mProperty);
-
-    MOZ_ASSERT(prop.mSegments.Length() > 0,
-               "property should not be in animations if it has no segments");
-
     // FIXME: Maybe cache the current segment?
     const AnimationPropertySegment *segment = prop.mSegments.Elements(),
                                 *segmentEnd = segment + prop.mSegments.Length();
     while (segment->mToKey <= computedTiming.mProgress.Value()) {
       MOZ_ASSERT(segment->mFromKey <= segment->mToKey, "incorrect keys");
       if ((segment+1) == segmentEnd) {
         break;
       }
--- a/dom/animation/KeyframeEffectReadOnly.h
+++ b/dom/animation/KeyframeEffectReadOnly.h
@@ -239,21 +239,18 @@ public:
     return mProperties;
   }
 
   // Update |mProperties| by recalculating from |mKeyframes| using
   // |aStyleContext| to resolve specified values.
   void UpdateProperties(nsStyleContext* aStyleContext);
 
   // Updates |aStyleRule| with the animation values produced by this
-  // AnimationEffect for the current time except any properties already
-  // contained in |aSetProperties|.
-  // Any updated properties are added to |aSetProperties|.
-  void ComposeStyle(RefPtr<AnimValuesStyleRule>& aStyleRule,
-                    nsCSSPropertyIDSet& aSetProperties);
+  // AnimationEffect for the current time.
+  void ComposeStyle(RefPtr<AnimValuesStyleRule>& aStyleRule);
   // Returns true if at least one property is being animated on compositor.
   bool IsRunningOnCompositor() const;
   void SetIsRunningOnCompositor(nsCSSPropertyID aProperty, bool aIsRunning);
   void ResetIsRunningOnCompositor();
 
   // Returns true if this effect, applied to |aFrame|, contains properties
   // that mean we shouldn't run transform compositor animations on this element.
   //