bug fix in matrix color filter

The current code for this filter premuls, then clamps.  We should be
clamping, then premulling.

If the matrix makes alpha greater than 1, these two orderings can result
in different color values.  Alpha will clamp to 1 either way, but the
color channels are multiplied by that >1 alpha in one case, and by =1
in the other.

The left column of the gm imagefilterscropexpand demonstrates this.
Its matrix adds 32/255 to alpha and 255/255 to green.  This produces
alpha ~= 1.12.  That's then multiplied by the relatively small red
and blue values in the grey checkerboard, resulting in different
in-range values than the ones we would have gotten if we clamped
alpha first.  Green wasn't affected because it was already fully
saturated.  255 * 1.12 == 255 no matter when we clamp.

Change-Id: I4b30bf64c30fe62526674ad5f32e9ca19ec84714
Reviewed-on: https://skia-review.googlesource.com/77902
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Mike Klein <mtklein@chromium.org>

1 files changed, 20 insertions, 0 deletions

diff --git a/tests/ColorMatrixTest.cpp b/tests/ColorMatrixTest.cpp
index 7919b40242..baef52088b 100644
--- a/tests/ColorMatrixTest.cpp
+++ b/tests/ColorMatrixTest.cpp
@@ -99,3 +99,23 @@ static inline void test_colorMatrixCTS(skiatest::Reporter* reporter) {
 DEF_TEST(ColorMatrix, reporter) {
     test_colorMatrixCTS(reporter);
 }
+
+
+DEF_TEST(ColorMatrix_clamp_while_unpremul, r) {
+    // This matrix does green += 255/255 and alpha += 32/255.  We want to test
+    // that if we pass it opaque alpha and small red and blue values, red and
+    // blue stay unchanged, not pumped up by that ~1.12 intermediate alpha.
+    SkScalar m[] = {
+        1, 0, 0, 0, 0,
+        0, 1, 0, 0, 255,
+        0, 0, 1, 0, 0,
+        0, 0, 0, 1, 32,
+    };
+    auto filter = SkColorFilter::MakeMatrixFilterRowMajor255(m);
+
+    SkColor filtered = filter->filterColor(0xff0a0b0c);
+    REPORTER_ASSERT(r, SkColorGetA(filtered) == 0xff);
+    REPORTER_ASSERT(r, SkColorGetR(filtered) == 0x0a);
+    REPORTER_ASSERT(r, SkColorGetG(filtered) == 0xff);
+    REPORTER_ASSERT(r, SkColorGetB(filtered) == 0x0c);
+}