Bug 1303233 - Part 2: Reverse composite order of effects. r?birtles
MozReview-Commit-ID: FPGB07KCvE3
--- 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.
//