From 7d26383426086745993152db452174cabf9d2caa Mon Sep 17 00:00:00 2001 From: MacDue Date: Sat, 1 Jul 2023 21:24:04 +0100 Subject: [PATCH] LibWeb: Remove SVGContext The SVGContext is a leftover from when SVG properties were more ad-hoc. All properties are now (for better or worse) treated as CSS properties (or handled elsewhere). This makes the SVGContext's fill/stroke inheritance handling unnecessary. --- .../LibWeb/Painting/PaintContext.cpp | 18 ------ .../Libraries/LibWeb/Painting/PaintContext.h | 8 --- .../LibWeb/Painting/SVGGeometryPaintable.cpp | 26 ++++---- .../LibWeb/Painting/SVGGraphicsPaintable.cpp | 22 ------- .../LibWeb/Painting/SVGGraphicsPaintable.h | 2 - .../LibWeb/Painting/SVGPaintable.cpp | 16 ----- .../Libraries/LibWeb/Painting/SVGPaintable.h | 3 - .../LibWeb/Painting/SVGSVGPaintable.cpp | 4 -- .../LibWeb/Painting/SVGTextPaintable.cpp | 7 ++- Userland/Libraries/LibWeb/SVG/SVGContext.h | 61 ------------------- .../LibWeb/SVG/SVGGraphicsElement.cpp | 1 - 11 files changed, 18 insertions(+), 150 deletions(-) delete mode 100644 Userland/Libraries/LibWeb/SVG/SVGContext.h diff --git a/Userland/Libraries/LibWeb/Painting/PaintContext.cpp b/Userland/Libraries/LibWeb/Painting/PaintContext.cpp index c1048afd8b5..f602166248b 100644 --- a/Userland/Libraries/LibWeb/Painting/PaintContext.cpp +++ b/Userland/Libraries/LibWeb/Painting/PaintContext.cpp @@ -17,24 +17,6 @@ PaintContext::PaintContext(Gfx::Painter& painter, Palette const& palette, double { } -SVGContext& PaintContext::svg_context() -{ - // FIXME: This is a total hack to avoid crashing on content that has SVG elements embedded directly in HTML without an element. - if (!m_svg_context.has_value()) - m_svg_context = SVGContext { {} }; - return m_svg_context.value(); -} - -void PaintContext::set_svg_context(SVGContext context) -{ - m_svg_context = move(context); -} - -void PaintContext::clear_svg_context() -{ - m_svg_context.clear(); -} - CSSPixelRect PaintContext::css_viewport_rect() const { return { diff --git a/Userland/Libraries/LibWeb/Painting/PaintContext.h b/Userland/Libraries/LibWeb/Painting/PaintContext.h index 0f590eefa60..a5326fbb9b6 100644 --- a/Userland/Libraries/LibWeb/Painting/PaintContext.h +++ b/Userland/Libraries/LibWeb/Painting/PaintContext.h @@ -12,7 +12,6 @@ #include #include #include -#include namespace Web { @@ -23,11 +22,6 @@ public: Gfx::Painter& painter() const { return m_painter; } Palette const& palette() const { return m_palette; } - bool has_svg_context() const { return m_svg_context.has_value(); } - SVGContext& svg_context(); - void set_svg_context(SVGContext); - void clear_svg_context(); - bool should_show_line_box_borders() const { return m_should_show_line_box_borders; } void set_should_show_line_box_borders(bool value) { m_should_show_line_box_borders = value; } @@ -60,7 +54,6 @@ public: clone.m_device_viewport_rect = m_device_viewport_rect; clone.m_should_show_line_box_borders = m_should_show_line_box_borders; clone.m_focus = m_focus; - clone.m_svg_context = m_svg_context; return clone; } @@ -69,7 +62,6 @@ public: private: Gfx::Painter& m_painter; Palette m_palette; - Optional m_svg_context; double m_device_pixels_per_css_pixel { 0 }; DevicePixelRect m_device_viewport_rect; bool m_should_show_line_box_borders { false }; diff --git a/Userland/Libraries/LibWeb/Painting/SVGGeometryPaintable.cpp b/Userland/Libraries/LibWeb/Painting/SVGGeometryPaintable.cpp index a50dafb57b6..77bf7a1e637 100644 --- a/Userland/Libraries/LibWeb/Painting/SVGGeometryPaintable.cpp +++ b/Userland/Libraries/LibWeb/Painting/SVGGeometryPaintable.cpp @@ -66,15 +66,16 @@ void SVGGeometryPaintable::paint(PaintContext& context, PaintPhase phase) const auto& geometry_element = layout_box().dom_node(); + auto const* svg_element = geometry_element.shadow_including_first_ancestor_of_type(); + auto svg_element_rect = svg_element->paintable_box()->absolute_rect(); + Gfx::AntiAliasingPainter painter { context.painter() }; - auto& svg_context = context.svg_context(); // FIXME: This should not be trucated to an int. Gfx::PainterStateSaver save_painter { context.painter() }; - auto offset = context.floored_device_point(svg_context.svg_element_position()).to_type().to_type(); + auto offset = context.floored_device_point(svg_element_rect.location()).to_type().to_type(); painter.translate(offset); - auto const* svg_element = geometry_element.shadow_including_first_ancestor_of_type(); auto maybe_view_box = svg_element->view_box(); auto transform = layout_box().layout_transform(); @@ -102,7 +103,7 @@ void SVGGeometryPaintable::paint(PaintContext& context, PaintPhase phase) const auto svg_viewport = [&] { if (maybe_view_box.has_value()) return Gfx::FloatRect { maybe_view_box->min_x, maybe_view_box->min_y, maybe_view_box->width, maybe_view_box->height }; - return Gfx::FloatRect { { 0, 0 }, svg_context.svg_element_size().to_type().to_type() }; + return Gfx::FloatRect { { 0, 0 }, svg_element_rect.size().to_type().to_type() }; }(); SVG::SVGPaintContext paint_context { @@ -111,26 +112,25 @@ void SVGGeometryPaintable::paint(PaintContext& context, PaintPhase phase) const .transform = paint_transform }; - auto fill_opacity = geometry_element.fill_opacity().value_or(svg_context.fill_opacity()); - auto winding_rule = to_gfx_winding_rule(geometry_element.fill_rule().value_or(svg_context.fill_rule())); - + auto fill_opacity = geometry_element.fill_opacity().value_or(1); + auto winding_rule = to_gfx_winding_rule(geometry_element.fill_rule().value_or(SVG::FillRule::Nonzero)); if (auto paint_style = geometry_element.fill_paint_style(paint_context); paint_style.has_value()) { painter.fill_path( closed_path(), *paint_style, fill_opacity, winding_rule); - } else if (auto fill_color = geometry_element.fill_color().value_or(svg_context.fill_color()).with_opacity(fill_opacity); fill_color.alpha() > 0) { + } else if (auto fill_color = geometry_element.fill_color(); fill_color.has_value()) { painter.fill_path( closed_path(), - fill_color, + fill_color->with_opacity(fill_opacity), winding_rule); } - auto stroke_opacity = geometry_element.stroke_opacity().value_or(svg_context.stroke_opacity()); + auto stroke_opacity = geometry_element.stroke_opacity().value_or(1); // Note: This is assuming .x_scale() == .y_scale() (which it does currently). - float stroke_thickness = geometry_element.stroke_width().value_or(svg_context.stroke_width()) * viewbox_scale; + float stroke_thickness = geometry_element.stroke_width().value_or(1) * viewbox_scale; if (auto paint_style = geometry_element.stroke_paint_style(paint_context); paint_style.has_value()) { painter.stroke_path( @@ -138,10 +138,10 @@ void SVGGeometryPaintable::paint(PaintContext& context, PaintPhase phase) const *paint_style, stroke_thickness, stroke_opacity); - } else if (auto stroke_color = geometry_element.stroke_color().value_or(svg_context.stroke_color()).with_opacity(stroke_opacity); stroke_color.alpha() > 0) { + } else if (auto stroke_color = geometry_element.stroke_color(); stroke_color.has_value()) { painter.stroke_path( path, - stroke_color, + stroke_color->with_opacity(stroke_opacity), stroke_thickness); } } diff --git a/Userland/Libraries/LibWeb/Painting/SVGGraphicsPaintable.cpp b/Userland/Libraries/LibWeb/Painting/SVGGraphicsPaintable.cpp index 9af4b164982..0e30c14ad13 100644 --- a/Userland/Libraries/LibWeb/Painting/SVGGraphicsPaintable.cpp +++ b/Userland/Libraries/LibWeb/Painting/SVGGraphicsPaintable.cpp @@ -24,26 +24,4 @@ Layout::SVGGraphicsBox const& SVGGraphicsPaintable::layout_box() const return static_cast(layout_node()); } -void SVGGraphicsPaintable::before_children_paint(PaintContext& context, PaintPhase phase) const -{ - SVGPaintable::before_children_paint(context, phase); - if (phase != PaintPhase::Foreground) - return; - - auto& graphics_element = layout_box().dom_node(); - - if (auto fill_rule = graphics_element.fill_rule(); fill_rule.has_value()) - context.svg_context().set_fill_rule(*fill_rule); - if (auto fill_color = graphics_element.fill_color(); fill_color.has_value()) - context.svg_context().set_fill_color(*fill_color); - if (auto stroke_color = graphics_element.stroke_color(); stroke_color.has_value()) - context.svg_context().set_stroke_color(*stroke_color); - if (auto stroke_width = graphics_element.stroke_width(); stroke_width.has_value()) - context.svg_context().set_stroke_width(*stroke_width); - if (auto fill_opacity = graphics_element.fill_opacity(); fill_opacity.has_value()) - context.svg_context().set_fill_opacity(*fill_opacity); - if (auto stroke_opacity = graphics_element.stroke_opacity(); stroke_opacity.has_value()) - context.svg_context().set_stroke_opacity(*stroke_opacity); -} - } diff --git a/Userland/Libraries/LibWeb/Painting/SVGGraphicsPaintable.h b/Userland/Libraries/LibWeb/Painting/SVGGraphicsPaintable.h index ee811b7c522..5366a902656 100644 --- a/Userland/Libraries/LibWeb/Painting/SVGGraphicsPaintable.h +++ b/Userland/Libraries/LibWeb/Painting/SVGGraphicsPaintable.h @@ -17,8 +17,6 @@ class SVGGraphicsPaintable : public SVGPaintable { public: static JS::NonnullGCPtr create(Layout::SVGGraphicsBox const&); - virtual void before_children_paint(PaintContext&, PaintPhase) const override; - Layout::SVGGraphicsBox const& layout_box() const; protected: diff --git a/Userland/Libraries/LibWeb/Painting/SVGPaintable.cpp b/Userland/Libraries/LibWeb/Painting/SVGPaintable.cpp index 775028fb8f3..51d9a1c6746 100644 --- a/Userland/Libraries/LibWeb/Painting/SVGPaintable.cpp +++ b/Userland/Libraries/LibWeb/Painting/SVGPaintable.cpp @@ -20,22 +20,6 @@ Layout::SVGBox const& SVGPaintable::layout_box() const return static_cast(layout_node()); } -void SVGPaintable::before_children_paint(PaintContext& context, PaintPhase phase) const -{ - PaintableBox::before_children_paint(context, phase); - if (phase != PaintPhase::Foreground) - return; - context.svg_context().save(); -} - -void SVGPaintable::after_children_paint(PaintContext& context, PaintPhase phase) const -{ - PaintableBox::after_children_paint(context, phase); - if (phase != PaintPhase::Foreground) - return; - context.svg_context().restore(); -} - CSSPixelRect SVGPaintable::compute_absolute_rect() const { if (auto* svg_svg_box = layout_box().first_ancestor_of_type()) { diff --git a/Userland/Libraries/LibWeb/Painting/SVGPaintable.h b/Userland/Libraries/LibWeb/Painting/SVGPaintable.h index f634b850305..de9e26c9b68 100644 --- a/Userland/Libraries/LibWeb/Painting/SVGPaintable.h +++ b/Userland/Libraries/LibWeb/Painting/SVGPaintable.h @@ -15,9 +15,6 @@ class SVGPaintable : public PaintableBox { JS_CELL(SVGPaintable, PaintableBox); public: - virtual void before_children_paint(PaintContext&, PaintPhase) const override; - virtual void after_children_paint(PaintContext&, PaintPhase) const override; - Layout::SVGBox const& layout_box() const; protected: diff --git a/Userland/Libraries/LibWeb/Painting/SVGSVGPaintable.cpp b/Userland/Libraries/LibWeb/Painting/SVGSVGPaintable.cpp index 0f4135cad6d..461991d119a 100644 --- a/Userland/Libraries/LibWeb/Painting/SVGSVGPaintable.cpp +++ b/Userland/Libraries/LibWeb/Painting/SVGSVGPaintable.cpp @@ -30,9 +30,6 @@ void SVGSVGPaintable::before_children_paint(PaintContext& context, PaintPhase ph if (phase != PaintPhase::Foreground) return; - if (!context.has_svg_context()) - context.set_svg_context(SVGContext(absolute_rect())); - context.painter().save(); context.painter().add_clip_rect(context.enclosing_device_rect(absolute_rect()).to_type()); } @@ -44,7 +41,6 @@ void SVGSVGPaintable::after_children_paint(PaintContext& context, PaintPhase pha return; context.painter().restore(); - context.clear_svg_context(); } } diff --git a/Userland/Libraries/LibWeb/Painting/SVGTextPaintable.cpp b/Userland/Libraries/LibWeb/Painting/SVGTextPaintable.cpp index 32fd1fcd144..51a3db79559 100644 --- a/Userland/Libraries/LibWeb/Painting/SVGTextPaintable.cpp +++ b/Userland/Libraries/LibWeb/Painting/SVGTextPaintable.cpp @@ -38,9 +38,12 @@ void SVGTextPaintable::paint(PaintContext& context, PaintPhase phase) const auto& painter = context.painter(); + auto& text_element = layout_box().dom_node(); + auto const* svg_element = text_element.shadow_including_first_ancestor_of_type(); + auto svg_element_rect = svg_element->paintable_box()->absolute_rect(); + Gfx::PainterStateSaver save_painter { painter }; - auto& svg_context = context.svg_context(); - auto svg_context_offset = context.floored_device_point(svg_context.svg_element_position()).to_type(); + auto svg_context_offset = context.floored_device_point(svg_element_rect.location()).to_type(); painter.translate(svg_context_offset); auto const& dom_node = layout_box().dom_node(); diff --git a/Userland/Libraries/LibWeb/SVG/SVGContext.h b/Userland/Libraries/LibWeb/SVG/SVGContext.h deleted file mode 100644 index 2dfd307ede5..00000000000 --- a/Userland/Libraries/LibWeb/SVG/SVGContext.h +++ /dev/null @@ -1,61 +0,0 @@ -/* - * Copyright (c) 2020, Matthew Olsson - * - * SPDX-License-Identifier: BSD-2-Clause - */ - -#pragma once - -#include -#include -#include -#include - -namespace Web { - -class SVGContext { -public: - SVGContext(CSSPixelRect svg_element_bounds) - : m_svg_element_bounds(svg_element_bounds) - { - m_states.append(State()); - } - - SVG::FillRule fill_rule() const { return state().fill_rule; } - Gfx::Color fill_color() const { return state().fill_color; } - Gfx::Color stroke_color() const { return state().stroke_color; } - float stroke_width() const { return state().stroke_width; } - float fill_opacity() const { return state().fill_opacity; } - float stroke_opacity() const { return state().stroke_opacity; } - - void set_fill_rule(SVG::FillRule fill_rule) { state().fill_rule = fill_rule; } - void set_fill_color(Gfx::Color color) { state().fill_color = color; } - void set_stroke_color(Gfx::Color color) { state().stroke_color = color; } - void set_stroke_width(float width) { state().stroke_width = width; } - void set_fill_opacity(float opacity) { state().fill_opacity = opacity; } - void set_stroke_opacity(float opacity) { state().stroke_opacity = opacity; } - - CSSPixelPoint svg_element_position() const { return m_svg_element_bounds.top_left(); } - CSSPixelSize svg_element_size() const { return m_svg_element_bounds.size(); } - - void save() { m_states.append(m_states.last()); } - void restore() { m_states.take_last(); } - -private: - struct State { - SVG::FillRule fill_rule { SVG::FillRule::Nonzero }; - Gfx::Color fill_color { Gfx::Color::Transparent }; - Gfx::Color stroke_color { Gfx::Color::Transparent }; - float stroke_width { 1.0f }; - float fill_opacity { 1.0f }; - float stroke_opacity { 1.0f }; - }; - - State const& state() const { return m_states.last(); } - State& state() { return m_states.last(); } - - CSSPixelRect m_svg_element_bounds; - Vector m_states; -}; - -} diff --git a/Userland/Libraries/LibWeb/SVG/SVGGraphicsElement.cpp b/Userland/Libraries/LibWeb/SVG/SVGGraphicsElement.cpp index c5bcd3b4478..de6121e831e 100644 --- a/Userland/Libraries/LibWeb/SVG/SVGGraphicsElement.cpp +++ b/Userland/Libraries/LibWeb/SVG/SVGGraphicsElement.cpp @@ -108,7 +108,6 @@ Gfx::AffineTransform transform_from_transform_list(ReadonlySpan trans Gfx::AffineTransform SVGGraphicsElement::get_transform() const { - // FIXME: It would be nice to do this using the SVGContext, however, then layout/hit testing knows nothing about the transform. Gfx::AffineTransform transform = m_transform; for (auto* svg_ancestor = shadow_including_first_ancestor_of_type(); svg_ancestor; svg_ancestor = svg_ancestor->shadow_including_first_ancestor_of_type()) { transform = Gfx::AffineTransform { svg_ancestor->m_transform }.multiply(transform);