From 436c9072c44a730aef4b29044bd0339155ce3618 Mon Sep 17 00:00:00 2001 From: Martin Robinson Date: Wed, 9 Jul 2025 19:33:09 +0200 Subject: [PATCH] layout: Skip box tree construction when possible (#37957) When a style change does not chang the structure of the box tree, it is possible to skip box tree rebuilding for an element. This change adds support for reusing old box trees when no element has that type of damage. In order to make this happen, there needs to be a type of "empty" `LayoutDamage` that just indicates that a fragment tree layout is necessary. This is the first step toward incremental fragment tree layout. Testing: This should not change observable behavior and thus is covered by existing WPT tests. Performance numbers to follow. Signed-off-by: Martin Robinson Co-authored-by: Oriol Brufau --- components/layout/dom.rs | 20 +++---- components/layout/flexbox/mod.rs | 6 +- components/layout/flow/inline/mod.rs | 10 ++-- components/layout/flow/mod.rs | 4 +- components/layout/flow/root.rs | 17 ------ components/layout/layout_box_base.rs | 8 ++- components/layout/layout_impl.rs | 45 ++++++++------ components/layout/style_ext.rs | 4 ++ components/layout/table/mod.rs | 10 ++-- components/layout/taffy/mod.rs | 6 +- components/layout/traversal.rs | 35 +++++++---- components/script/layout_dom/element.rs | 72 ++++++++++++++++++++++- components/shared/layout/layout_damage.rs | 22 +++++++ 13 files changed, 181 insertions(+), 78 deletions(-) diff --git a/components/layout/dom.rs b/components/layout/dom.rs index 6617c4a2053..65055fe1a97 100644 --- a/components/layout/dom.rs +++ b/components/layout/dom.rs @@ -77,24 +77,24 @@ pub(super) enum LayoutBox { } impl LayoutBox { - fn invalidate_cached_fragment(&self) { + fn clear_fragment_layout_cache(&self) { match self { LayoutBox::DisplayContents(..) => {}, LayoutBox::BlockLevel(block_level_box) => { - block_level_box.borrow().invalidate_cached_fragment() + block_level_box.borrow().clear_fragment_layout_cache() }, LayoutBox::InlineLevel(inline_items) => { for inline_item in inline_items.iter() { - inline_item.borrow().invalidate_cached_fragment() + inline_item.borrow().clear_fragment_layout_cache() } }, LayoutBox::FlexLevel(flex_level_box) => { - flex_level_box.borrow().invalidate_cached_fragment() + flex_level_box.borrow().clear_fragment_layout_cache() }, LayoutBox::TaffyItemBox(taffy_item_box) => { - taffy_item_box.borrow_mut().invalidate_cached_fragment() + taffy_item_box.borrow_mut().clear_fragment_layout_cache() }, - LayoutBox::TableLevelBox(table_box) => table_box.invalidate_cached_fragment(), + LayoutBox::TableLevelBox(table_box) => table_box.clear_fragment_layout_cache(), } } @@ -247,7 +247,7 @@ pub(crate) trait NodeExt<'dom> { fn unset_all_pseudo_boxes(&self); fn fragments_for_pseudo(&self, pseudo_element: Option) -> Vec; - fn invalidate_cached_fragment(&self); + fn clear_fragment_layout_cache(&self); fn repair_style(&self, context: &SharedStyleContext); fn take_restyle_damage(&self) -> LayoutDamage; @@ -381,15 +381,15 @@ impl<'dom> NodeExt<'dom> for ServoLayoutNode<'dom> { self.layout_data_mut().pseudo_boxes.clear(); } - fn invalidate_cached_fragment(&self) { + fn clear_fragment_layout_cache(&self) { let data = self.layout_data_mut(); if let Some(data) = data.self_box.borrow_mut().as_ref() { - data.invalidate_cached_fragment(); + data.clear_fragment_layout_cache(); } for pseudo_layout_data in data.pseudo_boxes.iter() { if let Some(layout_box) = pseudo_layout_data.box_slot.borrow().as_ref() { - layout_box.invalidate_cached_fragment(); + layout_box.clear_fragment_layout_cache(); } } } diff --git a/components/layout/flexbox/mod.rs b/components/layout/flexbox/mod.rs index faa57ae2465..679689183e5 100644 --- a/components/layout/flexbox/mod.rs +++ b/components/layout/flexbox/mod.rs @@ -175,17 +175,17 @@ impl FlexLevelBox { } } - pub(crate) fn invalidate_cached_fragment(&self) { + pub(crate) fn clear_fragment_layout_cache(&self) { match self { FlexLevelBox::FlexItem(flex_item_box) => flex_item_box .independent_formatting_context .base - .invalidate_cached_fragment(), + .clear_fragment_layout_cache(), FlexLevelBox::OutOfFlowAbsolutelyPositionedBox(positioned_box) => positioned_box .borrow() .context .base - .invalidate_cached_fragment(), + .clear_fragment_layout_cache(), } } diff --git a/components/layout/flow/inline/mod.rs b/components/layout/flow/inline/mod.rs index 06cc7f2acad..fa2c1c8702a 100644 --- a/components/layout/flow/inline/mod.rs +++ b/components/layout/flow/inline/mod.rs @@ -251,10 +251,10 @@ impl InlineItem { } } - pub(crate) fn invalidate_cached_fragment(&self) { + pub(crate) fn clear_fragment_layout_cache(&self) { match self { InlineItem::StartInlineBox(inline_box) => { - inline_box.borrow().base.invalidate_cached_fragment() + inline_box.borrow().base.clear_fragment_layout_cache() }, InlineItem::EndInlineBox | InlineItem::TextRun(..) => {}, InlineItem::OutOfFlowAbsolutelyPositionedBox(positioned_box, ..) => { @@ -262,18 +262,18 @@ impl InlineItem { .borrow() .context .base - .invalidate_cached_fragment(); + .clear_fragment_layout_cache(); }, InlineItem::OutOfFlowFloatBox(float_box) => float_box .borrow() .contents .base - .invalidate_cached_fragment(), + .clear_fragment_layout_cache(), InlineItem::Atomic(independent_formatting_context, ..) => { independent_formatting_context .borrow() .base - .invalidate_cached_fragment(); + .clear_fragment_layout_cache() }, } } diff --git a/components/layout/flow/mod.rs b/components/layout/flow/mod.rs index 5775f20fb98..09529d4a737 100644 --- a/components/layout/flow/mod.rs +++ b/components/layout/flow/mod.rs @@ -133,8 +133,8 @@ impl BlockLevelBox { } } - pub(crate) fn invalidate_cached_fragment(&self) { - self.with_base(LayoutBoxBase::invalidate_cached_fragment); + pub(crate) fn clear_fragment_layout_cache(&self) { + self.with_base(|base| base.clear_fragment_layout_cache()); } pub(crate) fn fragments(&self) -> Vec { diff --git a/components/layout/flow/root.rs b/components/layout/flow/root.rs index a41f5504068..ad8d624cb77 100644 --- a/components/layout/flow/root.rs +++ b/components/layout/flow/root.rs @@ -417,25 +417,8 @@ impl<'dom> IncrementalBoxTreeUpdate<'dom> { }, } - // We are going to rebuild the box tree from the update point downward, but this update - // point is an absolute, which means that it needs to be laid out again in the containing - // block for absolutes, which is established by one of its ancestors. In addition, - // absolutes, when laid out, can produce more absolutes (either fixed or absolutely - // positioned) elements, so there may be yet more layout that has to happen in this - // ancestor. - // - // We do not know which ancestor is the one that established the containing block for this - // update point, so just invalidate the fragment cache of all ancestors, meaning that even - // though the box tree is preserved, the fragment tree from the root to the update point and - // all of its descendants will need to be rebuilt. This isn't as bad as it seems, because - // siblings and siblings of ancestors of this path through the tree will still have cached - // fragments. - // - // TODO: Do better. This is still a very crude way to do incremental layout. let mut invalidate_start_point = self.node; while let Some(parent_node) = invalidate_start_point.parent_node() { - parent_node.invalidate_cached_fragment(); - // Box tree reconstruction doesn't need to involve these ancestors, so their // damage isn't useful for us. // diff --git a/components/layout/layout_box_base.rs b/components/layout/layout_box_base.rs index 161aee0f9bb..a18584679ac 100644 --- a/components/layout/layout_box_base.rs +++ b/components/layout/layout_box_base.rs @@ -70,8 +70,12 @@ impl LayoutBoxBase { result } - pub(crate) fn invalidate_cached_fragment(&self) { - let _ = self.cached_layout_result.borrow_mut().take(); + /// Clear cached data accumulated during fragment tree layout, either fragments and + /// the cached inline content size, or just fragments. + pub(crate) fn clear_fragment_layout_cache(&self) { + self.fragments.borrow_mut().clear(); + *self.cached_layout_result.borrow_mut() = None; + *self.cached_inline_content_size.borrow_mut() = None; } pub(crate) fn fragments(&self) -> Vec { diff --git a/components/layout/layout_impl.rs b/components/layout/layout_impl.rs index cac6ec75b04..0c30866cadc 100644 --- a/components/layout/layout_impl.rs +++ b/components/layout/layout_impl.rs @@ -26,7 +26,7 @@ use fonts_traits::StylesheetWebFontLoadFinishedCallback; use fxhash::FxHashMap; use ipc_channel::ipc::IpcSender; use layout_api::{ - IFrameSizes, Layout, LayoutConfig, LayoutFactory, NodesFromPointQueryType, + IFrameSizes, Layout, LayoutConfig, LayoutDamage, LayoutFactory, NodesFromPointQueryType, OffsetParentResponse, QueryMsg, ReflowGoal, ReflowRequest, ReflowRequestRestyle, ReflowResult, TrustedNodeAddress, }; @@ -849,29 +849,38 @@ impl LayoutThread { } let root_node = root_element.as_node(); - let mut damage = compute_damage_and_repair_style(&layout_context.style_context, root_node); - if viewport_changed { - damage = RestyleDamage::RELAYOUT; - } else if !damage.contains(RestyleDamage::RELAYOUT) { + let damage_from_environment = viewport_changed + .then_some(RestyleDamage::RELAYOUT) + .unwrap_or_default(); + let damage = compute_damage_and_repair_style( + &layout_context.style_context, + root_node, + damage_from_environment, + ); + + if !damage.contains(RestyleDamage::RELAYOUT) { layout_context.style_context.stylist.rule_tree().maybe_gc(); return (damage, IFrameSizes::default()); } let mut box_tree = self.box_tree.borrow_mut(); let box_tree = &mut *box_tree; - let mut build_box_tree = || { - if !BoxTree::update(recalc_style_traversal.context(), dirty_root) { - *box_tree = Some(Arc::new(BoxTree::construct( - recalc_style_traversal.context(), - root_node, - ))); - } - }; - if let Some(pool) = rayon_pool { - pool.install(build_box_tree) - } else { - build_box_tree() - }; + let layout_damage: LayoutDamage = damage.into(); + if box_tree.is_none() || layout_damage.has_box_damage() { + let mut build_box_tree = || { + if !BoxTree::update(recalc_style_traversal.context(), dirty_root) { + *box_tree = Some(Arc::new(BoxTree::construct( + recalc_style_traversal.context(), + root_node, + ))); + } + }; + if let Some(pool) = rayon_pool { + pool.install(build_box_tree) + } else { + build_box_tree() + }; + } let viewport_size = self.stylist.device().au_viewport_size(); let run_layout = || { diff --git a/components/layout/style_ext.rs b/components/layout/style_ext.rs index c60958d0e82..168e023241c 100644 --- a/components/layout/style_ext.rs +++ b/components/layout/style_ext.rs @@ -653,6 +653,10 @@ impl ComputedValuesExt for ComputedValues { /// Return true if this style is a normal block and establishes /// a new block formatting context. + /// + /// NOTE: This should be kept in sync with the checks in `impl + /// TElement::compute_layout_damage` for `ServoLayoutElement` in + /// `components/script/layout_dom/element.rs`. fn establishes_block_formatting_context(&self, fragment_flags: FragmentFlags) -> bool { if self.establishes_scroll_container(fragment_flags) { return true; diff --git a/components/layout/table/mod.rs b/components/layout/table/mod.rs index 78884c377e9..062d765bdb1 100644 --- a/components/layout/table/mod.rs +++ b/components/layout/table/mod.rs @@ -398,18 +398,18 @@ pub(crate) enum TableLevelBox { } impl TableLevelBox { - pub(crate) fn invalidate_cached_fragment(&self) { + pub(crate) fn clear_fragment_layout_cache(&self) { match self { TableLevelBox::Caption(caption) => { - caption.borrow().context.base.invalidate_cached_fragment(); + caption.borrow().context.base.clear_fragment_layout_cache(); }, TableLevelBox::Cell(cell) => { - cell.borrow().base.invalidate_cached_fragment(); + cell.borrow().base.clear_fragment_layout_cache(); }, TableLevelBox::TrackGroup(track_group) => { - track_group.borrow().base.invalidate_cached_fragment() + track_group.borrow().base.clear_fragment_layout_cache() }, - TableLevelBox::Track(track) => track.borrow().base.invalidate_cached_fragment(), + TableLevelBox::Track(track) => track.borrow().base.clear_fragment_layout_cache(), } } diff --git a/components/layout/taffy/mod.rs b/components/layout/taffy/mod.rs index 927f6bffc60..afacd71ffcd 100644 --- a/components/layout/taffy/mod.rs +++ b/components/layout/taffy/mod.rs @@ -128,21 +128,21 @@ impl TaffyItemBox { } } - pub(crate) fn invalidate_cached_fragment(&mut self) { + pub(crate) fn clear_fragment_layout_cache(&mut self) { self.taffy_layout = Default::default(); self.positioning_context = PositioningContext::default(); match self.taffy_level_box { TaffyItemBoxInner::InFlowBox(ref independent_formatting_context) => { independent_formatting_context .base - .invalidate_cached_fragment() + .clear_fragment_layout_cache() }, TaffyItemBoxInner::OutOfFlowAbsolutelyPositionedBox(ref positioned_box) => { positioned_box .borrow() .context .base - .invalidate_cached_fragment() + .clear_fragment_layout_cache() }, } } diff --git a/components/layout/traversal.rs b/components/layout/traversal.rs index 9a2e97f3162..cd845e8e688 100644 --- a/components/layout/traversal.rs +++ b/components/layout/traversal.rs @@ -97,8 +97,9 @@ where pub(crate) fn compute_damage_and_repair_style( context: &SharedStyleContext, node: ServoLayoutNode<'_>, + damage_from_environment: RestyleDamage, ) -> RestyleDamage { - compute_damage_and_repair_style_inner(context, node, RestyleDamage::empty()) + compute_damage_and_repair_style_inner(context, node, damage_from_environment) } pub(crate) fn compute_damage_and_repair_style_inner( @@ -107,6 +108,7 @@ pub(crate) fn compute_damage_and_repair_style_inner( damage_from_parent: RestyleDamage, ) -> RestyleDamage { let mut element_damage; + let original_element_damage; let element_data = &node .style_data() .expect("Should not run `compute_damage` before styling.") @@ -114,6 +116,7 @@ pub(crate) fn compute_damage_and_repair_style_inner( { let mut element_data = element_data.borrow_mut(); + original_element_damage = element_data.damage; element_data.damage.insert(damage_from_parent); element_damage = element_data.damage; @@ -134,10 +137,6 @@ pub(crate) fn compute_damage_and_repair_style_inner( } } - if element_damage != RestyleDamage::reconstruct() && !element_damage.is_empty() { - node.repair_style(context); - } - // If one of our children needed to be reconstructed, we need to recollect children // during box tree construction. if damage_from_children.contains(LayoutDamage::recollect_box_tree_children()) { @@ -145,11 +144,27 @@ pub(crate) fn compute_damage_and_repair_style_inner( element_data.borrow_mut().damage.insert(element_damage); } - if element_damage.contains(LayoutDamage::recollect_box_tree_children()) { - node.invalidate_cached_fragment(); - } - // Only propagate up layout phases from children, as other types of damage are // incorporated into `element_damage` above. - element_damage | (damage_from_children & RestyleDamage::RELAYOUT) + let damage_for_parent = element_damage | (damage_from_children & RestyleDamage::RELAYOUT); + + // If we are going to potentially reuse this box tree node, then clear any cached + // fragment layout. + // + // TODO: If this node has `recollect_box_tree_children` damage, this is unecessary + // unless it's entirely above the dirty root. + if element_damage != RestyleDamage::reconstruct() && + damage_for_parent.contains(RestyleDamage::RELAYOUT) + { + node.clear_fragment_layout_cache(); + } + + // If the box will be preserved, update the box's style and also in any fragments + // that haven't been cleared. + let element_layout_damage: LayoutDamage = element_damage.into(); + if !element_layout_damage.has_box_damage() && !original_element_damage.is_empty() { + node.repair_style(context); + } + + damage_for_parent } diff --git a/components/script/layout_dom/element.rs b/components/script/layout_dom/element.rs index 7675eb247fd..4f8e6df9680 100644 --- a/components/script/layout_dom/element.rs +++ b/components/script/layout_dom/element.rs @@ -33,7 +33,9 @@ use style::selector_parser::{ }; use style::shared_lock::Locked as StyleLocked; use style::stylesheets::scope_rule::ImplicitScopeRoot; -use style::values::computed::Display; +use style::values::computed::{Display, Image}; +use style::values::specified::align::AlignFlags; +use style::values::specified::box_::{DisplayInside, DisplayOutside}; use style::values::{AtomIdent, AtomString}; use stylo_atoms::Atom; use stylo_dom::ElementState; @@ -586,8 +588,72 @@ impl<'dom> style::dom::TElement for ServoLayoutElement<'dom> { } } - fn compute_layout_damage(_old: &ComputedValues, _new: &ComputedValues) -> RestyleDamage { - RestyleDamage::from_bits_retain(LayoutDamage::REBUILD_BOX.bits()) + fn compute_layout_damage(old: &ComputedValues, new: &ComputedValues) -> RestyleDamage { + let box_tree_needs_rebuild = || { + let old_box = old.get_box(); + let new_box = new.get_box(); + + if old_box.display != new_box.display || + old_box.float != new_box.float || + old_box.position != new_box.position + { + return true; + } + + if old.get_font() != new.get_font() { + return true; + } + + // NOTE: This should be kept in sync with the checks in `impl + // StyleExt::establishes_block_formatting_context` for `ComputedValues` in + // `components/layout/style_ext.rs`. + if new_box.display.outside() == DisplayOutside::Block && + new_box.display.inside() == DisplayInside::Flow + { + let alignment_establishes_new_block_formatting_context = + |style: &ComputedValues| { + style.get_position().align_content.0.primary() != AlignFlags::NORMAL + }; + + let old_column = old.get_column(); + let new_column = new.get_column(); + if old_box.overflow_x.is_scrollable() != new_box.overflow_x.is_scrollable() || + old_column.is_multicol() != new_column.is_multicol() || + old_column.column_span != new_column.column_span || + alignment_establishes_new_block_formatting_context(old) != + alignment_establishes_new_block_formatting_context(new) + { + return true; + } + } + + if old_box.display.is_list_item() { + let old_list = old.get_list(); + let new_list = new.get_list(); + if old_list.list_style_position != new_list.list_style_position || + old_list.list_style_image != new_list.list_style_image || + (new_list.list_style_image == Image::None && + old_list.list_style_type != new_list.list_style_type) + { + return true; + } + } + + if new.is_pseudo_style() && old.get_counters().content != new.get_counters().content { + return true; + } + + false + }; + + if box_tree_needs_rebuild() { + RestyleDamage::from_bits_retain(LayoutDamage::REBUILD_BOX.bits()) + } else { + // This element needs to be laid out again, but does not have any damage to + // its box. In the future, we will distinguish between types of damage to the + // fragment as well. + RestyleDamage::RELAYOUT + } } } diff --git a/components/shared/layout/layout_damage.rs b/components/shared/layout/layout_damage.rs index 559177fd378..4917c439028 100644 --- a/components/shared/layout/layout_damage.rs +++ b/components/shared/layout/layout_damage.rs @@ -25,7 +25,29 @@ impl LayoutDamage { RestyleDamage::RELAYOUT } + pub fn rebuild_box_tree() -> RestyleDamage { + RestyleDamage::from_bits_retain(LayoutDamage::REBUILD_BOX.bits()) | RestyleDamage::RELAYOUT + } + pub fn has_box_damage(&self) -> bool { self.intersects(Self::REBUILD_BOX) } } + +impl From for LayoutDamage { + fn from(restyle_damage: RestyleDamage) -> Self { + LayoutDamage::from_bits_retain(restyle_damage.bits()) + } +} + +impl std::fmt::Debug for LayoutDamage { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + if self.contains(Self::REBUILD_BOX) { + f.write_str("REBUILD_BOX") + } else if self.contains(Self::RECOLLECT_BOX_TREE_CHILDREN) { + f.write_str("RECOLLECT_BOX_TREE_CHILDREN") + } else { + f.write_str("EMPTY") + } + } +}