Fix matrix similarity test on arm64
Addresses precision issue by using a simpler test.
Also fixes issues with IvMatrix::preservesRightAngles, and adds unit tests.
BUG=skia:2405
R=robertphillips@google.com, egdaniel@google.com, djsollen@google.com, reed@google.com
Author: jvanverth@google.com
Review URL: https://codereview.chromium.org/520123002
diff --git a/include/core/SkMatrix.h b/include/core/SkMatrix.h
index bfa03de..d2fb737 100644
--- a/include/core/SkMatrix.h
+++ b/include/core/SkMatrix.h
@@ -81,12 +81,12 @@
kPerspective_Mask);
}
- /** Returns true if the matrix contains only translation, rotation or uniform scale
+ /** Returns true if the matrix contains only translation, rotation/reflection or uniform scale
Returns false if other transformation types are included or is degenerate
*/
bool isSimilarity(SkScalar tol = SK_ScalarNearlyZero) const;
- /** Returns true if the matrix contains only translation, rotation or scale
+ /** Returns true if the matrix contains only translation, rotation/reflection or scale
(non-uniform scale is allowed).
Returns false if other transformation types are included or is degenerate
*/
diff --git a/src/core/SkMatrix.cpp b/src/core/SkMatrix.cpp
index 95662fc..814f16a 100644
--- a/src/core/SkMatrix.cpp
+++ b/src/core/SkMatrix.cpp
@@ -176,20 +176,16 @@
return false;
}
- // it has scales and skews, but it could also be rotation, check it out.
- SkVector vec[2];
- vec[0].set(mx, sx);
- vec[1].set(sy, my);
-
- return SkScalarNearlyZero(vec[0].dot(vec[1]), SkScalarSquare(tol)) &&
- SkScalarNearlyEqual(vec[0].lengthSqd(), vec[1].lengthSqd(),
- SkScalarSquare(tol));
+ // upper 2x2 is rotation/reflection + uniform scale if basis vectors
+ // are 90 degree rotations of each other
+ return (SkScalarNearlyEqual(mx, my, tol) && SkScalarNearlyEqual(sx, -sy, tol))
+ || (SkScalarNearlyEqual(mx, -my, tol) && SkScalarNearlyEqual(sx, sy, tol));
}
bool SkMatrix::preservesRightAngles(SkScalar tol) const {
TypeMask mask = this->getType();
- if (mask <= (SkMatrix::kTranslate_Mask | SkMatrix::kScale_Mask)) {
+ if (mask <= kTranslate_Mask) {
// identity, translate and/or scale
return true;
}
@@ -197,7 +193,7 @@
return false;
}
- SkASSERT(mask & kAffine_Mask);
+ SkASSERT(mask & (kAffine_Mask | kScale_Mask));
SkScalar mx = fMat[kMScaleX];
SkScalar my = fMat[kMScaleY];
@@ -208,14 +204,12 @@
return false;
}
- // it has scales and skews, but it could also be rotation, check it out.
+ // upper 2x2 is scale + rotation/reflection if basis vectors are orthogonal
SkVector vec[2];
- vec[0].set(mx, sx);
- vec[1].set(sy, my);
+ vec[0].set(mx, sy);
+ vec[1].set(sx, my);
- return SkScalarNearlyZero(vec[0].dot(vec[1]), SkScalarSquare(tol)) &&
- SkScalarNearlyEqual(vec[0].lengthSqd(), vec[1].lengthSqd(),
- SkScalarSquare(tol));
+ return SkScalarNearlyZero(vec[0].dot(vec[1]), SkScalarSquare(tol));
}
///////////////////////////////////////////////////////////////////////////////
diff --git a/tests/MatrixTest.cpp b/tests/MatrixTest.cpp
index cbec021..fc7ac42 100644
--- a/tests/MatrixTest.cpp
+++ b/tests/MatrixTest.cpp
@@ -245,88 +245,96 @@
}
}
-static void test_matrix_is_similarity(skiatest::Reporter* reporter) {
+static void test_matrix_preserve_shape(skiatest::Reporter* reporter) {
SkMatrix mat;
// identity
mat.setIdentity();
REPORTER_ASSERT(reporter, mat.isSimilarity());
+ REPORTER_ASSERT(reporter, mat.preservesRightAngles());
// translation only
mat.reset();
mat.setTranslate(SkIntToScalar(100), SkIntToScalar(100));
REPORTER_ASSERT(reporter, mat.isSimilarity());
+ REPORTER_ASSERT(reporter, mat.preservesRightAngles());
// scale with same size
mat.reset();
mat.setScale(SkIntToScalar(15), SkIntToScalar(15));
REPORTER_ASSERT(reporter, mat.isSimilarity());
+ REPORTER_ASSERT(reporter, mat.preservesRightAngles());
// scale with one negative
mat.reset();
mat.setScale(SkIntToScalar(-15), SkIntToScalar(15));
REPORTER_ASSERT(reporter, mat.isSimilarity());
+ REPORTER_ASSERT(reporter, mat.preservesRightAngles());
// scale with different size
mat.reset();
mat.setScale(SkIntToScalar(15), SkIntToScalar(20));
REPORTER_ASSERT(reporter, !mat.isSimilarity());
+ REPORTER_ASSERT(reporter, mat.preservesRightAngles());
// scale with same size at a pivot point
mat.reset();
mat.setScale(SkIntToScalar(15), SkIntToScalar(15),
SkIntToScalar(2), SkIntToScalar(2));
REPORTER_ASSERT(reporter, mat.isSimilarity());
+ REPORTER_ASSERT(reporter, mat.preservesRightAngles());
// scale with different size at a pivot point
mat.reset();
mat.setScale(SkIntToScalar(15), SkIntToScalar(20),
SkIntToScalar(2), SkIntToScalar(2));
REPORTER_ASSERT(reporter, !mat.isSimilarity());
+ REPORTER_ASSERT(reporter, mat.preservesRightAngles());
// skew with same size
mat.reset();
mat.setSkew(SkIntToScalar(15), SkIntToScalar(15));
REPORTER_ASSERT(reporter, !mat.isSimilarity());
+ REPORTER_ASSERT(reporter, !mat.preservesRightAngles());
// skew with different size
mat.reset();
mat.setSkew(SkIntToScalar(15), SkIntToScalar(20));
REPORTER_ASSERT(reporter, !mat.isSimilarity());
+ REPORTER_ASSERT(reporter, !mat.preservesRightAngles());
// skew with same size at a pivot point
mat.reset();
mat.setSkew(SkIntToScalar(15), SkIntToScalar(15),
SkIntToScalar(2), SkIntToScalar(2));
REPORTER_ASSERT(reporter, !mat.isSimilarity());
+ REPORTER_ASSERT(reporter, !mat.preservesRightAngles());
// skew with different size at a pivot point
mat.reset();
mat.setSkew(SkIntToScalar(15), SkIntToScalar(20),
SkIntToScalar(2), SkIntToScalar(2));
REPORTER_ASSERT(reporter, !mat.isSimilarity());
+ REPORTER_ASSERT(reporter, !mat.preservesRightAngles());
// perspective x
mat.reset();
mat.setPerspX(SkScalarToPersp(SK_Scalar1 / 2));
REPORTER_ASSERT(reporter, !mat.isSimilarity());
+ REPORTER_ASSERT(reporter, !mat.preservesRightAngles());
// perspective y
mat.reset();
mat.setPerspY(SkScalarToPersp(SK_Scalar1 / 2));
REPORTER_ASSERT(reporter, !mat.isSimilarity());
+ REPORTER_ASSERT(reporter, !mat.preservesRightAngles());
// rotate
for (int angle = 0; angle < 360; ++angle) {
mat.reset();
mat.setRotate(SkIntToScalar(angle));
-#ifndef SK_CPU_ARM64
REPORTER_ASSERT(reporter, mat.isSimilarity());
-#else
- // 64-bit ARM devices built with -O2 and -ffp-contract=fast have a loss
- // of precision and require that we have a higher tolerance
- REPORTER_ASSERT(reporter, mat.isSimilarity(SK_ScalarNearlyZero + 0.00010113f));
-#endif
+ REPORTER_ASSERT(reporter, mat.preservesRightAngles());
}
// see if there are any accumulated precision issues
@@ -335,38 +343,60 @@
mat.postRotate(SkIntToScalar(1));
}
REPORTER_ASSERT(reporter, mat.isSimilarity());
+ REPORTER_ASSERT(reporter, mat.preservesRightAngles());
// rotate + translate
mat.reset();
mat.setRotate(SkIntToScalar(30));
mat.postTranslate(SkIntToScalar(10), SkIntToScalar(20));
REPORTER_ASSERT(reporter, mat.isSimilarity());
+ REPORTER_ASSERT(reporter, mat.preservesRightAngles());
// rotate + uniform scale
mat.reset();
mat.setRotate(SkIntToScalar(30));
mat.postScale(SkIntToScalar(2), SkIntToScalar(2));
REPORTER_ASSERT(reporter, mat.isSimilarity());
+ REPORTER_ASSERT(reporter, mat.preservesRightAngles());
// rotate + non-uniform scale
mat.reset();
mat.setRotate(SkIntToScalar(30));
mat.postScale(SkIntToScalar(3), SkIntToScalar(2));
REPORTER_ASSERT(reporter, !mat.isSimilarity());
+ REPORTER_ASSERT(reporter, !mat.preservesRightAngles());
+
+ // non-uniform scale + rotate
+ mat.reset();
+ mat.setScale(SkIntToScalar(3), SkIntToScalar(2));
+ mat.postRotate(SkIntToScalar(30));
+ REPORTER_ASSERT(reporter, !mat.isSimilarity());
+ REPORTER_ASSERT(reporter, mat.preservesRightAngles());
// all zero
mat.setAll(0, 0, 0, 0, 0, 0, 0, 0, 0);
REPORTER_ASSERT(reporter, !mat.isSimilarity());
+ REPORTER_ASSERT(reporter, !mat.preservesRightAngles());
// all zero except perspective
+ mat.reset();
mat.setAll(0, 0, 0, 0, 0, 0, 0, 0, SK_Scalar1);
REPORTER_ASSERT(reporter, !mat.isSimilarity());
+ REPORTER_ASSERT(reporter, !mat.preservesRightAngles());
- // scales zero, only skews
+ // scales zero, only skews (rotation)
+ mat.setAll(0, SK_Scalar1, 0,
+ -SK_Scalar1, 0, 0,
+ 0, 0, SkMatrix::I()[8]);
+ REPORTER_ASSERT(reporter, mat.isSimilarity());
+ REPORTER_ASSERT(reporter, mat.preservesRightAngles());
+
+ // scales zero, only skews (reflection)
mat.setAll(0, SK_Scalar1, 0,
SK_Scalar1, 0, 0,
0, 0, SkMatrix::I()[8]);
REPORTER_ASSERT(reporter, mat.isSimilarity());
+ REPORTER_ASSERT(reporter, mat.preservesRightAngles());
}
// For test_matrix_decomposition, below.
@@ -815,7 +845,7 @@
REPORTER_ASSERT(reporter, !are_equal(reporter, mat, mat2));
test_matrix_min_max_scale(reporter);
- test_matrix_is_similarity(reporter);
+ test_matrix_preserve_shape(reporter);
test_matrix_recttorect(reporter);
test_matrix_decomposition(reporter);
test_matrix_homogeneous(reporter);
