This bugzilla service is closed. All entries have been migrated to https://gitlab.com/libeigen/eigen
Bug 758 - operator* allows premultiplication of a Quaterniond by a scalar and returns invalid results
operator* allows premultiplication of a Quaterniond by a scalar and returns i...
 Status: RESOLVED FIXED None Eigen Unclassified Geometry (show other bugs) 3.2 All All High Wrong Result Nobody 459 3.3 560 Show dependency tree / graph

 Reported: 2014-03-11 23:58 UTC by adampbry 2019-12-04 13:04 UTC (History) 4 users (show) chtz gael.guennebaud hauke.heibel jacob.benoit.1

Attachments

 adampbry 2014-03-11 23:58:51 UTC ```The following code should either not compile or give v1 = 4 * v2 Quaterniond q1 = Quaterniond::Identity(); Vector3d v1 = 4 * q1 * Vector3d::UnitZ(); Vector3d v2 = q1 * Vector3d::UnitZ(); Instead we get: v1 0 0 0 v2 0 0 1``` Christoph Hertzberg 2014-03-12 00:15:34 UTC ```The problem is that scaling currently scales a quaternion, but Quaternion * Vector3 assumes that the Quaternion is of unit length. Bug 459 would solve this. Note that you still could not express reflections (i.e. multiplying by -1.0 would not change anything) Somewhat related, but contradicting is the idea of Bug 560. For consistency, this would rather result in (4*q)*v == 16*(q*v); [so it would be a good idea not to overload GeneralizedQuaternion * Vector3]``` Christoph Hertzberg 2014-03-12 14:27:49 UTC ```Sorry, my assumptions were wrong; the error appears to be much more subtle. What happens is that Scalar * RotationBase calls operator*(DiagonalMatrix, RotationBase) and DiagonalMatrix has a constructor from type Index. Your case should actually result in a runtime assertion (if not disabled), but these examples would not: 3 * q; 3.2 * q; both result in DiagonalMatrix(3) * q which is bogus but valid. Another DiagonalMatrix constructor which I think should fail is DiagonalMatrix(3, 1); Furthermore, for DiagonalMatrix(1.0) I think it would make more sense to initialize the matrix entry by 1.0, instead of constructing an uninitialized matrix. The easiest solution to make your code not compile is to mark the corresponding constructor as explicit, which I think would be a good idea anyways. However, this results in a page full of "template argument deduction/substitution failed" messages. To make your code compile and work properly, we could add a friend operator*(const Scalar& s, const RotationBase& r); to RotationBase which returns a scaled rotation (i.e. s * r.toRotationMatrix()). For consistency, we should then also implement RotationBase::operator*(const Scalar& s); with the same result.``` Gael Guennebaud 2016-01-31 13:05:09 UTC ```Looks like this problem has already been partly fixed as "4 * q1 * Vector3d::UnitZ()" does not compile anymore in the 3.3 branch. This is because DiagonalMatrix(Index) is declared explicit now. In 3.2 it raises a runtime assertion, so actually, "4 * q1 * Vector3d::UnitZ()" has always been invalid.``` Nobody 2019-12-04 13:04:13 UTC ```-- GitLab Migration Automatic Message -- This bug has been migrated to gitlab.com's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.com/libeigen/eigen/issues/758.```

 Note You need to log in before you can comment on or make changes to this bug.