Move float<->byte conversions into Sk4f.

Move float<->byte conversions into Sk4f.

This lets us avoid conversions to [0.0, 1.0] space and rounding that aren't necessary
for SkColorCubeFilter_opts.h.

Dropping rounding on the way back to bytes means we'll see a bunch of off-by-1 diffs.

Rough perf effect:
  SSSE3: 110 -> 93  (~15%)
  NEON: 465 -> 375  (~20%)

This is the beginning of the end for SkPMFloat as an entity distinct from Sk4f.
I've kept it for now so I can convert sites one by one and think about how things
that really want to keep PM color order will work.


BUG=skia:4117
Committed: https://skia.googlesource.com/skia/+/aba1dc8c6aa5cbc4f38ddfce757832359f200b54

[mtklein_C, 2015-08-31 18:37:21]

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

[commit-bot: I haz the power, 2015-08-31 18:37:31]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1319413003/140001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1319413003/140001

[commit-bot: I haz the power, 2015-08-31 18:42:32]

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

[commit-bot: I haz the power, 2015-08-31 18:42:33]

Dry run: This issue passed the CQ dry run.

[mtklein_C, 2015-08-31 18:52:45]

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

[commit-bot: I haz the power, 2015-08-31 18:52:52]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1319413003/160001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1319413003/160001

[commit-bot: I haz the power, 2015-08-31 18:58:32]

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

[commit-bot: I haz the power, 2015-08-31 18:58:33]

Dry run: This issue passed the CQ dry run.

[mtklein_C, 2015-08-31 19:00:38]

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

[mtklein_C, 2015-08-31 19:00:38]

This is, I think, a better alternative to
https://codereview.chromium.org/1308903003/.

[mtklein_C, 2015-08-31 20:20:38]

If this all looks good, I'll be sending out this follow up to remove SkPMFloat
entirely: https://codereview.chromium.org/1324743002

[reed1, 2015-08-31 21:16:32]

yummy yummy fast fast

lgtm

[mtklein, 2015-08-31 21:39:07]

The CQ bit was checked by mtklein@google.com

[commit-bot: I haz the power, 2015-08-31 21:39:29]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1319413003/160001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1319413003/160001

[commit-bot: I haz the power, 2015-08-31 21:40:02]

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

[Noel Gordon, 2015-09-01 04:00:06]

https://codereview.chromium.org/1319413003/diff/160001/src/opts/SkColorCubeFi...
File src/opts/SkColorCubeFilter_opts.h (left):

https://codereview.chromium.org/1319413003/diff/160001/src/opts/SkColorCubeFi...
src/opts/SkColorCubeFilter_opts.h:77: 
round() clamped as well.  Dose the new CL clamp?

[Noel Gordon, 2015-09-01 04:04:37]

On 2015/09/01 04:00:06, noel gordon wrote:
>
https://codereview.chromium.org/1319413003/diff/160001/src/opts/SkColorCubeFi...
> File src/opts/SkColorCubeFilter_opts.h (left):
> 
>
https://codereview.chromium.org/1319413003/diff/160001/src/opts/SkColorCubeFi...
> src/opts/SkColorCubeFilter_opts.h:77: 
> round() clamped as well.  Dose the new CL clamp?

Ahem, does the new CL clamp?

[mtklein, 2015-09-01 11:57:42]

On 2015/09/01 04:04:37, noel gordon wrote:
> On 2015/09/01 04:00:06, noel gordon wrote:
> >
>
https://codereview.chromium.org/1319413003/diff/160001/src/opts/SkColorCubeFi...
> > File src/opts/SkColorCubeFilter_opts.h (left):
> > 
> >
>
https://codereview.chromium.org/1319413003/diff/160001/src/opts/SkColorCubeFi...
> > src/opts/SkColorCubeFilter_opts.h:77: 
> > round() clamped as well.  Dose the new CL clamp?
> 
> Ahem, does the new CL clamp?

