From 722555bebbe9128783b8dbe0e897c09c9ccb88ce Mon Sep 17 00:00:00 2001 From: "commit-bot@chromium.org" Date: Sat, 5 Oct 2013 01:16:30 +0000 Subject: [PATCH] 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. Previously committed and reverted, second attempt with fix for incorrect use of SkMScalar/SkScalar. BUG= R=reed@google.com, caryclark@google.com Author: aelias@chromium.org Review URL: https://codereview.chromium.org/25484006 git-svn-id: http://skia.googlecode.com/svn/trunk@11624 2bbb7eff-a529-9590-31e7-b0007b416f81 --- include/utils/SkMatrix44.h | 10 +++++++ src/utils/SkMatrix44.cpp | 15 +++++----- tests/Matrix44Test.cpp | 57 ++++++++++++++++++++++++++++++++++++-- 3 files changed, 71 insertions(+), 11 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..849f355489 100644 --- a/tests/Matrix44Test.cpp +++ b/tests/Matrix44Test.cpp @@ -16,7 +16,13 @@ static bool nearly_equal_double(double a, double b) { return diff <= tolerance; } -static bool nearly_equal_scalar(SkMScalar a, SkMScalar b) { +static bool nearly_equal_mscalar(SkMScalar a, SkMScalar b) { + const SkMScalar tolerance = SK_MScalar1 / 200000; + + return SkTAbs(a - b) <= tolerance; +} + +static bool nearly_equal_scalar(SkScalar a, SkScalar b) { // Note that we get more compounded error for multiple operations when // SK_SCALAR_IS_FIXED. #ifdef SK_SCALAR_IS_FLOAT @@ -25,7 +31,7 @@ static bool nearly_equal_scalar(SkMScalar a, SkMScalar b) { const SkScalar tolerance = SK_Scalar1 / 1024; #endif - return SkTAbs(a - b) <= tolerance; + return SkScalarAbs(a - b) <= tolerance; } template void assert16(skiatest::Reporter* reporter, const T data[], @@ -57,7 +63,7 @@ template void assert16(skiatest::Reporter* reporter, const T data[] static bool nearly_equal(const SkMatrix44& a, const SkMatrix44& b) { for (int i = 0; i < 4; ++i) { for (int j = 0; j < 4; ++j) { - if (!nearly_equal_scalar(a.get(i, j), b.get(i, j))) { + if (!nearly_equal_mscalar(a.get(i, j), b.get(i, j))) { printf("not equal %g %g\n", a.get(i, j), b.get(i, j)); return false; } @@ -471,6 +477,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 }; + SkScalar 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, nearly_equal_scalar(vec4transformed[0], vec3transformed[0])); + REPORTER_ASSERT(reporter, nearly_equal_scalar(vec4transformed[1], vec3transformed[1])); + REPORTER_ASSERT(reporter, nearly_equal_scalar(vec4transformed[3], vec3transformed[2])); + REPORTER_ASSERT(reporter, nearly_equal_scalar(vec4transformed[0], vec4transformed2[0])); + REPORTER_ASSERT(reporter, nearly_equal_scalar(vec4transformed[1], vec4transformed2[1])); + REPORTER_ASSERT(reporter, !nearly_equal_scalar(vec4transformed[2], vec4transformed2[2])); + REPORTER_ASSERT(reporter, nearly_equal_scalar(vec4transformed[3], vec4transformed2[3])); +} + static void TestMatrix44(skiatest::Reporter* reporter) { SkMatrix44 mat, inverse, iden1, iden2, rot; @@ -572,6 +622,7 @@ static void TestMatrix44(skiatest::Reporter* reporter) { test_translate(reporter); test_scale(reporter); test_map2(reporter); + test_3x3_conversion(reporter); } #include "TestClassDef.h" -- 2.34.1