From 3484db0dc15107753c6e91d7b33f12b50976068c Mon Sep 17 00:00:00 2001 From: MacDue Date: Mon, 10 Apr 2023 12:28:55 +0100 Subject: [PATCH] LibWeb: Remove SVG sizing hack and fix viewbox scaling Previously, if you had an SVG with a viewbox and a definite width and height, then all SVGGeometryBox boxes within that SVG would have a width and height set to the size of the parent SVG. This broke hit testing for SVG paths, and didn't make much sense. It seems like the SVG sizing hack was patching over the incorrect logic in viewbox_scaling() and the incorrect path sizing (which was never reached). Before this change the view box scaling was: element_dimension / viewbox_dimension Which only seemed to work because of the SVG sizing hack that made all paths the size of the containing SVG. After this change SVGGeometryBoxes are (in most cases) sized correctly based on their bounding boxes, which allows hit testing to function, and the view box scaling is updated now to: containing_SVG_dimension / viewbox_dimension Which works with one less hack :^) This now also handles centering the viewbox within the parent SVG element and applying any tranforms to the bounding box. This still a bit ad-hoc, but much more closely matches other browsers now. --- .../LibWeb/Layout/SVGFormattingContext.cpp | 55 +++++++++---------- 1 file changed, 25 insertions(+), 30 deletions(-) diff --git a/Userland/Libraries/LibWeb/Layout/SVGFormattingContext.cpp b/Userland/Libraries/LibWeb/Layout/SVGFormattingContext.cpp index 4219f449dc..de3e4c493d 100644 --- a/Userland/Libraries/LibWeb/Layout/SVGFormattingContext.cpp +++ b/Userland/Libraries/LibWeb/Layout/SVGFormattingContext.cpp @@ -2,6 +2,7 @@ * Copyright (c) 2021, Andreas Kling * Copyright (c) 2022, Sam Atkins * Copyright (c) 2022, Tobias Christiansen + * Copyright (c) 2023, MacDue * * SPDX-License-Identifier: BSD-2-Clause */ @@ -37,7 +38,8 @@ void SVGFormattingContext::run(Box const& box, LayoutMode, [[maybe_unused]] Avai auto& svg_svg_element = verify_cast(*box.dom_node()); - auto root_offset = m_state.get(box).offset; + auto svg_box_state = m_state.get(box); + auto root_offset = svg_box_state.offset; box.for_each_child_of_type([&](BlockContainer const& child_box) { if (is(child_box.dom_node())) { @@ -53,48 +55,41 @@ void SVGFormattingContext::run(Box const& box, LayoutMode, [[maybe_unused]] Avai box.for_each_in_subtree_of_type([&](SVGBox const& descendant) { if (is(descendant)) { auto const& geometry_box = static_cast(descendant); - auto& geometry_box_state = m_state.get_mutable(geometry_box); - auto& dom_node = const_cast(geometry_box).dom_node(); - auto& svg_svg_state = m_state.get(static_cast(*svg_svg_element.layout_node())); - - if (svg_svg_state.has_definite_width() && svg_svg_state.has_definite_height()) { - geometry_box_state.set_content_offset({ 0, 0 }); - geometry_box_state.set_content_width(svg_svg_state.content_width()); - geometry_box_state.set_content_height(svg_svg_state.content_height()); - return IterationDecision::Continue; - } - - // FIXME: Allow for one of {width, height} to not be specified} - if (svg_svg_element.has_attribute(HTML::AttributeNames::width)) { - } - - if (svg_svg_element.has_attribute(HTML::AttributeNames::height)) { - } - 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. - CSSPixels stroke_width = geometry_box.dom_node().stroke_width().value_or(0); - path_bounding_box.inflate(stroke_width, stroke_width); + auto transform = dom_node.get_transform(); auto& maybe_view_box = svg_svg_element.view_box(); + float viewbox_scale = 1.0f; + CSSPixelPoint offset {}; if (maybe_view_box.has_value()) { auto view_box = maybe_view_box.value(); - CSSPixelPoint viewbox_offset = { view_box.min_x, view_box.min_y }; - geometry_box_state.set_content_offset(path_bounding_box.top_left() + viewbox_offset); + // FIXME: This should allow just one of width or height to be specified. + // E.g. We should be able to layout where height is unspecified/auto. + if (!svg_box_state.has_definite_width() || !svg_box_state.has_definite_height()) { + dbgln("FIXME: Attempting to layout indefinitely sized SVG with a viewbox -- this likely won't work!"); + } + auto scale_width = svg_box_state.has_definite_width() ? svg_box_state.content_width().value() / view_box.width : 1; + auto scale_height = svg_box_state.has_definite_height() ? svg_box_state.content_height().value() / view_box.height : 1; + viewbox_scale = min(scale_width, scale_height); - geometry_box_state.set_content_width(view_box.width); - geometry_box_state.set_content_height(view_box.height); + // Center the viewbox within the SVG element: + if (svg_box_state.has_definite_width()) + offset.translate_by((svg_box_state.content_width() - (view_box.width * viewbox_scale)) / 2, 0); + if (svg_box_state.has_definite_height()) + offset.translate_by(0, (svg_box_state.content_height() - (view_box.height * viewbox_scale)) / 2); - return IterationDecision::Continue; + transform = Gfx::AffineTransform {}.scale(viewbox_scale, viewbox_scale).translate({ -view_box.min_x, -view_box.min_y }).multiply(transform); } - geometry_box_state.set_content_offset(path_bounding_box.top_left()); + // Stroke increases the path's size by stroke_width/2 per side. + auto path_bounding_box = transform.map(path.bounding_box()).to_type(); + CSSPixels stroke_width = geometry_box.dom_node().stroke_width().value_or(0); + path_bounding_box.inflate(stroke_width, stroke_width); + geometry_box_state.set_content_offset(path_bounding_box.top_left() + offset); geometry_box_state.set_content_width(path_bounding_box.width()); geometry_box_state.set_content_height(path_bounding_box.height()); }