Bug 1646811 - servo: animations: Don't always re-resolve the node style.
authorMartin Robinson <mrobinson@igalia.com>
Thu, 18 Jun 2020 18:12:34 +0000
changeset 536366 023935dbbdd66d3d5f78c82a48306ab79879188d
parent 536365 08aa02d53ba51711c80c252a4010593b3e090809
child 536367 366e36da6a7a77057a479b8bd20c246d78435d1f
push id119420
push userealvarez@mozilla.com
push dateThu, 18 Jun 2020 18:18:12 +0000
treeherderautoland@206011a5cc20 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1646811
milestone79.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1646811 - servo: animations: Don't always re-resolve the node style. When animations and transitions change don't always re-resolve node style, just replace the animation and transition rules and re-cascade. Depends on D80236 Differential Revision: https://phabricator.services.mozilla.com/D80237
servo/components/style/animation.rs
servo/components/style/matching.rs
servo/components/style/style_resolver.rs
--- a/servo/components/style/animation.rs
+++ b/servo/components/style/animation.rs
@@ -877,20 +877,19 @@ impl ElementAnimationSet {
         self.dirty = !self.animations.is_empty();
         for animation in self.animations.iter_mut() {
             animation.state = AnimationState::Canceled;
         }
         self.cancel_active_transitions();
     }
 
     fn cancel_active_transitions(&mut self) {
-        self.dirty = !self.transitions.is_empty();
-
         for transition in self.transitions.iter_mut() {
             if transition.state != AnimationState::Finished {
+                self.dirty = true;
                 transition.state = AnimationState::Canceled;
             }
         }
     }
 
     pub(crate) fn apply_active_animations(
         &mut self,
         context: &SharedStyleContext,
--- a/servo/components/style/matching.rs
+++ b/servo/components/style/matching.rs
@@ -12,18 +12,20 @@ use crate::context::{CascadeInputs, Elem
 use crate::context::{SharedStyleContext, StyleContext};
 use crate::data::ElementData;
 use crate::dom::TElement;
 #[cfg(feature = "servo")]
 use crate::dom::TNode;
 use crate::invalidation::element::restyle_hints::RestyleHint;
 use crate::properties::longhands::display::computed_value::T as Display;
 use crate::properties::ComputedValues;
+use crate::properties::PropertyDeclarationBlock;
 use crate::rule_tree::{CascadeLevel, StrongRuleNode};
 use crate::selector_parser::{PseudoElement, RestyleDamage};
+use crate::shared_lock::Locked;
 use crate::style_resolver::ResolvedElementStyles;
 use crate::style_resolver::{PseudoElementResolution, StyleResolverForElement};
 use crate::stylist::RuleInclusion;
 use crate::traversal_flags::TraversalFlags;
 use selectors::matching::ElementSelectorFlags;
 use servo_arc::{Arc, ArcBorrow};
 
 /// Represents the result of comparing an element's old and new style.
@@ -82,76 +84,77 @@ enum CascadeVisitedMode {
     Unvisited,
     /// Cascade the styles used when an element's relevant link is visited.  A
     /// "relevant link" is the element being matched if it is a link or the
     /// nearest ancestor link.
     Visited,
 }
 
 trait PrivateMatchMethods: TElement {
+    fn replace_single_rule_node(
+        context: &mut StyleContext<Self>,
+        level: CascadeLevel,
+        pdb: Option<ArcBorrow<Locked<PropertyDeclarationBlock>>>,
+        path: &mut StrongRuleNode,
+    ) -> bool {
+        let stylist = &context.shared.stylist;
+        let guards = &context.shared.guards;
+
+        let mut important_rules_changed = false;
+        let new_node = stylist.rule_tree().update_rule_at_level(
+            level,
+            pdb,
+            path,
+            guards,
+            &mut important_rules_changed,
+        );
+        if let Some(n) = new_node {
+            *path = n;
+        }
+        important_rules_changed
+    }
+
     /// Updates the rule nodes without re-running selector matching, using just
     /// the rule tree, for a specific visited mode.
     ///
     /// Returns true if an !important rule was replaced.
     fn replace_rules_internal(
         &self,
         replacements: RestyleHint,
         context: &mut StyleContext<Self>,
         cascade_visited: CascadeVisitedMode,
         cascade_inputs: &mut ElementCascadeInputs,
     ) -> bool {
-        use crate::properties::PropertyDeclarationBlock;
-        use crate::shared_lock::Locked;
-
         debug_assert!(
             replacements.intersects(RestyleHint::replacements()) &&
                 (replacements & !RestyleHint::replacements()).is_empty()
         );
 
-        let stylist = &context.shared.stylist;
-        let guards = &context.shared.guards;
-
         let primary_rules = match cascade_visited {
             CascadeVisitedMode::Unvisited => cascade_inputs.primary.rules.as_mut(),
             CascadeVisitedMode::Visited => cascade_inputs.primary.visited_rules.as_mut(),
         };
 
         let primary_rules = match primary_rules {
             Some(r) => r,
             None => return false,
         };
 
-        let replace_rule_node = |level: CascadeLevel,
-                                 pdb: Option<ArcBorrow<Locked<PropertyDeclarationBlock>>>,
-                                 path: &mut StrongRuleNode|
-         -> bool {
-            let mut important_rules_changed = false;
-            let new_node = stylist.rule_tree().update_rule_at_level(
-                level,
-                pdb,
-                path,
-                guards,
-                &mut important_rules_changed,
-            );
-            if let Some(n) = new_node {
-                *path = n;
-            }
-            important_rules_changed
-        };
-
         if !context.shared.traversal_flags.for_animation_only() {
             let mut result = false;
             if replacements.contains(RestyleHint::RESTYLE_STYLE_ATTRIBUTE) {
                 let style_attribute = self.style_attribute();
-                result |= replace_rule_node(
+                result |= Self::replace_single_rule_node(
+                    context,
                     CascadeLevel::same_tree_author_normal(),
                     style_attribute,
                     primary_rules,
                 );
-                result |= replace_rule_node(
+                result |= Self::replace_single_rule_node(
+                    context,
                     CascadeLevel::same_tree_author_important(),
                     style_attribute,
                     primary_rules,
                 );
                 // FIXME(emilio): Still a hack!
                 self.unset_dirty_style_attribute();
             }
             return result;
@@ -161,35 +164,38 @@ trait PrivateMatchMethods: TElement {
         // hints in the animation-only traversal.
         //
         // Non-animation restyle hints will be processed in a subsequent
         // normal traversal.
         if replacements.intersects(RestyleHint::for_animations()) {
             debug_assert!(context.shared.traversal_flags.for_animation_only());
 
             if replacements.contains(RestyleHint::RESTYLE_SMIL) {
-                replace_rule_node(
+                Self::replace_single_rule_node(
+                    context,
                     CascadeLevel::SMILOverride,
                     self.smil_override(),
                     primary_rules,
                 );
             }
 
             if replacements.contains(RestyleHint::RESTYLE_CSS_TRANSITIONS) {
-                replace_rule_node(
+                Self::replace_single_rule_node(
+                    context,
                     CascadeLevel::Transitions,
                     self.transition_rule(&context.shared)
                         .as_ref()
                         .map(|a| a.borrow_arc()),
                     primary_rules,
                 );
             }
 
             if replacements.contains(RestyleHint::RESTYLE_CSS_ANIMATIONS) {
-                replace_rule_node(
+                Self::replace_single_rule_node(
+                    context,
                     CascadeLevel::Animations,
                     self.animation_rule(&context.shared)
                         .as_ref()
                         .map(|a| a.borrow_arc()),
                     primary_rules,
                 );
             }
         }
@@ -369,22 +375,23 @@ trait PrivateMatchMethods: TElement {
         }
     }
 
     #[cfg(feature = "gecko")]
     fn process_animations(
         &self,
         context: &mut StyleContext<Self>,
         old_values: &mut Option<Arc<ComputedValues>>,
-        new_values: &mut Arc<ComputedValues>,
+        new_styles: &mut ResolvedElementStyles,
         restyle_hint: RestyleHint,
         important_rules_changed: bool,
     ) {
         use crate::context::UpdateAnimationsTasks;
 
+        let new_values = new_styles.primary_style_mut();
         if context.shared.traversal_flags.for_animation_only() {
             self.handle_display_change_for_smil_if_needed(
                 context,
                 old_values.as_ref().map(|v| &**v),
                 new_values,
                 restyle_hint,
             );
             return;
@@ -456,20 +463,75 @@ trait PrivateMatchMethods: TElement {
         }
     }
 
     #[cfg(feature = "servo")]
     fn process_animations(
         &self,
         context: &mut StyleContext<Self>,
         old_values: &mut Option<Arc<ComputedValues>>,
-        new_values: &mut Arc<ComputedValues>,
+        new_resolved_styles: &mut ResolvedElementStyles,
         _restyle_hint: RestyleHint,
         _important_rules_changed: bool,
     ) {
+        if !self.process_animations_for_style(
+            context,
+            old_values,
+            new_resolved_styles.primary_style_mut(),
+        ) {
+            return;
+        }
+
+        // If we have modified animation or transitions, we recascade style for this node.
+        let mut rule_node = new_resolved_styles.primary_style().rules().clone();
+        Self::replace_single_rule_node(
+            context,
+            CascadeLevel::Transitions,
+            self.transition_rule(&context.shared)
+                .as_ref()
+                .map(|a| a.borrow_arc()),
+            &mut rule_node,
+        );
+        Self::replace_single_rule_node(
+            context,
+            CascadeLevel::Animations,
+            self.animation_rule(&context.shared)
+                .as_ref()
+                .map(|a| a.borrow_arc()),
+            &mut rule_node,
+        );
+
+        // If these animations haven't modified the rule now, we can just exit early.
+        if rule_node == *new_resolved_styles.primary_style().rules() {
+            return;
+        }
+
+        let inputs = CascadeInputs {
+            rules: Some(rule_node),
+            visited_rules: new_resolved_styles.primary_style().visited_rules().cloned(),
+        };
+
+        let style = StyleResolverForElement::new(
+            *self,
+            context,
+            RuleInclusion::All,
+            PseudoElementResolution::IfApplicable,
+        )
+        .cascade_style_and_visited_with_default_parents(inputs);
+
+        new_resolved_styles.primary.style = style;
+    }
+
+    #[cfg(feature = "servo")]
+    fn process_animations_for_style(
+        &self,
+        context: &mut StyleContext<Self>,
+        old_values: &mut Option<Arc<ComputedValues>>,
+        new_values: &mut Arc<ComputedValues>,
+    ) -> bool {
         use crate::animation::AnimationState;
 
         // We need to call this before accessing the `ElementAnimationSet` from the
         // map because this call will do a RwLock::read().
         let needs_animations_update =
             self.needs_animations_update(context, old_values.as_ref().map(|s| &**s), new_values);
         let might_need_transitions_update = self.might_need_transitions_update(
             context,
@@ -531,27 +593,17 @@ trait PrivateMatchMethods: TElement {
         if !animation_set.is_empty() {
             animation_set.dirty = false;
             shared_context
                 .animation_states
                 .write()
                 .insert(this_opaque, animation_set);
         }
 
-        // If we have modified animation or transitions, we recascade style for this node.
-        if changed_animations {
-            let mut resolver = StyleResolverForElement::new(
-                *self,
-                context,
-                RuleInclusion::All,
-                PseudoElementResolution::IfApplicable,
-            );
-            let new_primary = resolver.resolve_style_with_default_parents();
-            *new_values = new_primary.primary.style.0;
-        }
+        changed_animations
     }
 
     /// Computes and applies non-redundant damage.
     fn accumulate_damage_for(
         &self,
         shared_context: &SharedStyleContext,
         damage: &mut RestyleDamage,
         old_values: &ComputedValues,
@@ -707,17 +759,17 @@ pub trait MatchMethods: TElement {
         mut new_styles: ResolvedElementStyles,
         important_rules_changed: bool,
     ) -> ChildCascadeRequirement {
         use std::cmp;
 
         self.process_animations(
             context,
             &mut data.styles.primary,
-            &mut new_styles.primary.style.0,
+            &mut new_styles,
             data.hint,
             important_rules_changed,
         );
 
         // First of all, update the styles.
         let old_styles = data.set_styles(new_styles);
 
         let new_primary_style = data.styles.primary.as_ref().unwrap();
--- a/servo/components/style/style_resolver.rs
+++ b/servo/components/style/style_resolver.rs
@@ -61,16 +61,28 @@ pub struct PrimaryStyle {
 /// A set of style returned from the resolver machinery.
 pub struct ResolvedElementStyles {
     /// Primary style.
     pub primary: PrimaryStyle,
     /// Pseudo styles.
     pub pseudos: EagerPseudoStyles,
 }
 
+impl ResolvedElementStyles {
+    /// Convenience accessor for the primary style.
+    pub fn primary_style(&self) -> &Arc<ComputedValues> {
+        &self.primary.style.0
+    }
+
+    /// Convenience mutable accessor for the style.
+    pub fn primary_style_mut(&mut self) -> &mut Arc<ComputedValues> {
+        &mut self.primary.style.0
+    }
+}
+
 impl PrimaryStyle {
     /// Convenience accessor for the style.
     pub fn style(&self) -> &ComputedValues {
         &*self.style.0
     }
 }
 
 impl From<ResolvedElementStyles> for ElementStyles {