Use SkICC in gfx::ICCProfile and gfx::ColorSpace

Use SkICC in gfx::ICCProfile and gfx::ColorSpace

This finally reaches the original goal of having gfx::ColorSpace be
a (potentially lossily) compressed version of gfx::ICCProfile.

The ICC profile's primaries and transfer funtion are extracted and
stored in the gfx::ColorSpace, and are used in the event that the
underlying ICC profile is purged from the cache.

Update ICCProfile's IPC methods to ensure that the profile is touched
in the cache and that its SkColorSpace is computed when it is
received via IPC.

BUG=634102
Review-Url: https://codereview.chromium.org/2652503002
Cr-Commit-Position: refs/heads/master@{#446923}
Committed: https://chromium.googlesource.com/chromium/src/+/549738ba7a6df5e4872271233d40ecabc9857e7e

[ccameron, 2017-01-21 19:22:47]

The CQ bit was checked by ccameron@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-21 19:23:02]

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

[ccameron, 2017-01-21 19:23:59]

Description was changed from

==========
Use SkICC in gfx::ICCProfile and gfx::ColorSpace

This finally reaches the original goal of having gfx::ColorSpace be
a (potentially lossily) compressed version of gfx::ICCProfile.

The ICC profile's primaries and transfer funtion are extracted and
stored in the gfx::ColorSpace, and are used in the event that the
underlying ICC profile is purged from the cache.

Update ICCProfile's IPC methods to ensure that the profile is touched
in the cache (and that its SkColorSpace is computed) when it is
received via IPC.

BUG=634102
==========

to

==========
Use SkICC in gfx::ICCProfile and gfx::ColorSpace

This finally reaches the original goal of having gfx::ColorSpace be
a (potentially lossily) compressed version of gfx::ICCProfile.

The ICC profile's primaries and transfer funtion are extracted and
stored in the gfx::ColorSpace, and are used in the event that the
underlying ICC profile is purged from the cache.

Update ICCProfile's IPC methods to ensure that the profile is touched
in the cache and that its SkColorSpace is computed when it is
received via IPC.

BUG=634102
==========

[ccameron, 2017-01-21 19:23:59]

ccameron@chromium.org changed reviewers:
+ dcheng@chromium.org, msarett@chromium.org

[ccameron, 2017-01-21 20:31:44]

Ptal -- very excited to finally have the library support to make this change!

[commit-bot: I haz the power, 2017-01-21 20:44:11]

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

[commit-bot: I haz the power, 2017-01-21 20:44:12]

Dry run: This issue passed the CQ dry run.

[hubbe, 2017-01-22 04:30:30]

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

[hubbe, 2017-01-22 04:30:31]

https://codereview.chromium.org/2652503002/diff/1/ui/gfx/color_transform.cc
File ui/gfx/color_transform.cc (right):

https://codereview.chromium.org/2652503002/diff/1/ui/gfx/color_transform.cc#n...
ui/gfx/color_transform.cc:168: case ColorSpace::TransferID::CUSTOM:
Maybe implement this? (And also in ToLinear?)

[ccameron, 2017-01-23 18:54:55]

The CQ bit was checked by ccameron@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-23 18:55:52]

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

[ccameron, 2017-01-23 18:58:58]

https://codereview.chromium.org/2652503002/diff/1/ui/gfx/color_transform.cc
File ui/gfx/color_transform.cc (right):

https://codereview.chromium.org/2652503002/diff/1/ui/gfx/color_transform.cc#n...
ui/gfx/color_transform.cc:168: case ColorSpace::TransferID::CUSTOM:
On 2017/01/22 04:30:30, hubbe wrote:
> Maybe implement this? (And also in ToLinear?)

Good point.

I started trying to put this in this patch, but it starts to pull in a bunch of
parts of tests, and the patch snowballs a bit, so I'd prefer to put it in a
separate one.

WRT ToLinear, yes, definitely needs to be done. I'd like to put this in a
separate patch for the same testing reasons, and also because it involves a bit
of being careful with numerics (making sure I translate everything correctly),
so I'd like to keep that out of the bigger refactor patch.

[commit-bot: I haz the power, 2017-01-23 19:43:43]

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

[commit-bot: I haz the power, 2017-01-23 19:43:44]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[dcheng, 2017-01-23 23:26:46]

https://codereview.chromium.org/2652503002/diff/20001/ui/gfx/icc_profile.cc
File ui/gfx/icc_profile.cc (left):

https://codereview.chromium.org/2652503002/diff/20001/ui/gfx/icc_profile.cc#o...
ui/gfx/icc_profile.cc:60: return !(*this == other);
Nit: probably best to leave it defined in terms of operator==

https://codereview.chromium.org/2652503002/diff/20001/ui/gfx/ipc/color/gfx_pa...
File ui/gfx/ipc/color/gfx_param_traits.cc (right):

https://codereview.chromium.org/2652503002/diff/20001/ui/gfx/ipc/color/gfx_pa...
ui/gfx/ipc/color/gfx_param_traits.cc:97: r->ComputeColorSpaceAndCache();
What happens if the cached entry doesn't have matching data? Is it safe to
ignore that?

[ccameron, 2017-01-24 07:07:58]

