Re: [eigen] Patch to AlignedBox

[Gael Guennebaud <gael.guennebaud@xxxxxxxxx>]

On Thu, Oct 11, 2012 at 10:50 AM, Wood, Tobias <tobias.wood@xxxxxxxxx> wrote:
> Hi Gael,
> Thanks for the response.
>
>> what's the use case for "expand"?
> In this case I am using an AlignedBox3d to keep track of the view limits for an OpenGL volume rendering application. I implemented zooming into/out of the volume by expanding/contracting the AlignedBox. AlignedBox already has a translate() method, so I thought it was odd that is was missing some way to scale it as well.

ok, why not.

>> (I think "scale" would be less
>> misleading since we already have a "extend" function with a totally
>> different behavior)
> I completely agree.
>
>> Regarding CornerMatrixType I think it should be a fixed size matrix
>> instead of a dynamic one.
> For some odd reason when I wrote it the first time I convinced myself you couldn't compute the matrix size at compile time. But obviously you can, so I've actually made this change already by defining:
> typedef Matrix<Scalar,AmbientDimAtCompileTime,1 << AmbientDimAtCompileTime> CornerMatrixType;

good.

>> Finally, regarding the new names, I'd rather sort the words in X, Y, Z
>> order, what do you think?
> This is unfortunately a matter of personal preference. I think that X, Y, Z is the more logical ordering. However the 2D names are currently defined in Y, X order, so I went Z, Y, X for consistency. Perhaps it is also more common to say "Bottom Left" rather than "Left Bottom" in daily usage? To make it even more confusing, the 1D names are "Min" and "Max", which don't correspond to any particular axis but are more general. I would argue the most important thing here is consistency, so which ever ordering is chosen the 1D and 2D names should match.

ok, I forgot about the 2D names.

> Is there some way to mark the old names as deprecated, beyond a comment in the code and note in the documentation?

in the doc use \deprecated, and in the code there is a
EIGEN_DEPRECATED macro, but it won't work for individual enum name, so
we can only mark it in the doc.

also, your new function could probably call corner() instead of
duplicating the code.

cheers,
gael

> I'm happy to submit an updated patch once the above is decided on.
>
> Best wishes,
> Toby
>
>