That's an area with some flexibility.  Right now, the _sse and _neon
implementations truncate and clamp to [0,255], while the portable implementation
simply casts to a uint8_t, which seems to return 0 when out of range.  Not
requiring clamping does give us some slight room to speed up _sse + _neon
toBytes(), but so far it really does seem slight.  Alternatively, if we do
require clamping, it'd be easy enough to fix up the portable path as
(uint8_t)(SkTMin(val, 255.0f) and add a note to the API.

[Noel Gordon, 2015-09-01 12:43:47]

On 2015/09/01 11:57:42, mtklein wrote:
> On 2015/09/01 04:04:37, noel gordon wrote:
> > On 2015/09/01 04:00:06, noel gordon wrote:
> > >
> >
>
https://codereview.chromium.org/1319413003/diff/160001/src/opts/SkColorCubeFi...
> > > File src/opts/SkColorCubeFilter_opts.h (left):
> > > 
> > >
> >
>
https://codereview.chromium.org/1319413003/diff/160001/src/opts/SkColorCubeFi...
> > > src/opts/SkColorCubeFilter_opts.h:77: 
> > > round() clamped as well.  Dose the new CL clamp?
> > 
> > Ahem, does the new CL clamp?
> 
> That's an area with some flexibility.  Right now, the _sse and _neon
> implementations truncate and clamp to [0,255], while the portable
implementation
> simply casts to a uint8_t, which seems to return 0 when out of range.

IC.

> Not requiring clamping does give us some slight room to speed up _sse + _neon
> toBytes(), but so far it really does seem slight.  Alternatively, if we do
> require clamping, it'd be easy enough to fix up the portable path as
> (uint8_t)(SkTMin(val, 255.0f) and add a note to the API.

As an FYI, note that some color corrections cause the resulting colors in
floating point space to exceed 1.0, or be less than 0.0, and convention is to
clamp to [0.0..1.0], aka [0..255] as bytes, with rounding. My understanding was
the previous implementation SkPMFloat.round() did that, yes?

[mtklein, 2015-09-01 12:45:47]

On 2015/09/01 12:43:47, noel gordon wrote:
> On 2015/09/01 11:57:42, mtklein wrote:
> > On 2015/09/01 04:04:37, noel gordon wrote:
> > > On 2015/09/01 04:00:06, noel gordon wrote:
> > > >
> > >
> >
>
https://codereview.chromium.org/1319413003/diff/160001/src/opts/SkColorCubeFi...
> > > > File src/opts/SkColorCubeFilter_opts.h (left):
> > > > 
> > > >
> > >
> >
>
https://codereview.chromium.org/1319413003/diff/160001/src/opts/SkColorCubeFi...
> > > > src/opts/SkColorCubeFilter_opts.h:77: 
> > > > round() clamped as well.  Dose the new CL clamp?
> > > 
> > > Ahem, does the new CL clamp?
> > 
> > That's an area with some flexibility.  Right now, the _sse and _neon
> > implementations truncate and clamp to [0,255], while the portable
> implementation
> > simply casts to a uint8_t, which seems to return 0 when out of range.
> 
> IC.
> 
> > Not requiring clamping does give us some slight room to speed up _sse +
_neon
> > toBytes(), but so far it really does seem slight.  Alternatively, if we do
> > require clamping, it'd be easy enough to fix up the portable path as
> > (uint8_t)(SkTMin(val, 255.0f) and add a note to the API.
> 
> As an FYI, note that some color corrections cause the resulting colors in
> floating point space to exceed 1.0, or be less than 0.0, and convention is to
> clamp to [0.0..1.0], aka [0..255] as bytes, with rounding. My understanding
was
> the previous implementation SkPMFloat.round() did that, yes?

Yes, round() rounded and clamped.  https://codereview.chromium.org/1312053004/ ?

[Noel Gordon, 2015-09-01 12:50:00]

On 2015/09/01 12:45:47, mtklein wrote:
 
> Yes, round() rounded and clamped.  https://codereview.chromium.org/1312053004/
?

That was quick, let me look.

[Noel Gordon, 2015-09-01 13:09:31]

On 2015/09/01 12:50:00, noel gordon wrote:
> On 2015/09/01 12:45:47, mtklein wrote:
>  
> > Yes, round() rounded and clamped. 
https://codereview.chromium.org/1312053004

Ok looked. That CL truncates and clamps, which is not quite that same as
round(), so a change in behavior.  I think your earlier patch rounded and
clamped and was fast too ...

[mtklein, 2015-09-01 13:19:32]

On 2015/09/01 13:09:31, noel gordon wrote:
> On 2015/09/01 12:50:00, noel gordon wrote:
> > On 2015/09/01 12:45:47, mtklein wrote:
> >  
> > > Yes, round() rounded and clamped. 
> https://codereview.chromium.org/1312053004
> 
> Ok looked. That CL truncates and clamps, which is not quite that same as
> round(), so a change in behavior.  I think your earlier patch rounded and
> clamped and was fast too ...

I was about to say, it's about 2% faster to not round, but I've just seen a way
to incorporate it for ~free.  Another CL incoming!

As you might imagine, not rounding did cause some pixels to be 1 component
darker than they used to be with rounding.

[Noel Gordon, 2015-09-01 13:57:42]

On 2015/09/01 13:19:32, mtklein wrote:
> On 2015/09/01 13:09:31, noel gordon wrote:
> > On 2015/09/01 12:50:00, noel gordon wrote:
> > > On 2015/09/01 12:45:47, mtklein wrote:
> > >  
> > > > Yes, round() rounded and clamped. 
> > https://codereview.chromium.org/1312053004
> > 
> > Ok looked. That CL truncates and clamps, which is not quite that same as
> > round(), so a change in behavior.  I think your earlier patch rounded and
> > clamped and was fast too ...
> 
> I was about to say, it's about 2% faster to not round, but I've just seen a
way
> to incorporate it for ~free.  Another CL incoming!

Fantastic!

> As you might imagine, not rounding did cause some pixels to be 1 component
> darker than they used to be with rounding.

Yes, and chromium users report bugs about off-by-one colors, and we have to
track down where we lost precision.  Precision is important in color correction
[1].

[1] I had to separately measure the colorimetric accuracy of SkColorCubeFilter
using a Munsell color test chart about a month ago since no one ever measured
it.  Long test, but the results are good [2]
and if we maintain precision, I won't have to remeasure (off-by-ones would cause
higher RMS error in that test).

[2] Row 15, http://bit.ly/1Gkadze

[Noel Gordon, 2015-09-01 14:00:45]

Note http://bit.ly/1Gkadze row 15 measured the GPU case, so if the software
SkColorCubeFilter case matches the GPU, then ...