(Some) gradients are gamma and gamut correct on GPU

Add storage and computation of SkColor4f version of gradient stops.

For now, we still only have the SkColor factory, but the Descriptor can
now carry either an SkColor or SkColor4f specified gradient. Base class
constructor automatically populates both forms of color, so that legacy
raster backend will continue to work, and new backend work can operate
directly from the float4 version.

On the GPU side, we have similar logic, but GrGradientEffect only
keeps one version of colors around: SkColor if the destination is
legacy, and SkColor4f (with an optional gamut xform) if the destination
is gamma correct. The 4f colors are already linear, and we gamut xform
them in setData, so gradients are now fully color-correct in sRGB and
F16 modes...

... unless there are more than three stops. Then we use a texture, and
that code path isn't handled yet. We have a few choices here (do we
use an 8-bit sRGB atlas, or just always use F16 linear atlas so we can
share it among both sRGB and wide-gamut rendering). In any case, I'd
like to defer that to a second CL.

This change does fix the non-texture gradients in the gamut GM.

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

Committed: https://skia.googlesource.com/skia/+/b9c5137a1cedbe8a3641db0b7c081881a2d5d58c

[Brian Osman, 2016-09-13 21:28:10]

Description was changed from

==========
Add storage and computation of SkColor4f version of gradient stops

For now, we still only have the SkColor factory, but the Descriptor can
now carry either an SkColor or SkColor4f specified gradient. Base class
constuctor automatically populates both forms of color, so that legacy
raster backend will continue to work, and new backend work can operate
directly from the float4 version.

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

to

==========
Add storage and computation of SkColor4f version of gradient stops

For now, we still only have the SkColor factory, but the Descriptor can
now carry either an SkColor or SkColor4f specified gradient. Base class
constuctor automatically populates both forms of color, so that legacy
raster backend will continue to work, and new backend work can operate
directly from the float4 version.

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

[Brian Osman, 2016-09-13 21:30:01]

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

[commit-bot: I haz the power, 2016-09-13 21:30:04]

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-09-13 21:51:37]

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

[commit-bot: I haz the power, 2016-09-13 21:51:38]

Dry run: This issue passed the CQ dry run.

[Brian Osman, 2016-09-14 18:42:39]

Description was changed from

==========
Add storage and computation of SkColor4f version of gradient stops

For now, we still only have the SkColor factory, but the Descriptor can
now carry either an SkColor or SkColor4f specified gradient. Base class
constuctor automatically populates both forms of color, so that legacy
raster backend will continue to work, and new backend work can operate
directly from the float4 version.

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

to

==========
NOTE: Please be picky about this one. I went through a few different
ideas, particularly about when to convert things, what to store at
each stage, etc... And I'm not sure that I landed on the best solution.

Add storage and computation of SkColor4f version of gradient stops.

For now, we still only have the SkColor factory, but the Descriptor can
now carry either an SkColor or SkColor4f specified gradient. Base class
constructor automatically populates both forms of color, so that legacy
raster backend will continue to work, and new backend work can operate
directly from the float4 version.

On the GPU side, we have similar logic, but GrGradient effect only
keeps one version of colors around: SkColor if the destination is
legacy, and SkColor4f (with an optional gamut xform) if the destination
is gamma correct. The 4f colors are already linear, and we gamut xform
them in setData, so gradients are now fully color-correct in sRGB and
F16 modes...

... unless there are more than three stops. Then we use a texture, and
that code path isn't handled yet. We have a few choices here (do we
use an 8-bit sRGB atlas, or just always use F16 linear atlas so we can
share it among both sRGB and wide-gamut rendering). In any case, I'd
like to defer that to a second CL.

This change does fix the non-texture gradients in the gamut GM.

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

[Brian Osman, 2016-09-14 18:43:14]

brianosman@google.com changed reviewers:
+ bsalomon@google.com, fmalita@chromium.org, mtklein@google.com, reed@google.com

[reed1, 2016-09-14 19:03:53]

https://codereview.chromium.org/2337313002/diff/40001/src/effects/gradients/S...
File src/effects/gradients/SkGradientShaderPriv.h (right):

https://codereview.chromium.org/2337313002/diff/40001/src/effects/gradients/S...
src/effects/gradients/SkGradientShaderPriv.h:430: // fColors4f and
fColorSpaceXform will be populated.
not specific to this CL... but you went from 2 mallocs to 3. I wonder if we can
collapse all 3 into 1 storage?

[Brian Osman, 2016-09-14 20:30:20]

https://codereview.chromium.org/2337313002/diff/40001/src/effects/gradients/S...
File src/effects/gradients/SkGradientShaderPriv.h (right):

