From d37c0a2cabdd6d605a78d24f15829ed4008da1a8 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Tue, 26 Mar 2024 20:17:07 +0100 Subject: [PATCH] LibWeb: Don't make flex layout responsible for flex container cross size Although the flex algorithm as specified does say to determine the cross size of the flex container, this is not how our layout engine works. The parent formatting context is responsible for sizing its children, and since that's already happening, we can simply remove the cross sizing step from FFC. --- .../atomic-inline-with-aspect-ratio-2.txt | 13 ++++++ ...-intrinsic-aspect-ratio-and-max-height.txt | 6 +-- .../inline-flex-with-aspect-ratio.txt | 16 +++---- .../atomic-inline-with-aspect-ratio-2.html | 10 +++++ .../LibWeb/Layout/FlexFormattingContext.cpp | 43 +------------------ .../LibWeb/Layout/FlexFormattingContext.h | 2 - 6 files changed, 36 insertions(+), 54 deletions(-) create mode 100644 Tests/LibWeb/Layout/expected/block-and-inline/atomic-inline-with-aspect-ratio-2.txt create mode 100644 Tests/LibWeb/Layout/input/block-and-inline/atomic-inline-with-aspect-ratio-2.html diff --git a/Tests/LibWeb/Layout/expected/block-and-inline/atomic-inline-with-aspect-ratio-2.txt b/Tests/LibWeb/Layout/expected/block-and-inline/atomic-inline-with-aspect-ratio-2.txt new file mode 100644 index 00000000000..aa9842c244b --- /dev/null +++ b/Tests/LibWeb/Layout/expected/block-and-inline/atomic-inline-with-aspect-ratio-2.txt @@ -0,0 +1,13 @@ +Viewport <#document> at (0,0) content-size 800x600 children: not-inline + BlockContainer at (0,0) content-size 800x116 [BFC] children: not-inline + Box at (8,8) content-size 100x100 flex-container(row) [FFC] children: not-inline + BlockContainer <(anonymous)> at (8,8) content-size 9.46875x100 flex-item [BFC] children: inline + frag 0 from TextNode start: 0, length: 1, rect: [8,8 9.46875x17] baseline: 13.296875 + "b" + TextNode <#text> + +ViewportPaintable (Viewport<#document>) [0,0 800x600] + PaintableWithLines (BlockContainer) [0,0 800x116] + PaintableBox (Box) [8,8 100x100] + PaintableWithLines (BlockContainer(anonymous)) [8,8 9.46875x100] + TextPaintable (TextNode<#text>) diff --git a/Tests/LibWeb/Layout/expected/flex/flex-item-with-intrinsic-aspect-ratio-and-max-height.txt b/Tests/LibWeb/Layout/expected/flex/flex-item-with-intrinsic-aspect-ratio-and-max-height.txt index b2e710b739e..e4a3e136faa 100644 --- a/Tests/LibWeb/Layout/expected/flex/flex-item-with-intrinsic-aspect-ratio-and-max-height.txt +++ b/Tests/LibWeb/Layout/expected/flex/flex-item-with-intrinsic-aspect-ratio-and-max-height.txt @@ -1,11 +1,11 @@ Viewport <#document> at (0,0) content-size 800x600 children: not-inline BlockContainer at (1,1) content-size 798x69.984375 [BFC] children: not-inline Box at (10,10) content-size 780x51.984375 flex-container(row) [FFC] children: not-inline - ImageBox at (11,11) content-size 66.65625x50 flex-item children: not-inline + ImageBox at (11,10) content-size 66.65625x50 flex-item children: not-inline BlockContainer <(anonymous)> (not painted) [BFC] children: inline TextNode <#text> ViewportPaintable (Viewport<#document>) [0,0 800x600] PaintableWithLines (BlockContainer) [0,0 800x71.984375] - PaintableBox (Box) [9,9 782x53.984375] overflow: [10,10 780x52] - ImagePaintable (ImageBox) [10,10 68.65625x52] + PaintableBox (Box) [9,9 782x53.984375] overflow: [10,9 780x52.984375] + ImagePaintable (ImageBox) [10,9 68.65625x52] diff --git a/Tests/LibWeb/Layout/expected/inline-flex-with-aspect-ratio.txt b/Tests/LibWeb/Layout/expected/inline-flex-with-aspect-ratio.txt index bc0a0fb316f..5c401ed1dda 100644 --- a/Tests/LibWeb/Layout/expected/inline-flex-with-aspect-ratio.txt +++ b/Tests/LibWeb/Layout/expected/inline-flex-with-aspect-ratio.txt @@ -1,15 +1,15 @@ Viewport <#document> at (0,0) content-size 800x600 children: not-inline BlockContainer at (0,0) content-size 800x33 [BFC] children: not-inline BlockContainer at (8,8) content-size 784x17 children: inline - frag 0 from Box start: 0, length: 0, rect: [8,8 10x10.3125] baseline: 13.296875 - Box at (8,8) content-size 10x10.3125 flex-container(row) [FFC] children: not-inline - BlockContainer at (8,8) content-size 10x17 flex-item [BFC] children: inline - frag 0 from ImageBox start: 0, length: 0, rect: [8,11 10x10] baseline: 10 - ImageBox at (8,11) content-size 10x10 children: not-inline + frag 0 from Box start: 0, length: 0, rect: [8,11 10x10.3125] baseline: 9.953125 + Box at (8,11) content-size 10x10.3125 flex-container(row) [FFC] children: not-inline + BlockContainer at (8,7.65625) content-size 10x17 flex-item [BFC] children: inline + frag 0 from ImageBox start: 0, length: 0, rect: [8,10.65625 10x10] baseline: 10 + ImageBox at (8,10.65625) content-size 10x10 children: not-inline ViewportPaintable (Viewport<#document>) [0,0 800x600] PaintableWithLines (BlockContainer) [0,0 800x33] PaintableWithLines (BlockContainer) [8,8 784x17] - PaintableBox (Box
.inline-flex.aspect-ratio) [8,8 10x10.3125] overflow: [8,8 10x17] - PaintableWithLines (BlockContainer
.img-wrapper) [8,8 10x17] - ImagePaintable (ImageBox) [8,11 10x10] + PaintableBox (Box
.inline-flex.aspect-ratio) [8,11 10x10.3125] overflow: [8,7.65625 10x17] + PaintableWithLines (BlockContainer
.img-wrapper) [8,7.65625 10x17] + ImagePaintable (ImageBox) [8,10.65625 10x10] diff --git a/Tests/LibWeb/Layout/input/block-and-inline/atomic-inline-with-aspect-ratio-2.html b/Tests/LibWeb/Layout/input/block-and-inline/atomic-inline-with-aspect-ratio-2.html new file mode 100644 index 00000000000..3d9b54bd8b0 --- /dev/null +++ b/Tests/LibWeb/Layout/input/block-and-inline/atomic-inline-with-aspect-ratio-2.html @@ -0,0 +1,10 @@ +b \ No newline at end of file diff --git a/Userland/Libraries/LibWeb/Layout/FlexFormattingContext.cpp b/Userland/Libraries/LibWeb/Layout/FlexFormattingContext.cpp index dbcd5ee5d1f..b6f6416e14b 100644 --- a/Userland/Libraries/LibWeb/Layout/FlexFormattingContext.cpp +++ b/Userland/Libraries/LibWeb/Layout/FlexFormattingContext.cpp @@ -143,8 +143,8 @@ void FlexFormattingContext::run(Box const& run_box, LayoutMode, AvailableSpace c // 14. Align all flex items along the cross-axis align_all_flex_items_along_the_cross_axis(); - // 15. Determine the flex container’s used cross size: - determine_flex_container_used_cross_size(); + // 15. Determine the flex container’s used cross size + // NOTE: This is handled by the parent formatting context. { // https://drafts.csswg.org/css-flexbox-1/#definite-sizes @@ -1435,45 +1435,6 @@ void FlexFormattingContext::align_all_flex_items_along_the_cross_axis() } } -// https://www.w3.org/TR/css-flexbox-1/#algo-cross-container -void FlexFormattingContext::determine_flex_container_used_cross_size() -{ - CSSPixels cross_size = 0; - if (has_definite_cross_size(m_flex_container_state)) { - // Flex container has definite cross size: easy-peasy. - cross_size = inner_cross_size(m_flex_container_state); - } else { - // Flex container has indefinite cross size. - auto cross_size_value = is_row_layout() ? flex_container().computed_values().height() : flex_container().computed_values().width(); - if (cross_size_value.is_auto() || cross_size_value.contains_percentage()) { - // If a content-based cross size is needed, use the sum of the flex lines' cross sizes. - CSSPixels sum_of_flex_lines_cross_sizes = 0; - for (auto& flex_line : m_flex_lines) { - sum_of_flex_lines_cross_sizes += flex_line.cross_size; - } - cross_size = sum_of_flex_lines_cross_sizes; - - if (cross_size_value.contains_percentage()) { - // FIXME: Handle percentage values here! Right now we're just treating them as "auto" - } - } else { - // Otherwise, resolve the indefinite size at this point. - cross_size = cross_size_value.to_px(flex_container(), inner_cross_size(m_state.get(*flex_container().containing_block()))); - } - } - - // AD-HOC: We don't apply min/max cross size constraints when sizing the flex container under an intrinsic sizing constraint. - if (!m_available_space_for_items->cross.is_intrinsic_sizing_constraint()) { - auto const& computed_min_size = this->computed_cross_min_size(flex_container()); - auto const& computed_max_size = this->computed_cross_max_size(flex_container()); - auto cross_min_size = (!computed_min_size.is_auto() && !computed_min_size.contains_percentage()) ? specified_cross_min_size(flex_container()) : 0; - auto cross_max_size = (!computed_max_size.is_none() && !computed_max_size.contains_percentage()) ? specified_cross_max_size(flex_container()) : CSSPixels::max(); - set_cross_size(flex_container(), css_clamp(cross_size, cross_min_size, cross_max_size)); - } else { - set_cross_size(flex_container(), cross_size); - } -} - // https://www.w3.org/TR/css-flexbox-1/#algo-line-align void FlexFormattingContext::align_all_flex_lines() { diff --git a/Userland/Libraries/LibWeb/Layout/FlexFormattingContext.h b/Userland/Libraries/LibWeb/Layout/FlexFormattingContext.h index 2f6c1130e9f..29ce48054d3 100644 --- a/Userland/Libraries/LibWeb/Layout/FlexFormattingContext.h +++ b/Userland/Libraries/LibWeb/Layout/FlexFormattingContext.h @@ -191,8 +191,6 @@ private: void align_all_flex_items_along_the_cross_axis(); - void determine_flex_container_used_cross_size(); - void align_all_flex_lines(); bool is_row_layout() const { return m_flex_direction == CSS::FlexDirection::Row || m_flex_direction == CSS::FlexDirection::RowReverse; }