Bug 669

There are other Transform-related operations/conversions that don't work, e.g.

  AngleAxisd a;
  DiagonalMatrix<double,3> d;
  
  Affine3d T0 = d;               // not ok
  Affine3d T1 = a * d;           // ok
  Affine3d T2 = a * d.inverse(); // not ok
  Affine3d T3(a * d.inverse());  // ok (explicit conversion from Matrix3d)
  Affine3d T4 = a.toRotationMatrix(); // not ok

I assume there are more examples.

Maybe we should introduce a TransformBase from which all classes which can be converted to a Transform must inherit. Then we implement several Transform * Transform products which must return something derived from TransformBase (with a fall-back to Transform(a) * Transform(b) if no specialization is applicable).

Alternatively, instead of TransformBase, introduce a transform_traits struct. That would also avoid having to avoid DiagonalBase (which should not depend on the Geometry module).

I would also not limit x to be a vector in Transform * x. If it is a matrix, apply Transform to each column separately:

  AngleAxisd a;
  DiagonalMatrix<double,3> d;
  Matrix<double, 3, 2> m;   // essentially, a pair of 3d vectors
  a * m;                    // does not work
  a * d * m;                // ok
  a * (d * m);              // does not work
  a.toRotationMatrix() * m; // ok

There is a problem however, when multiplying Translation3d * Matrix3d. The latter could be interpreted as three separate vectors (resulting in 3 vectors) or a linear transformation (resulting in an affine transformation).

(In reply to comment #1)
> Alternatively, instead of TransformBase, introduce a transform_traits struct.
> That would also avoid having to avoid DiagonalBase (which should not depend on
> the Geometry module).

Sorry for the messed up last sentence. What I wanted to say is that this way DiagonalBase does not have to inherit from TransformBase (and the diagonal-traits can be defined inside the Geometry module).

Looking at this general issue again, it also seems that the documentation of Transform * MatrixBase is misleading. Among the different possibilities for the rhs, it mentions:

"a linear transformation matrix of size Dim x Dim,"

suggesting that the lhs L should in this case be interpreted as the following affine transformation:

  L 0
  0 1

but, that's not the case as it is interpreted as a collection of Dim column vectors of size Dim+1 completed with 1 for the homogeneous coordinate:

  L
  1

I think that the current behavior is the only one we could do, but then the documentation is wrong, and we also have a serious discrepancy between Translation and Transform:

Translation3f T;
Matrix3f L;

T * Affine3f(L) == Affine3f(T) * Affine3f(L)  // OK
T * L == T * Affine3f(L)                      // might look ok at a first glance, but:
Affine3f(T) * L != T * L                      // clearly not OK


I don't known how to fix this without breaking existing code, perhaps even silently. This also complicates the generalisation of Translation::operator*. IMO, the "correct" fix would be to add a Linear<> decorator and always see a MatrixBase as a collection of column vectors and never as a rotation, linear or affine transform.

Regarding the doc issue: https://bitbucket.org/eigen/eigen/commits/969affbdd8a

Regarding comment 3:
I like the idea of a Linear<> decorator (or we could add Linear, Rotation, ScaledRotation, ... modes to Transform).

An idea which at least would not silently break existing code:

Transform<...> * MatrixBase<...> results in a Product<Transform, Matrix> object which can only be assigned to another MatrixBase object, but not to a Transform object and also is not multipliable by another Matrix; and Transform * Transform (including Translation/Rotation/...) results in a Product<Transform, Transform> object, which can only be assigned to a transform, or multiplied by either another Transform or a Matrix.

  Transform<...> T1, T2, T3;  // or Translation/Scaling/Rotation/...
  Matrix M1, M2, M3;

  T1 = T2 * T3;         // ok, works as before
  T1 = T2 * M1;         // compile error!
  T1 = T2*Affine3d(M1); // ok, M1 is interpreted as linear operator
  M1 = T1 * M2;         // ok, M2 is interpreted as set of vectors
  M1 = T1 * T2;         // does not compile
  M1 = (T1 * T2) * M2;  // ok, T1 and T2 result in a Transform 
                        // which is applied to M2
  M1 = T1 * (T2 * M2);  // ok, same result as above
  M1 = T1 * (M2 * M3);  // ok, M2*M3 is evaluated first, then transformed
  M1 = (T1 * M2) * M3;  // compile error, dubious whether M2 is a
                        // set of vectors or a linear operator
  M1 = Matrix(T1*M2)*M3; // ok, result of T1*M2 is interpreted as matrix
  M1 = (T1*M2).matrix()*M3; // same result, without explicit temporary

The not compiling cases can give reasonable compile errors and we might even provide a compile time flag to temporarily switch to the old behavior (as we did with EIGEN2_SUPPORT)

-- 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/669.