Luminance-to-alpha color filter (SkLumaColorFilter).

Luminance-to-alpha color filter (SkLumaColorFilter).

Adding a color filter luma implementation. The plan is to convert
existing clients and then deprecate SkLumaXfermode.

R=reed@google.com,bsalomon@google.com,robertphillips@google.com

Committed: http://code.google.com/p/skia/source/detail?r=11636

[reed1, 2013-10-01 16:10:35]

https://codereview.chromium.org/25453004/diff/1/include/effects/SkLumaColorFi...
File include/effects/SkLumaColorFilter.h (right):

https://codereview.chromium.org/25453004/diff/1/include/effects/SkLumaColorFi...
include/effects/SkLumaColorFilter.h:24: class SK_API SkLumaColorFilter : public
SkColorFilter {
Is it also correct to document this as:

"scale each color by its luminance"?

r' = r / a
...
L = func(r', g', b')
a * L = func(r, g, b)
L = func(r, g, b) / a

C' = [func(r, g, b), L * r, L * g, L * b]
?

[f(malita), 2013-10-01 16:26:49]

https://codereview.chromium.org/25453004/diff/1/include/effects/SkLumaColorFi...
File include/effects/SkLumaColorFilter.h (right):

https://codereview.chromium.org/25453004/diff/1/include/effects/SkLumaColorFi...
include/effects/SkLumaColorFilter.h:24: class SK_API SkLumaColorFilter : public
SkColorFilter {
On 2013/10/01 16:10:35, reed1 wrote:
> Is it also correct to document this as:
> 
> "scale each color by its luminance"?
> 
> r' = r / a
> ...
> L = func(r', g', b')
> a * L = func(r, g, b)
> L = func(r, g, b) / a
> 
> C' = [func(r, g, b), L * r, L * g, L * b]
> ?

Yes, in premul space that looks correct. And since color filters operate in
premul, it's probably a better way to document.

[reed1, 2013-10-01 16:53:26]

On 2013/10/01 16:26:49, Florin Malita wrote:
>
https://codereview.chromium.org/25453004/diff/1/include/effects/SkLumaColorFi...
> File include/effects/SkLumaColorFilter.h (right):
> 
>
https://codereview.chromium.org/25453004/diff/1/include/effects/SkLumaColorFi...
> include/effects/SkLumaColorFilter.h:24: class SK_API SkLumaColorFilter :
public
> SkColorFilter {
> On 2013/10/01 16:10:35, reed1 wrote:
> > Is it also correct to document this as:
> > 
> > "scale each color by its luminance"?
> > 
> > r' = r / a
> > ...
> > L = func(r', g', b')
> > a * L = func(r, g, b)
> > L = func(r, g, b) / a
> > 
> > C' = [func(r, g, b), L * r, L * g, L * b]
> > ?
> 
> Yes, in premul space that looks correct. And since color filters operate in
> premul, it's probably a better way to document.

Might also simplify/speedup your impl

[f(malita), 2013-10-02 16:48:41]

On 2013/10/01 16:53:26, reed1 wrote:
> On 2013/10/01 16:26:49, Florin Malita wrote:
> >
>
https://codereview.chromium.org/25453004/diff/1/include/effects/SkLumaColorFi...
> > File include/effects/SkLumaColorFilter.h (right):
> > 
> >
>
https://codereview.chromium.org/25453004/diff/1/include/effects/SkLumaColorFi...
> > include/effects/SkLumaColorFilter.h:24: class SK_API SkLumaColorFilter :
> public
> > SkColorFilter {
> > On 2013/10/01 16:10:35, reed1 wrote:
> > > Is it also correct to document this as:
> > > 
> > > "scale each color by its luminance"?
> > > 
> > > r' = r / a
> > > ...
> > > L = func(r', g', b')
> > > a * L = func(r, g, b)
> > > L = func(r, g, b) / a
> > > 
> > > C' = [func(r, g, b), L * r, L * g, L * b]
> > > ?
> > 
> > Yes, in premul space that looks correct. And since color filters operate in
> > premul, it's probably a better way to document.
> 
> Might also simplify/speedup your impl

Yup, I think we can avoid two un-premultiply ops. PTAL.

[reed1, 2013-10-03 09:08:15]

suggested tests:

1. draw black w/ this filter, and confirm that it was a no-op (since lum should
come out to 0, and hence we're completely transparent).

2. draw various transparency levels of white, and check that the output is the
same as w/o the filter (since luminance should be 1).

https://codereview.chromium.org/25453004/diff/22001/src/effects/SkLumaColorFi...
File src/effects/SkLumaColorFilter.cpp (right):

https://codereview.chromium.org/25453004/diff/22001/src/effects/SkLumaColorFi...
src/effects/SkLumaColorFilter.cpp:35: // with the luminance computed in PM
space.
I think this is correct, but perhaps we can paste in a comment block somewhere
describing how we arrived at this impl.

[bsalomon, 2013-10-03 14:52:13]

On 2013/10/03 09:08:15, reed1 wrote:
> suggested tests:
> 
> 1. draw black w/ this filter, and confirm that it was a no-op (since lum
should
> come out to 0, and hence we're completely transparent).
> 
> 2. draw various transparency levels of white, and check that the output is the
> same as w/o the filter (since luminance should be 1).
> 
>
https://codereview.chromium.org/25453004/diff/22001/src/effects/SkLumaColorFi...
> File src/effects/SkLumaColorFilter.cpp (right):
> 
>
https://codereview.chromium.org/25453004/diff/22001/src/effects/SkLumaColorFi...
> src/effects/SkLumaColorFilter.cpp:35: // with the luminance computed in PM
> space.
> I think this is correct, but perhaps we can paste in a comment block somewhere
> describing how we arrived at this impl.

the gpu part lgtm

[reed1, 2013-10-03 14:54:04]

On 2013/10/03 14:52:13, bsalomon wrote:
> On 2013/10/03 09:08:15, reed1 wrote:
> > suggested tests:
> > 
> > 1. draw black w/ this filter, and confirm that it was a no-op (since lum
> should
> > come out to 0, and hence we're completely transparent).
> > 
> > 2. draw various transparency levels of white, and check that the output is
the
> > same as w/o the filter (since luminance should be 1).
> > 
> >
>
https://codereview.chromium.org/25453004/diff/22001/src/effects/SkLumaColorFi...
> > File src/effects/SkLumaColorFilter.cpp (right):
> > 
> >
>
https://codereview.chromium.org/25453004/diff/22001/src/effects/SkLumaColorFi...
> > src/effects/SkLumaColorFilter.cpp:35: // with the luminance computed in PM
> > space.
> > I think this is correct, but perhaps we can paste in a comment block
somewhere
> > describing how we arrived at this impl.
> 
> the gpu part lgtm

+1 if we can start running GM with a (fuzzy?) mode that compares CPU and GPU
output...

[commit-bot: I haz the power, 2013-10-07 17:49:17]

CQ is trying da patch. Follow status at
https://skia-tree-status.appspot.com/cq/fmalita@chromium.org/25453004/31001

[commit-bot: I haz the power, 2013-10-07 18:00:20]

Change committed as 11636