This bugzilla service is closed. All entries have been migrated to

Bug 758

Summary: operator* allows premultiplication of a Quaterniond by a scalar and returns invalid results
Product: Eigen Reporter: adampbry
Component: GeometryAssignee: Nobody <eigen.nobody>
Severity: Wrong Result CC: chtz, gael.guennebaud, hauke.heibel, jacob.benoit.1
Priority: High    
Version: 3.2   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 459, 558, 560    

Description 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:

Comment 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]
Comment 2 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<double,3>(3) * q which is bogus but valid.
Another DiagonalMatrix constructor which I think should fail is
  DiagonalMatrix<int, 3>(3, 1);
Furthermore, for DiagonalMatrix<double, 1>(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.
Comment 3 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.
Comment 4 Nobody 2019-12-04 13:04:13 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to'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: