From dd2fde5ae059cfc30c98599ac16102c31818f490 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Fri, 8 May 2015 11:58:04 +0200 Subject: [PATCH 1/2] Math: improve QuaternionTest to verify all quaternion-from-matrix cases. The test fails. I was able to craft inputs so that all cases were passing even with the obviously wrong algorithm. Huh. --- src/Magnum/Math/Test/QuaternionTest.cpp | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/src/Magnum/Math/Test/QuaternionTest.cpp b/src/Magnum/Math/Test/QuaternionTest.cpp index 3da43b1c7..b9aba3231 100644 --- a/src/Magnum/Math/Test/QuaternionTest.cpp +++ b/src/Magnum/Math/Test/QuaternionTest.cpp @@ -276,7 +276,7 @@ void QuaternionTest::angle() { } void QuaternionTest::matrix() { - Vector3 axis = Vector3(1.0f, -3.0f, 5.0f).normalized(); + Vector3 axis = Vector3(-3.0f, 1.0f, 5.0f).normalized(); Quaternion q = Quaternion::rotation(Deg(37.0f), axis); Matrix3x3 m = Matrix4::rotation(Deg(37.0f), axis).rotationScaling(); @@ -294,11 +294,28 @@ void QuaternionTest::matrix() { CORRADE_VERIFY(m.trace() > 0.0f); CORRADE_COMPARE(Quaternion::fromMatrix(m), q); - /* Trace < 0 */ + /* Trace < 0, max is diagonal[2] */ Matrix3x3 m2 = Matrix4::rotation(Deg(130.0f), axis).rotationScaling(); Quaternion q2 = Quaternion::rotation(Deg(130.0f), axis); CORRADE_VERIFY(m2.trace() < 0.0f); + CORRADE_VERIFY(m2.diagonal()[2] > std::max(m2.diagonal()[0], m2.diagonal()[1])); CORRADE_COMPARE(Quaternion::fromMatrix(m2), q2); + + /* Trace < 0, max is diagonal[1] */ + Vector3 axis2 = Vector3(-3.0f, 5.0f, 1.0f).normalized(); + Matrix3x3 m3 = Matrix4::rotation(Deg(130.0f), axis2).rotationScaling(); + Quaternion q3 = Quaternion::rotation(Deg(130.0f), axis2); + CORRADE_VERIFY(m3.trace() < 0.0f); + CORRADE_VERIFY(m3.diagonal()[1] > std::max(m3.diagonal()[0], m3.diagonal()[2])); + CORRADE_COMPARE(Quaternion::fromMatrix(m3), q3); + + /* Trace < 0, max is diagonal[0] */ + Vector3 axis3 = Vector3(5.0f, -3.0f, 1.0f).normalized(); + Matrix3x3 m4 = Matrix4::rotation(Deg(130.0f), axis3).rotationScaling(); + Quaternion q4 = Quaternion::rotation(Deg(130.0f), axis3); + CORRADE_VERIFY(m4.trace() < 0.0f); + CORRADE_VERIFY(m4.diagonal()[0] > std::max(m4.diagonal()[1], m4.diagonal()[2])); + CORRADE_COMPARE(Quaternion::fromMatrix(m4), q4); } void QuaternionTest::lerp() { From b2d7f4ecc7d15882e0c1c260c20c0a811c66cd1e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Fri, 8 May 2015 12:11:52 +0200 Subject: [PATCH 2/2] Revert "Math: use Vector::max() instead of custom ugly solution." This reverts commit 71db38cb2ff4b8e2fcd1d3877aed580cf7a4e846. The test passes again. --- src/Magnum/Math/Quaternion.h | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/Magnum/Math/Quaternion.h b/src/Magnum/Math/Quaternion.h index 3f1e1809a..838e6ecdd 100644 --- a/src/Magnum/Math/Quaternion.h +++ b/src/Magnum/Math/Quaternion.h @@ -543,7 +543,10 @@ template Quaternion quaternionFromMatrix(const Matrix3x3& m) { } /* Diagonal is negative */ - const std::size_t i = diagonal.max(); + std::size_t i = 0; + if(diagonal[1] > diagonal[0]) i = 1; + if(diagonal[2] > diagonal[i]) i = 2; + const std::size_t j = (i + 1) % 3; const std::size_t k = (i + 2) % 3;