From 07d55a4bf842a996157276525f5cb5afd9078b26 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Tue, 26 Sep 2017 07:25:06 +0200 Subject: [PATCH] style: Lazily tweak the traversal root to account for sibling invalidations. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bug: 1403078 Reviewed-by: heycam MozReview-Commit-ID: Ij3nMOKu5FO Signed-off-by: Emilio Cobos Álvarez --- components/layout_thread/lib.rs | 15 ++++--- components/style/driver.rs | 6 +-- components/style/gecko/wrapper.rs | 17 -------- components/style/traversal.rs | 65 ++++++++++++++++++++----------- ports/geckolib/glue.rs | 29 ++++++++------ 5 files changed, 69 insertions(+), 63 deletions(-) diff --git a/components/layout_thread/lib.rs b/components/layout_thread/lib.rs index 64ef126b79e..9e59dc666e3 100644 --- a/components/layout_thread/lib.rs +++ b/components/layout_thread/lib.rs @@ -1268,15 +1268,11 @@ impl LayoutThread { None }; - // NB: Type inference falls apart here for some reason, so we need to be very verbose. :-( let traversal = RecalcStyleAndConstructFlows::new(layout_context); let token = { - let context = >::shared_context(&traversal); - >::pre_traverse(element, - context, - TraversalFlags::empty()) + let shared = + >::shared_context(&traversal); + RecalcStyleAndConstructFlows::pre_traverse(element, shared) }; if token.should_traverse() { @@ -1287,7 +1283,10 @@ impl LayoutThread { || { // Perform CSS selector matching and flow construction. driver::traverse_dom::( - &traversal, element, token, thread_pool); + &traversal, + token, + thread_pool, + ); }); // TODO(pcwalton): Measure energy usage of text shaping, perhaps? let text_shaping_time = diff --git a/components/style/driver.rs b/components/style/driver.rs index c6370d012e5..8040757d7c4 100644 --- a/components/style/driver.rs +++ b/components/style/driver.rs @@ -27,15 +27,15 @@ use traversal::{DomTraversal, PerLevelTraversalData, PreTraverseToken}; /// then transfer control over to the parallel traversal. pub fn traverse_dom( traversal: &D, - root: E, - token: PreTraverseToken, + token: PreTraverseToken, pool: Option<&rayon::ThreadPool> ) where E: TElement, D: DomTraversal, { - debug_assert!(token.should_traverse()); + let root = + token.traversal_root().expect("Should've ensured we needed to traverse"); let dump_stats = traversal.shared_context().options.dump_style_statistics; let start_time = if dump_stats { Some(time::precise_time_s()) } else { None }; diff --git a/components/style/gecko/wrapper.rs b/components/style/gecko/wrapper.rs index e4f2ad31b4b..174ae623931 100644 --- a/components/style/gecko/wrapper.rs +++ b/components/style/gecko/wrapper.rs @@ -488,23 +488,6 @@ impl<'le> GeckoElement<'le> { self.flags() & (NODE_NEEDS_FRAME as u32) != 0 } - /// Returns true if a traversal starting from this element requires a post-traversal. - pub fn needs_post_traversal(&self) -> bool { - debug!("needs_post_traversal: dd={}, aodd={}, lfcd={}, lfc={}, data={:?}", - self.has_dirty_descendants(), - self.has_animation_only_dirty_descendants(), - self.descendants_need_frames(), - self.needs_frame(), - self.borrow_data().unwrap()); - - let has_flag = - self.flags() & (ELEMENT_HAS_DIRTY_DESCENDANTS_FOR_SERVO as u32 | - ELEMENT_HAS_ANIMATION_ONLY_DIRTY_DESCENDANTS_FOR_SERVO as u32 | - NODE_DESCENDANTS_NEED_FRAMES as u32 | - NODE_NEEDS_FRAME as u32) != 0; - has_flag || self.borrow_data().unwrap().contains_restyle_data() - } - /// Returns true if this element has a shadow root. fn has_shadow_root(&self) -> bool { self.get_extended_slots().map_or(false, |slots| !slots.mShadowRoot.mRawPtr.is_null()) diff --git a/components/style/traversal.rs b/components/style/traversal.rs index 56df2d08728..c8ec158bf6e 100644 --- a/components/style/traversal.rs +++ b/components/style/traversal.rs @@ -34,10 +34,17 @@ pub struct PerLevelTraversalData { /// We use this structure, rather than just returning a boolean from pre_traverse, /// to enfore that callers process root invalidations before starting the traversal. -pub struct PreTraverseToken(bool); -impl PreTraverseToken { +pub struct PreTraverseToken(Option); +impl PreTraverseToken { /// Whether we should traverse children. - pub fn should_traverse(&self) -> bool { self.0 } + pub fn should_traverse(&self) -> bool { + self.0.is_some() + } + + /// Returns the traversal root for the current traversal. + pub(crate) fn traversal_root(self) -> Option { + self.0 + } } #[cfg(feature = "servo")] @@ -139,36 +146,48 @@ pub trait DomTraversal : Sync { fn pre_traverse( root: E, shared_context: &SharedStyleContext, - traversal_flags: TraversalFlags, - ) -> PreTraverseToken { + ) -> PreTraverseToken { + let traversal_flags = shared_context.traversal_flags; + // If this is an unstyled-only traversal, the caller has already verified // that there's something to traverse, and we don't need to do any // invalidation since we're not doing any restyling. if traversal_flags.contains(traversal_flags::UnstyledOnly) { - return PreTraverseToken(true) + return PreTraverseToken(Some(root)) } - let flags = shared_context.traversal_flags; let mut data = root.mutate_data(); let mut data = data.as_mut().map(|d| &mut **d); - - if let Some(ref mut data) = data { - // Invalidate our style, and the one of our siblings and descendants - // as needed. - data.invalidate_style_if_needed(root, shared_context, None); - - // Make sure we don't have any stale RECONSTRUCTED_ANCESTOR bits from - // the last traversal (at a potentially-higher root). From the - // perspective of this traversal, the root cannot have reconstructed - // ancestors. - data.set_reconstructed_ancestor(false); - }; - let parent = root.traversal_parent(); let parent_data = parent.as_ref().and_then(|p| p.borrow_data()); + + if let Some(ref mut data) = data { + // Make sure we don't have any stale RECONSTRUCTED_ANCESTOR bits + // from the last traversal (at a potentially-higher root). + // + // From the perspective of this traversal, the root cannot have + // reconstructed ancestors. + data.set_reconstructed_ancestor(false); + + if !traversal_flags.for_animation_only() { + // Invalidate our style, and that of our siblings and + // descendants as needed. + let invalidation_result = + data.invalidate_style_if_needed(root, shared_context, None); + + if invalidation_result.has_invalidated_siblings() { + let actual_root = + parent.expect("How in the world can you invalidate \ + siblings without a parent?"); + unsafe { actual_root.set_dirty_descendants() } + return PreTraverseToken(Some(actual_root)); + } + } + } + let should_traverse = Self::element_needs_traversal( root, - flags, + traversal_flags, data.as_mut().map(|d| &**d), parent_data.as_ref().map(|d| &**d) ); @@ -176,10 +195,10 @@ pub trait DomTraversal : Sync { // If we're not going to traverse at all, we may need to clear some state // off the root (which would normally be done at the end of recalc_style_at). if !should_traverse && data.is_some() { - clear_state_after_traversing(root, data.unwrap(), flags); + clear_state_after_traversing(root, data.unwrap(), traversal_flags); } - PreTraverseToken(should_traverse) + PreTraverseToken(if should_traverse { Some(root) } else { None }) } /// Returns true if traversal should visit a text node. The style system diff --git a/ports/geckolib/glue.rs b/ports/geckolib/glue.rs index 18f7afb224f..7f7faf250ae 100644 --- a/ports/geckolib/glue.rs +++ b/ports/geckolib/glue.rs @@ -235,16 +235,19 @@ fn traverse_subtree(element: GeckoElement, let global_style_data = &*GLOBAL_STYLE_DATA; let guard = global_style_data.shared_lock.read(); - let shared_style_context = create_shared_context(&global_style_data, - &guard, - &per_doc_data, - traversal_flags, - snapshots); + let shared_style_context = create_shared_context( + &global_style_data, + &guard, + &per_doc_data, + traversal_flags, + snapshots, + ); + let token = RecalcStyleOnly::pre_traverse( + element, + &shared_style_context, + ); - let token = RecalcStyleOnly::pre_traverse(element, - &shared_style_context, - traversal_flags); if !token.should_traverse() { return; } @@ -259,13 +262,13 @@ fn traverse_subtree(element: GeckoElement, }; let traversal = RecalcStyleOnly::new(shared_style_context); - driver::traverse_dom(&traversal, element, token, thread_pool); + driver::traverse_dom(&traversal, token, thread_pool); } /// Traverses the subtree rooted at `root` for restyling. /// -/// Returns whether a Gecko post-traversal (to perform lazy frame construction, -/// or consume any RestyleData, or drop any ElementData) is required. +/// Returns whether the root was restyled. Whether anything else was restyled or +/// not can be inferred from the dirty bits in the rest of the tree. #[no_mangle] pub extern "C" fn Servo_TraverseSubtree( root: RawGeckoElementBorrowed, @@ -309,7 +312,9 @@ pub extern "C" fn Servo_TraverseSubtree( element.needs_frame(), element.borrow_data().unwrap()); - element.needs_post_traversal() + let element_was_restyled = + element.borrow_data().unwrap().contains_restyle_data(); + element_was_restyled } /// Checks whether the rule tree has crossed its threshold for unused nodes, and