LibWeb: Store final box model metrics in paint tree, not layout tree

This was a weird case of layout results being stored in the layout tree instead of in the paint tree like everything else.
Author: https://github.com/awesomekling Commit: https://github.com/LadybirdBrowser/ladybird/commit/fb020a3c8f6 Pull-request: https://github.com/LadybirdBrowser/ladybird/pull/3603 Reviewed-by: https://github.com/gmta ✅
2025-04-22 04:25:13 +00:00 · 2025-02-17 13:54:36 +01:00 · 2025-02-17 13:54:36 +01:00 · fb020a3c8f · 2025-02-17 17:29:27 +00:00
commit fb020a3c8f
parent 71cb04d8cb
8 changed files with 65 additions and 58 deletions
--- a/Libraries/LibWeb/CMakeLists.txt
+++ b/Libraries/LibWeb/CMakeLists.txt
@ -554,7 +554,6 @@ set(SOURCES
    Layout/BlockContainer.cpp
    Layout/BlockFormattingContext.cpp
    Layout/Box.cpp
-    Layout/BoxModelMetrics.cpp
    Layout/BreakNode.cpp
    Layout/CanvasBox.cpp
    Layout/CheckBox.cpp
@ -634,6 +633,7 @@ set(SOURCES
    Painting/BorderPainting.cpp
    Painting/BorderRadiusCornerClipper.cpp
    Painting/BordersData.cpp
+    Painting/BoxModelMetrics.cpp
    Painting/CanvasPaintable.cpp
    Painting/Command.cpp
    Painting/CheckBoxPaintable.cpp
--- a/Libraries/LibWeb/Dump.cpp
+++ b/Libraries/LibWeb/Dump.cpp
@ -201,6 +201,32 @@ void dump_tree(StringBuilder& builder, Layout::Node const& layout_node, bool sho
        color_off = "\033[0m"sv;
    }

+    auto dump_box_model = [&] {
+        if (show_box_model && layout_node.first_paintable() && layout_node.first_paintable()->is_paintable_box()) {
+            auto const& paintable_box = static_cast<Painting::PaintableBox const&>(*layout_node.first_paintable());
+            auto const& box_model = paintable_box.box_model();
+            // Dump the horizontal box properties
+            builder.appendff(" [{}+{}+{} {} {}+{}+{}]",
+                box_model.margin.left,
+                box_model.border.left,
+                box_model.padding.left,
+                paintable_box.content_width(),
+                box_model.padding.right,
+                box_model.border.right,
+                box_model.margin.right);
+
+            // And the vertical box properties
+            builder.appendff(" [{}+{}+{} {} {}+{}+{}]",
+                box_model.margin.top,
+                box_model.border.top,
+                box_model.padding.top,
+                paintable_box.content_height(),
+                box_model.padding.bottom,
+                box_model.border.bottom,
+                box_model.margin.bottom);
+        }
+    };
+
    if (!is<Layout::Box>(layout_node)) {
        builder.appendff("{}{}{} <{}{}{}{}>",
            nonbox_color_on,
@ -210,6 +236,8 @@ void dump_tree(StringBuilder& builder, Layout::Node const& layout_node, bool sho
            nonbox_color_on,
            identifier,
            color_off);
+
+        dump_box_model();
    } else {
        auto& box = as<Layout::Box>(layout_node);
        StringView color_on = is<Layout::SVGBox>(box) ? svg_box_color_on : box_color_on;
@ -276,27 +304,7 @@ void dump_tree(StringBuilder& builder, Layout::Node const& layout_node, bool sho
        if (box.display().is_table_cell())
            builder.appendff(" {}table-cell{}", table_color_on, color_off);

-        if (show_box_model) {
-            // Dump the horizontal box properties
-            builder.appendff(" [{}+{}+{} {} {}+{}+{}]",
-                box.box_model().margin.left,
-                box.box_model().border.left,
-                box.box_model().padding.left,
-                box.paintable_box() ? box.paintable_box()->content_width() : 0,
-                box.box_model().padding.right,
-                box.box_model().border.right,
-                box.box_model().margin.right);
-
-            // And the vertical box properties
-            builder.appendff(" [{}+{}+{} {} {}+{}+{}]",
-                box.box_model().margin.top,
-                box.box_model().border.top,
-                box.box_model().padding.top,
-                box.paintable_box() ? box.paintable_box()->content_height() : 0,
-                box.box_model().padding.bottom,
-                box.box_model().border.bottom,
-                box.box_model().margin.bottom);
-        }
+        dump_box_model();

        if (auto formatting_context_type = Layout::FormattingContext::formatting_context_type_created_by_box(box); formatting_context_type.has_value()) {
            switch (formatting_context_type.value()) {
--- a/Libraries/LibWeb/Layout/LayoutState.cpp
+++ b/Libraries/LibWeb/Layout/LayoutState.cpp
@ -141,7 +141,7 @@ static CSSPixelRect measure_scrollable_overflow(Box const& box)
    //   to enable a scroll position that satisfies the requirements of place-content: end alignment.
    auto has_scrollable_overflow = !paintable_box.absolute_padding_box_rect().contains(scrollable_overflow_rect);
    if (has_scrollable_overflow) {
-        scrollable_overflow_rect.set_height(max(scrollable_overflow_rect.height(), content_overflow_rect.height() + box.box_model().padding.bottom));
+        scrollable_overflow_rect.set_height(max(scrollable_overflow_rect.height(), content_overflow_rect.height() + paintable_box.box_model().padding.bottom));
    }

    paintable_box.set_overflow_data(Painting::PaintableBox::OverflowData {
@ -177,7 +177,8 @@ void LayoutState::resolve_relative_positions()
                    if (!ancestor->display().is_inline_outside() || !ancestor->display().is_flow_inside())
                        break;
                    if (ancestor->computed_values().position() == CSS::Positioning::Relative) {
-                        auto const& ancestor_node = static_cast<Layout::NodeWithStyleAndBoxModelMetrics const&>(*ancestor);
+                        VERIFY(ancestor->first_paintable());
+                        auto const& ancestor_node = as<Painting::PaintableBox>(*ancestor->first_paintable());
                        auto const& inset = ancestor_node.box_model().inset;
                        offset.translate_by(inset.left, inset.top);
                    }
@ -235,12 +236,21 @@ void LayoutState::commit(Box& root)
    HashTable<Layout::TextNode*> text_nodes;
    HashTable<Painting::PaintableWithLines*> inline_node_paintables;

+    auto transfer_box_model_metrics = [&](Painting::BoxModelMetrics& box_model, UsedValues const& used_values) {
+        box_model.inset = { used_values.inset_top, used_values.inset_right, used_values.inset_bottom, used_values.inset_left };
+        box_model.padding = { used_values.padding_top, used_values.padding_right, used_values.padding_bottom, used_values.padding_left };
+        box_model.border = { used_values.border_top, used_values.border_right, used_values.border_bottom, used_values.border_left };
+        box_model.margin = { used_values.margin_top, used_values.margin_right, used_values.margin_bottom, used_values.margin_left };
+    };
+
    auto try_to_relocate_fragment_in_inline_node = [&](auto& fragment, size_t line_index) -> bool {
        for (auto const* parent = fragment.layout_node().parent(); parent; parent = parent->parent()) {
            if (is<InlineNode>(*parent)) {
                auto& inline_node = const_cast<InlineNode&>(static_cast<InlineNode const&>(*parent));
                auto line_paintable = inline_node.create_paintable_for_line_with_index(line_index);
                line_paintable->add_fragment(fragment);
+                if (auto const* used_values = used_values_per_layout_node.get(inline_node).value_or(nullptr))
+                    transfer_box_model_metrics(line_paintable->box_model(), *used_values);
                if (!inline_node_paintables.contains(line_paintable.ptr())) {
                    inline_node_paintables.set(line_paintable.ptr());
                    inline_node.add_paintable(line_paintable);
@ -255,21 +265,15 @@ void LayoutState::commit(Box& root)
        auto& used_values = *it.value;
        auto& node = const_cast<NodeWithStyle&>(used_values.node());

-        if (is<NodeWithStyleAndBoxModelMetrics>(node)) {
-            // Transfer box model metrics.
-            auto& box_model = static_cast<NodeWithStyleAndBoxModelMetrics&>(node).box_model();
-            box_model.inset = { used_values.inset_top, used_values.inset_right, used_values.inset_bottom, used_values.inset_left };
-            box_model.padding = { used_values.padding_top, used_values.padding_right, used_values.padding_bottom, used_values.padding_left };
-            box_model.border = { used_values.border_top, used_values.border_right, used_values.border_bottom, used_values.border_left };
-            box_model.margin = { used_values.margin_top, used_values.margin_right, used_values.margin_bottom, used_values.margin_left };
-        }
-
        auto paintable = node.create_paintable();
        node.add_paintable(paintable);

        // For boxes, transfer all the state needed for painting.
        if (paintable && is<Painting::PaintableBox>(*paintable)) {
            auto& paintable_box = static_cast<Painting::PaintableBox&>(*paintable);
+
+            transfer_box_model_metrics(paintable_box.box_model(), used_values);
+
            paintable_box.set_offset(used_values.offset);
            paintable_box.set_content_size(used_values.content_width(), used_values.content_height());
            if (used_values.override_borders_data().has_value()) {
@ -319,6 +323,8 @@ void LayoutState::commit(Box& root)
        auto line_paintable = inline_node->create_paintable_for_line_with_index(0);
        inline_node->add_paintable(line_paintable);
        inline_node_paintables.set(line_paintable.ptr());
+        if (auto const* used_values = used_values_per_layout_node.get(*inline_node).value_or(nullptr))
+            transfer_box_model_metrics(line_paintable->box_model(), *used_values);
    }

    // Resolve relative positions for regular boxes (not line box fragments):
@ -331,7 +337,7 @@ void LayoutState::commit(Box& root)
        if (!node.is_box())
            continue;

-        auto& paintable = static_cast<Painting::PaintableBox&>(*node.first_paintable());
+        auto& paintable = as<Painting::PaintableBox>(*node.first_paintable());
        CSSPixelPoint offset;

        if (used_values.containing_line_box_fragment.has_value()) {
@ -351,7 +357,7 @@ void LayoutState::commit(Box& root)
        }
        // Apply relative position inset if appropriate.
        if (node.computed_values().position() == CSS::Positioning::Relative && is<NodeWithStyleAndBoxModelMetrics>(node)) {
-            auto const& inset = static_cast<NodeWithStyleAndBoxModelMetrics const&>(node).box_model().inset;
+            auto const& inset = paintable.box_model().inset;
            offset.translate_by(inset.left, inset.top);
        }
        paintable.set_offset(offset);
--- a/Libraries/LibWeb/Layout/Node.h
+++ b/Libraries/LibWeb/Layout/Node.h
@ -15,7 +15,6 @@
 #include <LibWeb/CSS/StyleValues/ImageStyleValue.h>
 #include <LibWeb/DOM/Document.h>
 #include <LibWeb/Forward.h>
-#include <LibWeb/Layout/BoxModelMetrics.h>
 #include <LibWeb/Painting/PaintContext.h>
 #include <LibWeb/Painting/Paintable.h>
 #include <LibWeb/TreeNode.h>
@ -263,9 +262,6 @@ class NodeWithStyleAndBoxModelMetrics : public NodeWithStyle {
    GC_CELL(NodeWithStyleAndBoxModelMetrics, NodeWithStyle);

 public:
-    BoxModelMetrics& box_model() { return m_box_model; }
-    BoxModelMetrics const& box_model() const { return m_box_model; }
-
    GC::Ptr<NodeWithStyleAndBoxModelMetrics> continuation_of_node() const { return m_continuation_of_node; }
    void set_continuation_of_node(Badge<TreeBuilder>, GC::Ptr<NodeWithStyleAndBoxModelMetrics> node) { m_continuation_of_node = node; }

@ -287,7 +283,6 @@ protected:
 private:
    virtual bool is_node_with_style_and_box_model_metrics() const final { return true; }

-    BoxModelMetrics m_box_model;
    GC::Ptr<NodeWithStyleAndBoxModelMetrics> m_continuation_of_node;
 };

--- a/Libraries/LibWeb/Painting/BoxModelMetrics.cpp
+++ b/Libraries/LibWeb/Painting/BoxModelMetrics.cpp
@ -1,12 +1,12 @@
 /*
- * Copyright (c) 2018-2020, Andreas Kling <andreas@ladybird.org>
+ * Copyright (c) 2018-2025, Andreas Kling <andreas@ladybird.org>
 *
 * SPDX-License-Identifier: BSD-2-Clause
 */

-#include <LibWeb/Layout/BoxModelMetrics.h>
+#include <LibWeb/Painting/BoxModelMetrics.h>

-namespace Web::Layout {
+namespace Web::Painting {

 PixelBox BoxModelMetrics::margin_box() const
 {
--- a/Libraries/LibWeb/Painting/BoxModelMetrics.h
+++ b/Libraries/LibWeb/Painting/BoxModelMetrics.h
@ -1,15 +1,14 @@
 /*
- * Copyright (c) 2018-2020, Andreas Kling <andreas@ladybird.org>
+ * Copyright (c) 2018-2025, Andreas Kling <andreas@ladybird.org>
 *
 * SPDX-License-Identifier: BSD-2-Clause
 */

 #pragma once

-#include <LibGfx/Size.h>
 #include <LibWeb/PixelUnits.h>

-namespace Web::Layout {
+namespace Web::Painting {

 struct PixelBox {
    CSSPixels top { 0 };
--- a/Libraries/LibWeb/Painting/PaintableBox.h
+++ b/Libraries/LibWeb/Painting/PaintableBox.h
@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2022, Andreas Kling <andreas@ladybird.org>
+ * Copyright (c) 2022-2025, Andreas Kling <andreas@ladybird.org>
 * Copyright (c) 2024, Aliaksandr Kalenik <kalenik.aliaksandr@gmail.com>
 *
 * SPDX-License-Identifier: BSD-2-Clause
@ -13,6 +13,7 @@
 #include <LibWeb/Painting/BackgroundPainting.h>
 #include <LibWeb/Painting/BorderPainting.h>
 #include <LibWeb/Painting/BorderRadiusCornerClipper.h>
+#include <LibWeb/Painting/BoxModelMetrics.h>
 #include <LibWeb/Painting/ClipFrame.h>
 #include <LibWeb/Painting/ClippableAndScrollable.h>
 #include <LibWeb/Painting/Paintable.h>
@ -49,7 +50,8 @@ public:
    Layout::NodeWithStyleAndBoxModelMetrics& layout_node_with_style_and_box_metrics() { return static_cast<Layout::NodeWithStyleAndBoxModelMetrics&>(Paintable::layout_node()); }
    Layout::NodeWithStyleAndBoxModelMetrics const& layout_node_with_style_and_box_metrics() const { return static_cast<Layout::NodeWithStyleAndBoxModelMetrics const&>(Paintable::layout_node()); }

-    auto const& box_model() const { return layout_node_with_style_and_box_metrics().box_model(); }
+    auto& box_model() { return m_box_model; }
+    auto const& box_model() const { return m_box_model; }

    struct OverflowData {
        CSSPixelRect scrollable_overflow_rect;
@ -306,6 +308,8 @@ private:

    RefPtr<CSS::GridTrackSizeListStyleValue> m_used_values_for_grid_template_columns;
    RefPtr<CSS::GridTrackSizeListStyleValue> m_used_values_for_grid_template_rows;
+
+    BoxModelMetrics m_box_model;
 };

 class PaintableWithLines : public PaintableBox {
--- a/Services/WebContent/ConnectionFromClient.cpp
+++ b/Services/WebContent/ConnectionFromClient.cpp
@ -503,11 +503,11 @@ void ConnectionFromClient::inspect_dom_node(u64 page_id, Web::UniqueNodeID const
            return builder.to_byte_string();
        };
        auto serialize_node_box_sizing_json = [](Web::Layout::Node const* layout_node) -> ByteString {
-            if (!layout_node || !layout_node->is_box()) {
+            if (!layout_node || !layout_node->is_box() || !layout_node->first_paintable() || !layout_node->first_paintable()->is_paintable_box()) {
                return "{}";
            }
-            auto* box = static_cast<Web::Layout::Box const*>(layout_node);
-            auto box_model = box->box_model();
+            auto const& paintable_box = as<Web::Painting::PaintableBox>(*layout_node->first_paintable());
+            auto const& box_model = paintable_box.box_model();
            StringBuilder builder;
            auto serializer = MUST(JsonObjectSerializer<>::try_create(builder));
            MUST(serializer.add("padding_top"sv, box_model.padding.top.to_double()));
@ -522,13 +522,8 @@ void ConnectionFromClient::inspect_dom_node(u64 page_id, Web::UniqueNodeID const
            MUST(serializer.add("border_right"sv, box_model.border.right.to_double()));
            MUST(serializer.add("border_bottom"sv, box_model.border.bottom.to_double()));
            MUST(serializer.add("border_left"sv, box_model.border.left.to_double()));
-            if (auto* paintable_box = box->paintable_box()) {
-                MUST(serializer.add("content_width"sv, paintable_box->content_width().to_double()));
-                MUST(serializer.add("content_height"sv, paintable_box->content_height().to_double()));
-            } else {
-                MUST(serializer.add("content_width"sv, 0));
-                MUST(serializer.add("content_height"sv, 0));
-            }
+            MUST(serializer.add("content_width"sv, paintable_box.content_width().to_double()));
+            MUST(serializer.add("content_height"sv, paintable_box.content_height().to_double()));

            MUST(serializer.finish());
            return builder.to_byte_string();