Clean up SVG gradient set up/align with pattern set up

Replace CollectGradientAttributes() with EnsureAttributes() that returns
a reference to the base class GradientAttributes. Drop the
GradientUnits() and CalculateGradientTransform() functions, because both
implementation are identical and the data can be accessed via the
attributes object.

Also switch to have LayoutSVGResourceGradient::BuildGradientData() add
the stops rather than the BuildGradient() overrides. (The stops are also
in the base attribute class.)

Bug: 1414383
Change-Id: I661d6b63ccff62650371dd51fa5810cc45cbe9c5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4236269
Reviewed-by: Philip Rogers <pdr@chromium.org>
Commit-Queue: Fredrik Söderquist <fs@opera.com>
Cr-Commit-Position: refs/heads/main@{#1103821}

third_party/blink/renderer/core/layout/svg/layout_svg_resource_gradient.cc

@@ -24,11 +24,13 @@
 
 #include <memory>
 
+#include "third_party/blink/renderer/core/svg/gradient_attributes.h"
 #include "third_party/blink/renderer/core/svg/svg_length.h"
 #include "third_party/blink/renderer/core/svg/svg_length_context.h"
 #include "third_party/blink/renderer/platform/graphics/gradient.h"
 #include "third_party/blink/renderer/platform/graphics/graphics_context.h"
 #include "third_party/blink/renderer/platform/graphics/skia/skia_utils.h"
+#include "third_party/blink/renderer/platform/transforms/affine_transform.h"
 
 namespace blink {
 
@@ -69,8 +71,7 @@ struct GradientData {
 };
 
 LayoutSVGResourceGradient::LayoutSVGResourceGradient(SVGGradientElement* node)
-    : LayoutSVGResourcePaintServer(node),
-      should_collect_gradient_attributes_(true) {}
+    : LayoutSVGResourcePaintServer(node) {}
 
 void LayoutSVGResourceGradient::Trace(Visitor* visitor) const {
   visitor->Trace(gradient_map_);
@@ -97,7 +98,7 @@ bool LayoutSVGResourceGradient::RemoveClientFromCache(
 }
 
 std::unique_ptr<GradientData> LayoutSVGResourceGradient::BuildGradientData(
-    const gfx::RectF& object_bounding_box) {
+    const gfx::RectF& object_bounding_box) const {
   NOT_DESTROYED();
   // Create gradient object
   auto gradient_data = std::make_unique<GradientData>();
@@ -106,14 +107,12 @@ std::unique_ptr<GradientData> LayoutSVGResourceGradient::BuildGradientData(
   // gradient. This should avoid tearing down the gradient we're
   // currently working on. Preferably the state validation should have
   // no side-effects though.
-  if (should_collect_gradient_attributes_) {
-    CollectGradientAttributes();
-    should_collect_gradient_attributes_ = false;
-  }
+  const GradientAttributes& attributes = EnsureAttributes();
 
   // We want the text bounding box applied to the gradient space transform
   // now, so the gradient shader can use it.
-  if (GradientUnits() == SVGUnitTypes::kSvgUnitTypeObjectboundingbox) {
+  if (attributes.GradientUnits() ==
+      SVGUnitTypes::kSvgUnitTypeObjectboundingbox) {
     // Spec: When the geometry of the applicable element has no width or height
     // and objectBoundingBox is specified, then the given effect (e.g. a
     // gradient or a filter) will be ignored.
@@ -127,8 +126,9 @@ std::unique_ptr<GradientData> LayoutSVGResourceGradient::BuildGradientData(
 
   // Create gradient object
   gradient_data->gradient = BuildGradient();
+  gradient_data->gradient->AddColorStops(attributes.Stops());
 
-  AffineTransform gradient_transform = CalculateGradientTransform();
+  AffineTransform gradient_transform = attributes.GradientTransform();
   gradient_data->userspace_transform *= gradient_transform;
 
   return gradient_data;

third_party/blink/renderer/core/layout/svg/layout_svg_resource_gradient.h

@@ -27,10 +27,10 @@
 #include "third_party/blink/renderer/core/layout/svg/layout_svg_resource_paint_server.h"
 #include "third_party/blink/renderer/core/svg/svg_gradient_element.h"
 #include "third_party/blink/renderer/platform/heap/collection_support/heap_hash_map.h"
-#include "third_party/blink/renderer/platform/transforms/affine_transform.h"
 
 namespace blink {
 
+class AffineTransform;
 class SVGLength;
 struct GradientData;
 
@@ -51,9 +51,7 @@ class LayoutSVGResourceGradient : public LayoutSVGResourcePaintServer {
   bool IsChildAllowed(LayoutObject* child, const ComputedStyle&) const final;
 
  protected:
-  virtual SVGUnitTypes::SVGUnitType GradientUnits() const = 0;
-  virtual AffineTransform CalculateGradientTransform() const = 0;
-  virtual void CollectGradientAttributes() = 0;
+  virtual const GradientAttributes& EnsureAttributes() const = 0;
   virtual scoped_refptr<Gradient> BuildGradient() const = 0;
 
   gfx::PointF ResolvePoint(SVGUnitTypes::SVGUnitType,
@@ -63,11 +61,12 @@ class LayoutSVGResourceGradient : public LayoutSVGResourcePaintServer {
   static GradientSpreadMethod PlatformSpreadMethodFromSVGType(
       SVGSpreadMethodType);
 
+  mutable bool should_collect_gradient_attributes_ = true;
+
  private:
   std::unique_ptr<GradientData> BuildGradientData(
-      const gfx::RectF& object_bounding_box);
+      const gfx::RectF& object_bounding_box) const;
 
-  bool should_collect_gradient_attributes_ : 1;
   using GradientMap = HeapHashMap<Member<const SVGResourceClient>,
                                   std::unique_ptr<GradientData>>;
   GradientMap gradient_map_;

third_party/blink/renderer/core/layout/svg/layout_svg_resource_linear_gradient.cc

@@ -21,7 +21,6 @@
 #include "third_party/blink/renderer/core/layout/svg/layout_svg_resource_linear_gradient.h"
 
 #include "third_party/blink/renderer/core/svg/svg_linear_gradient_element.h"
-#include "third_party/blink/renderer/platform/heap/garbage_collected.h"
 
 namespace blink {
 
@@ -36,12 +35,17 @@ void LayoutSVGResourceLinearGradient::Trace(Visitor* visitor) const {
   LayoutSVGResourceGradient::Trace(visitor);
 }
 
-void LayoutSVGResourceLinearGradient::CollectGradientAttributes() {
+const GradientAttributes& LayoutSVGResourceLinearGradient::EnsureAttributes()
+    const {
   NOT_DESTROYED();
   DCHECK(GetElement());
-  attributes_ = LinearGradientAttributes();
-  To<SVGLinearGradientElement>(GetElement())
-      ->CollectGradientAttributes(attributes_);
+  if (should_collect_gradient_attributes_) {
+    attributes_ = LinearGradientAttributes();
+    To<SVGLinearGradientElement>(GetElement())
+        ->CollectGradientAttributes(attributes_);
+    should_collect_gradient_attributes_ = false;
+  }
+  return attributes_;
 }
 
 gfx::PointF LayoutSVGResourceLinearGradient::StartPoint(
@@ -60,13 +64,12 @@ gfx::PointF LayoutSVGResourceLinearGradient::EndPoint(
 
 scoped_refptr<Gradient> LayoutSVGResourceLinearGradient::BuildGradient() const {
   NOT_DESTROYED();
-  scoped_refptr<Gradient> gradient = Gradient::CreateLinear(
+  DCHECK(!should_collect_gradient_attributes_);
+  return Gradient::CreateLinear(
       StartPoint(attributes_), EndPoint(attributes_),
       PlatformSpreadMethodFromSVGType(attributes_.SpreadMethod()),
       Gradient::ColorInterpolation::kUnpremultiplied,
       Gradient::DegenerateHandling::kAllow);
-  gradient->AddColorStops(attributes_.Stops());
-  return gradient;
 }
 
 }  // namespace blink

third_party/blink/renderer/core/layout/svg/layout_svg_resource_linear_gradient.h

@@ -46,22 +46,14 @@ class LayoutSVGResourceLinearGradient final : public LayoutSVGResourceGradient {
     return kResourceType;
   }
 
-  SVGUnitTypes::SVGUnitType GradientUnits() const override {
-    NOT_DESTROYED();
-    return attributes_.GradientUnits();
-  }
-  AffineTransform CalculateGradientTransform() const override {
-    NOT_DESTROYED();
-    return attributes_.GradientTransform();
-  }
-  void CollectGradientAttributes() override;
-  scoped_refptr<Gradient> BuildGradient() const override;
-
   gfx::PointF StartPoint(const LinearGradientAttributes&) const;
   gfx::PointF EndPoint(const LinearGradientAttributes&) const;
 
  private:
-  LinearGradientAttributes attributes_;
+  const GradientAttributes& EnsureAttributes() const override;
+  scoped_refptr<Gradient> BuildGradient() const override;
+
+  mutable LinearGradientAttributes attributes_;
 };
 
 }  // namespace blink

third_party/blink/renderer/core/layout/svg/layout_svg_resource_pattern.h

@@ -27,7 +27,6 @@
 #include "third_party/blink/renderer/core/layout/svg/layout_svg_resource_paint_server.h"
 #include "third_party/blink/renderer/core/svg/pattern_attributes.h"
 #include "third_party/blink/renderer/platform/heap/collection_support/heap_hash_map.h"
-#include "third_party/blink/renderer/platform/heap/garbage_collected.h"
 
 namespace blink {
 

third_party/blink/renderer/core/layout/svg/layout_svg_resource_radial_gradient.cc

@@ -22,7 +22,6 @@
 #include "third_party/blink/renderer/core/layout/svg/layout_svg_resource_radial_gradient.h"
 
 #include "third_party/blink/renderer/core/svg/svg_radial_gradient_element.h"
-#include "third_party/blink/renderer/platform/heap/garbage_collected.h"
 
 namespace blink {
 
@@ -37,12 +36,17 @@ void LayoutSVGResourceRadialGradient::Trace(Visitor* visitor) const {
   LayoutSVGResourceGradient::Trace(visitor);
 }
 
-void LayoutSVGResourceRadialGradient::CollectGradientAttributes() {
+const GradientAttributes& LayoutSVGResourceRadialGradient::EnsureAttributes()
+    const {
   NOT_DESTROYED();
   DCHECK(GetElement());
-  attributes_ = RadialGradientAttributes();
-  To<SVGRadialGradientElement>(GetElement())
-      ->CollectGradientAttributes(attributes_);
+  if (should_collect_gradient_attributes_) {
+    attributes_ = RadialGradientAttributes();
+    To<SVGRadialGradientElement>(GetElement())
+        ->CollectGradientAttributes(attributes_);
+    should_collect_gradient_attributes_ = false;
+  }
+  return attributes_;
 }
 
 gfx::PointF LayoutSVGResourceRadialGradient::CenterPoint(
@@ -73,14 +77,13 @@ float LayoutSVGResourceRadialGradient::FocalRadius(
 
 scoped_refptr<Gradient> LayoutSVGResourceRadialGradient::BuildGradient() const {
   NOT_DESTROYED();
-  scoped_refptr<Gradient> gradient = Gradient::CreateRadial(
+  DCHECK(!should_collect_gradient_attributes_);
+  return Gradient::CreateRadial(
       FocalPoint(attributes_), FocalRadius(attributes_),
       CenterPoint(attributes_), Radius(attributes_), 1,
       PlatformSpreadMethodFromSVGType(attributes_.SpreadMethod()),
       Gradient::ColorInterpolation::kUnpremultiplied,
       Gradient::DegenerateHandling::kAllow);
-  gradient->AddColorStops(attributes_.Stops());
-  return gradient;
 }
 
 }  // namespace blink

third_party/blink/renderer/core/layout/svg/layout_svg_resource_radial_gradient.h

@@ -46,24 +46,16 @@ class LayoutSVGResourceRadialGradient final : public LayoutSVGResourceGradient {
     return kResourceType;
   }
 
-  SVGUnitTypes::SVGUnitType GradientUnits() const override {
-    NOT_DESTROYED();
-    return attributes_.GradientUnits();
-  }
-  AffineTransform CalculateGradientTransform() const override {
-    NOT_DESTROYED();
-    return attributes_.GradientTransform();
-  }
-  void CollectGradientAttributes() override;
-  scoped_refptr<Gradient> BuildGradient() const override;
-
   gfx::PointF CenterPoint(const RadialGradientAttributes&) const;
   gfx::PointF FocalPoint(const RadialGradientAttributes&) const;
   float Radius(const RadialGradientAttributes&) const;
   float FocalRadius(const RadialGradientAttributes&) const;
 
  private:
-  RadialGradientAttributes attributes_;
+  const GradientAttributes& EnsureAttributes() const override;
+  scoped_refptr<Gradient> BuildGradient() const override;
+
+  mutable RadialGradientAttributes attributes_;
 };
 
 template <>