From 070c1a117cfb242465e8e50347d1394abdebab9e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Thu, 3 Aug 2023 09:04:11 +0000 Subject: [PATCH] Bug 1844466 - Speed up and simplify cumulative changehint computation. r=boris,hiro As a bonus we now can throttle some additive and visibility animations more properly, because before we couldn't compute a change hint for those but now we don't need to. Differential Revision: https://phabricator.services.mozilla.com/D185175 --- dom/animation/KeyframeEffect.cpp | 162 +++++------------- dom/animation/KeyframeEffect.h | 46 ++--- dom/animation/test/mozilla/file_restyles.html | 13 +- layout/base/nsChangeHint.h | 9 - layout/style/ServoCSSPropList.mako.py | 12 +- layout/style/ServoStyleSet.cpp | 9 - layout/style/ServoStyleSet.h | 15 -- servo/components/style/rule_tree/mod.rs | 25 +-- servo/ports/geckolib/glue.rs | 57 ------ 9 files changed, 86 insertions(+), 262 deletions(-) diff --git a/dom/animation/KeyframeEffect.cpp b/dom/animation/KeyframeEffect.cpp index 2f0d886ac107..2183301b8b03 100644 --- a/dom/animation/KeyframeEffect.cpp +++ b/dom/animation/KeyframeEffect.cpp @@ -446,11 +446,6 @@ void KeyframeEffect::UpdateProperties(const ComputedStyle* aStyle, if (baseStylesChanged) { RequestRestyle(EffectCompositor::RestyleType::Layer); } - // Check if we need to update the cumulative change hint because we now have - // style data. - if (mNeedsStyleData && mTarget && mTarget.mElement->HasServoData()) { - CalculateCumulativeChangeHint(aStyle); - } return; } @@ -466,23 +461,13 @@ void KeyframeEffect::UpdateProperties(const ComputedStyle* aStyle, mProperties = std::move(properties); UpdateEffectSet(); - mHasCurrentColor = false; - + mCumulativeChanges = {}; for (AnimationProperty& property : mProperties) { property.mIsRunningOnCompositor = runningOnCompositorProperties.HasProperty(property.mProperty); - - if (property.mProperty == eCSSProperty_background_color && - !mHasCurrentColor) { - if (HasCurrentColor(property.mSegments)) { - mHasCurrentColor = true; - break; - } - } + CalculateCumulativeChangesForProperty(property); } - CalculateCumulativeChangeHint(aStyle); - MarkCascadeNeedsUpdate(); if (mAnimation) { @@ -1098,10 +1083,9 @@ already_AddRefed KeyframeEffect::Constructor( // aSource's timing object can be assumed valid. RefPtr effect = new KeyframeEffect(doc, OwningAnimationTarget{aSource.mTarget}, aSource); - // Copy cumulative change hint. mCumulativeChangeHint should be the same as - // the source one because both of targets are the same. - effect->mCumulativeChangeHint = aSource.mCumulativeChangeHint; - + // Copy cumulative changes. mCumulativeChangeHint should be the same as the + // source one because both of targets are the same. + effect->mCumulativeChanges = aSource.mCumulativeChanges; return effect.forget(); } @@ -1774,104 +1758,50 @@ void KeyframeEffect::SetPerformanceWarning( } } -already_AddRefed -KeyframeEffect::CreateComputedStyleForAnimationValue( - nsCSSPropertyID aProperty, const AnimationValue& aValue, - nsPresContext* aPresContext, const ComputedStyle* aBaseComputedStyle) { - MOZ_ASSERT(aBaseComputedStyle, - "CreateComputedStyleForAnimationValue needs to be called " - "with a valid ComputedStyle"); - - Element* elementForResolve = AnimationUtils::GetElementForRestyle( - mTarget.mElement, mTarget.mPseudoType); - // The element may be null if, for example, we target a pseudo-element that no - // longer exists. - if (!elementForResolve) { - return nullptr; +void KeyframeEffect::CalculateCumulativeChangesForProperty( + const AnimationProperty& aProperty) { + constexpr auto kInterestingFlags = + CSSPropFlags::AffectsLayout | CSSPropFlags::AffectsOverflow; + if (aProperty.mProperty == eCSSProperty_opacity) { + mCumulativeChanges.mOpacity = true; + return; // We know opacity is visual-only. } - ServoStyleSet* styleSet = aPresContext->StyleSet(); - return styleSet->ResolveServoStyleByAddingAnimation( - elementForResolve, aBaseComputedStyle, aValue.mServo); -} + if (aProperty.mProperty == eCSSProperty_visibility) { + mCumulativeChanges.mVisibility = true; + return; // We know visibility is visual-only. + } -void KeyframeEffect::CalculateCumulativeChangeHint( - const ComputedStyle* aComputedStyle) { - mCumulativeChangeHint = nsChangeHint(0); - mNeedsStyleData = false; + if (aProperty.mProperty == eCSSProperty_background_color) { + if (!mCumulativeChanges.mHasBackgroundColorCurrentColor) { + mCumulativeChanges.mHasBackgroundColorCurrentColor = + HasCurrentColor(aProperty.mSegments); + } + return; // We know background-color is visual-only. + } - nsPresContext* presContext = - mTarget ? nsContentUtils::GetContextForContent(mTarget.mElement) - : nullptr; - if (!presContext) { - // Change hints make no sense if we're not rendered. - // - // Actually, we cannot even post them anywhere. - mNeedsStyleData = true; + auto flags = nsCSSProps::PropFlags(aProperty.mProperty); + if (!(flags & kInterestingFlags)) { + return; // Property is visual-only. + } + + bool anyChange = false; + for (const AnimationPropertySegment& segment : aProperty.mSegments) { + if (!segment.HasReplaceableValues() || + segment.mFromValue != segment.mToValue) { + // We can't know non-replaceable values until we compose the animation, so + // assume a change there. + anyChange = true; + break; + } + } + + if (!anyChange) { return; } - for (const AnimationProperty& property : mProperties) { - // For opacity property we don't produce any change hints that are not - // included in nsChangeHint_Hints_CanIgnoreIfNotVisible so we can throttle - // opacity animations regardless of the change they produce. This - // optimization is particularly important since it allows us to throttle - // opacity animations with missing 0%/100% keyframes. - if (property.mProperty == eCSSProperty_opacity) { - continue; - } - - for (const AnimationPropertySegment& segment : property.mSegments) { - // In case composite operation is not 'replace' or value is null, - // we can't throttle animations which will not cause any layout changes - // on invisible elements because we can't calculate the change hint for - // such properties until we compose it. - if (!segment.HasReplaceableValues()) { - if (!nsCSSPropertyIDSet::TransformLikeProperties().HasProperty( - property.mProperty)) { - mCumulativeChangeHint = ~nsChangeHint_Hints_CanIgnoreIfNotVisible; - return; - } - // We try a little harder to optimize transform animations simply - // because they are so common (the second-most commonly animated - // property at the time of writing). So if we encounter a transform - // segment that needs composing with the underlying value, we just add - // all the change hints a transform animation is known to be able to - // generate. - mCumulativeChangeHint |= - nsChangeHint_ComprehensiveAddOrRemoveTransform | - nsChangeHint_UpdatePostTransformOverflow | - nsChangeHint_UpdateTransformLayer; - continue; - } - - RefPtr fromContext = - CreateComputedStyleForAnimationValue(property.mProperty, - segment.mFromValue, presContext, - aComputedStyle); - if (!fromContext) { - mCumulativeChangeHint = ~nsChangeHint_Hints_CanIgnoreIfNotVisible; - mNeedsStyleData = true; - return; - } - - RefPtr toContext = - CreateComputedStyleForAnimationValue(property.mProperty, - segment.mToValue, presContext, - aComputedStyle); - if (!toContext) { - mCumulativeChangeHint = ~nsChangeHint_Hints_CanIgnoreIfNotVisible; - mNeedsStyleData = true; - return; - } - - uint32_t equalStructs = 0; - nsChangeHint changeHint = - fromContext->CalcStyleDifference(*toContext, &equalStructs); - - mCumulativeChangeHint |= changeHint; - } - } + mCumulativeChanges.mOverflow |= bool(flags & CSSPropFlags::AffectsOverflow); + mCumulativeChanges.mLayout |= bool(flags & CSSPropFlags::AffectsLayout); } void KeyframeEffect::SetAnimation(Animation* aAnimation) { @@ -1903,11 +1833,7 @@ bool KeyframeEffect::CanIgnoreIfNotVisible() const { if (!StaticPrefs::dom_animations_offscreen_throttling()) { return false; } - - // FIXME: For further sophisticated optimization we need to check - // change hint on the segment corresponding to computedTiming.progress. - return NS_IsHintSubset(mCumulativeChangeHint, - nsChangeHint_Hints_CanIgnoreIfNotVisible); + return !mCumulativeChanges.mLayout; } void KeyframeEffect::MarkCascadeNeedsUpdate() { @@ -2104,7 +2030,7 @@ KeyframeEffect::MatchForCompositor KeyframeEffect::IsMatchForCompositor( // We can't run this background color animation on the compositor if there // is any `current-color` keyframe. - if (mHasCurrentColor) { + if (mCumulativeChanges.mHasBackgroundColorCurrentColor) { aPerformanceWarning = AnimationPerformanceWarning::Type::HasCurrentColor; return KeyframeEffect::MatchForCompositor::NoAndBlockThisProperty; } diff --git a/dom/animation/KeyframeEffect.h b/dom/animation/KeyframeEffect.h index 8f8466369e1c..dfe3a33643c9 100644 --- a/dom/animation/KeyframeEffect.h +++ b/dom/animation/KeyframeEffect.h @@ -313,7 +313,7 @@ class KeyframeEffect : public AnimationEffect { // Cumulative change hint on each segment for each property. // This is used for deciding the animation is paint-only. - void CalculateCumulativeChangeHint(const ComputedStyle* aStyle); + void CalculateCumulativeChangesForProperty(const AnimationProperty&); // Returns true if all of animation properties' change hints // can ignore painting if the animation is not visible. @@ -362,9 +362,7 @@ class KeyframeEffect : public AnimationEffect { const Nullable& aProgressOnLastCompose, uint64_t aCurrentIterationOnLastCompose); - bool HasOpacityChange() const { - return mCumulativeChangeHint & nsChangeHint_UpdateOpacityLayer; - } + bool HasOpacityChange() const { return mCumulativeChanges.mOpacity; } protected: ~KeyframeEffect() override = default; @@ -451,15 +449,6 @@ class KeyframeEffect : public AnimationEffect { // EffectSet. bool mInEffectSet = false; - // True if the last time we tried to update the cumulative change hint we - // didn't have style data to do so. We set this flag so that the next time - // our style context changes we know to update the cumulative change hint even - // if our properties haven't changed. - bool mNeedsStyleData = false; - - // True if there is any current-color for background color in this keyframes. - bool mHasCurrentColor = false; - // The non-animated values for properties in this effect that contain at // least one animation value that is composited with the underlying value // (i.e. it uses the additive or accumulate composite mode). @@ -468,7 +457,27 @@ class KeyframeEffect : public AnimationEffect { BaseValuesHashmap mBaseValues; private: - nsChangeHint mCumulativeChangeHint = nsChangeHint{0}; + // The cumulative changes of all the animation segments. + struct CumulativeChanges { + // Whether the opacity property is changing. + bool mOpacity : 1; + // Whether the visibility property is changing. + bool mVisibility : 1; + // Whether layout is changing. + bool mLayout : 1; + // Whether overflow is changing. + bool mOverflow : 1; + // True if there is any current-color for background color. + bool mHasBackgroundColorCurrentColor : 1; + + CumulativeChanges() + : mOpacity(false), + mVisibility(false), + mLayout(false), + mOverflow(false), + mHasBackgroundColorCurrentColor(false) {} + }; + CumulativeChanges mCumulativeChanges; void ComposeStyleRule(StyleAnimationValueMap& aAnimationValues, const AnimationProperty& aProperty, @@ -510,17 +519,12 @@ class KeyframeEffect : public AnimationEffect { // This function is used for updating scroll bars or notifying intersection // observers reflected by the transform. bool HasPropertiesThatMightAffectOverflow() const { - return mCumulativeChangeHint & - (nsChangeHint_AddOrRemoveTransform | nsChangeHint_UpdateOverflow | - nsChangeHint_UpdatePostTransformOverflow | - nsChangeHint_UpdateTransformLayer); + return mCumulativeChanges.mOverflow; } // Returns true if this effect causes visibility change. // (i.e. 'visibility: hidden' -> 'visibility: visible' and vice versa.) - bool HasVisibilityChange() const { - return mCumulativeChangeHint & nsChangeHint_VisibilityChange; - } + bool HasVisibilityChange() const { return mCumulativeChanges.mVisibility; } }; } // namespace dom diff --git a/dom/animation/test/mozilla/file_restyles.html b/dom/animation/test/mozilla/file_restyles.html index 8d72cb6c44c8..3bd6d3809f8e 100644 --- a/dom/animation/test/mozilla/file_restyles.html +++ b/dom/animation/test/mozilla/file_restyles.html @@ -1270,9 +1270,9 @@ waitForAllPaints(() => { const markers = await observeStyling(5); - is(markers.length, 5, + is(markers.length, 0, 'Additive animation has no keyframe whose offset is 0 or 1 in an ' + - 'out-of-view element should not be throttled'); + 'out-of-view element should be throttled'); await ensureElementRemoval(div); } ); @@ -1287,9 +1287,9 @@ waitForAllPaints(() => { const markers = await observeStyling(5); - is(markers.length, 5, + is(markers.length, 0, 'Discrete animation has has no keyframe whose offset is 0 or 1 in an ' + - 'out-of-view element should not be throttled'); + 'out-of-view element should be throttled'); await ensureElementRemoval(div); }); @@ -1305,10 +1305,9 @@ waitForAllPaints(() => { await waitForAnimationReadyToRestyle(animation); const markers = await observeStyling(5); - is(markers.length, 5, + is(markers.length, 0, 'visibility animation has no keyframe whose offset is 0 or 1 in an ' + - 'out-of-view element and produces change hint other than paint-only ' + - 'change hint should not be throttled'); + 'out-of-view element should be throttled'); await ensureElementRemoval(div); } ); diff --git a/layout/base/nsChangeHint.h b/layout/base/nsChangeHint.h index 9137f9512292..5806abfd8496 100644 --- a/layout/base/nsChangeHint.h +++ b/layout/base/nsChangeHint.h @@ -425,15 +425,6 @@ static_assert(!(nsChangeHint_Hints_AlwaysHandledForDescendants & #define NS_STYLE_HINT_REFLOW \ nsChangeHint(NS_STYLE_HINT_VISUAL | nsChangeHint_AllReflowHints) -#define nsChangeHint_Hints_CanIgnoreIfNotVisible \ - nsChangeHint( \ - NS_STYLE_HINT_VISUAL | nsChangeHint_NeutralChange | \ - nsChangeHint_UpdateOpacityLayer | nsChangeHint_AddOrRemoveTransform | \ - nsChangeHint_UpdateContainingBlock | nsChangeHint_UpdateOverflow | \ - nsChangeHint_UpdatePostTransformOverflow | \ - nsChangeHint_UpdateTransformLayer | nsChangeHint_UpdateUsesOpacity | \ - nsChangeHint_UpdateBackgroundPosition | nsChangeHint_VisibilityChange) - // Change hints for added or removed transform style. // // If we've added or removed the transform property, we need to reconstruct the diff --git a/layout/style/ServoCSSPropList.mako.py b/layout/style/ServoCSSPropList.mako.py index 9f1b0383a794..56061288d5c1 100644 --- a/layout/style/ServoCSSPropList.mako.py +++ b/layout/style/ServoCSSPropList.mako.py @@ -112,6 +112,13 @@ def exposed_on_getcs(prop): def rules(prop): return ", ".join('"{}"'.format(rule) for rule in prop.rule_types_allowed_names()) +RUST_TO_CPP_FLAGS = { + "CAN_ANIMATE_ON_COMPOSITOR": "CanAnimateOnCompositor", + "AFFECTS_LAYOUT": "AffectsLayout", + "AFFECTS_PAINT": "AffectsPaint", + "AFFECTS_OVERFLOW": "AffectsOverflow", +} + def flags(prop): result = [] if prop.explicitly_enabled_in_chrome(): @@ -122,8 +129,9 @@ def flags(prop): result.append("Internal") if prop.enabled_in == "": result.append("Inaccessible") - if "CAN_ANIMATE_ON_COMPOSITOR" in prop.flags: - result.append("CanAnimateOnCompositor") + for (k, v) in RUST_TO_CPP_FLAGS.items(): + if k in prop.flags: + result.append(v) if exposed_on_getcs(prop): result.append("ExposedOnGetCS") if prop.type() == "shorthand" and "SHORTHAND_IN_GETCS" in prop.flags: diff --git a/layout/style/ServoStyleSet.cpp b/layout/style/ServoStyleSet.cpp index f4c09462eeca..aaf1a5424b2c 100644 --- a/layout/style/ServoStyleSet.cpp +++ b/layout/style/ServoStyleSet.cpp @@ -1091,15 +1091,6 @@ already_AddRefed ServoStyleSet::GetBaseContextForElement( .Consume(); } -already_AddRefed -ServoStyleSet::ResolveServoStyleByAddingAnimation( - Element* aElement, const ComputedStyle* aStyle, - StyleAnimationValue* aAnimationValue) { - return Servo_StyleSet_GetComputedValuesByAddingAnimation( - mRawData.get(), aElement, aStyle, &Snapshots(), aAnimationValue) - .Consume(); -} - already_AddRefed ServoStyleSet::ComputeAnimationValue( Element* aElement, StyleLockedDeclarationBlock* aDeclarations, const ComputedStyle* aStyle) { diff --git a/layout/style/ServoStyleSet.h b/layout/style/ServoStyleSet.h index 1a0f0ad59426..fef712362afc 100644 --- a/layout/style/ServoStyleSet.h +++ b/layout/style/ServoStyleSet.h @@ -399,21 +399,6 @@ class ServoStyleSet { already_AddRefed GetBaseContextForElement( dom::Element* aElement, const ComputedStyle* aStyle); - // Get a ComputedStyle that represents |aStyle|, but as though it additionally - // matched the rules of the newly added |aAnimaitonaValue|. - // - // We use this function to temporarily generate a ComputedStyle for - // calculating the cumulative change hints. - // - // This must hold: - // The additional rules must be appropriate for the transition - // level of the cascade, which is the highest level of the cascade. - // (This is the case for one current caller, the cover rule used - // for CSS transitions.) - // Note: |aElement| should be the generated element if it is pseudo. - already_AddRefed ResolveServoStyleByAddingAnimation( - dom::Element* aElement, const ComputedStyle* aStyle, - StyleAnimationValue* aAnimationValue); /** * Resolve style for a given declaration block with/without the parent style. * If the parent style is not specified, the document default computed values diff --git a/servo/components/style/rule_tree/mod.rs b/servo/components/style/rule_tree/mod.rs index 01510e62070d..18ee018d64a0 100644 --- a/servo/components/style/rule_tree/mod.rs +++ b/servo/components/style/rule_tree/mod.rs @@ -10,7 +10,7 @@ use crate::applicable_declarations::{ApplicableDeclarationList, CascadePriority} use crate::properties::{LonghandIdSet, PropertyDeclarationBlock}; use crate::shared_lock::{Locked, StylesheetGuards}; use crate::stylesheets::layer_rule::LayerOrder; -use servo_arc::{Arc, ArcBorrow}; +use servo_arc::ArcBorrow; use smallvec::SmallVec; use std::io::{self, Write}; @@ -321,29 +321,6 @@ impl RuleTree { .insert_ordered_rules_from(last.parent().unwrap().clone(), children.drain(..).rev()); rule } - - /// Returns new rule node by adding animation rules at transition level. - /// The additional rules must be appropriate for the transition - /// level of the cascade, which is the highest level of the cascade. - /// (This is the case for one current caller, the cover rule used - /// for CSS transitions.) - pub fn add_animation_rules_at_transition_level( - &self, - path: &StrongRuleNode, - pdb: Arc>, - guards: &StylesheetGuards, - ) -> StrongRuleNode { - let mut dummy = false; - self.update_rule_at_level( - CascadeLevel::Transitions, - LayerOrder::root(), - Some(pdb.borrow_arc()), - path, - guards, - &mut dummy, - ) - .expect("Should return a valid rule node") - } } impl StrongRuleNode { diff --git a/servo/ports/geckolib/glue.rs b/servo/ports/geckolib/glue.rs index 2cc5df1e9b4b..335c6412be60 100644 --- a/servo/ports/geckolib/glue.rs +++ b/servo/ports/geckolib/glue.rs @@ -1159,63 +1159,6 @@ pub extern "C" fn Servo_StyleSet_GetBaseComputedValuesForElement( .into() } -#[no_mangle] -pub extern "C" fn Servo_StyleSet_GetComputedValuesByAddingAnimation( - raw_style_set: &PerDocumentStyleData, - element: &RawGeckoElement, - computed_values: &ComputedValues, - snapshots: *const ServoElementSnapshotTable, - animation_value: &AnimationValue, -) -> Strong { - debug_assert!(!snapshots.is_null()); - let rules = match computed_values.rules { - None => return Strong::null(), - Some(ref rules) => rules, - }; - - let global_style_data = &*GLOBAL_STYLE_DATA; - let guard = global_style_data.shared_lock.read(); - let uncomputed_value = animation_value.uncompute(); - let doc_data = raw_style_set.borrow(); - - let with_animations_rules = { - let guards = StylesheetGuards::same(&guard); - let declarations = Arc::new(global_style_data.shared_lock.wrap( - PropertyDeclarationBlock::with_one(uncomputed_value, Importance::Normal), - )); - doc_data - .stylist - .rule_tree() - .add_animation_rules_at_transition_level(rules, declarations, &guards) - }; - - let element = GeckoElement(element); - if element.borrow_data().is_none() { - return Strong::null(); - } - - let shared = create_shared_context( - &global_style_data, - &guard, - &doc_data.stylist, - TraversalFlags::empty(), - unsafe { &*snapshots }, - ); - let mut tlc: ThreadLocalStyleContext = ThreadLocalStyleContext::new(); - let context = StyleContext { - shared: &shared, - thread_local: &mut tlc, - }; - - resolve_rules_for_element_with_context( - element, - context, - with_animations_rules, - &computed_values, - ) - .into() -} - #[no_mangle] pub extern "C" fn Servo_ComputedValues_ExtractAnimationValue( computed_values: &ComputedValues,