From 16a47165ee3646227404e7417070896f36e3a63a Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Sun, 27 Feb 2022 10:26:38 +0100 Subject: [PATCH] LibWeb: Use coordinate instead of WeakPtr for box->fragment connection Using WeakPtr to remember which LineBoxFragment owns which Box was imposing some annoying constraints on the layout code. Importantly, it was forcing us to heap-allocate fragments, which makes it much harder to clone a FormattingState. This patch replaces the WeakPtr with a coordinate system instead. Fragments are referred to by their line box index + fragment index within the line box. --- Userland/Libraries/LibWeb/Layout/Box.cpp | 10 ++++++---- Userland/Libraries/LibWeb/Layout/Box.h | 8 ++++++-- Userland/Libraries/LibWeb/Layout/LineBox.cpp | 5 ----- Userland/Libraries/LibWeb/Layout/LineBoxFragment.h | 3 +-- Userland/Libraries/LibWeb/Layout/LineBuilder.cpp | 9 ++++++++- 5 files changed, 21 insertions(+), 14 deletions(-) diff --git a/Userland/Libraries/LibWeb/Layout/Box.cpp b/Userland/Libraries/LibWeb/Layout/Box.cpp index c432427015..ccc1fa14c0 100644 --- a/Userland/Libraries/LibWeb/Layout/Box.cpp +++ b/Userland/Libraries/LibWeb/Layout/Box.cpp @@ -220,8 +220,10 @@ void Box::set_content_size(Gfx::FloatSize const& size) Gfx::FloatPoint Box::effective_offset() const { - if (m_containing_line_box_fragment) - return m_containing_line_box_fragment->offset(); + if (m_containing_line_box_fragment.has_value()) { + auto const& fragment = containing_block()->line_boxes()[m_containing_line_box_fragment->line_box_index].fragments()[m_containing_line_box_fragment->fragment_index]; + return fragment.offset(); + } return m_offset; } @@ -234,9 +236,9 @@ const Gfx::FloatRect Box::absolute_rect() const return rect; } -void Box::set_containing_line_box_fragment(LineBoxFragment& fragment) +void Box::set_containing_line_box_fragment(LineBoxFragmentCoordinate fragment_coordinate) { - m_containing_line_box_fragment = fragment.make_weak_ptr(); + m_containing_line_box_fragment = fragment_coordinate; } StackingContext* Box::enclosing_stacking_context() diff --git a/Userland/Libraries/LibWeb/Layout/Box.h b/Userland/Libraries/LibWeb/Layout/Box.h index ed5733c5b4..a84b35e0f2 100644 --- a/Userland/Libraries/LibWeb/Layout/Box.h +++ b/Userland/Libraries/LibWeb/Layout/Box.h @@ -82,7 +82,11 @@ public: bool is_body() const; - void set_containing_line_box_fragment(LineBoxFragment&); + struct LineBoxFragmentCoordinate { + size_t line_box_index { 0 }; + size_t fragment_index { 0 }; + }; + void set_containing_line_box_fragment(LineBoxFragmentCoordinate); StackingContext* stacking_context() { return m_stacking_context; } const StackingContext* stacking_context() const { return m_stacking_context; } @@ -138,7 +142,7 @@ private: Gfx::FloatSize m_content_size; // Some boxes hang off of line box fragments. (inline-block, inline-table, replaced, etc) - WeakPtr m_containing_line_box_fragment; + Optional m_containing_line_box_fragment; OwnPtr m_stacking_context; diff --git a/Userland/Libraries/LibWeb/Layout/LineBox.cpp b/Userland/Libraries/LibWeb/Layout/LineBox.cpp index 3152a43dd8..3a65f89155 100644 --- a/Userland/Libraries/LibWeb/Layout/LineBox.cpp +++ b/Userland/Libraries/LibWeb/Layout/LineBox.cpp @@ -27,11 +27,6 @@ void LineBox::add_fragment(Node const& layout_node, int start, int length, float m_fragments.append(make(layout_node, start, length, Gfx::FloatPoint(m_width + leading_size, 0.0f), Gfx::FloatSize(content_width, content_height), border_box_top, border_box_bottom, fragment_type)); } m_width += content_width + leading_size + trailing_size; - - if (is(layout_node)) { - // FIXME: Move this to FormattingContext! - const_cast(static_cast(layout_node)).set_containing_line_box_fragment(m_fragments.last()); - } } void LineBox::trim_trailing_whitespace() diff --git a/Userland/Libraries/LibWeb/Layout/LineBoxFragment.h b/Userland/Libraries/LibWeb/Layout/LineBoxFragment.h index 1b042ddc39..bb9977c332 100644 --- a/Userland/Libraries/LibWeb/Layout/LineBoxFragment.h +++ b/Userland/Libraries/LibWeb/Layout/LineBoxFragment.h @@ -6,14 +6,13 @@ #pragma once -#include #include #include #include namespace Web::Layout { -class LineBoxFragment : public Weakable { +class LineBoxFragment { friend class LineBox; public: diff --git a/Userland/Libraries/LibWeb/Layout/LineBuilder.cpp b/Userland/Libraries/LibWeb/Layout/LineBuilder.cpp index f62a7109a5..ce01a1193d 100644 --- a/Userland/Libraries/LibWeb/Layout/LineBuilder.cpp +++ b/Userland/Libraries/LibWeb/Layout/LineBuilder.cpp @@ -52,8 +52,15 @@ LineBox& LineBuilder::ensure_last_line_box() void LineBuilder::append_box(Box const& box, float leading_size, float trailing_size) { auto const& box_state = m_formatting_state.get(box); - ensure_last_line_box().add_fragment(box, 0, 0, leading_size, trailing_size, box_state.content_width, box_state.content_height, box_state.border_box_top(), box_state.border_box_bottom()); + auto& line_box = ensure_last_line_box(); + line_box.add_fragment(box, 0, 0, leading_size, trailing_size, box_state.content_width, box_state.content_height, box_state.border_box_top(), box_state.border_box_bottom()); m_max_height_on_current_line = max(m_max_height_on_current_line, box_state.content_height); + + // FIXME: Move this to FormattingContext! + const_cast(box).set_containing_line_box_fragment({ + .line_box_index = m_containing_block_state.line_boxes.size() - 1, + .fragment_index = line_box.fragments().size() - 1, + }); } void LineBuilder::append_text_chunk(TextNode const& text_node, size_t offset_in_node, size_t length_in_node, float leading_size, float trailing_size, float content_width, float content_height)