AuroraMiddleware/skia2
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

bug fix in matrix color filter

Browse Source 
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>
This commit is contained in:
Mike Klein 2017-11-29 16:31:37 -05:00 committed by Skia Commit-Bot
parent edfa0d2f62
commit d53f9a1913
2 changed files with 22 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
src/core/SkColorMatrixFilterRowMajor255.cpp
View File

@ -158,9 +158,9 @@ void SkColorMatrixFilterRowMajor255::onAppendStages(SkRasterPipeline* p,
if (!shaderIsOpaque) { p->append(SkRasterPipeline::unpremul); }
if ( true) { p->append(SkRasterPipeline::matrix_4x5, fTranspose); }
if (!willStayOpaque) { p->append(SkRasterPipeline::premul); }
if ( needsClamp0) { p->append(SkRasterPipeline::clamp_0); }
if ( needsClamp1) { p->append(SkRasterPipeline::clamp_a); }
if ( needsClamp1) { p->append(SkRasterPipeline::clamp_1); }
if (!willStayOpaque) { p->append(SkRasterPipeline::premul); }
}
sk_sp<SkColorFilter>

20
tests/ColorMatrixTest.cpp
View File

@ -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);
}