From c4bb74f40bbe5a02a0d825cccc33ea710571784f Mon Sep 17 00:00:00 2001 From: Jelle Raaijmakers Date: Thu, 27 Mar 2025 00:16:02 +0000 Subject: [PATCH] LibWeb: Fix and improve float positioning behavior Our recent change to get rid of the "move 1px at a time" algorithm in the float positioning logic introduced the issue that potentially intersecting float boxes were not evaluated in order anymore. This could result in float boxes being pushed down further than strictly necessary. By finding the highest point we can move the floating box to and repeating the process until we're no longer intersecting any floating box, we also solve some edge cases like intersecting with very long floating boxes whose edges lay outside the current box' edges. This is by no means the most efficient solution, but it is more correct than what we had until now. Fixes #4110. --- Libraries/LibWeb/Layout/LineBuilder.cpp | 53 +++++++++++++------ ...loat-initial-available-space-vs-height.txt | 25 +++++++++ ...rtical-clearance-ignores-opposite-side.txt | 22 ++++++++ ...oat-initial-available-space-vs-height.html | 29 ++++++++++ ...tical-clearance-ignores-opposite-side.html | 30 +++++++++++ 5 files changed, 143 insertions(+), 16 deletions(-) create mode 100644 Tests/LibWeb/Layout/expected/block-and-inline/float-initial-available-space-vs-height.txt create mode 100644 Tests/LibWeb/Layout/expected/block-and-inline/float-vertical-clearance-ignores-opposite-side.txt create mode 100644 Tests/LibWeb/Layout/input/block-and-inline/float-initial-available-space-vs-height.html create mode 100644 Tests/LibWeb/Layout/input/block-and-inline/float-vertical-clearance-ignores-opposite-side.html diff --git a/Libraries/LibWeb/Layout/LineBuilder.cpp b/Libraries/LibWeb/Layout/LineBuilder.cpp index 60d53e4d98b..89f16e12bf3 100644 --- a/Libraries/LibWeb/Layout/LineBuilder.cpp +++ b/Libraries/LibWeb/Layout/LineBuilder.cpp @@ -129,23 +129,44 @@ CSSPixels LineBuilder::y_for_float_to_be_inserted_here(Box const& box) // Then, look for the next Y position where we can fit the new float. auto box_in_root_rect = m_context.parent().content_box_rect_in_ancestor_coordinate_space(box_state, m_context.parent().root()); - m_context.parent().for_each_floating_box([&](auto const& float_box) { - auto candidate_block_offset_in_root = box_in_root_rect.y() + candidate_block_offset; - if (float_box.margin_box_rect_in_root_coordinate_space.bottom() < candidate_block_offset_in_root) + + HashMap available_space_cache; + for (;;) { + Optional highest_intersection_bottom; + + auto candidate_block_top_in_root = box_in_root_rect.y() + candidate_block_offset; + auto candidate_block_bottom_in_root = candidate_block_top_in_root + height; + + m_context.parent().for_each_floating_box([&](auto const& float_box) { + auto float_box_top = float_box.margin_box_rect_in_root_coordinate_space.top(); + auto float_box_bottom = float_box.margin_box_rect_in_root_coordinate_space.bottom(); + if (float_box_bottom <= candidate_block_top_in_root) + return IterationDecision::Continue; + + auto intersection_test = [&](auto y_coordinate, auto top, auto bottom) { + if (y_coordinate < top || y_coordinate > bottom) + return; + auto available_space = available_space_cache.ensure(y_coordinate, [&]() { + return m_context.available_space_for_line(y_coordinate); + }); + if (width > available_space) { + auto bottom_relative = float_box_bottom - box_in_root_rect.y(); + highest_intersection_bottom = min(highest_intersection_bottom.value_or(bottom_relative), bottom_relative); + } + }; + + intersection_test(float_box_top, candidate_block_top_in_root, candidate_block_bottom_in_root); + intersection_test(float_box_bottom, candidate_block_top_in_root, candidate_block_bottom_in_root); + intersection_test(candidate_block_top_in_root, float_box_top, float_box_bottom); + intersection_test(candidate_block_bottom_in_root, float_box_top, float_box_bottom); + return IterationDecision::Continue; - auto space_at_y_top = m_context.available_space_for_line(candidate_block_offset); - auto space_at_y_bottom = m_context.available_space_for_line(candidate_block_offset + height); - if (width > space_at_y_top || width > space_at_y_bottom) { - if (!m_context.any_floats_intrude_at_block_offset(candidate_block_offset) && !m_context.any_floats_intrude_at_block_offset(candidate_block_offset + height)) { - return IterationDecision::Break; - } - } else { - return IterationDecision::Break; - } - // candidate_block_offset needs to stay relative to the current box - candidate_block_offset = float_box.margin_box_rect_in_root_coordinate_space.bottom() - box_in_root_rect.y(); - return IterationDecision::Continue; - }); + }); + if (!highest_intersection_bottom.has_value() || highest_intersection_bottom.value() == candidate_block_offset) + break; + candidate_block_offset = highest_intersection_bottom.value(); + } + return candidate_block_offset; } diff --git a/Tests/LibWeb/Layout/expected/block-and-inline/float-initial-available-space-vs-height.txt b/Tests/LibWeb/Layout/expected/block-and-inline/float-initial-available-space-vs-height.txt new file mode 100644 index 00000000000..4f66fd80532 --- /dev/null +++ b/Tests/LibWeb/Layout/expected/block-and-inline/float-initial-available-space-vs-height.txt @@ -0,0 +1,25 @@ +Viewport <#document> at (0,0) content-size 800x600 children: not-inline + BlockContainer at (0,0) content-size 800x413 [BFC] children: not-inline + BlockContainer at (8,8) content-size 784x5 children: not-inline + BlockContainer at (8,8) content-size 100x5 children: inline + frag 0 from TextNode start: 1, length: 1, rect: [8,8 4.328125x5] baseline: 4 + "H" + TextNode <#text> + BlockContainer at (8,13) content-size 100x100 floating [BFC] children: not-inline + TextNode <#text> + BlockContainer at (8,113) content-size 30x300 floating [BFC] children: not-inline + TextNode <#text> + BlockContainer at (78,113) content-size 30x300 floating [BFC] children: not-inline + TextNode <#text> + BlockContainer <(anonymous)> at (8,13) content-size 784x0 children: inline + TextNode <#text> + +ViewportPaintable (Viewport<#document>) [0,0 800x600] + PaintableWithLines (BlockContainer) [0,0 800x413] + PaintableWithLines (BlockContainer) [8,8 784x5] + PaintableWithLines (BlockContainer
.a) [8,8 100x5] + TextPaintable (TextNode<#text>) + PaintableWithLines (BlockContainer
.b.l) [8,13 100x100] + PaintableWithLines (BlockContainer
.c.l) [8,113 30x300] + PaintableWithLines (BlockContainer
.c.r) [78,113 30x300] + PaintableWithLines (BlockContainer(anonymous)) [8,13 784x0] diff --git a/Tests/LibWeb/Layout/expected/block-and-inline/float-vertical-clearance-ignores-opposite-side.txt b/Tests/LibWeb/Layout/expected/block-and-inline/float-vertical-clearance-ignores-opposite-side.txt new file mode 100644 index 00000000000..36b6ae487f2 --- /dev/null +++ b/Tests/LibWeb/Layout/expected/block-and-inline/float-vertical-clearance-ignores-opposite-side.txt @@ -0,0 +1,22 @@ +Viewport <#document> at (0,0) content-size 800x600 children: not-inline + BlockContainer at (0,0) content-size 800x216 [BFC] children: not-inline + BlockContainer at (8,8) content-size 784x200 children: not-inline + BlockContainer at (8,8) content-size 500x200 children: inline + TextNode <#text> + BlockContainer at (8,8) content-size 200x150 floating [BFC] children: not-inline + TextNode <#text> + BlockContainer at (308,8) content-size 200x100 floating [BFC] children: not-inline + TextNode <#text> + BlockContainer at (308,108) content-size 200x100 floating [BFC] children: not-inline + TextNode <#text> + BlockContainer <(anonymous)> at (8,208) content-size 784x0 children: inline + TextNode <#text> + +ViewportPaintable (Viewport<#document>) [0,0 800x600] + PaintableWithLines (BlockContainer) [0,0 800x216] + PaintableWithLines (BlockContainer) [8,8 784x200] + PaintableWithLines (BlockContainer
.a) [8,8 500x200] + PaintableWithLines (BlockContainer
.b) [8,8 200x150] + PaintableWithLines (BlockContainer
.c) [308,8 200x100] + PaintableWithLines (BlockContainer
.d) [308,108 200x100] + PaintableWithLines (BlockContainer(anonymous)) [8,208 784x0] diff --git a/Tests/LibWeb/Layout/input/block-and-inline/float-initial-available-space-vs-height.html b/Tests/LibWeb/Layout/input/block-and-inline/float-initial-available-space-vs-height.html new file mode 100644 index 00000000000..2d3433d52db --- /dev/null +++ b/Tests/LibWeb/Layout/input/block-and-inline/float-initial-available-space-vs-height.html @@ -0,0 +1,29 @@ + + +
+ H +
+
+
+
diff --git a/Tests/LibWeb/Layout/input/block-and-inline/float-vertical-clearance-ignores-opposite-side.html b/Tests/LibWeb/Layout/input/block-and-inline/float-vertical-clearance-ignores-opposite-side.html new file mode 100644 index 00000000000..4760e5c46fe --- /dev/null +++ b/Tests/LibWeb/Layout/input/block-and-inline/float-vertical-clearance-ignores-opposite-side.html @@ -0,0 +1,30 @@ + + +
+
+
+
+