color: Add histograms for gamut

color: Add histograms for gamut

Add histograms recording color gamut information about images and screens.

BUG=667431
Committed: https://crrev.com/487d13845d53146564b64659c9a9611c93e23e22
Cr-Commit-Position: refs/heads/master@{#438715}

[hubbe, 2016-12-12 23:20:30]

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

[hubbe, 2016-12-12 23:21:32]

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

[commit-bot: I haz the power, 2016-12-12 23:21:34]

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

[hubbe, 2016-12-12 23:48:53]

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

[commit-bot: I haz the power, 2016-12-12 23:49:53]

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

[hubbe, 2016-12-13 01:18:52]

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

[Ken Russell (switch to Gerrit), 2016-12-13 01:45:52]

https://codereview.chromium.org/2567983004/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/graphics/BitmapImageMetrics.h (right):

https://codereview.chromium.org/2567983004/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/graphics/BitmapImageMetrics.h:62: GamutEnd =
GamutBT2020 + 1,
GamutEnd looks incorrectly computed. Why not just leave its value undefined, so
that it's implicitly GamutUltraWide + 1?

[Ken Russell (switch to Gerrit), 2016-12-13 01:46:15]

Description was changed from

==========
color: Add historgrams for gamut

Add histograms recording color gamut information about images and screens.
==========

to

==========
color: Add histograms for gamut

Add histograms recording color gamut information about images and screens.
==========

[Ken Russell (switch to Gerrit), 2016-12-13 01:46:29]

Also, could you please reference a bug ID?

[commit-bot: I haz the power, 2016-12-13 01:46:40]

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

[commit-bot: I haz the power, 2016-12-13 01:46:40]

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

[hubbe, 2016-12-13 04:13:08]

Description was changed from

==========
color: Add histograms for gamut

Add histograms recording color gamut information about images and screens.
==========

to

==========
color: Add histograms for gamut

Add histograms recording color gamut information about images and screens.

BUG=44872
==========

[hubbe, 2016-12-13 04:14:56]

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

[commit-bot: I haz the power, 2016-12-13 04:15:18]

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

[hubbe, 2016-12-13 04:15:18]

Added BUG

https://codereview.chromium.org/2567983004/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/graphics/BitmapImageMetrics.h (right):

https://codereview.chromium.org/2567983004/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/graphics/BitmapImageMetrics.h:62: GamutEnd =
GamutBT2020 + 1,
On 2016/12/13 01:45:52, Ken Russell wrote:
> GamutEnd looks incorrectly computed. Why not just leave its value undefined,
so
> that it's implicitly GamutUltraWide + 1?

Done.

[commit-bot: I haz the power, 2016-12-13 06:16:13]

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

[commit-bot: I haz the power, 2016-12-13 06:16:14]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Ken Russell (switch to Gerrit), 2016-12-13 07:20:14]

Issue 44872 has been closed as a duplicate. Could you refer to an open bug
(presumably Issue 667431?) Thanks.

LGTM with that change.

[hubbe, 2016-12-13 07:21:01]

Description was changed from

==========
color: Add histograms for gamut

Add histograms recording color gamut information about images and screens.

BUG=44872
==========

to

==========
color: Add histograms for gamut

Add histograms recording color gamut information about images and screens.

BUG=667431
==========

[hubbe, 2016-12-13 07:22:02]

The CQ bit was checked by hubbe@chromium.org

[commit-bot: I haz the power, 2016-12-13 07:22:18]

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-12-13 07:30:10]

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

[commit-bot: I haz the power, 2016-12-13 07:30:11]

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...)

[msarett1, 2016-12-13 16:00:08]

Cool, glad we're measuring this!

lgtm

[ccameron, 2016-12-13 16:56:41]

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

[ccameron, 2016-12-13 16:56:41]

Could we histogram the continuous variable instead?

[hubbe, 2016-12-13 18:09:18]

On 2016/12/13 16:56:41, ccameron (OOO til 12-19) wrote:
> Could we histogram the continuous variable instead?

How accurate are those histograms?
I'd worry that we wouldn't see the difference between AdobeRGB and P3, as the
cutoff has to be in the right place.

[hubbe, 2016-12-13 18:37:32]

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

[hubbe, 2016-12-13 18:37:33]

+rkaplow for histograms.xml

[hubbe, 2016-12-14 18:38:04]

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

[hubbe, 2016-12-14 18:38:04]

+ jwd for histograms.xml

[rkaplow, 2016-12-14 20:54:10]

lgtm

enum version looks good.

