Cache the inverse matrix on SkColorSpace. Rename xyz() to toXYZ().

Cache the inverse matrix on SkColorSpace. Rename xyz() to toXYZ().

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

Committed: https://skia.googlesource.com/skia/+/971cd496b9e25f87f3a75a0015c203322907136a

[Brian Osman, 2016-09-08 14:50:12]

Description was changed from

==========
Cache the inverse matrix on SkColorSpace. Rename xyz() to toXYZ().

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

to

==========
Cache the inverse matrix on SkColorSpace. Rename xyz() to toXYZ().

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

[Brian Osman, 2016-09-08 14:52:05]

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

[commit-bot: I haz the power, 2016-09-08 14:52:17]

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-08 14:52:37]

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

[mtklein, 2016-09-08 15:04:36]

https://codereview.chromium.org/2323003002/diff/1/include/core/SkColorSpace.h
File include/core/SkColorSpace.h (right):

https://codereview.chromium.org/2323003002/diff/1/include/core/SkColorSpace.h...
include/core/SkColorSpace.h:69: const SkMatrix44& toXYZ() const { return
fToXYZD50; }
You may want to keep an alias xyz() to stage updating the name in Chrome.

[msarett, 2016-09-08 15:06:07]

https://codereview.chromium.org/2323003002/diff/1/include/core/SkColorSpace.h
File include/core/SkColorSpace.h (right):

https://codereview.chromium.org/2323003002/diff/1/include/core/SkColorSpace.h...
include/core/SkColorSpace.h:69: const SkMatrix44& toXYZ() const { return
fToXYZD50; }
Now that you've done all this typing, do you think it should be "toXYZD50" and
"fromXYZD50"?  I don't feel strongly, just asking.

https://codereview.chromium.org/2323003002/diff/1/src/core/SkColorSpace.cpp
File src/core/SkColorSpace.cpp (right):

https://codereview.chromium.org/2323003002/diff/1/src/core/SkColorSpace.cpp#n...
src/core/SkColorSpace.cpp:48: const SkMatrix44& SkColorSpace_Base::fromXYZ()
const {
nit: In my head, this should be next to the other "getters" in this file
(gammaIsLinear(), gammaCloseSRGB()).

Does that make sense to anyone else?

https://codereview.chromium.org/2323003002/diff/1/src/core/SkColorSpace.cpp#n...
src/core/SkColorSpace.cpp:52: SkMatrix44
srgbToxyzD50(SkMatrix44::kUninitialized_Constructor);
I'm fine with this behavior, but just to be clear on the implications of this:

Every time a client asks for a transform to a dst gamut that we can't invert -
we will instead give them a transform to sRGB gamut.

Maybe a comment?

https://codereview.chromium.org/2323003002/diff/1/src/core/SkColorSpaceXform.cpp
File src/core/SkColorSpaceXform.cpp (right):

https://codereview.chromium.org/2323003002/diff/1/src/core/SkColorSpaceXform....
src/core/SkColorSpaceXform.cpp:449: static inline bool
compute_gamut_xform(SkMatrix44* srcToDst, const SkColorSpace* src,
Thanks for integrating this change here!

Let's go ahead and make this void?

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

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

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

Dry run: This issue passed the CQ dry run.

[Brian Osman, 2016-09-08 15:28:19]

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

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

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-08 15:29:34]

Addressed everything that was brought up. (I had niggling
feelings about adding D50 to the name anyways, so yeah.)
Matt has been stripping out usage of xyz() from Chrome, so
the legacy function may not be needed, but this ensures
that this CL won't have to get backed out. :)

[mtklein, 2016-09-08 15:33:15]

On 2016/09/08 15:29:34, Brian Osman wrote:
> Addressed everything that was brought up. (I had niggling
> feelings about adding D50 to the name anyways, so yeah.)
> Matt has been stripping out usage of xyz() from Chrome, so
> the legacy function may not be needed, but this ensures
> that this CL won't have to get backed out. :)

Ah, very good.  Ignore me...

[msarett, 2016-09-08 15:35:21]

lgtm

https://codereview.chromium.org/2323003002/diff/20001/include/core/SkColorSpa...
File include/core/SkColorSpace.h (right):

https://codereview.chromium.org/2323003002/diff/20001/include/core/SkColorSpa...
include/core/SkColorSpace.h:74: const SkMatrix44& xyz() const { return
toXYZD50(); }
I think you can go ahead and remove this.  Trybot seems to show that no one is
using this.

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

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

[commit-bot: I haz the power, 2016-09-08 16:05:12]

Dry run: This issue passed the CQ dry run.

[Brian Osman, 2016-09-08 16:34:45]

On 2016/09/08 15:35:21, msarett wrote:
> lgtm
> 
>
https://codereview.chromium.org/2323003002/diff/20001/include/core/SkColorSpa...
> File include/core/SkColorSpace.h (right):
> 
>
https://codereview.chromium.org/2323003002/diff/20001/include/core/SkColorSpa...
> include/core/SkColorSpace.h:74: const SkMatrix44& xyz() const { return
> toXYZD50(); }
> I think you can go ahead and remove this.  Trybot seems to show that no one is
> using this.

Done. Just need a review from reed@ for public API. ;)

[reed1, 2016-09-08 16:35:24]

lgtm

[Brian Osman, 2016-09-08 16:37:48]

The CQ bit was checked by brianosman@google.com

[Brian Osman, 2016-09-08 16:37:49]

The patchset sent to the CQ was uploaded after l-g-t-m from msarett@google.com
Link to the patchset: https://codereview.chromium.org/2323003002/#ps40001
(title: "Remove xyz() again")

[commit-bot: I haz the power, 2016-09-08 16:37:56]

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-08 17:10:13]

Description was changed from

==========
Cache the inverse matrix on SkColorSpace. Rename xyz() to toXYZ().

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

to

==========
Cache the inverse matrix on SkColorSpace. Rename xyz() to toXYZ().

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

Committed:
https://skia.googlesource.com/skia/+/971cd496b9e25f87f3a75a0015c203322907136a
==========

[commit-bot: I haz the power, 2016-09-08 17:10:14]

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