Fix the assert and clamping in SkMatrix::get*Scale[s]().

Fix the assert and clamping in SkMatrix::get*Scale[s]().

Follow-up after https://codereview.chromium.org/2143133005/.

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

Committed: https://skia.googlesource.com/skia/+/718aab1e3f02f57f79072f70906acab866ace155

[Krzysztof Olczyk, 2016-07-18 06:39:23]

Description was changed from

==========
Fix the assert and clamping in SkMatrix::get*Scale[s]().

Follow-up after https://codereview.chromium.org/2143133005/.

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

to

==========
Fix the assert and clamping in SkMatrix::get*Scale[s]().

Follow-up after https://codereview.chromium.org/2143133005/.

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

[Krzysztof Olczyk, 2016-07-18 06:40:55]

kolczyk@opera.com changed reviewers:
+ bsalomon@google.com, jvanverth@google.com, reed@google.com

[Krzysztof Olczyk, 2016-07-18 06:40:56]

Hi, 

this is a fixup to fix the assert + clamping to zero as discussed in
https://codereview.chromium.org/2143133005/

PTAL

[Krzysztof Olczyk, 2016-07-18 06:41:39]

The CQ bit was checked by kolczyk@opera.com

[commit-bot: I haz the power, 2016-07-18 06:41:42]

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

[Krzysztof Olczyk, 2016-07-18 06:41:49]

The CQ bit was unchecked by kolczyk@opera.com

[reed1, 2016-07-18 14:10:06]

https://codereview.chromium.org/2154123003/diff/1/src/core/SkMatrix.cpp
File src/core/SkMatrix.cpp (right):

https://codereview.chromium.org/2154123003/diff/1/src/core/SkMatrix.cpp#newco...
src/core/SkMatrix.cpp:1574: SkASSERT(results[0] >= -SK_ScalarNearlyZero);
Thanks, this is better. Lets add a comment why we're checking for -nearly.
Something about numerical stability (esp. if you can reference where in the
above math the slight error might come from.)

Speaking of which, can you add the assert right after the questionable line?
e.g. right after the dot-prod or whereever?

[Krzysztof Olczyk, 2016-07-19 06:24:59]

On 2016/07/18 at 14:10:06, reed wrote:
> https://codereview.chromium.org/2154123003/diff/1/src/core/SkMatrix.cpp
> File src/core/SkMatrix.cpp (right):
> 
>
https://codereview.chromium.org/2154123003/diff/1/src/core/SkMatrix.cpp#newco...
> src/core/SkMatrix.cpp:1574: SkASSERT(results[0] >= -SK_ScalarNearlyZero);
> Thanks, this is better. Lets add a comment why we're checking for -nearly.
Something about numerical stability (esp. if you can reference where in the
above math the slight error might come from.)

Yes, I think it comes from dot products. There had already been a check against
nearly zeros in line 1546.

> Speaking of which, can you add the assert right after the questionable line?
e.g. right after the dot-prod or whereever?

It's not possible, I'm afraid. While the float precision error can indeed come
from dot products, we can't assert anything earlier as we can't say much about
the value in the middle of the mathematical formula.
We only know that the final result, at the end of the formula, we should get
some non-negative value. However the earlier precision errors, further added,
multiplied, square-rooted can give as a nearly-zero.

[Krzysztof Olczyk, 2016-07-19 07:16:10]

Description was changed from

==========
Fix the assert and clamping in SkMatrix::get*Scale[s]().

Follow-up after https://codereview.chromium.org/2143133005/.

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

to

==========
Fix the assert and clamping in SkMatrix::get*Scale[s]().

Follow-up after https://codereview.chromium.org/2143133005/.

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

[reed1, 2016-07-19 10:31:57]

lgtm

[Krzysztof Olczyk, 2016-07-19 10:34:16]

The CQ bit was checked by kolczyk@opera.com

[commit-bot: I haz the power, 2016-07-19 10:34:22]

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-19 10:49:23]

Description was changed from

==========
Fix the assert and clamping in SkMatrix::get*Scale[s]().

Follow-up after https://codereview.chromium.org/2143133005/.

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

to

==========
Fix the assert and clamping in SkMatrix::get*Scale[s]().

Follow-up after https://codereview.chromium.org/2143133005/.

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

Committed:
https://skia.googlesource.com/skia/+/718aab1e3f02f57f79072f70906acab866ace155
==========

[commit-bot: I haz the power, 2016-07-19 10:49:24]

Committed patchset #2 (id:20001) as
https://skia.googlesource.com/skia/+/718aab1e3f02f57f79072f70906acab866ace155