From 864c877be5ee16340f0511e3ebfe2ba2a802fb6d Mon Sep 17 00:00:00 2001 From: JoeDow Date: Sat, 5 Jul 2025 16:33:04 +0800 Subject: [PATCH] layout: Add incremental box tree construction for inline floats and abspos (#37892) Layout: Add incremental box tree construction for inline floats and abspos Due to false positives in the memory benchmark on CI, the previous PR [37868](https://github.com/servo/servo/pull/37868) reverted. Now it is resubmitted. Signed-off-by: sharpshooter_pt --- components/layout/dom.rs | 14 ++++++ components/layout/flow/construct.rs | 28 +++++++----- components/layout/flow/inline/construct.rs | 52 ++++++++++++++-------- 3 files changed, 66 insertions(+), 28 deletions(-) diff --git a/components/layout/dom.rs b/components/layout/dom.rs index 40023439982..124bc9269b6 100644 --- a/components/layout/dom.rs +++ b/components/layout/dom.rs @@ -136,6 +136,20 @@ impl LayoutBox { .repair_style(context, node, new_style), } } + + /// If this [`LayoutBox`] represents an unsplit (due to inline-block splits) inline + /// level item, unwrap and return it. If not, return `None`. + pub(crate) fn unsplit_inline_level_layout_box(self) -> Option> { + let LayoutBox::InlineLevel(inline_level_boxes) = self else { + return None; + }; + // If this element box has been subject to inline-block splitting, ignore it. It's + // not useful currently for incremental box tree construction. + if inline_level_boxes.len() != 1 { + return None; + } + inline_level_boxes.into_iter().next() + } } /// A wrapper for [`InnerDOMLayoutData`]. This is necessary to give the entire data diff --git a/components/layout/flow/construct.rs b/components/layout/flow/construct.rs index 48493eceade..024122823f3 100644 --- a/components/layout/flow/construct.rs +++ b/components/layout/flow/construct.rs @@ -604,13 +604,17 @@ impl<'dom> BlockContainerBuilder<'dom, '_> { ) { if let Some(builder) = self.inline_formatting_context_builder.as_mut() { if !builder.is_empty() { - let inline_level_box = - builder.push_absolutely_positioned_box(AbsolutelyPositionedBox::construct( + let constructor = || { + ArcRefCell::new(AbsolutelyPositionedBox::construct( self.context, info, display_inside, contents, - )); + )) + }; + let old_layout_box = box_slot.take_layout_box_if_undamaged(info.damage); + let inline_level_box = + builder.push_absolutely_positioned_box(constructor, old_layout_box); box_slot.set(LayoutBox::InlineLevel(vec![inline_level_box])); return; } @@ -637,13 +641,17 @@ impl<'dom> BlockContainerBuilder<'dom, '_> { ) { if let Some(builder) = self.inline_formatting_context_builder.as_mut() { if !builder.is_empty() { - let inline_level_box = builder.push_float_box(FloatBox::construct( - self.context, - info, - display_inside, - contents, - self.propagated_data, - )); + let constructor = || { + ArcRefCell::new(FloatBox::construct( + self.context, + info, + display_inside, + contents, + self.propagated_data, + )) + }; + let old_layout_box = box_slot.take_layout_box_if_undamaged(info.damage); + let inline_level_box = builder.push_float_box(constructor, old_layout_box); box_slot.set(LayoutBox::InlineLevel(vec![inline_level_box])); return; } diff --git a/components/layout/flow/inline/construct.rs b/components/layout/flow/inline/construct.rs index 1b4fe869416..b7c124819b8 100644 --- a/components/layout/flow/inline/construct.rs +++ b/components/layout/flow/inline/construct.rs @@ -160,19 +160,10 @@ impl InlineFormattingContextBuilder { ) -> ArcRefCell { // If there is an existing undamaged layout box that's compatible, use that. let independent_formatting_context = old_layout_box - .and_then(|layout_box| { - let LayoutBox::InlineLevel(inline_level_boxes) = layout_box else { - return None; - }; - - // If there's an existing box, it should be a compatible atomic inline and should - // not have been subject to inline-block splitting. - assert_eq!(inline_level_boxes.len(), 1); - let first_box = inline_level_boxes.into_iter().next()?; - match &*first_box.borrow() { - InlineItem::Atomic(atomic, ..) => Some(atomic.clone()), - _ => None, - } + .and_then(LayoutBox::unsplit_inline_level_layout_box) + .and_then(|inline_item| match &*inline_item.borrow() { + InlineItem::Atomic(atomic, ..) => Some(atomic.clone()), + _ => None, }) .unwrap_or_else(independent_formatting_context_creator); @@ -195,9 +186,20 @@ impl InlineFormattingContextBuilder { pub(crate) fn push_absolutely_positioned_box( &mut self, - absolutely_positioned_box: AbsolutelyPositionedBox, + absolutely_positioned_box_creator: impl FnOnce() -> ArcRefCell, + old_layout_box: Option, ) -> ArcRefCell { - let absolutely_positioned_box = ArcRefCell::new(absolutely_positioned_box); + let absolutely_positioned_box = old_layout_box + .and_then(LayoutBox::unsplit_inline_level_layout_box) + .and_then(|inline_item| match &*inline_item.borrow() { + InlineItem::OutOfFlowAbsolutelyPositionedBox(positioned_box, ..) => { + Some(positioned_box.clone()) + }, + _ => None, + }) + .unwrap_or_else(absolutely_positioned_box_creator); + + // We cannot just reuse the old inline item, because the `current_text_offset` may have changed. let inline_level_box = ArcRefCell::new(InlineItem::OutOfFlowAbsolutelyPositionedBox( absolutely_positioned_box, self.current_text_offset, @@ -207,9 +209,23 @@ impl InlineFormattingContextBuilder { inline_level_box } - pub(crate) fn push_float_box(&mut self, float_box: FloatBox) -> ArcRefCell { - let inline_level_box = - ArcRefCell::new(InlineItem::OutOfFlowFloatBox(ArcRefCell::new(float_box))); + pub(crate) fn push_float_box( + &mut self, + float_box_creator: impl FnOnce() -> ArcRefCell, + old_layout_box: Option, + ) -> ArcRefCell { + let inline_level_box = old_layout_box + .and_then(LayoutBox::unsplit_inline_level_layout_box) + .unwrap_or_else(|| ArcRefCell::new(InlineItem::OutOfFlowFloatBox(float_box_creator()))); + + debug_assert!( + matches!( + &*inline_level_box.borrow(), + InlineItem::OutOfFlowFloatBox(..), + ), + "Created float box with incompatible `old_layout_box`" + ); + self.inline_items.push(inline_level_box.clone()); self.contains_floats = true; inline_level_box