LibWeb+LibGfx: Fix SVG userSpaceOnUse gradient coordinate transformation

We were transforming coordinates for SVG gradients in a pretty
convoluted way: an inverse, unscaled transformation matrix was set up in
order to work around some (old?) technical limitations.

Rework this so the coordinate transformation no longer needs to be
inversed. This fixes gradients with "userSpaceOnUse" for its
gradientUnits attribute, which might cause coordinates to lie outside of
the bounding box of the gradient.

Two tests have updated reference screenshots with minor pixel updates;
this is probably the result of floating point precision improvements by
not inversing the matrix.

One test (svg-text-effects) has a bigger change: the gradient stops seem
to have moved along the text. This does seem to match other browsers
slightly better, so I'm moving forward with this ref update.

Tests/LibWeb/Screenshot/reference/svg-gradient-userSpaceOnUse-ref.html

@@ -0,0 +1,9 @@
+<style>
+    * {
+        margin: 0;
+    }
+    body {
+        background-color: white;
+    }
+</style>
+<img src="../images/svg-gradient-userSpaceOnUse-ref.png">

Tests/LibWeb/Screenshot/svg-gradient-userSpaceOnUse.html

@@ -0,0 +1,11 @@
+<link rel="match" href="reference/svg-gradient-userSpaceOnUse-ref.html" />
+<svg width="64" height="64" viewBox="0 0 64 64" fill="none" xmlns="http://www.w3.org/2000/svg">
+    <rect x="10" y="10" width="44" height="44" fill="url(#a)" />
+    <defs>
+        <linearGradient id="a" x1="10" y1="10" x2="10" y2="64" gradientUnits="userSpaceOnUse">
+            <stop offset="0" stop-color="#f00" />
+            <stop offset="0.0001" stop-color="#0f0" stop-opacity="0.3" />
+            <stop offset="1" stop-color="#0f0" stop-opacity="0.7" />
+        </linearGradient>
+    </defs>
+</svg>

Userland/Libraries/LibGfx/PaintStyle.h

@@ -324,9 +324,6 @@ public:
         m_spread_method = spread_method;
     }
 
-    void set_inverse_transform(AffineTransform transform) { m_inverse_transform = transform; }
-    void set_scale(float scale) { m_scale = scale; }
-
 protected:
     Optional<AffineTransform> const& scale_adjusted_inverse_gradient_transform() const { return m_inverse_transform; }
     float gradient_transform_scale() const { return m_scale; }

Userland/Libraries/LibWeb/CMakeLists.txt

