From f2840e5880ae5314ec4ca3089aac7fdaa679864e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Tue, 20 Aug 2013 21:03:08 +0200 Subject: [PATCH] Math: assert uniform scaling in Matrix[34]::rotation(). Needed to adjust the test cases slightly, because they were firing the assert even if they shouldn't and the "expect fail" case wasn't working at all. I now badly need proper floating-point equality comparison. --- src/Math/Matrix3.h | 6 ++++-- src/Math/Matrix4.h | 7 +++++-- src/Math/Test/Matrix3Test.cpp | 15 ++++++--------- src/Math/Test/Matrix4Test.cpp | 15 ++++++--------- 4 files changed, 21 insertions(+), 22 deletions(-) diff --git a/src/Math/Matrix3.h b/src/Math/Matrix3.h index ab88b85c4..6b7398102 100644 --- a/src/Math/Matrix3.h +++ b/src/Math/Matrix3.h @@ -191,12 +191,14 @@ template class Matrix3: public Matrix<3, T> { /** * @brief 2D rotation part of the matrix * - * Normalized upper-left 2x2 part of the matrix. + * Normalized upper-left 2x2 part of the matrix. Expects uniform + * scaling. * @see rotationNormalized(), rotationScaling(), @ref uniformScaling(), * rotation(T), Matrix4::rotation() const - * @todo assert uniform scaling (otherwise this would be garbage) */ Matrix<2, T> rotation() const { + CORRADE_ASSERT(TypeTraits::equals((*this)[0].xy().dot(), (*this)[1].xy().dot()), + "Math::Matrix3::rotation(): the matrix doesn't have uniform scaling", {}); return {(*this)[0].xy().normalized(), (*this)[1].xy().normalized()}; } diff --git a/src/Math/Matrix4.h b/src/Math/Matrix4.h index a082f3bbd..48f5c84db 100644 --- a/src/Math/Matrix4.h +++ b/src/Math/Matrix4.h @@ -258,11 +258,11 @@ template class Matrix4: public Matrix<4, T> { /** * @brief 3D rotation part of the matrix * - * Normalized upper-left 3x3 part of the matrix. + * Normalized upper-left 3x3 part of the matrix. Expects uniform + * scaling. * @see rotationNormalized(), rotationScaling() const, * @ref uniformScaling(), rotation(T, const Vector3&), * Matrix3::rotation() const - * @todo assert uniform scaling (otherwise this would be garbage) */ /* Not Matrix3, because it is for affine 2D transformations */ Matrix<3, T> rotation() const; @@ -452,6 +452,9 @@ template Matrix4 Matrix4::perspectiveProjection(const Vector2& } template inline Matrix<3, T> Matrix4::rotation() const { + CORRADE_ASSERT(TypeTraits::equals((*this)[0].xyz().dot(), (*this)[1].xyz().dot()) && + TypeTraits::equals((*this)[1].xyz().dot(), (*this)[2].xyz().dot()), + "Math::Matrix4::rotation(): the matrix doesn't have uniform scaling", {}); return {(*this)[0].xyz().normalized(), (*this)[1].xyz().normalized(), (*this)[2].xyz().normalized()}; diff --git a/src/Math/Test/Matrix3Test.cpp b/src/Math/Test/Matrix3Test.cpp index c55425112..2cf0d8b2a 100644 --- a/src/Math/Test/Matrix3Test.cpp +++ b/src/Math/Test/Matrix3Test.cpp @@ -319,21 +319,18 @@ void Matrix3Test::rotationPart() { CORRADE_COMPARE(rotationTranslationPart, expectedRotationPart); /* Test uniform scaling */ - Matrix3 rotationScaling = rotation*Matrix3::scaling(Vector2(9.0f)); + Matrix3 rotationScaling = rotation*Matrix3::scaling(Vector2(3.0f)); Matrix2 rotationScalingPart = rotationScaling.rotation(); CORRADE_COMPARE(rotationScalingPart.determinant(), 1.0f); CORRADE_COMPARE(rotationScalingPart*rotationScalingPart.transposed(), Matrix2()); CORRADE_COMPARE(rotationScalingPart, expectedRotationPart); /* Fails on non-uniform scaling */ - { - CORRADE_EXPECT_FAIL("Assertion on uniform scaling is not implemented yet."); - std::ostringstream o; - Error::setOutput(&o); - Matrix3 rotationScaling2 = rotation*Matrix3::scaling(Vector2::yScale(3.5f)); - CORRADE_COMPARE(o.str(), "Math::Matrix3::rotation(): the matrix doesn't have uniform scaling\n"); - CORRADE_COMPARE(rotationScaling2, Matrix3(Matrix3::Zero)); - } + std::ostringstream o; + Error::setOutput(&o); + Matrix2 rotationScaling2 = (rotation*Matrix3::scaling(Vector2::yScale(3.5f))).rotation(); + CORRADE_COMPARE(o.str(), "Math::Matrix3::rotation(): the matrix doesn't have uniform scaling\n"); + CORRADE_COMPARE(rotationScaling2, Matrix2()); } void Matrix3Test::uniformScalingPart() { diff --git a/src/Math/Test/Matrix4Test.cpp b/src/Math/Test/Matrix4Test.cpp index fb84fc54e..066a4715d 100644 --- a/src/Math/Test/Matrix4Test.cpp +++ b/src/Math/Test/Matrix4Test.cpp @@ -405,21 +405,18 @@ void Matrix4Test::rotationPart() { CORRADE_COMPARE(rotationTranslationPart, expectedRotationPart); /* Test uniform scaling */ - Matrix4 rotationScaling = rotation*Matrix4::scaling(Vector3(9.0f)); + Matrix4 rotationScaling = rotation*Matrix4::scaling(Vector3(3.0f)); Matrix3 rotationScalingPart = rotationScaling.rotation(); CORRADE_COMPARE(rotationScalingPart.determinant(), 1.0f); CORRADE_COMPARE(rotationScalingPart*rotationScalingPart.transposed(), Matrix3()); CORRADE_COMPARE(rotationScalingPart, expectedRotationPart); /* Fails on non-uniform scaling */ - { - CORRADE_EXPECT_FAIL("Assertion on uniform scaling is not implemented yet."); - std::ostringstream o; - Error::setOutput(&o); - Matrix4 rotationScaling2 = rotation*Matrix4::scaling(Vector3::yScale(3.5f)); - CORRADE_COMPARE(o.str(), "Math::Matrix4::rotation(): the matrix doesn't have uniform scaling\n"); - CORRADE_COMPARE(rotationScaling2, Matrix4(Matrix4::Zero)); - } + std::ostringstream o; + Error::setOutput(&o); + Matrix3 rotationScaling2 = (rotation*Matrix4::scaling(Vector3::yScale(3.5f))).rotation(); + CORRADE_COMPARE(o.str(), "Math::Matrix4::rotation(): the matrix doesn't have uniform scaling\n"); + CORRADE_COMPARE(rotationScaling2, Matrix3()); } void Matrix4Test::uniformScalingPart() {