From d494b09f554d470fc6411d0924879bbfb0cb0e95 Mon Sep 17 00:00:00 2001 From: "commit-bot@chromium.org" Date: Thu, 10 Oct 2013 20:13:51 +0000 Subject: [PATCH] Align SkLumaColorFilter with the spec. The spec requires the resulting RGB channels for LuminanceToAlpha to be 0 (and not just scaled by the luminance value): | R' | | 0 0 0 0 0 | | R | | G' | | 0 0 0 0 0 | | G | | B' | = | 0 0 0 0 0 | * | B | | A' | | 0.2125 0.7154 0.0721 0 0 | | A | | 1 | | 0 0 0 0 1 | | 1 | (http://www.w3.org/TR/2011/REC-SVG11-20110816/filters.html#feColorMatrixElement) This doesn't affect luminance masking (which depends only on the resulting alpha channel), but other color filter users may care about all color components. R=bsalomon@google.com, reed@google.com, robertphillips@google.com Author: fmalita@chromium.org Review URL: https://codereview.chromium.org/26467003 git-svn-id: http://skia.googlecode.com/svn/trunk@11713 2bbb7eff-a529-9590-31e7-b0007b416f81 --- gm/lumafilter.cpp | 1 + include/effects/SkLumaColorFilter.h | 5 ++-- src/effects/SkLumaColorFilter.cpp | 59 +++++++++++-------------------------- tests/ColorFilterTest.cpp | 15 ++++++---- 4 files changed, 30 insertions(+), 50 deletions(-) diff --git a/gm/lumafilter.cpp b/gm/lumafilter.cpp index 69a4f07..c12bae0 100644 --- a/gm/lumafilter.cpp +++ b/gm/lumafilter.cpp @@ -34,6 +34,7 @@ static void draw_scene(SkCanvas* canvas, SkColorFilter* filter, c = bounds; c.fRight = bounds.centerX(); + paint.setARGB(0x20, 0, 0, 0xff); canvas->drawRect(bounds, paint); canvas->saveLayer(&bounds, NULL); diff --git a/include/effects/SkLumaColorFilter.h b/include/effects/SkLumaColorFilter.h index fef06ab..a0c08bb 100644 --- a/include/effects/SkLumaColorFilter.h +++ b/include/effects/SkLumaColorFilter.h @@ -15,9 +15,10 @@ * http://www.w3.org/TR/SVG/masking.html#Masking * http://www.w3.org/TR/css-masking/#MaskValues * - * Each color is scaled by the (unpremultiplied) luminance value: + * The resulting color is black with transparency equal to the + * luminance value modulated by alpha: * - * C' = [Lum * a, Lum * r, Lum * g, Lum * b] + * C' = [ Lum * a, 0, 0, 0 ] * */ class SK_API SkLumaColorFilter : public SkColorFilter { diff --git a/src/effects/SkLumaColorFilter.cpp b/src/effects/SkLumaColorFilter.cpp index eff1645..fd4826b 100644 --- a/src/effects/SkLumaColorFilter.cpp +++ b/src/effects/SkLumaColorFilter.cpp @@ -9,7 +9,6 @@ #include "SkColorPriv.h" #include "SkString.h" -#include "SkUnPreMultiply.h" #if SK_SUPPORT_GPU #include "gl/GrGLEffect.h" @@ -19,44 +18,21 @@ void SkLumaColorFilter::filterSpan(const SkPMColor src[], int count, SkPMColor dst[]) const { - const SkUnPreMultiply::Scale* table = SkUnPreMultiply::GetScaleTable(); - for (int i = 0; i < count; ++i) { SkPMColor c = src[i]; - unsigned r = SkGetPackedR32(c); - unsigned g = SkGetPackedG32(c); - unsigned b = SkGetPackedB32(c); - unsigned a = SkGetPackedA32(c); - - // No need to do anything for white (luminance == 1.0) - if (a != r || a != g || a != b) { - /* - * To avoid un-premultiplying multiple components, we can start - * with the luminance computed in PM space: - * - * Lum = i * (r / a) + j * (g / a) + k * (b / a) - * Lum = (i * r + j * g + k * b) / a - * Lum = Lum'(PM) / a - * - * Then the filter function is: - * - * C' = [ Lum * a, Lum * r, Lum * g, Lum * b ] - * - * which is equivalent to: - * - * C' = [ Lum'(PM), Lum * r, Lum * g, Lum * b ] - */ - unsigned pm_lum = SkComputeLuminance(r, g, b); - unsigned lum = SkUnPreMultiply::ApplyScale(table[a], pm_lum); - - c = SkPackARGB32(pm_lum, - SkMulDiv255Round(r, lum), - SkMulDiv255Round(g, lum), - SkMulDiv255Round(b, lum)); - } - - dst[i] = c; + /* + * While LuminanceToAlpha is defined to operate on un-premultiplied + * inputs, due to the final alpha scaling it can be computed based on + * premultipled components: + * + * LumA = (k1 * r / a + k2 * g / a + k3 * b / a) * a + * LumA = (k1 * r + k2 * g + k3 * b) + */ + unsigned luma = SkComputeLuminance(SkGetPackedR32(c), + SkGetPackedG32(c), + SkGetPackedB32(c)); + dst[i] = SkPackARGB32(luma, 0, 0, 0); } } @@ -97,7 +73,9 @@ public: virtual void getConstantColorComponents(GrColor* color, uint32_t* validFlags) const SK_OVERRIDE { - *validFlags = 0; + // The output is always black. + *color = GrColorPackRGBA(0, 0, 0, GrColorUnpackA(*color)); + *validFlags = kRGB_GrColorComponentFlags; } class GLEffect : public GrGLEffect { @@ -123,16 +101,13 @@ public: inputColor = "vec4(1)"; } - // The max() is to guard against 0 / 0 during unpremul when the incoming color is - // transparent black. - builder->fsCodeAppendf("\tfloat nonZeroAlpha = max(%s.a, 0.00001);\n", inputColor); builder->fsCodeAppendf("\tfloat luma = dot(vec3(%f, %f, %f), %s.rgb);\n", SK_ITU_BT709_LUM_COEFF_R, SK_ITU_BT709_LUM_COEFF_G, SK_ITU_BT709_LUM_COEFF_B, inputColor); - builder->fsCodeAppendf("\t%s = vec4(%s.rgb * luma / nonZeroAlpha, luma);\n", - outputColor, inputColor); + builder->fsCodeAppendf("\t%s = vec4(0, 0, 0, luma);\n", + outputColor); } diff --git a/tests/ColorFilterTest.cpp b/tests/ColorFilterTest.cpp index b333ad6..8c96d75 100644 --- a/tests/ColorFilterTest.cpp +++ b/tests/ColorFilterTest.cpp @@ -101,11 +101,14 @@ static void test_lumaColorFilter(skiatest::Reporter* reporter) { SkPMColor in, out; SkAutoTUnref lf(SkLumaColorFilter::Create()); - // Applying luma to white is a nop (luminance(white) == 1.0) + // Applying luma to white produces black with the same transparency. for (unsigned i = 0; i < 256; ++i) { in = SkPackARGB32(i, i, i, i); lf->filterSpan(&in, 1, &out); - REPORTER_ASSERT(reporter, out == in); + REPORTER_ASSERT(reporter, SkGetPackedA32(out) == i); + REPORTER_ASSERT(reporter, SkGetPackedR32(out) == 0); + REPORTER_ASSERT(reporter, SkGetPackedG32(out) == 0); + REPORTER_ASSERT(reporter, SkGetPackedB32(out) == 0); } // Applying luma to black yields transparent black (luminance(black) == 0) @@ -115,15 +118,15 @@ static void test_lumaColorFilter(skiatest::Reporter* reporter) { REPORTER_ASSERT(reporter, out == SK_ColorTRANSPARENT); } - // For general colors, a luma filter has an attenuating effect. + // For general colors, a luma filter generates black with an attenuated alpha channel. for (unsigned i = 1; i < 256; ++i) { in = SkPackARGB32(i, i, i / 2, i / 3); lf->filterSpan(&in, 1, &out); REPORTER_ASSERT(reporter, out != in); REPORTER_ASSERT(reporter, SkGetPackedA32(out) <= i); - REPORTER_ASSERT(reporter, SkGetPackedR32(out) <= i); - REPORTER_ASSERT(reporter, SkGetPackedG32(out) <= i / 2); - REPORTER_ASSERT(reporter, SkGetPackedB32(out) <= i / 3); + REPORTER_ASSERT(reporter, SkGetPackedR32(out) == 0); + REPORTER_ASSERT(reporter, SkGetPackedG32(out) == 0); + REPORTER_ASSERT(reporter, SkGetPackedB32(out) == 0); } } -- 2.7.4