|Product:||Eigen||Reporter:||Dennis Schridde <devurandom>|
|Severity:||Unknown||CC:||gael.guennebaud, hauke.heibel, jacob.benoit.1, jitseniesen|
Description Dennis Schridde 2011-10-31 13:40:40 UTC
It would be nice if Eigen::AlignedBox would have 3f,2f,... typedefs, like Eigen::Matrix has.
Comment 1 Benoit Jacob 2011-10-31 13:44:03 UTC
Want to make a patch? ;-) see how it's done in Matrix.h
Comment 2 Dennis Schridde 2011-11-08 14:38:51 UTC
Created attachment 226 [details] Proposed patch Attached patch appears to work. Review it anyway, please.
Comment 3 Dennis Schridde 2011-11-08 14:39:41 UTC
Created attachment 227 [details] Patch to Matrix typedefs While creating attachment #226 [details] I came across a potential mistake in the Matrix typedefs.
Comment 4 Jitse Niesen 2011-11-09 12:32:46 UTC
Comment on attachment 226 [details] Proposed patch 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. ::: Eigen/src/Geometry/AlignedBox.h @@ +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) \ > +EIGEN_USING_ALIGNEDBOX_TYPEDEFS_FOR_TYPE(cd) 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.
Comment 5 Jitse Niesen 2011-11-09 13:27:29 UTC
(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 > typedefs. I am committing this patch. Thanks.
Comment 6 Dennis Schridde 2011-11-09 22:15:25 UTC
Created attachment 228 [details] Improved patch 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.
Comment 7 Jitse Niesen 2011-11-16 15:35:10 UTC
I committed your patch (changeset 1c6393ab51e0). Thanks very much for your help.
Comment 8 Nobody 2019-12-04 11:14:33 UTC
-- GitLab Migration Automatic Message -- This bug has been migrated to gitlab.com's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.com/libeigen/eigen/issues/370.