Skip to content

Commit 5af02a9

Browse files
committed
LibWeb: Let parent formatting context determine size of flex containers
Until now, we had implemented flex container sizing by awkwardly doing exactly what the spec said (basically having FFC size the container) despite that not really making sense in the big picture. (Parent formatting contexts should be responsible for sizing and placing their children) This patch moves us away from the Flexbox spec text a little bit, by removing the logic for sizing the flex container in FFC, and instead making sure that all formatting contexts can set both width and height of flex container children. This required changes in BFC and IFC, but it's actually quite simple! Width was already not a problem, and it turns out height isn't either, since the automatic height of a flex container is max-content. With this in mind, we can simply determine the height of flex containers before transferring control to FFC, and everything flows nicely. With this change, we can remove all the virtuals and FFC logic for negotiating container size with the parent formatting context. We also don't need the "available space for flex container" stuff anymore either, so that's gone as well. There are some minor diffs in layout test results from this, but the new results actually match other browsers more closely, so that's fine. This should make flex layout, and indeed layout in general, easier to understand, since this was the main weird special case outside of BFC/IFC where a formatting context delegates work to its parent instead of the other way around. :^)
1 parent 5f0230a commit 5af02a9

9 files changed

+40
-196
lines changed

Tests/LibWeb/Layout/expected/flex-item-with-cyclic-percentage-height.txt

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ Viewport <#document> at (0,0) content-size 800x600 children: not-inline
22
BlockContainer <html> at (1,1) content-size 798x39.46875 [BFC] children: not-inline
33
BlockContainer <body> at (10,10) content-size 780x21.46875 children: not-inline
44
Box <div.flexrow> at (11,11) content-size 778x19.46875 flex-container(row) [FFC] children: not-inline
5-
Box <div.project> at (12,12) content-size 44.03125x17.46875 flex-container(column) flex-item [FFC] children: not-inline
5+
Box <div.project> at (12,12) content-size 44.03125x19.46875 flex-container(column) flex-item [FFC] children: not-inline
66
BlockContainer <(anonymous)> at (12,12) content-size 44.03125x17.46875 flex-item [BFC] children: inline
77
line 0 width: 44.03125, height: 17.46875, bottom: 17.46875, baseline: 13.53125
88
frag 0 from TextNode start: 0, length: 6, rect: [12,12 44.03125x17.46875]
@@ -11,8 +11,8 @@ Viewport <#document> at (0,0) content-size 800x600 children: not-inline
1111