https://codereview.chromium.org/2337313002/diff/40001/src/effects/gradients/S...
src/effects/gradients/SkGradientShaderPriv.h:430: // fColors4f and
fColorSpaceXform will be populated.
On 2016/09/14 19:03:53, reed1 wrote:
> not specific to this CL... but you went from 2 mallocs to 3. I wonder if we
can
> collapse all 3 into 1 storage?

Probably. Although we still only have 2 (only one of fColors and fColors4f will
be allocated).

[Brian Osman, 2016-09-14 21:08:32]

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

[commit-bot: I haz the power, 2016-09-14 21:08:34]

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-09-14 21:51:19]

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

[commit-bot: I haz the power, 2016-09-14 21:51:20]

Dry run: This issue passed the CQ dry run.

[f(malita), 2016-09-14 23:32:23]

https://codereview.chromium.org/2337313002/diff/40001/src/effects/gradients/S...
File src/effects/gradients/SkGradientShaderPriv.h (right):

https://codereview.chromium.org/2337313002/diff/40001/src/effects/gradients/S...
src/effects/gradients/SkGradientShaderPriv.h:87: const SkColor*      fColors;
Could we drop fColors from Descriptor?  IIRC it's a private struct used to hold
gradient info temporarily, so as long as we're keeping the most accurate
representation (fColors4f + fColorSpace) we should be able to reconstruct the
other format when needed.

I guess it adds an extra transformation on legacy gradient data (SkColor[] ->
SkColor4f[] -> SkColor[]), but it should allow us to get rid of one of the
branches in SkGradientShaderBase(), right?

[Brian Osman, 2016-09-15 14:37:06]

On 2016/09/14 23:32:23, f(malita) wrote:
>
https://codereview.chromium.org/2337313002/diff/40001/src/effects/gradients/S...
> File src/effects/gradients/SkGradientShaderPriv.h (right):
> 
>
https://codereview.chromium.org/2337313002/diff/40001/src/effects/gradients/S...
> src/effects/gradients/SkGradientShaderPriv.h:87: const SkColor*      fColors;
> Could we drop fColors from Descriptor?  IIRC it's a private struct used to
hold
> gradient info temporarily, so as long as we're keeping the most accurate
> representation (fColors4f + fColorSpace) we should be able to reconstruct the
> other format when needed.
> 
> I guess it adds an extra transformation on legacy gradient data (SkColor[] ->
> SkColor4f[] -> SkColor[]), but it should allow us to get rid of one of the
> branches in SkGradientShaderBase(), right?

I wanted to do something like that, but:

1) Performance of the common (legacy) case, like you said. (And those
   conversions are gamma, too - we need to have linear floats).
2) Descriptor is used for serialization of gradients. I wasn't quite ready to
   break compatibility and make things bigger, so we'd end up squishing back to
   SkColor in flatten()?

But I'm open to this idea, if people think those are acceptable tradeoffs?

[f(malita), 2016-09-15 15:28:54]

On 2016/09/15 14:37:06, Brian Osman wrote:
> On 2016/09/14 23:32:23, f(malita) wrote:
> >
>
https://codereview.chromium.org/2337313002/diff/40001/src/effects/gradients/S...
> > File src/effects/gradients/SkGradientShaderPriv.h (right):
> > 
> >
>
https://codereview.chromium.org/2337313002/diff/40001/src/effects/gradients/S...
> > src/effects/gradients/SkGradientShaderPriv.h:87: const SkColor*     
fColors;
> > Could we drop fColors from Descriptor?  IIRC it's a private struct used to
> hold
> > gradient info temporarily, so as long as we're keeping the most accurate
> > representation (fColors4f + fColorSpace) we should be able to reconstruct
the
> > other format when needed.
> > 
> > I guess it adds an extra transformation on legacy gradient data (SkColor[]
->
> > SkColor4f[] -> SkColor[]), but it should allow us to get rid of one of the
> > branches in SkGradientShaderBase(), right?
> 
> I wanted to do something like that, but:
> 
> 1) Performance of the common (legacy) case, like you said. (And those
>    conversions are gamma, too - we need to have linear floats).
> 2) Descriptor is used for serialization of gradients. I wasn't quite ready to
>    break compatibility and make things bigger, so we'd end up squishing back
to
>    SkColor in flatten()?
> 
> But I'm open to this idea, if people think those are acceptable tradeoffs?

Per conversation, it makes sense to go ahead with your current approach, and we
can revisit once the dust settles.

Non-GPU bits LGTM.

[Brian Osman, 2016-09-15 16:42:14]

Description was changed from

==========
NOTE: Please be picky about this one. I went through a few different
ideas, particularly about when to convert things, what to store at
each stage, etc... And I'm not sure that I landed on the best solution.

Add storage and computation of SkColor4f version of gradient stops.

