From aab7b04925830ab0a97b313e2b21f16c63941277 Mon Sep 17 00:00:00 2001 From: Bobby Holley Date: Wed, 26 Oct 2016 06:36:06 -0500 Subject: [PATCH] servo: Merge #13913 - incremental restyle: Introduce StylingMode and deprecate explicit dirtiness (from bholley:styling_mode); r=emilio This is another chunk of work to move us toward the new incremental restyle architecture. Eventually, we'll make a fine-grained decision at each node about what style to recompute based on the RestyleHint on the node data (along with other things). For now, we use the existence of RestyleData as a coarse-grained approximation of this. Source-Repo: https://github.com/servo/servo Source-Revision: c8b6ece97b6eec8ac4e16a7a79a605356279cfb6 --- servo/components/layout/traversal.rs | 14 +++- servo/components/layout_thread/lib.rs | 2 +- servo/components/script/layout_wrapper.rs | 32 +++++++-- servo/components/style/data.rs | 15 ++--- servo/components/style/dom.rs | 78 +++++++++++++++++++--- servo/components/style/gecko/traversal.rs | 8 ++- servo/components/style/gecko/wrapper.rs | 38 +++++------ servo/components/style/parallel.rs | 28 ++------ servo/components/style/sequential.rs | 16 ++--- servo/components/style/traversal.rs | 80 ++++++++++++++--------- servo/ports/geckolib/glue.rs | 9 ++- 11 files changed, 204 insertions(+), 116 deletions(-) diff --git a/servo/components/layout/traversal.rs b/servo/components/layout/traversal.rs index bec162f00107..5cd1a236f5cd 100644 --- a/servo/components/layout/traversal.rs +++ b/servo/components/layout/traversal.rs @@ -13,8 +13,10 @@ use gfx::display_list::OpaqueNode; use script_layout_interface::restyle_damage::{BUBBLE_ISIZES, REFLOW, REFLOW_OUT_OF_FLOW, REPAINT, RestyleDamage}; use script_layout_interface::wrapper_traits::{LayoutNode, ThreadSafeLayoutNode}; use std::mem; +use style::atomic_refcell::AtomicRefCell; use style::context::{LocalStyleContext, SharedStyleContext, StyleContext}; -use style::dom::TNode; +use style::data::NodeData; +use style::dom::{TNode, TRestyleDamage}; use style::selector_impl::ServoSelectorImpl; use style::traversal::{DomTraversalContext, recalc_style_at, remove_from_bloom_filter}; use style::traversal::RestyleResult; @@ -75,13 +77,18 @@ impl<'lc, N> DomTraversalContext for RecalcStyleAndConstructFlows<'lc> // done by the HTML parser. node.initialize_data(); - recalc_style_at(&self.context, self.root, node) + recalc_style_at::<_, _, Self>(&self.context, self.root, node) } fn process_postorder(&self, node: N) { construct_flows_at(&self.context, self.root, node); } + fn ensure_node_data(node: &N) -> &AtomicRefCell { + node.initialize_data(); + node.get_style_data().unwrap() + } + fn local_context(&self) -> &LocalStyleContext { self.context.local_context() } @@ -103,7 +110,8 @@ fn construct_flows_at<'a, N: LayoutNode>(context: &'a LayoutContext<'a>, root: O // Always reconstruct if incremental layout is turned off. let nonincremental_layout = opts::get().nonincremental_layout; - if nonincremental_layout || node.is_dirty() || node.has_dirty_descendants() { + if nonincremental_layout || node.has_dirty_descendants() || + node.restyle_damage() != N::ConcreteRestyleDamage::empty() { let mut flow_constructor = FlowConstructor::new(context); if nonincremental_layout || !flow_constructor.repair_if_possible(&tnode) { flow_constructor.process(&tnode); diff --git a/servo/components/layout_thread/lib.rs b/servo/components/layout_thread/lib.rs index 24c59c1b339c..51ec52cb0d04 100644 --- a/servo/components/layout_thread/lib.rs +++ b/servo/components/layout_thread/lib.rs @@ -1146,7 +1146,7 @@ impl LayoutThread { if !needs_dirtying { for (el, snapshot) in modified_elements { let hint = rw_data.stylist.compute_restyle_hint(&el, &snapshot, el.get_state()); - el.note_restyle_hint(hint); + el.note_restyle_hint::(hint); } } diff --git a/servo/components/script/layout_wrapper.rs b/servo/components/script/layout_wrapper.rs index 653bcae478bc..4d615d2a50e4 100644 --- a/servo/components/script/layout_wrapper.rs +++ b/servo/components/script/layout_wrapper.rs @@ -186,14 +186,10 @@ impl<'ln> TNode for ServoLayoutNode<'ln> { self.node.downcast().map(ServoLayoutDocument::from_layout_js) } - fn is_dirty(&self) -> bool { + fn deprecated_dirty_bit_is_set(&self) -> bool { unsafe { self.node.get_flag(IS_DIRTY) } } - unsafe fn set_dirty(&self) { - self.node.set_flag(IS_DIRTY, true) - } - fn has_dirty_descendants(&self) -> bool { unsafe { self.node.get_flag(HAS_DIRTY_DESCENDANTS) } } @@ -242,6 +238,20 @@ impl<'ln> TNode for ServoLayoutNode<'ln> { data.style_data.style_text_node(style); if self.has_changed() { data.restyle_damage = RestyleDamage::rebuild_and_reflow(); + } else { + // FIXME(bholley): This is necessary to make it correct to use restyle + // damage in construct_flows_at to determine whether to reconstruct + // text nodes. Without it, we fail cascade-import-dynamic-002.htm. + // + // Long-term, We should teach layout how to correctly propagate + // style changes from elements to child text nodes so that we don't + // need to do this explicitly here. This will likely all be rolled + // into a patch where we stop styling text nodes from the style + // system and instead generate the styles on the fly during frame + // construction / repair. + let parent = self.parent_node().unwrap(); + let parent_data = parent.get_partial_layout_data().unwrap().borrow(); + data.restyle_damage = parent_data.restyle_damage; } } @@ -358,6 +368,14 @@ impl<'ln> LayoutNode for ServoLayoutNode<'ln> { } impl<'ln> ServoLayoutNode<'ln> { + pub fn is_dirty(&self) -> bool { + unsafe { self.node.get_flag(IS_DIRTY) } + } + + pub unsafe fn set_dirty(&self) { + self.node.set_flag(IS_DIRTY, true) + } + fn get_partial_layout_data(&self) -> Option<&AtomicRefCell> { unsafe { self.get_jsmanaged().get_style_and_layout_data().map(|d| { @@ -397,7 +415,9 @@ impl<'ln> ServoLayoutNode<'ln> { fn debug_str(self) -> String { format!("{:?}: changed={} dirty={} dirty_descendants={}", - self.script_type_id(), self.has_changed(), self.is_dirty(), self.has_dirty_descendants()) + self.script_type_id(), self.has_changed(), + self.deprecated_dirty_bit_is_set(), + self.has_dirty_descendants()) } fn debug_style_str(self) -> String { diff --git a/servo/components/style/data.rs b/servo/components/style/data.rs index 511b1e174c31..9e6d6054fb03 100644 --- a/servo/components/style/data.rs +++ b/servo/components/style/data.rs @@ -115,14 +115,14 @@ impl RestyleData { #[derive(Debug)] pub struct NodeData { styles: NodeDataStyles, - pub restyle: Option, + pub restyle_data: Option, } impl NodeData { pub fn new() -> Self { NodeData { styles: NodeDataStyles::Uninitialized, - restyle: None, + restyle_data: None, } } @@ -175,25 +175,24 @@ impl NodeData { self.styles = match mem::replace(&mut self.styles, Uninitialized) { Uninitialized => Previous(f()), Current(x) => Previous(Some(x)), - _ => panic!("Already have previous styles"), + Previous(x) => Previous(x), }; } - // FIXME(bholley): Called in future patches. pub fn ensure_restyle_data(&mut self) { - if self.restyle.is_none() { - self.restyle = Some(RestyleData::new()); + if self.restyle_data.is_none() { + self.restyle_data = Some(RestyleData::new()); } } pub fn style_text_node(&mut self, style: Arc) { - debug_assert!(self.restyle.is_none()); self.styles = NodeDataStyles::Current(NodeStyles::new(style)); + self.restyle_data = None; } pub fn finish_styling(&mut self, styles: NodeStyles) { debug_assert!(self.styles.is_previous()); self.styles = NodeDataStyles::Current(styles); - self.restyle = None; + self.restyle_data = None; } } diff --git a/servo/components/style/dom.rs b/servo/components/style/dom.rs index d8b82d94d048..8f8a53f2c1f8 100644 --- a/servo/components/style/dom.rs +++ b/servo/components/style/dom.rs @@ -7,7 +7,7 @@ #![allow(unsafe_code)] use atomic_refcell::{AtomicRef, AtomicRefMut}; -use data::NodeData; +use data::{NodeStyles, NodeData}; use element_state::ElementState; use parking_lot::RwLock; use properties::{ComputedValues, PropertyDeclarationBlock}; @@ -19,6 +19,8 @@ use std::fmt::Debug; use std::ops::BitOr; use std::sync::Arc; use string_cache::{Atom, Namespace}; +use traversal::DomTraversalContext; +use util::opts; pub use style_traits::UnsafeNode; @@ -43,6 +45,19 @@ impl OpaqueNode { } } +#[derive(Clone, Copy, PartialEq)] +pub enum StylingMode { + /// The node has never been styled before, and needs a full style computation. + Initial, + /// The node has been styled before, but needs some amount of recomputation. + Restyle, + /// The node does not need any style processing, but one or more of its + /// descendants do. + Traverse, + /// No nodes in this subtree require style processing. + Stop, +} + pub trait TRestyleDamage : Debug + PartialEq + BitOr + Copy { /// The source for our current computed values in the cascade. This is a /// ComputedValues in Servo and a StyleContext in Gecko. @@ -117,9 +132,11 @@ pub trait TNode : Sized + Copy + Clone + NodeInfo { fn as_document(&self) -> Option; - fn is_dirty(&self) -> bool; - - unsafe fn set_dirty(&self); + /// The concept of a dirty bit doesn't exist in our new restyle algorithm. + /// Instead, we associate restyle and change hints with nodes. However, we + /// continue to allow the dirty bit to trigger unconditional restyles while + /// we transition both Servo and Stylo to the new architecture. + fn deprecated_dirty_bit_is_set(&self) -> bool; fn has_dirty_descendants(&self) -> bool; @@ -141,6 +158,51 @@ pub trait TNode : Sized + Copy + Clone + NodeInfo { /// traversal. Returns the number of children left to process. fn did_process_child(&self) -> isize; + /// Returns true if this node has a styled layout frame that owns the style. + fn frame_has_style(&self) -> bool { false } + + /// Returns the styles from the layout frame that owns them, if any. + /// + /// FIXME(bholley): Once we start dropping NodeData from nodes when + /// creating frames, we'll want to teach this method to actually get + /// style data from the frame. + fn get_styles_from_frame(&self) -> Option { None } + + /// Returns the styling mode for this node. This is only valid to call before + /// and during restyling, before finish_styling is invoked. + /// + /// See the comments around StylingMode. + fn styling_mode(&self) -> StylingMode { + use self::StylingMode::*; + + // Non-incremental layout impersonates Initial. + if opts::get().nonincremental_layout { + return Initial; + } + + // Compute the default result if this node doesn't require processing. + let mode_for_descendants = if self.has_dirty_descendants() { + Traverse + } else { + Stop + }; + + match self.borrow_data() { + // No node data, no style on the frame. + None if !self.frame_has_style() => Initial, + // No node data, style on the frame. + None => mode_for_descendants, + Some(d) => { + if d.restyle_data.is_some() || self.deprecated_dirty_bit_is_set() { + Restyle + } else { + debug_assert!(!self.frame_has_style()); // display:none etc + mode_for_descendants + } + }, + } + } + /// Sets up the appropriate data structures to style a node, returing a /// mutable handle to the node data upon which further style calculations /// can be performed. @@ -212,7 +274,7 @@ pub trait TElement : PartialEq + Debug + Sized + Copy + Clone + ElementExt + Pre fn attr_equals(&self, namespace: &Namespace, attr: &Atom, value: &Atom) -> bool; /// Properly marks nodes as dirty in response to restyle hints. - fn note_restyle_hint(&self, hint: RestyleHint) { + fn note_restyle_hint>(&self, hint: RestyleHint) { // Bail early if there's no restyling to do. if hint.is_empty() { return; @@ -230,13 +292,13 @@ pub trait TElement : PartialEq + Debug + Sized + Copy + Clone + ElementExt + Pre // Process hints. if hint.contains(RESTYLE_SELF) { - unsafe { node.set_dirty(); } + unsafe { C::ensure_node_data(&node).borrow_mut().ensure_restyle_data(); } // XXX(emilio): For now, dirty implies dirty descendants if found. } else if hint.contains(RESTYLE_DESCENDANTS) { unsafe { node.set_dirty_descendants(); } let mut current = node.first_child(); while let Some(node) = current { - unsafe { node.set_dirty(); } + unsafe { C::ensure_node_data(&node).borrow_mut().ensure_restyle_data(); } current = node.next_sibling(); } } @@ -245,7 +307,7 @@ pub trait TElement : PartialEq + Debug + Sized + Copy + Clone + ElementExt + Pre let mut next = ::selectors::Element::next_sibling_element(self); while let Some(sib) = next { let sib_node = sib.as_node(); - unsafe { sib_node.set_dirty() }; + unsafe { C::ensure_node_data(&sib_node).borrow_mut().ensure_restyle_data() }; next = ::selectors::Element::next_sibling_element(&sib); } } diff --git a/servo/components/style/gecko/traversal.rs b/servo/components/style/gecko/traversal.rs index 2325bd1d3032..61a9bd32de86 100644 --- a/servo/components/style/gecko/traversal.rs +++ b/servo/components/style/gecko/traversal.rs @@ -2,7 +2,9 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ +use atomic_refcell::AtomicRefCell; use context::{LocalStyleContext, SharedStyleContext, StyleContext}; +use data::NodeData; use dom::OpaqueNode; use gecko::context::StandaloneStyleContext; use gecko::wrapper::GeckoNode; @@ -29,7 +31,7 @@ impl<'lc, 'ln> DomTraversalContext> for RecalcStyleOnly<'lc> { } fn process_preorder(&self, node: GeckoNode<'ln>) -> RestyleResult { - recalc_style_at(&self.context, self.root, node) + recalc_style_at::<_, _, Self>(&self.context, self.root, node) } fn process_postorder(&self, _: GeckoNode<'ln>) { @@ -39,6 +41,10 @@ impl<'lc, 'ln> DomTraversalContext> for RecalcStyleOnly<'lc> { /// We don't use the post-order traversal for anything. fn needs_postorder_traversal(&self) -> bool { false } + fn ensure_node_data<'a>(node: &'a GeckoNode<'ln>) -> &'a AtomicRefCell { + node.ensure_data() + } + fn local_context(&self) -> &LocalStyleContext { self.context.local_context() } diff --git a/servo/components/style/gecko/wrapper.rs b/servo/components/style/gecko/wrapper.rs index 9340dc687765..187aed0acd80 100644 --- a/servo/components/style/gecko/wrapper.rs +++ b/servo/components/style/gecko/wrapper.rs @@ -6,7 +6,7 @@ use atomic_refcell::{AtomicRef, AtomicRefCell, AtomicRefMut}; -use data::{NodeData, NodeStyles}; +use data::NodeData; use dom::{LayoutIterator, NodeInfo, TDocument, TElement, TNode, TRestyleDamage, UnsafeNode}; use dom::{OpaqueNode, PresentationalHintsSynthetizer}; use element_state::ElementState; @@ -95,18 +95,11 @@ impl<'ln> GeckoNode<'ln> { .get(pseudo).map(|c| c.clone())) } - fn styles_from_frame(&self) -> Option { - // FIXME(bholley): Once we start dropping NodeData from nodes when - // creating frames, we'll want to teach this method to actually get - // style data from the frame. - None - } - fn get_node_data(&self) -> Option<&AtomicRefCell> { unsafe { self.0.mServoData.get().as_ref() } } - fn ensure_node_data(&self) -> &AtomicRefCell { + pub fn ensure_data(&self) -> &AtomicRefCell { match self.get_node_data() { Some(x) => x, None => { @@ -224,20 +217,10 @@ impl<'ln> TNode for GeckoNode<'ln> { unimplemented!() } - fn is_dirty(&self) -> bool { - // Return true unconditionally if we're not yet styled. This is a hack - // and should go away soon. - if self.get_node_data().is_none() { - return true; - } - + fn deprecated_dirty_bit_is_set(&self) -> bool { self.flags() & (NODE_IS_DIRTY_FOR_SERVO as u32) != 0 } - unsafe fn set_dirty(&self) { - self.set_flags(NODE_IS_DIRTY_FOR_SERVO as u32) - } - fn has_dirty_descendants(&self) -> bool { // Return true unconditionally if we're not yet styled. This is a hack // and should go away soon. @@ -271,14 +254,19 @@ impl<'ln> TNode for GeckoNode<'ln> { } fn begin_styling(&self) -> AtomicRefMut { - let mut data = self.ensure_node_data().borrow_mut(); - data.gather_previous_styles(|| self.styles_from_frame()); + let mut data = self.ensure_data().borrow_mut(); + data.gather_previous_styles(|| self.get_styles_from_frame()); data } fn style_text_node(&self, style: Arc) { debug_assert!(self.is_text_node()); - self.ensure_node_data().borrow_mut().style_text_node(style); + + // FIXME(bholley): Gecko currently relies on the dirty bit being set to + // drive the post-traversal. This will go away soon. + unsafe { self.set_flags(NODE_IS_DIRTY_FOR_SERVO as u32); } + + self.ensure_data().borrow_mut().style_text_node(style); } fn borrow_data(&self) -> Option> { @@ -291,6 +279,10 @@ impl<'ln> TNode for GeckoNode<'ln> { } fn set_restyle_damage(self, damage: Self::ConcreteRestyleDamage) { + // FIXME(bholley): Gecko currently relies on the dirty bit being set to + // drive the post-traversal. This will go away soon. + unsafe { self.set_flags(NODE_IS_DIRTY_FOR_SERVO as u32) } + unsafe { Gecko_StoreStyleDifference(self.0, damage.0) } } diff --git a/servo/components/style/parallel.rs b/servo/components/style/parallel.rs index d8dc89d43de7..ad83d3ac475c 100644 --- a/servo/components/style/parallel.rs +++ b/servo/components/style/parallel.rs @@ -8,7 +8,7 @@ #![allow(unsafe_code)] -use dom::{OpaqueNode, TNode, UnsafeNode}; +use dom::{OpaqueNode, StylingMode, TNode, UnsafeNode}; use std::mem; use std::sync::atomic::Ordering; use traversal::{RestyleResult, DomTraversalContext}; @@ -47,6 +47,7 @@ pub fn traverse_dom(root: N, where N: TNode, C: DomTraversalContext { + debug_assert!(root.styling_mode() != StylingMode::Stop); if opts::get().style_sharing_stats { STYLE_SHARING_CACHE_HITS.store(0, Ordering::SeqCst); STYLE_SHARING_CACHE_MISSES.store(0, Ordering::SeqCst); @@ -80,28 +81,13 @@ fn top_down_dom(unsafe_nodes: UnsafeNodeList, // Get a real layout node. let node = unsafe { N::from_unsafe(&unsafe_node) }; - if !context.should_process(node) { - continue; - } - - // Possibly enqueue the children. - let mut children_to_process = 0isize; // Perform the appropriate traversal. + let mut children_to_process = 0isize; if let RestyleResult::Continue = context.process_preorder(node) { - 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()) - } - } + C::traverse_children(node, |kid| { + children_to_process += 1; + discovered_child_nodes.push(kid.to_unsafe()) + }); } // Reset the count of children if we need to do a bottom-up traversal diff --git a/servo/components/style/sequential.rs b/servo/components/style/sequential.rs index d3db166b446e..87925b4d5f7e 100644 --- a/servo/components/style/sequential.rs +++ b/servo/components/style/sequential.rs @@ -4,7 +4,7 @@ //! Implements sequential traversal over the DOM tree. -use dom::TNode; +use dom::{StylingMode, TNode}; use traversal::{RestyleResult, DomTraversalContext}; pub fn traverse_dom(root: N, @@ -16,14 +16,8 @@ pub fn traverse_dom(root: N, where N: TNode, C: DomTraversalContext { - debug_assert!(context.should_process(node)); if let RestyleResult::Continue = context.process_preorder(node) { - for kid in node.children() { - context.pre_process_child_hook(node, kid); - if context.should_process(kid) { - doit::(context, kid); - } - } + C::traverse_children(node, |kid| doit::(context, kid)); } if context.needs_postorder_traversal() { @@ -31,10 +25,10 @@ pub fn traverse_dom(root: N, } } + debug_assert!(root.styling_mode() != StylingMode::Stop); let context = C::new(shared, root.opaque()); - if context.should_process(root) { - doit::(&context, root); - } + doit::(&context, root); + // Clear the local LRU cache since we store stateful elements inside. context.local_context().style_sharing_candidate_cache.borrow_mut().clear(); } diff --git a/servo/components/style/traversal.rs b/servo/components/style/traversal.rs index 0405c2f90bd6..1efe9721b6c0 100644 --- a/servo/components/style/traversal.rs +++ b/servo/components/style/traversal.rs @@ -4,8 +4,10 @@ //! Traversing the DOM tree; the bloom filter. +use atomic_refcell::AtomicRefCell; use context::{LocalStyleContext, SharedStyleContext, StyleContext}; -use dom::{OpaqueNode, TNode, UnsafeNode}; +use data::NodeData; +use dom::{OpaqueNode, StylingMode, TNode, UnsafeNode}; use matching::{ApplicableDeclarations, ElementMatchMethods, MatchMethods, StyleSharingResult}; use selectors::bloom::BloomFilter; use selectors::matching::StyleRelations; @@ -22,6 +24,7 @@ pub type Generation = u32; /// an element. /// /// So far this only happens where a display: none node is found. +#[derive(Clone, Copy, PartialEq)] pub enum RestyleResult { Continue, Stop, @@ -172,30 +175,30 @@ pub trait DomTraversalContext { /// If it's false, then process_postorder has no effect at all. fn needs_postorder_traversal(&self) -> bool { true } - /// Returns if the node should be processed by the preorder traversal (and - /// then by the post-order one). - /// - /// Note that this is true unconditionally for servo, since it requires to - /// bubble the widths bottom-up for all the DOM. - fn should_process(&self, node: N) -> bool { - opts::get().nonincremental_layout || node.is_dirty() || node.has_dirty_descendants() - } + /// Helper for the traversal implementations to select the children that + /// should be enqueued for processing. + fn traverse_children(parent: N, mut f: F) + { + // If we enqueue any children for traversal, we need to set the dirty + // descendants bit. Avoid doing it more than once. + let mut marked_dirty_descendants = false; - /// Do an action over the child before pushing him to the work queue. - /// - /// By default, propagate the IS_DIRTY flag down the tree. - #[allow(unsafe_code)] - fn pre_process_child_hook(&self, parent: N, kid: N) { - // NOTE: At this point is completely safe to modify either the parent or - // the child, since we have exclusive access to both of them. - if parent.is_dirty() { - unsafe { - kid.set_dirty(); - parent.set_dirty_descendants(); + for kid in parent.children() { + if kid.styling_mode() != StylingMode::Stop { + if !marked_dirty_descendants { + unsafe { parent.set_dirty_descendants(); } + marked_dirty_descendants = true; + } + f(kid); } } } + /// Ensures the existence of the NodeData, and returns it. This can't live + /// on TNode because of the trait-based separation between Servo's script + /// and layout crates. + fn ensure_node_data(node: &N) -> &AtomicRefCell; + fn local_context(&self) -> &LocalStyleContext; } @@ -281,11 +284,12 @@ fn ensure_node_styled_internal<'a, N, C>(node: N, /// Calculates the style for a single node. #[inline] #[allow(unsafe_code)] -pub fn recalc_style_at<'a, N, C>(context: &'a C, - root: OpaqueNode, - node: N) -> RestyleResult +pub fn recalc_style_at<'a, N, C, D>(context: &'a C, + root: OpaqueNode, + node: N) -> RestyleResult where N: TNode, - C: StyleContext<'a> + C: StyleContext<'a>, + D: DomTraversalContext { // Get the parent node. let parent_opt = match node.parent_node() { @@ -296,9 +300,10 @@ pub fn recalc_style_at<'a, N, C>(context: &'a C, // Get the style bloom filter. 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() { + let mode = node.styling_mode(); + debug_assert!(mode != StylingMode::Stop, "Parent should not have enqueued us"); + if mode != StylingMode::Traverse { // Check to see whether we can share a style with someone. let style_sharing_candidate_cache = &mut context.local_context().style_sharing_candidate_cache.borrow_mut(); @@ -370,6 +375,23 @@ pub fn recalc_style_at<'a, N, C>(context: &'a C, } } + // If we restyled this node, conservatively mark all our children as needing + // processing. The eventual algorithm we're designing does this in a more granular + // fashion. + if mode == StylingMode::Restyle && restyle_result == RestyleResult::Continue { + for kid in node.children() { + let mut data = D::ensure_node_data(&kid).borrow_mut(); + if kid.is_text_node() { + data.ensure_restyle_data(); + } else { + data.gather_previous_styles(|| kid.get_styles_from_frame()); + if data.previous_styles().is_some() { + data.ensure_restyle_data(); + } + } + } + } + let unsafe_layout_node = node.to_unsafe(); // Before running the children, we need to insert our nodes into the bloom @@ -380,9 +402,5 @@ pub fn recalc_style_at<'a, N, C>(context: &'a C, // NB: flow construction updates the bloom filter on the way up. put_thread_local_bloom_filter(bf, &unsafe_layout_node, context.shared_context()); - if nonincremental_layout { - RestyleResult::Continue - } else { - restyle_result - } + restyle_result } diff --git a/servo/ports/geckolib/glue.rs b/servo/ports/geckolib/glue.rs index 75cede9de3a3..49f62c0d7267 100644 --- a/servo/ports/geckolib/glue.rs +++ b/servo/ports/geckolib/glue.rs @@ -11,7 +11,7 @@ use std::mem::transmute; use std::sync::{Arc, Mutex}; use style::arc_ptr_eq; use style::context::{LocalStyleContextCreationInfo, ReflowGoal, SharedStyleContext}; -use style::dom::{NodeInfo, TElement, TNode}; +use style::dom::{NodeInfo, StylingMode, TElement, TNode}; use style::error_reporting::StdoutErrorReporter; use style::gecko::data::{NUM_THREADS, PerDocumentStyleData}; use style::gecko::selector_impl::{GeckoSelectorImpl, PseudoElement}; @@ -106,8 +106,11 @@ fn restyle_subtree(node: GeckoNode, raw_data: RawServoStyleSetBorrowed) { timer: Timer::new(), }; - // We ensure this is true before calling Servo_RestyleSubtree() - debug_assert!(node.is_dirty() || node.has_dirty_descendants()); + if node.styling_mode() == StylingMode::Stop { + error!("Unnecessary call to restyle_subtree"); + return; + } + if per_doc_data.num_threads == 1 || per_doc_data.work_queue.is_none() { sequential::traverse_dom::(node, &shared_style_context); } else {