Re: [eigen] RotationBase times DiagonalMatrix

[Hauke Heibel <hauke.heibel@xxxxxxxxxxxxxx>]

Thanks for the feedback Christoph,

I admit that the first fix is trivial and does no harm.

The more interesting question is what we do about the explicit ctor's.
Keep them and prevent hidden temporaries or remove the 'explicit'
keyword and have a nicer user interface.

- Hauke

On Thu, Sep 22, 2011 at 5:06 PM, Christoph Hertzberg
<chtz@xxxxxxxxxxxxxxxxxxxxxxxx> wrote:
> Hi,
>
> I don't see much to review about your proposal, in my eyes it merely asks
> for a poll. But maybe I missed an important point of it ...
>
> Giving my 0.02€, I would agree that in this case making the constructor
> explicit does not really make sense.
>
> Seen from a concept-POV a DiagonalMatrix *is a* transformation matrix (even
> though it does not inherit from it), so I would expect that they can be
> converted implicitly.
>
> Christoph
>
>
> On 22.09.2011 07:36, Hauke Heibel wrote:
>>
>> Sorry guys,
>>
>> but I wanted to bump this thread since it seems to fall asleep and it
>> would be great to get some feedback.
>>
>> If you are too busy to make such reviews I will simply start creating
>> a fork, from which you then may pull ...
>>
>> I am even fine, if we come to a negative conclusion regarding my
>> proposals as long as we manage to get to some decision.
>>
>> Regards,
>> Hauke
>>
>> On Tue, Aug 9, 2011 at 8:48 AM, Hauke Heibel
>> <hauke.heibel@xxxxxxxxxxxxxx>  wrote:
>>>
>>> Hi Benoit,
>>>
>>> welcome back. :)
>>>
>>> On Tue, Aug 9, 2011 at 7:25 AM, Benoit Jacob<jacob.benoit.1@xxxxxxxxx>
>>>  wrote:
>>>>>
>>>>> inline Transform<Scalar,Dim,AffineCompact>  operator*(const
>>>>> DiagonalMatrix<Scalar,Dim>&  s) const
>>>>> { return Transform<Scalar,Dim,AffineCompact>(*this) * s; }
>>>>
>>>> My problem with this solution is that the product of a rotation times
>>>> a diagonal matrix is still a linear transformation, so why return a
>>>> Transform which is specifically an affine (not linear) transformation?
>>>> I'm in favor of using plain matrices everytime that a plain, arbitrary
>>>> linear transformation is meant.
>>>
>>> I see what you mean and when Affine transformations were allowed to be
>>> implicitly constructed from any Dim x Dim matrix that were a perfect
>>> solution.
>>>
>>>> Ah OK, I see. It doesn't compile with a) because the assignment in the
>>>> declaration is interpreted as construction, so it tries to use the
>>>> constructor (taking EigenBase) instead of operator= and fails as it's
>>>> an explicit constructor.
>>>
>>> Exactly.
>>>
>>>> That's really stupid :-/ C++ is able to convert "T a = b;" into "T
>>>> a(b);" only to fail when the constructor here is explicit. But if one
>>>> writes T a(b); or T a; a=b;  then the error goes away.
>>>
>>> I think that behavior is perfectly fine. The question is why at all allow
>>>
>>> T a; a=b;
>>>
>>> while declaring the ctor explicit!? That's a little bit of a
>>> contradiction. From what I understand the reason Gael implemented it
>>> like this is to prevent the creation of hidden temporaries but I am
>>> not sure anymore whether this preemptive optimization step is useful.
>>> Is there at all a measurable performance penalty for these little
>>> stack objects?
>>>
>>>> I'd say that's a problem with C++ itself and I see only two approaches:
>>>>  - either live with that and tell Eigen users to use A a(b) instead of
>>>> A a  = b when the class A has an explicit constructor
>>>
>>> The syntax becomes really ugly. My colleagues and me are working since
>>> 4-5 weeks extensively with the Geometry parts of Eigen and just one
>>> example is writing and using functions that take Eigen::Transform<...>
>>> as an input parameter. You cannot pass an Eigen::Translation, you
>>> cannot pass Eigen::Scaling, all due to the explicit constructors. In
>>> these cases, I really want would love to have the explicit conversion
>>> capability which would make the code much more readable at the cost of
>>> a few extra bytes wasted when converting an Eigen::Translation into an
>>> Eigen::Transform.
>>>
>>>>  - or stop making constructors 'explicit', consider that C++ language
>>>> feature flawed: it doesn't work nicely with C++'s
>>>> convert-assignment-to-construction rule
>>>
>>> I still think explicit does exactly what it is meant to do. The only
>>> question is whether we really want it in this case.
>>>
>>> Regards,
>>> Hauke
>>>
>>
>
>
> --
> ----------------------------------------------
> Dipl.-Inf. Christoph Hertzberg
> Cartesium 0.051
> Universität Bremen
> Enrique-Schmidt-Straße 5
> 28359 Bremen
>
> Tel: (+49) 421-218-64252
> ----------------------------------------------
>
>
>