Add color gamut xform helpers to GrGLSLShaderBuilder

Add color gamut xform helpers to GrGLSLShaderBuilder

New helper functions inject the necessary shader
function. Texture lookup functions can now insert
the gamut xform at the appropriate place, too.

As written, could be used to transform non-texture
colors (e.g. vertex colors) as well.

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2180803005

Committed: https://skia.googlesource.com/skia/+/77320dbabcddf05c0a1489eaf1f496729dc8de0e

[Brian Osman, 2016-07-26 23:26:24]

Description was changed from

==========
Adding color xform to appendTextureLookup*

BUG=skia:
==========

to

==========
Adding color xform to appendTextureLookup*

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2180803005
==========

[Brian Osman, 2016-07-27 20:25:31]

Description was changed from

==========
Adding color xform to appendTextureLookup*

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2180803005
==========

to

==========
Add color gamut xform helpers to GrGLSLShaderBuilder

New helper functions inject the necessary shader
function. Texture lookup functions can now insert
the gamut xform at the appropriate place, too.

As written, could be used to transform non-texture
colors (e.g. vertex colors) as well.

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2180803005
==========

[Brian Osman, 2016-08-12 18:35:17]

The CQ bit was checked by brianosman@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-08-12 18:35:23]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Brian Osman, 2016-08-12 19:27:28]

brianosman@google.com changed reviewers:
+ bsalomon@google.com, robertphillips@google.com

[Brian Osman, 2016-08-12 19:27:29]

Code I wrote before vacation - refactors the color xform shader support to
handle premul, and limit the amount of copy-paste code needed in other effects
to support the feature.

[commit-bot: I haz the power, 2016-08-12 19:45:15]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-12 19:45:16]

Dry run: This issue passed the CQ dry run.

[bsalomon, 2016-08-12 21:34:06]

looks good but should we pass a param to know whether the input color is premul
or not? Thinking ahead to adding support for non-premul SkImages.

[Brian Osman, 2016-08-16 16:18:54]

On 2016/08/12 21:34:06, bsalomon wrote:
> looks good but should we pass a param to know whether the input color is
premul
> or not? Thinking ahead to adding support for non-premul SkImages.

FYI: I'm working on doing this, but it requires plumbing/exposing alpha
type in several other places. I've already landed the change that adds
it to SkSpecialImage, but I'm still waiting to hear from reed@ if it's
okay on SkImage, and then we need it on GrTextureProducer, as well.

[Brian Osman, 2016-08-18 15:22:29]

The CQ bit was checked by brianosman@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-08-18 15:22:38]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-18 15:42:44]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-18 15:42:44]

Dry run: This issue passed the CQ dry run.

[Brian Osman, 2016-08-31 13:14:10]

Change from before the Android performance push. This adds gamut transformation
to more of the common image rendering paths. It also incorporates the premul vs.
non-premul support that Brian asked for.

[Brian Osman, 2016-08-31 13:15:56]

On 2016/08/31 13:14:10, Brian Osman wrote:
> Change from before the Android performance push. This adds gamut
transformation
> to more of the common image rendering paths. It also incorporates the premul
vs.
> non-premul support that Brian asked for.

FYI: I'm working on testing this in another change, although it's proving
tricky.
I'll be hammering on strategies for validating this stuff.

[bsalomon, 2016-08-31 17:45:47]

On 2016/08/31 13:15:56, Brian Osman wrote:
> On 2016/08/31 13:14:10, Brian Osman wrote:
> > Change from before the Android performance push. This adds gamut
> transformation
> > to more of the common image rendering paths. It also incorporates the premul
> vs.
> > non-premul support that Brian asked for.
> 
> FYI: I'm working on testing this in another change, although it's proving
> tricky.
> I'll be hammering on strategies for validating this stuff.

It is possible to right GMs that run against GrDrawContext, GrPaint, etc. Not
sure if that helps you at all.

[Brian Osman, 2016-08-31 17:49:53]

On 2016/08/31 17:45:47, bsalomon wrote:
> On 2016/08/31 13:15:56, Brian Osman wrote:
> > On 2016/08/31 13:14:10, Brian Osman wrote:
> > > Change from before the Android performance push. This adds gamut
> > transformation
> > > to more of the common image rendering paths. It also incorporates the
premul
> > vs.
> > > non-premul support that Brian asked for.
> > 
> > FYI: I'm working on testing this in another change, although it's proving
> > tricky.
> > I'll be hammering on strategies for validating this stuff.
> 
> It is possible to right GMs that run against GrDrawContext, GrPaint, etc. Not
> sure if that helps you at all.

mtklein and I came up with a strategy that always outputs things in our current
formats (sRGB), but which tests that we are correctly working in a different
space, and that we can round-trip. I'm working on that GM now, hopefully it
behaves as we expect. :)

