diff --git a/dom/animation/EffectCompositor.cpp b/dom/animation/EffectCompositor.cpp index 23ee135eaa9a..a740e4990bf7 100644 --- a/dom/animation/EffectCompositor.cpp +++ b/dom/animation/EffectCompositor.cpp @@ -148,7 +148,7 @@ FindAnimationsForCompositor(const nsIFrame* aFrame, return false; } - if (!effect->HasAnimationOfProperty(aProperty)) { + if (!effect->HasEffectiveAnimationOfProperty(aProperty)) { continue; } diff --git a/dom/animation/KeyframeEffectReadOnly.cpp b/dom/animation/KeyframeEffectReadOnly.cpp index a8fdde9c6649..bf5f495ad1ff 100644 --- a/dom/animation/KeyframeEffectReadOnly.cpp +++ b/dom/animation/KeyframeEffectReadOnly.cpp @@ -192,7 +192,8 @@ KeyframeEffectReadOnly::SetKeyframes(nsTArray&& aKeyframes, } const AnimationProperty* -KeyframeEffectReadOnly::GetAnimationOfProperty(nsCSSPropertyID aProperty) const +KeyframeEffectReadOnly::GetEffectiveAnimationOfProperty( + nsCSSPropertyID aProperty) const { if (!IsInEffect()) { return nullptr; @@ -219,6 +220,17 @@ KeyframeEffectReadOnly::GetAnimationOfProperty(nsCSSPropertyID aProperty) const return nullptr; } +bool +KeyframeEffectReadOnly::HasAnimationOfProperty(nsCSSPropertyID aProperty) const +{ + for (const AnimationProperty& property : mProperties) { + if (property.mProperty == aProperty) { + return true; + } + } + return false; +} + #ifdef DEBUG bool SpecifiedKeyframeArraysAreEqual(const nsTArray& aA, @@ -926,10 +938,12 @@ KeyframeEffectReadOnly::CanThrottle() const // already running on compositor. for (const LayerAnimationInfo::Record& record : LayerAnimationInfo::sRecords) { - // Skip properties that are overridden in the cascade. - // (GetAnimationOfProperty, as called by HasAnimationOfProperty, - // only returns an animation if it currently wins in the cascade.) - if (!HasAnimationOfProperty(record.mProperty)) { + // Skip properties that are overridden by !important rules. + // (GetEffectiveAnimationOfProperty, as called by + // HasEffectiveAnimationOfProperty, only returns a property which is + // neither overridden by !important rules nor overridden by other + // animation.) + if (!HasEffectiveAnimationOfProperty(record.mProperty)) { continue; } diff --git a/dom/animation/KeyframeEffectReadOnly.h b/dom/animation/KeyframeEffectReadOnly.h index b2b6d4dca470..1e506ff67e06 100644 --- a/dom/animation/KeyframeEffectReadOnly.h +++ b/dom/animation/KeyframeEffectReadOnly.h @@ -235,12 +235,25 @@ public: ErrorResult& aRv); void SetKeyframes(nsTArray&& aKeyframes, nsStyleContext* aStyleContext); - const AnimationProperty* - GetAnimationOfProperty(nsCSSPropertyID aProperty) const; - bool HasAnimationOfProperty(nsCSSPropertyID aProperty) const + + // Returns true if the effect includes |aProperty| regardless of whether the + // property is overridden by !important rule. + bool HasAnimationOfProperty(nsCSSPropertyID aProperty) const; + + // GetEffectiveAnimationOfProperty returns AnimationProperty corresponding + // to a given CSS property if the effect includes the property and the + // property is not overridden by !important rules. + // Also EffectiveAnimationOfProperty returns true under the same condition. + // + // NOTE: We don't currently check for !important rules for properties that + // can't run on the compositor. + bool HasEffectiveAnimationOfProperty(nsCSSPropertyID aProperty) const { - return GetAnimationOfProperty(aProperty) != nullptr; + return GetEffectiveAnimationOfProperty(aProperty) != nullptr; } + const AnimationProperty* GetEffectiveAnimationOfProperty( + nsCSSPropertyID aProperty) const; + const InfallibleTArray& Properties() const { return mProperties; diff --git a/layout/base/RestyleManager.cpp b/layout/base/RestyleManager.cpp index ca90e8009e88..8e5936c979cb 100644 --- a/layout/base/RestyleManager.cpp +++ b/layout/base/RestyleManager.cpp @@ -1569,8 +1569,7 @@ ElementRestyler::AddLayerChangesForAnimation() // setKeyframes or changing target element from other target which prevents // running on the compositor, etc. if (!layer && - nsLayoutUtils::HasRelevantAnimationOfProperty(mFrame, - layerInfo.mProperty)) { + nsLayoutUtils::HasEffectiveAnimation(mFrame, layerInfo.mProperty)) { hint |= layerInfo.mChangeHint; } } diff --git a/layout/base/nsDisplayList.cpp b/layout/base/nsDisplayList.cpp index 98d9cb810bb0..cd174dbcd927 100644 --- a/layout/base/nsDisplayList.cpp +++ b/layout/base/nsDisplayList.cpp @@ -487,21 +487,21 @@ AddAnimationsForProperty(nsIFrame* aFrame, nsCSSPropertyID aProperty, MOZ_ASSERT(keyframeEffect, "A playing animation should have a keyframe effect"); const AnimationProperty* property = - keyframeEffect->GetAnimationOfProperty(aProperty); + keyframeEffect->GetEffectiveAnimationOfProperty(aProperty); if (!property) { continue; } // Note that if the property is overridden by !important rules, - // GetAnimationOfProperty returns null instead. + // GetEffectiveAnimationOfProperty returns null instead. // This is what we want, since if we have animations overridden by // !important rules, we don't want to send them to the compositor. MOZ_ASSERT(anim->CascadeLevel() != EffectCompositor::CascadeLevel::Animations || !effects->PropertiesWithImportantRules() .HasProperty(aProperty), - "GetAnimationOfProperty already tested the property is not " - "overridden by !important rules"); + "GetEffectiveAnimationOfProperty already tested the property " + "is not overridden by !important rules"); // Don't add animations that are pending if their timeline does not // track wallclock time. This is because any pending animations on layers diff --git a/layout/base/nsLayoutUtils.cpp b/layout/base/nsLayoutUtils.cpp index cc047bb0bed4..fa1d97c17b6f 100644 --- a/layout/base/nsLayoutUtils.cpp +++ b/layout/base/nsLayoutUtils.cpp @@ -483,7 +483,7 @@ nsLayoutUtils::HasActiveAnimationOfProperty(const nsIFrame* aFrame, [&aProperty](KeyframeEffectReadOnly& aEffect) { return aEffect.IsCurrent() && aEffect.IsInEffect() && - aEffect.HasAnimationOfProperty(aProperty); + aEffect.HasEffectiveAnimationOfProperty(aProperty); } ); } @@ -502,8 +502,8 @@ nsLayoutUtils::HasCurrentTransitions(const nsIFrame* aFrame) } bool -nsLayoutUtils::HasRelevantAnimationOfProperty(const nsIFrame* aFrame, - nsCSSPropertyID aProperty) +nsLayoutUtils::HasAnimationOfProperty(const nsIFrame* aFrame, + nsCSSPropertyID aProperty) { return HasMatchingAnimations(aFrame, [&aProperty](KeyframeEffectReadOnly& aEffect) @@ -514,6 +514,19 @@ nsLayoutUtils::HasRelevantAnimationOfProperty(const nsIFrame* aFrame, ); } +bool +nsLayoutUtils::HasEffectiveAnimation(const nsIFrame* aFrame, + nsCSSPropertyID aProperty) +{ + return HasMatchingAnimations(aFrame, + [&aProperty](KeyframeEffectReadOnly& aEffect) + { + return (aEffect.IsInEffect() || aEffect.IsCurrent()) && + aEffect.HasEffectiveAnimationOfProperty(aProperty); + } + ); +} + static float GetSuitableScale(float aMaxScale, float aMinScale, nscoord aVisibleDimension, nscoord aDisplayDimension) diff --git a/layout/base/nsLayoutUtils.h b/layout/base/nsLayoutUtils.h index 1931fadea36f..cb9d0c1537ad 100644 --- a/layout/base/nsLayoutUtils.h +++ b/layout/base/nsLayoutUtils.h @@ -2244,12 +2244,18 @@ public: static bool HasCurrentTransitions(const nsIFrame* aFrame); /** - * Returns true if the frame has current or in-effect (i.e. in before phase, - * running or filling) animations or transitions for the - * property. + * Returns true if |aFrame| has an animation of |aProperty| regardless of + * whether the property is overridden by !important rule. */ - static bool HasRelevantAnimationOfProperty(const nsIFrame* aFrame, - nsCSSPropertyID aProperty); + static bool HasAnimationOfProperty(const nsIFrame* aFrame, + nsCSSPropertyID aProperty); + + /** + * Returns true if |aFrame| has an animation of |aProperty| which is + * not overridden by !important rules. + */ + static bool HasEffectiveAnimation(const nsIFrame* aFrame, + nsCSSPropertyID aProperty); /** * Checks if off-main-thread animations are enabled. diff --git a/layout/generic/nsFrame.cpp b/layout/generic/nsFrame.cpp index 5184c69c94ca..b0ec8922c162 100644 --- a/layout/generic/nsFrame.cpp +++ b/layout/generic/nsFrame.cpp @@ -554,8 +554,7 @@ nsFrame::Init(nsIContent* aContent, } const nsStyleDisplay *disp = StyleDisplay(); if (disp->HasTransform(this) || - nsLayoutUtils::HasRelevantAnimationOfProperty(this, - eCSSProperty_transform)) { + nsLayoutUtils::HasAnimationOfProperty(this, eCSSProperty_transform)) { // The frame gets reconstructed if we toggle the -moz-transform // property, so we can set this bit here and then ignore it. mState |= NS_FRAME_MAY_BE_TRANSFORMED; @@ -1140,8 +1139,8 @@ nsIFrame::IsTransformed() const (StyleDisplay()->HasTransform(this) || IsSVGTransformed() || (mContent && - nsLayoutUtils::HasRelevantAnimationOfProperty( - this, eCSSProperty_transform) && + nsLayoutUtils::HasAnimationOfProperty(this, + eCSSProperty_transform) && IsFrameOfType(eSupportsCSSTransforms) && mContent->GetPrimaryFrame() == this))); } @@ -1153,8 +1152,7 @@ nsIFrame::HasOpacityInternal(float aThreshold) const return StyleEffects()->mOpacity < aThreshold || (StyleDisplay()->mWillChangeBitField & NS_STYLE_WILL_CHANGE_OPACITY) || (mContent && - nsLayoutUtils::HasRelevantAnimationOfProperty( - this, eCSSProperty_opacity) && + nsLayoutUtils::HasAnimationOfProperty(this, eCSSProperty_opacity) && mContent->GetPrimaryFrame() == this); } @@ -2115,8 +2113,7 @@ nsIFrame::BuildDisplayListForStackingContext(nsDisplayListBuilder* aBuilder, bool opacityItemForEventsAndPluginsOnly = false; if (effects->mOpacity == 0.0 && aBuilder->IsForPainting() && !(disp->mWillChangeBitField & NS_STYLE_WILL_CHANGE_OPACITY) && - !nsLayoutUtils::HasActiveAnimationOfProperty(this, - eCSSProperty_opacity)) { + !nsLayoutUtils::HasAnimationOfProperty(this, eCSSProperty_opacity)) { if (needEventRegions || aBuilder->WillComputePluginGeometry()) { opacityItemForEventsAndPluginsOnly = true; diff --git a/layout/reftests/css-animations/reftest.list b/layout/reftests/css-animations/reftest.list index 2cbf7c8e3d24..86ceb28ae6c7 100644 --- a/layout/reftests/css-animations/reftest.list +++ b/layout/reftests/css-animations/reftest.list @@ -14,8 +14,8 @@ test-pref(layers.offmainthreadcomposition.async-animations,false) == stacking-co test-pref(layers.offmainthreadcomposition.async-animations,false) == stacking-context-transform-none-animation.html stacking-context-animation-ref.html == no-stacking-context-opacity-removing-animation-in-delay.html no-stacking-context-animation-ref.html == no-stacking-context-transform-removing-animation-in-delay.html no-stacking-context-animation-ref.html -fails == stacking-context-lose-opacity-1.html stacking-context-animation-ref.html -fails == stacking-context-lose-transform-none.html stacking-context-animation-ref.html +== stacking-context-lose-opacity-1.html stacking-context-animation-ref.html +== stacking-context-lose-transform-none.html stacking-context-animation-ref.html == stacking-context-opacity-win-in-delay.html stacking-context-animation-ref.html == stacking-context-opacity-win-in-delay-on-main-thread.html stacking-context-animation-ref.html == stacking-context-opacity-wins-over-transition.html stacking-context-animation-ref.html @@ -33,10 +33,10 @@ fails == stacking-context-lose-transform-none.html stacking-context-animation-re == stacking-context-transform-none-animation-with-preserve-3d.html stacking-context-animation-ref.html == stacking-context-transform-none-with-fill-backwards.html stacking-context-animation-ref.html == stacking-context-transform-none-with-fill-forwards.html stacking-context-animation-ref.html -fails == stacking-context-opacity-1-in-delay.html stacking-context-animation-ref.html # bug 1278136 and bug 1279403 -fails == stacking-context-opacity-removing-important-in-delay.html stacking-context-animation-ref.html -fails == stacking-context-transform-none-in-delay.html stacking-context-animation-ref.html # bug 1278136 and bug 1279403 -fails == stacking-context-transform-removing-important-in-delay.html stacking-context-animation-ref.html +== stacking-context-opacity-1-in-delay.html stacking-context-animation-ref.html +== stacking-context-opacity-removing-important-in-delay.html stacking-context-animation-ref.html +== stacking-context-transform-none-in-delay.html stacking-context-animation-ref.html +== stacking-context-transform-removing-important-in-delay.html stacking-context-animation-ref.html == background-position-in-delay.html background-position-ref.html == background-position-after-finish.html background-position-ref.html fails == background-position-running.html background-position-ref.html # This test fails the reftest-opaque-layer check since animating background-position currently creates an active layer, and reftest-opaque-layer only handles items assigned to PaintedLayers. diff --git a/layout/reftests/css-transitions/reftest.list b/layout/reftests/css-transitions/reftest.list index 83ae7230fbaa..e4e8c7adf574 100644 --- a/layout/reftests/css-transitions/reftest.list +++ b/layout/reftests/css-transitions/reftest.list @@ -2,7 +2,7 @@ == transitions-inline-already-wrapped-2.html transitions-inline-ref.html == transitions-inline-rewrap-1.html transitions-inline-ref.html == transitions-inline-rewrap-2.html transitions-inline-ref.html -fails == stacking-context-opacity-lose-to-animation.html stacking-context-transition-ref.html -fails == stacking-context-transform-lose-to-animation.html stacking-context-transition-ref.html +== stacking-context-opacity-lose-to-animation.html stacking-context-transition-ref.html +== stacking-context-transform-lose-to-animation.html stacking-context-transition-ref.html == stacking-context-opacity-wins-over-important-style.html stacking-context-transition-ref.html == stacking-context-transform-wins-over-important-style.html stacking-context-transition-ref.html