Re: [eigen] Do we need geometry refactoring? |

[ Thread Index | Date Index | More lists.tuxfamily.org/eigen Archives ]

*To*: eigen@xxxxxxxxxxxxxxxxxxx*Subject*: Re: [eigen] Do we need geometry refactoring?*From*: Benoit Jacob <jacob.benoit.1@xxxxxxxxx>*Date*: Wed, 18 Aug 2010 09:34:56 -0400*Dkim-signature*: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:received:received:in-reply-to :references:date:message-id:subject:from:to:content-type; bh=gRPDiMofYodu4iaNWyo1FqkTSH7BUKc0fja1px0FY0s=; b=lT6lX2B+ezAhAPgl9cFULaXwPB4/e9sj9XpDw1ZU2VQzJEo1ofxnuYe4tdB8jj8PXt eeoQVdYuqaqsrqVgTtleZfq2+nXHT8Gx/1Ytse111hQ3kXTkEVKbSqElGBZx2slZZ3S8 P8H2XwM44fZeFCo5pSiotMgnQ2MAIl37eXVzc=*Domainkey-signature*: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :content-type; b=iQTgAeg22uTYVP/XJ3iiLq/3p1mjSBYKtU4q9iu+4wtaBp2V286Jynm7PStfKSke7K CmOSjshQ0DmZG0F3z2iQPdPAp3MRADdv1pdYL7v+UpXiiE1iVQQicGnww4rjKg+P9OCe OpapS30cWlueif0FByKULLihICmMyGuUWQ/M0=

2010/8/17 Hauke Heibel <hauke.heibel@xxxxxxxxxxxxxx>

Ah, thanks for this summary.

This would be the only place where I don't agree: linear/affine/whatever transformations don't necessarily have to be invertible.

Indeed, multiplying by a non-isometry (i.e. non-orthogonal) destroys being an isometry.. Elsewhere in Eigen we make this sort of thing the responsibility of the user, but I agree that it's unreasonable here as the user didn't _ask_ for the result to be considered an isometry.

I understand that's useful when you want to apply a transformation to a homogeneous vector, or to a batch of homogeneous vectors stored as the columns of a matrix.

However, if we do that, then I would also suggest that we do that in the non-homogeneous case for affine transforms, that is, in case 1) above:

Affine3d * Vector3d ---> Vector3d

more generally:

Affine3d * MatrixWith3Rows ---> MatrixWith3Rows

This is just for consistency. If we get the user used to the convenience that transform can be applied to a matrix, we should be consistent. Also, if we agree that the ability to do these products is useful (which I believe) then they are also useful in the affine case with non-homogeneous vectors.

right.

right.

do we really special-case 1 and Dynamic? I think, if we agree on the plan outlined above, we should be generic in the number of columns.

Right. An affine transform is just a special case of projective transform.

Exactly!

Exactly what I had in mind! So actually, we do agree about the point 1) above! I thought you wanted to disallow the latter product and suggested that it does what you just described.

Yep, it looks all good to me. Can you just clarify the apparent contradiction between what you said at the top (disallow 1)) and what you just said (allow the latter product).

Cheers,

Benoit

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

Ah, thanks for this summary.

Both are violated in case the RHS matrices are not invertible.

This would be the only place where I don't agree: linear/affine/whatever transformations don't necessarily have to be invertible.

Rule 1) is on top violated, in case the "linear" matrix is not

orthogonal and applied to an Isometry.

Indeed, multiplying by a non-isometry (i.e. non-orthogonal) destroys being an isometry.. Elsewhere in Eigen we make this sort of thing the responsibility of the user, but I agree that it's unreasonable here as the user didn't _ask_ for the result to be considered 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.

I understand that's useful when you want to apply a transformation to a homogeneous vector, or to a batch of homogeneous vectors stored as the columns of a matrix.

However, if we do that, then I would also suggest that we do that in the non-homogeneous case for affine transforms, that is, in case 1) above:

Affine3d * Vector3d ---> Vector3d

more generally:

Affine3d * MatrixWith3Rows ---> MatrixWith3Rows

This is just for consistency. If we get the user used to the convenience that transform can be applied to a matrix, we should be consistent. Also, if we agree that the ability to do these products is useful (which I believe) then they are also useful in the affine case with non-homogeneous vectors.

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.

right.

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.

right.

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>

do we really special-case 1 and Dynamic? I think, if we agree on the plan outlined above, we should be generic in the number of columns.

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.

Right. An affine transform is just a special case of projective transform.

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.

Exactly!

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,*>

Exactly what I had in mind! So actually, we do agree about the point 1) above! I thought you wanted to disallow the latter product and suggested that it does what you just described.

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.

Yep, it looks all good to me. Can you just clarify the apparent contradiction between what you said at the top (disallow 1)) and what you just said (allow the latter product).

Cheers,

Benoit

- Hauke

**Follow-Ups**:**Re: [eigen] Do we need geometry refactoring?***From:*Hauke Heibel

**References**:**Re: [eigen] Do we need geometry refactoring?***From:*Manuel Yguel

**Re: [eigen] Do we need geometry refactoring?***From:*Hauke Heibel

**Re: [eigen] Do we need geometry refactoring?***From:*Benoit Jacob

**Re: [eigen] Do we need geometry refactoring?***From:*Hauke Heibel

**Re: [eigen] Do we need geometry refactoring?***From:*Benoit Jacob

**Re: [eigen] Do we need geometry refactoring?***From:*Hauke Heibel

**Re: [eigen] Do we need geometry refactoring?***From:*Benoit Jacob

**Re: [eigen] Do we need geometry refactoring?***From:*Benoit Jacob

**Re: [eigen] Do we need geometry refactoring?***From:*Benoit Jacob

**Re: [eigen] Do we need geometry refactoring?***From:*Hauke Heibel

**Re: [eigen] Do we need geometry refactoring?***From:*Benoit Jacob

**Re: [eigen] Do we need geometry refactoring?***From:*Hauke Heibel

**Re: [eigen] Do we need geometry refactoring?***From:*Hauke Heibel

**Re: [eigen] Do we need geometry refactoring?***From:*Hauke Heibel

**Re: [eigen] Do we need geometry refactoring?***From:*Gael Guennebaud

**Re: [eigen] Do we need geometry refactoring?***From:*Hauke Heibel

**Re: [eigen] Do we need geometry refactoring?***From:*Hauke Heibel

**Messages sorted by:**[ date | thread ]- Prev by Date:
**Re: [eigen] Getting to a solution from sparse matrices, Eigen3** - Next by Date:
**Re: [eigen] about changeset 6eb14e380** - Previous by thread:
**Re: [eigen] Do we need geometry refactoring?** - Next by thread:
**Re: [eigen] Do we need geometry refactoring?**

Mail converted by MHonArc 2.6.19+ | http://listengine.tuxfamily.org/ |