From f0560fd08786e18563b8c63a8b1dab0a830a4915 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Fri, 19 May 2023 18:14:37 +0200 Subject: [PATCH] LibWeb: Support elements with `display: block` There are a couple of things that went into this: - We now calculate the intrinsic width/height and aspect ratio of elements based on the spec algorithm instead of our previous ad-hoc guesswork solution. - Replaced elements with automatic size and intrinsic aspect ratio but no intrinsic dimensions are now sized with the stretch-fit width formula. - We take care to assign both used width and used height to elements before running their SVG formatting contexts. This ensures that the inside SVG content is laid out with knowledge of its viewport geometry. - We avoid infinite recursion in tentative_height_for_replaced_element() by using the already-calculated used width instead of calling the function that calculates the used width (since that may call us right back again). --- .../expected/svg/svg-fill-with-bogus-url.txt | 12 +- .../svg-intrinsic-size-in-one-dimension.txt | 7 ++ .../expected/svg/svg-with-display-block.txt | 5 + .../svg-intrinsic-size-in-one-dimension.html | 6 + .../input/svg/svg-with-display-block.html | 3 + .../LibWeb/Layout/BlockFormattingContext.cpp | 3 + .../LibWeb/Layout/FormattingContext.cpp | 23 +++- .../LibWeb/Layout/FormattingContext.h | 3 + .../LibWeb/Layout/InlineFormattingContext.cpp | 6 +- .../Libraries/LibWeb/Layout/SVGSVGBox.cpp | 110 ++++++++---------- Userland/Libraries/LibWeb/Layout/SVGSVGBox.h | 3 + 11 files changed, 107 insertions(+), 74 deletions(-) create mode 100644 Tests/LibWeb/Layout/expected/svg/svg-intrinsic-size-in-one-dimension.txt create mode 100644 Tests/LibWeb/Layout/expected/svg/svg-with-display-block.txt create mode 100644 Tests/LibWeb/Layout/input/svg/svg-intrinsic-size-in-one-dimension.html create mode 100644 Tests/LibWeb/Layout/input/svg/svg-with-display-block.html diff --git a/Tests/LibWeb/Layout/expected/svg/svg-fill-with-bogus-url.txt b/Tests/LibWeb/Layout/expected/svg/svg-fill-with-bogus-url.txt index 6dddd2e8da..5e727c696e 100644 --- a/Tests/LibWeb/Layout/expected/svg/svg-fill-with-bogus-url.txt +++ b/Tests/LibWeb/Layout/expected/svg/svg-fill-with-bogus-url.txt @@ -1,7 +1,7 @@ Viewport <#document> at (0,0) content-size 800x600 children: not-inline - BlockContainer at (0,0) content-size 800x37.835937 [BFC] children: not-inline - BlockContainer at (8,8) content-size 784x21.835937 children: inline - line 0 width: 0, height: 21.835937, bottom: 21.835937, baseline: 100 - frag 0 from SVGSVGBox start: 0, length: 0, rect: [8,8 0x0] - SVGSVGBox at (8,8) content-size 0x0 [SVG] children: not-inline - SVGGeometryBox at (8,8) content-size 100x100 children: not-inline + BlockContainer at (0,0) content-size 800x800 [BFC] children: not-inline + BlockContainer at (8,8) content-size 784x784 children: inline + line 0 width: 784, height: 784, bottom: 784, baseline: 784 + frag 0 from SVGSVGBox start: 0, length: 0, rect: [8,8 784x784] + SVGSVGBox at (8,8) content-size 784x784 [SVG] children: not-inline + SVGGeometryBox at (8,8) content-size 784x784 children: not-inline diff --git a/Tests/LibWeb/Layout/expected/svg/svg-intrinsic-size-in-one-dimension.txt b/Tests/LibWeb/Layout/expected/svg/svg-intrinsic-size-in-one-dimension.txt new file mode 100644 index 0000000000..52f7fb2ba4 --- /dev/null +++ b/Tests/LibWeb/Layout/expected/svg/svg-intrinsic-size-in-one-dimension.txt @@ -0,0 +1,7 @@ +Viewport <#document> at (0,0) content-size 800x600 children: not-inline + BlockContainer at (0,0) content-size 800x170 [BFC] children: not-inline + BlockContainer at (8,8) content-size 784x154 children: not-inline + SVGSVGBox at (9,9) content-size 100x50 [SVG] children: not-inline + SVGGeometryBox at (21.5,21.5) content-size 75x25 children: not-inline + SVGSVGBox at (9,61) content-size 200x100 [SVG] children: not-inline + SVGGeometryBox at (34,86) content-size 150x50 children: not-inline diff --git a/Tests/LibWeb/Layout/expected/svg/svg-with-display-block.txt b/Tests/LibWeb/Layout/expected/svg/svg-with-display-block.txt new file mode 100644 index 0000000000..a1d4c13847 --- /dev/null +++ b/Tests/LibWeb/Layout/expected/svg/svg-with-display-block.txt @@ -0,0 +1,5 @@ +Viewport <#document> at (0,0) content-size 800x600 children: not-inline + BlockContainer at (0,0) content-size 800x800 [BFC] children: not-inline + BlockContainer at (8,8) content-size 784x784 children: not-inline + SVGSVGBox at (8,8) content-size 784x784 [SVG] children: not-inline + SVGGeometryBox at (8,8) content-size 784x784 children: not-inline diff --git a/Tests/LibWeb/Layout/input/svg/svg-intrinsic-size-in-one-dimension.html b/Tests/LibWeb/Layout/input/svg/svg-intrinsic-size-in-one-dimension.html new file mode 100644 index 0000000000..3306cc6af5 --- /dev/null +++ b/Tests/LibWeb/Layout/input/svg/svg-intrinsic-size-in-one-dimension.html @@ -0,0 +1,6 @@ + \ No newline at end of file diff --git a/Tests/LibWeb/Layout/input/svg/svg-with-display-block.html b/Tests/LibWeb/Layout/input/svg/svg-with-display-block.html new file mode 100644 index 0000000000..cc3c1b732b --- /dev/null +++ b/Tests/LibWeb/Layout/input/svg/svg-with-display-block.html @@ -0,0 +1,3 @@ + \ No newline at end of file diff --git a/Userland/Libraries/LibWeb/Layout/BlockFormattingContext.cpp b/Userland/Libraries/LibWeb/Layout/BlockFormattingContext.cpp index 40ff7b255e..6da13110ee 100644 --- a/Userland/Libraries/LibWeb/Layout/BlockFormattingContext.cpp +++ b/Userland/Libraries/LibWeb/Layout/BlockFormattingContext.cpp @@ -577,6 +577,9 @@ void BlockFormattingContext::layout_block_level_box(Box const& box, BlockContain place_block_level_element_in_normal_flow_horizontally(box, available_space); + if (box.is_replaced_box()) + compute_height(box, available_space); + if (independent_formatting_context) { // This box establishes a new formatting context. Pass control to it. independent_formatting_context->run(box, layout_mode, box_state.available_inner_space_or_constraints_from(available_space)); diff --git a/Userland/Libraries/LibWeb/Layout/FormattingContext.cpp b/Userland/Libraries/LibWeb/Layout/FormattingContext.cpp index 11fb9add66..bc36da0fcb 100644 --- a/Userland/Libraries/LibWeb/Layout/FormattingContext.cpp +++ b/Userland/Libraries/LibWeb/Layout/FormattingContext.cpp @@ -398,6 +398,9 @@ CSSPixels FormattingContext::tentative_width_for_replaced_element(LayoutState co // then the used value of 'width' is undefined in CSS 2.2. However, it is suggested that, if the containing block's width does not itself // depend on the replaced element's width, then the used value of 'width' is calculated from the constraint equation used for block-level, // non-replaced elements in normal flow. + if (computed_height.is_auto() && computed_width.is_auto() && !box.has_intrinsic_width() && !box.has_intrinsic_height() && box.has_intrinsic_aspect_ratio()) { + return calculate_stretch_fit_width(box, available_space.width, state); + } // Otherwise, if 'width' has a computed value of 'auto', and the element has an intrinsic width, then that intrinsic width is the used value of 'width'. if (computed_width.is_auto() && box.has_intrinsic_width()) @@ -483,7 +486,7 @@ CSSPixels FormattingContext::tentative_height_for_replaced_element(LayoutState c // // (used width) / (intrinsic ratio) if (computed_height.is_auto() && box.has_intrinsic_aspect_ratio()) - return compute_width_for_replaced_element(state, box, available_space) / box.intrinsic_aspect_ratio().value(); + return state.get(box).content_width() / box.intrinsic_aspect_ratio().value(); // Otherwise, if 'height' has a computed value of 'auto', and the element has an intrinsic height, then that intrinsic height is the used value of 'height'. if (computed_height.is_auto() && box.has_intrinsic_height()) @@ -1412,12 +1415,12 @@ CSSPixels FormattingContext::containing_block_height_for(Box const& box, LayoutS } // https://drafts.csswg.org/css-sizing-3/#stretch-fit-size -CSSPixels FormattingContext::calculate_stretch_fit_width(Box const& box, AvailableSize const& available_width) const +CSSPixels FormattingContext::calculate_stretch_fit_width(Box const& box, AvailableSize const& available_width, LayoutState const& state) { // The size a box would take if its outer size filled the available space in the given axis; // in other words, the stretch fit into the available space, if that is definite. // Undefined if the available space is indefinite. - auto const& box_state = m_state.get(box); + auto const& box_state = state.get(box); return available_width.to_px() - box_state.margin_left - box_state.margin_right @@ -1427,13 +1430,18 @@ CSSPixels FormattingContext::calculate_stretch_fit_width(Box const& box, Availab - box_state.border_right; } +CSSPixels FormattingContext::calculate_stretch_fit_width(Box const& box, AvailableSize const& available_width) const +{ + return calculate_stretch_fit_width(box, available_width, m_state); +} + // https://drafts.csswg.org/css-sizing-3/#stretch-fit-size -CSSPixels FormattingContext::calculate_stretch_fit_height(Box const& box, AvailableSize const& available_height) const +CSSPixels FormattingContext::calculate_stretch_fit_height(Box const& box, AvailableSize const& available_height, LayoutState const& state) { // The size a box would take if its outer size filled the available space in the given axis; // in other words, the stretch fit into the available space, if that is definite. // Undefined if the available space is indefinite. - auto const& box_state = m_state.get(box); + auto const& box_state = state.get(box); return available_height.to_px() - box_state.margin_top - box_state.margin_bottom @@ -1443,6 +1451,11 @@ CSSPixels FormattingContext::calculate_stretch_fit_height(Box const& box, Availa - box_state.border_bottom; } +CSSPixels FormattingContext::calculate_stretch_fit_height(Box const& box, AvailableSize const& available_height) const +{ + return calculate_stretch_fit_height(box, available_height, m_state); +} + bool FormattingContext::should_treat_width_as_auto(Box const& box, AvailableSpace const& available_space) { return box.computed_values().width().is_auto() diff --git a/Userland/Libraries/LibWeb/Layout/FormattingContext.h b/Userland/Libraries/LibWeb/Layout/FormattingContext.h index c203db9479..9762b1263a 100644 --- a/Userland/Libraries/LibWeb/Layout/FormattingContext.h +++ b/Userland/Libraries/LibWeb/Layout/FormattingContext.h @@ -79,6 +79,9 @@ public: [[nodiscard]] CSSPixels calculate_stretch_fit_width(Box const&, AvailableSize const&) const; [[nodiscard]] CSSPixels calculate_stretch_fit_height(Box const&, AvailableSize const&) const; + [[nodiscard]] static CSSPixels calculate_stretch_fit_width(Box const&, AvailableSize const&, LayoutState const&); + [[nodiscard]] static CSSPixels calculate_stretch_fit_height(Box const&, AvailableSize const&, LayoutState const&); + virtual bool can_determine_size_of_child() const { return false; } virtual void determine_width_of_child(Box const&, AvailableSpace const&) { } virtual void determine_height_of_child(Box const&, AvailableSpace const&) { } diff --git a/Userland/Libraries/LibWeb/Layout/InlineFormattingContext.cpp b/Userland/Libraries/LibWeb/Layout/InlineFormattingContext.cpp index c21cbe12f9..7ad5f2a4ff 100644 --- a/Userland/Libraries/LibWeb/Layout/InlineFormattingContext.cpp +++ b/Userland/Libraries/LibWeb/Layout/InlineFormattingContext.cpp @@ -113,11 +113,11 @@ void InlineFormattingContext::dimension_box_on_line(Box const& box, LayoutMode l if (is(box)) { auto& replaced = verify_cast(box); - if (is(box)) - (void)layout_inside(replaced, layout_mode, box_state.available_inner_space_or_constraints_from(*m_available_space)); - box_state.set_content_width(compute_width_for_replaced_element(m_state, replaced, *m_available_space)); box_state.set_content_height(compute_height_for_replaced_element(m_state, replaced, *m_available_space)); + + if (is(box)) + (void)layout_inside(replaced, layout_mode, box_state.available_inner_space_or_constraints_from(*m_available_space)); return; } diff --git a/Userland/Libraries/LibWeb/Layout/SVGSVGBox.cpp b/Userland/Libraries/LibWeb/Layout/SVGSVGBox.cpp index f42e5c1237..72875a85f5 100644 --- a/Userland/Libraries/LibWeb/Layout/SVGSVGBox.cpp +++ b/Userland/Libraries/LibWeb/Layout/SVGSVGBox.cpp @@ -24,70 +24,60 @@ JS::GCPtr SVGSVGBox::create_paintable() const void SVGSVGBox::prepare_for_replaced_layout() { - if (dom_node().has_attribute(HTML::AttributeNames::width) && dom_node().has_attribute(HTML::AttributeNames::height)) { - Optional w; - Optional h; - auto parsing_context = CSS::Parser::ParsingContext { document() }; - auto width = parse_css_value(parsing_context, dom_node().attribute(Web::HTML::AttributeNames::width), CSS::PropertyID::Width).release_value_but_fixme_should_propagate_errors(); - if (!width.is_null() && width->has_length()) - w = width->to_length().to_px(*this); + // https://www.w3.org/TR/SVG2/coords.html#SizingSVGInCSS - auto height = parse_css_value(parsing_context, dom_node().attribute((HTML::AttributeNames::height)), CSS::PropertyID::Height).release_value_but_fixme_should_propagate_errors(); - if (!height.is_null() && height->has_length()) - h = height->to_length().to_px(*this); - if (w.has_value() && h.has_value()) { - set_intrinsic_width(*w); - set_intrinsic_height(*h); - set_intrinsic_aspect_ratio(w->value() / h->value()); - return; - } + // The intrinsic dimensions must also be determined from the width and height sizing properties. + // If either width or height are not specified, the used value is the initial value 'auto'. + // 'auto' and percentage lengths must not be used to determine an intrinsic width or intrinsic height. + auto const& computed_width = computed_values().width(); + if (computed_width.is_length() && !computed_width.contains_percentage()) { + set_intrinsic_width(computed_width.to_px(*this, 0)); } - Optional united_rect; - - auto add_to_united_rect = [&](CSSPixelRect const& rect) { - if (united_rect.has_value()) - united_rect = united_rect->united(rect); - else - united_rect = rect; - }; - - for_each_in_subtree_of_type([&](SVGGeometryBox const& geometry_box) { - auto& dom_node = const_cast(geometry_box.dom_node()); - if (dom_node.has_attribute(HTML::AttributeNames::width) && dom_node.has_attribute(HTML::AttributeNames::height)) { - CSSPixelRect rect; - // FIXME: Allow for relative lengths here - rect.set_width(computed_values().width().to_px(*this, 0)); - rect.set_height(computed_values().height().to_px(*this, 0)); - add_to_united_rect(rect); - return IterationDecision::Continue; - } - - auto& path = dom_node.get_path(); - auto path_bounding_box = path.bounding_box().to_type(); - - // Stroke increases the path's size by stroke_width/2 per side. - auto stroke_width = geometry_box.dom_node().stroke_width().value_or(0); - path_bounding_box.inflate(stroke_width, stroke_width); - - auto& maybe_view_box = this->dom_node().view_box(); - - if (maybe_view_box.has_value()) { - auto view_box = maybe_view_box.value(); - CSSPixelRect rect(view_box.min_x, view_box.min_y, view_box.width, view_box.height); - add_to_united_rect(rect); - return IterationDecision::Continue; - } - - add_to_united_rect(path_bounding_box); - return IterationDecision::Continue; - }); - - if (united_rect.has_value()) { - set_intrinsic_width(united_rect->width()); - set_intrinsic_height(united_rect->height()); - set_intrinsic_aspect_ratio(united_rect->width().value() / united_rect->height().value()); + auto const& computed_height = computed_values().height(); + if (computed_height.is_length() && !computed_height.contains_percentage()) { + set_intrinsic_height(computed_height.to_px(*this, 0)); } + + set_intrinsic_aspect_ratio(calculate_intrinsic_aspect_ratio()); +} + +Optional SVGSVGBox::calculate_intrinsic_aspect_ratio() const +{ + // https://www.w3.org/TR/SVG2/coords.html#SizingSVGInCSS + // The intrinsic aspect ratio must be calculated using the following algorithm. If the algorithm returns null, then there is no intrinsic aspect ratio. + + auto const& computed_width = computed_values().width(); + auto const& computed_height = computed_values().height(); + + // 1. If the width and height sizing properties on the ‘svg’ element are both absolute values: + if (computed_width.is_length() && !computed_width.contains_percentage() && computed_height.is_length() && !computed_height.contains_percentage()) { + auto width = computed_width.to_px(*this, 0); + auto height = computed_height.to_px(*this, 0); + + if (width != 0 && height != 0) { + // 1. return width / height + return width.value() / height.value(); + } + + return {}; + } + + // FIXME: 2. If an SVG View is active: + // FIXME: 1. let viewbox be the viewbox defined by the active SVG View + // FIXME: 2. return viewbox.width / viewbox.height + + // 3. If the ‘viewBox’ on the ‘svg’ element is correctly specified: + if (dom_node().view_box().has_value()) { + // 1. let viewbox be the viewbox defined by the ‘viewBox’ attribute on the ‘svg’ element + auto const& viewbox = dom_node().view_box().value(); + + // 2. return viewbox.width / viewbox.height + return viewbox.width / viewbox.height; + } + + // 4. return null + return {}; } } diff --git a/Userland/Libraries/LibWeb/Layout/SVGSVGBox.h b/Userland/Libraries/LibWeb/Layout/SVGSVGBox.h index 4e91029664..45bdffc31b 100644 --- a/Userland/Libraries/LibWeb/Layout/SVGSVGBox.h +++ b/Userland/Libraries/LibWeb/Layout/SVGSVGBox.h @@ -19,6 +19,7 @@ public: virtual ~SVGSVGBox() override = default; SVG::SVGSVGElement& dom_node() { return verify_cast(ReplacedBox::dom_node()); } + SVG::SVGSVGElement const& dom_node() const { return verify_cast(ReplacedBox::dom_node()); } virtual bool can_have_children() const override { return true; } @@ -28,6 +29,8 @@ public: private: virtual bool is_svg_svg_box() const final { return true; } + + [[nodiscard]] Optional calculate_intrinsic_aspect_ratio() const; }; template<>