Tune linear->sRGB constants to round-trip all bytes.

Tune linear->sRGB constants to round-trip all bytes.

I basically just ran a big 5-deep for-loop over the five constants here.
This is the first set of coefficients I found that round trips all bytes.
I suspect there are many such sets.

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2162063003
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/+/566ea9b9fc6746ffad390a4029e56d985eb2aec8

[mtklein_C, 2016-07-20 00:17:27]

Description was changed from

==========
Tune linear->sRGB constants to round-trip all bytes.

I basically just ran a big 5-deep for-loop over the five constants here.
This is the first set of coefficients I found that round trips all bytes.
I suspect there are many such sets.

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

to

==========
Tune linear->sRGB constants to round-trip all bytes.

I basically just ran a big 5-deep for-loop over the five constants here.
This is the first set of coefficients I found that round trips all bytes.
I suspect there are many such sets.

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

[mtklein_C, 2016-07-20 00:18:40]

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

[mtklein_C, 2016-07-20 00:18:41]

The CQ bit was checked by mtklein@chromium.org

[commit-bot: I haz the power, 2016-07-20 00:18:44]

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

[mtklein, 2016-07-20 00:57:50]

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

[mtklein, 2016-07-20 00:57:50]

https://codereview.chromium.org/2162063003/diff/1/src/core/SkSRGB.h
File src/core/SkSRGB.h (right):

https://codereview.chromium.org/2162063003/diff/1/src/core/SkSRGB.h#newcode36
src/core/SkSRGB.h:36: // to find one that round trips all bytes correctly while
minimizing some overall error.
Another thought for a follow up:

12.93, -0.097, 0.689, 0.411, 0.005 is perfect after truncation, rather than
rounding.  

That'll be more ARM-friendly, but will require we simultaneously convert
Sk4f_round back to SkNx_cast<int>.

[mtklein, 2016-07-20 01:37:21]

On 2016/07/20 00:57:50, mtklein wrote:
> https://codereview.chromium.org/2162063003/diff/1/src/core/SkSRGB.h
> File src/core/SkSRGB.h (right):
> 
> https://codereview.chromium.org/2162063003/diff/1/src/core/SkSRGB.h#newcode36
> src/core/SkSRGB.h:36: // to find one that round trips all bytes correctly
while
> minimizing some overall error.
> Another thought for a follow up:
> 
> 12.93, -0.097, 0.689, 0.411, 0.005 is perfect after truncation, rather than
> rounding.  
> 
> That'll be more ARM-friendly, but will require we simultaneously convert
> Sk4f_round back to SkNx_cast<int>.

