Miscellaneous color space refactors

Miscellaneous color space refactors

(1) Use float matrix[16] everywhere (enables future code
    sharing).
(2) SkColorLookUpTable refactors
    *** Store in a single allocation (like SkGammas)
    *** Eliminate fOutputChannels (we always require 3,
        and probably always will)
(3) Change names of read_big_endian_* helpers

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2166093003
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/+/959d45b43357a40854938586c303177c6aa53220

[msarett, 2016-07-20 22:36:04]

Description was changed from

==========
Miscellaneous color space refactors

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

to

==========
Miscellaneous color space refactors

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

[msarett, 2016-07-20 22:50:56]

Patchset #1 (id:1) has been deleted

[msarett, 2016-07-20 22:51:05]

Patchset #1 (id:20001) has been deleted

[msarett, 2016-07-20 23:12:07]

Patchset #1 (id:40001) has been deleted

[msarett, 2016-07-21 14:28:55]

Patchset #1 (id:60001) has been deleted

[msarett, 2016-07-21 14:29:44]

Description was changed from

==========
Miscellaneous color space refactors

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

to

==========
Miscellaneous color space refactors

(1) Use float matrix[16] everywhere (enables future refactors).

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

[msarett, 2016-07-21 14:31:37]

Description was changed from

==========
Miscellaneous color space refactors

(1) Use float matrix[16] everywhere (enables future refactors).

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

to

==========
Miscellaneous color space refactors

(1) Use float matrix[16] everywhere (enables future refactors).
(2) SkColorLookUpTable refactors
    *** Store in a single allocation (like SkGammas)
    *** Eliminate fOutputChannels (we always require 3)

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

[msarett, 2016-07-21 14:35:21]

Description was changed from

==========
Miscellaneous color space refactors

(1) Use float matrix[16] everywhere (enables future refactors).
(2) SkColorLookUpTable refactors
    *** Store in a single allocation (like SkGammas)
    *** Eliminate fOutputChannels (we always require 3)

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

to

==========
Miscellaneous color space refactors

(1) Use float matrix[16] everywhere (enables future refactors).
(2) SkColorLookUpTable refactors
    *** Store in a single allocation (like SkGammas)
    *** Eliminate fOutputChannels (we always require 3)
(3) Change names of read_big_endian_* helpers

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

[msarett, 2016-07-21 14:43:36]

Description was changed from

==========
Miscellaneous color space refactors

(1) Use float matrix[16] everywhere (enables future refactors).
(2) SkColorLookUpTable refactors
    *** Store in a single allocation (like SkGammas)
    *** Eliminate fOutputChannels (we always require 3)
(3) Change names of read_big_endian_* helpers

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

to

==========
Miscellaneous color space refactors

(1) Use float matrix[16] everywhere (enables future code
    sharing).
(2) SkColorLookUpTable refactors
    *** Store in a single allocation (like SkGammas)
    *** Eliminate fOutputChannels (we always require 3,
        and probably always will)
(3) Change names of read_big_endian_* helpers

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

[msarett, 2016-07-21 14:46:31]

Patchset #2 (id:100001) has been deleted

[msarett, 2016-07-21 14:47:36]

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

[msarett, 2016-07-21 14:47:37]

Now that the giant refactor has landed, here's a variety of follow-ups.

https://codereview.chromium.org/2166093003/diff/80001/src/core/SkColorSpace_B...
File src/core/SkColorSpace_Base.h (right):

https://codereview.chromium.org/2166093003/diff/80001/src/core/SkColorSpace_B...
src/core/SkColorSpace_Base.h:149: void* operator new(size_t size) = delete;
I know we were somewhat on the fence about using single allocations for our
structs...  I lean towards yes just because parsing these profiles happens at
image decode time (where we do tend to care about performance and memory use).

Whatever we decide, I think we should be consistent between SkGammas and
SkColorLookUpTable.

https://codereview.chromium.org/2166093003/diff/80001/src/core/SkColorSpace_I...
File src/core/SkColorSpace_ICC.cpp (right):

https://codereview.chromium.org/2166093003/diff/80001/src/core/SkColorSpace_I...
src/core/SkColorSpace_ICC.cpp:628: SkASSERT(3 == inputChannels);
I think it's worth keeping this variable around because we will eventually
extend to support other numbers of input channels.  Definitely CMYK (4).

[mtklein, 2016-07-21 19:46:45]

lgtm

https://codereview.chromium.org/2166093003/diff/120001/src/core/SkColorSpace_...
File src/core/SkColorSpace_Base.h (left):

https://codereview.chromium.org/2166093003/diff/120001/src/core/SkColorSpace_...
src/core/SkColorSpace_Base.h:132: uint8_t                  fInputChannels;
Is this one ever going to be not 3?

https://codereview.chromium.org/2166093003/diff/120001/src/core/SkColorSpace_...
File src/core/SkColorSpace_Base.h (right):

https://codereview.chromium.org/2166093003/diff/120001/src/core/SkColorSpace_...
src/core/SkColorSpace_Base.h:145: memcpy(fGridPoints, gridPoints, 3 *
sizeof(uint8_t));
a.k.a, 3

[msarett, 2016-07-21 19:49:29]

https://codereview.chromium.org/2166093003/diff/120001/src/core/SkColorSpace_...
File src/core/SkColorSpace_Base.h (left):

https://codereview.chromium.org/2166093003/diff/120001/src/core/SkColorSpace_...
src/core/SkColorSpace_Base.h:132: uint8_t                  fInputChannels;
On 2016/07/21 19:46:45, mtklein wrote:
> Is this one ever going to be not 3?

Once we support CMYK, yes.  That's why I'm letting it live.

https://codereview.chromium.org/2166093003/diff/120001/src/core/SkColorSpace_...
File src/core/SkColorSpace_Base.h (right):

https://codereview.chromium.org/2166093003/diff/120001/src/core/SkColorSpace_...
src/core/SkColorSpace_Base.h:145: memcpy(fGridPoints, gridPoints, 3 *
sizeof(uint8_t));
On 2016/07/21 19:46:45, mtklein wrote:
> a.k.a, 3

True.

[msarett, 2016-07-21 19:49:53]

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

[commit-bot: I haz the power, 2016-07-21 19:49:55]

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-07-21 20:11:48]

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

[commit-bot: I haz the power, 2016-07-21 20:11:49]

Dry run: This issue passed the CQ dry run.

[msarett, 2016-07-21 20:17:55]

The CQ bit was checked by msarett@google.com

[commit-bot: I haz the power, 2016-07-21 20:18:08]

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-07-21 20:19:11]

Description was changed from

==========
Miscellaneous color space refactors

(1) Use float matrix[16] everywhere (enables future code
    sharing).
(2) SkColorLookUpTable refactors
    *** Store in a single allocation (like SkGammas)
    *** Eliminate fOutputChannels (we always require 3,
        and probably always will)
(3) Change names of read_big_endian_* helpers

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

to

==========
Miscellaneous color space refactors

(1) Use float matrix[16] everywhere (enables future code
    sharing).
(2) SkColorLookUpTable refactors
    *** Store in a single allocation (like SkGammas)
    *** Eliminate fOutputChannels (we always require 3,
        and probably always will)
(3) Change names of read_big_endian_* helpers

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2166093003
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/+/959d45b43357a40854938586c303177c6aa53220
==========

[commit-bot: I haz the power, 2016-07-21 20:19:12]

Committed patchset #2 (id:120001) as
https://skia.googlesource.com/skia/+/959d45b43357a40854938586c303177c6aa53220