guard SkTableColorFilter against out-of-range inputs
I was going to be clever here and only clamp when we
know the inputs are out of range, but this filter is
rare and slow enough that I think I'd rather it just
be super paranoid safe.
The test crashes without this fix and passes with it.
Change-Id: I4e17aad2b5c1e96180ce8d73b97bee746cf985c2
Reviewed-on: https://skia-review.googlesource.com/26702
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
diff --git a/gn/tests.gni b/gn/tests.gni
index f2737fd..7b2d25c 100644
--- a/gn/tests.gni
+++ b/gn/tests.gni
@@ -238,6 +238,7 @@
"$_tests/SwizzlerTest.cpp",
"$_tests/TArrayTest.cpp",
"$_tests/TDPQueueTest.cpp",
+ "$_tests/TableColorFilterTest.cpp",
"$_tests/TemplatesTest.cpp",
"$_tests/TessellatingPathRendererTests.cpp",
"$_tests/Test.cpp",
diff --git a/src/effects/SkTableColorFilter.cpp b/src/effects/SkTableColorFilter.cpp
index d763358..6472ccb 100644
--- a/src/effects/SkTableColorFilter.cpp
+++ b/src/effects/SkTableColorFilter.cpp
@@ -112,6 +112,13 @@
if (fFlags & kG_Flag) { g = ptr; ptr += 256; }
if (fFlags & kB_Flag) { b = ptr; }
+ // If our inputs are out of range, we'd attempt to read values outside our tables.
+ // We could finesse this with p->clamp_if_unclamped(kPremul_SkAlphaType) here, but
+ // this filter is already slow enough that I'd rather just be paranoid and safe.
+ p->append(SkRasterPipeline::clamp_0);
+ p->append(SkRasterPipeline::clamp_a);
+ p->set_clamped(true);
+
if (!shaderIsOpaque) {
p->append(SkRasterPipeline::unpremul);
}
diff --git a/tests/TableColorFilterTest.cpp b/tests/TableColorFilterTest.cpp
new file mode 100644
index 0000000..baf6a47
--- /dev/null
+++ b/tests/TableColorFilterTest.cpp
@@ -0,0 +1,37 @@
+/*
+ * Copyright 2017 Google Inc.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#include "Test.h"
+
+#include "SkColorSpace.h"
+#include "SkTableColorFilter.h"
+#include "SkToSRGBColorFilter.h"
+
+// SkToSRGBColorFilter makes it easy to create out of range (>1, <0) color values.
+// Those can be dangerous as inputs to naive implementation of SkTableColorFilter.
+// This tests that our implementation is safe.
+
+DEF_TEST(TableColorFilter, r) {
+ // Using a wide source gamut will make saturated colors go well out of range of sRGB.
+ auto rec2020 = SkColorSpace::MakeRGB(SkColorSpace::kSRGB_RenderTargetGamma,
+ SkColorSpace::kRec2020_Gamut);
+ sk_sp<SkColorFilter> to_srgb = SkToSRGBColorFilter::Make(rec2020);
+
+ // Any table will work fine here. An identity table makes testing easy.
+ uint8_t identity[256];
+ for (int i = 0; i < 256; i++) {
+ identity[i] = i;
+ }
+ sk_sp<SkColorFilter> table = SkTableColorFilter::Make(identity);
+
+ // The rec2020 primaries are not representable in sRGB, but will clamp to the sRGB primaries.
+ SkColor colors[] = { SK_ColorRED, SK_ColorGREEN, SK_ColorBLUE };
+ sk_sp<SkColorFilter> composed = SkColorFilter::MakeComposeFilter(table, to_srgb);
+ for (auto color : colors) {
+ REPORTER_ASSERT(r, composed->filterColor(color) == color);
+ }
+}
