From 25a3d0d643dd5bbe667a380050ef145a88c4947b Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Tue, 15 Aug 2023 15:11:10 +0200 Subject: [PATCH] LibWeb: Resolve relative offsets *once* after layout Instead of applying relative offsets (like position:relative insets) during painting and hit testing, we now do a pass at the end of layout and assign the final resolved offsets to paintables. This makes painting and hit testing easier since they don't have to think about relative offsets, and it also fixes a bug where offsets were not applied to text fragments inside inline-flow elements that were themselves position:relative. --- .../relpos-inline-elements.txt | 31 ++++++++ .../relpos-inline-elements.html | 12 ++++ .../Libraries/LibWeb/Layout/LayoutState.cpp | 72 ++++++++++++++++++- .../Libraries/LibWeb/Layout/LayoutState.h | 3 + .../LibWeb/Painting/PaintableBox.cpp | 19 +---- .../Libraries/LibWeb/Painting/PaintableBox.h | 4 -- 6 files changed, 117 insertions(+), 24 deletions(-) create mode 100644 Tests/LibWeb/Layout/expected/block-and-inline/relpos-inline-elements.txt create mode 100644 Tests/LibWeb/Layout/input/block-and-inline/relpos-inline-elements.html diff --git a/Tests/LibWeb/Layout/expected/block-and-inline/relpos-inline-elements.txt b/Tests/LibWeb/Layout/expected/block-and-inline/relpos-inline-elements.txt new file mode 100644 index 0000000000..f775b22ad4 --- /dev/null +++ b/Tests/LibWeb/Layout/expected/block-and-inline/relpos-inline-elements.txt @@ -0,0 +1,31 @@ +Viewport <#document> at (0,0) content-size 800x600 children: not-inline + BlockContainer at (0,0) content-size 800x600 [BFC] children: not-inline + BlockContainer at (8,8) content-size 784x17.46875 children: inline + line 0 width: 98, height: 17.46875, bottom: 17.46875, baseline: 13.53125 + frag 0 from TextNode start: 0, length: 4, rect: [8,8 35.15625x17.46875] + "foo " + frag 1 from TextNode start: 0, length: 3, rect: [43,33 27.640625x17.46875] + "bar" + frag 2 from TextNode start: 0, length: 1, rect: [71,8 8x17.46875] + " " + frag 3 from TextNode start: 0, length: 3, rect: [54,58 27.203125x17.46875] + "baz" + TextNode <#text> + InlineNode + TextNode <#text> + TextNode <#text> + InlineNode + InlineNode + TextNode <#text> + TextNode <#text> + +PaintableWithLines (Viewport<#document>) [0,0 800x600] + PaintableWithLines (BlockContainer) [0,0 800x600] + PaintableWithLines (BlockContainer) [8,8 784x17.46875] overflow: [8,8 784x67.46875] + TextPaintable (TextNode<#text>) + InlinePaintable (InlineNode) + TextPaintable (TextNode<#text>) + TextPaintable (TextNode<#text>) + InlinePaintable (InlineNode) + InlinePaintable (InlineNode) + TextPaintable (TextNode<#text>) diff --git a/Tests/LibWeb/Layout/input/block-and-inline/relpos-inline-elements.html b/Tests/LibWeb/Layout/input/block-and-inline/relpos-inline-elements.html new file mode 100644 index 0000000000..dabe5600e3 --- /dev/null +++ b/Tests/LibWeb/Layout/input/block-and-inline/relpos-inline-elements.html @@ -0,0 +1,12 @@ + +foo bar baz diff --git a/Userland/Libraries/LibWeb/Layout/LayoutState.cpp b/Userland/Libraries/LibWeb/Layout/LayoutState.cpp index 51ddbeed77..956a947633 100644 --- a/Userland/Libraries/LibWeb/Layout/LayoutState.cpp +++ b/Userland/Libraries/LibWeb/Layout/LayoutState.cpp @@ -131,6 +131,71 @@ static CSSPixelRect measure_scrollable_overflow(Box const& box) return scrollable_overflow_rect; } +void LayoutState::resolve_relative_positions(Vector const& paintables_with_lines) +{ + // This function resolves relative position offsets of all the boxes & fragments in the paint tree. + // It runs *after* the paint tree has been constructed, so it modifies paintable node & fragment offsets directly. + + // Regular boxes (not line box fragments): + for (auto& it : used_values_per_layout_node) { + auto& used_values = *it.value; + auto& node = const_cast(used_values.node()); + + if (!node.is_box()) + continue; + + auto& paintable = static_cast(*node.paintable()); + CSSPixelPoint offset; + + if (used_values.containing_line_box_fragment.has_value()) { + // Atomic inline case: + // We know that `node` is an atomic inline because `containing_line_box_fragments` refers to the + // line box fragment in the parent block container that contains it. + auto const& containing_line_box_fragment = used_values.containing_line_box_fragment.value(); + auto const& containing_block = *node.containing_block(); + auto const& containing_block_paintable = verify_cast(*containing_block.paintable_box()); + auto const& fragment = containing_block_paintable.line_boxes()[containing_line_box_fragment.line_box_index].fragments()[containing_line_box_fragment.fragment_index]; + + // The fragment has the final offset for the atomic inline, so we just need to copy it from there. + offset = fragment.offset(); + } else { + // Not an atomic inline, much simpler case. + offset = used_values.offset; + } + // Apply relative position inset if appropriate. + if (node.computed_values().position() == CSS::Position::Relative) { + auto& inset = node.box_model().inset; + offset.translate_by(inset.left, inset.top); + } + paintable.set_offset(offset); + } + + // Line box fragments: + for (auto const& paintable_with_lines : paintables_with_lines) { + for (auto const& line_box : paintable_with_lines.line_boxes()) { + for (auto& fragment : line_box.fragments()) { + auto const& fragment_node = fragment.layout_node(); + if (!is(*fragment_node.parent())) + continue; + // Collect effective relative position offset from inline-flow parent chain. + CSSPixelPoint offset; + for (auto* ancestor = fragment_node.parent(); ancestor; ancestor = ancestor->parent()) { + if (!is(*ancestor)) + break; + if (!ancestor->display().is_inline_outside() || !ancestor->display().is_flow_inside()) + break; + if (ancestor->computed_values().position() == CSS::Position::Relative) { + auto const& ancestor_node = static_cast(*ancestor); + auto const& inset = ancestor_node.box_model().inset; + offset.translate_by(inset.left, inset.top); + } + } + const_cast(fragment).set_offset(fragment.offset().translated(offset)); + } + } + } +} + void LayoutState::commit() { // Only the top-level LayoutState should ever be committed. @@ -158,7 +223,6 @@ void LayoutState::commit() auto& paintable_box = const_cast(*box.paintable_box()); paintable_box.set_offset(used_values.offset); paintable_box.set_content_size(used_values.content_width(), used_values.content_height()); - paintable_box.set_containing_line_box_fragment(used_values.containing_line_box_fragment); if (used_values.override_borders_data().has_value()) { paintable_box.set_override_borders_data(used_values.override_borders_data().value()); } @@ -174,7 +238,11 @@ void LayoutState::commit() } } - // Measure absolute rect of each line box. Also collect all text nodes. + resolve_relative_positions(paintables_with_lines); + + // Make a pass over all the line boxes to: + // - Measure absolute rect of each line box. + // - Collect all text nodes, so we can create paintables for them later. for (auto& paintable_with_lines : paintables_with_lines) { for (auto& line_box : paintable_with_lines.line_boxes()) { CSSPixelRect line_box_absolute_rect; diff --git a/Userland/Libraries/LibWeb/Layout/LayoutState.h b/Userland/Libraries/LibWeb/Layout/LayoutState.h index b599668d27..0715490181 100644 --- a/Userland/Libraries/LibWeb/Layout/LayoutState.h +++ b/Userland/Libraries/LibWeb/Layout/LayoutState.h @@ -172,6 +172,9 @@ struct LayoutState { LayoutState const* m_parent { nullptr }; LayoutState const& m_root; + +private: + void resolve_relative_positions(Vector const&); }; } diff --git a/Userland/Libraries/LibWeb/Painting/PaintableBox.cpp b/Userland/Libraries/LibWeb/Painting/PaintableBox.cpp index e38c70dd3e..232aa498fd 100644 --- a/Userland/Libraries/LibWeb/Painting/PaintableBox.cpp +++ b/Userland/Libraries/LibWeb/Painting/PaintableBox.cpp @@ -117,19 +117,7 @@ void PaintableBox::set_content_size(CSSPixelSize size) CSSPixelPoint PaintableBox::effective_offset() const { - CSSPixelPoint offset; - if (containing_block() && m_containing_line_box_fragment.has_value()) { - auto& paintable_with_lines = *verify_cast(containing_block()->paintable_box()); - auto const& fragment = paintable_with_lines.line_boxes()[m_containing_line_box_fragment->line_box_index].fragments()[m_containing_line_box_fragment->fragment_index]; - offset = fragment.offset(); - } else { - offset = m_offset; - } - if (layout_box().computed_values().position() == CSS::Position::Relative) { - auto const& inset = layout_box().box_model().inset; - offset.translate_by(inset.left, inset.top); - } - return offset; + return m_offset; } CSSPixelRect PaintableBox::compute_absolute_rect() const @@ -176,11 +164,6 @@ CSSPixelRect PaintableBox::absolute_paint_rect() const return *m_absolute_paint_rect; } -void PaintableBox::set_containing_line_box_fragment(Optional fragment_coordinate) -{ - m_containing_line_box_fragment = move(fragment_coordinate); -} - StackingContext* PaintableBox::enclosing_stacking_context() { for (auto* ancestor = layout_box().parent(); ancestor; ancestor = ancestor->parent()) { diff --git a/Userland/Libraries/LibWeb/Painting/PaintableBox.h b/Userland/Libraries/LibWeb/Painting/PaintableBox.h index 3a788a4be9..58ad6ed342 100644 --- a/Userland/Libraries/LibWeb/Painting/PaintableBox.h +++ b/Userland/Libraries/LibWeb/Painting/PaintableBox.h @@ -112,7 +112,6 @@ public: Optional calculate_overflow_clipped_rect() const; void set_overflow_data(OverflowData data) { m_overflow_data = move(data); } - void set_containing_line_box_fragment(Optional); StackingContext* stacking_context() { return m_stacking_context; } StackingContext const* stacking_context() const { return m_stacking_context; } @@ -201,9 +200,6 @@ private: CSSPixelPoint m_offset; CSSPixelSize m_content_size; - // Some boxes hang off of line box fragments. (inline-block, inline-table, replaced, etc) - Optional m_containing_line_box_fragment; - OwnPtr m_stacking_context; Optional mutable m_absolute_rect;