Re: [eigen] Do we need geometry refactoring?

[Benoit Jacob <jacob.benoit.1@xxxxxxxxx>]

2010/7/29 Adolfo Rodríguez Tsouroukdissian <adolfo.rodriguez@xxxxxxxxxxxxxxxxx>:
> My opinions,
>
> On Thu, Jul 29, 2010 at 12:56 PM, Hauke Heibel <hauke.heibel@xxxxxxxxxxxxxx>
> wrote:
>>
>> Hi,
>>
>> I was once again working a bit with the geometry module and stumbled
>> over a few tiny issues.
>>
>> 1) I discussed this before with Gael but did not commit yet so I am
>> repeating it here. We think it makes sense to default transformation
>> modes to Projective. This is more generic and safer.
>
> Makes sense

OK for that. This is also what OpenGL matrices are (just mentioning
one more reason why this is a plausible default).

>
>>
>> 2) Unifying member access.
>
> Yes!
>
>>
>> We have several classes that behave like matrix transformations in the
>> sense that they define the required products in order to mimic a
>> matrix. The core class is Transform and transforms allows the user to
>> access the underlying matrix in several ways
>>
>> Transform::matrix() returns a homogeneous representation of the transform
>> Transform::linear() returns the upper left Dim by Dim component of the
>> matrix
>> Transform::translation() returns the translation, upper right Dim by 1
>>
>> But what about other classes, how about e.g. Translation? Translation
>> allows to access the data via a single function called
>> Translation::vector() - this is not very intuitive. And I would prefer
>> another function
>>
>> Translation::translation()
>>
>> That may sound redundant, but it would be following what we do in
>> transform.

I see... ok for the consistency reason, but it's of course
incomfortable to have it differ from a constructor only by letter case
(t vs T)... perhaps we can have both.

Initially I thought that there was little use for that (why would you
store a translation as a dense matrix) but indeed since your method
would return just a read-only expression, it could be reasonable e.g.
when constructing a matrix from some algebraic operation involving
that translation expression.

>> We could even offer Translation::matrix() and return a
>> read-only identity.

Assuming you mean a read-only expression.

>> This function is one step towards being able to
>> interchange types as Isometry3f and Translation3f (in case the
>> isometry were just a translation).

OK for that. I just note that
1) it's not going to be used very often
2) it's purely an API addition so doesn't block 3.0

>>
>> Ok, the question is actually whether we want Translation, Quaternion,
>> etc. behave like Transformations?

Translation and Quaternion are different beasts here.

The reason for Transform to exist is to represent non-linear (i.e.
affine/projective) transformations.

Translation is indeed a special case of that. But a Quaternion of norm
1 represents a rotation which is linear, so there is no reason to
bother about Transform here; instead we unify Quaternion with other
rotations in RotationBase and we take care once and for all of the
interplay of rotations with general Transformations.

>> Should we allow the same
>> initialization as well, so would it make sense to allow initialization
>> of a translation from a (Dim+1) by (Dim+1) matrix?
>
> This all sounds pretty reasonable to me.

Ouch...! What would possibly be the use case for that? Think of it this way:

     "Should we add to DiagonalMatrix a general constructor
       taking a dense Matrix (assuming all the off-diagonal coeffs
       are zero) ?"

I you agree with me that the answer to the latter question is "no" do
you also agree with me it should be "no" here?

My best rationale would be that if the user calls this, he knows that
the matrix is a translation matrix, so _why_ would he ever have stored
it as a dense matrix?

>>
>> 3) This is more like a tiny question. Why are we setting the
>> projective part of the matrix to 0 ... 0 1 in the inverse method? I
>> think it is absolutely useless since it should already be like that...

I agree.

>>
>> - Hauke
>>
>>
> I haven't looked into the geometry module for a while, and I see that some
> things have been changing (dev branch). I have a couple of quick questions
> regarding the Transform class:
>
> - The introduction of the Mode template parameter looks promising, but it
> does not (yet) seem to be fully in use. For example, the inverse method does
> not take much advantage of it, and still uses the hint TransformTraits
> parameter. Will the hint parameter be preserved once the Mode template
> parameter is better supported?.

This seems to be an oversight indeed. I would like the hint parameter
to go away.

>
> - For Mode=Isometry (which does not seem to be documented yet) the
> transformation should be stored as a (Dim)x(Dim+1) matrix, as in the
> AffineCompact case, right?.

I guess so. Not as memory efficient as quaternion+vector or
dual-quaternion representations, but... probably the most reasonable
representation in the context of Transform which is a single matrix in
all other cases.

Benoit

>
> Cheers,
>
> Adolfo
>
> --
> Adolfo Rodríguez Tsouroukdissian, Ph. D.
>
> Robotics engineer
> PAL ROBOTICS S.L
> http://www.pal-robotics.com
> Tel. +34.93.414.53.47
> Fax.+34.93.209.11.09
> AVISO DE CONFIDENCIALIDAD: Este mensaje y sus documentos adjuntos, pueden
> contener información privilegiada y/o confidencial que está dirigida
> exclusivamente a su destinatario. Si usted recibe este mensaje y no es el
> destinatario indicado, o el empleado encargado de su entrega a dicha
> persona, por favor, notifíquelo inmediatamente y remita el mensaje original
> a la dirección de correo electrónico indicada. Cualquier copia, uso o
> distribución no autorizados de esta comunicación queda estrictamente
> prohibida.
>
> CONFIDENTIALITY NOTICE: This e-mail and the accompanying document(s) may
> contain confidential information which is privileged and intended only for
> the individual or entity to whom they are addressed.  If you are not the
> intended recipient, you are hereby notified that any disclosure, copying,
> distribution or use of this e-mail and/or accompanying document(s) is
> strictly prohibited.  If you have received this e-mail in error, please
> immediately notify the sender at the above e-mail address.
>