From: commit-bot@chromium.org <commit-bot@chromium.org@2bbb7eff-a529-9590-31e7-b0007b416f81>
Date: Tue, 20 Aug 2013 15:25:04 +0000 (+0000)
Subject: Make SkMatrix44::invert() check for finite 1/det instead of magic value
X-Git-Tag: accepted/tizen/5.0/unified/20181102.025319~11190
X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=f02f07835e417a5baff27dc8ef0b0e773ab7be00;p=platform%2Fupstream%2FlibSkiaSharp.git

Make SkMatrix44::invert() check for finite 1/det instead of magic value

Previously we were checking to see if the magnitude of
determinant of the matrix to be inverted was less than
1.0e-8, which is a magic number possibly plucked from
Graphics Gems. After some discussion, it's been determined
(ha) that we can simply check to see if 1/det is finite and
if so proceed.

BUG=222926

Committed: http://code.google.com/p/skia/source/detail?r=10758

R=reed@google.com, shawnsingh@chromium.org

Author: jvanverth@google.com

Review URL: https://chromiumcodereview.appspot.com/22904003

git-svn-id: http://skia.googlecode.com/svn/trunk@10818 2bbb7eff-a529-9590-31e7-b0007b416f81
---

diff --git a/src/utils/SkMatrix44.cpp b/src/utils/SkMatrix44.cpp
index 92c8715..9ceecbd 100644
--- a/src/utils/SkMatrix44.cpp
+++ b/src/utils/SkMatrix44.cpp
@@ -529,13 +529,16 @@ bool SkMatrix44::invert(SkMatrix44* inverse) const {
     // Calculate the determinant
     double det = b00 * b11 - b01 * b10 + b02 * b09 + b03 * b08 - b04 * b07 + b05 * b06;
 
-    if (dabs(det) < TOO_SMALL_FOR_DETERMINANT) {
+    double invdet = 1.0 / det;
+    // If det is zero, we want to return false. However, we also want to return false
+    // if 1/det overflows to infinity (i.e. det is denormalized). Both of these are
+    // handled by checking that 1/det is finite.
+    if (!sk_float_isfinite(invdet)) {
         return false;
     }
     if (NULL == inverse) {
         return true;
     }
-    double invdet = 1.0 / det;
 
     b00 *= invdet;
     b01 *= invdet;
@@ -568,7 +571,6 @@ bool SkMatrix44::invert(SkMatrix44* inverse) const {
     inverse->fMat[3][3] = SkDoubleToMScalar(a20 * b03 - a21 * b01 + a22 * b00);
     inverse->dirtyTypeMask();
 
-    inverse->dirtyTypeMask();
     return true;
 }
 
diff --git a/tests/Matrix44Test.cpp b/tests/Matrix44Test.cpp
index b2c7071..7e31840 100644
--- a/tests/Matrix44Test.cpp
+++ b/tests/Matrix44Test.cpp
@@ -403,6 +403,33 @@ static void TestMatrix44(skiatest::Reporter* reporter) {
     iden2.setConcat(inverse, mat);
     REPORTER_ASSERT(reporter, is_identity(iden2));
 
+    // test tiny-valued matrix inverse
+    mat.reset();
+    mat.setScale(1.0e-12, 1.0e-12, 1.0e-12);
+    rot.setRotateDegreesAbout(0, 0, -1, 90);
+    mat.postConcat(rot);
+    mat.postTranslate(1.0e-12, 1.0e-12, 1.0e-12);
+    REPORTER_ASSERT(reporter, mat.invert(NULL));
+    mat.invert(&inverse);
+    iden1.setConcat(mat, inverse);
+    REPORTER_ASSERT(reporter, is_identity(iden1));
+    
+    // test mixed-valued matrix inverse
+    mat.reset();
+    mat.setScale(1.0e-11, 3.0, 1.0e+11);
+    rot.setRotateDegreesAbout(0, 0, -1, 90);
+    mat.postConcat(rot);
+    mat.postTranslate(1.0e+11, 3.0, 1.0e-11);
+    REPORTER_ASSERT(reporter, mat.invert(NULL));
+    mat.invert(&inverse);
+    iden1.setConcat(mat, inverse);
+    REPORTER_ASSERT(reporter, is_identity(iden1));
+    
+    // test degenerate matrix
+    mat.reset();
+    mat.set3x3(1.0, 1.0, 0.0, 1.0, 0.0, 0.0, 0.0, 1.0, 0.0);
+    REPORTER_ASSERT(reporter, !mat.invert(NULL));
+    
     // test rol/col Major getters
     {
         mat.setTranslate(2, 3, 4);