diff --git a/components/layout/traversal.rs b/components/layout/traversal.rs index f9dcd975f9c..a8c94c52715 100644 --- a/components/layout/traversal.rs +++ b/components/layout/traversal.rs @@ -16,6 +16,7 @@ use std::mem; use style::context::SharedStyleContext; use style::dom::TNode; use style::selector_impl::ServoSelectorImpl; +use style::traversal::RestyleResult; use style::traversal::{DomTraversalContext, remove_from_bloom_filter, recalc_style_at}; use util::opts; use wrapper::{LayoutNodeLayoutData, ThreadSafeLayoutNodeHelpers}; @@ -69,12 +70,12 @@ impl<'lc, N> DomTraversalContext for RecalcStyleAndConstructFlows<'lc> } } - fn process_preorder(&self, node: N) { - // FIXME(pcwalton): Stop allocating here. Ideally this should just be done by the HTML - // parser. + fn process_preorder(&self, node: N) -> RestyleResult { + // FIXME(pcwalton): Stop allocating here. Ideally this should just be + // done by the HTML parser. node.initialize_data(); - recalc_style_at(&self.context, self.root, node); + recalc_style_at(&self.context, self.root, node) } fn process_postorder(&self, node: N) { diff --git a/components/script_layout_interface/restyle_damage.rs b/components/script_layout_interface/restyle_damage.rs index fc7c11d4259..95722cc1a86 100644 --- a/components/script_layout_interface/restyle_damage.rs +++ b/components/script_layout_interface/restyle_damage.rs @@ -47,6 +47,10 @@ impl TRestyleDamage for RestyleDamage { /// For Servo the style source is always the computed values. type PreExistingComputedValues = Arc; + fn empty() -> Self { + RestyleDamage::empty() + } + fn compute(old: Option<&Arc>, new: &Arc) -> RestyleDamage { compute_damage(old, new) diff --git a/components/style/dom.rs b/components/style/dom.rs index 510f139b401..e21b0de6d4d 100644 --- a/components/style/dom.rs +++ b/components/style/dom.rs @@ -15,6 +15,7 @@ use restyle_hints::{RESTYLE_DESCENDANTS, RESTYLE_LATER_SIBLINGS, RESTYLE_SELF, R use selector_impl::ElementExt; use selectors::matching::DeclarationBlock; use sink::Push; +use std::fmt::Debug; use std::ops::BitOr; use std::sync::Arc; use string_cache::{Atom, Namespace}; @@ -44,7 +45,7 @@ impl OpaqueNode { } } -pub trait TRestyleDamage : BitOr + Copy { +pub trait TRestyleDamage : Debug + BitOr + Copy { /// The source for our current computed values in the cascade. This is a /// ComputedValues in Servo and a StyleContext in Gecko. /// @@ -58,6 +59,8 @@ pub trait TRestyleDamage : BitOr + Copy { fn compute(old: Option<&Self::PreExistingComputedValues>, new: &Arc) -> Self; + fn empty() -> Self; + fn rebuild_and_reflow() -> Self; } diff --git a/components/style/matching.rs b/components/style/matching.rs index aabecf86d7a..3ac9929098d 100644 --- a/components/style/matching.rs +++ b/components/style/matching.rs @@ -12,6 +12,7 @@ use cache::{LRUCache, SimpleHashCache}; use context::{StyleContext, SharedStyleContext}; use data::PrivateStyleData; use dom::{TElement, TNode, TRestyleDamage}; +use properties::longhands::display::computed_value as display; use properties::{ComputedValues, PropertyDeclaration, cascade}; use selector_impl::{ElementExt, TheSelectorImpl, PseudoElement}; use selector_matching::{DeclarationBlock, Stylist}; @@ -25,6 +26,7 @@ use std::hash::{BuildHasherDefault, Hash, Hasher}; use std::slice::Iter; use std::sync::Arc; use string_cache::{Atom, Namespace}; +use traversal::RestyleResult; use util::opts; fn create_common_style_affecting_attributes_from_element(element: &E) @@ -410,9 +412,9 @@ impl StyleSharingCandidateCache { pub enum StyleSharingResult { /// We didn't find anybody to share the style with. CannotShare, - /// The node's style can be shared. The integer specifies the index in the LRU cache that was - /// hit and the damage that was done. - StyleWasShared(usize, ConcreteRestyleDamage), + /// The node's style can be shared. The integer specifies the index in the + /// LRU cache that was hit and the damage that was done. + StyleWasShared(usize, ConcreteRestyleDamage, RestyleResult), } trait PrivateMatchMethods: TNode { @@ -424,22 +426,22 @@ trait PrivateMatchMethods: TNode { context: &Ctx, parent_style: Option<&Arc>, applicable_declarations: &[DeclarationBlock], - mut style: Option<&mut Arc>, + mut old_style: Option<&mut Arc>, applicable_declarations_cache: &mut ApplicableDeclarationsCache, shareable: bool, animate_properties: bool) - -> (Self::ConcreteRestyleDamage, Arc) - where Ctx: StyleContext<'a> { + -> Arc + where Ctx: StyleContext<'a> + { let mut cacheable = true; let shared_context = context.shared_context(); if animate_properties { cacheable = !self.update_animations_for_cascade(shared_context, - &mut style) && cacheable; + &mut old_style) && cacheable; } - let this_style; - match parent_style { + let (this_style, is_cacheable) = match parent_style { Some(ref parent_style) => { let cache_entry = applicable_declarations_cache.find(applicable_declarations); let cached_computed_values = match cache_entry { @@ -447,27 +449,25 @@ trait PrivateMatchMethods: TNode { None => None, }; - let (the_style, is_cacheable) = cascade(shared_context.viewport_size, - applicable_declarations, - shareable, - Some(&***parent_style), - cached_computed_values, - shared_context.error_reporter.clone()); - cacheable = cacheable && is_cacheable; - this_style = the_style + cascade(shared_context.viewport_size, + applicable_declarations, + shareable, + Some(&***parent_style), + cached_computed_values, + shared_context.error_reporter.clone()) } None => { - let (the_style, is_cacheable) = cascade(shared_context.viewport_size, - applicable_declarations, - shareable, - None, - None, - shared_context.error_reporter.clone()); - cacheable = cacheable && is_cacheable; - this_style = the_style + cascade(shared_context.viewport_size, + applicable_declarations, + shareable, + None, + None, + shared_context.error_reporter.clone()) } }; + cacheable = cacheable && is_cacheable; + let mut this_style = Arc::new(this_style); if animate_properties { @@ -482,7 +482,7 @@ trait PrivateMatchMethods: TNode { // Trigger transitions if necessary. This will reset `this_style` back // to its old value if it did trigger a transition. - if let Some(ref style) = style { + if let Some(ref style) = old_style { animations_started |= animation::start_transitions_if_applicable( new_animations_sender, @@ -496,21 +496,13 @@ trait PrivateMatchMethods: TNode { } - let existing_style = - self.existing_style_for_restyle_damage(style.map(|s| &*s)); - - // Calculate style difference. - let damage = - Self::ConcreteRestyleDamage::compute(existing_style, &this_style); - // Cache the resolved style if it was cacheable. if cacheable { applicable_declarations_cache.insert(applicable_declarations.to_vec(), this_style.clone()); } - // Return the final style and the damage done to our caller. - (damage, this_style) + this_style } fn update_animations_for_cascade(&self, @@ -654,8 +646,15 @@ pub trait ElementMatchMethods : TElement { damage }; + let restyle_result = if shared_style.get_box().clone_display() == display::T::none { + RestyleResult::Stop + } else { + RestyleResult::Continue + }; + *style = Some(shared_style); - return StyleSharingResult::StyleWasShared(i, damage) + + return StyleSharingResult::StyleWasShared(i, damage, restyle_result) } } @@ -718,6 +717,7 @@ pub trait MatchMethods : TNode { context: &Ctx, parent: Option, applicable_declarations: &ApplicableDeclarations) + -> RestyleResult where Ctx: StyleContext<'a> { // Get our parent's style. This must be unsafe so that we don't touch the parent's @@ -736,70 +736,146 @@ pub trait MatchMethods : TNode { let mut applicable_declarations_cache = context.local_context().applicable_declarations_cache.borrow_mut(); - let damage; - if self.is_text_node() { + let (damage, restyle_result) = if self.is_text_node() { let mut data_ref = self.mutate_data().unwrap(); let mut data = &mut *data_ref; let cloned_parent_style = ComputedValues::style_for_child_text_node(parent_style.unwrap()); - { + let damage = { let existing_style = self.existing_style_for_restyle_damage(data.style.as_ref()); - damage = Self::ConcreteRestyleDamage::compute(existing_style, - &cloned_parent_style); - } + Self::ConcreteRestyleDamage::compute(existing_style, + &cloned_parent_style) + }; data.style = Some(cloned_parent_style); + + (damage, RestyleResult::Continue) } else { - damage = { - let mut data_ref = self.mutate_data().unwrap(); - let mut data = &mut *data_ref; - let (mut damage, final_style) = self.cascade_node_pseudo_element( - context, - parent_style, - &applicable_declarations.normal, - data.style.as_mut(), - &mut applicable_declarations_cache, - applicable_declarations.normal_shareable, - true); + let mut data_ref = self.mutate_data().unwrap(); + let mut data = &mut *data_ref; + let final_style = + self.cascade_node_pseudo_element(context, parent_style, + &applicable_declarations.normal, + data.style.as_mut(), + &mut applicable_declarations_cache, + applicable_declarations.normal_shareable, + /* should_animate = */ true); - data.style = Some(final_style); - - ::Impl::each_eagerly_cascaded_pseudo_element(|pseudo| { - let applicable_declarations_for_this_pseudo = - applicable_declarations.per_pseudo.get(&pseudo).unwrap(); - - if !applicable_declarations_for_this_pseudo.is_empty() { - // NB: Transitions and animations should only work for - // pseudo-elements ::before and ::after - let should_animate_properties = - ::Impl::pseudo_is_before_or_after(&pseudo); - let (new_damage, style) = self.cascade_node_pseudo_element( - context, - Some(data.style.as_ref().unwrap()), - &*applicable_declarations_for_this_pseudo, - data.per_pseudo.get_mut(&pseudo), - &mut applicable_declarations_cache, - false, - should_animate_properties); - data.per_pseudo.insert(pseudo, style); - - damage = damage | new_damage; - } - }); - - damage - }; + let (damage, restyle_result) = + self.compute_damage_and_cascade_pseudos(final_style, + data, + context, + applicable_declarations, + &mut applicable_declarations_cache); self.set_can_be_fragmented(parent.map_or(false, |p| { p.can_be_fragmented() || parent_style.as_ref().unwrap().is_multicol() })); + + (damage, restyle_result) + }; + + + // This method needs to borrow the data as mutable, so make sure + // data_ref goes out of scope first. + self.set_restyle_damage(damage); + + restyle_result + } + + fn compute_damage_and_cascade_pseudos<'a, Ctx>(&self, + final_style: Arc, + data: &mut PrivateStyleData, + context: &Ctx, + applicable_declarations: &ApplicableDeclarations, + mut applicable_declarations_cache: &mut ApplicableDeclarationsCache) + -> (Self::ConcreteRestyleDamage, RestyleResult) + where Ctx: StyleContext<'a> + { + // Here we optimise the case of the style changing but both the + // previous and the new styles having display: none. In this + // case, we can always optimize the traversal, regardless of the + // restyle hint. + let this_display = final_style.get_box().clone_display(); + if this_display == display::T::none { + let old_display = data.style.as_ref().map(|old_style| { + old_style.get_box().clone_display() + }); + + // If display passed from none to something, then we need to reflow, + // otherwise, we don't do anything. + let damage = match old_display { + Some(display) if display == this_display => { + Self::ConcreteRestyleDamage::empty() + } + _ => { + Self::ConcreteRestyleDamage::rebuild_and_reflow() + } + }; + + debug!("Short-circuiting traversal: {:?} {:?} {:?}", + this_display, old_display, damage); + + data.style = Some(final_style); + return (damage, RestyleResult::Stop); } - // This method needs to borrow the data as mutable, so make sure data_ref goes out of - // scope first. - self.set_restyle_damage(damage); + // Otherwise, we just compute the damage normally, and sum up the damage + // related to pseudo-elements. + let mut damage = { + let existing_style = + self.existing_style_for_restyle_damage(data.style.as_ref()); + + Self::ConcreteRestyleDamage::compute(existing_style, &final_style) + }; + + data.style = Some(final_style); + + // FIXME(emilio): This is not pretty, and in the Gecko case means + // effectively comparing with the old computed values (given our style + // source is the old nsStyleContext). + // + // I... don't think any difference can arise from comparing with the old + // element restyle damage vs the new one, given that we're only summing + // the changes, and any change that we could miss would already have + // been caught by the parent's change. If for some reason I'm wrong on + // this, we'd have to compare computed values in Gecko too. + let existing_style = + self.existing_style_for_restyle_damage(data.style.as_ref()); + + let data_per_pseudo = &mut data.per_pseudo; + let new_style = data.style.as_ref(); + debug_assert!(new_style.is_some()); + + ::Impl::each_eagerly_cascaded_pseudo_element(|pseudo| { + let applicable_declarations_for_this_pseudo = + applicable_declarations.per_pseudo.get(&pseudo).unwrap(); + + if !applicable_declarations_for_this_pseudo.is_empty() { + // NB: Transitions and animations should only work for + // pseudo-elements ::before and ::after + let should_animate_properties = + ::Impl::pseudo_is_before_or_after(&pseudo); + + let new_pseudo_style = + self.cascade_node_pseudo_element(context, + new_style, + &*applicable_declarations_for_this_pseudo, + data_per_pseudo.get_mut(&pseudo), + &mut applicable_declarations_cache, + /* shareable = */ false, + should_animate_properties); + + let new_damage = + Self::ConcreteRestyleDamage::compute(existing_style, &new_pseudo_style); + damage = damage | new_damage; + data_per_pseudo.insert(pseudo, new_pseudo_style); + } + }); + + (damage, RestyleResult::Continue) } } diff --git a/components/style/parallel.rs b/components/style/parallel.rs index 8f9927d4992..25d61d99d79 100644 --- a/components/style/parallel.rs +++ b/components/style/parallel.rs @@ -11,7 +11,7 @@ use dom::{OpaqueNode, TNode, UnsafeNode}; use std::mem; use std::sync::atomic::Ordering; -use traversal::DomTraversalContext; +use traversal::{RestyleResult, DomTraversalContext}; use workqueue::{WorkQueue, WorkUnit, WorkerProxy}; #[allow(dead_code)] @@ -68,21 +68,27 @@ fn top_down_dom(unsafe_nodes: UnsafeNodeList, } // Perform the appropriate traversal. - context.process_preorder(node); + let should_stop = match context.process_preorder(node) { + RestyleResult::Stop => true, + RestyleResult::Continue => false, + }; // Possibly enqueue the children. let mut children_to_process = 0isize; - for kid in node.children() { - // Trigger the hook pre-adding the kid to the list. This can (and in - // fact uses to) change the result of the should_process operation. - // - // As of right now, this hook takes care of propagating the restyle - // flag down the tree. In the future, more accurate behavior is - // probably going to be needed. - context.pre_process_child_hook(node, kid); - if context.should_process(kid) { - children_to_process += 1; - discovered_child_nodes.push(kid.to_unsafe()) + if !should_stop { + for kid in node.children() { + // Trigger the hook pre-adding the kid to the list. This can + // (and in fact uses to) change the result of the should_process + // operation. + // + // As of right now, this hook takes care of propagating the + // restyle flag down the tree. In the future, more accurate + // behavior is probably going to be needed. + context.pre_process_child_hook(node, kid); + if context.should_process(kid) { + children_to_process += 1; + discovered_child_nodes.push(kid.to_unsafe()) + } } } diff --git a/components/style/sequential.rs b/components/style/sequential.rs index ac431b80057..b19f376302c 100644 --- a/components/style/sequential.rs +++ b/components/style/sequential.rs @@ -5,7 +5,7 @@ //! Implements sequential traversal over the DOM tree. use dom::TNode; -use traversal::DomTraversalContext; +use traversal::{RestyleResult, DomTraversalContext}; pub fn traverse_dom(root: N, shared: &C::SharedContext) @@ -17,12 +17,17 @@ pub fn traverse_dom(root: N, C: DomTraversalContext { debug_assert!(context.should_process(node)); - context.process_preorder(node); + let should_stop = match context.process_preorder(node) { + RestyleResult::Stop => true, + RestyleResult::Continue => false, + }; - for kid in node.children() { - context.pre_process_child_hook(node, kid); - if context.should_process(kid) { - doit::(context, kid); + if !should_stop { + for kid in node.children() { + context.pre_process_child_hook(node, kid); + if context.should_process(kid) { + doit::(context, kid); + } } } diff --git a/components/style/traversal.rs b/components/style/traversal.rs index d154145bcd4..6c13af1566d 100644 --- a/components/style/traversal.rs +++ b/components/style/traversal.rs @@ -18,6 +18,16 @@ use values::HasViewportPercentage; /// detected by ticking a generation number every layout. pub type Generation = u32; +/// This enum tells us about whether we can stop restyling or not after styling +/// an element. +/// +/// So far this only happens where a display: none node is found. +pub enum RestyleResult { + Continue, + Stop, +} + + /// A pair of the bloom filter used for css selector matching, and the node to /// which it applies. This is used to efficiently do `Descendant` selector /// matches. Thanks to the bloom filter, we can avoid walking up the tree @@ -145,7 +155,7 @@ pub trait DomTraversalContext { fn new<'a>(&'a Self::SharedContext, OpaqueNode) -> Self; /// Process `node` on the way down, before its children have been processed. - fn process_preorder(&self, node: N) -> ForceTraversalStop; + fn process_preorder(&self, node: N) -> RestyleResult; /// Process `node` on the way up, after its children have been processed. /// @@ -188,7 +198,7 @@ pub trait DomTraversalContext { #[allow(unsafe_code)] pub fn recalc_style_at<'a, N, C>(context: &'a C, root: OpaqueNode, - node: N) + node: N) -> RestyleResult where N: TNode, C: StyleContext<'a> { // Get the parent node. @@ -201,6 +211,7 @@ pub fn recalc_style_at<'a, N, C>(context: &'a C, let mut bf = take_thread_local_bloom_filter(parent_opt, root, context.shared_context()); let nonincremental_layout = opts::get().nonincremental_layout; + let mut restyle_result = RestyleResult::Continue; if nonincremental_layout || node.is_dirty() { // Remove existing CSS styles from nodes whose content has changed (e.g. text changed), // to force non-incremental reflow. @@ -250,9 +261,9 @@ pub fn recalc_style_at<'a, N, C>(context: &'a C, // Perform the CSS cascade. unsafe { - node.cascade_node(context, - parent_opt, - &applicable_declarations); + restyle_result = node.cascade_node(context, + parent_opt, + &applicable_declarations); } // Add ourselves to the LRU cache. @@ -260,7 +271,8 @@ pub fn recalc_style_at<'a, N, C>(context: &'a C, style_sharing_candidate_cache.insert_if_possible::<'ln, N>(&element); } } - StyleSharingResult::StyleWasShared(index, damage) => { + StyleSharingResult::StyleWasShared(index, damage, restyle_result_cascade) => { + restyle_result = restyle_result_cascade; style_sharing_candidate_cache.touch(index); node.set_restyle_damage(damage); } @@ -297,4 +309,10 @@ pub fn recalc_style_at<'a, N, C>(context: &'a C, } } } + + if nonincremental_layout { + RestyleResult::Continue + } else { + restyle_result + } } diff --git a/ports/geckolib/traversal.rs b/ports/geckolib/traversal.rs index 55fa99b6c69..50aa40dbd79 100644 --- a/ports/geckolib/traversal.rs +++ b/ports/geckolib/traversal.rs @@ -6,6 +6,7 @@ use context::StandaloneStyleContext; use std::mem; use style::context::SharedStyleContext; use style::dom::OpaqueNode; +use style::traversal::RestyleResult; use style::traversal::{DomTraversalContext, recalc_style_at}; use wrapper::GeckoNode; @@ -27,12 +28,12 @@ impl<'lc, 'ln> DomTraversalContext> for RecalcStyleOnly<'lc> { } } - fn process_preorder(&self, node: GeckoNode<'ln>) { + fn process_preorder(&self, node: GeckoNode<'ln>) -> RestyleResult { // FIXME(pcwalton): Stop allocating here. Ideally this should just be done by the HTML // parser. node.initialize_data(); - recalc_style_at(&self.context, self.root, node); + recalc_style_at(&self.context, self.root, node) } fn process_postorder(&self, _: GeckoNode<'ln>) { diff --git a/ports/geckolib/wrapper.rs b/ports/geckolib/wrapper.rs index efa8624d39a..272910ceb7a 100644 --- a/ports/geckolib/wrapper.rs +++ b/ports/geckolib/wrapper.rs @@ -95,16 +95,40 @@ impl<'ln> GeckoNode<'ln> { } } -#[derive(Clone, Copy)] +#[derive(Clone, Copy, Debug)] pub struct GeckoRestyleDamage(nsChangeHint); impl TRestyleDamage for GeckoRestyleDamage { type PreExistingComputedValues = nsStyleContext; + + fn empty() -> Self { + use std::mem; + GeckoRestyleDamage(unsafe { mem::transmute(0u32) }) + } + fn compute(source: Option<&nsStyleContext>, new_style: &Arc) -> Self { type Helpers = ArcHelpers; let context = match source { Some(ctx) => ctx as *const nsStyleContext as *mut nsStyleContext, + // If there's no style source (that is, no style context), there can + // be two reasons for it. + // + // The first one, is that this is not an incremental restyle (so we + // also don't have the old computed values). In that case the best + // we can do is return rebuild_and_reflow. + // + // The second one is that this is an incremental restyle, but the + // node has display: none. In this case, the old computed values + // should still be present, and we should be able to compare the new + // to the old display to see if it effectively needs a reflow, or we + // can do nothing on it because the old and the new display values + // are none. + // + // This is done outside of this method in servo itself, so we + // effectively only need to check for the first case. Ideally, we + // should be able to assert that in this case the display values + // differ or are not none, but we can't reach the node from here. None => return Self::rebuild_and_reflow(), };