This bugzilla service is closed. All entries have been migrated to https://gitlab.com/libeigen/eigen
Bug 370 - Eigen::AlignedBox3f typedef
Summary: Eigen::AlignedBox3f typedef
Status: RESOLVED FIXED
Alias: None
Product: Eigen
Classification: Unclassified
Component: Geometry (show other bugs)
Version: 3.0
Hardware: All All
: Normal Unknown
Assignee: Nobody
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-10-31 13:40 UTC by Dennis Schridde
Modified: 2019-12-04 11:14 UTC (History)
4 users (show)



Attachments
Proposed patch (2.72 KB, patch)
2011-11-08 14:38 UTC, Dennis Schridde
no flags Details | Diff
Patch to Matrix typedefs (726 bytes, patch)
2011-11-08 14:39 UTC, Dennis Schridde
no flags Details | Diff
Improved patch (3.98 KB, patch)
2011-11-09 22:15 UTC, Dennis Schridde
no flags Details | Diff

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.

Note You need to log in before you can comment on or make changes to this bug.