For now, we still only have the SkColor factory, but the Descriptor can
now carry either an SkColor or SkColor4f specified gradient. Base class
constructor automatically populates both forms of color, so that legacy
raster backend will continue to work, and new backend work can operate
directly from the float4 version.

On the GPU side, we have similar logic, but GrGradient effect only
keeps one version of colors around: SkColor if the destination is
legacy, and SkColor4f (with an optional gamut xform) if the destination
is gamma correct. The 4f colors are already linear, and we gamut xform
them in setData, so gradients are now fully color-correct in sRGB and
F16 modes...

... unless there are more than three stops. Then we use a texture, and
that code path isn't handled yet. We have a few choices here (do we
use an 8-bit sRGB atlas, or just always use F16 linear atlas so we can
share it among both sRGB and wide-gamut rendering). In any case, I'd
like to defer that to a second CL.

This change does fix the non-texture gradients in the gamut GM.

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

to

==========
Add storage and computation of SkColor4f version of gradient stops.

For now, we still only have the SkColor factory, but the Descriptor can
now carry either an SkColor or SkColor4f specified gradient. Base class
constructor automatically populates both forms of color, so that legacy
raster backend will continue to work, and new backend work can operate
directly from the float4 version.

On the GPU side, we have similar logic, but GrGradientEffect only
keeps one version of colors around: SkColor if the destination is
legacy, and SkColor4f (with an optional gamut xform) if the destination
is gamma correct. The 4f colors are already linear, and we gamut xform
them in setData, so gradients are now fully color-correct in sRGB and
F16 modes...

... unless there are more than three stops. Then we use a texture, and
that code path isn't handled yet. We have a few choices here (do we
use an 8-bit sRGB atlas, or just always use F16 linear atlas so we can
share it among both sRGB and wide-gamut rendering). In any case, I'd
like to defer that to a second CL.

This change does fix the non-texture gradients in the gamut GM.

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

[Brian Osman, 2016-09-15 17:05:20]

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

[Brian Osman, 2016-09-15 17:05:21]

Just waiting for GPU approval. Adding another set of GPU eyes. ;)

[bsalomon, 2016-09-15 17:52:21]

On 2016/09/15 17:05:21, Brian Osman wrote:
> Just waiting for GPU approval. Adding another set of GPU eyes. ;)

lgtm

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

The CQ bit was checked by brianosman@google.com

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

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-15 18:09:47]

Description was changed from

==========
Add storage and computation of SkColor4f version of gradient stops.

For now, we still only have the SkColor factory, but the Descriptor can
now carry either an SkColor or SkColor4f specified gradient. Base class
constructor automatically populates both forms of color, so that legacy
raster backend will continue to work, and new backend work can operate
directly from the float4 version.

On the GPU side, we have similar logic, but GrGradientEffect only
keeps one version of colors around: SkColor if the destination is
legacy, and SkColor4f (with an optional gamut xform) if the destination
is gamma correct. The 4f colors are already linear, and we gamut xform
them in setData, so gradients are now fully color-correct in sRGB and
F16 modes...

... unless there are more than three stops. Then we use a texture, and
that code path isn't handled yet. We have a few choices here (do we
use an 8-bit sRGB atlas, or just always use F16 linear atlas so we can
share it among both sRGB and wide-gamut rendering). In any case, I'd
like to defer that to a second CL.

This change does fix the non-texture gradients in the gamut GM.

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

to

==========
Add storage and computation of SkColor4f version of gradient stops.

For now, we still only have the SkColor factory, but the Descriptor can
now carry either an SkColor or SkColor4f specified gradient. Base class
constructor automatically populates both forms of color, so that legacy
raster backend will continue to work, and new backend work can operate
directly from the float4 version.

On the GPU side, we have similar logic, but GrGradientEffect only
keeps one version of colors around: SkColor if the destination is
legacy, and SkColor4f (with an optional gamut xform) if the destination
is gamma correct. The 4f colors are already linear, and we gamut xform
them in setData, so gradients are now fully color-correct in sRGB and
F16 modes...

... unless there are more than three stops. Then we use a texture, and
that code path isn't handled yet. We have a few choices here (do we
use an 8-bit sRGB atlas, or just always use F16 linear atlas so we can
share it among both sRGB and wide-gamut rendering). In any case, I'd
like to defer that to a second CL.

This change does fix the non-texture gradients in the gamut GM.

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

Committed:
https://skia.googlesource.com/skia/+/b9c5137a1cedbe8a3641db0b7c081881a2d5d58c
==========

[commit-bot: I haz the power, 2016-09-15 18:09:48]

Committed patchset #3 (id:40001) as
https://skia.googlesource.com/skia/+/b9c5137a1cedbe8a3641db0b7c081881a2d5d58c