Add perspective support to SkMatrix44 initializers.

I noticed SkMatrix <-> SkMatrix44 conversions were dropping the
perspective values on the floor.  As we use SkMatrix44 heavily in
Chromium, I'm concerned this missing code will cause a bug eventually.
It should be correct to simply use the bottom row of the 4x4 matrix
excluding the third column.

BUG=
R=reed@google.com

Author: aelias@chromium.org

Review URL: https://codereview.chromium.org/25484006

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

3 files changed, 62 insertions, 8 deletions

diff --git a/include/utils/SkMatrix44.h b/include/utils/SkMatrix44.h
index 6e85bb56f4..9282770588 100644
--- a/include/utils/SkMatrix44.h
+++ b/include/utils/SkMatrix44.h
@@ -137,6 +137,14 @@ public:
         return !(other == *this);
     }
 
+    /* When converting from SkMatrix44 to SkMatrix, the third row and
+     * column is dropped.  When converting from SkMatrix to SkMatrix44
+     * the third row and column remain as identity:
+     * [ a b c ]      [ a b 0 c ]
+     * [ d e f ]  ->  [ d e 0 f ]
+     * [ g h i ]      [ 0 0 1 0 ]
+     *                [ g h 0 i ]
+     */
     SkMatrix44(const SkMatrix&);
     SkMatrix44& operator=(const SkMatrix& src);
     operator SkMatrix() const;
@@ -259,6 +267,8 @@ public:
     void setRowMajor(const SkMScalar data[]) { this->setRowMajord(data); }
 #endif
 
+    /* This sets the top-left of the matrix and clears the translation and
+     * perspective components (with [3][3] set to 1). */
     void set3x3(SkMScalar m00, SkMScalar m01, SkMScalar m02,
                 SkMScalar m10, SkMScalar m11, SkMScalar m12,
                 SkMScalar m20, SkMScalar m21, SkMScalar m22);