1212
ViewportPaintable (Viewport<#document>) [0,0 800x600]
1313
PaintableWithLines (BlockContainer<HTML>) [0,0 800x41.46875]
14-
PaintableWithLines (BlockContainer<BODY>) [9,9 782x23.46875]
15-
PaintableBox (Box<DIV>.flexrow) [10,10 780x21.46875]
16-
PaintableBox (Box<DIV>.project) [11,11 46.03125x19.46875]
14+
PaintableWithLines (BlockContainer<BODY>) [9,9 782x23.46875] overflow: [10,10 780x22.46875]
15+
PaintableBox (Box<DIV>.flexrow) [10,10 780x21.46875] overflow: [11,11 778x21.46875]
16+
PaintableBox (Box<DIV>.project) [11,11 46.03125x21.46875]
1717
PaintableWithLines (BlockContainer(anonymous)) [12,12 44.03125x17.46875]
1818
TextPaintable (TextNode<#text>)
Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
Viewport <#document> at (0,0) content-size 800x600 children: not-inline
2-
BlockContainer <html> at (1,1) content-size 798x69.96875 [BFC] children: not-inline
3-
Box <body> at (10,10) content-size 780x51.96875 flex-container(row) [FFC] children: not-inline
4-
ImageBox <img> at (11,11) content-size 66.65625x49.984375 flex-item children: not-inline
2+
BlockContainer <html> at (1,1) content-size 798x69.984375 [BFC] children: not-inline
3+
Box <body> at (10,10) content-size 780x51.984375 flex-container(row) [FFC] children: not-inline
4+
ImageBox <img> at (11,11) content-size 64x49.984375 flex-item children: not-inline
55
BlockContainer <(anonymous)> (not painted) [BFC] children: inline
66
TextNode <#text>
77

88
ViewportPaintable (Viewport<#document>) [0,0 800x600]
9-
PaintableWithLines (BlockContainer<HTML>) [0,0 800x71.96875]
10-
PaintableBox (Box<BODY>) [9,9 782x53.96875] overflow: [10,10 780x51.984375]
11-
ImagePaintable (ImageBox<IMG>) [10,10 68.65625x51.984375]
9+
PaintableWithLines (BlockContainer<HTML>) [0,0 800x71.984375]
10+
PaintableBox (Box<BODY>) [9,9 782x53.984375]
11+
ImagePaintable (ImageBox<IMG>) [10,10 66x51.984375]

Userland/Libraries/LibWeb/Layout/BlockFormattingContext.cpp

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -680,7 +680,8 @@ void BlockFormattingContext::layout_block_level_box(Box const& box, BlockContain
680680

681681
place_block_level_element_in_normal_flow_horizontally(box, available_space);
682682

683-
if (box.is_replaced_box())
683+
// NOTE: Flex containers with `auto` height are treated as `max-content`, so we can compute their height early.
684+
if (box.is_replaced_box() || box.display().is_flex_inside())
684685
compute_height(box, available_space);
685686

686687
if (independent_formatting_context) {
@@ -1247,15 +1248,4 @@ CSSPixels BlockFormattingContext::greatest_child_width(Box const& box) const
12471248
return max_width;
12481249
}
12491250

1250-
void BlockFormattingContext::determine_width_of_child(Box const&, AvailableSpace const&)
1251-
{
1252-
// NOTE: We don't do anything here, since we'll have already determined the width of the child
1253-
// before recursing into nested layout within the child.
1254-
}
1255-
1256-
void BlockFormattingContext::determine_height_of_child(Box const& box, AvailableSpace const& available_space)
1257-
{
1258-
compute_height(box, available_space);
1259-
}
1260-
12611251
}

Userland/Libraries/LibWeb/Layout/BlockFormattingContext.h

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -50,10 +50,6 @@ class BlockFormattingContext : public FormattingContext {
5050

5151
void layout_block_level_box(Box const&, BlockContainer const&, LayoutMode, CSSPixels& bottom_of_lowest_margin_box, AvailableSpace const&);
5252

53-
virtual bool can_determine_size_of_child() const override { return true; }
54-
virtual void determine_width_of_child(Box const&, AvailableSpace const&) override;
55-
virtual void determine_height_of_child(Box const&, AvailableSpace const&) override;
56-
5753
void resolve_vertical_box_model_metrics(Box const&);
5854

5955
enum class DidIntroduceClearance {

Userland/Libraries/LibWeb/Layout/FlexFormattingContext.cpp

Lines changed: 23 additions & 139 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2021-2023, Andreas Kling <kling@serenityos.org>
2+
* Copyright (c) 2021-2024, Andreas Kling <kling@serenityos.org>
33
* Copyright (c) 2021, Tobias Christiansen <tobyase@serenityos.org>
44
*
55
* SPDX-License-Identifier: BSD-2-Clause
@@ -56,35 +56,17 @@ CSSPixels FlexFormattingContext::automatic_content_height() const
5656
return m_flex_container_state.content_height();
5757
}
5858

59-
void FlexFormattingContext::run(Box const& run_box, LayoutMode, AvailableSpace const& available_content_space)
59+
void FlexFormattingContext::run(Box const& run_box, LayoutMode, AvailableSpace const& available_space)
6060
{
6161
VERIFY(&run_box == &flex_container());
6262

63-
// NOTE: The available space provided by the parent context is basically our *content box*.
64-
// FFC is currently written in a way that expects that to include padding and border as well,
65-
// so we pad out the available space here to accommodate that.
66-
// FIXME: Refactor the necessary parts of FFC so we don't need this hack!
67-
68-
auto available_width = available_content_space.width;
69-
if (available_width.is_definite())
70-
available_width = AvailableSize::make_definite(available_width.to_px_or_zero() + m_flex_container_state.border_box_left() + m_flex_container_state.border_box_right());
71-
auto available_height = available_content_space.height;
72-
if (available_height.is_definite())
73-
available_height = AvailableSize::make_definite(available_height.to_px_or_zero() + m_flex_container_state.border_box_top() + m_flex_container_state.border_box_bottom());
74-
75-
m_available_space_for_flex_container = AxisAgnosticAvailableSpace {
76-
.main = is_row_layout() ? available_width : available_height,
77-
.cross = !is_row_layout() ? available_width : available_height,
78-
.space = { available_width, available_height },
79-
};
80-
8163
// This implements https://www.w3.org/TR/css-flexbox-1/#layout-algorithm
8264

8365
// 1. Generate anonymous flex items
8466
generate_anonymous_flex_items();
8567

8668
// 2. Determine the available main and cross space for the flex items
87-
determine_available_space_for_items(AvailableSpace(available_width, available_height));
69+
determine_available_space_for_items(available_space);
8870

8971
{
9072
// https://drafts.csswg.org/css-flexbox-1/#definite-sizes
@@ -114,12 +96,16 @@ void FlexFormattingContext::run(Box const& run_box, LayoutMode, AvailableSpace c
11496
determine_flex_base_size_and_hypothetical_main_size(item);
11597
}
11698

117-
if (available_width.is_intrinsic_sizing_constraint() || available_height.is_intrinsic_sizing_constraint()) {
99+
if (available_space.width.is_intrinsic_sizing_constraint() || available_space.height.is_intrinsic_sizing_constraint()) {
118100
// We're computing intrinsic size for the flex container. This happens at the end of run().
119101
} else {
120-
121102
// 4. Determine the main size of the flex container
122-
determine_main_size_of_flex_container();
103+
// Determine the main size of the flex container using the rules of the formatting context in which it participates.
104+
// NOTE: The automatic block size of a block-level flex container is its max-content size.
105+
106+
// NOTE: We've already handled this in the parent formatting context.
107+
// Specifically, all formatting contexts will have assigned width & height to the flex container
108+
// before this formatting context runs.
123109
}
124110

125111
// 5. Collect flex items into flex lines:
@@ -186,7 +172,7 @@ void FlexFormattingContext::run(Box const& run_box, LayoutMode, AvailableSpace c
186172
// 16. Align all flex lines (per align-content)
187173
align_all_flex_lines();
188174

189-
if (available_width.is_intrinsic_sizing_constraint() || available_height.is_intrinsic_sizing_constraint()) {
175+
if (available_space.width.is_intrinsic_sizing_constraint() || available_space.height.is_intrinsic_sizing_constraint()) {
190176
// We're computing intrinsic size for the flex container.
191177
determine_intrinsic_size_of_flex_container();
192178
} else {
@@ -195,7 +181,7 @@ void FlexFormattingContext::run(Box const& run_box, LayoutMode, AvailableSpace c
195181
copy_dimensions_from_flex_items_to_boxes();
196182
for (auto& item : m_flex_items) {
197183
auto& box_state = m_state.get(item.box);
198-
if (auto independent_formatting_context = layout_inside(item.box, LayoutMode::Normal, box_state.available_inner_space_or_constraints_from(m_available_space_for_flex_container->space)))
184+
if (auto independent_formatting_context = layout_inside(item.box, LayoutMode::Normal, box_state.available_inner_space_or_constraints_from(m_available_space_for_items->space)))
199185
independent_formatting_context->parent_context_did_dimension_child_root_box();
200186

201187
compute_inset(item.box);
@@ -443,66 +429,17 @@ void FlexFormattingContext::set_main_axis_second_margin(FlexItem& item, CSSPixel
443429
// https://drafts.csswg.org/css-flexbox-1/#algo-available
444430
void FlexFormattingContext::determine_available_space_for_items(AvailableSpace const& available_space)
445431
{
446-
// For each dimension, if that dimension of the flex container’s content box is a definite size, use that;
447-
// if that dimension of the flex container is being sized under a min or max-content constraint, the available space in that dimension is that constraint;
448-
// otherwise, subtract the flex container’s margin, border, and padding from the space available to the flex container in that dimension and use that value.
449-
// This might result in an infinite value.
450-
451-
Optional<AvailableSize> available_width_for_items;
452-
if (m_flex_container_state.has_definite_width()) {
453-
available_width_for_items = AvailableSize::make_definite(m_flex_container_state.content_width());
454-
} else {
455-
if (available_space.width.is_intrinsic_sizing_constraint()) {
456-
available_width_for_items = available_space.width;
457-
} else {
458-
if (available_space.width.is_definite()) {
459-
auto remaining = available_space.width.to_px_or_zero()
460-
- m_flex_container_state.margin_left
461-
- m_flex_container_state.margin_right
462-
- m_flex_container_state.border_left
463-
- m_flex_container_state.padding_right
464-
- m_flex_container_state.padding_left
465-
- m_flex_container_state.padding_right;
466-
available_width_for_items = AvailableSize::make_definite(remaining);
467-
} else {
468-
available_width_for_items = AvailableSize::make_indefinite();
469-
}
470-
}
471-
}
472-
473-
Optional<AvailableSize> available_height_for_items;
474-
if (m_flex_container_state.has_definite_height()) {
475-
available_height_for_items = AvailableSize::make_definite(m_flex_container_state.content_height());
476-
} else {
477-
if (available_space.height.is_intrinsic_sizing_constraint()) {
478-
available_height_for_items = available_space.height;
479-
} else {
480-
if (available_space.height.is_definite()) {
481-
auto remaining = available_space.height.to_px_or_zero()
482-
- m_flex_container_state.margin_top
483-
- m_flex_container_state.margin_bottom
484-
- m_flex_container_state.border_top
485-
- m_flex_container_state.padding_bottom
486-
- m_flex_container_state.padding_top
487-
- m_flex_container_state.padding_bottom;
488-
available_height_for_items = AvailableSize::make_definite(remaining);
489-
} else {
490-
available_height_for_items = AvailableSize::make_indefinite();
491-
}
492-
}
493-
}
494-
495432
if (is_row_layout()) {
496433
m_available_space_for_items = AxisAgnosticAvailableSpace {
497-
.main = *available_width_for_items,
498-
.cross = *available_height_for_items,
499-
.space = { *available_width_for_items, *available_height_for_items },
434+
.main = available_space.width,
435+
.cross = available_space.height,
436+
.space = { available_space.width, available_space.height },
500437
};
501438
} else {
502439
m_available_space_for_items = AxisAgnosticAvailableSpace {
503-
.main = *available_height_for_items,
504-
.cross = *available_width_for_items,
505-
.space = { *available_width_for_items, *available_height_for_items },
440+
.main = available_space.height,
441+
.cross = available_space.width,
442+
.space = { available_space.width, available_space.height },
506443
};
507444
}
508445
}
@@ -785,59 +722,6 @@ CSSPixels FlexFormattingContext::content_based_minimum_size(FlexItem const& item
785722
return unclamped_size;
786723
}
787724

788-
bool FlexFormattingContext::can_determine_size_of_child() const
789-
{
790-
return true;
791-
}
792-
793-
void FlexFormattingContext::determine_width_of_child(Box const&, AvailableSpace const&)
794-
{
795-
// NOTE: For now, we simply do nothing here. If a child context is calling up to us
796-
// and asking us to determine its width, we've already done so as part of the
797-
// flex layout algorithm.
798-
}
799-
800-
void FlexFormattingContext::determine_height_of_child(Box const&, AvailableSpace const&)
801-
{
802-
// NOTE: For now, we simply do nothing here. If a child context is calling up to us
803-
// and asking us to determine its height, we've already done so as part of the
804-
// flex layout algorithm.
805-
}
806-
807-
// https://drafts.csswg.org/css-flexbox-1/#algo-main-container
808-
void FlexFormattingContext::determine_main_size_of_flex_container()
809-
{
810-
// Determine the main size of the flex container using the rules of the formatting context in which it participates.
811-
// NOTE: The automatic block size of a block-level flex container is its max-content size.
812-
813-
// FIXME: The code below doesn't know how to size absolutely positioned flex containers at all.
814-
// We just leave it alone for now and let the parent context deal with it.
815-
if (flex_container().is_absolutely_positioned())
816-
return;
817-
818-
// FIXME: Once all parent contexts now how to size a given child, we can remove
819-
// `can_determine_size_of_child()`.
820-
if (parent()->can_determine_size_of_child()) {
821-
if (is_row_layout()) {
822-
parent()->determine_width_of_child(flex_container(), m_available_space_for_flex_container->space);
823-
} else {
824-
parent()->determine_height_of_child(flex_container(), m_available_space_for_flex_container->space);
825-
}
826-
return;
827-
}
828-
829-
if (is_row_layout()) {
830-
if (!flex_container().is_out_of_flow(*parent()) && m_state.get(*flex_container().containing_block()).has_definite_width()) {
831-
set_main_size(flex_container(), calculate_stretch_fit_width(flex_container(), m_available_space_for_flex_container->space.width));
832-
} else {
833-
set_main_size(flex_container(), calculate_max_content_width(flex_container()));
834-
}
835-
} else {
836-
if (!has_definite_main_size(flex_container()))
837-
set_main_size(flex_container(), calculate_max_content_height(flex_container(), m_available_space_for_flex_container->space.width.to_px_or_zero()));
838-
}
839-
}
840-
841725
// https://www.w3.org/TR/css-flexbox-1/#algo-line-break
842726
void FlexFormattingContext::collect_flex_items_into_flex_lines()
843727
{
@@ -1229,7 +1113,7 @@ void FlexFormattingContext::calculate_cross_size_of_each_flex_line()
12291113
// If the flex container is single-line, then clamp the line’s cross-size to be within the container’s computed min and max cross sizes.
12301114
// Note that if CSS 2.1’s definition of min/max-width/height applied more generally, this behavior would fall out automatically.
12311115
// AD-HOC: We don't do this when the flex container is being sized under a min-content or max-content constraint.
1232-
if (is_single_line() && !m_available_space_for_flex_container->cross.is_intrinsic_sizing_constraint()) {
1116+
if (is_single_line() && !m_available_space_for_items->cross.is_intrinsic_sizing_constraint()) {
12331117
auto const& computed_min_size = this->computed_cross_min_size(flex_container());
12341118
auto const& computed_max_size = this->computed_cross_max_size(flex_container());
12351119
auto cross_min_size = (!computed_min_size.is_auto() && !computed_min_size.contains_percentage()) ? specified_cross_min_size(flex_container()) : 0;
@@ -1553,7 +1437,7 @@ void FlexFormattingContext::determine_flex_container_used_cross_size()
15531437
}
15541438

15551439
// AD-HOC: We don't apply min/max cross size constraints when sizing the flex container under an intrinsic sizing constraint.
1556-
if (!m_available_space_for_flex_container->cross.is_intrinsic_sizing_constraint()) {
1440+
if (!m_available_space_for_items->cross.is_intrinsic_sizing_constraint()) {
15571441
auto const& computed_min_size = this->computed_cross_min_size(flex_container());
15581442
auto const& computed_max_size = this->computed_cross_max_size(flex_container());
15591443
auto cross_min_size = (!computed_min_size.is_auto() && !computed_min_size.contains_percentage()) ? specified_cross_min_size(flex_container()) : 0;
@@ -1689,7 +1573,7 @@ void FlexFormattingContext::copy_dimensions_from_flex_items_to_boxes()
16891573
// https://drafts.csswg.org/css-flexbox-1/#intrinsic-sizes
16901574
void FlexFormattingContext::determine_intrinsic_size_of_flex_container()
16911575
{
1692-
if (m_available_space_for_flex_container->main.is_intrinsic_sizing_constraint()) {
1576+
if (m_available_space_for_items->main.is_intrinsic_sizing_constraint()) {
16931577
CSSPixels main_size = calculate_intrinsic_main_size_of_flex_container();
16941578
set_main_size(flex_container(), main_size);
16951579
}
@@ -2051,7 +1935,7 @@ CSSPixels FlexFormattingContext::calculate_fit_content_cross_size(FlexItem const
20511935
CSSPixels FlexFormattingContext::calculate_min_content_cross_size(FlexItem const& item) const
20521936
{
20531937
if (is_row_layout()) {
2054-
auto available_space = m_state.get(item.box).available_inner_space_or_constraints_from(m_available_space_for_flex_container->space);
1938+
auto available_space = m_state.get(item.box).available_inner_space_or_constraints_from(m_available_space_for_items->space);
20551939
if (available_space.width.is_indefinite()) {
20561940
available_space.width = AvailableSize::make_definite(calculate_width_to_use_when_determining_intrinsic_height_of_item(item));
20571941
}
@@ -2063,7 +1947,7 @@ CSSPixels FlexFormattingContext::calculate_min_content_cross_size(FlexItem const
20631947
CSSPixels FlexFormattingContext::calculate_max_content_cross_size(FlexItem const& item) const
20641948
{
20651949
if (is_row_layout()) {
2066-
auto available_space = m_state.get(item.box).available_inner_space_or_constraints_from(m_available_space_for_flex_container->space);
1950+
auto available_space = m_state.get(item.box).available_inner_space_or_constraints_from(m_available_space_for_items->space);
20671951
if (available_space.width.is_indefinite()) {
20681952
available_space.width = AvailableSize::make_definite(calculate_width_to_use_when_determining_intrinsic_height_of_item(item));
20691953
}

0 commit comments

Comments
 (0)