Reland "Improve degenerate 2pt conical gradient cases"
This reverts commit 95af4726bf91669e51250ebd3baa2925b6975e8e.
Reason for revert: I think this may not have been the reason the Android roll was failing. We've rolled, so it's a good time to try again.
Original change's description:
> Revert "Improve degenerate 2pt conical gradient cases"
>
> This reverts commit 879dab87ab78613458f9d36f843bc3c6ffdafc74.
>
> Reason for revert: Android roll failed.
> https://sponge.corp.google.com/target?id=93bc6b8d-9b42-4805-b204-46ae62f1b005&target=x86+CtsGraphicsTestCases&searchFor=&show=FAILED&sortBy=STATUS
> A test VectorDrawableTest.testVectorDrawableGradient fails.
>
> Original change's description:
> > Improve degenerate 2pt conical gradient cases
> >
> > This was originally a reland of "Fix div-by-zero loophole in gradient factory func", c34dd6c5263490b94ef9af7a14dee1b4bc872b58, but:
> >
> > The change caused blink layout tests when encountering very small or zero radii. The original patch switched the order of checking if the radii are equal and if the start radius was 0. In the case where both radii are 0, the original code created an actual radial gradient of radius 0 and the new code rejected the shader. A radial gradient with radius of 0 properly renders the last border color as a fill.
> >
> > This made me realize that the case when the center positions and the radii are the same can be handled more correctly than just always returning an empty shader, so the fix now applies simplifications to the gradient definition depending on the tile mode and should not trigger any blink tests. I added a row to the gradient edge cases GM to make sure it degrades gracefully.
> >
> > Original change's description:
> > > Fix div-by-zero loophole in gradient factory func
> > >
> > > Bug: oss-fuzz:10373
> > > Change-Id: I4277fb63e3186ee34feaf09ecf6aeddeb532f9c1
> > > Reviewed-on: https://skia-review.googlesource.com/c/168269
> > > Reviewed-by: Kevin Lubick <kjlubick@google.com>
> > > Commit-Queue: Michael Ludwig <michaelludwig@google.com>
> >
> > Docs-Preview: https://skia.org/?cl=168487
> > Bug: oss-fuzz:10373
> > Change-Id: Ib0a6e7f807560a5dcf24d1c8e0146817af2d9606
> > Reviewed-on: https://skia-review.googlesource.com/c/168487
> > Reviewed-by: Mike Reed <reed@google.com>
> > Reviewed-by: Florin Malita <fmalita@chromium.org>
> > Commit-Queue: Michael Ludwig <michaelludwig@google.com>
>
> TBR=caryclark@google.com,fmalita@chromium.org,fmalita@google.com,reed@google.com,michaelludwig@google.com
>
> Change-Id: I91b896c4a438c02206679b327a01b47f40993965
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: oss-fuzz:10373
> Reviewed-on: https://skia-review.googlesource.com/c/170272
> Reviewed-by: Stan Iliev <stani@google.com>
> Commit-Queue: Stan Iliev <stani@google.com>
TBR=caryclark@google.com,fmalita@chromium.org,fmalita@google.com,reed@google.com,stani@google.com,michaelludwig@google.com
# Not skipping CQ checks because original CL landed > 1 day ago.
Bug: oss-fuzz:10373
Change-Id: I7577fcea9eb8a875e94723ab2cca2fcc990b82b2
Reviewed-on: https://skia-review.googlesource.com/c/170279
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
diff --git a/gm/gradients_degenerate.cpp b/gm/gradients_degenerate.cpp
new file mode 100644
index 0000000..583bbed
--- /dev/null
+++ b/gm/gradients_degenerate.cpp
@@ -0,0 +1,150 @@
+/*
+ * Copyright 2018 Google Inc.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#include "gm.h"
+#include "SkGradientShader.h"
+
+// NOTE: The positions define hardstops for the red and green borders. For the repeating degenerate
+// gradients, that means the red and green are never visible, so the average color used should only
+// be based off of the white, blue, black blend.
+static const SkColor COLORS[] = { SK_ColorRED, SK_ColorWHITE, SK_ColorBLUE,
+ SK_ColorBLACK, SK_ColorGREEN };
+static const SkScalar POS[] = { 0.0, 0.0, 0.5, 1.0, 1.0 };
+static const int COLOR_CT = SK_ARRAY_COUNT(COLORS);
+
+static const SkShader::TileMode TILE_MODES[] = { SkShader::kDecal_TileMode,
+ SkShader::kRepeat_TileMode,
+ SkShader::kMirror_TileMode,
+ SkShader::kClamp_TileMode };
+static const char* TILE_NAMES[] = { "decal", "repeat", "mirror", "clamp" };
+static const int TILE_MODE_CT = SK_ARRAY_COUNT(TILE_MODES);
+
+static constexpr int TILE_SIZE = 100;
+static constexpr int TILE_GAP = 10;
+
+static const SkPoint CENTER = SkPoint::Make(TILE_SIZE / 2, TILE_SIZE / 2);
+
+typedef sk_sp<SkShader> (*GradientFactory)(SkShader::TileMode tm);
+
+static void draw_tile_header(SkCanvas* canvas) {
+ canvas->save();
+
+ SkPaint paint;
+ paint.setColor(SK_ColorBLACK);
+ paint.setTextSize(12.0f);
+ paint.setAntiAlias(true);
+
+ for (int i = 0; i < TILE_MODE_CT; ++i) {
+ canvas->drawString(TILE_NAMES[i], 0, 0, paint);
+ canvas->translate(TILE_SIZE + TILE_GAP, 0);
+ }
+
+ canvas->restore();
+
+ // Now adjust to start at rows below the header
+ canvas->translate(0, 2 * TILE_GAP);
+}
+
+static void draw_row(SkCanvas* canvas, const char* desc, GradientFactory factory) {
+ canvas->save();
+
+ SkPaint text;
+ text.setColor(SK_ColorBLACK);
+ text.setTextSize(12.0f);
+ text.setAntiAlias(true);
+
+ canvas->translate(0, TILE_GAP);
+ canvas->drawString(desc, 0, 0, text);
+ canvas->translate(0, TILE_GAP);
+
+ SkPaint paint;
+ paint.setColor(SK_ColorBLACK);
+ paint.setStyle(SkPaint::kStrokeAndFill_Style);
+ paint.setStrokeWidth(2.0f);
+
+ for (int i = 0; i < TILE_MODE_CT; ++i) {
+ paint.setShader(factory(TILE_MODES[i]));
+ canvas->drawRect(SkRect::MakeWH(TILE_SIZE, TILE_SIZE), paint);
+ canvas->translate(TILE_SIZE + TILE_GAP, 0);
+ }
+
+ canvas->restore();
+
+ // Now adjust to start the next row below this one (1 gap for text and 2 gap for margin)
+ canvas->translate(0, 3 * TILE_GAP + TILE_SIZE);
+}
+
+static sk_sp<SkShader> make_linear(SkShader::TileMode mode) {
+ // Same position
+ SkPoint pts[2] = {CENTER, CENTER};
+ return SkGradientShader::MakeLinear(pts, COLORS, POS, COLOR_CT, mode);
+}
+
+static sk_sp<SkShader> make_radial(SkShader::TileMode mode) {
+ // Radius = 0
+ return SkGradientShader::MakeRadial(CENTER, 0.0, COLORS, POS, COLOR_CT, mode);
+}
+
+static sk_sp<SkShader> make_sweep(SkShader::TileMode mode) {
+ // Start and end angles at 45
+ static constexpr SkScalar SWEEP_ANG = 45.0;
+ return SkGradientShader::MakeSweep(CENTER.fX, CENTER.fY, COLORS, POS, COLOR_CT, mode,
+ SWEEP_ANG, SWEEP_ANG, 0, nullptr);
+}
+
+static sk_sp<SkShader> make_sweep_zero_ang(SkShader::TileMode mode) {
+ // Start and end angles at 0
+ return SkGradientShader::MakeSweep(CENTER.fX, CENTER.fY, COLORS, POS, COLOR_CT, mode,
+ 0.0, 0.0, 0, nullptr);
+}
+
+static sk_sp<SkShader> make_2pt_conic(SkShader::TileMode mode) {
+ // Start and end radius = TILE_SIZE, same position
+ return SkGradientShader::MakeTwoPointConical(CENTER, TILE_SIZE / 2, CENTER, TILE_SIZE / 2,
+ COLORS, POS, COLOR_CT, mode);
+}
+
+static sk_sp<SkShader> make_2pt_conic_zero_rad(SkShader::TileMode mode) {
+ // Start and end radius = 0, same position
+ return SkGradientShader::MakeTwoPointConical(CENTER, 0.0, CENTER, 0.0, COLORS, POS,
+ COLOR_CT, mode);
+}
+
+class DegenerateGradientGM : public skiagm::GM {
+public:
+ DegenerateGradientGM() {
+
+ }
+
+protected:
+ SkString onShortName() override {
+ return SkString("degenerate_gradients");
+ }
+
+ SkISize onISize() override {
+ return SkISize::Make(800, 800);
+ }
+
+ void onDraw(SkCanvas* canvas) override {
+ canvas->translate(3 * TILE_GAP, 3 * TILE_GAP);
+ draw_tile_header(canvas);
+
+ draw_row(canvas, "linear: empty, blue, blue, green", make_linear);
+ draw_row(canvas, "radial: empty, blue, blue, green", make_radial);
+ draw_row(canvas, "sweep-0: empty, blue, blue, green", make_sweep_zero_ang);
+ draw_row(canvas, "sweep-45: empty, blue, blue, red 45 degree sector then green",
+ make_sweep);
+ draw_row(canvas, "2pt-conic-0: empty, blue, blue, green", make_2pt_conic_zero_rad);
+ draw_row(canvas, "2pt-conic-1: empty, blue, blue, full red circle on green",
+ make_2pt_conic);
+ }
+
+private:
+ typedef skiagm::GM INHERITED;
+};
+
+DEF_GM(return new DegenerateGradientGM;)
diff --git a/gn/gm.gni b/gn/gm.gni
index ecc0828..1fbe831 100644
--- a/gn/gm.gni
+++ b/gn/gm.gni
@@ -157,6 +157,7 @@
"$_gm/gradient_matrix.cpp",
"$_gm/gradientDirtyLaundry.cpp",
"$_gm/gradients.cpp",
+ "$_gm/gradients_degenerate.cpp",
"$_gm/gradients_2pt_conical.cpp",
"$_gm/gradients_no_texture.cpp",
"$_gm/gradtext.cpp",
diff --git a/include/effects/SkGradientShader.h b/include/effects/SkGradientShader.h
index 2919261..3b537a5 100644
--- a/include/effects/SkGradientShader.h
+++ b/include/effects/SkGradientShader.h
@@ -13,7 +13,36 @@
/** \class SkGradientShader
SkGradientShader hosts factories for creating subclasses of SkShader that
- render linear and radial gradients.
+ render linear and radial gradients. In general, degenerate cases should not
+ produce surprising results, but there are several types of degeneracies:
+
+ * A linear gradient made from the same two points.
+ * A radial gradient with a radius of zero.
+ * A sweep gradient where the start and end angle are the same.
+ * A two point conical gradient where the two centers and the two radii are
+ the same.
+
+ For any degenerate gradient with a decal tile mode, it will draw empty since the interpolating
+ region is zero area and the outer region is discarded by the decal mode.
+
+ For any degenerate gradient with a repeat or mirror tile mode, it will draw a solid color that
+ is the average gradient color, since infinitely many repetitions of the gradients will fill the
+ shape.
+
+ For a clamped gradient, every type is well-defined at the limit except for linear gradients. The
+ radial gradient with zero radius becomes the last color. The sweep gradient draws the sector
+ from 0 to the provided angle with the first color, with a hardstop switching to the last color.
+ When the provided angle is 0, this is just the solid last color again. Similarly, the two point
+ conical gradient becomes a circle filled with the first color, sized to the provided radius,
+ with a hardstop switching to the last color. When the two radii are both zero, this is just the
+ solid last color.
+
+ As a linear gradient approaches the degenerate case, its shader will approach the appearance of
+ two half planes, each filled by the first and last colors of the gradient. The planes will be
+ oriented perpendicular to the vector between the two defining points of the gradient. However,
+ once they become the same point, Skia cannot reconstruct what that expected orientation is. To
+ provide a stable and predictable color in this case, Skia just uses the last color as a solid
+ fill to be similar to many of the other degenerate gradients' behaviors in clamp mode.
*/
class SK_API SkGradientShader {
public:
diff --git a/src/shaders/gradients/SkGradientShader.cpp b/src/shaders/gradients/SkGradientShader.cpp
index 52a9b56..4726f09 100644
--- a/src/shaders/gradients/SkGradientShader.cpp
+++ b/src/shaders/gradients/SkGradientShader.cpp
@@ -517,6 +517,74 @@
desc->fLocalMatrix = localMatrix;
}
+static SkColor4f average_gradient_color(const SkColor4f colors[], const SkScalar pos[],
+ int colorCount) {
+ // The gradient is a piecewise linear interpolation between colors. For a given interval,
+ // the integral between the two endpoints is 0.5 * (ci + cj) * (pj - pi), which provides that
+ // intervals average color. The overall average color is thus the sum of each piece. The thing
+ // to keep in mind is that the provided gradient definition may implicitly use p=0 and p=1.
+ Sk4f blend(0.0);
+ // Bake 1/(colorCount - 1) uniform stop difference into this scale factor
+ SkScalar wScale = pos ? 0.5 : 0.5 / (colorCount - 1);
+ for (int i = 0; i < colorCount - 1; ++i) {
+ // Calculate the average color for the interval between pos(i) and pos(i+1)
+ Sk4f c0 = Sk4f::Load(&colors[i]);
+ Sk4f c1 = Sk4f::Load(&colors[i + 1]);
+ // when pos == null, there are colorCount uniformly distributed stops, going from 0 to 1,
+ // so pos[i + 1] - pos[i] = 1/(colorCount-1)
+ SkScalar w = pos ? (pos[i + 1] - pos[i]) : SK_Scalar1;
+ blend += wScale * w * (c1 + c0);
+ }
+
+ // Now account for any implicit intervals at the start or end of the stop definitions
+ if (pos) {
+ if (pos[0] > 0.0) {
+ // The first color is fixed between p = 0 to pos[0], so 0.5 * (ci + cj) * (pj - pi)
+ // becomes 0.5 * (c + c) * (pj - 0) = c * pj
+ Sk4f c = Sk4f::Load(&colors[0]);
+ blend += pos[0] * c;
+ }
+ if (pos[colorCount - 1] < SK_Scalar1) {
+ // The last color is fixed between pos[n-1] to p = 1, so 0.5 * (ci + cj) * (pj - pi)
+ // becomes 0.5 * (c + c) * (1 - pi) = c * (1 - pi)
+ Sk4f c = Sk4f::Load(&colors[colorCount - 1]);
+ blend += (1 - pos[colorCount - 1]) * c;
+ }
+ }
+
+ SkColor4f avg;
+ blend.store(&avg);
+ return avg;
+}
+
+// Except for special circumstances of clamped gradients, every gradient shape--when degenerate--
+// can be mapped to the same fallbacks. The specific shape factories must account for special
+// clamped conditions separately, this will always return the last color for clamped gradients.
+static sk_sp<SkShader> make_degenerate_gradient(const SkColor4f colors[], const SkScalar pos[],
+ int colorCount, sk_sp<SkColorSpace> colorSpace,
+ SkShader::TileMode mode) {
+ switch(mode) {
+ case SkShader::kDecal_TileMode:
+ // normally this would reject the area outside of the interpolation region, so since
+ // inside region is empty when the radii are equal, the entire draw region is empty
+ return SkShader::MakeEmptyShader();
+ case SkShader::kRepeat_TileMode:
+ case SkShader::kMirror_TileMode:
+ // repeat and mirror are treated the same: the border colors are never visible,
+ // but approximate the final color as infinite repetitions of the colors, so
+ // it can be represented as the average color of the gradient.
+ return SkShader::MakeColorShader(
+ average_gradient_color(colors, pos, colorCount), std::move(colorSpace));
+ case SkShader::kClamp_TileMode:
+ // Depending on how the gradient shape degenerates, there may be a more specialized
+ // fallback representation for the factories to use, but this is a reasonable default.
+ return SkShader::MakeColorShader(colors[colorCount - 1], std::move(colorSpace));
+ default:
+ SkDEBUGFAIL("Should not be reached");
+ return nullptr;
+ }
+}
+
// assumes colors is SkColor4f* and pos is SkScalar*
#define EXPAND_1_COLOR(count) \
SkColor4f tmp[2]; \
@@ -618,6 +686,14 @@
return nullptr;
}
+ if (SkScalarNearlyZero((pts[1] - pts[0]).length())) {
+ // Degenerate gradient, the only tricky complication is when in clamp mode, the limit of
+ // the gradient approaches two half planes of solid color (first and last). However, they
+ // are divided by the line perpendicular to the start and end point, which becomes undefined
+ // once start and end are exactly the same, so just use the end color for a stable solution.
+ return make_degenerate_gradient(colors, pos, colorCount, std::move(colorSpace), mode);
+ }
+
ColorStopOptimizer opt(colors, pos, colorCount, mode);
SkGradientShaderBase::Descriptor desc;
@@ -644,7 +720,7 @@
SkShader::TileMode mode,
uint32_t flags,
const SkMatrix* localMatrix) {
- if (radius <= 0) {
+ if (radius < 0) {
return nullptr;
}
if (!valid_grad(colors, pos, colorCount, mode)) {
@@ -657,6 +733,11 @@
return nullptr;
}
+ if (SkScalarNearlyZero(radius)) {
+ // Degenerate gradient optimization, and no special logic needed for clamped radial gradient
+ return make_degenerate_gradient(colors, pos, colorCount, std::move(colorSpace), mode);
+ }
+
ColorStopOptimizer opt(colors, pos, colorCount, mode);
SkGradientShaderBase::Descriptor desc;
@@ -694,19 +775,40 @@
if (startRadius < 0 || endRadius < 0) {
return nullptr;
}
- if (SkScalarNearlyZero((start - end).length()) && SkScalarNearlyZero(startRadius)) {
- // We can treat this gradient as radial, which is faster.
- return MakeRadial(start, endRadius, colors, std::move(colorSpace), pos, colorCount,
- mode, flags, localMatrix);
- }
if (!valid_grad(colors, pos, colorCount, mode)) {
return nullptr;
}
- if (startRadius == endRadius) {
- if (start == end || startRadius == 0) {
- return SkShader::MakeEmptyShader();
+ if (SkScalarNearlyZero((start - end).length())) {
+ // If the center positions are the same, then the gradient is the radial variant of a 2 pt
+ // conical gradient, an actual radial gradient (startRadius == 0), or it is fully degenerate
+ // (startRadius == endRadius).
+ if (SkScalarNearlyEqual(startRadius, endRadius)) {
+ // Degenerate case, where the interpolation region area approaches zero. The proper
+ // behavior depends on the tile mode, which is consistent with the default degenerate
+ // gradient behavior, except when mode = clamp and the radii > 0.
+ if (mode == SkShader::TileMode::kClamp_TileMode && endRadius > SK_ScalarNearlyZero) {
+ // The interpolation region becomes an infinitely thin ring at the radius, so the
+ // final gradient will be the first color repeated from p=0 to 1, and then a hard
+ // stop switching to the last color at p=1.
+ static constexpr SkScalar circlePos[3] = {0, 1, 1};
+ SkColor4f reColors[3] = {colors[0], colors[0], colors[colorCount - 1]};
+ return MakeRadial(start, endRadius, reColors, std::move(colorSpace),
+ circlePos, 3, mode, flags, localMatrix);
+ } else {
+ // Otherwise use the default degenerate case
+ return make_degenerate_gradient(
+ colors, pos, colorCount, std::move(colorSpace), mode);
+ }
+ } else if (SkScalarNearlyZero(startRadius)) {
+ // We can treat this gradient as radial, which is faster. If we got here, we know
+ // that endRadius is not equal to 0, so this produces a meaningful gradient
+ return MakeRadial(start, endRadius, colors, std::move(colorSpace), pos, colorCount,
+ mode, flags, localMatrix);
}
+ // Else it's the 2pt conical radial variant with no degenerate radii, so fall through to the
+ // regular 2pt constructor.
}
+
if (localMatrix && !localMatrix->invert(nullptr)) {
return nullptr;
}
@@ -750,13 +852,29 @@
if (1 == colorCount) {
return SkShader::MakeColorShader(colors[0], std::move(colorSpace));
}
- if (!SkScalarIsFinite(startAngle) || !SkScalarIsFinite(endAngle) || startAngle >= endAngle) {
+ if (!SkScalarIsFinite(startAngle) || !SkScalarIsFinite(endAngle) || startAngle > endAngle) {
return nullptr;
}
if (localMatrix && !localMatrix->invert(nullptr)) {
return nullptr;
}
+ if (SkScalarNearlyEqual(startAngle, endAngle)) {
+ // Degenerate gradient, which should follow default degenerate behavior unless it is
+ // clamped and the angle is greater than 0.
+ if (mode == SkShader::kClamp_TileMode && endAngle > SK_ScalarNearlyZero) {
+ // In this case, the first color is repeated from 0 to the angle, then a hardstop
+ // switches to the last color (all other colors are compressed to the infinitely thin
+ // interpolation region).
+ static constexpr SkScalar clampPos[3] = {0, 1, 1};
+ SkColor4f reColors[3] = {colors[0], colors[0], colors[colorCount - 1]};
+ return MakeSweep(cx, cy, reColors, std::move(colorSpace), clampPos, 3, mode, 0,
+ endAngle, flags, localMatrix);
+ } else {
+ return make_degenerate_gradient(colors, pos, colorCount, std::move(colorSpace), mode);
+ }
+ }
+
if (startAngle <= 0 && endAngle >= 360) {
// If the t-range includes [0,1], then we can always use clamping (presumably faster).
mode = SkShader::kClamp_TileMode;
diff --git a/src/shaders/gradients/SkTwoPointConicalGradient.cpp b/src/shaders/gradients/SkTwoPointConicalGradient.cpp
index 49d0631..469c8ad 100644
--- a/src/shaders/gradients/SkTwoPointConicalGradient.cpp
+++ b/src/shaders/gradients/SkTwoPointConicalGradient.cpp
@@ -55,8 +55,10 @@
Type gradientType;
if (SkScalarNearlyZero((c0 - c1).length())) {
- if (SkScalarNearlyZero(SkTMax(r0, r1))) {
- return nullptr; // Degenerate case; avoid dividing by zero.
+ if (SkScalarNearlyZero(SkTMax(r0, r1)) || SkScalarNearlyEqual(r0, r1)) {
+ // Degenerate case; avoid dividing by zero. Should have been caught by caller but
+ // just in case, recheck here.
+ return nullptr;
}
// Concentric case: we can pretend we're radial (with a tiny twist).
const SkScalar scale = sk_ieee_float_divide(1, SkTMax(r0, r1));