From 197efc898540f0e1ab0a3f697508a67d09336bf6 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Tue, 9 May 2023 09:56:30 +0200 Subject: [PATCH] LibWeb: Improve handling of min/max constraint violations on images Instead of bailing after resolving one violated constraint, we have to continue down the list of remaining constraints. We now also call the constraint solver for all replaced elements with "auto" for both width and height. Co-authored-by: 0GreenClover0 --- ...ge-with-multiple-constraint-violations.txt | 6 ++ ...e-with-multiple-constraint-violations.html | 9 +++ .../LibWeb/Layout/FormattingContext.cpp | 79 +++++++++++++------ 3 files changed, 71 insertions(+), 23 deletions(-) create mode 100644 Tests/LibWeb/Layout/expected/image-with-multiple-constraint-violations.txt create mode 100644 Tests/LibWeb/Layout/input/image-with-multiple-constraint-violations.html diff --git a/Tests/LibWeb/Layout/expected/image-with-multiple-constraint-violations.txt b/Tests/LibWeb/Layout/expected/image-with-multiple-constraint-violations.txt new file mode 100644 index 0000000000..5aadb8406e --- /dev/null +++ b/Tests/LibWeb/Layout/expected/image-with-multiple-constraint-violations.txt @@ -0,0 +1,6 @@ +Viewport <#document> at (0,0) content-size 800x600 children: not-inline + BlockContainer at (1,1) content-size 798x80 [BFC] children: not-inline + BlockContainer at (10,10) content-size 780x62 children: inline + line 0 width: 82, height: 62, bottom: 62, baseline: 62 + frag 0 from ImageBox start: 0, length: 0, rect: [11,11 80x60] + ImageBox at (11,11) content-size 80x60 children: not-inline diff --git a/Tests/LibWeb/Layout/input/image-with-multiple-constraint-violations.html b/Tests/LibWeb/Layout/input/image-with-multiple-constraint-violations.html new file mode 100644 index 0000000000..b80aa17ebb --- /dev/null +++ b/Tests/LibWeb/Layout/input/image-with-multiple-constraint-violations.html @@ -0,0 +1,9 @@ + \ No newline at end of file diff --git a/Userland/Libraries/LibWeb/Layout/FormattingContext.cpp b/Userland/Libraries/LibWeb/Layout/FormattingContext.cpp index 3151640722..5d6ea5e744 100644 --- a/Userland/Libraries/LibWeb/Layout/FormattingContext.cpp +++ b/Userland/Libraries/LibWeb/Layout/FormattingContext.cpp @@ -258,7 +258,7 @@ FormattingContext::ShrinkToFitResult FormattingContext::calculate_shrink_to_fit_ }; } -static CSSPixelSize solve_replaced_size_constraint(LayoutState const& state, CSSPixels w, CSSPixels h, ReplacedBox const& box) +static CSSPixelSize solve_replaced_size_constraint(LayoutState const& state, CSSPixels input_width, CSSPixels input_height, ReplacedBox const& box) { // 10.4 Minimum and maximum widths: 'min-width' and 'max-width' @@ -268,35 +268,42 @@ static CSSPixelSize solve_replaced_size_constraint(LayoutState const& state, CSS auto height_of_containing_block = containing_block_state.content_height(); CSSPixels specified_min_width = box.computed_values().min_width().is_auto() ? 0 : box.computed_values().min_width().to_px(box, width_of_containing_block); - CSSPixels specified_max_width = box.computed_values().max_width().is_none() ? w : box.computed_values().max_width().to_px(box, width_of_containing_block); + CSSPixels specified_max_width = box.computed_values().max_width().is_none() ? input_width : box.computed_values().max_width().to_px(box, width_of_containing_block); CSSPixels specified_min_height = box.computed_values().min_height().is_auto() ? 0 : box.computed_values().min_height().to_px(box, height_of_containing_block); - CSSPixels specified_max_height = box.computed_values().max_height().is_none() ? h : box.computed_values().max_height().to_px(box, height_of_containing_block); + CSSPixels specified_max_height = box.computed_values().max_height().is_none() ? input_height : box.computed_values().max_height().to_px(box, height_of_containing_block); auto min_width = min(specified_min_width, specified_max_width); auto max_width = max(specified_min_width, specified_max_width); auto min_height = min(specified_min_height, specified_max_height); auto max_height = max(specified_min_height, specified_max_height); + struct Size { + CSSPixels width; + CSSPixels height; + } size = { input_width, input_height }; + auto& w = size.width; + auto& h = size.height; + if (w > max_width) - return { w, max(max_width * (h / w), min_height) }; + size = { max_width, max(max_width * (h / w), min_height) }; if (w < min_width) - return { max_width, min(min_width * (h / w), max_height) }; + size = { min_width, min(min_width * (h / w), max_height) }; if (h > max_height) - return { max(max_height * (w / h), min_width), max_height }; + size = { max(max_height * (w / h), min_width), max_height }; if (h < min_height) - return { min(min_height * (w / h), max_width), min_height }; - if ((w > max_width && h > max_height) && (max_width / w < max_height / h)) - return { max_width, max(min_height, max_width * (h / w)) }; + size = { min(min_height * (w / h), max_width), min_height }; + if ((w > max_width && h > max_height) && (max_width / w <= max_height / h)) + size = { max_width, max(min_height, max_width * (h / w)) }; if ((w > max_width && h > max_height) && (max_width / w > max_height / h)) - return { max(min_width, max_height * (w / h)), max_height }; - if ((w < min_width && h < min_height) && (min_width / w < min_height / h)) - return { min(max_width, min_height * (w / h)), min_height }; + size = { max(min_width, max_height * (w / h)), max_height }; + if ((w < min_width && h < min_height) && (min_width / w <= min_height / h)) + size = { min(max_width, min_height * (w / h)), min_height }; if ((w < min_width && h < min_height) && (min_width / w > min_height / h)) - return { min_width, min(max_height, min_width * (h / w)) }; + size = { min_width, min(max_height, min_width * (h / w)) }; if (w < min_width && h > max_height) - return { min_width, max_height }; + size = { min_width, max_height }; if (w > max_width && h < min_height) - return { max_width, min_height }; + size = { max_width, min_height }; return { w, h }; } @@ -430,10 +437,18 @@ CSSPixels FormattingContext::compute_width_for_replaced_element(LayoutState cons auto width_of_containing_block_as_length = CSS::Length::make_px(width_of_containing_block); auto computed_width = should_treat_width_as_auto(box, available_space) ? CSS::Size::make_auto() : box.computed_values().width(); + auto computed_height = should_treat_height_as_auto(box, available_space) ? CSS::Size::make_auto() : box.computed_values().height(); // 1. The tentative used width is calculated (without 'min-width' and 'max-width') auto used_width = tentative_width_for_replaced_element(state, box, computed_width, available_space); + if (computed_width.is_auto() && computed_height.is_auto() && box.has_intrinsic_aspect_ratio()) { + CSSPixels w = used_width; + CSSPixels h = tentative_height_for_replaced_element(state, box, computed_height, available_space); + used_width = solve_replaced_size_constraint(state, w, h, box).width(); + return used_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'. auto computed_max_width = box.computed_values().max_width(); @@ -485,28 +500,46 @@ CSSPixels FormattingContext::tentative_height_for_replaced_element(LayoutState c CSSPixels FormattingContext::compute_height_for_replaced_element(LayoutState const& state, ReplacedBox const& box, AvailableSpace const& available_space) { - // 10.6.2 Inline replaced elements, block-level replaced elements in normal flow, - // 'inline-block' replaced elements in normal flow and floating replaced elements + // 10.6.2 Inline replaced elements + // 10.6.4 Block-level replaced elements in normal flow + // 10.6.6 Floating replaced elements + // 10.6.10 'inline-block' replaced elements in normal flow auto height_of_containing_block = state.get(*box.containing_block()).content_height(); auto computed_width = should_treat_width_as_auto(box, available_space) ? CSS::Size::make_auto() : box.computed_values().width(); auto computed_height = should_treat_height_as_auto(box, available_space) ? CSS::Size::make_auto() : box.computed_values().height(); + // 1. The tentative used height is calculated (without 'min-height' and 'max-height') CSSPixels used_height = tentative_height_for_replaced_element(state, box, computed_height, available_space); + // However, for replaced elements with both 'width' and 'height' computed as 'auto', + // use the algorithm under 'Minimum and maximum widths' + // https://www.w3.org/TR/CSS22/visudet.html#min-max-widths + // to find the used width and height. if (computed_width.is_auto() && computed_height.is_auto() && box.has_intrinsic_aspect_ratio()) { CSSPixels w = tentative_width_for_replaced_element(state, box, computed_width, available_space); CSSPixels h = used_height; used_height = solve_replaced_size_constraint(state, w, h, box).height(); + return used_height; } - auto const& computed_min_height = box.computed_values().min_height(); - auto const& computed_max_height = box.computed_values().max_height(); + // 2. If this tentative height is greater than 'max-height', the rules above are applied again, + // but this time using the value of 'max-height' as the computed value for 'height'. + auto computed_max_height = box.computed_values().max_height(); + if (!computed_max_height.is_none()) { + if (used_height > computed_max_height.to_px(box, height_of_containing_block)) { + used_height = tentative_height_for_replaced_element(state, box, computed_max_height, available_space); + } + } - if (!computed_max_height.is_none()) - used_height = min(used_height, computed_max_height.to_px(box, height_of_containing_block)); - if (!computed_min_height.is_auto()) - used_height = max(used_height, computed_min_height.to_px(box, height_of_containing_block)); + // 3. If the resulting height is smaller than 'min-height', the rules above are applied again, + // but this time using the value of 'min-height' as the computed value for 'height'. + auto computed_min_height = box.computed_values().min_height(); + if (!computed_min_height.is_auto()) { + if (used_height < computed_min_height.to_px(box, height_of_containing_block)) { + used_height = tentative_height_for_replaced_element(state, box, computed_min_height, available_space); + } + } return used_height; }