The CQ bit was checked by ccameron@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-24 07:08:19]

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

[ccameron, 2017-01-24 07:09:57]

Thanks -- done!

https://codereview.chromium.org/2652503002/diff/20001/ui/gfx/icc_profile.cc
File ui/gfx/icc_profile.cc (left):

https://codereview.chromium.org/2652503002/diff/20001/ui/gfx/icc_profile.cc#o...
ui/gfx/icc_profile.cc:60: return !(*this == other);
On 2017/01/23 23:26:46, dcheng wrote:
> Nit: probably best to leave it defined in terms of operator==

Good point -- done.

https://codereview.chromium.org/2652503002/diff/20001/ui/gfx/ipc/color/gfx_pa...
File ui/gfx/ipc/color/gfx_param_traits.cc (right):

https://codereview.chromium.org/2652503002/diff/20001/ui/gfx/ipc/color/gfx_pa...
ui/gfx/ipc/color/gfx_param_traits.cc:97: r->ComputeColorSpaceAndCache();
On 2017/01/23 23:26:46, dcheng wrote:
> What happens if the cached entry doesn't have matching data? Is it safe to
> ignore that?

If we don't have the original ICC profile in the cache (on the other side, when
we convert from gfx::ColorSpace to gfx::ICCprofile), then we'll do a "best
effort" to re-create the profile (from the parametric transfer function and
matrix).

This very well could happen in real life -- you background a tab, change monitor
settings 5 times (flushing out the browser's ICC profile cache), then you switch
back to the original tab.

The compositor will receive gfx::ColorSpace, and it will draw it according to
the "best-effort" instead of the true ICC profile. The result will be
potentially slightly different coloring than expected for a few frames as Blink
re-rasterizes everything.

So, the consequence will be a brief flash of incorrect color. It'll be an
improvement on the current behavior (which is always incorrect) and slightly
better than Safari (which is incorrect for an undefined period of time after
switching monitor settings, though it snaps out of it reasonably quickly).

[commit-bot: I haz the power, 2017-01-24 07:14:56]

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

[commit-bot: I haz the power, 2017-01-24 07:14:57]

Dry run: Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[ccameron, 2017-01-25 06:59:12]

The CQ bit was checked by ccameron@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-25 06:59:31]

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, 2017-01-25 07:07:06]

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

[commit-bot: I haz the power, 2017-01-25 07:07:06]

Dry run: Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[ccameron, 2017-01-25 07:09:44]

ptal

I've gone ahead and implemented analytic transfer functions (and ensured that
they returned the same values as the previous ones).

https://codereview.chromium.org/2652503002/diff/1/ui/gfx/color_transform.cc
File ui/gfx/color_transform.cc (right):

https://codereview.chromium.org/2652503002/diff/1/ui/gfx/color_transform.cc#n...
ui/gfx/color_transform.cc:168: case ColorSpace::TransferID::CUSTOM:
Went ahead and did this.

[ccameron, 2017-01-25 22:41:13]

ping on this

[dcheng, 2017-01-25 22:42:14]

On 2017/01/25 22:41:13, ccameron wrote:
> ping on this

LGTM

[ccameron, 2017-01-25 22:50:47]

Thanks!

hubbe/msarett -- did you want to go over this again, or were you good with it
as-is?

[msarett1, 2017-01-25 22:51:49]

lgtm

[hubbe, 2017-01-25 22:54:59]

lgtm

[ccameron, 2017-01-25 23:01:03]

The CQ bit was checked by ccameron@chromium.org

[commit-bot: I haz the power, 2017-01-25 23:03:07]

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, 2017-01-25 23:18:29]

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

[commit-bot: I haz the power, 2017-01-25 23:18:31]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[ccameron, 2017-01-25 23:22:00]

What's the deal with that?

** Presubmit ERRORS **
Found changes to IPC files without a security OWNER!

***************
ui/gfx/mojo/OWNERS is missing the following lines:

per-file *.typemap=set noparent
per-file *.typemap=file://ipc/SECURITY_OWNERS

for changed files:
  ui/gfx/mojo/icc_profile.mojom
  ui/gfx/mojo/icc_profile.typemap
***************

dcheng is the first entry in ipc/SECURITY_OWNERS

[ccameron, 2017-01-25 23:22:45]

The CQ bit was checked by ccameron@chromium.org

[commit-bot: I haz the power, 2017-01-25 23:23:48]

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

[ccameron, 2017-01-25 23:30:47]

Description was changed from

==========
Use SkICC in gfx::ICCProfile and gfx::ColorSpace

This finally reaches the original goal of having gfx::ColorSpace be
a (potentially lossily) compressed version of gfx::ICCProfile.

The ICC profile's primaries and transfer funtion are extracted and
stored in the gfx::ColorSpace, and are used in the event that the
underlying ICC profile is purged from the cache.

Update ICCProfile's IPC methods to ensure that the profile is touched
in the cache and that its SkColorSpace is computed when it is
received via IPC.

BUG=634102
==========

to

==========
Use SkICC in gfx::ICCProfile and gfx::ColorSpace

This finally reaches the original goal of having gfx::ColorSpace be
a (potentially lossily) compressed version of gfx::ICCProfile.

