From 05a1dbeb91d6fbeda65a69736052ce5aedcc85d6 Mon Sep 17 00:00:00 2001 From: Aliaksandr Kalenik Date: Sun, 7 Jan 2024 05:24:09 +0100 Subject: [PATCH] LibWeb: Return CSSPixels from `calculate_inner_width()` Initially, this function was made to return CSS::Length because some of its callers were expecting it to ignore "auto" width on input and return it as is. Instead, let's just forbid using "auto" for input width and always return CSSPixels. --- .../LibWeb/Layout/BlockFormattingContext.cpp | 30 +++++++------- .../LibWeb/Layout/FlexFormattingContext.cpp | 2 +- .../LibWeb/Layout/FormattingContext.cpp | 40 +++++++++++-------- .../LibWeb/Layout/FormattingContext.h | 2 +- .../LibWeb/Layout/GridFormattingContext.cpp | 8 ++-- .../LibWeb/Layout/InlineFormattingContext.cpp | 6 +-- 6 files changed, 48 insertions(+), 40 deletions(-) diff --git a/Userland/Libraries/LibWeb/Layout/BlockFormattingContext.cpp b/Userland/Libraries/LibWeb/Layout/BlockFormattingContext.cpp index 5afcb0394d1..a2378ebb97d 100644 --- a/Userland/Libraries/LibWeb/Layout/BlockFormattingContext.cpp +++ b/Userland/Libraries/LibWeb/Layout/BlockFormattingContext.cpp @@ -199,7 +199,7 @@ void BlockFormattingContext::compute_width(Box const& box, AvailableSpace const& if (box_state.width_constraint != SizeConstraint::None) return; - auto try_compute_width = [&](auto const& a_width) { + auto try_compute_width = [&](CSS::Length const& a_width) { CSS::Length width = a_width; margin_left = computed_values.margin().left().resolved(box, width_of_containing_block); margin_right = computed_values.margin().right().resolved(box, width_of_containing_block); @@ -265,7 +265,7 @@ void BlockFormattingContext::compute_width(Box const& box, AvailableSpace const& return CSS::Length::make_px(compute_table_box_width_inside_table_wrapper(box, remaining_available_space)); if (should_treat_width_as_auto(box, remaining_available_space)) return CSS::Length::make_auto(); - return calculate_inner_width(box, remaining_available_space.width, computed_values.width()); + return CSS::Length::make_px(calculate_inner_width(box, remaining_available_space.width, computed_values.width())); }(); // 1. The tentative used width is calculated (without 'min-width' and 'max-width') @@ -276,8 +276,8 @@ void BlockFormattingContext::compute_width(Box const& box, AvailableSpace const& if (!should_treat_max_width_as_none(box, available_space.width)) { auto max_width = calculate_inner_width(box, remaining_available_space.width, computed_values.max_width()); auto used_width_px = used_width.is_auto() ? CSSPixels { 0 } : used_width.to_px(box); - if (used_width_px > max_width.to_px(box)) { - used_width = try_compute_width(max_width); + if (used_width_px > max_width) { + used_width = try_compute_width(CSS::Length::make_px(max_width)); } } @@ -286,8 +286,8 @@ void BlockFormattingContext::compute_width(Box const& box, AvailableSpace const& if (!computed_values.min_width().is_auto()) { auto min_width = calculate_inner_width(box, remaining_available_space.width, computed_values.min_width()); auto used_width_px = used_width.is_auto() ? remaining_available_space.width : AvailableSize::make_definite(used_width.to_px(box)); - if (used_width_px < min_width.to_px(box)) { - used_width = try_compute_width(min_width); + if (used_width_px < min_width) { + used_width = try_compute_width(CSS::Length::make_px(min_width)); } } @@ -346,7 +346,7 @@ void BlockFormattingContext::compute_width_for_floating_box(Box const& box, Avai auto input_width = [&] { if (should_treat_width_as_auto(box, available_space)) return CSS::Length::make_auto(); - return calculate_inner_width(box, available_space.width, computed_values.width()); + return CSS::Length::make_px(calculate_inner_width(box, available_space.width, computed_values.width())); }(); // 1. The tentative used width is calculated (without 'min-width' and 'max-width') @@ -356,16 +356,16 @@ void BlockFormattingContext::compute_width_for_floating_box(Box const& box, Avai // but this time using the computed value of 'max-width' as the computed value for 'width'. if (!should_treat_max_width_as_none(box, available_space.width)) { auto max_width = calculate_inner_width(box, available_space.width, computed_values.max_width()); - if (width.to_px(box) > max_width.to_px(box)) - width = compute_width(max_width); + if (width.to_px(box) > max_width) + width = compute_width(CSS::Length::make_px(max_width)); } // 3. If the resulting width is smaller than 'min-width', the rules above are applied again, // but this time using the value of 'min-width' as the computed value for 'width'. if (!computed_values.min_width().is_auto()) { auto min_width = calculate_inner_width(box, available_space.width, computed_values.min_width()); - if (width.to_px(box) < min_width.to_px(box)) - width = compute_width(min_width); + if (width.to_px(box) < min_width) + width = compute_width(CSS::Length::make_px(min_width)); } auto& box_state = m_state.get_mutable(box); @@ -524,7 +524,7 @@ void BlockFormattingContext::layout_inline_children(BlockContainer const& block_ auto containing_block_width = m_state.get(*block_container.containing_block()).content_width(); auto available_width = AvailableSize::make_definite(containing_block_width); if (!should_treat_max_width_as_none(block_container, available_space.width)) { - auto max_width_px = calculate_inner_width(block_container, available_width, block_container.computed_values().max_width()).to_px(block_container); + auto max_width_px = calculate_inner_width(block_container, available_width, block_container.computed_values().max_width()); if (used_width_px > max_width_px) used_width_px = max_width_px; } @@ -543,7 +543,7 @@ void BlockFormattingContext::layout_inline_children(BlockContainer const& block_ return false; }(); if (!should_treat_min_width_as_auto) { - auto min_width_px = calculate_inner_width(block_container, available_width, block_container.computed_values().min_width()).to_px(block_container); + auto min_width_px = calculate_inner_width(block_container, available_width, block_container.computed_values().min_width()); if (used_width_px < min_width_px) used_width_px = min_width_px; } @@ -763,12 +763,12 @@ void BlockFormattingContext::layout_block_level_children(BlockContainer const& b if (!should_treat_max_width_as_none(block_container, available_space.width)) { auto max_width = calculate_inner_width(block_container, available_space.width, computed_values.max_width()); - width = min(width, max_width.to_px(block_container)); + width = min(width, max_width); } if (!computed_values.min_width().is_auto()) { auto min_width = calculate_inner_width(block_container, available_space.width, computed_values.min_width()); - width = max(width, min_width.to_px(block_container)); + width = max(width, min_width); } } block_container_state.set_content_width(width); diff --git a/Userland/Libraries/LibWeb/Layout/FlexFormattingContext.cpp b/Userland/Libraries/LibWeb/Layout/FlexFormattingContext.cpp index f1ada492f83..708991bf217 100644 --- a/Userland/Libraries/LibWeb/Layout/FlexFormattingContext.cpp +++ b/Userland/Libraries/LibWeb/Layout/FlexFormattingContext.cpp @@ -29,7 +29,7 @@ template CSSPixels FlexFormattingContext::get_pixel_width(Box const& box, CSS::Size const& size) const { - return calculate_inner_width(box, containing_block_width_as_available_size(box), size).to_px(box); + return calculate_inner_width(box, containing_block_width_as_available_size(box), size); } CSSPixels FlexFormattingContext::get_pixel_height(Box const& box, CSS::Size const& size) const diff --git a/Userland/Libraries/LibWeb/Layout/FormattingContext.cpp b/Userland/Libraries/LibWeb/Layout/FormattingContext.cpp index 81d9f24d08e..88bdf0f07ee 100644 --- a/Userland/Libraries/LibWeb/Layout/FormattingContext.cpp +++ b/Userland/Libraries/LibWeb/Layout/FormattingContext.cpp @@ -388,7 +388,12 @@ CSSPixels FormattingContext::tentative_width_for_replaced_element(Box const& box auto computed_height = should_treat_height_as_auto(box, available_space) ? CSS::Size::make_auto() : box.computed_values().height(); - CSSPixels used_width = calculate_inner_width(box, available_space.width, computed_width).to_px(box); + CSSPixels used_width = 0; + if (computed_width.is_auto()) { + used_width = computed_width.to_px(box, available_space.width.to_px_or_zero()); + } else { + used_width = calculate_inner_width(box, available_space.width, computed_width); + } // If 'height' and 'width' both have computed values of 'auto' and the element also has an intrinsic width, // then that intrinsic width is the used value of 'width'. @@ -577,7 +582,7 @@ void FormattingContext::compute_width_for_absolutely_positioned_non_replaced_ele auto left = computed_values.inset().left().to_px(box, width_of_containing_block); auto right = computed_values.inset().right().to_px(box, width_of_containing_block); - auto try_compute_width = [&](auto const& a_width) { + auto try_compute_width = [&](CSS::Length const& a_width) { margin_left = computed_values.margin().left().resolved(box, width_of_containing_block); margin_right = computed_values.margin().right().resolved(box, width_of_containing_block); @@ -697,14 +702,18 @@ void FormattingContext::compute_width_for_absolutely_positioned_non_replaced_ele }; // 1. The tentative used width is calculated (without 'min-width' and 'max-width') - auto used_width = try_compute_width(calculate_inner_width(box, available_space.width, computed_values.width())); + auto used_width = try_compute_width([&] { + if (computed_values.width().is_auto()) + return CSS::Length::make_auto(); + return CSS::Length::make_px(calculate_inner_width(box, available_space.width, computed_values.width())); + }()); // 2. The tentative used width is greater than 'max-width', the rules above are applied again, // but this time using the computed value of 'max-width' as the computed value for 'width'. if (!should_treat_max_width_as_none(box, available_space.width)) { auto max_width = calculate_inner_width(box, available_space.width, computed_values.max_width()); - if (used_width.to_px(box) > max_width.to_px(box)) { - used_width = try_compute_width(max_width); + if (used_width.to_px(box) > max_width) { + used_width = try_compute_width(CSS::Length::make_px(max_width)); } } @@ -712,8 +721,8 @@ void FormattingContext::compute_width_for_absolutely_positioned_non_replaced_ele // but this time using the value of 'min-width' as the computed value for 'width'. if (!computed_values.min_width().is_auto()) { auto min_width = calculate_inner_width(box, available_space.width, computed_values.min_width()); - if (used_width.to_px(box) < min_width.to_px(box)) { - used_width = try_compute_width(min_width); + if (used_width.to_px(box) < min_width) { + used_width = try_compute_width(CSS::Length::make_px(min_width)); } } @@ -1454,20 +1463,19 @@ CSSPixels FormattingContext::calculate_max_content_height(Layout::Box const& box return max_content_height; } -CSS::Length FormattingContext::calculate_inner_width(Layout::Box const& box, AvailableSize const& available_width, CSS::Size const& width) const +CSSPixels FormattingContext::calculate_inner_width(Layout::Box const& box, AvailableSize const& available_width, CSS::Size const& width) const { + VERIFY(!width.is_auto()); + auto width_of_containing_block = available_width.to_px_or_zero(); - if (width.is_auto()) { - return width.resolved(box, width_of_containing_block); - } if (width.is_fit_content()) { - return CSS::Length::make_px(calculate_fit_content_width(box, AvailableSpace { available_width, AvailableSize::make_indefinite() })); + return calculate_fit_content_width(box, AvailableSpace { available_width, AvailableSize::make_indefinite() }); } if (width.is_max_content()) { - return CSS::Length::make_px(calculate_max_content_width(box)); + return calculate_max_content_width(box); } if (width.is_min_content()) { - return CSS::Length::make_px(calculate_min_content_width(box)); + return calculate_min_content_width(box); } auto& computed_values = box.computed_values(); @@ -1480,10 +1488,10 @@ CSS::Length FormattingContext::calculate_inner_width(Layout::Box const& box, Ava - padding_left.to_px(box) - computed_values.border_right().width - padding_right.to_px(box); - return CSS::Length::make_px(max(inner_width, 0)); + return max(inner_width, 0); } - return width.resolved(box, width_of_containing_block); + return width.resolved(box, width_of_containing_block).to_px(box); } CSS::Length FormattingContext::calculate_inner_height(Layout::Box const& box, AvailableSize const&, CSS::Size const& height) const diff --git a/Userland/Libraries/LibWeb/Layout/FormattingContext.h b/Userland/Libraries/LibWeb/Layout/FormattingContext.h index 4b8030f6e87..a4cc1c7f0ed 100644 --- a/Userland/Libraries/LibWeb/Layout/FormattingContext.h +++ b/Userland/Libraries/LibWeb/Layout/FormattingContext.h @@ -65,7 +65,7 @@ public: CSSPixels calculate_fit_content_height(Layout::Box const&, AvailableSpace const&) const; CSSPixels calculate_fit_content_width(Layout::Box const&, AvailableSpace const&) const; - CSS::Length calculate_inner_width(Layout::Box const&, AvailableSize const&, CSS::Size const& width) const; + CSSPixels calculate_inner_width(Layout::Box const&, AvailableSize const&, CSS::Size const& width) const; CSS::Length calculate_inner_height(Layout::Box const&, AvailableSize const&, CSS::Size const& height) const; virtual CSSPixels greatest_child_width(Box const&) const; diff --git a/Userland/Libraries/LibWeb/Layout/GridFormattingContext.cpp b/Userland/Libraries/LibWeb/Layout/GridFormattingContext.cpp index 580315f7ecd..71fd5eb82e1 100644 --- a/Userland/Libraries/LibWeb/Layout/GridFormattingContext.cpp +++ b/Userland/Libraries/LibWeb/Layout/GridFormattingContext.cpp @@ -1694,12 +1694,12 @@ void GridFormattingContext::resolve_grid_item_widths() } else if (computed_width.is_fit_content()) { used_alignment = try_compute_width(calculate_fit_content_width(item.box, available_space)); } else { - auto width_px = calculate_inner_width(item.box, available_space.width, computed_width).to_px(item.box); + auto width_px = calculate_inner_width(item.box, available_space.width, computed_width); used_alignment = try_compute_width(width_px); } if (!should_treat_max_width_as_none(item.box, m_available_space->width)) { - auto max_width_px = calculate_inner_width(item.box, available_space.width, computed_values.max_width()).to_px(item.box); + auto max_width_px = calculate_inner_width(item.box, available_space.width, computed_values.max_width()); auto max_width_alignment = try_compute_width(max_width_px); if (used_alignment.width > max_width_alignment.width) { used_alignment = max_width_alignment; @@ -1707,7 +1707,7 @@ void GridFormattingContext::resolve_grid_item_widths() } if (!computed_values.min_width().is_auto()) { - auto min_width_px = calculate_inner_width(item.box, available_space.width, computed_values.min_width()).to_px(item.box); + auto min_width_px = calculate_inner_width(item.box, available_space.width, computed_values.min_width()); auto min_width_alignment = try_compute_width(min_width_px); if (used_alignment.width < min_width_alignment.width) { used_alignment = min_width_alignment; @@ -2330,7 +2330,7 @@ CSSPixels GridFormattingContext::calculate_grid_container_maximum_size(GridDimen { auto const& computed_values = grid_container().computed_values(); if (dimension == GridDimension::Column) - return calculate_inner_width(grid_container(), m_available_space->width, computed_values.max_width()).to_px(grid_container()); + return calculate_inner_width(grid_container(), m_available_space->width, computed_values.max_width()); return calculate_inner_height(grid_container(), m_available_space->height, computed_values.max_height()).to_px(grid_container()); } diff --git a/Userland/Libraries/LibWeb/Layout/InlineFormattingContext.cpp b/Userland/Libraries/LibWeb/Layout/InlineFormattingContext.cpp index 127f212934a..fd3f6454767 100644 --- a/Userland/Libraries/LibWeb/Layout/InlineFormattingContext.cpp +++ b/Userland/Libraries/LibWeb/Layout/InlineFormattingContext.cpp @@ -154,19 +154,19 @@ void InlineFormattingContext::dimension_box_on_line(Box const& box, LayoutMode l // NOTE: We can't resolve percentages yet. We'll have to wait until after inner layout. } else { auto inner_width = calculate_inner_width(box, m_available_space->width, width_value); - unconstrained_width = inner_width.to_px(box); + unconstrained_width = inner_width; } } CSSPixels width = unconstrained_width; if (!should_treat_max_width_as_none(box, m_available_space->width)) { - auto max_width = calculate_inner_width(box, m_available_space->width, box.computed_values().max_width()).to_px(box); + auto max_width = calculate_inner_width(box, m_available_space->width, box.computed_values().max_width()); width = min(width, max_width); } auto computed_min_width = box.computed_values().min_width(); if (!computed_min_width.is_auto()) { - auto min_width = calculate_inner_width(box, m_available_space->width, computed_min_width).to_px(box); + auto min_width = calculate_inner_width(box, m_available_space->width, computed_min_width); width = max(width, min_width); }