Support Float32 output from SkColorSpaceXform

Support Float32 output from SkColorSpaceXform

* Adds Float32 support to SkColorSpaceXform
* Changes API to allows clients to ask for F32, updates clients to
  new API
* Adds Sk4f_load4 and Sk4f_store4 to SkNx
* Make use of new xform in SkGr.cpp

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2339233003
CQ_INCLUDE_TRYBOTS=master.client.skia:Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-SKNX_NO_SIMD-Trybot

Committed: https://skia.googlesource.com/skia/+/43d6651111374b5d1e4ddd9030dcf079b448ec47
Committed: https://skia.googlesource.com/skia/+/c0444615ed76360f680619ad4d1f92cda6181a50

[msarett, 2016-09-14 20:00:59]

Description was changed from

==========
Support Float32 output from SkColorSpaceXform

* Adds Float32 support to SkColorSpaceXform
* Changes API to allows clients to ask for F32, updates clients to
  new API
* Adds Sk4f_load4 and Sk4f_store4 to SkNx
* Make use of new xform in SkGr.cpp

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

to

==========
Support Float32 output from SkColorSpaceXform

* Adds Float32 support to SkColorSpaceXform
* Changes API to allows clients to ask for F32, updates clients to
  new API
* Adds Sk4f_load4 and Sk4f_store4 to SkNx
* Make use of new xform in SkGr.cpp

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2339233003
CQ_INCLUDE_TRYBOTS=master.client.skia:Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-SKNX_NO_SIMD-Trybot
==========

[msarett, 2016-09-14 20:19:21]

Patchset #2 (id:20001) has been deleted

[msarett, 2016-09-14 20:20:34]

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

[msarett, 2016-09-14 20:20:35]

https://codereview.chromium.org/2339233003/diff/40001/include/gpu/GrDrawConte...
File include/gpu/GrDrawContext.h (right):

https://codereview.chromium.org/2339233003/diff/40001/include/gpu/GrDrawConte...
include/gpu/GrDrawContext.h:36: class SkColorSpaceXform;
Brian, we can back out the gpu parts of this change depending on what you think.
 Mostly I was looking for a way to test.  But I think I'm going to write
additional tests anyway.

https://codereview.chromium.org/2339233003/diff/40001/src/codec/SkPngCodec.cpp
File src/codec/SkPngCodec.cpp (right):

https://codereview.chromium.org/2339233003/diff/40001/src/codec/SkPngCodec.cp...
src/codec/SkPngCodec.cpp:384: SkColorSpaceXform::ColorFormat colorFormat =
select_xform_format(colorType);
Leon, ideally your PNG change can land first?  I think this will refactor nicer
under your design.

https://codereview.chromium.org/2339233003/diff/40001/src/core/SkColorSpaceXf...
File src/core/SkColorSpaceXform.cpp (right):