The ICC profile's primaries and transfer funtion are extracted and
stored in the gfx::ColorSpace, and are used in the event that the
underlying ICC profile is purged from the cache.

Update ICCProfile's IPC methods to ensure that the profile is touched
in the cache and that its SkColorSpace is computed when it is
received via IPC.

BUG=634102
TBR=dcheng (CQ issues)
==========

[commit-bot: I haz the power, 2017-01-25 23:35:52]

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

[commit-bot: I haz the power, 2017-01-25 23:35:54]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[ccameron, 2017-01-25 23:50:24]

The CQ bit was checked by ccameron@chromium.org

[ccameron, 2017-01-25 23:50:33]

The CQ bit was unchecked by ccameron@chromium.org

[ccameron, 2017-01-25 23:54:45]

Description was changed from

==========
Use SkICC in gfx::ICCProfile and gfx::ColorSpace

This finally reaches the original goal of having gfx::ColorSpace be
a (potentially lossily) compressed version of gfx::ICCProfile.

The ICC profile's primaries and transfer funtion are extracted and
stored in the gfx::ColorSpace, and are used in the event that the
underlying ICC profile is purged from the cache.

Update ICCProfile's IPC methods to ensure that the profile is touched
in the cache and that its SkColorSpace is computed when it is
received via IPC.

BUG=634102
TBR=dcheng (CQ issues)
==========

to

==========
Use SkICC in gfx::ICCProfile and gfx::ColorSpace

This finally reaches the original goal of having gfx::ColorSpace be
a (potentially lossily) compressed version of gfx::ICCProfile.

The ICC profile's primaries and transfer funtion are extracted and
stored in the gfx::ColorSpace, and are used in the event that the
underlying ICC profile is purged from the cache.

Update ICCProfile's IPC methods to ensure that the profile is touched
in the cache and that its SkColorSpace is computed when it is
received via IPC.

BUG=634102
==========

[ccameron, 2017-01-25 23:55:11]

The CQ bit was checked by ccameron@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-25 23:56:02]

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

[ccameron, 2017-01-26 00:01:57]

On 2017/01/25 23:22:00, ccameron wrote:
> What's the deal with that?
> 
> ** Presubmit ERRORS **
> Found changes to IPC files without a security OWNER!
> 
> ***************
> ui/gfx/mojo/OWNERS is missing the following lines:
> 
> per-file *.typemap=set noparent
> per-file *.typemap=file://ipc/SECURITY_OWNERS
> 
> for changed files:
>   ui/gfx/mojo/icc_profile.mojom
>   ui/gfx/mojo/icc_profile.typemap
> ***************
> 
> dcheng is the first entry in ipc/SECURITY_OWNERS

Oooh, it's saying that the OWNERs file isn't how it should be. That's fix-able.

[commit-bot: I haz the power, 2017-01-26 01:36:02]

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

[commit-bot: I haz the power, 2017-01-26 01:36:03]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[ccameron, 2017-01-26 02:08:00]

The CQ bit was checked by ccameron@chromium.org

[ccameron, 2017-01-26 02:08:00]

The patchset sent to the CQ was uploaded after l-g-t-m from
msarett@chromium.org, hubbe@chromium.org, dcheng@chromium.org
Link to the patchset: https://codereview.chromium.org/2652503002/#ps90001
(title: "Fix OWNERS")

[commit-bot: I haz the power, 2017-01-26 02:09:06]

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, 2017-01-26 05:03:15]

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

[commit-bot: I haz the power, 2017-01-26 05:03:17]

Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[ccameron, 2017-01-26 06:45:00]

The CQ bit was checked by ccameron@chromium.org

[commit-bot: I haz the power, 2017-01-26 06:45:23]

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, 2017-01-26 08:32:46]

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

[commit-bot: I haz the power, 2017-01-26 08:32:50]

Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[ccameron, 2017-01-26 19:49:21]

The CQ bit was checked by ccameron@chromium.org

[ccameron, 2017-01-26 19:49:21]

The patchset sent to the CQ was uploaded after l-g-t-m from
msarett@chromium.org, hubbe@chromium.org, dcheng@chromium.org
Link to the patchset: https://codereview.chromium.org/2652503002/#ps110001
(title: "Rebase")

[commit-bot: I haz the power, 2017-01-26 19:50:12]

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, 2017-01-26 21:05:10]

CQ is committing da patch.

Bot data: {"patchset_id": 110001, "attempt_start_ts": 1485460161472540,
"parent_rev": "4e4f32ae6ea8802694dd36543d4b18f080b5b231", "commit_rev":
"9791f229838cb7ebf67f111023350fef4bcaeef1"}

[commit-bot: I haz the power, 2017-01-26 21:05:38]

Description was changed from

==========
Use SkICC in gfx::ICCProfile and gfx::ColorSpace

This finally reaches the original goal of having gfx::ColorSpace be
a (potentially lossily) compressed version of gfx::ICCProfile.

The ICC profile's primaries and transfer funtion are extracted and
stored in the gfx::ColorSpace, and are used in the event that the
underlying ICC profile is purged from the cache.

Update ICCProfile's IPC methods to ensure that the profile is touched
in the cache and that its SkColorSpace is computed when it is
received via IPC.

BUG=634102
==========