@@ -591,7 +591,6 @@ set(SOURCES
     Painting/MediaPaintable.cpp
     Painting/NestedBrowsingContextPaintable.cpp
     Painting/PaintContext.cpp
-    Painting/PaintStyle.cpp
     Painting/Paintable.cpp
     Painting/PaintableBox.cpp
     Painting/PaintableFragment.cpp

Userland/Libraries/LibWeb/Painting/DisplayListPlayerSkia.cpp

@@ -937,43 +937,34 @@ static SkPaint paint_style_to_skia_paint(Painting::SVGGradientPaintStyle const&
         positions.append(color_stop.position);
     }
 
+    SkMatrix matrix;
+    matrix.setTranslate(bounding_rect.x(), bounding_rect.y());
+    if (auto gradient_transform = paint_style.gradient_transform(); gradient_transform.has_value())
+        matrix = matrix * to_skia_matrix(gradient_transform.value());
+
+    auto tile_mode = to_skia_tile_mode(paint_style.spread_method());
+
+    sk_sp<SkShader> shader;
     if (is<SVGLinearGradientPaintStyle>(paint_style)) {
         auto const& linear_gradient_paint_style = static_cast<SVGLinearGradientPaintStyle const&>(paint_style);
 
-        SkMatrix matrix;
-        auto scale = linear_gradient_paint_style.scale();
-        auto start_point = linear_gradient_paint_style.start_point().scaled(scale);
-        auto end_point = linear_gradient_paint_style.end_point().scaled(scale);
-
-        start_point.translate_by(bounding_rect.location());
-        end_point.translate_by(bounding_rect.location());
-
-        Array<SkPoint, 2> points;
-        points[0] = to_skia_point(start_point);
-        points[1] = to_skia_point(end_point);
-
-        auto shader = SkGradientShader::MakeLinear(points.data(), colors.data(), positions.data(), color_stops.size(), to_skia_tile_mode(paint_style.spread_method()), 0, &matrix);
-        paint.setShader(shader);
+        Array<SkPoint, 2> points {
+            to_skia_point(linear_gradient_paint_style.start_point()),
+            to_skia_point(linear_gradient_paint_style.end_point()),
+        };
+        shader = SkGradientShader::MakeLinear(points.data(), colors.data(), positions.data(), color_stops.size(), tile_mode, 0, &matrix);
     } else if (is<SVGRadialGradientPaintStyle>(paint_style)) {
         auto const& radial_gradient_paint_style = static_cast<SVGRadialGradientPaintStyle const&>(paint_style);
 
-        SkMatrix matrix;
-        auto scale = radial_gradient_paint_style.scale();
+        auto start_center = to_skia_point(radial_gradient_paint_style.start_center());
+        auto end_center = to_skia_point(radial_gradient_paint_style.end_center());
 
-        auto start_center = radial_gradient_paint_style.start_center().scaled(scale);
-        auto end_center = radial_gradient_paint_style.end_center().scaled(scale);
-        auto start_radius = radial_gradient_paint_style.start_radius() * scale;
-        auto end_radius = radial_gradient_paint_style.end_radius() * scale;
+        auto start_radius = radial_gradient_paint_style.start_radius();
+        auto end_radius = radial_gradient_paint_style.end_radius();
 
-        start_center.translate_by(bounding_rect.location());
-        end_center.translate_by(bounding_rect.location());
-
-        auto start_sk_point = to_skia_point(start_center);
-        auto end_sk_point = to_skia_point(end_center);
-
-        auto shader = SkGradientShader::MakeTwoPointConical(start_sk_point, start_radius, end_sk_point, end_radius, colors.data(), positions.data(), color_stops.size(), to_skia_tile_mode(paint_style.spread_method()), 0, &matrix);
-        paint.setShader(shader);
+        shader = SkGradientShader::MakeTwoPointConical(start_center, start_radius, end_center, end_radius, colors.data(), positions.data(), color_stops.size(), tile_mode, 0, &matrix);
     }
+    paint.setShader(shader);
 
     return paint;
 }

Userland/Libraries/LibWeb/Painting/PaintStyle.cpp

@@ -1,25 +0,0 @@
-/*
- * Copyright (c) 2024, Aliaksandr Kalenik <kalenik.aliaksandr@gmail.com>
- *
- * SPDX-License-Identifier: BSD-2-Clause
- */
-
-#include <LibWeb/Painting/PaintStyle.h>
-
-namespace Web::Painting {
-
-void SVGGradientPaintStyle::set_gradient_transform(Gfx::AffineTransform transform)
-{
-    // Note: The scaling is removed so enough points on the gradient line are generated.
-    // Otherwise, if you scale a tiny path the gradient looks pixelated.
-    m_scale = 1.0f;
-    if (auto inverse = transform.inverse(); inverse.has_value()) {
-        auto transform_scale = transform.scale();
-        m_scale = max(transform_scale.x(), transform_scale.y());
-        m_inverse_transform = Gfx::AffineTransform {}.scale(m_scale, m_scale).multiply(*inverse);
-    } else {
-        m_inverse_transform = OptionalNone {};
-    }
-}
-
-}

Userland/Libraries/LibWeb/Painting/PaintStyle.h

