Bugzilla – Bug 370
Last modified: 2011-11-16 15:35:10 UTC
It would be nice if Eigen::AlignedBox would have 3f,2f,... typedefs, like Eigen::Matrix has.
Want to make a patch? ;-) see how it's done in Matrix.h
Created attachment 226 [details]
Attached patch appears to work. Review it anyway, please.
Created attachment 227 [details]
Patch to Matrix typedefs
While creating attachment #226 [details] I came across a potential mistake in the Matrix typedefs.
Comment on attachment 226 [details]
Review of attachment 226 [details]:
I do not use the Geometry module, but this patch seems very localized and thus safe. I have some questions on details (see the comments). I would also like to see a test; this could be as simple as changing test/geo_alignedbox to use the new typedef in some places.
@@ +379,5 @@
> +EIGEN_MAKE_TYPEDEFS_ALL_SIZES(int, i)
> +EIGEN_MAKE_TYPEDEFS_ALL_SIZES(float, f)
> +EIGEN_MAKE_TYPEDEFS_ALL_SIZES(double, d)
> +EIGEN_MAKE_TYPEDEFS_ALL_SIZES(std::complex<float>, cf)
> +EIGEN_MAKE_TYPEDEFS_ALL_SIZES(std::complex<double>, cd)
Does AlignedBox work when Type is complex? I would think not; you need an ordering of the numbers.
@@ +398,5 @@
> +EIGEN_USING_ALIGNEDBOX_TYPEDEFS_FOR_TYPE(i) \
> +EIGEN_USING_ALIGNEDBOX_TYPEDEFS_FOR_TYPE(f) \
> +EIGEN_USING_ALIGNEDBOX_TYPEDEFS_FOR_TYPE(d) \
> +EIGEN_USING_ALIGNEDBOX_TYPEDEFS_FOR_TYPE(cf) \
Is the EIGEN_USING_ALIGNED_TYPEDEFS macro needed? I assume the definitions were copied from Matrix.h. In the case of Matrix.h, the macros are used in the definition of USING_PART_OF_NAMESPACE_EIGEN in the Eigen2 era; you can still find that macro in Eigen2Support. However, I think this idea has been abandoned in Eigen3.
Incidentally, the EIGEN_USING_MATRIX_TYPEDEFS macro should perhaps be moved from Matrix.h to Eigen2Support.
(In reply to comment #3)
> Created attachment 227 [details] [review]
> Patch to Matrix typedefs
> While creating attachment #226 [details] [review] I came across a potential mistake in the Matrix
I am committing this patch. Thanks.
Created attachment 228 [details]
Improved according to comment #4.
[Geometry/AlignedBox] New typedefs, like for Core/Matrix
Includes 1-4 and dynamic sized boxes for int, float and double type.
Also changes the tests to use these typedefs.
I added 1-dim boxes because the tests used them and removed the c* types.
I committed your patch (changeset 1c6393ab51e0).
Thanks very much for your help.