From 92266d2247aae2b528307bab40fd0bbcbe491765 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Mon, 21 Feb 2022 01:43:37 +0100 Subject: [PATCH] LibWeb: Create list-item markers during layout tree construction Previously, these were added during layout. This didn't fit into the new world where layout doesn't mutate the tree incrementally, so this patch adds logic to Layout::TreeBuilder for adding a marker to each list-item box after its children have been constructed. --- Userland/Libraries/LibWeb/Forward.h | 2 + .../LibWeb/Layout/BlockFormattingContext.cpp | 39 +++++++++++++++++-- .../LibWeb/Layout/BlockFormattingContext.h | 4 +- .../Libraries/LibWeb/Layout/ListItemBox.cpp | 27 ++----------- .../Libraries/LibWeb/Layout/ListItemBox.h | 9 ++--- .../LibWeb/Layout/ListItemMarkerBox.cpp | 18 +-------- .../LibWeb/Layout/ListItemMarkerBox.h | 6 ++- .../Libraries/LibWeb/Layout/TreeBuilder.cpp | 14 ++++++- 8 files changed, 65 insertions(+), 54 deletions(-) diff --git a/Userland/Libraries/LibWeb/Forward.h b/Userland/Libraries/LibWeb/Forward.h index 6f8628b7098..99a6dc760ea 100644 --- a/Userland/Libraries/LibWeb/Forward.h +++ b/Userland/Libraries/LibWeb/Forward.h @@ -291,6 +291,8 @@ class Label; class LabelableNode; class LineBox; class LineBoxFragment; +class ListItemBox; +class ListItemMarkerBox; class Node; class NodeWithStyle; class NodeWithStyleAndBoxModelMetrics; diff --git a/Userland/Libraries/LibWeb/Layout/BlockFormattingContext.cpp b/Userland/Libraries/LibWeb/Layout/BlockFormattingContext.cpp index 1105aaec007..a1040b4057f 100644 --- a/Userland/Libraries/LibWeb/Layout/BlockFormattingContext.cpp +++ b/Userland/Libraries/LibWeb/Layout/BlockFormattingContext.cpp @@ -405,10 +405,9 @@ void BlockFormattingContext::layout_block_level_children(BlockContainer const& b if (is(child_box) || is(child_box)) place_block_level_element_in_normal_flow_horizontally(child_box, block_container); - // FIXME: This should be factored differently. It's uncool that we mutate the tree *during* layout! - // Instead, we should generate the marker box during the tree build. - if (is(child_box)) - verify_cast(child_box).layout_marker(); + if (is(child_box)) { + layout_list_item_marker(static_cast(child_box)); + } content_height = max(content_height, box_state.offset.y() + box_state.content_height + box_state.margin_box_bottom()); content_width = max(content_width, box_state.content_width); @@ -663,4 +662,36 @@ void BlockFormattingContext::layout_floating_child(Box const& box, BlockContaine float_box(FloatSide::Right, m_right_floats); } } + +void BlockFormattingContext::layout_list_item_marker(ListItemBox const& list_item_box) +{ + if (!list_item_box.marker()) + return; + + auto& marker = *list_item_box.marker(); + auto& marker_state = m_state.ensure(marker); + auto& list_item_state = m_state.ensure(list_item_box); + + int image_width = 0; + int image_height = 0; + if (auto const* list_style_image = marker.list_style_image_bitmap()) { + image_width = list_style_image->rect().width(); + image_height = list_style_image->rect().height(); + } + + if (marker.text().is_empty()) { + marker_state.content_width = image_width + 4; + } else { + auto text_width = marker.font().width(marker.text()); + marker_state.content_width = image_width + text_width; + } + + marker_state.content_height = max(image_height, marker.line_height()); + + marker_state.offset = { -(marker_state.content_width + 4), 0 }; + + if (marker_state.content_height > list_item_state.content_height) + list_item_state.content_height = marker_state.content_height; +} + } diff --git a/Userland/Libraries/LibWeb/Layout/BlockFormattingContext.h b/Userland/Libraries/LibWeb/Layout/BlockFormattingContext.h index ef8371969b6..1b8f38f8ba0 100644 --- a/Userland/Libraries/LibWeb/Layout/BlockFormattingContext.h +++ b/Userland/Libraries/LibWeb/Layout/BlockFormattingContext.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2020, Andreas Kling + * Copyright (c) 2020-2022, Andreas Kling * * SPDX-License-Identifier: BSD-2-Clause */ @@ -58,6 +58,8 @@ private: void apply_transformations_to_children(Box const&); + void layout_list_item_marker(ListItemBox const&); + enum class FloatSide { Left, Right, diff --git a/Userland/Libraries/LibWeb/Layout/ListItemBox.cpp b/Userland/Libraries/LibWeb/Layout/ListItemBox.cpp index 590873a943f..a830b46c1f7 100644 --- a/Userland/Libraries/LibWeb/Layout/ListItemBox.cpp +++ b/Userland/Libraries/LibWeb/Layout/ListItemBox.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2018-2020, Andreas Kling + * Copyright (c) 2018-2022, Andreas Kling * Copyright (c) 2021, Tobias Christiansen * * SPDX-License-Identifier: BSD-2-Clause @@ -19,30 +19,9 @@ ListItemBox::~ListItemBox() { } -void ListItemBox::layout_marker() +void ListItemBox::set_marker(RefPtr marker) { - if (m_marker) { - remove_child(*m_marker); - m_marker = nullptr; - } - - if (computed_values().list_style_type() == CSS::ListStyleType::None) - return; - - if (!m_marker) { - auto* marker_style = dom_node().specified_css_values(); - VERIFY(marker_style); - int child_index = parent()->index_of_child(*this).value(); - m_marker = adopt_ref(*new ListItemMarkerBox(document(), computed_values().list_style_type(), child_index + 1, *marker_style)); - if (first_child()) - m_marker->set_inline(first_child()->is_inline()); - append_child(*m_marker); - } - - m_marker->set_offset(-(m_marker->content_width() + 4), 0); - - if (m_marker->content_height() > content_height()) - set_content_height(m_marker->content_height()); + m_marker = move(marker); } } diff --git a/Userland/Libraries/LibWeb/Layout/ListItemBox.h b/Userland/Libraries/LibWeb/Layout/ListItemBox.h index 4181b25ec7b..eb08aea4fb2 100644 --- a/Userland/Libraries/LibWeb/Layout/ListItemBox.h +++ b/Userland/Libraries/LibWeb/Layout/ListItemBox.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2018-2020, Andreas Kling + * Copyright (c) 2018-2022, Andreas Kling * * SPDX-License-Identifier: BSD-2-Clause */ @@ -11,18 +11,17 @@ namespace Web::Layout { -class ListItemMarkerBox; - class ListItemBox final : public BlockContainer { public: ListItemBox(DOM::Document&, DOM::Element&, NonnullRefPtr); virtual ~ListItemBox() override; - void layout_marker(); - DOM::Element& dom_node() { return static_cast(*BlockContainer::dom_node()); } DOM::Element const& dom_node() const { return static_cast(*BlockContainer::dom_node()); } + ListItemMarkerBox const* marker() const { return m_marker; } + void set_marker(RefPtr); + private: RefPtr m_marker; }; diff --git a/Userland/Libraries/LibWeb/Layout/ListItemMarkerBox.cpp b/Userland/Libraries/LibWeb/Layout/ListItemMarkerBox.cpp index e9f4bc87209..9525e90f4ae 100644 --- a/Userland/Libraries/LibWeb/Layout/ListItemMarkerBox.cpp +++ b/Userland/Libraries/LibWeb/Layout/ListItemMarkerBox.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2018-2020, Andreas Kling + * Copyright (c) 2018-2022, Andreas Kling * Copyright (c) 2021, Tobias Christiansen * * SPDX-License-Identifier: BSD-2-Clause @@ -48,22 +48,6 @@ ListItemMarkerBox::ListItemMarkerBox(DOM::Document& document, CSS::ListStyleType default: VERIFY_NOT_REACHED(); } - - int image_width = 0; - int image_height = 0; - if (auto const* list_style_image = list_style_image_bitmap()) { - image_width = list_style_image->rect().width(); - image_height = list_style_image->rect().height(); - } - - if (m_text.is_null()) { - set_content_width(image_width + 4); - } else { - auto text_width = font().width(m_text); - set_content_width(image_width + text_width); - } - - set_content_height(max(image_height, line_height())); } ListItemMarkerBox::~ListItemMarkerBox() diff --git a/Userland/Libraries/LibWeb/Layout/ListItemMarkerBox.h b/Userland/Libraries/LibWeb/Layout/ListItemMarkerBox.h index 85e79ef869b..0e2b3d4a2bc 100644 --- a/Userland/Libraries/LibWeb/Layout/ListItemMarkerBox.h +++ b/Userland/Libraries/LibWeb/Layout/ListItemMarkerBox.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2018-2020, Andreas Kling + * Copyright (c) 2018-2022, Andreas Kling * Copyright (c) 2021, Tobias Christiansen * * SPDX-License-Identifier: BSD-2-Clause @@ -18,9 +18,11 @@ public: virtual void paint(PaintContext&, PaintPhase) override; + Gfx::Bitmap const* list_style_image_bitmap() const; + String const& text() const { return m_text; } + private: virtual bool can_have_children() const override { return false; } - Gfx::Bitmap const* list_style_image_bitmap() const; CSS::ListStyleType m_list_style_type { CSS::ListStyleType::None }; size_t m_index; diff --git a/Userland/Libraries/LibWeb/Layout/TreeBuilder.cpp b/Userland/Libraries/LibWeb/Layout/TreeBuilder.cpp index d5ccbc8ecec..aff5847572c 100644 --- a/Userland/Libraries/LibWeb/Layout/TreeBuilder.cpp +++ b/Userland/Libraries/LibWeb/Layout/TreeBuilder.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2018-2021, Andreas Kling + * Copyright (c) 2018-2022, Andreas Kling * * SPDX-License-Identifier: BSD-2-Clause */ @@ -12,6 +12,8 @@ #include #include #include +#include +#include #include #include #include @@ -147,6 +149,16 @@ void TreeBuilder::create_layout_tree(DOM::Node& dom_node, TreeBuilder::Context& }); pop_parent(); } + + if (is(*layout_node)) { + int child_index = layout_node->parent()->index_of_child(*layout_node).value(); + auto marker_style = static_cast(dom_node).specified_css_values(); + auto list_item_marker = adopt_ref(*new ListItemMarkerBox(document, layout_node->computed_values().list_style_type(), child_index + 1, *marker_style)); + if (layout_node->first_child()) + list_item_marker->set_inline(layout_node->first_child()->is_inline()); + static_cast(*layout_node).set_marker(list_item_marker); + layout_node->append_child(move(list_item_marker)); + } } RefPtr TreeBuilder::build(DOM::Node& dom_node)