From 1e0ea45fe549c3429b84ca8c9dfdf0d88019152e Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Sat, 19 Aug 2023 19:42:31 +0200 Subject: [PATCH] LibWeb: Make sure that SVG use and symbol elements get paintables I'm about to make StackingContext traverse the paintable tree instead of actually traversing the layout tree, and it turns out we were not creating paintables for these SVG elements. Also switch them to Layout::Box instead of the default InlineNode to make the trees look a bit less weird. Ultimately, we should do something specialized for these subtrees, but for now this'll do. --- .../expected/svg/svg-symbol-with-viewbox.txt | 7 ++-- .../LibWeb/Layout/SVGFormattingContext.cpp | 35 +++++++++++++------ .../Libraries/LibWeb/SVG/SVGSymbolElement.cpp | 6 ++++ .../Libraries/LibWeb/SVG/SVGSymbolElement.h | 2 ++ .../Libraries/LibWeb/SVG/SVGUseElement.cpp | 6 ++++ Userland/Libraries/LibWeb/SVG/SVGUseElement.h | 2 ++ 6 files changed, 45 insertions(+), 13 deletions(-) diff --git a/Tests/LibWeb/Layout/expected/svg/svg-symbol-with-viewbox.txt b/Tests/LibWeb/Layout/expected/svg/svg-symbol-with-viewbox.txt index 47d12d6f20..4b256824a7 100644 --- a/Tests/LibWeb/Layout/expected/svg/svg-symbol-with-viewbox.txt +++ b/Tests/LibWeb/Layout/expected/svg/svg-symbol-with-viewbox.txt @@ -10,8 +10,8 @@ Viewport <#document> at (0,0) content-size 800x600 children: not-inline TextNode <#text> SVGSVGBox at (8,8) content-size 300x150 [SVG] children: inline TextNode <#text> - InlineNode - InlineNode + Box at (8,8) content-size 0x0 children: inline + Box at (8,8) content-size 0x0 children: inline TextNode <#text> SVGGeometryBox at (92.375,26.75) content-size 131.25x112.15625 children: inline TextNode <#text> @@ -25,3 +25,6 @@ ViewportPaintable (Viewport<#document>) [0,0 800x600] PaintableWithLines (BlockContainer(anonymous)) [8,8 784x0] PaintableWithLines (BlockContainer
) [8,8 784x150] SVGSVGPaintable (SVGSVGBox) [8,8 300x150] + PaintableBox (Box) [8,8 0x0] + PaintableBox (Box#braces) [8,8 0x0] + SVGGeometryPaintable (SVGGeometryBox) [92.375,26.75 131.25x112.15625] diff --git a/Userland/Libraries/LibWeb/Layout/SVGFormattingContext.cpp b/Userland/Libraries/LibWeb/Layout/SVGFormattingContext.cpp index 539b7e75f5..cd1432df6c 100644 --- a/Userland/Libraries/LibWeb/Layout/SVGFormattingContext.cpp +++ b/Userland/Libraries/LibWeb/Layout/SVGFormattingContext.cpp @@ -15,6 +15,8 @@ #include #include #include +#include +#include namespace Web::Layout { @@ -131,6 +133,21 @@ static ViewBoxTransform scale_and_align_viewbox_content(SVG::PreserveAspectRatio return viewbox_transform; } +static bool should_ensure_creation_of_paintable(Node const& node) +{ + if (is(node)) + return true; + if (is(node)) + return true; + if (node.dom_node()) { + if (is(*node.dom_node())) + return true; + if (is(*node.dom_node())) + return true; + } + return false; +} + void SVGFormattingContext::run(Box const& box, LayoutMode layout_mode, AvailableSpace const& available_space) { auto& svg_svg_element = verify_cast(*box.dom_node()); @@ -149,7 +166,7 @@ void SVGFormattingContext::run(Box const& box, LayoutMode layout_mode, Available return IterationDecision::Continue; }); - box.for_each_in_subtree_of_type([&](Box const& descendant) { + box.for_each_in_subtree([&](Node const& descendant) { if (is(descendant)) { auto const& geometry_box = static_cast(descendant); auto& geometry_box_state = m_state.get_mutable(geometry_box); @@ -186,16 +203,12 @@ void SVGFormattingContext::run(Box const& box, LayoutMode layout_mode, Available geometry_box_state.set_content_width(path_bounding_box.width()); geometry_box_state.set_content_height(path_bounding_box.height()); } else if (is(descendant)) { - SVGFormattingContext nested_context(m_state, descendant, this); - nested_context.run(descendant, layout_mode, available_space); - } else if (is(descendant)) { - auto const& svg_text_box = static_cast(descendant); - // NOTE: This hack creates a layout state to ensure the existence of a paintable box node in LayoutState::commit(), even when none of the values from UsedValues impact the SVG text. - m_state.get_mutable(svg_text_box); - } else if (is(descendant)) { - // Same hack as above. - auto const& svg_graphics_box = static_cast(descendant); - m_state.get_mutable(svg_graphics_box); + SVGFormattingContext nested_context(m_state, static_cast(descendant), this); + nested_context.run(static_cast(descendant), layout_mode, available_space); + } else if (should_ensure_creation_of_paintable(descendant)) { + // NOTE: This hack creates a layout state to ensure the existence of + // a paintable in LayoutState::commit(). + m_state.get_mutable(static_cast(descendant)); } return IterationDecision::Continue; diff --git a/Userland/Libraries/LibWeb/SVG/SVGSymbolElement.cpp b/Userland/Libraries/LibWeb/SVG/SVGSymbolElement.cpp index c7273425eb..e98bd5d31d 100644 --- a/Userland/Libraries/LibWeb/SVG/SVGSymbolElement.cpp +++ b/Userland/Libraries/LibWeb/SVG/SVGSymbolElement.cpp @@ -10,6 +10,7 @@ #include #include #include +#include #include #include #include @@ -62,4 +63,9 @@ bool SVGSymbolElement::is_direct_child_of_use_shadow_tree() const return is(host); } +JS::GCPtr SVGSymbolElement::create_layout_node(NonnullRefPtr style) +{ + return heap().allocate_without_realm(document(), this, move(style)); +} + } diff --git a/Userland/Libraries/LibWeb/SVG/SVGSymbolElement.h b/Userland/Libraries/LibWeb/SVG/SVGSymbolElement.h index 481ca2b5ba..7ce10c9847 100644 --- a/Userland/Libraries/LibWeb/SVG/SVGSymbolElement.h +++ b/Userland/Libraries/LibWeb/SVG/SVGSymbolElement.h @@ -25,6 +25,8 @@ private: virtual void initialize(JS::Realm&) override; + virtual JS::GCPtr create_layout_node(NonnullRefPtr) override; + bool is_direct_child_of_use_shadow_tree() const; virtual void attribute_changed(DeprecatedFlyString const& name, DeprecatedString const& value) override; diff --git a/Userland/Libraries/LibWeb/SVG/SVGUseElement.cpp b/Userland/Libraries/LibWeb/SVG/SVGUseElement.cpp index 947db7bb1a..49dce642b1 100644 --- a/Userland/Libraries/LibWeb/SVG/SVGUseElement.cpp +++ b/Userland/Libraries/LibWeb/SVG/SVGUseElement.cpp @@ -9,6 +9,7 @@ #include #include #include +#include #include #include #include @@ -172,4 +173,9 @@ JS::GCPtr SVGUseElement::animated_instance_root() const return instance_root(); } +JS::GCPtr SVGUseElement::create_layout_node(NonnullRefPtr style) +{ + return heap().allocate_without_realm(document(), this, move(style)); +} + } diff --git a/Userland/Libraries/LibWeb/SVG/SVGUseElement.h b/Userland/Libraries/LibWeb/SVG/SVGUseElement.h index 55891fecf3..496da7b3fb 100644 --- a/Userland/Libraries/LibWeb/SVG/SVGUseElement.h +++ b/Userland/Libraries/LibWeb/SVG/SVGUseElement.h @@ -42,6 +42,8 @@ private: virtual bool is_svg_use_element() const override { return true; } + virtual JS::GCPtr create_layout_node(NonnullRefPtr) override; + Optional parse_id_from_href(DeprecatedString const& href); JS::GCPtr referenced_element();