From d30c4fe79dbd1d20a0c509fa1d28398cf168e138 Mon Sep 17 00:00:00 2001 From: Hiroyuki Ikezoe Date: Wed, 24 May 2017 11:11:25 +0900 Subject: [PATCH 1/6] Use SMIL override value that has been processed during animation-only restyles for normal restyle. In the case where we process an element which has SMIL animations in normal travesal the SMIL styles must have been computed in animation-only restyles. So we have only to pick the computed styles instead of recomputing it. --- components/style/data.rs | 17 +++++++++++-- components/style/matching.rs | 40 ++++++++++++++++--------------- components/style/rule_tree/mod.rs | 24 +++++++++++++++++++ 3 files changed, 60 insertions(+), 21 deletions(-) diff --git a/components/style/data.rs b/components/style/data.rs index 6e0feb35b68..41a618dd32e 100644 --- a/components/style/data.rs +++ b/components/style/data.rs @@ -6,12 +6,12 @@ use context::SharedStyleContext; use dom::TElement; -use properties::ComputedValues; +use properties::{ComputedValues, PropertyDeclarationBlock}; use properties::longhands::display::computed_value as display; use restyle_hints::{HintComputationContext, RestyleReplacements, RestyleHint}; use rule_tree::StrongRuleNode; use selector_parser::{EAGER_PSEUDO_COUNT, PseudoElement, RestyleDamage}; -use shared_lock::StylesheetGuards; +use shared_lock::{Locked, StylesheetGuards}; use std::fmt; use stylearc::Arc; use traversal::TraversalFlags; @@ -558,4 +558,17 @@ impl ElementData { pub fn restyle_mut(&mut self) -> &mut RestyleData { self.get_restyle_mut().expect("Calling restyle_mut without RestyleData") } + + /// Returns SMIL overriden value if exists. + pub fn get_smil_override(&self) -> Option<&Arc>> { + if cfg!(feature = "servo") { + // Servo has no knowledge of a SMIL rule, so just avoid looking for it. + return None; + } + + match self.get_styles() { + Some(s) => s.primary.rules.get_smil_animation_rule(), + None => None, + } + } } diff --git a/components/style/matching.rs b/components/style/matching.rs index e0a8e4ec755..f0e23baecff 100644 --- a/components/style/matching.rs +++ b/components/style/matching.rs @@ -736,30 +736,32 @@ pub trait MatchMethods : TElement { let stylist = &context.shared.stylist; let style_attribute = self.style_attribute(); - let smil_override = self.get_smil_override(); - let animation_rules = self.get_animation_rules(); - let bloom = context.thread_local.bloom_filter.filter(); + { + let smil_override = data.get_smil_override(); + let animation_rules = self.get_animation_rules(); + let bloom = context.thread_local.bloom_filter.filter(); - let map = &mut context.thread_local.selector_flags; - let mut set_selector_flags = |element: &Self, flags: ElementSelectorFlags| { - self.apply_selector_flags(map, element, flags); - }; + let map = &mut context.thread_local.selector_flags; + let mut set_selector_flags = |element: &Self, flags: ElementSelectorFlags| { + self.apply_selector_flags(map, element, flags); + }; - let mut matching_context = - MatchingContext::new(MatchingMode::Normal, Some(bloom)); + let mut matching_context = + MatchingContext::new(MatchingMode::Normal, Some(bloom)); - // Compute the primary rule node. - stylist.push_applicable_declarations(self, - implemented_pseudo.as_ref(), - style_attribute, - smil_override, - animation_rules, - &mut applicable_declarations, - &mut matching_context, - &mut set_selector_flags); + // Compute the primary rule node. + stylist.push_applicable_declarations(self, + implemented_pseudo.as_ref(), + style_attribute, + smil_override, + animation_rules, + &mut applicable_declarations, + &mut matching_context, + &mut set_selector_flags); - *relations = matching_context.relations; + *relations = matching_context.relations; + } let primary_rule_node = compute_rule_node::(stylist.rule_tree(), diff --git a/components/style/rule_tree/mod.rs b/components/style/rule_tree/mod.rs index bfc6c19bf96..d5c44329b72 100644 --- a/components/style/rule_tree/mod.rs +++ b/components/style/rule_tree/mod.rs @@ -1128,6 +1128,30 @@ impl StrongRuleNode { } result } + + /// Returns PropertyDeclarationBlock for this node. + /// This function must be called only for animation level node. + fn get_animation_style(&self) -> &Arc> { + debug_assert!(self.cascade_level().is_animation(), + "The cascade level should be an animation level"); + match *self.style_source().unwrap() { + StyleSource::Declarations(ref block) => block, + StyleSource::Style(_) => unreachable!("animating style should not be a style rule"), + } + } + + /// Returns SMIL override declaration block if exists. + pub fn get_smil_animation_rule(&self) -> Option<&Arc>> { + if cfg!(feature = "servo") { + // Servo has no knowledge of a SMIL rule, so just avoid looking for it. + return None; + } + + self.self_and_ancestors() + .take_while(|node| node.cascade_level() >= CascadeLevel::SMILOverride) + .find(|node| node.cascade_level() == CascadeLevel::SMILOverride) + .map(|node| node.get_animation_style()) + } } /// An iterator over a rule node and its ancestors. From 35e0e3aa11c8bb4cd960923a354b4b25482ee84d Mon Sep 17 00:00:00 2001 From: Hiroyuki Ikezoe Date: Wed, 24 May 2017 11:11:41 +0900 Subject: [PATCH 2/6] Move AnimationRules into declaration_block.rs. --- components/style/dom.rs | 16 +--------------- components/style/gecko/wrapper.rs | 4 ++-- components/style/matching.rs | 4 ++-- components/style/properties/declaration_block.rs | 15 +++++++++++++++ components/style/stylist.rs | 4 ++-- 5 files changed, 22 insertions(+), 21 deletions(-) diff --git a/components/style/dom.rs b/components/style/dom.rs index 36997de5c32..de9d68396c7 100644 --- a/components/style/dom.rs +++ b/components/style/dom.rs @@ -13,7 +13,7 @@ use atomic_refcell::{AtomicRef, AtomicRefCell, AtomicRefMut}; use data::ElementData; use element_state::ElementState; use font_metrics::FontMetricsProvider; -use properties::{ComputedValues, PropertyDeclarationBlock}; +use properties::{AnimationRules, ComputedValues, PropertyDeclarationBlock}; #[cfg(feature = "gecko")] use properties::animated_properties::AnimationValue; #[cfg(feature = "gecko")] use properties::animated_properties::TransitionProperty; use rule_tree::CascadeLevel; @@ -274,20 +274,6 @@ pub trait PresentationalHintsSynthesizer { where V: Push; } -/// The animation rules. -/// -/// The first one is for Animation cascade level, and the second one is for -/// Transition cascade level. -pub struct AnimationRules(pub Option>>, - pub Option>>); - -impl AnimationRules { - /// Returns whether these animation rules represents an actual rule or not. - pub fn is_empty(&self) -> bool { - self.0.is_none() && self.1.is_none() - } -} - /// The element trait, the main abstraction the style crate acts over. pub trait TElement : Eq + PartialEq + Debug + Hash + Sized + Copy + Clone + ElementExt + PresentationalHintsSynthesizer { diff --git a/components/style/gecko/wrapper.rs b/components/style/gecko/wrapper.rs index e043ff57223..011e0bff078 100644 --- a/components/style/gecko/wrapper.rs +++ b/components/style/gecko/wrapper.rs @@ -18,7 +18,7 @@ use app_units::Au; use atomic_refcell::AtomicRefCell; use context::{QuirksMode, SharedStyleContext, UpdateAnimationsTasks}; use data::ElementData; -use dom::{self, AnimationRules, DescendantsBit, LayoutIterator, NodeInfo, TElement, TNode, UnsafeNode}; +use dom::{self, DescendantsBit, LayoutIterator, NodeInfo, TElement, TNode, UnsafeNode}; use dom::{OpaqueNode, PresentationalHintsSynthesizer}; use element_state::ElementState; use error_reporting::RustLogReporter; @@ -57,7 +57,7 @@ use gecko_bindings::structs::NODE_IS_NATIVE_ANONYMOUS; use gecko_bindings::sugar::ownership::HasArcFFI; use logical_geometry::WritingMode; use media_queries::Device; -use properties::{ComputedValues, parse_style_attribute}; +use properties::{AnimationRules, ComputedValues, parse_style_attribute}; use properties::{Importance, PropertyDeclaration, PropertyDeclarationBlock}; use properties::animated_properties::{AnimationValue, AnimationValueMap, TransitionProperty}; use properties::style_structs::Font; diff --git a/components/style/matching.rs b/components/style/matching.rs index f0e23baecff..578ecb4f2d5 100644 --- a/components/style/matching.rs +++ b/components/style/matching.rs @@ -10,10 +10,10 @@ use cascade_info::CascadeInfo; use context::{SelectorFlagsMap, SharedStyleContext, StyleContext}; use data::{ComputedStyle, ElementData, RestyleData}; -use dom::{AnimationRules, TElement, TNode}; +use dom::{TElement, TNode}; use font_metrics::FontMetricsProvider; use log::LogLevel::Trace; -use properties::{CascadeFlags, ComputedValues, SKIP_ROOT_AND_ITEM_BASED_DISPLAY_FIXUP, cascade}; +use properties::{AnimationRules, CascadeFlags, ComputedValues, SKIP_ROOT_AND_ITEM_BASED_DISPLAY_FIXUP, cascade}; use properties::longhands::display::computed_value as display; use restyle_hints::{RESTYLE_CSS_ANIMATIONS, RESTYLE_CSS_TRANSITIONS, RestyleReplacements}; use restyle_hints::{RESTYLE_STYLE_ATTRIBUTE, RESTYLE_SMIL}; diff --git a/components/style/properties/declaration_block.rs b/components/style/properties/declaration_block.rs index 1bd565f2153..c826d56a6b3 100644 --- a/components/style/properties/declaration_block.rs +++ b/components/style/properties/declaration_block.rs @@ -11,6 +11,7 @@ use cssparser::{DeclarationListParser, parse_important}; use cssparser::{Parser, AtRuleParser, DeclarationParser, Delimiter}; use error_reporting::ParseErrorReporter; use parser::{PARSING_MODE_DEFAULT, ParsingMode, ParserContext, log_css_error}; +use shared_lock::Locked; use std::fmt; use std::slice::Iter; use style_traits::ToCss; @@ -18,6 +19,20 @@ use stylesheets::{CssRuleType, Origin, UrlExtraData}; use super::*; #[cfg(feature = "gecko")] use properties::animated_properties::AnimationValueMap; +/// The animation rules. +/// +/// The first one is for Animation cascade level, and the second one is for +/// Transition cascade level. +pub struct AnimationRules(pub Option>>, + pub Option>>); + +impl AnimationRules { + /// Returns whether these animation rules represents an actual rule or not. + pub fn is_empty(&self) -> bool { + self.0.is_none() && self.1.is_none() + } +} + /// A declaration [importance][importance]. /// /// [importance]: https://drafts.csswg.org/css-cascade/#importance diff --git a/components/style/stylist.rs b/components/style/stylist.rs index 7fb64c8acf5..315e0b5207c 100644 --- a/components/style/stylist.rs +++ b/components/style/stylist.rs @@ -8,7 +8,7 @@ use {Atom, LocalName, Namespace}; use bit_vec::BitVec; use context::{QuirksMode, SharedStyleContext}; use data::ComputedStyle; -use dom::{AnimationRules, TElement}; +use dom::TElement; use element_state::ElementState; use error_reporting::RustLogReporter; use font_metrics::FontMetricsProvider; @@ -17,9 +17,9 @@ use gecko_bindings::structs::nsIAtom; use keyframes::KeyframesAnimation; use media_queries::Device; use properties::{self, CascadeFlags, ComputedValues}; +use properties::{AnimationRules, PropertyDeclarationBlock}; #[cfg(feature = "servo")] use properties::INHERIT_ALL; -use properties::PropertyDeclarationBlock; use restyle_hints::{HintComputationContext, DependencySet, RestyleHint}; use rule_tree::{CascadeLevel, RuleTree, StrongRuleNode, StyleSource}; use selector_map::{SelectorMap, SelectorMapEntry}; From 4935d972e54744cdc7b494a406b3f3570de395c9 Mon Sep 17 00:00:00 2001 From: Hiroyuki Ikezoe Date: Wed, 24 May 2017 11:11:56 +0900 Subject: [PATCH 3/6] Use animation values that have been processed during animation-only restyle for normal restyle. --- components/style/data.rs | 14 ++++- components/style/dom.rs | 7 +-- components/style/gecko/wrapper.rs | 7 +-- components/style/matching.rs | 56 +++++++++---------- .../style/properties/declaration_block.rs | 6 +- components/style/rule_tree/mod.rs | 28 +++++++++- components/style/stylist.rs | 5 +- 7 files changed, 76 insertions(+), 47 deletions(-) diff --git a/components/style/data.rs b/components/style/data.rs index 41a618dd32e..a0d694b9518 100644 --- a/components/style/data.rs +++ b/components/style/data.rs @@ -6,7 +6,7 @@ use context::SharedStyleContext; use dom::TElement; -use properties::{ComputedValues, PropertyDeclarationBlock}; +use properties::{AnimationRules, ComputedValues, PropertyDeclarationBlock}; use properties::longhands::display::computed_value as display; use restyle_hints::{HintComputationContext, RestyleReplacements, RestyleHint}; use rule_tree::StrongRuleNode; @@ -571,4 +571,16 @@ impl ElementData { None => None, } } + + /// Returns AnimationRules that has processed during animation-only restyles. + pub fn get_animation_rules(&self) -> AnimationRules { + if cfg!(feature = "servo") { + return AnimationRules(None, None) + } + + match self.get_styles() { + Some(s) => s.primary.rules.get_animation_rules(), + None => AnimationRules(None, None), + } + } } diff --git a/components/style/dom.rs b/components/style/dom.rs index de9d68396c7..44d1b0c3147 100644 --- a/components/style/dom.rs +++ b/components/style/dom.rs @@ -13,7 +13,7 @@ use atomic_refcell::{AtomicRef, AtomicRefCell, AtomicRefMut}; use data::ElementData; use element_state::ElementState; use font_metrics::FontMetricsProvider; -use properties::{AnimationRules, ComputedValues, PropertyDeclarationBlock}; +use properties::{ComputedValues, PropertyDeclarationBlock}; #[cfg(feature = "gecko")] use properties::animated_properties::AnimationValue; #[cfg(feature = "gecko")] use properties::animated_properties::TransitionProperty; use rule_tree::CascadeLevel; @@ -334,11 +334,6 @@ pub trait TElement : Eq + PartialEq + Debug + Hash + Sized + Copy + Clone + None } - /// Get this element's animation rules. - fn get_animation_rules(&self) -> AnimationRules { - AnimationRules(None, None) - } - /// Get this element's animation rule by the cascade level. fn get_animation_rule_by_cascade(&self, _cascade_level: CascadeLevel) diff --git a/components/style/gecko/wrapper.rs b/components/style/gecko/wrapper.rs index 011e0bff078..38f2af26249 100644 --- a/components/style/gecko/wrapper.rs +++ b/components/style/gecko/wrapper.rs @@ -57,7 +57,7 @@ use gecko_bindings::structs::NODE_IS_NATIVE_ANONYMOUS; use gecko_bindings::sugar::ownership::HasArcFFI; use logical_geometry::WritingMode; use media_queries::Device; -use properties::{AnimationRules, ComputedValues, parse_style_attribute}; +use properties::{ComputedValues, parse_style_attribute}; use properties::{Importance, PropertyDeclaration, PropertyDeclarationBlock}; use properties::animated_properties::{AnimationValue, AnimationValueMap, TransitionProperty}; use properties::style_structs::Font; @@ -613,11 +613,6 @@ impl<'le> TElement for GeckoElement<'le> { declarations.map(|s| s.as_arc_opt()).unwrap_or(None) } - fn get_animation_rules(&self) -> AnimationRules { - AnimationRules(self.get_animation_rule(), - self.get_transition_rule()) - } - fn get_animation_rule_by_cascade(&self, cascade_level: ServoCascadeLevel) -> Option>> { match cascade_level { diff --git a/components/style/matching.rs b/components/style/matching.rs index 578ecb4f2d5..5dd820f1666 100644 --- a/components/style/matching.rs +++ b/components/style/matching.rs @@ -254,7 +254,7 @@ trait PrivateMatchMethods: TElement { // We could make that a bit better if the complexity cost is not too // big, but given further restyles are posted directly to // pseudo-elements, it doesn't seem worth the effort at a glance. - if pseudo.is_eager() && self.get_animation_rules().is_empty() { + if pseudo.is_eager() && primary_style.rules.get_animation_rules().is_empty() { let parent = self.parent_element().unwrap(); let parent_data = parent.borrow_data().unwrap(); let pseudo_style = @@ -692,38 +692,39 @@ pub trait MatchMethods : TElement { let pseudo_style = parent_data.styles().pseudos.get(&pseudo).unwrap(); let mut rules = pseudo_style.rules.clone(); - let animation_rules = self.get_animation_rules(); + { + let animation_rules = data.get_animation_rules(); - // Handle animations here. - if let Some(animation_rule) = animation_rules.0 { - let animation_rule_node = - context.shared.stylist.rule_tree() - .update_rule_at_level(CascadeLevel::Animations, - Some(&animation_rule), - &mut rules, - &context.shared.guards); - if let Some(node) = animation_rule_node { - rules = node; + // Handle animations here. + if let Some(animation_rule) = animation_rules.0 { + let animation_rule_node = + context.shared.stylist.rule_tree() + .update_rule_at_level(CascadeLevel::Animations, + Some(&animation_rule), + &mut rules, + &context.shared.guards); + if let Some(node) = animation_rule_node { + rules = node; + } + } + + if let Some(animation_rule) = animation_rules.1 { + let animation_rule_node = + context.shared.stylist.rule_tree() + .update_rule_at_level(CascadeLevel::Transitions, + Some(&animation_rule), + &mut rules, + &context.shared.guards); + if let Some(node) = animation_rule_node { + rules = node; + } } } - - if let Some(animation_rule) = animation_rules.1 { - let animation_rule_node = - context.shared.stylist.rule_tree() - .update_rule_at_level(CascadeLevel::Transitions, - Some(&animation_rule), - &mut rules, - &context.shared.guards); - if let Some(node) = animation_rule_node { - rules = node; - } - } - let important_rules_changed = self.has_animations() && data.has_styles() && data.important_rules_are_different(&rules, - &context.shared.guards); + &context.shared.guards); return RulesMatchedResult { rule_nodes_changed: data.set_primary_rules(rules), @@ -738,10 +739,9 @@ pub trait MatchMethods : TElement { let style_attribute = self.style_attribute(); { let smil_override = data.get_smil_override(); - let animation_rules = self.get_animation_rules(); + let animation_rules = data.get_animation_rules(); let bloom = context.thread_local.bloom_filter.filter(); - let map = &mut context.thread_local.selector_flags; let mut set_selector_flags = |element: &Self, flags: ElementSelectorFlags| { self.apply_selector_flags(map, element, flags); diff --git a/components/style/properties/declaration_block.rs b/components/style/properties/declaration_block.rs index c826d56a6b3..e7c1b73d6c8 100644 --- a/components/style/properties/declaration_block.rs +++ b/components/style/properties/declaration_block.rs @@ -23,10 +23,10 @@ use super::*; /// /// The first one is for Animation cascade level, and the second one is for /// Transition cascade level. -pub struct AnimationRules(pub Option>>, - pub Option>>); +pub struct AnimationRules<'a>(pub Option<&'a Arc>>, + pub Option<&'a Arc>>); -impl AnimationRules { +impl<'a> AnimationRules<'a> { /// Returns whether these animation rules represents an actual rule or not. pub fn is_empty(&self) -> bool { self.0.is_none() && self.1.is_none() diff --git a/components/style/rule_tree/mod.rs b/components/style/rule_tree/mod.rs index d5c44329b72..c7d7e9815b7 100644 --- a/components/style/rule_tree/mod.rs +++ b/components/style/rule_tree/mod.rs @@ -8,7 +8,7 @@ #[cfg(feature = "servo")] use heapsize::HeapSizeOf; -use properties::{Importance, LonghandIdSet, PropertyDeclarationBlock}; +use properties::{AnimationRules, Importance, LonghandIdSet, PropertyDeclarationBlock}; use shared_lock::{Locked, StylesheetGuards, SharedRwLockReadGuard}; use smallvec::SmallVec; use std::io::{self, Write}; @@ -1152,6 +1152,32 @@ impl StrongRuleNode { .find(|node| node.cascade_level() == CascadeLevel::SMILOverride) .map(|node| node.get_animation_style()) } + + /// Returns AnimationRules that has processed during animation-only restyles. + pub fn get_animation_rules(&self) -> AnimationRules { + if cfg!(feature = "servo") { + return AnimationRules(None, None); + } + + let mut animation = None; + let mut transition = None; + + for node in self.self_and_ancestors() + .take_while(|node| node.cascade_level() >= CascadeLevel::Animations) { + match node.cascade_level() { + CascadeLevel::Animations => { + debug_assert!(animation.is_none()); + animation = Some(node.get_animation_style()) + }, + CascadeLevel::Transitions => { + debug_assert!(transition.is_none()); + transition = Some(node.get_animation_style()) + }, + _ => {}, + } + } + AnimationRules(animation, transition) + } } /// An iterator over a rule node and its ancestors. diff --git a/components/style/stylist.rs b/components/style/stylist.rs index 315e0b5207c..6093aa50820 100644 --- a/components/style/stylist.rs +++ b/components/style/stylist.rs @@ -943,7 +943,7 @@ impl Stylist { if let Some(anim) = animation_rules.0 { Push::push( applicable_declarations, - ApplicableDeclarationBlock::from_declarations(anim, + ApplicableDeclarationBlock::from_declarations(anim.clone(), CascadeLevel::Animations)); } debug!("animation: {:?}", context.relations); @@ -961,7 +961,8 @@ impl Stylist { if let Some(anim) = animation_rules.1 { Push::push( applicable_declarations, - ApplicableDeclarationBlock::from_declarations(anim, CascadeLevel::Transitions)); + ApplicableDeclarationBlock::from_declarations(anim.clone(), + CascadeLevel::Transitions)); } debug!("transition: {:?}", context.relations); debug!("push_applicable_declarations: shareable: {:?}", context.relations); From fce7c2d885ee326e750591818a4a7c9ad7070aff Mon Sep 17 00:00:00 2001 From: Hiroyuki Ikezoe Date: Wed, 24 May 2017 11:12:09 +0900 Subject: [PATCH 4/6] Check ElementHasAnimations before trying to get animations rules. In case of ::before and ::after element, the generated content has no ElementHasAnimations flag, their parent has the flag. --- components/style/dom.rs | 5 +++++ components/style/gecko/wrapper.rs | 10 +++++----- components/style/matching.rs | 21 ++++++++++++++------- 3 files changed, 24 insertions(+), 12 deletions(-) diff --git a/components/style/dom.rs b/components/style/dom.rs index 44d1b0c3147..188cbb10d32 100644 --- a/components/style/dom.rs +++ b/components/style/dom.rs @@ -506,6 +506,11 @@ pub trait TElement : Eq + PartialEq + Debug + Hash + Sized + Copy + Clone + /// Returns true if the element has all the specified selector flags. fn has_selector_flags(&self, flags: ElementSelectorFlags) -> bool; + /// In Gecko, element has a flag that represents the element may have + /// any type of animations or not to bail out animation stuff early. + /// Whereas Servo doesn't have such flag. + fn may_have_animations(&self) -> bool { false } + /// Creates a task to update various animation state on a given (pseudo-)element. #[cfg(feature = "gecko")] fn update_animations(&self, diff --git a/components/style/gecko/wrapper.rs b/components/style/gecko/wrapper.rs index 38f2af26249..919f17bb0d5 100644 --- a/components/style/gecko/wrapper.rs +++ b/components/style/gecko/wrapper.rs @@ -422,11 +422,6 @@ impl<'le> GeckoElement<'le> { } } - #[inline] - fn may_have_animations(&self) -> bool { - self.as_node().get_bool_flag(nsINode_BooleanFlag::ElementHasAnimations) - } - #[inline] fn has_id(&self) -> bool { self.as_node().get_bool_flag(nsINode_BooleanFlag::ElementHasID) @@ -777,6 +772,11 @@ impl<'le> TElement for GeckoElement<'le> { (self.flags() & node_flags) == node_flags } + #[inline] + fn may_have_animations(&self) -> bool { + self.as_node().get_bool_flag(nsINode_BooleanFlag::ElementHasAnimations) + } + fn update_animations(&self, before_change_style: Option>, tasks: UpdateAnimationsTasks) { diff --git a/components/style/matching.rs b/components/style/matching.rs index 5dd820f1666..e11141e8d63 100644 --- a/components/style/matching.rs +++ b/components/style/matching.rs @@ -254,12 +254,15 @@ trait PrivateMatchMethods: TElement { // We could make that a bit better if the complexity cost is not too // big, but given further restyles are posted directly to // pseudo-elements, it doesn't seem worth the effort at a glance. - if pseudo.is_eager() && primary_style.rules.get_animation_rules().is_empty() { + if pseudo.is_eager() { let parent = self.parent_element().unwrap(); - let parent_data = parent.borrow_data().unwrap(); - let pseudo_style = - parent_data.styles().pseudos.get(&pseudo).unwrap(); - return pseudo_style.values().clone() + if !parent.may_have_animations() || + primary_style.rules.get_animation_rules().is_empty() { + let parent_data = parent.borrow_data().unwrap(); + let pseudo_style = + parent_data.styles().pseudos.get(&pseudo).unwrap(); + return pseudo_style.values().clone() + } } } @@ -692,7 +695,7 @@ pub trait MatchMethods : TElement { let pseudo_style = parent_data.styles().pseudos.get(&pseudo).unwrap(); let mut rules = pseudo_style.rules.clone(); - { + if parent.may_have_animations() { let animation_rules = data.get_animation_rules(); // Handle animations here. @@ -739,7 +742,11 @@ pub trait MatchMethods : TElement { let style_attribute = self.style_attribute(); { let smil_override = data.get_smil_override(); - let animation_rules = data.get_animation_rules(); + let animation_rules = if self.may_have_animations() { + data.get_animation_rules() + } else { + AnimationRules(None, None) + }; let bloom = context.thread_local.bloom_filter.filter(); let map = &mut context.thread_local.selector_flags; From a921d1af224cba485c217dc3480a9acb072bc6d1 Mon Sep 17 00:00:00 2001 From: Hiroyuki Ikezoe Date: Wed, 24 May 2017 14:00:16 +0900 Subject: [PATCH 5/6] Make replace_rules returning boolean. We only use whether the return value is IMPORTANT_RULES_CHANGED or not, so we can just return true if an important rules was changed in the function. Also, we can just return false in case of animation rules changes sine for animation we can ensure there is no importan rules. Because of these changes, replace_rule_node does not borrow |result| so that we can drop a scope there. --- components/style/matching.rs | 110 ++++++++++++++++------------------ components/style/traversal.rs | 4 +- 2 files changed, 55 insertions(+), 59 deletions(-) diff --git a/components/style/matching.rs b/components/style/matching.rs index e11141e8d63..61130b7d609 100644 --- a/components/style/matching.rs +++ b/components/style/matching.rs @@ -972,78 +972,74 @@ pub trait MatchMethods : TElement { } /// Updates the rule nodes without re-running selector matching, using just - /// the rule tree. Returns RulesChanged which indicates whether the rule nodes changed - /// and whether the important rules changed. + /// the rule tree. Returns true if an !important rule was replaced. fn replace_rules(&self, replacements: RestyleReplacements, context: &StyleContext, data: &mut ElementData) - -> RulesChanged { + -> bool { use properties::PropertyDeclarationBlock; use shared_lock::Locked; let element_styles = &mut data.styles_mut(); let primary_rules = &mut element_styles.primary.rules; - let mut result = RulesChanged::empty(); + let mut result = false; - { - let mut replace_rule_node = |level: CascadeLevel, - pdb: Option<&Arc>>, - path: &mut StrongRuleNode| { - let new_node = context.shared.stylist.rule_tree() - .update_rule_at_level(level, pdb, path, &context.shared.guards); - if let Some(n) = new_node { + let replace_rule_node = |level: CascadeLevel, + pdb: Option<&Arc>>, + path: &mut StrongRuleNode| -> bool { + let new_node = context.shared.stylist.rule_tree() + .update_rule_at_level(level, pdb, path, &context.shared.guards); + match new_node { + Some(n) => { *path = n; - if level.is_important() { - result.insert(IMPORTANT_RULES_CHANGED); - } else { - result.insert(NORMAL_RULES_CHANGED); - } - } - }; + level.is_important() + }, + None => false, + } + }; - // Animation restyle hints are processed prior to other restyle - // hints in the animation-only traversal. - // - // Non-animation restyle hints will be processed in a subsequent - // normal traversal. - if replacements.intersects(RestyleReplacements::for_animations()) { - debug_assert!(context.shared.traversal_flags.for_animation_only()); + // Animation restyle hints are processed prior to other restyle + // hints in the animation-only traversal. + // + // Non-animation restyle hints will be processed in a subsequent + // normal traversal. + if replacements.intersects(RestyleReplacements::for_animations()) { + debug_assert!(context.shared.traversal_flags.for_animation_only()); - if replacements.contains(RESTYLE_SMIL) { - replace_rule_node(CascadeLevel::SMILOverride, - self.get_smil_override(), - primary_rules); - } - - let mut replace_rule_node_for_animation = |level: CascadeLevel, - primary_rules: &mut StrongRuleNode| { - let animation_rule = self.get_animation_rule_by_cascade(level); - replace_rule_node(level, - animation_rule.as_ref(), - primary_rules); - }; - - // Apply Transition rules and Animation rules if the corresponding restyle hint - // is contained. - if replacements.contains(RESTYLE_CSS_TRANSITIONS) { - replace_rule_node_for_animation(CascadeLevel::Transitions, - primary_rules); - } - - if replacements.contains(RESTYLE_CSS_ANIMATIONS) { - replace_rule_node_for_animation(CascadeLevel::Animations, - primary_rules); - } - } else if replacements.contains(RESTYLE_STYLE_ATTRIBUTE) { - let style_attribute = self.style_attribute(); - replace_rule_node(CascadeLevel::StyleAttributeNormal, - style_attribute, - primary_rules); - replace_rule_node(CascadeLevel::StyleAttributeImportant, - style_attribute, + if replacements.contains(RESTYLE_SMIL) { + replace_rule_node(CascadeLevel::SMILOverride, + self.get_smil_override(), primary_rules); } + + let replace_rule_node_for_animation = |level: CascadeLevel, + primary_rules: &mut StrongRuleNode| { + let animation_rule = self.get_animation_rule_by_cascade(level); + replace_rule_node(level, + animation_rule.as_ref(), + primary_rules); + }; + + // Apply Transition rules and Animation rules if the corresponding restyle hint + // is contained. + if replacements.contains(RESTYLE_CSS_TRANSITIONS) { + replace_rule_node_for_animation(CascadeLevel::Transitions, + primary_rules); + } + + if replacements.contains(RESTYLE_CSS_ANIMATIONS) { + replace_rule_node_for_animation(CascadeLevel::Animations, + primary_rules); + } + } else if replacements.contains(RESTYLE_STYLE_ATTRIBUTE) { + let style_attribute = self.style_attribute(); + result |= replace_rule_node(CascadeLevel::StyleAttributeNormal, + style_attribute, + primary_rules); + result |= replace_rule_node(CascadeLevel::StyleAttributeImportant, + style_attribute, + primary_rules); } result diff --git a/components/style/traversal.rs b/components/style/traversal.rs index 6e6ccf813a1..146069b1bc3 100644 --- a/components/style/traversal.rs +++ b/components/style/traversal.rs @@ -770,11 +770,11 @@ fn compute_style(_traversal: &D, ) } CascadeWithReplacements(flags) => { - let rules_changed = element.replace_rules(flags, context, data); + let important_rules_changed = element.replace_rules(flags, context, data); element.cascade_primary_and_pseudos( context, data, - rules_changed.important_rules_changed() + important_rules_changed ) } CascadeOnly => { From 976b3ba6e79b4c2682b961a5defe88f5e1195d00 Mon Sep 17 00:00:00 2001 From: Hiroyuki Ikezoe Date: Wed, 24 May 2017 14:00:30 +0900 Subject: [PATCH 6/6] Don't process style attribute changes in animation-only restyle. --- components/style/matching.rs | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/components/style/matching.rs b/components/style/matching.rs index 61130b7d609..edb6f923fe3 100644 --- a/components/style/matching.rs +++ b/components/style/matching.rs @@ -983,7 +983,6 @@ pub trait MatchMethods : TElement { let element_styles = &mut data.styles_mut(); let primary_rules = &mut element_styles.primary.rules; - let mut result = false; let replace_rule_node = |level: CascadeLevel, pdb: Option<&Arc>>, @@ -999,6 +998,20 @@ pub trait MatchMethods : TElement { } }; + if !context.shared.traversal_flags.for_animation_only() { + let mut result = false; + if replacements.contains(RESTYLE_STYLE_ATTRIBUTE) { + let style_attribute = self.style_attribute(); + result |= replace_rule_node(CascadeLevel::StyleAttributeNormal, + style_attribute, + primary_rules); + result |= replace_rule_node(CascadeLevel::StyleAttributeImportant, + style_attribute, + primary_rules); + } + return result; + } + // Animation restyle hints are processed prior to other restyle // hints in the animation-only traversal. // @@ -1032,17 +1045,9 @@ pub trait MatchMethods : TElement { replace_rule_node_for_animation(CascadeLevel::Animations, primary_rules); } - } else if replacements.contains(RESTYLE_STYLE_ATTRIBUTE) { - let style_attribute = self.style_attribute(); - result |= replace_rule_node(CascadeLevel::StyleAttributeNormal, - style_attribute, - primary_rules); - result |= replace_rule_node(CascadeLevel::StyleAttributeImportant, - style_attribute, - primary_rules); } - result + false } /// Attempts to share a style with another node. This method is unsafe