Explicit check for degenerate SVG radial gradients

Both Canvas2D and SVG expect degenerate radial gradients
(p0 == p1 && r0 == r1) to not draw anything [1].  Canvas2D enforces this
behavior explicitly, but SVG relies on the current Skia semantics.

Skia is about to start handling this case differently [2].  This CL
updates LayoutSVGResourceRadialGradient to also enforce the spec behavior
explicitly:

  * add a DegenerateHandling flag to Gradient (all subtypes) to control
    degenerate behavior
  * when degenerates are not allowed, return an SkEmptyShader to draw nothing
  * in order to support the above, plumb SkEmptyShader in PaintShader
  * update clients to pass this new flag as required by spec
  * clean up degenerate tracking in CanvasGradient/BaseRenderingContext2D
    (no longer needed)
  * also drop the bool return type for BaseRenderingContext2D::Draw (not used)


[1] https://html.spec.whatwg.org/multipage/canvas.html#dom-context-2d-createradialgradient
[2] https://skia-review.googlesource.com/c/skia/+/168487


Change-Id: I940fcfd87907ebd2df9fae59102770a6031606e5
Reviewed-on: https://chromium-review.googlesource.com/c/1324612
Reviewed-by: Fernando Serboncini <fserb@chromium.org>
Reviewed-by: Khushal <khushalsagar@chromium.org>
Reviewed-by: Fredrik Söderquist <fs@opera.com>
Commit-Queue: Florin Malita <fmalita@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606553}

cc/paint/paint_shader.cc

