Skip to content

Commit

Permalink
Revert "Improve degenerate 2pt conical gradient cases"
Browse files Browse the repository at this point in the history 
This reverts commit 879dab8.

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", c34dd6c, 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 <[email protected]>
> > Commit-Queue: Michael Ludwig <[email protected]>
> 
> 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 <[email protected]>
> Reviewed-by: Florin Malita <[email protected]>
> Commit-Queue: Michael Ludwig <[email protected]>

[email protected],[email protected],[email protected],[email protected],[email protected]

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 <[email protected]>
Commit-Queue: Stan Iliev <[email protected]>
  • Loading branch information
@staniliev
staniliev authored and Skia Commit-Bot committed Nov 9, 2018
1 parent 0b473fb commit 95af472
Show file tree
Hide file tree
Showing 5 changed files with 13 additions and 313 deletions.
150 changes: 0 additions & 150 deletions gm/gradients_degenerate.cpp
View file

This file was deleted.

1 change: 0 additions & 1 deletion gn/gm.gni
View file
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,6 @@ gm_sources = [
"$_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",
Expand Down
31 changes: 1 addition & 30 deletions include/effects/SkGradientShader.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,36 +13,7 @@
/** \class SkGradientShader
SkGradientShader hosts factories for creating subclasses of SkShader that
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.
render linear and radial gradients.
*/
class SK_API SkGradientShader {
public:
Expand Down
138 changes: 10 additions & 128 deletions src/shaders/gradients/SkGradientShader.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -517,74 +517,6 @@ static void desc_init(SkGradientShaderBase::Descriptor* desc,
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]; \
Expand Down Expand Up @@ -686,14 +618,6 @@ sk_sp<SkShader> SkGradientShader::MakeLinear(const SkPoint pts[2],
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;
Expand All @@ -720,7 +644,7 @@ sk_sp<SkShader> SkGradientShader::MakeRadial(const SkPoint& center, SkScalar rad
SkShader::TileMode mode,
uint32_t flags,
const SkMatrix* localMatrix) {
if (radius < 0) {
if (radius <= 0) {
return nullptr;
}
if (!valid_grad(colors, pos, colorCount, mode)) {
Expand All @@ -733,11 +657,6 @@ sk_sp<SkShader> SkGradientShader::MakeRadial(const SkPoint& center, SkScalar rad
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;
Expand Down Expand Up @@ -775,40 +694,19 @@ sk_sp<SkShader> SkGradientShader::MakeTwoPointConical(const SkPoint& start,
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 (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);
if (startRadius == endRadius) {
if (start == end || startRadius == 0) {
return SkShader::MakeEmptyShader();
}
// 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;
}
Expand Down Expand Up @@ -852,29 +750,13 @@ sk_sp<SkShader> SkGradientShader::MakeSweep(SkScalar cx, SkScalar cy,
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;
Expand Down
Loading

0 comments on commit 95af472

Please sign in to comment.