@@ -20,19 +20,17 @@ struct ColorStop {
 
 class SVGGradientPaintStyle : public RefCounted<SVGGradientPaintStyle> {
 public:
-    void set_gradient_transform(Gfx::AffineTransform transform);
-
     enum class SpreadMethod {
         Pad,
         Repeat,
         Reflect
     };
 
-    void set_spread_method(SpreadMethod spread_method) { m_spread_method = spread_method; }
+    Optional<Gfx::AffineTransform> const& gradient_transform() const { return m_gradient_transform; }
+    void set_gradient_transform(Gfx::AffineTransform transform) { m_gradient_transform = transform; }
 
-    Optional<Gfx::AffineTransform> const& scale_adjusted_inverse_gradient_transform() const { return m_inverse_transform; }
-    float gradient_transform_scale() const { return m_scale; }
     SpreadMethod spread_method() const { return m_spread_method; }
+    void set_spread_method(SpreadMethod spread_method) { m_spread_method = spread_method; }
 
     void add_color_stop(float position, Color color, Optional<float> transition_hint = {})
     {
@@ -49,16 +47,13 @@ public:
     ReadonlySpan<ColorStop> color_stops() const { return m_color_stops; }
     Optional<float> repeat_length() const { return m_repeat_length; }
 
-    float scale() const { return m_scale; }
-
     virtual ~SVGGradientPaintStyle() {};
 
 protected:
     Vector<ColorStop, 4> m_color_stops;
     Optional<float> m_repeat_length;
 
-    Optional<Gfx::AffineTransform> m_inverse_transform {};
-    float m_scale { 1.0f };
+    Optional<Gfx::AffineTransform> m_gradient_transform {};
     SpreadMethod m_spread_method { SpreadMethod::Pad };
 };
 

Userland/Libraries/LibWeb/Painting/SVGPathPaintable.cpp

@@ -106,7 +106,6 @@ void SVGPathPaintable::paint(PaintContext& context, PaintPhase phase) const
     SVG::SVGPaintContext paint_context {
         .viewport = svg_viewport,
         .path_bounding_box = computed_path()->bounding_box(),
-        .transform = paint_transform
     };
 
     auto fill_opacity = graphics_element.fill_opacity().value_or(1);

Userland/Libraries/LibWeb/SVG/SVGGradientElement.cpp

@@ -83,15 +83,21 @@ Optional<Gfx::AffineTransform> SVGGradientElement::gradient_transform_impl(HashT
 // The gradient transform, appropriately scaled and combined with the paint transform.
 Gfx::AffineTransform SVGGradientElement::gradient_paint_transform(SVGPaintContext const& paint_context) const
 {
-    auto transform = gradient_transform().value_or(Gfx::AffineTransform {});
-    if (gradient_units() == GradientUnits::ObjectBoundingBox) {
-        // Adjust transform to take place in the coordinate system defined by the bounding box:
-        return Gfx::AffineTransform { paint_context.transform }
-            .translate(paint_context.path_bounding_box.location())
-            .scale(paint_context.path_bounding_box.width(), paint_context.path_bounding_box.height())
-            .multiply(transform);
+    Gfx::AffineTransform gradient_paint_transform;
+    auto const& bounding_box = paint_context.path_bounding_box;
+
+    if (gradient_units() == SVGUnits::ObjectBoundingBox) {
+        // Scale points from 0..1 to bounding box coordinates:
+        gradient_paint_transform.scale(bounding_box.width(), bounding_box.height());
+    } else {
+        // Translate points from viewport to bounding box coordinates:
+        gradient_paint_transform.translate(paint_context.viewport.location() - bounding_box.location());
     }
-    return Gfx::AffineTransform { paint_context.transform }.multiply(transform);
+
+    if (auto transform = gradient_transform(); transform.has_value())
+        gradient_paint_transform.multiply(transform.value());
+
+    return gradient_paint_transform;
 }
 
 void SVGGradientElement::add_color_stops(Painting::SVGGradientPaintStyle& paint_style) const

Userland/Libraries/LibWeb/SVG/SVGGradientElement.h

@@ -19,7 +19,6 @@ namespace Web::SVG {
 struct SVGPaintContext {
     Gfx::FloatRect viewport;
     Gfx::FloatRect path_bounding_box;
-    Gfx::AffineTransform transform;
 };
 
 inline Painting::SVGGradientPaintStyle::SpreadMethod to_painting_spread_method(SpreadMethod spread_method)