Re: [eigen] Transform products

[Gael Guennebaud <gael.guennebaud@xxxxxxxxx>]

Hi,

first of al let me say l I agree with the changes proposed in the other thread.

However, DiagonalMatrix is used to represent non uniform scaling. For
instance Scaling(1,2) returns a 2x2 DiagonalMatrix. So this time I
think that

T * DiagonalMatrix

should really be interpreted as T * scaling, whence the following rules:

Affine = Isometry * diagonal
Affine = Affine * diagonal
Projective = Projective * diagonal

Who want to represent points using a diagonal matrix....

Otherwise we would have to add an explicit NonUniformScaling class,
but this is exactly what a DiagonalMatrix is....

gael

On Thu, Aug 19, 2010 at 1:00 PM, Hauke Heibel
<hauke.heibel@xxxxxxxxxxxxxx> wrote:
> Ok, I am starting a new thread. The other one was way too long.
>
> I started with the RHS product refactoring. Everything went well,
> until I hit the case, where the RHS is a DiagonalMatrix. The patch is
> attached but I will anyways past the most important piece of code over
> here because it is hard to find in the diff.
>
> The function causing troubles is ei_transform_right_product_impl::run
> for Affine and Isometry. I tried to make a one fits all implementation
> and in theory, it works well. Even the inlining on MSVC seems to be
> working for fixed size types. For DiagonalMatrix, I am hitting the
> issue, that there is now way to extract a block.
>
> Here is how I tried to implement run:
>
>  EIGEN_STRONG_INLINE static ResultType run(const
> Transform<Scalar,Dim,Mode>& T, const Other& other)
>  {
>    EIGEN_STATIC_ASSERT(OtherRows==Dim || OtherRows==HDim,
> YOU_MIXED_MATRICES_OF_DIFFERENT_SIZES);
>
>    typedef Block<ResultType, Dim, OtherCols> TopLeftLhs;
>    typedef Block<Other, Dim, OtherCols>      TopLeftRhs;
>
>    ResultType res(other.rows(),other.cols());
>
>    TopLeftLhs(res, 0, 0, Dim, other.cols()) =
>      ( T.linear() * TopLeftRhs(other, 0, 0, Dim, other.cols()) ).colwise() +
>        T.translation();
>
>    // This is a hack to circumvent the fact that DiagonalMatrix does
> not expose ::row(int)
>    // In fact, the code below will not even be used in release mode,
> but it will still be compiled.
>    if (OtherRows==HDim)
>      Block<ResultType, 1, OtherCols>(res,0,0,1,other.cols()) =
>      Block<Other, 1, OtherCols>(other.derived(),0,0,1,other.cols());
>
>    return res;
>  }
>
> I used Block in the end, because row() does not exist for
> DiagonalMatrix. Any Ideas? I would be too sad, if I were required to
> write a specialization just because of this since in general, the if
> clause is already removed by the compiler -- though it is still
> compiled.
>
> On the other hand side, who want to do something like this because now
> everything on the RHS is interpreted as points. If the RHS is 3x3 it
> is not any more interpreted as [S 0; 0 1]. The correct move would be
>
> A.linear() *= S;
>
> and even this works
>
> A * (S * pts_33)
>
> though this does not
>
> (A * S) * pts_33
>
> I could even go back to row() instead of block().
>
> I am pausing the patching for now, until I get feedback.
>
> - Hauke
>