Sketch SkPMFloat

Sketch SkPMFloat

BUG=skia:

Committed: https://skia.googlesource.com/skia/+/50d2b3114b3e59dc84811881591bf25b2c1ecb9f

CQ_EXTRA_TRYBOTS=client.skia.compile:Build-Ubuntu13.10-GCC4.8-Arm7-Release-Android_Neon-Trybot

http://build.chromium.org/p/client.skia.compile/builders/Build-Ubuntu13.10-GCC4.8-Arm7-Release-Android_Neon/builds/2120/steps/build%20most/logs/stdio

Committed: https://skia.googlesource.com/skia/+/a2f4be76a9d453f1fdfd55b0cec6a683f23ffe0f

[mtklein_C, 2015-02-17 23:52:07]

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

[mtklein_C, 2015-02-17 23:52:07]

There's probably more here than we need in some places, less in others, etc.  I
figure it'd be good to have something to stare at and play with.

[mtklein, 2015-02-18 17:32:01]

The CQ bit was checked by mtklein@google.com

[commit-bot: I haz the power, 2015-02-18 17:32:47]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/936633002/190001

[commit-bot: I haz the power, 2015-02-18 17:32:47]

Note for Reviewers:
The CQ is waiting for an approval.
If you believe that the CL is not ready yet, or if you would like to L-G-T-M
with comments then please uncheck the CQ checkbox.

Waiting for LGTM from valid reviewer(s) till 2015-02-18 23:32 UTC

[commit-bot: I haz the power, 2015-02-18 23:32:54]

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

[commit-bot: I haz the power, 2015-02-18 23:32:55]

No LGTM from a valid reviewer yet. Please ask for an LGTM from a
full Skia committer from
https://skia.googlesource.com/skia/+/master/CQ_COMMITTERS

[mtklein, 2015-02-19 22:10:03]

The CQ bit was checked by mtklein@google.com

[commit-bot: I haz the power, 2015-02-19 22:10:56]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/936633002/250001

[commit-bot: I haz the power, 2015-02-19 22:10:57]

Note for Reviewers:
The CQ is waiting for an approval.
If you believe that the CL is not ready yet, or if you would like to L-G-T-M
with comments then please uncheck the CQ checkbox.

Waiting for LGTM from valid reviewer(s) till 2015-02-20 04:10 UTC

[reed1, 2015-02-19 22:14:50]

lgtm w/ naming questions/suggestions.

https://codereview.chromium.org/936633002/diff/250001/src/core/SkPMFloat.h
File src/core/SkPMFloat.h (right):

https://codereview.chromium.org/936633002/diff/250001/src/core/SkPMFloat.h#ne...
src/core/SkPMFloat.h:33: bool sane() const {
bikeshed: isValid(), isLegal(), isClamped() ?

https://codereview.chromium.org/936633002/diff/250001/src/core/SkPMFloat.h#ne...
src/core/SkPMFloat.h:35: this->r() >= 0 && this->r() <= 1 &&
can replace the 2nd test on r() with r() <= a() and remove the additional check
at the bottom. i.e. only 8 checks are needed total.

[mtklein_C, 2015-02-19 22:47:38]

New patchsets have been uploaded after l-g-t-m from reed@google.com

[mtklein, 2015-02-19 22:48:35]

Also added a new unit test for clamped().

https://codereview.chromium.org/936633002/diff/250001/src/core/SkPMFloat.h
File src/core/SkPMFloat.h (right):

https://codereview.chromium.org/936633002/diff/250001/src/core/SkPMFloat.h#ne...
src/core/SkPMFloat.h:33: bool sane() const {
On 2015/02/19 22:14:49, reed1 wrote:
> bikeshed: isValid(), isLegal(), isClamped() ?

Done.

https://codereview.chromium.org/936633002/diff/250001/src/core/SkPMFloat.h#ne...
src/core/SkPMFloat.h:35: this->r() >= 0 && this->r() <= 1 &&
On 2015/02/19 22:14:50, reed1 wrote:
> can replace the 2nd test on r() with r() <= a() and remove the additional
check
> at the bottom. i.e. only 8 checks are needed total.

Oh, nice.  Done.

[mtklein, 2015-02-19 22:51:12]

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

[mtklein, 2015-02-19 22:51:13]

+Matt, who I was talking about this a bit just now.  Want to take a look to see
if anything looks super silly in src/opts?

[mtklein, 2015-02-19 22:53:09]

The CQ bit was checked by mtklein@google.com

[commit-bot: I haz the power, 2015-02-19 22:53:27]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/936633002/270001

[mtklein, 2015-02-19 22:53:43]

The CQ bit was unchecked by mtklein@google.com

[mtklein, 2015-02-23 17:38:39]

The CQ bit was checked by mtklein@google.com

[commit-bot: I haz the power, 2015-02-23 17:39:19]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/936633002/270001

[commit-bot: I haz the power, 2015-02-23 17:39:32]

Committed patchset #15 (id:270001) as
https://skia.googlesource.com/skia/+/50d2b3114b3e59dc84811881591bf25b2c1ecb9f

[mtklein, 2015-02-23 17:42:39]

A revert of this CL (patchset #15 id:270001) has been created in
https://codereview.chromium.org/952453004/ by mtklein@google.com.

The reason for reverting is:
http://build.chromium.org/p/client.skia.compile/builders/Build-Ubuntu13.10-GC....

[mtklein, 2015-02-23 17:56:23]

The CQ bit was checked by mtklein@google.com

[commit-bot: I haz the power, 2015-02-23 17:57:21]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/936633002/290001

[commit-bot: I haz the power, 2015-02-23 18:04:38]

Committed patchset #16 (id:290001) as
https://skia.googlesource.com/skia/+/a2f4be76a9d453f1fdfd55b0cec6a683f23ffe0f