color: Update ICC histograms based on results

color: Update ICC histograms based on results

Based on the data gathered with the previous set of histograms, we can
assume that the range of the transfer function is [0, 1]. Bake this
assumption into the numerical approximation model (so we're solving for
fewer parameters).

Merge some of the histograms together under ICC analysis.

Add a histogram of the combined channels' convergence results and error
as well.

BUG=667966
Review-Url: https://codereview.chromium.org/2717263003
Cr-Commit-Position: refs/heads/master@{#453645}
Committed: https://chromium.googlesource.com/chromium/src/+/f3aa7226ee993cda646b50b271753d3bdf661cf9

[ccameron, 2017-02-27 09:01:09]

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

[commit-bot: I haz the power, 2017-02-27 09:01:48]

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

[ccameron, 2017-02-27 09:07:39]

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

[ccameron, 2017-02-27 09:07:40]

ptal -- tightening this up a bit.

Also merge the various steps of analyzing the ICC profile.

This also adds histograms for combined versus separate channels, and some tests
for that.

[commit-bot: I haz the power, 2017-02-27 09:45:23]

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

[commit-bot: I haz the power, 2017-02-27 09:45:24]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[msarett, 2017-02-27 14:35:22]

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

[msarett, 2017-02-27 14:35:23]

lgtm

https://codereview.chromium.org/2717263003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/graphics/ColorBehavior.cpp (right):

https://codereview.chromium.org/2717263003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/graphics/ColorBehavior.cpp:42: return
ICCFailedToExtractMatrix;
The set of profiles for which we fail to extract the matrix should be exactly
the same as the set of profiles for which we fail to extract the raw transfer
functions.

Maybe just track these in one bucket and add DCHECKs?

https://codereview.chromium.org/2717263003/diff/20001/ui/gfx/skia_color_space...
File ui/gfx/skia_color_space_util.cc (right):

https://codereview.chromium.org/2717263003/diff/20001/ui/gfx/skia_color_space...
ui/gfx/skia_color_space_util.cc:99: SkMatrix44 ne_lhs;
Missed this last time.  I think the preferred use would be:

SkMatrix44 ne_lhs(SkMatrix44::kIdentity_Constructor); // Then zero the diagonal
or
SkMatrix44 ne_lhs(SkMatrix44::kUninitialized_Constructor); // Then zero
everything

[ccameron, 2017-02-27 21:31:00]

ccameron@chromium.org changed reviewers:
+ isherman@chromium.org, pdr@chromium.org

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

Thanks!

Adding isherman@ for histograms.xml (removing and consolidating)

Adding pdr@ WebKit/

https://codereview.chromium.org/2717263003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/graphics/ColorBehavior.cpp (right):

https://codereview.chromium.org/2717263003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/graphics/ColorBehavior.cpp:42: return
ICCFailedToExtractMatrix;
On 2017/02/27 14:35:23, msarett wrote:
> The set of profiles for which we fail to extract the matrix should be exactly
> the same as the set of profiles for which we fail to extract the raw transfer
> functions.
> 
> Maybe just track these in one bucket and add DCHECKs?

I added a DCHECK and a comment, but left the enum ICCFailedToExtractRawTrFn,
since it keeps the flow of the function.

https://codereview.chromium.org/2717263003/diff/20001/ui/gfx/skia_color_space...
File ui/gfx/skia_color_space_util.cc (right):

https://codereview.chromium.org/2717263003/diff/20001/ui/gfx/skia_color_space...
ui/gfx/skia_color_space_util.cc:99: SkMatrix44 ne_lhs;
On 2017/02/27 14:35:23, msarett wrote:
> Missed this last time.  I think the preferred use would be:
> 
> SkMatrix44 ne_lhs(SkMatrix44::kIdentity_Constructor); // Then zero the
diagonal
> or
> SkMatrix44 ne_lhs(SkMatrix44::kUninitialized_Constructor); // Then zero
> everything

Good point -- done.

[ccameron, 2017-02-27 21:32:12]

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

[commit-bot: I haz the power, 2017-02-27 21:33:25]

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

[pdr., 2017-02-27 21:50:34]

LGTM

[commit-bot: I haz the power, 2017-02-27 23:30:55]

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

[commit-bot: I haz the power, 2017-02-27 23:30:56]

Dry run: This issue passed the CQ dry run.

[Ilya Sherman, 2017-02-28 02:02:16]

https://codereview.chromium.org/2717263003/diff/60001/tools/metrics/histogram...
File tools/metrics/histograms/histograms.xml (left):

https://codereview.chromium.org/2717263003/diff/60001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:4451: -<histogram
name="Blink.ColorSpace.Destination.LinearFitError">
Please mark removed histograms as <obsolete> rather than removing them entirely
from histograms.xml.

[ccameron, 2017-02-28 16:33:44]

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

[commit-bot: I haz the power, 2017-02-28 16:34:04]

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

[ccameron, 2017-02-28 16:34:41]

https://codereview.chromium.org/2717263003/diff/60001/tools/metrics/histogram...
File tools/metrics/histograms/histograms.xml (left):

https://codereview.chromium.org/2717263003/diff/60001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:4451: -<histogram
name="Blink.ColorSpace.Destination.LinearFitError">
On 2017/02/28 02:02:16, Ilya Sherman wrote:
> Please mark removed histograms as <obsolete> rather than removing them
entirely
> from histograms.xml.

Done.

[Ilya Sherman, 2017-02-28 18:13:22]

LGTM, thanks.

[ccameron, 2017-02-28 18:21:06]

The CQ bit was unchecked by ccameron@chromium.org

[ccameron, 2017-02-28 18:21:22]

The CQ bit was checked by ccameron@chromium.org

[ccameron, 2017-02-28 18:21:23]

The patchset sent to the CQ was uploaded after l-g-t-m from msarett@google.com,
pdr@chromium.org
Link to the patchset: https://codereview.chromium.org/2717263003/#ps80001
(title: "Add obsolete tags")

[commit-bot: I haz the power, 2017-02-28 18:21:34]

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-02-28 18:25:20]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1488306082763430,
"parent_rev": "ed6598e48801874306c942cf34fed2c9b5570dbe", "commit_rev":
"f3aa7226ee993cda646b50b271753d3bdf661cf9"}

[commit-bot: I haz the power, 2017-02-28 18:26:00]

Description was changed from

==========
color: Update ICC histograms based on results

Based on the data gathered with the previous set of histograms, we can
assume that the range of the transfer function is [0, 1]. Bake this
assumption into the numerical approximation model (so we're solving for
fewer parameters).

Merge some of the histograms together under ICC analysis.

Add a histogram of the combined channels' convergence results and error
as well.

BUG=667966
==========

to

==========
color: Update ICC histograms based on results

Based on the data gathered with the previous set of histograms, we can
assume that the range of the transfer function is [0, 1]. Bake this
assumption into the numerical approximation model (so we're solving for
fewer parameters).

Merge some of the histograms together under ICC analysis.

Add a histogram of the combined channels' convergence results and error
as well.

BUG=667966

Review-Url: https://codereview.chromium.org/2717263003
Cr-Commit-Position: refs/heads/master@{#453645}
Committed:
https://chromium.googlesource.com/chromium/src/+/f3aa7226ee993cda646b50b27175...
==========

[commit-bot: I haz the power, 2017-02-28 18:26:02]

Committed patchset #5 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/f3aa7226ee993cda646b50b27175...