Flush denorm half floats to zero.

Flush denorm half floats to zero.

I think we convinced ourselves that denorms, while a good chunk of half floats,
cover a rather small fraction of the representable range, which is always
close enough to zero to flush.

This makes both paths of the conversion to or from float considerably simpler.

These functions now work for zero-or-normal half floats (excluding infinite, NaN).
I'm not aware of a term for this class so I've called them "ordinary".

A handful of GMs and SKPs draw differently in --config f16, but all imperceptibly.

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

Committed: https://skia.googlesource.com/skia/+/8ae991e433d2c0814ea5579613f00173805ff057

[mtklein_C, 2016-08-18 00:01:21]

Description was changed from

==========
Flush denorm halfs to zero.

I think we convinced ourselves that this, while a good chunk of half floats,
represents a rather small fraction of the representable range, which is always
close enough to zero to flush.

This makes both paths of the conversion to or from float considerably simpler.

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

to

==========
Flush denorm halfs to zero.

I think we convinced ourselves that this, while a good chunk of half floats,
represents a rather small fraction of the representable range, which is always
close enough to zero to flush.

This makes both paths of the conversion to or from float considerably simpler.

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

[mtklein, 2016-08-18 00:02:12]

Description was changed from

==========
Flush denorm halfs to zero.

I think we convinced ourselves that this, while a good chunk of half floats,
represents a rather small fraction of the representable range, which is always
close enough to zero to flush.

This makes both paths of the conversion to or from float considerably simpler.

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

to

==========
Flush denorm halfs to zero.

I think we convinced ourselves that this, while a good chunk of half floats,
represents a rather small fraction of the representable range, which is always
close enough to zero to flush.

This makes both paths of the conversion to or from float considerably simpler.

These functions now work for zero-or-normal half floatss (excluding denorm,
infinite, NaN).  I'm not aware of a term for this class so I've called them
"ordinary".

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

[mtklein_C, 2016-08-18 00:04:26]

Description was changed from

==========
Flush denorm halfs to zero.

I think we convinced ourselves that this, while a good chunk of half floats,
represents a rather small fraction of the representable range, which is always
close enough to zero to flush.

This makes both paths of the conversion to or from float considerably simpler.

These functions now work for zero-or-normal half floatss (excluding denorm,
infinite, NaN).  I'm not aware of a term for this class so I've called them
"ordinary".

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

to

==========
Flush denorm halfs to zero.

I think we convinced ourselves that this, while a good chunk of half floats,
represents a rather small fraction of the representable range, which is always
close enough to zero to flush.

This makes both paths of the conversion to or from float considerably simpler.

These functions now work for zero-or-normal half floatss (excluding denorm,
infinite, NaN).  I'm not aware of a term for this class so I've called them
"ordinary".

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

[mtklein_C, 2016-08-18 00:06:03]

Description was changed from

==========
Flush denorm halfs to zero.

I think we convinced ourselves that this, while a good chunk of half floats,
represents a rather small fraction of the representable range, which is always
close enough to zero to flush.

This makes both paths of the conversion to or from float considerably simpler.

These functions now work for zero-or-normal half floatss (excluding denorm,
infinite, NaN).  I'm not aware of a term for this class so I've called them
"ordinary".

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

to

==========
Flush denorm half floats to zero.

I think we convinced ourselves that denorms, while a good chunk of half floats,
represent a rather small fraction of the representable range, which is always
close enough to zero to flush.

This makes both paths of the conversion to or from float considerably simpler.

These functions now work for zero-or-normal half floats (excluding denorm,
infinite, NaN).  I'm not aware of a term for this class so I've called them
"ordinary".

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

[mtklein_C, 2016-08-18 00:06:12]

Description was changed from

==========
Flush denorm half floats to zero.

I think we convinced ourselves that denorms, while a good chunk of half floats,
represent a rather small fraction of the representable range, which is always
close enough to zero to flush.

This makes both paths of the conversion to or from float considerably simpler.

These functions now work for zero-or-normal half floats (excluding denorm,
infinite, NaN).  I'm not aware of a term for this class so I've called them
"ordinary".

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

to

==========
Flush denorm half floats to zero.

I think we convinced ourselves that denorms, while a good chunk of half floats,
represent a rather small fraction of the representable range, which is always
close enough to zero to flush.

This makes both paths of the conversion to or from float considerably simpler.

These functions now work for zero-or-normal half floats (excluding denorm,
infinite, NaN).
I'm not aware of a term for this class so I've called them "ordinary".

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

[mtklein_C, 2016-08-18 00:09:02]

Description was changed from

==========
Flush denorm half floats to zero.

I think we convinced ourselves that denorms, while a good chunk of half floats,
represent a rather small fraction of the representable range, which is always
close enough to zero to flush.