diff --git a/src/utils/SkMatrix44.cpp b/src/utils/SkMatrix44.cpp
index 1e48f393ac..9803b52095 100644
--- a/src/utils/SkMatrix44.cpp
+++ b/src/utils/SkMatrix44.cpp
@@ -902,8 +902,6 @@ void SkMatrix44::dump() const {
 
 ///////////////////////////////////////////////////////////////////////////////
 
-// TODO: make this support src' perspective elements
-//
 static void initFromMatrix(SkMScalar dst[4][4], const SkMatrix& src) {
     dst[0][0] = SkScalarToMScalar(src[SkMatrix::kMScaleX]);
     dst[1][0] = SkScalarToMScalar(src[SkMatrix::kMSkewX]);
@@ -917,10 +915,10 @@ static void initFromMatrix(SkMScalar dst[4][4], const SkMatrix& src) {
     dst[1][2] = 0;
     dst[2][2] = 1;
     dst[3][2] = 0;
-    dst[0][3] = 0;
-    dst[1][3] = 0;
+    dst[0][3] = SkScalarToMScalar(src[SkMatrix::kMPersp0]);
+    dst[1][3] = SkScalarToMScalar(src[SkMatrix::kMPersp1]);
     dst[2][3] = 0;
-    dst[3][3] = 1;
+    dst[3][3] = SkScalarToMScalar(src[SkMatrix::kMPersp2]);
 }
 
 SkMatrix44::SkMatrix44(const SkMatrix& src) {
@@ -938,11 +936,8 @@ SkMatrix44& SkMatrix44::operator=(const SkMatrix& src) {
     return *this;
 }
 
-// TODO: make this support our perspective elements
-//
 SkMatrix44::operator SkMatrix() const {
     SkMatrix dst;
-    dst.reset();    // setup our perspective correctly for identity
 
     dst[SkMatrix::kMScaleX]  = SkMScalarToScalar(fMat[0][0]);
     dst[SkMatrix::kMSkewX]  = SkMScalarToScalar(fMat[1][0]);
@@ -952,5 +947,9 @@ SkMatrix44::operator SkMatrix() const {
     dst[SkMatrix::kMScaleY] = SkMScalarToScalar(fMat[1][1]);
     dst[SkMatrix::kMTransY] = SkMScalarToScalar(fMat[3][1]);
 
+    dst[SkMatrix::kMPersp0] = SkMScalarToScalar(fMat[0][3]);
+    dst[SkMatrix::kMPersp1] = SkMScalarToScalar(fMat[1][3]);
+    dst[SkMatrix::kMPersp2] = SkMScalarToScalar(fMat[3][3]);
+
     return dst;
 }
diff --git a/tests/Matrix44Test.cpp b/tests/Matrix44Test.cpp
index 67af60df90..001d8a695f 100644
--- a/tests/Matrix44Test.cpp
+++ b/tests/Matrix44Test.cpp
@@ -471,6 +471,50 @@ static void test_set_row_col_major(skiatest::Reporter* reporter) {
     REPORTER_ASSERT(reporter, nearly_equal(a, b));
 }
 
+static void test_3x3_conversion(skiatest::Reporter* reporter) {
+    SkMScalar values4x4[16] = { 1, 2, 3, 4,
+                                5, 6, 7, 8,
+                                9, 10, 11, 12,
+                                13, 14, 15, 16 };
+    SkMScalar values3x3[9] = { 1, 2, 4,
+                               5, 6, 8,
+                               13, 14, 16 };
+    SkMScalar values4x4flattened[16] = { 1, 2, 0, 4,
+                                         5, 6, 0, 8,
+                                         0, 0, 1, 0,
+                                         13, 14, 0, 16 };
+    SkMatrix44 a44;
+    a44.setRowMajor(values4x4);
+
+    SkMatrix a33 = a44;
+    SkMatrix expected33;
+    for (int i = 0; i < 9; i++) expected33[i] = values3x3[i];
+    REPORTER_ASSERT(reporter, expected33 == a33);
+
+    SkMatrix44 a44flattened = a33;
+    SkMatrix44 expected44flattened;
+    expected44flattened.setRowMajor(values4x4flattened);
+    REPORTER_ASSERT(reporter, nearly_equal(a44flattened, expected44flattened));
+
+    // Test that a point with a Z value of 0 is transformed the same way.
+    SkScalar vec4[4] = { 2, 4, 0, 8 };
+    SkScalar vec3[3] = { 2, 4, 8 };
+
+    SkScalar vec4transformed[4];
+    SkScalar vec3transformed[3];
+    SkScalar vec4transformed2[4];
+    a44.mapScalars(vec4, vec4transformed);
+    a33.mapHomogeneousPoints(vec3transformed, vec3, 1);
+    a44flattened.mapScalars(vec4, vec4transformed2);
+    REPORTER_ASSERT(reporter, vec4transformed[0] == vec3transformed[0]);
+    REPORTER_ASSERT(reporter, vec4transformed[1] == vec3transformed[1]);
+    REPORTER_ASSERT(reporter, vec4transformed[3] == vec3transformed[2]);
+    REPORTER_ASSERT(reporter, vec4transformed[0] == vec4transformed2[0]);
+    REPORTER_ASSERT(reporter, vec4transformed[1] == vec4transformed2[1]);
+    REPORTER_ASSERT(reporter, vec4transformed[2] != vec4transformed2[2]);
+    REPORTER_ASSERT(reporter, vec4transformed[3] == vec4transformed2[3]);
+}
+
 static void TestMatrix44(skiatest::Reporter* reporter) {
     SkMatrix44 mat, inverse, iden1, iden2, rot;
 
@@ -572,6 +616,7 @@ static void TestMatrix44(skiatest::Reporter* reporter) {
     test_translate(reporter);
     test_scale(reporter);
     test_map2(reporter);
+    test_3x3_conversion(reporter);
 }
 
 #include "TestClassDef.h"