Skip to content

Commit

Permalink
LibWeb: Store final box model metrics in paint tree, not layout tree
Browse files Browse the repository at this point in the history
This was a weird case of layout results being stored in the layout tree
instead of in the paint tree like everything else.
  • Loading branch information
awesomekling committed Feb 17, 2025
1 parent dcfc552 commit fadf0a2
Show file tree
Hide file tree
Showing 8 changed files with 65 additions and 58 deletions.
2 changes: 1 addition & 1 deletion Libraries/LibWeb/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
50 changes: 29 additions & 21 deletions Libraries/LibWeb/Dump.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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;
Expand Down Expand Up @@ -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()) {
Expand Down
32 changes: 19 additions & 13 deletions Libraries/LibWeb/Layout/LayoutState.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
Expand All @@ -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()) {
Expand Down Expand Up @@ -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):
Expand All @@ -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()) {
Expand All @@ -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);
Expand Down
5 changes: 0 additions & 5 deletions Libraries/LibWeb/Layout/Node.h
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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; }

Expand All @@ -287,7 +283,6 @@ class NodeWithStyleAndBoxModelMetrics : public NodeWithStyle {
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;
};

Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
/*
* Copyright (c) 2018-2020, Andreas Kling <[email protected]>
* Copyright (c) 2018-2025, Andreas Kling <[email protected]>
*
* 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
{
Expand Down
Original file line number Diff line number Diff line change
@@ -1,15 +1,14 @@
/*
* Copyright (c) 2018-2020, Andreas Kling <[email protected]>
* Copyright (c) 2018-2025, Andreas Kling <[email protected]>
*
* 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 };
Expand Down
8 changes: 6 additions & 2 deletions Libraries/LibWeb/Painting/PaintableBox.h
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2022, Andreas Kling <[email protected]>
* Copyright (c) 2022-2025, Andreas Kling <[email protected]>
* Copyright (c) 2024, Aliaksandr Kalenik <[email protected]>
*
* SPDX-License-Identifier: BSD-2-Clause
Expand All @@ -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>
Expand Down Expand Up @@ -49,7 +50,8 @@ class PaintableBox : public Paintable
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;
Expand Down Expand Up @@ -306,6 +308,8 @@ class PaintableBox : public Paintable

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 {
Expand Down
15 changes: 5 additions & 10 deletions Services/WebContent/ConnectionFromClient.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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()));
Expand All @@ -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();
Expand Down

0 comments on commit fadf0a2

Please sign in to comment.