https://codereview.chromium.org/2339233003/diff/40001/src/core/SkColorSpaceXf...
src/core/SkColorSpaceXform.cpp:910: static inline void store_f32(void* dst,
const uint32_t* src,
I originally wrote a "store_f32_opaque()" version as well...  But I'm not
convinced it would be noticeably faster than this.

[mtklein, 2016-09-14 20:30:37]

https://codereview.chromium.org/2339233003/diff/40001/src/opts/SkNx_sse.h
File src/opts/SkNx_sse.h (right):

https://codereview.chromium.org/2339233003/diff/40001/src/opts/SkNx_sse.h#new...
src/opts/SkNx_sse.h:481: static inline void Sk4f_load4(const void* ptr, Sk4f* r,
Sk4f* g, Sk4f* b, Sk4f* a) {
May want to call _MM_TRANSPOSE_PS, or crib off it:

https://software.intel.com/sites/landingpage/IntrinsicsGuide/#text=_MM_TRANS&...

[scroggo, 2016-09-14 20:35:02]

https://codereview.chromium.org/2339233003/diff/40001/src/codec/SkPngCodec.cpp
File src/codec/SkPngCodec.cpp (right):

https://codereview.chromium.org/2339233003/diff/40001/src/codec/SkPngCodec.cp...
src/codec/SkPngCodec.cpp:384: SkColorSpaceXform::ColorFormat colorFormat =
select_xform_format(colorType);
On 2016/09/14 20:20:35, msarett wrote:
> Leon, ideally your PNG change can land first?  I think this will refactor
nicer
> under your design.

That depends. How much of a hurry are you to get this in?

I saw an error on the bots that I have figured out how to repro locally, but I'm
still investigating. (It happens in a webp image, which should not be affected
by my changes...)

Once that's fixed, I still need to update Google3, unless we can work around it.
Given the lack of progress there (taviso@ and clay@ have not been responsive) it
might be worth trying to get an exception for using our own copy of the library.
It won't get us off the hook for updating it, but it can give us some more time
to do so.

[Brian Osman, 2016-09-14 20:46:53]

https://codereview.chromium.org/2339233003/diff/40001/src/opts/SkNx_neon.h
File src/opts/SkNx_neon.h (right):

https://codereview.chromium.org/2339233003/diff/40001/src/opts/SkNx_neon.h#ne...
src/opts/SkNx_neon.h:526: static inline void Sk4f_store4(void* dst, const Sk4h&
r, const Sk4h& g, const Sk4h& b,
This looks like it should still be Sk4h_store4 ? And the one below is meant to
be Sk4f_store4?

https://codereview.chromium.org/2339233003/diff/40001/src/opts/SkNx_neon.h#ne...
src/opts/SkNx_neon.h:538: uint16x4x4_t rgba = vld4q_f32((const float*) ptr);
float32x4x4_t ?

[msarett, 2016-09-14 21:09:43]

https://codereview.chromium.org/2339233003/diff/40001/src/codec/SkPngCodec.cpp
File src/codec/SkPngCodec.cpp (right):

https://codereview.chromium.org/2339233003/diff/40001/src/codec/SkPngCodec.cp...
src/codec/SkPngCodec.cpp:384: SkColorSpaceXform::ColorFormat colorFormat =
select_xform_format(colorType);
On 2016/09/14 20:35:01, scroggo wrote:
> On 2016/09/14 20:20:35, msarett wrote:
> > Leon, ideally your PNG change can land first?  I think this will refactor
> nicer
> > under your design.
> 
> That depends. How much of a hurry are you to get this in?
> 

I'll probably want to write some stuff based on this, but I won't rush it until
I have a queue.

> I saw an error on the bots that I have figured out how to repro locally, but
I'm
> still investigating. (It happens in a webp image, which should not be affected
> by my changes...)
> 
> Once that's fixed, I still need to update Google3, unless we can work around
it.
> Given the lack of progress there (taviso@ and clay@ have not been responsive)
it
> might be worth trying to get an exception for using our own copy of the
library.
> It won't get us off the hook for updating it, but it can give us some more
time
> to do so.

IMHO if you're blocked by waiting on code reviews, we might as well reland the
special Skia copy.  When someone complains it might be a good way to ping the
review.

https://codereview.chromium.org/2339233003/diff/40001/src/opts/SkNx_neon.h
File src/opts/SkNx_neon.h (right):

https://codereview.chromium.org/2339233003/diff/40001/src/opts/SkNx_neon.h#ne...
src/opts/SkNx_neon.h:526: static inline void Sk4f_store4(void* dst, const Sk4h&
r, const Sk4h& g, const Sk4h& b,
On 2016/09/14 20:46:53, Brian Osman wrote:
> This looks like it should still be Sk4h_store4 ? And the one below is meant to
> be Sk4f_store4?

Yes :).  Done.

https://codereview.chromium.org/2339233003/diff/40001/src/opts/SkNx_neon.h#ne...
src/opts/SkNx_neon.h:538: uint16x4x4_t rgba = vld4q_f32((const float*) ptr);
On 2016/09/14 20:46:53, Brian Osman wrote:
> float32x4x4_t ?

Done.

https://codereview.chromium.org/2339233003/diff/40001/src/opts/SkNx_sse.h
File src/opts/SkNx_sse.h (right):

https://codereview.chromium.org/2339233003/diff/40001/src/opts/SkNx_sse.h#new...
src/opts/SkNx_sse.h:481: static inline void Sk4f_load4(const void* ptr, Sk4f* r,
Sk4f* g, Sk4f* b, Sk4f* a) {
On 2016/09/14 20:30:36, mtklein wrote:
> May want to call _MM_TRANSPOSE_PS, or crib off it:
> 
>
https://software.intel.com/sites/landingpage/IntrinsicsGuide/#text=_MM_TRANS&...

Cool!  Done.

[msarett, 2016-09-14 22:12:58]

Added a unit test and a gm.

[mtklein_C, 2016-09-14 22:22:19]

mtklein@chromium.org changed reviewers:
+ mtklein@chromium.org

[mtklein_C, 2016-09-14 22:22:19]

lgtm, looking most carefully at the stuff I'm familiar with

[Brian Osman, 2016-09-15 18:11:54]

lgtm as well. GPU changes are fine. I'm just now landing a gradient change that
uses some similar code. I think we're going to need to do a more thorough
cleanup pass later to unify the xform stuff. (I'm still interested in having
something like SkGamutXform that's just the concat'd matrix, then building
SkColorSpaceXform on top of that).

[msarett, 2016-09-16 15:43:56]

On 2016/09/15 18:11:54, Brian Osman wrote:
> lgtm as well. GPU changes are fine. I'm just now landing a gradient change
that
> uses some similar code. I think we're going to need to do a more thorough
> cleanup pass later to unify the xform stuff. (I'm still interested in having
> something like SkGamutXform that's just the concat'd matrix, then building
> SkColorSpaceXform on top of that).

Sounds good to me.

Leon, any thoughts on src/codec?  I've rebased on your png change and fixed
conflicts in SkPngCodec.h/.cpp.

[msarett, 2016-09-16 15:44:08]

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

[commit-bot: I haz the power, 2016-09-16 15:44:09]

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-16 15:50:39]

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

[commit-bot: I haz the power, 2016-09-16 15:50:40]

Dry run: Try jobs failed on following builders:
  Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Debug-GN-Trybot on master.client.skia
(JOB_FAILED,
http://build.chromium.org/p/client.skia/builders/Test-Ubuntu-GCC-GCE-CPU-AVX2...)

[msarett, 2016-09-16 16:06:03]

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

[commit-bot: I haz the power, 2016-09-16 16:06:10]

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-16 16:25:41]

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

[commit-bot: I haz the power, 2016-09-16 16:25:42]

Dry run: This issue passed the CQ dry run.

[scroggo, 2016-09-16 16:32:51]

On 2016/09/16 15:43:56, msarett wrote:
> On 2016/09/15 18:11:54, Brian Osman wrote:
> > lgtm as well. GPU changes are fine. I'm just now landing a gradient change
> that
> > uses some similar code. I think we're going to need to do a more thorough
> > cleanup pass later to unify the xform stuff. (I'm still interested in having
> > something like SkGamutXform that's just the concat'd matrix, then building
> > SkColorSpaceXform on top of that).
> 
> Sounds good to me.
> 
> Leon, any thoughts on src/codec?  I've rebased on your png change and fixed
> conflicts in SkPngCodec.h/.cpp.

lgtm

[msarett, 2016-09-16 16:39:11]

The CQ bit was checked by msarett@google.com

[msarett, 2016-09-16 16:39:12]

The patchset sent to the CQ was uploaded after l-g-t-m from
brianosman@google.com, mtklein@chromium.org
Link to the patchset: https://codereview.chromium.org/2339233003/#ps140001
(title: "Bug fix, more logical function name")

[commit-bot: I haz the power, 2016-09-16 16:39:16]

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-16 16:40:45]

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

[commit-bot: I haz the power, 2016-09-16 16:40:46]

Try jobs failed on following builders:
  skia_presubmit-Trybot on master.client.skia.fyi (JOB_FAILED,
http://build.chromium.org/p/client.skia.fyi/builders/skia_presubmit-Trybot/bu...)

[msarett, 2016-09-16 16:46:11]

msarett@google.com changed reviewers:
+ bsalomon@google.com

[msarett, 2016-09-16 16:46:12]

Forgot there is an API change...  Brian can you take a look?

[bsalomon, 2016-09-16 16:49:05]

lgtm

[msarett, 2016-09-16 16:49:54]

The CQ bit was checked by msarett@google.com

[commit-bot: I haz the power, 2016-09-16 16:49:58]

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-16 16:51:14]

Description was changed from

==========
Support Float32 output from SkColorSpaceXform

* Adds Float32 support to SkColorSpaceXform
* Changes API to allows clients to ask for F32, updates clients to
  new API
* Adds Sk4f_load4 and Sk4f_store4 to SkNx
* Make use of new xform in SkGr.cpp

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2339233003
CQ_INCLUDE_TRYBOTS=master.client.skia:Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-SKNX_NO_SIMD-Trybot
==========

to

==========
Support Float32 output from SkColorSpaceXform

* Adds Float32 support to SkColorSpaceXform
* Changes API to allows clients to ask for F32, updates clients to
  new API
* Adds Sk4f_load4 and Sk4f_store4 to SkNx
* Make use of new xform in SkGr.cpp

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2339233003
CQ_INCLUDE_TRYBOTS=master.client.skia:Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-SKNX_NO_SIMD-Trybot

Committed:
https://skia.googlesource.com/skia/+/43d6651111374b5d1e4ddd9030dcf079b448ec47
==========

[commit-bot: I haz the power, 2016-09-16 16:51:16]

Committed patchset #7 (id:140001) as
https://skia.googlesource.com/skia/+/43d6651111374b5d1e4ddd9030dcf079b448ec47

[msarett, 2016-09-16 18:00:59]

A revert of this CL (patchset #7 id:140001) has been created in
https://codereview.chromium.org/2347473007/ by msarett@google.com.

The reason for reverting is: Hitting an assert.

[msarett, 2016-09-16 18:18:07]

Description was changed from

==========
Support Float32 output from SkColorSpaceXform

* Adds Float32 support to SkColorSpaceXform
* Changes API to allows clients to ask for F32, updates clients to
  new API
* Adds Sk4f_load4 and Sk4f_store4 to SkNx
* Make use of new xform in SkGr.cpp

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2339233003
CQ_INCLUDE_TRYBOTS=master.client.skia:Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-SKNX_NO_SIMD-Trybot

Committed:
https://skia.googlesource.com/skia/+/43d6651111374b5d1e4ddd9030dcf079b448ec47
==========

to

==========
Support Float32 output from SkColorSpaceXform

* Adds Float32 support to SkColorSpaceXform
* Changes API to allows clients to ask for F32, updates clients to
  new API
* Adds Sk4f_load4 and Sk4f_store4 to SkNx
* Make use of new xform in SkGr.cpp

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2339233003
CQ_INCLUDE_TRYBOTS=master.client.skia:Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-SKNX_NO_SIMD-Trybot

Committed:
https://skia.googlesource.com/skia/+/43d6651111374b5d1e4ddd9030dcf079b448ec47
==========

[msarett, 2016-09-16 18:21:41]

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

[commit-bot: I haz the power, 2016-09-16 18:21:53]

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

[msarett, 2016-09-16 18:22:52]

On 2016/09/16 18:00:59, msarett wrote:
> A revert of this CL (patchset #7 id:140001) has been created in
> https://codereview.chromium.org/2347473007/ by mailto:msarett@google.com.
> 
> The reason for reverting is: Hitting an assert.

Trying this again without the gpu changes.

SkColorSpaceXform asserts that the dst color space must be linear in order to
convert to F32.  GrColorSpaceXform is more lenient.

[commit-bot: I haz the power, 2016-09-16 18:43:12]

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

[commit-bot: I haz the power, 2016-09-16 18:43:13]

Dry run: This issue passed the CQ dry run.

[msarett, 2016-09-16 18:44:34]

The CQ bit was checked by msarett@google.com

[msarett, 2016-09-16 18:44:35]

The patchset sent to the CQ was uploaded after l-g-t-m from scroggo@google.com,
brianosman@google.com, bsalomon@google.com, mtklein@chromium.org
Link to the patchset: https://codereview.chromium.org/2339233003/#ps160001
(title: "Remove gpu changes")

[commit-bot: I haz the power, 2016-09-16 18:44:43]

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-16 18:46:00]

Description was changed from

==========
Support Float32 output from SkColorSpaceXform

* Adds Float32 support to SkColorSpaceXform
* Changes API to allows clients to ask for F32, updates clients to
  new API
* Adds Sk4f_load4 and Sk4f_store4 to SkNx
* Make use of new xform in SkGr.cpp

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2339233003
CQ_INCLUDE_TRYBOTS=master.client.skia:Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-SKNX_NO_SIMD-Trybot

Committed:
https://skia.googlesource.com/skia/+/43d6651111374b5d1e4ddd9030dcf079b448ec47
==========

to

==========
Support Float32 output from SkColorSpaceXform

* Adds Float32 support to SkColorSpaceXform
* Changes API to allows clients to ask for F32, updates clients to
  new API
* Adds Sk4f_load4 and Sk4f_store4 to SkNx
* Make use of new xform in SkGr.cpp

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2339233003
CQ_INCLUDE_TRYBOTS=master.client.skia:Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-SKNX_NO_SIMD-Trybot

Committed:
https://skia.googlesource.com/skia/+/43d6651111374b5d1e4ddd9030dcf079b448ec47
Committed:
https://skia.googlesource.com/skia/+/c0444615ed76360f680619ad4d1f92cda6181a50
==========

[commit-bot: I haz the power, 2016-09-16 18:46:02]

Committed patchset #8 (id:160001) as
https://skia.googlesource.com/skia/+/c0444615ed76360f680619ad4d1f92cda6181a50