Re: [eigen] Do we need geometry refactoring?

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

2010/7/29 Benoit Jacob <jacob.benoit.1@xxxxxxxxx>:
> 2010/7/29 Adolfo Rodríguez Tsouroukdissian <adolfo.rodriguez@xxxxxxxxxxxxxxxx>:
>> 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.

The above paragraph ("Initially...") was meant to be moved below:

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

 <---- HERE.


Benoit


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