From d53f9a1913a3e5b3592f134e5a0836dd90491439 Mon Sep 17 00:00:00 2001 From: Mike Klein Date: Wed, 29 Nov 2017 16:31:37 -0500 Subject: 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 Commit-Queue: Mike Klein --- tests/ColorMatrixTest.cpp | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) (limited to 'tests/ColorMatrixTest.cpp') 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); +} -- cgit v1.2.3