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</facepalm>):

  | 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

4 files changed, 30 insertions, 50 deletions

diff --git a/gm/lumafilter.cpp b/gm/lumafilter.cpp
index 69a4f073f0..c12bae0ba5 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 fef06ab12a..a0c08bbb98 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 eff1645d27..fd4826b5a9 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 b333ad68f0..8c96d752fb 100644
--- a/tests/ColorFilterTest.cpp
+++ b/tests/ColorFilterTest.cpp
@@ -101,11 +101,14 @@ static void test_lumaColorFilter(skiatest::Reporter* reporter) {
     SkPMColor in, out;
     SkAutoTUnref<SkColorFilter> 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);
     }
 }