669
2013-10-02 23:41:09 +0000
Translation * Mapped vector does not compile
2019-12-04 12:39:46 +0000
1
1
1
Unclassified
Eigen
Geometry
unspecified
All
All
DECISIONNEEDED
Normal
API Change
---
814
1
gael.guennebaud
eigen.nobody
chtz
gael.guennebaud
hauke.heibel
jacob.benoit.1
oldest_to_newest
2759
0
gael.guennebaud
2013-10-02 23:41:09 +0000
See: http://forum.kde.org/viewtopic.php?f=74&t=117764
The problem is that this product does not instanciate the right Translation::operator* overload. The fix consists in having a general overload for MatrixBase<> objects with meta-programming tricks to return the right type (Affine or Vector) with the right implementation more like Transform::operator*.
2780
1
chtz
2013-10-16 12:38:59 +0000
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).
2781
2
chtz
2013-10-16 12:43:18 +0000
(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).
4745
3
gael.guennebaud
2015-06-10 09:59:34 +0000
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.
5238
4
gael.guennebaud
2015-12-10 17:09:11 +0000
Regarding the doc issue: https://bitbucket.org/eigen/eigen/commits/969affbdd8a
5244
5
chtz
2015-12-14 01:57:37 +0000
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)
9113
6
eigen.nobody
2019-12-04 12:39:46 +0000
-- 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.