to

==========
Use SkICC in gfx::ICCProfile and gfx::ColorSpace

This finally reaches the original goal of having gfx::ColorSpace be
a (potentially lossily) compressed version of gfx::ICCProfile.

The ICC profile's primaries and transfer funtion are extracted and
stored in the gfx::ColorSpace, and are used in the event that the
underlying ICC profile is purged from the cache.

Update ICCProfile's IPC methods to ensure that the profile is touched
in the cache and that its SkColorSpace is computed when it is
received via IPC.

BUG=634102

Review-Url: https://codereview.chromium.org/2652503002
Cr-Commit-Position: refs/heads/master@{#446443}
Committed:
https://chromium.googlesource.com/chromium/src/+/9791f229838cb7ebf67f11102335...
==========

[commit-bot: I haz the power, 2017-01-26 21:05:39]

Committed patchset #7 (id:110001) as
https://chromium.googlesource.com/chromium/src/+/9791f229838cb7ebf67f11102335...

[suzyh_UTC10 (ex-contributor), 2017-01-27 00:24:15]

A revert of this CL (patchset #7 id:110001) has been created in
https://codereview.chromium.org/2663453002/ by suzyh@chromium.org.

The reason for reverting is: Suspecting this patch is responsible for
webkit_tests failures on Mac, where image diffs are showing slight colour
changes.
e.g.
https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Mac10.9/builds...
.

[ccameron, 2017-01-27 21:02:25]

Description was changed from

==========
Use SkICC in gfx::ICCProfile and gfx::ColorSpace

This finally reaches the original goal of having gfx::ColorSpace be
a (potentially lossily) compressed version of gfx::ICCProfile.

The ICC profile's primaries and transfer funtion are extracted and
stored in the gfx::ColorSpace, and are used in the event that the
underlying ICC profile is purged from the cache.

Update ICCProfile's IPC methods to ensure that the profile is touched
in the cache and that its SkColorSpace is computed when it is
received via IPC.

BUG=634102

Review-Url: https://codereview.chromium.org/2652503002
Cr-Commit-Position: refs/heads/master@{#446443}
Committed:
https://chromium.googlesource.com/chromium/src/+/9791f229838cb7ebf67f11102335...
==========

to

==========
Use SkICC in gfx::ICCProfile and gfx::ColorSpace

This finally reaches the original goal of having gfx::ColorSpace be
a (potentially lossily) compressed version of gfx::ICCProfile.

The ICC profile's primaries and transfer funtion are extracted and
stored in the gfx::ColorSpace, and are used in the event that the
underlying ICC profile is purged from the cache.

Update ICCProfile's IPC methods to ensure that the profile is touched
in the cache and that its SkColorSpace is computed when it is
received via IPC.

BUG=634102

Review-Url: https://codereview.chromium.org/2652503002
Cr-Commit-Position: refs/heads/master@{#446443}
Committed:
https://chromium.googlesource.com/chromium/src/+/9791f229838cb7ebf67f11102335...
==========

[ccameron, 2017-01-27 21:03:43]

Okay, I think the re-land won't have problems. This wasn't the
try-versus-continuous issue we saw earlier (AFAICT).

https://codereview.chromium.org/2652503002/diff/110001/ui/gfx/icc_profile.cc
File ui/gfx/icc_profile.cc (right):

https://codereview.chromium.org/2652503002/diff/110001/ui/gfx/icc_profile.cc#...
ui/gfx/icc_profile.cc:48: return data_ == data_;
Oops. Tests added.

[ccameron, 2017-01-27 21:07:55]

Description was changed from

==========
Use SkICC in gfx::ICCProfile and gfx::ColorSpace

This finally reaches the original goal of having gfx::ColorSpace be
a (potentially lossily) compressed version of gfx::ICCProfile.

The ICC profile's primaries and transfer funtion are extracted and
stored in the gfx::ColorSpace, and are used in the event that the
underlying ICC profile is purged from the cache.

Update ICCProfile's IPC methods to ensure that the profile is touched
in the cache and that its SkColorSpace is computed when it is
received via IPC.

BUG=634102

Review-Url: https://codereview.chromium.org/2652503002
Cr-Commit-Position: refs/heads/master@{#446443}
Committed:
https://chromium.googlesource.com/chromium/src/+/9791f229838cb7ebf67f11102335...
==========

to

==========
Use SkICC in gfx::ICCProfile and gfx::ColorSpace

This finally reaches the original goal of having gfx::ColorSpace be
a (potentially lossily) compressed version of gfx::ICCProfile.

The ICC profile's primaries and transfer funtion are extracted and
stored in the gfx::ColorSpace, and are used in the event that the
underlying ICC profile is purged from the cache.

Update ICCProfile's IPC methods to ensure that the profile is touched
in the cache and that its SkColorSpace is computed when it is
received via IPC.

BUG=634102
==========

[ccameron, 2017-01-27 21:08:53]

Description was changed from

==========
Use SkICC in gfx::ICCProfile and gfx::ColorSpace

This finally reaches the original goal of having gfx::ColorSpace be
a (potentially lossily) compressed version of gfx::ICCProfile.

The ICC profile's primaries and transfer funtion are extracted and
stored in the gfx::ColorSpace, and are used in the event that the
underlying ICC profile is purged from the cache.

Update ICCProfile's IPC methods to ensure that the profile is touched
in the cache and that its SkColorSpace is computed when it is
received via IPC.

BUG=634102
==========

to

==========
Use SkICC in gfx::ICCProfile and gfx::ColorSpace

This finally reaches the original goal of having gfx::ColorSpace be
a (potentially lossily) compressed version of gfx::ICCProfile.

The ICC profile's primaries and transfer funtion are extracted and
stored in the gfx::ColorSpace, and are used in the event that the
underlying ICC profile is purged from the cache.

Update ICCProfile's IPC methods to ensure that the profile is touched
in the cache and that its SkColorSpace is computed when it is
received via IPC.

BUG=634102
CQ_INCLUDE_TRYBOTS=mac10.10_blink_rel
==========

[ccameron, 2017-01-27 21:09:25]

The CQ bit was checked by ccameron@chromium.org to run a CQ dry run

[ccameron, 2017-01-27 21:09:38]

The CQ bit was checked by ccameron@chromium.org

[ccameron, 2017-01-27 21:09:38]

The patchset sent to the CQ was uploaded after l-g-t-m from
msarett@chromium.org, hubbe@chromium.org, dcheng@chromium.org
Link to the patchset: https://codereview.chromium.org/2652503002/#ps150001
(title: "Dont' change ColorBehavior")

[commit-bot: I haz the power, 2017-01-27 21:10:13]

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, 2017-01-27 21:10:15]

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

[commit-bot: I haz the power, 2017-01-27 21:10:16]

Your CL can not be processed by CQ because of:
 * Failed to parse additional trybots

[ccameron, 2017-01-27 21:13:48]

Description was changed from

==========
Use SkICC in gfx::ICCProfile and gfx::ColorSpace

This finally reaches the original goal of having gfx::ColorSpace be
a (potentially lossily) compressed version of gfx::ICCProfile.

The ICC profile's primaries and transfer funtion are extracted and
stored in the gfx::ColorSpace, and are used in the event that the
underlying ICC profile is purged from the cache.

Update ICCProfile's IPC methods to ensure that the profile is touched
in the cache and that its SkColorSpace is computed when it is
received via IPC.

BUG=634102
CQ_INCLUDE_TRYBOTS=mac10.10_blink_rel
==========

to

==========
Use SkICC in gfx::ICCProfile and gfx::ColorSpace

This finally reaches the original goal of having gfx::ColorSpace be
a (potentially lossily) compressed version of gfx::ICCProfile.

The ICC profile's primaries and transfer funtion are extracted and
stored in the gfx::ColorSpace, and are used in the event that the
underlying ICC profile is purged from the cache.

Update ICCProfile's IPC methods to ensure that the profile is touched
in the cache and that its SkColorSpace is computed when it is
received via IPC.

BUG=634102
CQ_INCLUDE_TRYBOTS=tryserver.blink:mac_blink_rel
==========

[ccameron, 2017-01-27 21:13:54]

The CQ bit was checked by ccameron@chromium.org

[commit-bot: I haz the power, 2017-01-27 21:14:41]

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, 2017-01-27 21:15:41]

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

[commit-bot: I haz the power, 2017-01-27 21:15:43]

CQ has no permission to schedule in bucket tryserver.blink

[ccameron, 2017-01-27 22:34:44]

Description was changed from

==========
Use SkICC in gfx::ICCProfile and gfx::ColorSpace

This finally reaches the original goal of having gfx::ColorSpace be
a (potentially lossily) compressed version of gfx::ICCProfile.

The ICC profile's primaries and transfer funtion are extracted and
stored in the gfx::ColorSpace, and are used in the event that the
underlying ICC profile is purged from the cache.

Update ICCProfile's IPC methods to ensure that the profile is touched
in the cache and that its SkColorSpace is computed when it is
received via IPC.

BUG=634102
CQ_INCLUDE_TRYBOTS=tryserver.blink:mac_blink_rel
==========

to

==========
Use SkICC in gfx::ICCProfile and gfx::ColorSpace

This finally reaches the original goal of having gfx::ColorSpace be
a (potentially lossily) compressed version of gfx::ICCProfile.

The ICC profile's primaries and transfer funtion are extracted and
stored in the gfx::ColorSpace, and are used in the event that the
underlying ICC profile is purged from the cache.

Update ICCProfile's IPC methods to ensure that the profile is touched
in the cache and that its SkColorSpace is computed when it is
received via IPC.

BUG=634102
==========

[ccameron, 2017-01-27 22:34:56]

The CQ bit was checked by ccameron@chromium.org

[commit-bot: I haz the power, 2017-01-27 22:35:47]

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, 2017-01-28 00:29:52]

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

[commit-bot: I haz the power, 2017-01-28 00:29:55]

Failed to apply patch for ui/gfx/color_transform.cc:
While running git apply --index -p1;
  error: patch failed: ui/gfx/color_transform.cc:29
  error: ui/gfx/color_transform.cc: patch does not apply

Patch:       ui/gfx/color_transform.cc
Index: ui/gfx/color_transform.cc
diff --git a/ui/gfx/color_transform.cc b/ui/gfx/color_transform.cc
index
3a63643c2a0fea8e6f8743984522a09a6e2e61d5..4d36fe29ca5b4bd32e6e3831c5479dec8c6e390c
100644
--- a/ui/gfx/color_transform.cc
+++ b/ui/gfx/color_transform.cc
@@ -21,6 +21,14 @@ extern "C" {
 
 namespace gfx {
 
+float EvalSkTransferFn(const SkColorSpaceTransferFn& fn, float x) {
+  if (x < 0)
+    return 0;
+  if (x < fn.fD)
+    return fn.fC * x + fn.fF;
+  return powf(fn.fA * x + fn.fB, fn.fG) + fn.fE;
+}
+
 Transform Invert(const Transform& t) {
   Transform ret = t;
   if (!t.GetInverse(&ret)) {
@@ -29,180 +37,12 @@ Transform Invert(const Transform& t) {
   return ret;
 }
 
-GFX_EXPORT Transform GetPrimaryMatrix(ColorSpace::PrimaryID id) {
-  SkColorSpacePrimaries primaries = {0};
-  switch (id) {
-    case ColorSpace::PrimaryID::CUSTOM:
-      NOTREACHED();
-
-    case ColorSpace::PrimaryID::RESERVED0:
-    case ColorSpace::PrimaryID::RESERVED:
-    case ColorSpace::PrimaryID::UNSPECIFIED:
-    case ColorSpace::PrimaryID::UNKNOWN:
-    case ColorSpace::PrimaryID::BT709:
-      // BT709 is our default case. Put it after the switch just
-      // in case we somehow get an id which is not listed in the switch.
-      // (We don't want to use "default", because we want the compiler
-      //  to tell us if we forgot some enum values.)
-      primaries.fRX = 0.640f;
-      primaries.fRY = 0.330f;
-      primaries.fGX = 0.300f;
-      primaries.fGY = 0.600f;
-      primaries.fBX = 0.150f;
-      primaries.fBY = 0.060f;
-      primaries.fWX = 0.3127f;
-      primaries.fWY = 0.3290f;
-      break;
-
-    case ColorSpace::PrimaryID::BT470M:
-      primaries.fRX = 0.67f;
-      primaries.fRY = 0.33f;
-      primaries.fGX = 0.21f;
-      primaries.fGY = 0.71f;
-      primaries.fBX = 0.14f;
-      primaries.fBY = 0.08f;
-      primaries.fWX = 0.31f;
-      primaries.fWY = 0.316f;
-      break;
-
-    case ColorSpace::PrimaryID::BT470BG:
-      primaries.fRX = 0.64f;
-      primaries.fRY = 0.33f;
-      primaries.fGX = 0.29f;
-      primaries.fGY = 0.60f;
-      primaries.fBX = 0.15f;
-      primaries.fBY = 0.06f;
-      primaries.fWX = 0.3127f;
-      primaries.fWY = 0.3290f;
-      break;
-
-    case ColorSpace::PrimaryID::SMPTE170M:
-    case ColorSpace::PrimaryID::SMPTE240M:
-      primaries.fRX = 0.630f;
-      primaries.fRY = 0.340f;
-      primaries.fGX = 0.310f;
-      primaries.fGY = 0.595f;
-      primaries.fBX = 0.155f;
-      primaries.fBY = 0.070f;
-      primaries.fWX = 0.3127f;
-      primaries.fWY = 0.3290f;
-      break;
-
-    case ColorSpace::PrimaryID::FILM:
-      primaries.fRX = 0.681f;
-      primaries.fRY = 0.319f;
-      primaries.fGX = 0.243f;
-      primaries.fGY = 0.692f;
-      primaries.fBX = 0.145f;
-      primaries.fBY = 0.049f;
-      primaries.fWX = 0.310f;
-      primaries.fWY = 0.136f;
-      break;
-
-    case ColorSpace::PrimaryID::BT2020:
-      primaries.fRX = 0.708f;
-      primaries.fRY = 0.292f;
-      primaries.fGX = 0.170f;
-      primaries.fGY = 0.797f;
-      primaries.fBX = 0.131f;
-      primaries.fBY = 0.046f;
-      primaries.fWX = 0.3127f;
-      primaries.fWY = 0.3290f;
-      break;
-
-    case ColorSpace::PrimaryID::SMPTEST428_1:
-      primaries.fRX = 1.0f;
-      primaries.fRY = 0.0f;
-      primaries.fGX = 0.0f;
-      primaries.fGY = 1.0f;
-      primaries.fBX = 0.0f;
-      primaries.fBY = 0.0f;
-      primaries.fWX = 1.0f / 3.0f;
-      primaries.fWY = 1.0f / 3.0f;
-      break;
-
-    case ColorSpace::PrimaryID::SMPTEST431_2:
-      primaries.fRX = 0.680f;
-      primaries.fRY = 0.320f;
-      primaries.fGX = 0.265f;
-      primaries.fGY = 0.690f;
-      primaries.fBX = 0.150f;
-      primaries.fBY = 0.060f;
-      primaries.fWX = 0.314f;
-      primaries.fWY = 0.351f;
-      break;
-
-    case ColorSpace::PrimaryID::SMPTEST432_1:
-      primaries.fRX = 0.680f;
-      primaries.fRY = 0.320f;
-      primaries.fGX = 0.265f;
-      primaries.fGY = 0.690f;
-      primaries.fBX = 0.150f;
-      primaries.fBY = 0.060f;
-      primaries.fWX = 0.3127f;
-      primaries.fWY = 0.3290f;
-      break;
-
-    case ColorSpace::PrimaryID::XYZ_D50:
-      primaries.fRX = 1.0f;
-      primaries.fRY = 0.0f;
-      primaries.fGX = 0.0f;
-      primaries.fGY = 1.0f;
-      primaries.fBX = 0.0f;
-      primaries.fBY = 0.0f;
-      primaries.fWX = 0.34567f;
-      primaries.fWY = 0.35850f;
-      break;
-  }
-
-  SkMatrix44 matrix;
-  primaries.toXYZD50(&matrix);
-  return Transform(matrix);
-}
-
-GFX_EXPORT float FromLinear(ColorSpace::TransferID id, float v) {
+float FromLinear(ColorSpace::TransferID id, float v) {
   switch (id) {
     case ColorSpace::TransferID::SMPTEST2084_NON_HDR:
       // Should already be handled.
-      NOTREACHED();
-    case ColorSpace::TransferID::CUSTOM:
-    // TODO(hubbe): Actually implement custom transfer functions.
-    case ColorSpace::TransferID::RESERVED0:
-    case ColorSpace::TransferID::RESERVED:
-    case ColorSpace::TransferID::UNSPECIFIED:
-    case ColorSpace::TransferID::UNKNOWN:
-    // All unknown values default to BT709
-
-    case ColorSpace::TransferID::BT709:
-    case ColorSpace::TransferID::SMPTE170M:
-    case ColorSpace::TransferID::BT2020_10:
-    case ColorSpace::TransferID::BT2020_12:
-      // BT709 is our "default" cause, so put the code after the switch
-      // to avoid "control reaches end of non-void function" errors.
       break;
 
-    case ColorSpace::TransferID::GAMMA22:
-      v = fmax(0.0f, v);
-      return powf(v, 1.0f / 2.2f);
-
-    case ColorSpace::TransferID::GAMMA28:
-      v = fmax(0.0f, v);
-      return powf(v, 1.0f / 2.8f);
-
-    case ColorSpace::TransferID::SMPTE240M: {
-      v = fmax(0.0f, v);
-      float a = 1.11157219592173128753f;
-      float b = 0.02282158552944503135f;
-      if (v <= b) {
-        return 4.0f * v;
-      } else {
-        return a * powf(v, 0.45f) - (a - 1.0f);
-      }
-    }
-
-    case ColorSpace::TransferID::LINEAR:
-      return v;
-
     case ColorSpace::TransferID::LOG:
       if (v < 0.01f)
         return 0.0f;
@@ -238,16 +78,6 @@ GFX_EXPORT float FromLinear(ColorSpace::TransferID id, float
v) {
       }
     }
 
-    case ColorSpace::TransferID::IEC61966_2_1: {  // SRGB
-      v = fmax(0.0f, v);
-      float a = 1.055f;
-      float b = 0.0031308f;
-      if (v < b) {
-        return 12.92f * v;
-      } else {
-        return a * powf(v, 1.0f / 2.4f) - (a - 1.0f);
-      }
-    }
     case ColorSpace::TransferID::SMPTEST2084: {
       // Go from scRGB levels to 0-1.
       v *= 80.0f / 10000.0f;
@@ -260,10 +90,6 @@ GFX_EXPORT float FromLinear(ColorSpace::TransferID id, float
v) {
       return powf((c1 + c2 * powf(v, m1)) / (1.0f + c3 * powf(v, m1)), m2);
     }
 
-    case ColorSpace::TransferID::SMPTEST428_1:
-      v = fmax(0.0f, v);
-      return powf(48.0f * v + 52.37f, 1.0f / 2.6f);
-
     // Spec: http://www.arib.or.jp/english/html/overview/doc/2-STD-B67v1_0.pdf
     case ColorSpace::TransferID::ARIB_STD_B67: {
       const float a = 0.17883277f;
@@ -276,62 +102,16 @@ GFX_EXPORT float FromLinear(ColorSpace::TransferID id,
float v) {
         return a * log(v - b) + c;
     }
 
-    // Chrome-specific values below
-    case ColorSpace::TransferID::GAMMA24:
-      v = fmax(0.0f, v);
-      return powf(v, 1.0f / 2.4f);
-  }
-
-  v = fmax(0.0f, v);
-  float a = 1.099296826809442f;
-  float b = 0.018053968510807f;
-  if (v <= b) {
-    return 4.5f * v;
-  } else {
-    return a * powf(v, 0.45f) - (a - 1.0f);
+    default:
+      // Handled by SkColorSpaceTransferFn.
+      break;
   }
+  NOTREACHED();
+  return 0;
 }
 
-GFX_EXPORT float ToLinear(ColorSpace::TransferID id, float v) {
+float ToLinear(ColorSpace::TransferID id, float v) {
   switch (id) {
-    case ColorSpace::TransferID::CUSTOM:
-    // TODO(hubbe): Actually implement custom transfer functions.
-    case ColorSpace::TransferID::RESERVED0:
-    case ColorSpace::TransferID::RESERVED:
-    case ColorSpace::TransferID::UNSPECIFIED:
-    case ColorSpace::TransferID::UNKNOWN:
-    // All unknown values default to BT709
-
-    case ColorSpace::TransferID::BT709:
-    case ColorSpace::TransferID::SMPTE170M:
-    case ColorSpace::TransferID::BT2020_10:
-    case ColorSpace::TransferID::BT2020_12:
-      // BT709 is our "default" cause, so put the code after the switch
-      // to avoid "control reaches end of non-void function" errors.
-      break;
-
-    case ColorSpace::TransferID::GAMMA22:
-      v = fmax(0.0f, v);
-      return powf(v, 2.2f);
-
-    case ColorSpace::TransferID::GAMMA28:
-      v = fmax(0.0f, v);
-      return powf(v, 2.8f);
-
-    case ColorSpace::TransferID::SMPTE240M: {
-      v = fmax(0.0f, v);
-      float a = 1.11157219592173128753f;
-      float b = 0.02282158552944503135f;
-      if (v <= FromLinear(ColorSpace::TransferID::SMPTE240M, b)) {
-        return v / 4.0f;
-      } else {
-        return powf((v + a - 1.0f) / a, 1.0f / 0.45f);
-      }
-    }
-
-    case ColorSpace::TransferID::LINEAR:
-      return v;
-
     case ColorSpace::TransferID::LOG:
       if (v < 0.0f)
         return 0.0f;
@@ -367,17 +147,6 @@ GFX_EXPORT float ToLinear(ColorSpace::TransferID id, float
v) {
       }
     }
 
-    case ColorSpace::TransferID::IEC61966_2_1: {  // SRGB
-      v = fmax(0.0f, v);
-      float a = 1.055f;
-      float b = 0.0031308f;
-      if (v < FromLinear(ColorSpace::TransferID::IEC61966_2_1, b)) {
-        return v / 12.92f;
-      } else {
-        return powf((v + a - 1.0f) / a, 2.4f);
-      }
-    }
-
     case ColorSpace::TransferID::SMPTEST2084: {
       v = fmax(0.0f, v);
       float m1 = (2610.0f / 4096.0f) / 4.0f;
@@ -395,14 +164,6 @@ GFX_EXPORT float ToLinear(ColorSpace::TransferID id, float
v) {
  …
(message too large)

[ccameron, 2017-01-28 02:01:01]

Patchset #10 (id:170001) has been deleted

[ccameron, 2017-01-28 04:35:31]

The CQ bit was checked by ccameron@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-28 04:35:45]

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

[ccameron, 2017-01-28 04:40:33]

The CQ bit was unchecked by ccameron@chromium.org

[ccameron, 2017-01-28 17:04:51]

The CQ bit was checked by ccameron@chromium.org

[ccameron, 2017-01-28 17:04:51]

The patchset sent to the CQ was uploaded after l-g-t-m from
msarett@chromium.org, hubbe@chromium.org, dcheng@chromium.org
Link to the patchset: https://codereview.chromium.org/2652503002/#ps190001
(title: "Rebase")

[commit-bot: I haz the power, 2017-01-28 17:05:04]

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, 2017-01-28 17:39:36]

CQ is committing da patch.

Bot data: {"patchset_id": 190001, "attempt_start_ts": 1485623091129990,
"parent_rev": "d2355655832d2e34e5a9159dd2fd47b069823ddd", "commit_rev":
"549738ba7a6df5e4872271233d40ecabc9857e7e"}

[commit-bot: I haz the power, 2017-01-28 17:40:17]

Description was changed from

==========
Use SkICC in gfx::ICCProfile and gfx::ColorSpace

This finally reaches the original goal of having gfx::ColorSpace be
a (potentially lossily) compressed version of gfx::ICCProfile.

The ICC profile's primaries and transfer funtion are extracted and
stored in the gfx::ColorSpace, and are used in the event that the
underlying ICC profile is purged from the cache.

Update ICCProfile's IPC methods to ensure that the profile is touched
in the cache and that its SkColorSpace is computed when it is
received via IPC.

BUG=634102
==========

to

==========
Use SkICC in gfx::ICCProfile and gfx::ColorSpace

This finally reaches the original goal of having gfx::ColorSpace be
a (potentially lossily) compressed version of gfx::ICCProfile.

The ICC profile's primaries and transfer funtion are extracted and
stored in the gfx::ColorSpace, and are used in the event that the
underlying ICC profile is purged from the cache.

Update ICCProfile's IPC methods to ensure that the profile is touched
in the cache and that its SkColorSpace is computed when it is
received via IPC.

BUG=634102

Review-Url: https://codereview.chromium.org/2652503002
Cr-Commit-Position: refs/heads/master@{#446923}
Committed:
https://chromium.googlesource.com/chromium/src/+/549738ba7a6df5e4872271233d40...
==========

[commit-bot: I haz the power, 2017-01-28 17:40:18]

Committed patchset #10 (id:190001) as
https://chromium.googlesource.com/chromium/src/+/549738ba7a6df5e4872271233d40...