you could make a numeric histogram that tracks the continuous variable - there's
flexibility in how the buckets are set, so you could make sure you had enough
buckets that it was clearly defined (for example linear with enough buckets such
that there would be no overlap).

[hubbe, 2016-12-14 20:56:59]

On 2016/12/14 20:54:10, rkaplow wrote:
> lgtm
> 
> enum version looks good.
> 
> you could make a numeric histogram that tracks the continuous variable -
there's
> flexibility in how the buckets are set, so you could make sure you had enough
> buckets that it was clearly defined (for example linear with enough buckets
such
> that there would be no overlap).

I did think about it some more, and while it's tempting, I think it makes the
data harder to use.
The value of the variable uses a completely made-up scale, so it's not really
possible to look up
what the values mean anywhere, you just have to Know.

[hubbe, 2016-12-14 20:57:04]

The CQ bit was checked by hubbe@chromium.org

[commit-bot: I haz the power, 2016-12-14 20:57:54]

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

[rkaplow, 2016-12-14 20:58:28]

lgtm

in that case giving a clear definition client side and using that as you did
seems simplest and best, so L G T M

[commit-bot: I haz the power, 2016-12-14 23:13:33]

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

[commit-bot: I haz the power, 2016-12-14 23:13:35]

Failed to apply patch for
third_party/WebKit/Source/platform/graphics/ColorBehavior.cpp:
While running git apply --index -p1;
  error: patch failed:
third_party/WebKit/Source/platform/graphics/ColorBehavior.cpp:35
  error: third_party/WebKit/Source/platform/graphics/ColorBehavior.cpp: patch
does not apply

Patch:       third_party/WebKit/Source/platform/graphics/ColorBehavior.cpp
Index: third_party/WebKit/Source/platform/graphics/ColorBehavior.cpp
diff --git a/third_party/WebKit/Source/platform/graphics/ColorBehavior.cpp
b/third_party/WebKit/Source/platform/graphics/ColorBehavior.cpp
index
10969e12369f813e18fee2c1787079da3769ce82..4a2c359b175ea89dad2c75a6f693a5a9c412d4dc
100644
--- a/third_party/WebKit/Source/platform/graphics/ColorBehavior.cpp
+++ b/third_party/WebKit/Source/platform/graphics/ColorBehavior.cpp
@@ -35,7 +35,7 @@ void ColorBehavior::setGlobalTargetColorProfile(
       SkColorSpace::MakeICC(profile.data(), profile.size()).release();
 
   // UMA statistics.
-  BitmapImageMetrics::countOutputGamma(gTargetColorSpace);
+  BitmapImageMetrics::countOutputGammaAndGamut(gTargetColorSpace);
 }
 
 // static

[hubbe, 2016-12-15 00:07:41]

The CQ bit was checked by hubbe@chromium.org

[hubbe, 2016-12-15 00:07:41]

The patchset sent to the CQ was uploaded after l-g-t-m from kbr@chromium.org,
msarett@chromium.org, rkaplow@chromium.org
Link to the patchset: https://codereview.chromium.org/2567983004/#ps60001
(title: "merged")

[commit-bot: I haz the power, 2016-12-15 00:08:26]

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-12-15 01:58:49]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1481760461497470,
"parent_rev": "0c45985a01a606db1b4e08ba26575721e074e8bc", "commit_rev":
"f867dc308c9bc0ec6e0874d1e3b939653a2d4f0e"}

[commit-bot: I haz the power, 2016-12-15 01:59:14]

Description was changed from

==========
color: Add histograms for gamut

Add histograms recording color gamut information about images and screens.

BUG=667431
==========

to

==========
color: Add histograms for gamut

Add histograms recording color gamut information about images and screens.

BUG=667431

Review-Url: https://codereview.chromium.org/2567983004
==========

[commit-bot: I haz the power, 2016-12-15 01:59:15]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-12-15 02:00:35]

Description was changed from

==========
color: Add histograms for gamut

Add histograms recording color gamut information about images and screens.

BUG=667431

Review-Url: https://codereview.chromium.org/2567983004
==========

to

==========
color: Add histograms for gamut

Add histograms recording color gamut information about images and screens.

BUG=667431

Committed: https://crrev.com/487d13845d53146564b64659c9a9611c93e23e22
Cr-Commit-Position: refs/heads/master@{#438715}
==========

[commit-bot: I haz the power, 2016-12-15 02:00:37]

Patchset 4 (id:??) landed as
https://crrev.com/487d13845d53146564b64659c9a9611c93e23e22
Cr-Commit-Position: refs/heads/master@{#438715}