@@ -52,6 +52,13 @@ bool CompareMatrices(const SkMatrix& a,
 
 const PaintShader::RecordShaderId PaintShader::kInvalidRecordShaderId = -1;
 
+sk_sp<PaintShader> PaintShader::MakeEmpty() {
+  sk_sp<PaintShader> shader(new PaintShader(Type::kEmpty));
+
+  shader->CreateSkShader();
+  return shader;
+}
+
 sk_sp<PaintShader> PaintShader::MakeColor(SkColor color) {
   sk_sp<PaintShader> shader(new PaintShader(Type::kColor));
 
@@ -373,6 +380,9 @@ void PaintShader::CreateSkShader(const gfx::SizeF* raster_scale,
   DCHECK(!cached_shader_);
 
   switch (shader_type_) {
+    case Type::kEmpty:
+      cached_shader_ = SkShader::MakeEmptyShader();
+      break;
     case Type::kColor:
       // This will be handled by the fallback check below.
       break;
@@ -487,6 +497,7 @@ bool PaintShader::IsValid() const {
     return true;
 
   switch (shader_type_) {
+    case Type::kEmpty:
     case Type::kColor:
       return true;
     case Type::kSweepGradient:
@@ -545,6 +556,7 @@ bool PaintShader::operator==(const PaintShader& other) const {
 
   // Variables that only some shaders use.
   switch (shader_type_) {
+    case Type::kEmpty:
     case Type::kColor:
       break;
     case Type::kSweepGradient:

cc/paint/paint_shader.h

@@ -27,6 +27,7 @@ using PaintRecord = PaintOpBuffer;
 class CC_PAINT_EXPORT PaintShader : public SkRefCnt {
  public:
   enum class Type : uint8_t {
+    kEmpty,
     kColor,
     kLinearGradient,
     kRadialGradient,
@@ -45,6 +46,8 @@ class CC_PAINT_EXPORT PaintShader : public SkRefCnt {
   // shader that is backed by the paint record.
   enum class ScalingBehavior : uint8_t { kRasterAtScale, kFixedScale };
 
+  static sk_sp<PaintShader> MakeEmpty();
+
   static sk_sp<PaintShader> MakeColor(SkColor color);
 
   static sk_sp<PaintShader> MakeLinearGradient(

cc/test/paint_op_helper.h

@@ -493,6 +493,8 @@ class PaintOpHelper {
 
   static std::string EnumToString(PaintShader::Type type) {
     switch (type) {
+      case PaintShader::Type::kEmpty:
+        return "kEmpty";
       case PaintShader::Type::kColor:
         return "kColor";
       case PaintShader::Type::kLinearGradient:

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

@@ -57,7 +57,8 @@ scoped_refptr<Gradient> LayoutSVGResourceLinearGradient::BuildGradient() const {
   scoped_refptr<Gradient> gradient = Gradient::CreateLinear(
       StartPoint(attributes), EndPoint(attributes),
       PlatformSpreadMethodFromSVGType(attributes.SpreadMethod()),
-      Gradient::ColorInterpolation::kUnpremultiplied);
+      Gradient::ColorInterpolation::kUnpremultiplied,
+      Gradient::DegenerateHandling::kAllow);
   gradient->AddColorStops(attributes.Stops());
   return gradient;
 }

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

@@ -71,7 +71,8 @@ scoped_refptr<Gradient> LayoutSVGResourceRadialGradient::BuildGradient() const {
       FocalPoint(attributes), FocalRadius(attributes), CenterPoint(attributes),
       Radius(attributes), 1,
       PlatformSpreadMethodFromSVGType(attributes.SpreadMethod()),
-      Gradient::ColorInterpolation::kUnpremultiplied);
+      Gradient::ColorInterpolation::kUnpremultiplied,
+      Gradient::DegenerateHandling::kDisallow);
   gradient->AddColorStops(attributes.Stops());
   return gradient;
 }

third_party/blink/renderer/modules/canvas/canvas2d/base_rendering_context_2d.h

@@ -323,7 +323,7 @@ class MODULES_EXPORT BaseRenderingContext2D : public GarbageCollectedMixin,
                         SkIRect*);
 
   template <typename DrawFunc, typename ContainsFunc>
-  bool Draw(const DrawFunc&,
+  void Draw(const DrawFunc&,
             const ContainsFunc&,
             const SkRect& bounds,
             CanvasRenderingContext2DState::PaintType,
@@ -434,26 +434,18 @@ class MODULES_EXPORT BaseRenderingContext2D : public GarbageCollectedMixin,
 };
 
 template <typename DrawFunc, typename ContainsFunc>
-bool BaseRenderingContext2D::Draw(
+void BaseRenderingContext2D::Draw(
     const DrawFunc& draw_func,
     const ContainsFunc& draw_covers_clip_bounds,
     const SkRect& bounds,
     CanvasRenderingContext2DState::PaintType paint_type,
     CanvasRenderingContext2DState::ImageType image_type) {
   if (!GetState().IsTransformInvertible())
-    return false;
+    return;
 
   SkIRect clip_bounds;
   if (!DrawingCanvas() || !DrawingCanvas()->getDeviceClipBounds(&clip_bounds))
-    return false;
-
-  // If gradient size is zero, then paint nothing.
-  CanvasStyle* style = GetState().Style(paint_type);
-  if (style) {
-    CanvasGradient* gradient = style->GetCanvasGradient();
-    if (gradient && gradient->IsZeroSize())
-      return false;
-  }
+    return;
 
   if (IsFullCanvasCompositeMode(GetState().GlobalComposite()) ||
       StateHasFilter()) {
@@ -477,7 +469,6 @@ bool BaseRenderingContext2D::Draw(
       DidDraw(dirty_rect);
     }
   }
-  return true;
 }
 
 template <typename DrawFunc>

third_party/blink/renderer/modules/canvas/canvas2d/canvas_gradient.cc

@@ -33,14 +33,26 @@
 namespace blink {
 
 CanvasGradient::CanvasGradient(const FloatPoint& p0, const FloatPoint& p1)
-    : gradient_(Gradient::CreateLinear(p0, p1)), is_zero_size_(p0 == p1) {}
+    : gradient_(
+          Gradient::CreateLinear(p0,
+                                 p1,
+                                 kSpreadMethodPad,
+                                 Gradient::ColorInterpolation::kUnpremultiplied,
+                                 Gradient::DegenerateHandling::kDisallow)) {}
 
 CanvasGradient::CanvasGradient(const FloatPoint& p0,
                                float r0,
                                const FloatPoint& p1,
                                float r1)
-    : gradient_(Gradient::CreateRadial(p0, r0, p1, r1)),
-      is_zero_size_(p0 == p1 && r0 == r1) {}
+    : gradient_(
+          Gradient::CreateRadial(p0,
+                                 r0,
+                                 p1,
+                                 r1,
+                                 1,
+                                 kSpreadMethodPad,
+                                 Gradient::ColorInterpolation::kUnpremultiplied,
+                                 Gradient::DegenerateHandling::kDisallow)) {}
 
 void CanvasGradient::addColorStop(float value,
                                   const String& color_string,

third_party/blink/renderer/modules/canvas/canvas2d/canvas_gradient.h

@@ -61,11 +61,8 @@ class MODULES_EXPORT CanvasGradient final : public ScriptWrappable {
 
   void addColorStop(float value, const String& color, ExceptionState&);
 
-  bool IsZeroSize() const { return is_zero_size_; }
-
  private:
   scoped_refptr<Gradient> gradient_;
-  const bool is_zero_size_;
 };
 
 }  // namespace blink

third_party/blink/renderer/platform/graphics/gradient.cc

@@ -42,10 +42,12 @@ namespace blink {
 
 Gradient::Gradient(Type type,
                    GradientSpreadMethod spread_method,
-                   ColorInterpolation interpolation)
+                   ColorInterpolation interpolation,
+                   DegenerateHandling degenerate_handling)
     : type_(type),
       spread_method_(spread_method),
       color_interpolation_(interpolation),
+      degenerate_handling_(degenerate_handling),
       stops_sorted_(true) {}
 
 Gradient::~Gradient() = default;
@@ -187,8 +189,12 @@ class LinearGradient final : public Gradient {
   LinearGradient(const FloatPoint& p0,
                  const FloatPoint& p1,
                  GradientSpreadMethod spread_method,
-                 ColorInterpolation interpolation)
-      : Gradient(Type::kLinear, spread_method, interpolation),
+                 ColorInterpolation interpolation,
+                 DegenerateHandling degenerate_handling)
+      : Gradient(Type::kLinear,
+                 spread_method,
+                 interpolation,
+                 degenerate_handling),
         p0_(p0),
         p1_(p1) {}
 
@@ -199,6 +205,11 @@ class LinearGradient final : public Gradient {
                                   uint32_t flags,
                                   const SkMatrix& local_matrix,
                                   SkColor fallback_color) const override {
+    if (GetDegenerateHandling() == DegenerateHandling::kDisallow &&
+        p0_ == p1_) {
+      return PaintShader::MakeEmpty();
+    }
+
     SkPoint pts[2] = {FloatPointToSkPoint(p0_), FloatPointToSkPoint(p1_)};
     return PaintShader::MakeLinearGradient(
         pts, colors.data(), pos.data(), static_cast<int>(colors.size()),
@@ -218,8 +229,12 @@ class RadialGradient final : public Gradient {
                  float r1,
                  float aspect_ratio,
                  GradientSpreadMethod spread_method,
-                 ColorInterpolation interpolation)
-      : Gradient(Type::kRadial, spread_method, interpolation),
+                 ColorInterpolation interpolation,
+                 DegenerateHandling degenerate_handling)
+      : Gradient(Type::kRadial,
+                 spread_method,
+                 interpolation,
+                 degenerate_handling),
         p0_(p0),
         p1_(p1),
         r0_(r0),
@@ -248,6 +263,12 @@ class RadialGradient final : public Gradient {
     // negative radius, ask for zero instead.
     const SkScalar radius0 = std::max(WebCoreFloatToSkScalar(r0_), 0.0f);
     const SkScalar radius1 = std::max(WebCoreFloatToSkScalar(r1_), 0.0f);
+
+    if (GetDegenerateHandling() == DegenerateHandling::kDisallow &&
+        p0_ == p1_ && radius0 == radius1) {
+      return PaintShader::MakeEmpty();
+    }
+
     return PaintShader::MakeTwoPointConicalGradient(
         FloatPointToSkPoint(p0_), radius0, FloatPointToSkPoint(p1_), radius1,
         colors.data(), pos.data(), static_cast<int>(colors.size()), tile_mode,
@@ -269,8 +290,12 @@ class ConicGradient final : public Gradient {
                 float start_angle,
                 float end_angle,
                 GradientSpreadMethod spread_method,
-                ColorInterpolation interpolation)
-      : Gradient(Type::kConic, spread_method, interpolation),
+                ColorInterpolation interpolation,
+                DegenerateHandling degenerate_handling)
+      : Gradient(Type::kConic,
+                 spread_method,
+                 interpolation,
+                 degenerate_handling),
         position_(position),
         rotation_(rotation),
         start_angle_(start_angle),
@@ -283,6 +308,11 @@ class ConicGradient final : public Gradient {
                                   uint32_t flags,
                                   const SkMatrix& local_matrix,
                                   SkColor fallback_color) const override {
+    if (GetDegenerateHandling() == DegenerateHandling::kDisallow &&
+        start_angle_ == end_angle_) {
+      return PaintShader::MakeEmpty();
+    }
+
     // Skia's sweep gradient angles are relative to the x-axis, not the y-axis.
     const float skia_rotation = rotation_ - 90;
     const SkMatrix* matrix = &local_matrix;
@@ -313,9 +343,10 @@ scoped_refptr<Gradient> Gradient::CreateLinear(
     const FloatPoint& p0,
     const FloatPoint& p1,
     GradientSpreadMethod spread_method,
-    ColorInterpolation interpolation) {
-  return base::AdoptRef(
-      new LinearGradient(p0, p1, spread_method, interpolation));
+    ColorInterpolation interpolation,
+    DegenerateHandling degenerate_handling) {
+  return base::AdoptRef(new LinearGradient(p0, p1, spread_method, interpolation,
+                                           degenerate_handling));
 }
 
 scoped_refptr<Gradient> Gradient::CreateRadial(
@@ -325,9 +356,11 @@ scoped_refptr<Gradient> Gradient::CreateRadial(
     float r1,
     float aspect_ratio,
     GradientSpreadMethod spread_method,
-    ColorInterpolation interpolation) {
+    ColorInterpolation interpolation,
+    DegenerateHandling degenerate_handling) {
   return base::AdoptRef(new RadialGradient(p0, r0, p1, r1, aspect_ratio,
-                                           spread_method, interpolation));
+                                           spread_method, interpolation,
+                                           degenerate_handling));
 }
 
 scoped_refptr<Gradient> Gradient::CreateConic(
@@ -336,10 +369,11 @@ scoped_refptr<Gradient> Gradient::CreateConic(
     float start_angle,
     float end_angle,
     GradientSpreadMethod spread_method,
-    ColorInterpolation interpolation) {
+    ColorInterpolation interpolation,
+    DegenerateHandling degenerate_handling) {
   return base::AdoptRef(new ConicGradient(position, rotation, start_angle,
                                           end_angle, spread_method,
-                                          interpolation));
+                                          interpolation, degenerate_handling));
 }
 
 }  // namespace blink

third_party/blink/renderer/platform/graphics/gradient.h

@@ -57,11 +57,17 @@ class PLATFORM_EXPORT Gradient : public RefCounted<Gradient> {
     kUnpremultiplied,
   };
 
+  enum class DegenerateHandling {
+    kAllow,
+    kDisallow,
+  };
+
   static scoped_refptr<Gradient> CreateLinear(
       const FloatPoint& p0,
       const FloatPoint& p1,
       GradientSpreadMethod = kSpreadMethodPad,
-      ColorInterpolation = ColorInterpolation::kUnpremultiplied);
+      ColorInterpolation = ColorInterpolation::kUnpremultiplied,
+      DegenerateHandling = DegenerateHandling::kAllow);
 
   static scoped_refptr<Gradient> CreateRadial(
       const FloatPoint& p0,
@@ -70,15 +76,17 @@ class PLATFORM_EXPORT Gradient : public RefCounted<Gradient> {
       float r1,
       float aspect_ratio = 1,
       GradientSpreadMethod = kSpreadMethodPad,
-      ColorInterpolation = ColorInterpolation::kUnpremultiplied);
+      ColorInterpolation = ColorInterpolation::kUnpremultiplied,
+      DegenerateHandling = DegenerateHandling::kAllow);
 
   static scoped_refptr<Gradient> CreateConic(
       const FloatPoint& position,
       float rotation,
       float start_angle,
       float end_angle,
       GradientSpreadMethod = kSpreadMethodPad,
-      ColorInterpolation = ColorInterpolation::kUnpremultiplied);
+      ColorInterpolation = ColorInterpolation::kUnpremultiplied,
+      DegenerateHandling = DegenerateHandling::kAllow);
 
   virtual ~Gradient();
 
@@ -100,7 +108,7 @@ class PLATFORM_EXPORT Gradient : public RefCounted<Gradient> {
   void ApplyToFlags(PaintFlags&, const SkMatrix& local_matrix);
 
  protected:
-  Gradient(Type, GradientSpreadMethod, ColorInterpolation);
+  Gradient(Type, GradientSpreadMethod, ColorInterpolation, DegenerateHandling);
 
   using ColorBuffer = Vector<SkColor, 8>;
   using OffsetBuffer = Vector<SkScalar, 8>;
@@ -111,6 +119,10 @@ class PLATFORM_EXPORT Gradient : public RefCounted<Gradient> {
                                           const SkMatrix&,
                                           SkColor) const = 0;
 
+  DegenerateHandling GetDegenerateHandling() const {
+    return degenerate_handling_;
+  }
+
  private:
   sk_sp<PaintShader> CreateShaderInternal(const SkMatrix& local_matrix);
 
@@ -122,6 +134,7 @@ class PLATFORM_EXPORT Gradient : public RefCounted<Gradient> {
   const Type type_;
   const GradientSpreadMethod spread_method_;
   const ColorInterpolation color_interpolation_;
+  const DegenerateHandling degenerate_handling_;
 
   Vector<ColorStop, 2> stops_;
   bool stops_sorted_;