[Brian Osman, 2016-08-31 17:50:34]

On 2016/08/31 17:49:53, Brian Osman wrote:
> On 2016/08/31 17:45:47, bsalomon wrote:
> > On 2016/08/31 13:15:56, Brian Osman wrote:
> > > On 2016/08/31 13:14:10, Brian Osman wrote:
> > > > Change from before the Android performance push. This adds gamut
> > > transformation
> > > > to more of the common image rendering paths. It also incorporates the
> premul
> > > vs.
> > > > non-premul support that Brian asked for.
> > > 
> > > FYI: I'm working on testing this in another change, although it's proving
> > > tricky.
> > > I'll be hammering on strategies for validating this stuff.
> > 
> > It is possible to right GMs that run against GrDrawContext, GrPaint, etc.
Not
> > sure if that helps you at all.
> 
> mtklein and I came up with a strategy that always outputs things in our
current
> formats (sRGB), but which tests that we are correctly working in a different
> space, and that we can round-trip. I'm working on that GM now, hopefully it
> behaves as we expect. :)

... also, we want this stuff to be supported by the raster backend, so I'm
trying to keep the API usage high level.

[Brian Osman, 2016-09-07 14:24:00]

The CQ bit was checked by brianosman@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-09-07 14:24:05]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Brian Osman, 2016-09-07 14:26:37]

brianosman@google.com changed reviewers:
+ msarett@google.com, mtklein@google.com

[Brian Osman, 2016-09-07 14:26:38]

Now that I've landed the gamut GM, this CL demonstrably
fixes gamut conversion in several more code paths.
(Bitmap draws that aren't using bicubic). Adding Mike and
Matt just to keep them informed of what I'm doing.

[bsalomon, 2016-09-07 14:46:37]

lgtm

https://codereview.chromium.org/2180803005/diff/200001/include/gpu/GrColorSpa...
File include/gpu/GrColorSpaceXform.h (right):

https://codereview.chromium.org/2180803005/diff/200001/include/gpu/GrColorSpa...
include/gpu/GrColorSpaceXform.h:31: * GrGLSLFragmentProcessor::GenKey() must
call this and include the returned value in its
The GrGLSLFragmentProcessor::GenKey() thing is sort of a weird pattern many but
not all classes follow. The real method is
GrFragmentProcessor::getGLSLProcessorKey().

[commit-bot: I haz the power, 2016-09-07 14:52:43]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-07 14:52:44]

Dry run: This issue passed the CQ dry run.

[Brian Osman, 2016-09-07 15:07:59]

On 2016/09/07 14:46:37, bsalomon wrote:
> lgtm
> 
>
https://codereview.chromium.org/2180803005/diff/200001/include/gpu/GrColorSpa...
> File include/gpu/GrColorSpaceXform.h (right):
> 
>
https://codereview.chromium.org/2180803005/diff/200001/include/gpu/GrColorSpa...
> include/gpu/GrColorSpaceXform.h:31: * GrGLSLFragmentProcessor::GenKey() must
> call this and include the returned value in its
> The GrGLSLFragmentProcessor::GenKey() thing is sort of a weird pattern many
but
> not all classes follow. The real method is
> GrFragmentProcessor::getGLSLProcessorKey().

Yeah, it looks like nearly all (except for a handful?) of FPs just forward to
the
static GenKey in the GLSLFP. Another fun fact: every single one of those GenKey
functions accepts GrGLSLCaps, and it's only used in (AFAICT) CustomXP.

[Brian Osman, 2016-09-07 15:08:07]

The CQ bit was checked by brianosman@google.com

[commit-bot: I haz the power, 2016-09-07 15:08:15]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-07 15:09:11]

Description was changed from

==========
Add color gamut xform helpers to GrGLSLShaderBuilder

New helper functions inject the necessary shader
function. Texture lookup functions can now insert
the gamut xform at the appropriate place, too.

As written, could be used to transform non-texture
colors (e.g. vertex colors) as well.

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2180803005
==========

to

==========
Add color gamut xform helpers to GrGLSLShaderBuilder

New helper functions inject the necessary shader
function. Texture lookup functions can now insert
the gamut xform at the appropriate place, too.

As written, could be used to transform non-texture
colors (e.g. vertex colors) as well.

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2180803005

Committed:
https://skia.googlesource.com/skia/+/77320dbabcddf05c0a1489eaf1f496729dc8de0e
==========

[commit-bot: I haz the power, 2016-09-07 15:09:13]

Committed patchset #11 (id:200001) as
https://skia.googlesource.com/skia/+/77320dbabcddf05c0a1489eaf1f496729dc8de0e