AuroraMiddleware/skia2
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Fix infinite matrix during an invert.

Browse Source 
This patch ensures that when inverting a SkMatrix44, we handle small
floats properly. When inverted these can cause infinite values, but
still evaluate to true in an if condition.

BUG=chromium:498516

Review URL: https://codereview.chromium.org/1209763002
This commit is contained in:
vmpstr 2015-06-30 13:36:04 -07:00 committed by Commit bot
parent 7f6283bdf8
commit a8d4559fd6
3 changed files with 84 additions and 34 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
include/utils/SkMatrix44.h
View File

@ -358,7 +358,8 @@ public:
}
/** If this is invertible, return that in inverse and return true. If it is
not invertible, return false and ignore the inverse parameter.
not invertible, return false and leave the inverse parameter in an
unspecified state.
*/
bool invert(SkMatrix44* inverse) const;

90
src/utils/SkMatrix44.cpp
View File

@ -448,54 +448,71 @@ double SkMatrix44::determinant() const {
///////////////////////////////////////////////////////////////////////////////
bool SkMatrix44::invert(SkMatrix44* inverse) const {
static bool is_matrix_finite(const SkMatrix44& matrix) {
SkMScalar accumulator = 0;
for (int row = 0; row < 4; ++row) {
for (int col = 0; col < 4; ++col) {
accumulator *= matrix.get(row, col);
}
}
return accumulator == 0;
}
bool SkMatrix44::invert(SkMatrix44* storage) const {
if (this->isIdentity()) {
if (inverse) {
inverse->setIdentity();
if (storage) {
storage->setIdentity();
}
return true;
}
if (this->isTranslate()) {
if (inverse) {
inverse->setTranslate(-fMat[3][0], -fMat[3][1], -fMat[3][2]);
if (storage) {
storage->setTranslate(-fMat[3][0], -fMat[3][1], -fMat[3][2]);
}
return true;
}
SkMatrix44 tmp(kUninitialized_Constructor);
// Use storage if it's available and distinct from this matrix.
SkMatrix44* inverse = (storage && storage != this) ? storage : &tmp;
if (this->isScaleTranslate()) {
if (0 == fMat[0][0] * fMat[1][1] * fMat[2][2]) {
return false;
}
if (inverse) {
double invXScale = 1 / fMat[0][0];
double invYScale = 1 / fMat[1][1];
double invZScale = 1 / fMat[2][2];
double invXScale = 1 / fMat[0][0];
double invYScale = 1 / fMat[1][1];
double invZScale = 1 / fMat[2][2];
inverse->fMat[0][0] = SkDoubleToMScalar(invXScale);
inverse->fMat[0][1] = 0;
inverse->fMat[0][2] = 0;
inverse->fMat[0][3] = 0;
inverse->fMat[0][0] = SkDoubleToMScalar(invXScale);
inverse->fMat[0][1] = 0;
inverse->fMat[0][2] = 0;
inverse->fMat[0][3] = 0;
inverse->fMat[1][0] = 0;
inverse->fMat[1][1] = SkDoubleToMScalar(invYScale);
inverse->fMat[1][2] = 0;
inverse->fMat[1][3] = 0;
inverse->fMat[1][0] = 0;
inverse->fMat[1][1] = SkDoubleToMScalar(invYScale);
inverse->fMat[1][2] = 0;
inverse->fMat[1][3] = 0;
inverse->fMat[2][0] = 0;
inverse->fMat[2][1] = 0;
inverse->fMat[2][2] = SkDoubleToMScalar(invZScale);
inverse->fMat[2][3] = 0;
inverse->fMat[2][0] = 0;
inverse->fMat[2][1] = 0;
inverse->fMat[2][2] = SkDoubleToMScalar(invZScale);
inverse->fMat[2][3] = 0;
inverse->fMat[3][0] = SkDoubleToMScalar(-fMat[3][0] * invXScale);
inverse->fMat[3][1] = SkDoubleToMScalar(-fMat[3][1] * invYScale);
inverse->fMat[3][2] = SkDoubleToMScalar(-fMat[3][2] * invZScale);
inverse->fMat[3][3] = 1;
inverse->fMat[3][0] = SkDoubleToMScalar(-fMat[3][0] * invXScale);
inverse->fMat[3][1] = SkDoubleToMScalar(-fMat[3][1] * invYScale);
inverse->fMat[3][2] = SkDoubleToMScalar(-fMat[3][2] * invZScale);
inverse->fMat[3][3] = 1;
inverse->setTypeMask(this->getType());
inverse->setTypeMask(this->getType());
if (!is_matrix_finite(*inverse)) {
return false;
}
if (storage && inverse != storage) {
*storage = *inverse;
}
return true;
}
@ -547,9 +564,6 @@ bool SkMatrix44::invert(SkMatrix44* inverse) const {
if (!sk_float_isfinite(invdet)) {
return false;
}
if (NULL == inverse) {
return true;
}
b00 *= invdet;
b01 *= invdet;
@ -579,6 +593,12 @@ bool SkMatrix44::invert(SkMatrix44* inverse) const {
inverse->fMat[3][3] = 1;
inverse->setTypeMask(this->getType());
if (!is_matrix_finite(*inverse)) {
return false;
}
if (storage && inverse != storage) {
*storage = *inverse;
}
return true;
}
@ -605,9 +625,6 @@ bool SkMatrix44::invert(SkMatrix44* inverse) const {
if (!sk_float_isfinite(invdet)) {
return false;
}
if (NULL == inverse) {
return true;
}
b00 *= invdet;
b01 *= invdet;
@ -640,6 +657,13 @@ bool SkMatrix44::invert(SkMatrix44* inverse) const {
inverse->fMat[3][3] = SkDoubleToMScalar(a20 * b03 - a21 * b01 + a22 * b00);
inverse->dirtyTypeMask();
inverse->setTypeMask(this->getType());
if (!is_matrix_finite(*inverse)) {
return false;
}
if (storage && inverse != storage) {
*storage = *inverse;
}
return true;
}

25
tests/Matrix44Test.cpp
View File

@ -426,6 +426,31 @@ static void test_invert(skiatest::Reporter* reporter) {
0, 0, -1, 1};
expected.setRowMajord(expectedInverseAffineAndPerspective);
REPORTER_ASSERT(reporter, nearly_equal(expected, inverse));
SkMatrix44 tinyScale(SkMatrix44::kIdentity_Constructor);
tinyScale.setDouble(0, 0, 1e-39);
REPORTER_ASSERT(reporter, tinyScale.getType() == SkMatrix44::kScale_Mask);
REPORTER_ASSERT(reporter, !tinyScale.invert(NULL));
REPORTER_ASSERT(reporter, !tinyScale.invert(&inverse));
SkMatrix44 tinyScaleTranslate(SkMatrix44::kIdentity_Constructor);
tinyScaleTranslate.setDouble(0, 0, 1e-38);
REPORTER_ASSERT(reporter, tinyScaleTranslate.invert(NULL));
tinyScaleTranslate.setDouble(0, 3, 10);
REPORTER_ASSERT(
reporter, tinyScaleTranslate.getType() ==
(SkMatrix44::kScale_Mask | SkMatrix44::kTranslate_Mask));
REPORTER_ASSERT(reporter, !tinyScaleTranslate.invert(NULL));
REPORTER_ASSERT(reporter, !tinyScaleTranslate.invert(&inverse));
SkMatrix44 tinyScalePerspective(SkMatrix44::kIdentity_Constructor);
tinyScalePerspective.setDouble(0, 0, 1e-39);
tinyScalePerspective.setDouble(3, 2, -1);
REPORTER_ASSERT(reporter, (tinyScalePerspective.getType() &
SkMatrix44::kPerspective_Mask) ==
SkMatrix44::kPerspective_Mask);
REPORTER_ASSERT(reporter, !tinyScalePerspective.invert(NULL));
REPORTER_ASSERT(reporter, !tinyScalePerspective.invert(&inverse));
}
static void test_transpose(skiatest::Reporter* reporter) {