From d8ff71fbb543653cc033ec7d611a917500015c3b Mon Sep 17 00:00:00 2001 From: Aliaksandr Kalenik Date: Fri, 14 Mar 2025 01:44:02 +0100 Subject: [PATCH] LibWeb: Delete parent state pointer in LayoutState It's safe to remove this pointer because intrinsic layout should never look up a box's state beyond its containing block. This change affects the expectations of two layout tests, but both already differ slightly from other browsers, and the difference between expectations is less than 5px. --- Libraries/LibWeb/Layout/FormattingContext.cpp | 13 +++++----- Libraries/LibWeb/Layout/LayoutState.cpp | 22 ----------------- Libraries/LibWeb/Layout/LayoutState.h | 12 ++-------- .../Layout/expected/grid/min-max-content.txt | 16 ++++++------- ...alidation-propagation-to-table-wrapper.txt | 24 +++++++++---------- 5 files changed, 29 insertions(+), 58 deletions(-) diff --git a/Libraries/LibWeb/Layout/FormattingContext.cpp b/Libraries/LibWeb/Layout/FormattingContext.cpp index 9e7df9fdbf7..6db7fc4b99c 100644 --- a/Libraries/LibWeb/Layout/FormattingContext.cpp +++ b/Libraries/LibWeb/Layout/FormattingContext.cpp @@ -437,7 +437,7 @@ CSSPixels FormattingContext::compute_table_box_width_inside_table_wrapper(Box co }); VERIFY(table_box.has_value()); - LayoutState throwaway_state(&m_state); + LayoutState throwaway_state; auto& table_box_state = throwaway_state.get_mutable(*table_box); auto const& table_box_computed_values = table_box->computed_values(); @@ -476,7 +476,8 @@ CSSPixels FormattingContext::compute_table_box_height_inside_table_wrapper(Box c // table-wrapper can't have borders or paddings but it might have margin taken from table-root. auto available_height = height_of_containing_block - margin_top.to_px(box) - margin_bottom.to_px(box); - LayoutState throwaway_state(&m_state); + LayoutState throwaway_state; + auto context = create_independent_formatting_context_if_needed(throwaway_state, LayoutMode::IntrinsicSizing, box); VERIFY(context); context->run(m_state.get(box).available_inner_space_or_constraints_from(available_space)); @@ -1454,7 +1455,7 @@ CSSPixels FormattingContext::calculate_min_content_width(Layout::Box const& box) if (cache.has_value()) return cache.value(); - LayoutState throwaway_state(&m_state); + LayoutState throwaway_state; auto& box_state = throwaway_state.get_mutable(box); box_state.width_constraint = SizeConstraint::MinContent; @@ -1486,7 +1487,7 @@ CSSPixels FormattingContext::calculate_max_content_width(Layout::Box const& box) if (cache.has_value()) return cache.value(); - LayoutState throwaway_state(&m_state); + LayoutState throwaway_state; auto& box_state = throwaway_state.get_mutable(box); box_state.width_constraint = SizeConstraint::MaxContent; @@ -1523,7 +1524,7 @@ CSSPixels FormattingContext::calculate_min_content_height(Layout::Box const& box if (cache.has_value()) return cache.value(); - LayoutState throwaway_state(&m_state); + LayoutState throwaway_state; auto& box_state = throwaway_state.get_mutable(box); box_state.height_constraint = SizeConstraint::MinContent; @@ -1554,7 +1555,7 @@ CSSPixels FormattingContext::calculate_max_content_height(Layout::Box const& box if (cache_slot.has_value()) return cache_slot.value(); - LayoutState throwaway_state(&m_state); + LayoutState throwaway_state; auto& box_state = throwaway_state.get_mutable(box); box_state.height_constraint = SizeConstraint::MaxContent; diff --git a/Libraries/LibWeb/Layout/LayoutState.cpp b/Libraries/LibWeb/Layout/LayoutState.cpp index c0c88f58b6d..c73c431004e 100644 --- a/Libraries/LibWeb/Layout/LayoutState.cpp +++ b/Libraries/LibWeb/Layout/LayoutState.cpp @@ -18,11 +18,6 @@ namespace Web::Layout { -LayoutState::LayoutState(LayoutState const* parent) - : m_parent(parent) -{ -} - LayoutState::~LayoutState() { } @@ -32,15 +27,6 @@ LayoutState::UsedValues& LayoutState::get_mutable(NodeWithStyle const& node) if (auto* used_values = used_values_per_layout_node.get(node).value_or(nullptr)) return *used_values; - for (auto const* ancestor = m_parent; ancestor; ancestor = ancestor->m_parent) { - if (auto* ancestor_used_values = ancestor->used_values_per_layout_node.get(node).value_or(nullptr)) { - auto cow_used_values = adopt_own(*new UsedValues(*ancestor_used_values)); - auto* cow_used_values_ptr = cow_used_values.ptr(); - used_values_per_layout_node.set(node, move(cow_used_values)); - return *cow_used_values_ptr; - } - } - auto const* containing_block_used_values = node.is_viewport() ? nullptr : &get(*node.containing_block()); auto new_used_values = adopt_own(*new UsedValues); @@ -55,11 +41,6 @@ LayoutState::UsedValues const& LayoutState::get(NodeWithStyle const& node) const if (auto const* used_values = used_values_per_layout_node.get(node).value_or(nullptr)) return *used_values; - for (auto const* ancestor = m_parent; ancestor; ancestor = ancestor->m_parent) { - if (auto const* ancestor_used_values = ancestor->used_values_per_layout_node.get(node).value_or(nullptr)) - return *ancestor_used_values; - } - auto const* containing_block_used_values = node.is_viewport() ? nullptr : &get(*node.containing_block()); auto new_used_values = adopt_own(*new UsedValues); @@ -205,9 +186,6 @@ static void build_paint_tree(Node& node, Painting::Paintable* parent_paintable = void LayoutState::commit(Box& root) { - // Only the top-level LayoutState should ever be committed. - VERIFY(!m_parent); - // NOTE: In case this is a relayout of an existing tree, we start by detaching the old paint tree // from the layout tree. This is done to ensure that we don't end up with any old-tree pointers // when text paintables shift around in the tree. diff --git a/Libraries/LibWeb/Layout/LayoutState.h b/Libraries/LibWeb/Layout/LayoutState.h index e39ab1b2b05..c6574939c49 100644 --- a/Libraries/LibWeb/Layout/LayoutState.h +++ b/Libraries/LibWeb/Layout/LayoutState.h @@ -55,11 +55,6 @@ struct StaticPositionRect { }; struct LayoutState { - LayoutState() = default; - - explicit LayoutState(LayoutState const* parent); - ~LayoutState(); - struct UsedValues { NodeWithStyle const& node() const { return *m_node; } NodeWithStyle& node() { return const_cast(*m_node); } @@ -200,19 +195,16 @@ struct LayoutState { Optional m_static_position_rect; }; + ~LayoutState(); + // Commits the used values produced by layout and builds a paintable tree. void commit(Box& root); - // NOTE: get_mutable() will CoW the UsedValues if it's inherited from an ancestor state; UsedValues& get_mutable(NodeWithStyle const&); - - // NOTE: get() will not CoW the UsedValues. UsedValues const& get(NodeWithStyle const&) const; HashMap, NonnullOwnPtr> used_values_per_layout_node; - LayoutState const* m_parent { nullptr }; - private: void resolve_relative_positions(); }; diff --git a/Tests/LibWeb/Layout/expected/grid/min-max-content.txt b/Tests/LibWeb/Layout/expected/grid/min-max-content.txt index 7df8b3d1d30..72b7f386875 100644 --- a/Tests/LibWeb/Layout/expected/grid/min-max-content.txt +++ b/Tests/LibWeb/Layout/expected/grid/min-max-content.txt @@ -4,20 +4,20 @@ Viewport <#document> at (0,0) content-size 800x600 children: not-inline Box at (8,8) content-size 784x17 [GFC] children: not-inline BlockContainer <(anonymous)> (not painted) [BFC] children: inline TextNode <#text> - BlockContainer at (8,8) content-size 98x17 [BFC] children: inline + BlockContainer at (8,8) content-size 100x17 [BFC] children: inline frag 0 from TextNode start: 0, length: 11, rect: [8,8 93.765625x17] baseline: 13.296875 "min-content" TextNode <#text> BlockContainer <(anonymous)> (not painted) [BFC] children: inline TextNode <#text> - BlockContainer at (204,8) content-size 98.640625x17 [BFC] children: inline - frag 0 from TextNode start: 0, length: 11, rect: [204,8 98.640625x17] baseline: 13.296875 + BlockContainer at (208,8) content-size 98.640625x17 [BFC] children: inline + frag 0 from TextNode start: 0, length: 11, rect: [208,8 98.640625x17] baseline: 13.296875 "max-content" TextNode <#text> BlockContainer <(anonymous)> (not painted) [BFC] children: inline TextNode <#text> - BlockContainer at (302.640625,8) content-size 489.359375x17 [BFC] children: inline - frag 0 from TextNode start: 0, length: 3, rect: [302.640625,8 21.609375x17] baseline: 13.296875 + BlockContainer at (306.640625,8) content-size 485.359375x17 [BFC] children: inline + frag 0 from TextNode start: 0, length: 3, rect: [306.640625,8 21.609375x17] baseline: 13.296875 "1fr" TextNode <#text> BlockContainer <(anonymous)> (not painted) [BFC] children: inline @@ -27,9 +27,9 @@ ViewportPaintable (Viewport<#document>) [0,0 800x600] PaintableWithLines (BlockContainer) [0,0 800x600] PaintableWithLines (BlockContainer) [8,8 784x17] PaintableBox (Box
.grid-container) [8,8 784x17] - PaintableWithLines (BlockContainer
.grid-item) [8,8 98x17] + PaintableWithLines (BlockContainer
.grid-item) [8,8 100x17] TextPaintable (TextNode<#text>) - PaintableWithLines (BlockContainer
.grid-item) [204,8 98.640625x17] + PaintableWithLines (BlockContainer
.grid-item) [208,8 98.640625x17] TextPaintable (TextNode<#text>) - PaintableWithLines (BlockContainer
.grid-item) [302.640625,8 489.359375x17] + PaintableWithLines (BlockContainer
.grid-item) [306.640625,8 485.359375x17] TextPaintable (TextNode<#text>) diff --git a/Tests/LibWeb/Layout/expected/table/style-invalidation-propagation-to-table-wrapper.txt b/Tests/LibWeb/Layout/expected/table/style-invalidation-propagation-to-table-wrapper.txt index 5c06e601f74..f7af1b80e80 100644 --- a/Tests/LibWeb/Layout/expected/table/style-invalidation-propagation-to-table-wrapper.txt +++ b/Tests/LibWeb/Layout/expected/table/style-invalidation-propagation-to-table-wrapper.txt @@ -2,21 +2,21 @@ Viewport <#document> at (0,0) content-size 800x600 children: not-inline BlockContainer at (0,0) content-size 800x120 [BFC] children: not-inline BlockContainer at (8,8) content-size 784x104 children: not-inline BlockContainer
at (8,8) content-size 784x104 children: not-inline - TableWrapper <(anonymous)> at (204,8) content-size 392x104 [BFC] children: not-inline - Box at (204,8) content-size 392x104 table-box [TFC] children: not-inline - Box at (206,10) content-size 388x100 table-row-group children: not-inline - Box at (206,10) content-size 388x100 table-row children: not-inline - BlockContainer
at (207,60) content-size 386x0 table-cell [BFC] children: not-inline - BlockContainer <(anonymous)> at (207,60) content-size 386x0 children: inline + TableWrapper <(anonymous)> at (200,8) content-size 392x104 [BFC] children: not-inline + Box at (200,8) content-size 392x104 table-box [TFC] children: not-inline + Box at (202,10) content-size 388x100 table-row-group children: not-inline + Box at (202,10) content-size 388x100 table-row children: not-inline + BlockContainer
at (203,60) content-size 386x0 table-cell [BFC] children: not-inline + BlockContainer <(anonymous)> at (203,60) content-size 386x0 children: inline TextNode <#text> ViewportPaintable (Viewport<#document>) [0,0 800x600] PaintableWithLines (BlockContainer) [0,0 800x120] PaintableWithLines (BlockContainer) [8,8 784x104] PaintableWithLines (BlockContainer
) [8,8 784x104] - PaintableWithLines (TableWrapper(anonymous)) [204,8 392x104] - PaintableBox (Box) [204,8 392x104] - PaintableBox (Box) [206,10 388x100] - PaintableBox (Box) [206,10 388x100] - PaintableWithLines (BlockContainer
) [206,10 388x100] - PaintableWithLines (BlockContainer(anonymous)) [207,60 386x0] + PaintableWithLines (TableWrapper(anonymous)) [200,8 392x104] + PaintableBox (Box) [200,8 392x104] + PaintableBox (Box) [202,10 388x100] + PaintableBox (Box) [202,10 388x100] + PaintableWithLines (BlockContainer
) [202,10 388x100] + PaintableWithLines (BlockContainer(anonymous)) [203,60 386x0]