Fix color stop positions when computing degenerate gradient color
For non-degenerate gradients, the positions are pinned to [0, 1] and
forced to be monotonically increasing in the constructor of
SkGradientShaderBase. This isn't ever called for degenerate gradients,
so the average_gradient_color helper function needs to make the same
fixes or it may calculate an invalid color (e.g. negative alpha because
the original positions are not sorted).
Bug: chromium:1149216
Change-Id: I95c6e66ebb57722370ef2f5dbba3d8d66727e48b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/336437
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
diff --git a/src/shaders/gradients/SkGradientShader.cpp b/src/shaders/gradients/SkGradientShader.cpp
index 75690ee..82aaf77 100644
--- a/src/shaders/gradients/SkGradientShader.cpp
+++ b/src/shaders/gradients/SkGradientShader.cpp
@@ -703,28 +703,38 @@
// 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])
- : (1.0f / (colorCount - 1));
+ SkScalar w;
+ if (pos) {
+ // Match position fixing in SkGradientShader's constructor, clamping positions outside
+ // [0, 1] and forcing the sequence to be monotonic
+ SkScalar p0 = SkTPin(pos[i], 0.f, 1.f);
+ SkScalar p1 = SkTPin(pos[i + 1], p0, 1.f);
+ w = p1 - p0;
+
+ // And account for any implicit intervals at the start or end of the positions
+ if (i == 0) {
+ if (p0 > 0.0f) {
+ // 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 += p0 * c;
+ }
+ }
+ if (i == colorCount - 2) {
+ if (p1 < 1.f) {
+ // 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.f - p1) * c;
+ }
+ }
+ } else {
+ w = 1.f / (colorCount - 1);
+ }
blend += 0.5f * 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;
diff --git a/tests/GradientTest.cpp b/tests/GradientTest.cpp
index bf70a9c..9595ffa 100644
--- a/tests/GradientTest.cpp
+++ b/tests/GradientTest.cpp
@@ -11,6 +11,7 @@
#include "include/core/SkSurface.h"
#include "include/effects/SkGradientShader.h"
#include "include/private/SkTemplates.h"
+#include "src/core/SkMatrixProvider.h"
#include "src/core/SkTLazy.h"
#include "src/shaders/SkColorShader.h"
#include "tests/Test.h"
@@ -368,6 +369,36 @@
// Passes if we don't trigger asserts.
}
+// http://crbug.com/1149216
+static void test_unsorted_degenerate(skiatest::Reporter* r) {
+ // Passes if a valid solid color is computed for the degenerate gradient
+ // (unsorted positions are fixed during regular gradient construction, so this ensures the
+ // same fixing happens for degenerate gradients as well). If they aren't fixed, this test
+ // case produces a negative alpha, which asserts during SkPMColor4f::isOpaque().
+ const SkColor4f colors[] = { {0.f, 0.f, 0.f, 0.f},
+ {0.00784314f, 0.f, 0.f, 0.0627451f},
+ {0.f, 0.00392157f, 0.f, 0.f} };
+ const SkScalar positions[] = {0.00753367f, 8.54792e-44f, 1.46955e-39f};
+
+ const SkPoint points[] { { 0.f, 0.f }, { 1e-20f, -1e-8f }}; // must be degenerate
+ // Use kMirror to go through average color stop calculation, vs. kClamp which would pick a color
+ sk_sp<SkShader> gradient = SkGradientShader::MakeLinear(points, colors, nullptr, positions, 3,
+ SkTileMode::kMirror);
+
+ // The degenerate gradient shouldn't be null
+ REPORTER_ASSERT(r, SkToBool(gradient));
+ // And it shouldn't crash when creating a fragment processor
+
+ SkSimpleMatrixProvider provider(SkMatrix::I());
+ GrColorInfo dstColorInfo(GrColorType::kRGBA_8888, kPremul_SkAlphaType,
+ SkColorSpace::MakeSRGB());
+ GrMockOptions options;
+ auto context = GrDirectContext::MakeMock(&options);
+
+ GrFPArgs args(context.get(), provider, kNone_SkFilterQuality, &dstColorInfo);
+ as_SB(gradient)->asFragmentProcessor(args);
+}
+
// "Interesting" fuzzer values.
static void test_linear_fuzzer(skiatest::Reporter*) {
static const SkColor gColors0[] = { 0x30303030, 0x30303030 };
@@ -550,4 +581,5 @@
test_degenerate_linear(reporter);
test_linear_fuzzer(reporter);
test_sweep_fuzzer(reporter);
+ test_unsorted_degenerate(reporter);
}