From 5ac883b10a3a7e99811f537be54552ec3e698b44 Mon Sep 17 00:00:00 2001 From: Benoit Jacob Date: Sun, 24 Aug 2008 23:16:51 +0000 Subject: [PATCH] Fix a bug discovered in Avogadro: the AngleAxis*Matrix and the newer AngleAxis*Vector products were wrong because they returned the product _expression_ toRotationMatrix()*other; and toRotationMatrix() died before that expression would be later evaluated. Here it would not have been practical to NestByValue as this is a whole matrix. So, let them simply evaluate and return the result by value. The geometry.cpp unit-test only checked for compatibility between various rotations, it didn't check the correctness of the rotations themselves. That's why this bug escaped us. So, this commit checks that the rotations produced by AngleAxis have all the expected properties. Since the compatibility with the other rotations is already checked, this should validate them as well. --- Eigen/src/Geometry/AngleAxis.h | 6 +++--- test/geometry.cpp | 11 +++++++++-- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/Eigen/src/Geometry/AngleAxis.h b/Eigen/src/Geometry/AngleAxis.h index 266529e7b..503a3d160 100644 --- a/Eigen/src/Geometry/AngleAxis.h +++ b/Eigen/src/Geometry/AngleAxis.h @@ -95,17 +95,17 @@ public: { return a * QuaternionType(b); } /** Concatenates two rotations */ - inline typename ProductReturnType::Type + inline Matrix3 operator* (const Matrix3& other) const { return toRotationMatrix() * other; } /** Concatenates two rotations */ - inline friend typename ProductReturnType::Type + inline friend Matrix3 operator* (const Matrix3& a, const AngleAxis& b) { return a * b.toRotationMatrix(); } /** Applies rotation to vector */ - inline typename ProductReturnType::Type + inline Vector3 operator* (const Vector3& other) const { return toRotationMatrix() * other; } diff --git a/test/geometry.cpp b/test/geometry.cpp index a3876c8d8..2bfdb04b4 100644 --- a/test/geometry.cpp +++ b/test/geometry.cpp @@ -65,8 +65,15 @@ template void geometry(void) VERIFY_IS_APPROX(v0.unitOrthogonal().norm(), Scalar(1)); - q1 = AngleAxis(ei_random(-M_PI, M_PI), v0.normalized()); - q2 = AngleAxis(ei_random(-M_PI, M_PI), v1.normalized()); + VERIFY_IS_APPROX(v0, AngleAxis(a, v0.normalized()) * v0); + VERIFY_IS_APPROX(-v0, AngleAxis(M_PI, v0.unitOrthogonal()) * v0); + VERIFY_IS_APPROX(cos(a)*v0.norm2(), v0.dot(AngleAxis(a, v0.unitOrthogonal()) * v0)); + m = AngleAxis(a, v0.normalized()).toRotationMatrix().adjoint(); + VERIFY_IS_APPROX(Matrix3::Identity(), m * AngleAxis(a, v0.normalized())); + VERIFY_IS_APPROX(Matrix3::Identity(), AngleAxis(a, v0.normalized()) * m); + + q1 = AngleAxis(a, v0.normalized()); + q2 = AngleAxis(a, v1.normalized()); // rotation matrix conversion VERIFY_IS_APPROX(q1 * v2, q1.toRotationMatrix() * v2);