From eb65efa1be70c18c95dd36fe41fc2b7c5529f8cb Mon Sep 17 00:00:00 2001 From: Aliaksandr Kalenik Date: Fri, 14 Aug 2026 11:57:52 +0100 Subject: [PATCH 1/4] LibWeb: Save committed SVG root geometry in an arena side table Partial relayout of an SVG root seeds the pass by reading the previous paintable's committed geometry back through an FFI callback, which is the one place layout still consumes the paint tree. Record the same scalars in a generation-checked arena side table instead, captured during the commit walk straight from the SVG root's fragment link, following the saved-abspos-layout-inputs pattern: a presence flag on NodeData keeps the table and flags consistent, free() wipes the slot, and the tree builder carries the entry to the replacement box when an SVG root is rebuilt in place. Nothing reads the table yet; switching the subtree-relayout seed over comes next. --- Libraries/LibWeb/Rust/src/layout/commit.rs | 34 ++++++++++ .../Rust/src/layout/formatting_context.rs | 4 ++ .../Rust/src/layout/layout_node_arena.rs | 65 +++++++++++++++++++ Libraries/LibWeb/Rust/src/layout/node_data.rs | 6 ++ .../LibWeb/Rust/src/layout/tree_builder.rs | 6 ++ 5 files changed, 115 insertions(+) diff --git a/Libraries/LibWeb/Rust/src/layout/commit.rs b/Libraries/LibWeb/Rust/src/layout/commit.rs index 50d8b7c490e5c..8c0eeb693685b 100644 --- a/Libraries/LibWeb/Rust/src/layout/commit.rs +++ b/Libraries/LibWeb/Rust/src/layout/commit.rs @@ -117,6 +117,40 @@ fn commit_subtree( let entry = scopes.link_for_slot(slot_index); if let Some(link) = entry { callbacks.set_saved_abspos_layout_inputs(node, link.abspos_layout_inputs); + // SVG roots are the only non-abspos partial relayout boundaries; save their committed + // geometry so a later subtree pass can seed itself without reading the old paintable. + if callbacks.node_data(node).kind == NodeKind::SVGSVGBox { + let fragment = &link.fragment; + debug_assert!( + fragment.svg_viewport_size.is_some(), + "committed SVG root fragment carries no viewport size" + ); + callbacks.set_saved_committed_geometry( + node, + FfiPaintableGeometry { + content_inline_size: fragment.content_inline_size, + content_block_size: fragment.content_block_size, + content_offset: link.committed_offset, + svg_viewport_size: fragment.svg_viewport_size.unwrap_or_default(), + margin_left: fragment.margin_left, + margin_right: fragment.margin_right, + margin_top: fragment.margin_top, + margin_bottom: fragment.margin_bottom, + border_left: fragment.border_left, + border_right: fragment.border_right, + border_top: fragment.border_top, + border_bottom: fragment.border_bottom, + padding_left: fragment.padding_left, + padding_right: fragment.padding_right, + padding_top: fragment.padding_top, + padding_bottom: fragment.padding_bottom, + inset_left: link.inset_left, + inset_right: link.inset_right, + inset_top: link.inset_top, + inset_bottom: link.inset_bottom, + }, + ); + } } // SAFETY: The C++ sink owns paintables and copies every plain-data // input synchronously. diff --git a/Libraries/LibWeb/Rust/src/layout/formatting_context.rs b/Libraries/LibWeb/Rust/src/layout/formatting_context.rs index 37c4866a8dda8..7d677150b0f06 100644 --- a/Libraries/LibWeb/Rust/src/layout/formatting_context.rs +++ b/Libraries/LibWeb/Rust/src/layout/formatting_context.rs @@ -1062,6 +1062,10 @@ impl FfiLayoutFcCallbacks { self.arena().saved_abspos_layout_inputs(data) } + pub(crate) fn set_saved_committed_geometry(&self, node: Node, geometry: crate::layout::FfiPaintableGeometry) { + self.arena().set_saved_committed_geometry(self.arena().data(node), geometry); + } + pub(crate) fn set_saved_abspos_layout_inputs(&self, node: Node, inputs: Option) { let data = self.arena().data(node); // Match prepare_node's former as_if() guard. diff --git a/Libraries/LibWeb/Rust/src/layout/layout_node_arena.rs b/Libraries/LibWeb/Rust/src/layout/layout_node_arena.rs index d25324e90213b..c639f7592b7ec 100644 --- a/Libraries/LibWeb/Rust/src/layout/layout_node_arena.rs +++ b/Libraries/LibWeb/Rust/src/layout/layout_node_arena.rs @@ -134,6 +134,12 @@ struct SavedAbsposLayoutInputsSlot { inputs: Option>, } +#[derive(Default)] +struct SavedCommittedGeometrySlot { + generation: u8, + geometry: Option>, +} + #[derive(Default)] pub(crate) struct TextContent { pub(crate) text: Vec, @@ -223,6 +229,7 @@ pub(crate) struct LayoutNodeArena { live_count: u32, intrinsic_size_caches: RefCell>, saved_abspos_layout_inputs: RefCell>, + saved_committed_geometries: RefCell>, text_contents: Vec, text_chunk_caches: RefCell>, replaced_content_facts: Vec, @@ -244,6 +251,7 @@ impl LayoutNodeArena { live_count: 0, intrinsic_size_caches: RefCell::new(Vec::new()), saved_abspos_layout_inputs: RefCell::new(Vec::new()), + saved_committed_geometries: RefCell::new(Vec::new()), text_contents: Vec::new(), text_chunk_caches: RefCell::new(Vec::new()), replaced_content_facts: Vec::new(), @@ -363,6 +371,9 @@ impl LayoutNodeArena { if let Some(slot) = self.saved_abspos_layout_inputs.get_mut().get_mut(index as usize) { *slot = SavedAbsposLayoutInputsSlot::default(); } + if let Some(slot) = self.saved_committed_geometries.get_mut().get_mut(index as usize) { + *slot = SavedCommittedGeometrySlot::default(); + } if let Some(slot) = self.text_contents.get_mut(index as usize) { *slot = TextContentSlot::default(); } @@ -709,6 +720,60 @@ impl LayoutNodeArena { } } + pub(crate) fn saved_committed_geometry( + &self, + data: *const NodeData, + ) -> Option { + let (index, metadata) = self.slot_for_data(data); + let slots = self.saved_committed_geometries.borrow(); + let geometry = slots + .get(index as usize) + .filter(|slot| slot.generation == metadata.generation) + .and_then(|slot| slot.geometry.as_deref().copied()); + + // SAFETY: slot_for_data() established that data points to a live slot + // in this arena. + let flags = unsafe { (&raw const (*data).flags).read() }; + assert_eq!( + flags & NodeFlag::HasSavedCommittedGeometry as u32 != 0, + geometry.is_some(), + "saved committed geometry presence flag disagrees with the arena side table" + ); + geometry + } + + pub(crate) fn set_saved_committed_geometry( + &self, + data: *mut NodeData, + geometry: crate::layout::FfiPaintableGeometry, + ) { + let (index, metadata) = self.slot_for_data(data); + let mut slots = self.saved_committed_geometries.borrow_mut(); + if slots.len() <= index as usize { + slots.resize_with(index as usize + 1, SavedCommittedGeometrySlot::default); + } + let slot = &mut slots[index as usize]; + if slot.generation != metadata.generation { + *slot = SavedCommittedGeometrySlot { + generation: metadata.generation, + geometry: Some(Box::new(geometry)), + }; + } else if let Some(saved_geometry) = &mut slot.geometry { + **saved_geometry = geometry; + } else { + slot.geometry = Some(Box::new(geometry)); + } + drop(slots); + + // SAFETY: slot_for_data() established that data points to a live slot + // in this arena, and layout/tree building serialize mutation on the + // arena's owner thread. + unsafe { + let flags = &raw mut (*data).flags; + flags.write(flags.read() | NodeFlag::HasSavedCommittedGeometry as u32); + } + } + pub(crate) fn set_text_content( &mut self, id: NodeSlotId, diff --git a/Libraries/LibWeb/Rust/src/layout/node_data.rs b/Libraries/LibWeb/Rust/src/layout/node_data.rs index ba3e85f117bd5..57ca21949c610 100644 --- a/Libraries/LibWeb/Rust/src/layout/node_data.rs +++ b/Libraries/LibWeb/Rust/src/layout/node_data.rs @@ -160,6 +160,7 @@ pub enum NodeFlag { ListMarkerIsInside = 1 << 23, HasAnchorNames = 1 << 24, InsetsUseAnchorFunctions = 1 << 25, + HasSavedCommittedGeometry = 1 << 26, } #[repr(C)] @@ -260,6 +261,11 @@ mod tests { assert_eq!(NodeFlag::ListMarkerIsInside as u32, 1 << 23); } + #[test] + fn saved_committed_geometry_flag_uses_a_previously_unassigned_bit() { + assert_eq!(NodeFlag::HasSavedCommittedGeometry as u32, 1 << 26); + } + #[test] fn stamped_fact_flags_use_previously_unassigned_bits() { assert_eq!(NodeFlag::IsHtmlInputElement as u32, 1 << 13); diff --git a/Libraries/LibWeb/Rust/src/layout/tree_builder.rs b/Libraries/LibWeb/Rust/src/layout/tree_builder.rs index 94fa9fdf00413..30518aa56687f 100644 --- a/Libraries/LibWeb/Rust/src/layout/tree_builder.rs +++ b/Libraries/LibWeb/Rust/src/layout/tree_builder.rs @@ -1470,6 +1470,12 @@ fn update_principal_node_after_entry( { arena.set_saved_abspos_layout_inputs(new_data, Some(inputs)); } + // SAFETY: data() returned pointers to live slots. + if unsafe { (*old_data).kind == NodeKind::SVGSVGBox && (*new_data).kind == NodeKind::SVGSVGBox } + && let Some(geometry) = arena.saved_committed_geometry(old_data) + { + arena.set_saved_committed_geometry(new_data, geometry); + } } unsafe { (host.callbacks.place_principal_layout)( From efe7f3162aade414367dfa039afc8a75bc8c6e6a Mon Sep 17 00:00:00 2001 From: Aliaksandr Kalenik Date: Fri, 14 Aug 2026 12:02:19 +0100 Subject: [PATCH 2/4] LibWeb: Seed SVG-root partial relayout from saved committed geometry Switch the subtree-relayout seed for in-flow SVG roots from the read_paintable_geometry callback to the arena side table the previous commit introduced, so the seed no longer reads the old paint tree. The in-flow SVG arm of is_partial_relayout_boundary now also requires the saved-geometry flag, which moves the "has this root ever committed" check from boundary collection time to registration time: an SVG root that never committed refuses registration up front and lets the invalidation reach an outer boundary, instead of aborting the whole partial attempt at collect time, so partial-vs-full pass counts can shift for such roots. The old paintable handed to the pass entry is now used only as the commit splice target. --- Libraries/LibWeb/Layout/Box.cpp | 7 ++++--- Libraries/LibWeb/Layout/Box.h | 1 + .../Rust/src/layout/formatting_context.rs | 6 +++++- .../LibWeb/Rust/src/layout/used_values.rs | 18 +++++------------- 4 files changed, 15 insertions(+), 17 deletions(-) diff --git a/Libraries/LibWeb/Layout/Box.cpp b/Libraries/LibWeb/Layout/Box.cpp index 964a4a2196f82..bad7021e8846f 100644 --- a/Libraries/LibWeb/Layout/Box.cpp +++ b/Libraries/LibWeb/Layout/Box.cpp @@ -41,10 +41,11 @@ bool Box::is_partial_relayout_boundary(RequireExistingPaintable require_existing // An in-flow SVG root's used size is determined solely by its own attributes and outer // context, never by its children, so its size and position from the previous layout can be - // reused. An absolutely positioned SVG root's placement is not frozen, so it must qualify - // through the saved-inputs replay path below instead. + // reused - provided a commit has actually saved them. An absolutely positioned SVG root's + // placement is not frozen, so it must qualify through the saved-inputs replay path below + // instead. if (is_svg_svg_box() && !is_absolutely_positioned()) - return is_outermost_svg_root; + return is_outermost_svg_root && has_saved_committed_geometry(); if (!is_absolutely_positioned()) return false; diff --git a/Libraries/LibWeb/Layout/Box.h b/Libraries/LibWeb/Layout/Box.h index c7f23e7ff2e53..dec540cfe0442 100644 --- a/Libraries/LibWeb/Layout/Box.h +++ b/Libraries/LibWeb/Layout/Box.h @@ -59,6 +59,7 @@ class WEB_API Box : public NodeWithStyle { virtual RefPtr create_paintable() const override; bool has_saved_abspos_layout_inputs() const { return has_flag(RustFFI::NodeFlag::HasSavedAbsposLayoutInputs); } + bool has_saved_committed_geometry() const { return has_flag(RustFFI::NodeFlag::HasSavedCommittedGeometry); } bool saved_abspos_cb_derives_from_own_computed_values() const { return has_flag(RustFFI::NodeFlag::SavedAbsposCbDerivesFromOwnComputedValues); } bool saved_abspos_alignment_derives_from_own_computed_values() const { return has_flag(RustFFI::NodeFlag::SavedAbsposAlignmentDerivesFromOwnComputedValues); } diff --git a/Libraries/LibWeb/Rust/src/layout/formatting_context.rs b/Libraries/LibWeb/Rust/src/layout/formatting_context.rs index 7d677150b0f06..84004ef346b6d 100644 --- a/Libraries/LibWeb/Rust/src/layout/formatting_context.rs +++ b/Libraries/LibWeb/Rust/src/layout/formatting_context.rs @@ -1062,6 +1062,10 @@ impl FfiLayoutFcCallbacks { self.arena().saved_abspos_layout_inputs(data) } + pub(crate) fn saved_committed_geometry(&self, node: Node) -> Option { + self.arena().saved_committed_geometry(self.arena().data(node)) + } + pub(crate) fn set_saved_committed_geometry(&self, node: Node, geometry: crate::layout::FfiPaintableGeometry) { self.arena().set_saved_committed_geometry(self.arena().data(node), geometry); } @@ -2165,7 +2169,7 @@ pub unsafe extern "C" fn rust_layout_compute_subtree_layout( let sink = unsafe { &*sink }; let entry_records = std::rc::Rc::new(RunRecords::new_unrooted(callbacks.arena, root)); - let root_used = used_values_from_paintable(&callbacks, root, paintable_to_replace) + let root_used = used_values_from_saved_committed_geometry(&callbacks, root) .expect("partial relayout root must have committed geometry"); entry_records.register(root, root_used.clone()); let entry_fragments = std::rc::Rc::new(RunFragmentBuilder::new_entry_accumulator(root)); diff --git a/Libraries/LibWeb/Rust/src/layout/used_values.rs b/Libraries/LibWeb/Rust/src/layout/used_values.rs index 4492effac4baf..e960d0ac5479c 100644 --- a/Libraries/LibWeb/Rust/src/layout/used_values.rs +++ b/Libraries/LibWeb/Rust/src/layout/used_values.rs @@ -740,23 +740,15 @@ pub(crate) fn create_used_values( std::rc::Rc::new(used) } -pub(crate) fn used_values_from_paintable( +pub(crate) fn used_values_from_saved_committed_geometry( callbacks: &FfiLayoutFcCallbacks, node: Node, - paintable: *mut c_void, ) -> Option> { - let mut geometry = FfiPaintableGeometry::default(); - let found = - unsafe { - (callbacks.read_paintable_geometry)(callbacks.context, callbacks.shell(node), paintable, &raw mut geometry) - }; - if !found { - return None; - } + let geometry = callbacks.saved_committed_geometry(node)?; // Skip normal node initialization: resolving computed sizes requires // percentage bases, and every resulting geometry field is replaced by - // the previous paintable's committed value immediately. + // the previously committed value immediately. let used = UsedValues::default(); used.set_content_inline_size(geometry.content_inline_size); used.set_content_block_size(geometry.content_block_size); @@ -779,8 +771,8 @@ pub(crate) fn used_values_from_paintable( used.inset_right.set(geometry.inset_right); used.inset_top.set(geometry.inset_top); used.inset_bottom.set(geometry.inset_bottom); - // Materialization is this box's placement: the previous paintable's - // committed geometry is final from the moment it is adopted. + // Materialization is this box's placement: the previously committed + // geometry is final from the moment it is adopted. used.has_content_offset.set(true); used.seal_committed_box_metrics(); From cea6d21b97077b94eae45f84b3c0381adef4da26 Mon Sep 17 00:00:00 2001 From: Aliaksandr Kalenik Date: Fri, 14 Aug 2026 12:03:35 +0100 Subject: [PATCH 3/4] LibWeb: Delete the read_paintable_geometry layout callback Nothing reads it since the partial relayout seed switched to the arena side table; this removes the last geometry read-back from Rust layout into the paint tree. FfiPaintableGeometry stays on as the side table's payload type. --- Libraries/LibWeb/Layout/LayoutRustBridge.cpp | 41 ------------------- .../Rust/src/layout/formatting_context.rs | 2 - 2 files changed, 43 deletions(-) diff --git a/Libraries/LibWeb/Layout/LayoutRustBridge.cpp b/Libraries/LibWeb/Layout/LayoutRustBridge.cpp index 5142e4e168e86..e8eddcbce49fd 100644 --- a/Libraries/LibWeb/Layout/LayoutRustBridge.cpp +++ b/Libraries/LibWeb/Layout/LayoutRustBridge.cpp @@ -1112,47 +1112,6 @@ RustFFI::FfiLayoutFcCallbacks LayoutRustBridge::formatting_context_callbacks() auto const* node_with_style = as_if(*static_cast(node)); VERIFY(node_with_style); return build_svg_element_facts(*node_with_style); }, - .read_paintable_geometry = [](void*, void* node, void* paintable_pointer, RustFFI::FfiPaintableGeometry* out) { - VERIFY(out); - VERIFY(paintable_pointer); - auto const* paintable = static_cast(paintable_pointer); - auto const& box_model = paintable->box_model(); - *out = { - .content_inline_size = paintable->content_width().raw_value(), - .content_block_size = paintable->content_height().raw_value(), - .content_offset = { - .x = paintable->offset().x().raw_value(), - .y = paintable->offset().y().raw_value(), - }, - .svg_viewport_size = {}, - .margin_left = box_model.margin.left.raw_value(), - .margin_right = box_model.margin.right.raw_value(), - .margin_top = box_model.margin.top.raw_value(), - .margin_bottom = box_model.margin.bottom.raw_value(), - .border_left = box_model.border.left.raw_value(), - .border_right = box_model.border.right.raw_value(), - .border_top = box_model.border.top.raw_value(), - .border_bottom = box_model.border.bottom.raw_value(), - .padding_left = box_model.padding.left.raw_value(), - .padding_right = box_model.padding.right.raw_value(), - .padding_top = box_model.padding.top.raw_value(), - .padding_bottom = box_model.padding.bottom.raw_value(), - .inset_left = box_model.inset.left.raw_value(), - .inset_right = box_model.inset.right.raw_value(), - .inset_top = box_model.inset.top.raw_value(), - .inset_bottom = box_model.inset.bottom.raw_value(), - }; - - // NB: We check the node type rather than the paintable type to mirror the rust-side logic. - if (is(*static_cast(node))) { - auto const* svg_svg_paintable = as_if(paintable); - VERIFY(svg_svg_paintable); - out->svg_viewport_size = { - .width = svg_svg_paintable->svg_viewport_size().width().raw_value(), - .height = svg_svg_paintable->svg_viewport_size().height().raw_value(), - }; - } - return true; }, .compute_svg_path = [](void*, void* node, RustFFI::FfiSvgPathRequest request) { auto const* node_with_style = as_if(*static_cast(node)); VERIFY(node_with_style); diff --git a/Libraries/LibWeb/Rust/src/layout/formatting_context.rs b/Libraries/LibWeb/Rust/src/layout/formatting_context.rs index 84004ef346b6d..09fc93718788f 100644 --- a/Libraries/LibWeb/Rust/src/layout/formatting_context.rs +++ b/Libraries/LibWeb/Rust/src/layout/formatting_context.rs @@ -965,8 +965,6 @@ pub struct FfiLayoutFcCallbacks { pub document_in_quirks_mode: bool, pub report_unexpected_fragmented_inline: unsafe extern "C" fn(*mut c_void, *mut c_void), pub build_svg_facts: unsafe extern "C" fn(*mut c_void, *mut c_void) -> FfiSvgElementFacts, - pub read_paintable_geometry: - unsafe extern "C" fn(*mut c_void, *mut c_void, *mut c_void, *mut crate::layout::FfiPaintableGeometry) -> bool, pub compute_svg_path: unsafe extern "C" fn(*mut c_void, *mut c_void, FfiSvgPathRequest) -> FfiSvgPathResult, pub svg_image_bounding_box: unsafe extern "C" fn(*mut c_void, *mut c_void, CssPixels, CssPixels) -> FfiFloatRect, pub anchor_lookup: unsafe extern "C" fn(*mut c_void, *mut c_void, usize, *const *mut c_void, usize) -> NodeSlotId, From 72f599d6bbed020a42f697fb6ed7e5050bad992e Mon Sep 17 00:00:00 2001 From: Aliaksandr Kalenik Date: Fri, 14 Aug 2026 12:11:34 +0100 Subject: [PATCH 4/4] LibWeb: Resolve the relayout splice target inside the commit sink The paintable a partial relayout replaces was resolved during boundary collection and threaded through every layer down to begin_commit, which could already resolve it from the commit root on its own. Do that: begin_commit takes the root's paintable and falls back to the one the DOM node still references (and the paint tree keeps alive) when the tree builder rebuilt the root's box, and the paintable parameter disappears from relayout_subtree, the bridge entry points, the Rust pass entries, and drain_and_commit_entry_pass. The splice-target existence requirement folds into is_partial_relayout_boundary as the same paintable-or-DOM-referenced check, replacing the RequireExistingPaintable knob: the rebuilt-box caller that passed No qualifies through the DOM-referenced arm, and registration-time callers now also accept a box whose old paintable only survives through its DOM node, which collection previously had to re-discover. --- Libraries/LibWeb/DOM/Document.cpp | 43 ++++++------------- Libraries/LibWeb/Layout/Box.cpp | 10 +++-- Libraries/LibWeb/Layout/Box.h | 7 +-- Libraries/LibWeb/Layout/LayoutRustBridge.cpp | 18 ++++---- Libraries/LibWeb/Layout/LayoutRustBridge.h | 4 +- Libraries/LibWeb/Rust/src/layout/commit.rs | 11 +++-- .../Rust/src/layout/formatting_context.rs | 10 +---- 7 files changed, 40 insertions(+), 63 deletions(-) diff --git a/Libraries/LibWeb/DOM/Document.cpp b/Libraries/LibWeb/DOM/Document.cpp index 78d0da49fa421..451af006bc675 100644 --- a/Libraries/LibWeb/DOM/Document.cpp +++ b/Libraries/LibWeb/DOM/Document.cpp @@ -1764,18 +1764,19 @@ void Document::end_style_stabilization_epoch() m_animations_created_in_stabilization_epoch.clear(); } -static void relayout_subtree(Layout::Box& subtree_root, Painting::Paintable& old_paintable) +static void relayout_subtree(Layout::Box& subtree_root) { Layout::LayoutRustBridge bridge; // Absolutely positioned boundaries re-resolve their own size and position by replaying - // their layout from saved inputs; SVG root boundaries keep the frozen geometry from the - // previous layout. Rust reads the old paintable before replacing it in either path. + // their layout from saved inputs; SVG root boundaries keep the frozen geometry saved at + // the previous commit. The commit sink resolves the paintable to splice out in either + // path. if (subtree_root.is_absolutely_positioned()) { VERIFY(subtree_root.containing_block()); VERIFY(subtree_root.has_saved_abspos_layout_inputs()); - bridge.replay_saved_abspos_layout(subtree_root, old_paintable); + bridge.replay_saved_abspos_layout(subtree_root); } else { - bridge.compute_subtree_layout(subtree_root, old_paintable); + bridge.compute_subtree_layout(subtree_root); } subtree_root.for_each_in_inclusive_subtree([](auto& node) { @@ -2042,35 +2043,19 @@ Document::PartialRelayoutResult Document::try_partial_relayout(HashTable old_paintable; - }; - Vector partial_relayout_roots; + Vector partial_relayout_roots; HashTable collected_boundaries; auto collect_boundary = [&](Layout::Box& box, bool box_was_replaced) { if (collected_boundaries.set(&box) != AK::HashSetResult::InsertedNewEntry) return true; - RefPtr old_paintable = box.paintable_box(); - if (!old_paintable && box_was_replaced && box.dom_node()) { - // A replaced box has no paintable yet; the previous one stays referenced by the - // DOM node until the next commit replaces it there. - old_paintable = box.dom_node()->unsafe_paintable(); - } - if (!old_paintable) - return false; - // A replaced box applies the saved-inputs validity check unconditionally: the change // that drove the replacement cannot be classified anymore. bool saved_inputs_may_be_style_stale = box.needs_own_geometry_update() || box_was_replaced; if (saved_inputs_may_be_style_stale && box.is_absolutely_positioned() && !Layout::can_replay_saved_abspos_layout_inputs_after_style_change(box)) return false; - partial_relayout_roots.append({ - .box = &box, - .old_paintable = old_paintable, - }); + partial_relayout_roots.append(&box); return true; }; @@ -2091,7 +2076,7 @@ Document::PartialRelayoutResult Document::try_partial_relayout(HashTable(*rebuilt_root); rebuilt_box && rebuilt_box->is_partial_relayout_boundary(Layout::RequireExistingPaintable::No)) + if (auto* rebuilt_box = as_if(*rebuilt_root); rebuilt_box && rebuilt_box->is_partial_relayout_boundary()) containing_boundary = rebuilt_box; for (auto* ancestor = rebuilt_root->parent(); !containing_boundary && ancestor; ancestor = ancestor->parent()) { if (auto* ancestor_box = as_if(*ancestor); ancestor_box && ancestor_box->is_partial_relayout_boundary()) @@ -2102,8 +2087,8 @@ Document::PartialRelayoutResult Document::try_partial_relayout(HashTableparent(); ancestor; ancestor = ancestor->parent()) { + partial_relayout_roots.remove_all_matching([&](auto* root) { + for (auto* ancestor = root->parent(); ancestor; ancestor = ancestor->parent()) { if (auto* ancestor_box = as_if(*ancestor); ancestor_box && collected_boundaries.contains(ancestor_box)) return true; } @@ -2114,11 +2099,11 @@ Document::PartialRelayoutResult Document::try_partial_relayout(HashTableunsafe_paintable())) + return false; + // A nested never qualifies: its subtree is laid out in the outer SVG's // viewBox-transformed coordinate system, which a relayout rooted at the inner cannot // reproduce. @@ -51,8 +57,6 @@ bool Box::is_partial_relayout_boundary(RequireExistingPaintable require_existing return false; if (is_anonymous()) return false; - if (require_existing_paintable == RequireExistingPaintable::Yes && !paintable_box()) - return false; if (dom_node() == document().document_element()) return false; if (!has_saved_abspos_layout_inputs()) diff --git a/Libraries/LibWeb/Layout/Box.h b/Libraries/LibWeb/Layout/Box.h index dec540cfe0442..bc114441f7e64 100644 --- a/Libraries/LibWeb/Layout/Box.h +++ b/Libraries/LibWeb/Layout/Box.h @@ -14,11 +14,6 @@ namespace Web::Layout { -enum class RequireExistingPaintable : u8 { - No, - Yes, -}; - struct LineBoxFragmentCoordinate { size_t line_box_index { 0 }; size_t fragment_index { 0 }; @@ -34,7 +29,7 @@ class WEB_API Box : public NodeWithStyle { // A partial relayout boundary is a box whose subtree can be re-laid out in // isolation: its own used size and position are guaranteed not to change // when layout is invalidated somewhere inside its subtree. - bool is_partial_relayout_boundary(RequireExistingPaintable = RequireExistingPaintable::Yes) const; + bool is_partial_relayout_boundary() const; // https://www.w3.org/TR/css-images-3/#natural-dimensions virtual CSS::SizeWithAspectRatio natural_size() const { return {}; } diff --git a/Libraries/LibWeb/Layout/LayoutRustBridge.cpp b/Libraries/LibWeb/Layout/LayoutRustBridge.cpp index e8eddcbce49fd..91a20830a157f 100644 --- a/Libraries/LibWeb/Layout/LayoutRustBridge.cpp +++ b/Libraries/LibWeb/Layout/LayoutRustBridge.cpp @@ -684,7 +684,7 @@ void LayoutRustBridge::run_root_layout(Box& viewport, CSSPixels viewport_inline_ VERIFY(!m_line_commit_context); } -void LayoutRustBridge::compute_subtree_layout(Box& root, Painting::Paintable& paintable_to_replace) +void LayoutRustBridge::compute_subtree_layout(Box& root) { VERIFY(!m_commit_root); m_commit_root = &root; @@ -702,7 +702,6 @@ void LayoutRustBridge::compute_subtree_layout(Box& root, Painting::Paintable& pa RustFFI::rust_layout_compute_subtree_layout( Node::slot_id(&root), Node::slot_id(&root.root()), - &paintable_to_replace, viewport_rect.width().raw_value(), viewport_rect.height().raw_value(), &callbacks, @@ -711,7 +710,7 @@ void LayoutRustBridge::compute_subtree_layout(Box& root, Painting::Paintable& pa VERIFY(!m_line_commit_context); } -void LayoutRustBridge::replay_saved_abspos_layout(Box& box, Painting::Paintable& paintable_to_replace) +void LayoutRustBridge::replay_saved_abspos_layout(Box& box) { VERIFY(!m_commit_root); m_commit_root = &box; @@ -725,7 +724,7 @@ void LayoutRustBridge::replay_saved_abspos_layout(Box& box, Painting::Paintable& auto sink = commit_sink(); { ActiveLayoutPassScope active_pass; - RustFFI::rust_layout_replay_saved_abspos_layout(Node::slot_id(&box), &paintable_to_replace, &callbacks, &sink); + RustFFI::rust_layout_replay_saved_abspos_layout(Node::slot_id(&box), &callbacks, &sink); } VERIFY(!m_line_commit_context); } @@ -755,17 +754,20 @@ RustFFI::FfiCommitSink LayoutRustBridge::commit_sink() { return { .context = this, - .begin_commit = [](void* context, void* root_pointer, void* paintable_to_replace_pointer) { + .begin_commit = [](void* context, void* root_pointer) { auto& bridge = *static_cast(context); auto& root = *static_cast(root_pointer); VERIFY(!bridge.m_replaced_paintable); VERIFY(!bridge.m_commit_parent_paintable); VERIFY(!bridge.m_commit_insert_before_paintable); - if (paintable_to_replace_pointer) { - bridge.m_replaced_paintable = *static_cast(paintable_to_replace_pointer); - } else if (!root.is_viewport()) { + if (!root.is_viewport()) { bridge.m_replaced_paintable = root.paintable(); + if (!bridge.m_replaced_paintable && root.dom_node()) { + // A rebuilt box has no paintable yet; the previous one stays referenced by + // the DOM node (and alive in the paint tree) until this commit replaces it. + bridge.m_replaced_paintable = root.dom_node()->unsafe_paintable(); + } } if (bridge.m_replaced_paintable) { diff --git a/Libraries/LibWeb/Layout/LayoutRustBridge.h b/Libraries/LibWeb/Layout/LayoutRustBridge.h index 6beadc3e99686..9070bfd034266 100644 --- a/Libraries/LibWeb/Layout/LayoutRustBridge.h +++ b/Libraries/LibWeb/Layout/LayoutRustBridge.h @@ -33,8 +33,8 @@ class LayoutRustBridge { ~LayoutRustBridge(); void run_root_layout(Box& viewport, CSSPixels viewport_inline_size, CSSPixels viewport_block_size, bool should_collect_devtools_layout_data); - void compute_subtree_layout(Box&, Painting::Paintable& paintable_to_replace); - void replay_saved_abspos_layout(Box&, Painting::Paintable& paintable_to_replace); + void compute_subtree_layout(Box&); + void replay_saved_abspos_layout(Box&); private: [[nodiscard]] RustFFI::FfiLayoutFcCallbacks formatting_context_callbacks(); diff --git a/Libraries/LibWeb/Rust/src/layout/commit.rs b/Libraries/LibWeb/Rust/src/layout/commit.rs index 8c0eeb693685b..2e92e846a578f 100644 --- a/Libraries/LibWeb/Rust/src/layout/commit.rs +++ b/Libraries/LibWeb/Rust/src/layout/commit.rs @@ -82,7 +82,7 @@ pub struct FfiPaintableGeometry { #[repr(C)] pub struct FfiCommitSink { pub context: *mut c_void, - pub begin_commit: unsafe extern "C" fn(*mut c_void, *mut c_void, *mut c_void) -> FfiCommitPosition, + pub begin_commit: unsafe extern "C" fn(*mut c_void, *mut c_void) -> FfiCommitPosition, pub finish_commit: unsafe extern "C" fn(*mut c_void), pub prepare_node: unsafe extern "C" fn(*mut c_void, *mut c_void, bool) -> *mut c_void, pub set_box_metrics: unsafe extern "C" fn(*mut c_void, *mut c_void, FfiCommittedBoxMetrics), @@ -297,16 +297,15 @@ fn commit_subtree( pub(crate) fn commit_replacing( root: Node, - paintable_to_replace: *mut c_void, callbacks: &FfiLayoutFcCallbacks, sink: &FfiCommitSink, pass_fragments: &crate::layout::CompletedPassFragments, ) { let mut scopes = crate::layout::CommitScopes::for_pass(pass_fragments); - // SAFETY: The sink retains the replaced paintable, detaches it, and - // returns borrowed insertion pointers that stay live until - // finish_commit(). - let position = unsafe { (sink.begin_commit)(sink.context, callbacks.shell(root), paintable_to_replace) }; + // SAFETY: The sink resolves and retains the paintable to replace from the + // root, detaches it, and returns borrowed insertion pointers that stay + // live until finish_commit(). + let position = unsafe { (sink.begin_commit)(sink.context, callbacks.shell(root)) }; commit_subtree( root, position.parent_paintable, diff --git a/Libraries/LibWeb/Rust/src/layout/formatting_context.rs b/Libraries/LibWeb/Rust/src/layout/formatting_context.rs index 09fc93718788f..99f4aeb32d51d 100644 --- a/Libraries/LibWeb/Rust/src/layout/formatting_context.rs +++ b/Libraries/LibWeb/Rust/src/layout/formatting_context.rs @@ -2116,7 +2116,6 @@ pub unsafe extern "C" fn rust_layout_run_root_layout( &callbacks, should_collect_devtools_layout_data, root, - std::ptr::null_mut(), sink, ); callbacks.arena().sweep_stale_fc_run_cache_entries(); @@ -2129,7 +2128,6 @@ fn drain_and_commit_entry_pass( callbacks: &FfiLayoutFcCallbacks, should_collect_devtools_layout_data: bool, commit_root: Node, - paintable_to_replace: *mut c_void, sink: &FfiCommitSink, ) { drain_abspos_with_placed_containing_blocks( @@ -2140,7 +2138,7 @@ fn drain_and_commit_entry_pass( ); let pass_fragments = entry_fragments.take_completed_pass(entry_records, callbacks); debug_assert!(!pass_fragments.roots.is_empty(), "an entry pass always produces the entry root's fragment"); - commit_replacing(commit_root, paintable_to_replace, callbacks, sink, &pass_fragments); + commit_replacing(commit_root, callbacks, sink, &pass_fragments); } /// # Safety @@ -2150,7 +2148,6 @@ fn drain_and_commit_entry_pass( pub unsafe extern "C" fn rust_layout_compute_subtree_layout( root: NodeSlotId, viewport: NodeSlotId, - paintable_to_replace: *mut c_void, viewport_inline_size_raw: i32, viewport_block_size_raw: i32, callbacks: *const FfiLayoutFcCallbacks, @@ -2158,7 +2155,6 @@ pub unsafe extern "C" fn rust_layout_compute_subtree_layout( ) { abort_on_panic(|| { assert!(!root.is_invalid()); - assert!(!paintable_to_replace.is_null()); assert!(!callbacks.is_null()); assert!(!sink.is_null()); // SAFETY: The C++ pass host keeps both callback tables live for this @@ -2238,7 +2234,6 @@ pub unsafe extern "C" fn rust_layout_compute_subtree_layout( &callbacks, false, root, - paintable_to_replace, sink, ); callbacks.arena().sweep_stale_fc_run_cache_entries(); @@ -2251,13 +2246,11 @@ pub unsafe extern "C" fn rust_layout_compute_subtree_layout( #[unsafe(no_mangle)] pub unsafe extern "C" fn rust_layout_replay_saved_abspos_layout( box_: NodeSlotId, - paintable_to_replace: *mut c_void, callbacks: *const FfiLayoutFcCallbacks, sink: *const FfiCommitSink, ) { abort_on_panic(|| { assert!(!box_.is_invalid()); - assert!(!paintable_to_replace.is_null()); assert!(!callbacks.is_null()); assert!(!sink.is_null()); // SAFETY: The C++ pass host keeps both callback tables live for this @@ -2285,7 +2278,6 @@ pub unsafe extern "C" fn rust_layout_replay_saved_abspos_layout( &callbacks, false, box_, - paintable_to_replace, sink, ); callbacks.arena().sweep_stale_fc_run_cache_entries();