This makes both paths of the conversion to or from float considerably simpler.

These functions now work for zero-or-normal half floats (excluding denorm,
infinite, NaN).
I'm not aware of a term for this class so I've called them "ordinary".

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

to

==========
Flush denorm half floats to zero.

I think we convinced ourselves that denorms, while a good chunk of half floats,
represent a rather small fraction of the representable range, which is always
close enough to zero to flush.

This makes both paths of the conversion to or from float considerably simpler.

These functions now work for zero-or-normal half floats (excluding infinite,
NaN).
I'm not aware of a term for this class so I've called them "ordinary".

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

[mtklein_C, 2016-08-18 00:11:10]

Description was changed from

==========
Flush denorm half floats to zero.

I think we convinced ourselves that denorms, while a good chunk of half floats,
represent a rather small fraction of the representable range, which is always
close enough to zero to flush.

This makes both paths of the conversion to or from float considerably simpler.

These functions now work for zero-or-normal half floats (excluding infinite,
NaN).
I'm not aware of a term for this class so I've called them "ordinary".

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

to

==========
Flush denorm half floats to zero.

I think we convinced ourselves that denorms, while a good chunk of half floats,
represent a rather small fraction of the representable range, which is always
close enough to zero to flush.

This makes both paths of the conversion to or from float considerably simpler.

These functions now work for zero-or-normal half floats (excluding infinite,
NaN).
I'm not aware of a term for this class so I've called them "ordinary".

A handful of GMs and SKPs draw differently in --config f16, but all
imperceptibly.

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

[mtklein_C, 2016-08-18 00:12:30]

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

[commit-bot: I haz the power, 2016-08-18 00:12:38]

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-08-18 00:16:40]

Description was changed from

==========
Flush denorm half floats to zero.

I think we convinced ourselves that denorms, while a good chunk of half floats,
represent a rather small fraction of the representable range, which is always
close enough to zero to flush.

This makes both paths of the conversion to or from float considerably simpler.

These functions now work for zero-or-normal half floats (excluding infinite,
NaN).
I'm not aware of a term for this class so I've called them "ordinary".

A handful of GMs and SKPs draw differently in --config f16, but all
imperceptibly.

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

to

==========
Flush denorm half floats to zero.

I think we convinced ourselves that denorms, while a good chunk of half floats,
cover a rather small fraction of the representable range, which is always
close enough to zero to flush.

This makes both paths of the conversion to or from float considerably simpler.

These functions now work for zero-or-normal half floats (excluding infinite,
NaN).
I'm not aware of a term for this class so I've called them "ordinary".

A handful of GMs and SKPs draw differently in --config f16, but all
imperceptibly.

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

[commit-bot: I haz the power, 2016-08-18 00:41:26]

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

[commit-bot: I haz the power, 2016-08-18 00:41:27]

Dry run: This issue passed the CQ dry run.

[mtklein_C, 2016-08-18 00:54:57]

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

[Brian Osman, 2016-08-18 12:45:57]

lgtm

Note: Smallest normalized half float is ~6.1*10^-5. Converted to sRGB, that's
~7.9*10^-4. If we then assume 10-bit output (so * 1023), we get ~0.806. So, we
do end up rounding the wrong way, but the overall error seems pretty
insignificant.

[msarett, 2016-08-18 13:19:55]

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

https://codereview.chromium.org/2256023002/diff/20001/src/core/SkHalf.h#newco...
src/core/SkHalf.h:82: Sk4i merged = (sign >> 16) | (will_be_norm & norm);
Woohoo!  This file gets a lot simpler!

https://codereview.chromium.org/2256023002/diff/20001/tests/Float16Test.cpp
File tests/Float16Test.cpp (right):

https://codereview.chromium.org/2256023002/diff/20001/tests/Float16Test.cpp#n...
tests/Float16Test.cpp:65: return f == 0.0f || isfinite(f);
nit: Why not sk_float_is_finite()?

nit: Change function name?  This seems to work because you also call
is_denorm(), but wouldn't a true is_ordinary() need to check denorms?
Ex: return isfinite(f) && (!will_be_denorm(f) || 0.0f == f);

https://codereview.chromium.org/2256023002/diff/20001/tests/Float16Test.cpp#n...
tests/Float16Test.cpp:75: continue;
Can we assert that this converts to zero here?

https://codereview.chromium.org/2256023002/diff/20001/tests/Float16Test.cpp#n...
tests/Float16Test.cpp:104: if (is_ordinary(f) &&
is_ordinary(SkHalfToFloat(SkFloatToHalf(f)))) {
Will this case ever not be true?
is_ordinary(SkHalfToFloat(SkFloatToHalf(f)))

Can we assert it?

https://codereview.chromium.org/2256023002/diff/20001/tests/Float16Test.cpp#n...
tests/Float16Test.cpp:108: continue;
Can we assert that h1 is zero in this case?

https://codereview.chromium.org/2256023002/diff/20001/tests/Float16Test.cpp#n...
tests/Float16Test.cpp:110: bool ok = (h1 == h2 || h1 == h2-1);
Realize this is not new...  But do we allow a tolerance?

[mtklein_C, 2016-08-22 19:45:47]

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

[commit-bot: I haz the power, 2016-08-22 19:45:49]

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-08-22 19:47:05]

