Re: [eigen] Do we need geometry refactoring?

[Hauke Heibel <hauke.heibel@xxxxxxxxxxxxxx>]

I looked into the remaining transformations and there seem to be some
bugs hidden which are not exposed by our unit tests. Maybe some of
them or only because of my changes.

First, on thing I recognized. You consider any fixed size (Dim)x(Dim)
or (Dim+1)x(Dim+1) matrix as a linear and generic transformation
respectively. Therefore, we have the following special product paths
for Transform

1)  T * linear matrix => T
2)  T * generic matrix => Projective

Both are violated in case the RHS matrices are not invertible.
Rule 1) is on top violated, in case the "linear" matrix is not
orthogonal and applied to an Isometry.

For these paths, my suggestion is to prohibit case 1) completely. I
think the functionality is well covered by

T.linear() *= linear matrix

For case 2), I would choose to simply return a matrix. This is
mathematically more sane and in case your are dead sure, what you are
doing, you can still do

T * Projective(generic matrix)
T * Affine(generic matrix)
etc.

This is even better because it will not necessarily end up with a
projective transformation. It is also more explicit (the ctors are
explicit) and leaves no option for interpretations as to what is
actually happening.

The next issue - and this maybe newly introduced by me - is that currently

  Transform<Scalar,Dim,Projective> * Matrix<Scalar,Dim,Dynamic>

compiles. The chosen implementation path is wrong because it assumes
Affine or Isometric times set of points and even if we agreed upon
extending the RHS matrix by 1s as their homogeneous coordinates, we
were suddenly required to return something homogeneous.

Let's quickly take a look at what kind of point transformations we
currently allow for Affine and Isometry:

a) T * Matrix<Scalar,Dim,Dynamic>
b) T * Matrix<Scalar,Dim,1>
c) T * Matrix<Scalar,Dim+1,1>

On top, we allow for Projective

d) T * Matrix<Scalar,Dim+1,Dynamic>

I would propose to allow d) for Affine and Isometry too. It is
mathematically perfectly valid and would simply leave the homogeneous
coordinates untouched.

Currently, the only issue is that we allow

T * Matrix<Scalar,Dim,Dim>  --> T
T * Matrix<Scalar,Dim+1,Dim+1>  --> Projective

as discussed above. In case we do what I proposed, the ambiguity were
resolved and we could allow d) for all transformations.

Here is the summary of allowed transforms as I suggest to offer them -
the asterisk is a wildcard for anything.

// this is all for projective
Matrix<Scalar,Dim+1,*> = Transform<Scalar,Dim,Projective> *
Matrix<Scalar,Dim+1,*>

// these for Affine, AffineCompact and Isometry
Matrix<Scalar,Dim+1,*> = Transform<Scalar,Dim,Affine> * Matrix<Scalar,Dim+1,*>
Matrix<Scalar,Dim,*> = Transform<Scalar,Dim,Affine> * Matrix<Scalar,Dim,*>

What do you say? The changes are easy and they will reduce code and
template instantiations, make our code more safe and more easy to
understand -- for the user and devloper.

- Hauke