I decided to make up an error function so I could tune a bit more.  The error
function I chose is, +inf if truncating to int is not correct, otherwise the
fractional part (always >0 and <1 otherwise truncation wouldn't be correct).  We
sum up those errors with uniform weighting across all 256 bytes (no squared
error).

That tuned me to these values,
    12.9232, -0.0958, 0.6946, 0.4047, 0.005
with a total error of 93.79 (--> avg. = 0.37)

[mtklein_C, 2016-07-20 12:22:29]

Description was changed from

==========
Tune linear->sRGB constants to round-trip all bytes.

I basically just ran a big 5-deep for-loop over the five constants here.
This is the first set of coefficients I found that round trips all bytes.
I suspect there are many such sets.

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

to

==========
Tune linear->sRGB constants to round-trip all bytes.

I basically just ran a big 5-deep for-loop over the five constants here.
This is the first set of coefficients I found that round trips all bytes.
I suspect there are many such sets.

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

[mtklein_C, 2016-07-20 12:25:21]

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

[commit-bot: I haz the power, 2016-07-20 12:25:26]

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

[mtklein_C, 2016-07-20 12:26:26]

mtklein@chromium.org changed reviewers:
- mtklein@google.com

[mtklein_C, 2016-07-20 12:26:26]

Added a second patch set that shows what a trunc world would look like.  It lets
us move the conversion to int inside sk_linear_to_srgb(), which I suspect will
make things a lot less error prone.  Look at all the caveats and notes we get to
remove from the comments!

[commit-bot: I haz the power, 2016-07-20 12:30:36]

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

[commit-bot: I haz the power, 2016-07-20 12:30:37]

Dry run: Try jobs failed on following builders:
  Build-Ubuntu-GCC-Mips-Debug-Android-Trybot on master.client.skia.compile
(JOB_FAILED,
http://build.chromium.org/p/client.skia.compile/builders/Build-Ubuntu-GCC-Mip...)

[mtklein_C, 2016-07-20 12:42:12]

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

[commit-bot: I haz the power, 2016-07-20 12:42:18]

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-20 13:15:49]

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

[commit-bot: I haz the power, 2016-07-20 13:15:50]

Dry run: This issue passed the CQ dry run.

[msarett, 2016-07-20 13:37:18]

I like the truncation idea.  Still trying to wrap my head around it in the sense
of being "optimal".

If we are planning to trunc, our "desired" value should really be X.5, where X
is whatever int we ultimately hope to end up with.  Then I think all the the
notions of accuracy are essentially the same.

This relies on the assumption that we never want sRGB gamma floats (which I
think is a fair assumption).

https://codereview.chromium.org/2162063003/diff/20001/src/core/SkSRGB.h
File src/core/SkSRGB.h (right):

https://codereview.chromium.org/2162063003/diff/20001/src/core/SkSRGB.h#newco...
src/core/SkSRGB.h:29: // TODO: It was easy enough to find these constants that
we should probably further tune
I've got something like this...  Minimizes the absolute error to 0.49 (which
means it should roundtrip if the hardware error doesn't mess us up).
int -> float -> round(int +/- 0.49) = int

I'm still refining things, but you might try:
0.702406539530268*sqrt + 0.393127893958605*ftrt -0.093617117847240
With the exact sRGB lower half and interval.

[msarett, 2016-07-20 13:56:49]

What are your thoughts on tuning the coefficients 5-deep vs. 3-deep?  I'm having
trouble feeling strongly.  It's nice to be perfect on the bottom segment, but
also nice to minimize error globally over the whole curve.

[mtklein, 2016-07-20 14:02:07]

On 2016/07/20 13:37:18, msarett wrote:

> This relies on the assumption that we never want sRGB gamma floats (which I
> think is a fair assumption).

Right.  I think this is a good assumption.  We never get them on the way from
bytes to linear floats,
so we shouldn't ever need them for anything except perhaps an intermediate on
the way back down.

> I'm still refining things, but you might try:
> 0.702406539530268*sqrt + 0.393127893958605*ftrt -0.093617117847240
> With the exact sRGB lower half and interval.

I'm seeing 15 values go off with rounding (including 1.0 rounding up to 256) and
most values off with truncation.


> ... the rest ...

I've been thinking that perhaps we can dodge the problem of having to invent an
error metric.
I also am not convinced that what I happened to tune on was optimal, and I'm not
convinced
that targeting X.5 is optimal either.

Here's what I propose instead: we continue to focus on a discrete metric, the
number of 
discrete points that map correctly.  We've now found that many sets of
coefficients can
map the 256 linear values corresponding to the 256 sRGB bytes correctly... let's
just
extend the pool of linear values we test.  E.g., while keeping the 256 values
which bytes
map to correct, how many of the 255 halfway points between those can we also
make
hit the correct byte?  If we get all those (unlikely?), try the 510 third and
two-third points, 
or quarter and three-quarter, etc.

[mtklein, 2016-07-20 14:08:04]

On 2016/07/20 13:56:49, msarett wrote:
> What are your thoughts on tuning the coefficients 5-deep vs. 3-deep?  I'm
having
> trouble feeling strongly.  It's nice to be perfect on the bottom segment, but
> also nice to minimize error globally over the whole curve.

It did seem like the linear parameters were nearly independent of the other
three,
and that the crossover threshold wasn't very sensitive, though that may change
if we test more points.

Intuitively it seems like we could tune the linear segment first to cover as
much
as it can do perfectly, to minimize the domain of the non-linear segment.  But
as
long as we keep a global perspective on error, it doesn't really matter what
order
we happen to tweak parameters... just where we get to in the end.

[msarett, 2016-07-20 14:13:17]

On 2016/07/20 14:02:07, mtklein wrote:
> On 2016/07/20 13:37:18, msarett wrote:
> 
> > This relies on the assumption that we never want sRGB gamma floats (which I
> > think is a fair assumption).
> 
> Right.  I think this is a good assumption.  We never get them on the way from
> bytes to linear floats,
> so we shouldn't ever need them for anything except perhaps an intermediate on
> the way back down.
> 
> > I'm still refining things, but you might try:
> > 0.702406539530268*sqrt + 0.393127893958605*ftrt -0.093617117847240
> > With the exact sRGB lower half and interval.
> 
> I'm seeing 15 values go off with rounding (including 1.0 rounding up to 256)
and
> most values off with truncation.
> 
> 
> > ... the rest ...
> 
> I've been thinking that perhaps we can dodge the problem of having to invent
an
> error metric.
> I also am not convinced that what I happened to tune on was optimal, and I'm
not
> convinced
> that targeting X.5 is optimal either.
> 
> Here's what I propose instead: we continue to focus on a discrete metric, the
> number of 
> discrete points that map correctly.  We've now found that many sets of
> coefficients can
> map the 256 linear values corresponding to the 256 sRGB bytes correctly...
let's
> just
> extend the pool of linear values we test.  E.g., while keeping the 256 values
> which bytes
> map to correct, how many of the 255 halfway points between those can we also
> make
> hit the correct byte?  If we get all those (unlikely?), try the 510 third and
> two-third points, 
> or quarter and three-quarter, etc.

I like that approach, but I'm not completely sure.  I see round-tripping as a
useful property (and a requirement now that we know we can get it), but not
really the metric to measure by.  More often than sRGB->sRGB, we'll do X->Y (I
think), and in that case we should be more concerned with being correct across
the entire float range.