On 2016/08/18 at 13:19:55, msarett wrote:
> https://codereview.chromium.org/2256023002/diff/20001/src/core/SkHalf.h
> File src/core/SkHalf.h (right):
> 
>
https://codereview.chromium.org/2256023002/diff/20001/src/core/SkHalf.h#newco...
> src/core/SkHalf.h:82: Sk4i merged = (sign >> 16) | (will_be_norm & norm);
> Woohoo!  This file gets a lot simpler!
> 
> https://codereview.chromium.org/2256023002/diff/20001/tests/Float16Test.cpp
> File tests/Float16Test.cpp (right):
> 
>
https://codereview.chromium.org/2256023002/diff/20001/tests/Float16Test.cpp#n...
> tests/Float16Test.cpp:65: return f == 0.0f || isfinite(f);
> nit: Why not sk_float_is_finite()?
> 
> nit: Change function name?  This seems to work because you also call
is_denorm(), but wouldn't a true is_ordinary() need to check denorms?
> Ex: return isfinite(f) && (!will_be_denorm(f) || 0.0f == f);
> 
>
https://codereview.chromium.org/2256023002/diff/20001/tests/Float16Test.cpp#n...
> tests/Float16Test.cpp:75: continue;
> Can we assert that this converts to zero here?
> 
>
https://codereview.chromium.org/2256023002/diff/20001/tests/Float16Test.cpp#n...
> tests/Float16Test.cpp:104: if (is_ordinary(f) &&
is_ordinary(SkHalfToFloat(SkFloatToHalf(f)))) {
> Will this case ever not be true?
> is_ordinary(SkHalfToFloat(SkFloatToHalf(f)))
> 
> Can we assert it?
> 
>
https://codereview.chromium.org/2256023002/diff/20001/tests/Float16Test.cpp#n...
> tests/Float16Test.cpp:108: continue;
> Can we assert that h1 is zero in this case?
> 
>
https://codereview.chromium.org/2256023002/diff/20001/tests/Float16Test.cpp#n...
> tests/Float16Test.cpp:110: bool ok = (h1 == h2 || h1 == h2-1);
> Realize this is not new...  But do we allow a tolerance?

Matt, can you take another look at PS 3?  I've renamed the functions to
*_finite_ftz, and rewritten the tests from scratch.  This in turn found a place
I was a little lazy with the will-be-denorm threshold.  The most interesting
changes are in the tests, which I hope now explain themselves better.

[msarett, 2016-08-22 20:02:04]

lgtm

[commit-bot: I haz the power, 2016-08-22 20:17:06]

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

[commit-bot: I haz the power, 2016-08-22 20:17:13]

Dry run: This issue passed the CQ dry run.

[mtklein_C, 2016-08-22 20:18:42]

The CQ bit was checked by mtklein@chromium.org

[mtklein_C, 2016-08-22 20:18:42]

The patchset sent to the CQ was uploaded after l-g-t-m from
brianosman@google.com
Link to the patchset: https://codereview.chromium.org/2256023002/#ps40001
(title: "fix threshold, clean up tests")

[commit-bot: I haz the power, 2016-08-22 20:18:45]

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-08-22 20:20:20]

Description was changed from

==========
Flush denorm half floats to zero.

I think we convinced ourselves that denorms, while a good chunk of half floats,
cover a rather small fraction of the representable range, which is always
close enough to zero to flush.

This makes both paths of the conversion to or from float considerably simpler.

These functions now work for zero-or-normal half floats (excluding infinite,
NaN).
I'm not aware of a term for this class so I've called them "ordinary".

A handful of GMs and SKPs draw differently in --config f16, but all
imperceptibly.

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

to

==========
Flush denorm half floats to zero.

I think we convinced ourselves that denorms, while a good chunk of half floats,
cover a rather small fraction of the representable range, which is always
close enough to zero to flush.

This makes both paths of the conversion to or from float considerably simpler.

These functions now work for zero-or-normal half floats (excluding infinite,
NaN).
I'm not aware of a term for this class so I've called them "ordinary".

A handful of GMs and SKPs draw differently in --config f16, but all
imperceptibly.

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

Committed:
https://skia.googlesource.com/skia/+/8ae991e433d2c0814ea5579613f00173805ff057
==========

[commit-bot: I haz the power, 2016-08-22 20:20:21]

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