I'm also not sure how much we should take the hardware behavior into account. 
My thoughts are:
Best Case: Our approximation is so good that the hardware error doesn't throw us
off enough to matter.
Other Case: We tune a little bit to how it behaves.  Probably take ARM into
account as well?  Will this continue to work on future versions?

[mtklein, 2016-07-20 14:15:50]

Here's my three laws of sRGBotics:
    1) All bytes 0..255 must round trip perfectly, especially 0 and 255.
    2) We're as fast as possible, except where that would conflict with rule 1.
    3) Maximum error is as low as possible, except where that would conflict
with rules 1 or 2.

Agree?

[msarett, 2016-07-20 14:31:08]

On 2016/07/20 14:15:50, mtklein wrote:
> Here's my three laws of sRGBotics:
>     1) All bytes 0..255 must round trip perfectly, especially 0 and 255.
>     2) We're as fast as possible, except where that would conflict with rule
1.
>     3) Maximum error is as low as possible, except where that would conflict
> with rules 1 or 2.
> 
> Agree?

Yes!  With the following additions.
(1*) ...on current Intel and Arm hardware.
(1a) No significant discontinutity where the segments match up.
(3*) ...measured as mathematical error.  I think this allows us to feel that the
equation isn't overly tuned to hardware quirks.

[mtklein_C, 2016-07-20 18:14:31]

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

[mtklein_C, 2016-07-20 18:14:40]

PTAL at PS7?

[commit-bot: I haz the power, 2016-07-20 18:14:42]

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

[mtklein_C, 2016-07-20 18:26:48]

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

[commit-bot: I haz the power, 2016-07-20 18:26:51]

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

[msarett, 2016-07-20 18:43:11]

This is great as is.  Really awesome actually!  But if you wanted to humor me,
I'd be interested to see the results for:
(1) Hold the two low params at 12.92 and 0.0031308.
(2) Use rounding strategy.
(3) Require continuity.

https://codereview.chromium.org/2162063003/diff/120001/tests/SRGBTest.cpp
File tests/SRGBTest.cpp (right):

https://codereview.chromium.org/2162063003/diff/120001/tests/SRGBTest.cpp#new...
tests/SRGBTest.cpp:29: // TODO: still needed?  more needed?
Update this comment.

https://codereview.chromium.org/2162063003/diff/120001/tests/SRGBTest.cpp#new...
tests/SRGBTest.cpp:34: for (float f = FLT_MIN; f <= 1.0f; ) {  // We don't
bother checking denorm values.
Why not?  Should we?

[mtklein_C, 2016-07-20 18:44:40]

On 2016/07/20 at 18:43:11, msarett wrote:
> This is great as is.  Really awesome actually!  But if you wanted to humor me,
I'd be interested to see the results for:
> (1) Hold the two low params at 12.92 and 0.0031308.
> (2) Use rounding strategy.
> (3) Require continuity.
Running.

> https://codereview.chromium.org/2162063003/diff/120001/tests/SRGBTest.cpp
> File tests/SRGBTest.cpp (right):
> 
>
https://codereview.chromium.org/2162063003/diff/120001/tests/SRGBTest.cpp#new...
> tests/SRGBTest.cpp:29: // TODO: still needed?  more needed?
> Update this comment.
Done.

>
https://codereview.chromium.org/2162063003/diff/120001/tests/SRGBTest.cpp#new...
> tests/SRGBTest.cpp:34: for (float f = FLT_MIN; f <= 1.0f; ) {  // We don't
bother checking denorm values.
> Why not?  Should we?
NEON can't handle it, and they'll all definitely be zero anyway.

[mtklein_C, 2016-07-20 18:45:56]

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

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

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

[msarett, 2016-07-20 18:47:49]

LGTM!!! Woohooo!!!!

[mtklein_C, 2016-07-20 18:52:29]

The CQ bit was unchecked by mtklein@chromium.org

[mtklein_C, 2016-07-20 18:52:31]

The CQ bit was checked by mtklein@chromium.org

[commit-bot: I haz the power, 2016-07-20 18:52:38]

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-20 19:10:14]

Description was changed from

==========
Tune linear->sRGB constants to round-trip all bytes.

I basically just ran a big 5-deep for-loop over the five constants here.
This is the first set of coefficients I found that round trips all bytes.
I suspect there are many such sets.

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

to

==========
Tune linear->sRGB constants to round-trip all bytes.

I basically just ran a big 5-deep for-loop over the five constants here.
This is the first set of coefficients I found that round trips all bytes.
I suspect there are many such sets.

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2162063003
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/+/566ea9b9fc6746ffad390a4029e56d985eb2aec8
==========

[commit-bot: I haz the power, 2016-07-20 19:10:15]

Committed patchset #9 (id:160001) as
https://skia.googlesource.com/skia/+/566ea9b9fc6746ffad390a4029e56d985eb2aec8