[CMake] Add option to choose pcl::index_t while compiling
#4166
Conversation
pcl_config.h.in
Outdated
|
|
||
| #cmakedefine HAVE_OPENNI2 1 | ||
|
|
||
|
|
cmake/pcl_options.cmake
Outdated
| option(WITH_DOCS "Build doxygen documentation" OFF) | ||
|
|
||
| # set index size | ||
| set(PCL_SIZE_INDEX 32 CACHE STRING "Set index size. Available options are: 8 16 32 64") |
There was a problem hiding this comment.
Please read the note in types.h regarding the default size. It can't be 32 for PCL 1.11 due to compatibility. It can be 32 for PCL 1.12 since the user can then override the size at compile time
There was a problem hiding this comment.
Would modifying types.h to be
#if PCL_MINOR_VERSION <= 11
// sizeof returns bytes, while we measure size by bits in the template
#undef PCL_INDEX_SIZE
#define PCL_INDEX_SIZE (sizeof(int) * 8)
#else
#ifndef PCL_INDEX_SIZE
#define PCL_INDEX_SIZE 32
#endif // PCL_INDEX_SIZE
#endif // PCL_MINOR_VERSIONbe an option or should the changes be made solely in the cmake configuration?
There was a problem hiding this comment.
You can modify anything as long as it keeps the behavior (and is sane) 😄
|
Maybe squash the commits to a single commit :) ? |
|
@kunaltyagi I moved the version check for changing indices type to the config file, so that all of the configuration happens in one place. |
| // sizeof returns bytes, while we measure size by bits in the template | ||
| #define PCL_INDEX_SIZE (sizeof(int) * 8) | ||
| #if (PCL_INDEX_SIZE_TEMP > 0) | ||
| #define PCL_INDEX_SIZE ${PCL_INDEX_SIZE} |
There was a problem hiding this comment.
Using PCL_INDEX_SIZE_TEMP might be better instead of ${PCL_INDEX_SIZE}
There was a problem hiding this comment.
It looks like I can't follow both of these suggestions together. If I undefine PCL_INDEX_SIZE_TEMP, then at compile time PCL_INDEX_SIZE cannot be set to it, because it is no longer available.
There was a problem hiding this comment.
Oh, yeah. Can we just remove the temporary variable in that case? It's sole use seems to be comparison with 0
There was a problem hiding this comment.
I'm unresolving the conversation, because PCL_INDEX_SIZE_TEMP still needs to be removed.
There was a problem hiding this comment.
My bad, I thought by remove @kunaltyagi meant I should undef PCL_INDEX_SIZE_TEMP, not get rid of it altogether.
Is this what you had in mind?
#if (${PCL_INDEX_SIZE} > 0)
#define PCL_INDEX_SIZE ${PCL_INDEX_SIZE}
#else
#if PCL_MINOR_VERSION <= 11
// sizeof returns bytes, while we measure size by bits in the template
#define PCL_INDEX_SIZE (sizeof(int) * 8)
#else
#define PCL_INDEX_SIZE 32
#endif //PCL_MINOR_VERSION
#endif There was a problem hiding this comment.
That's what I interpreted, because there's no need to actually define anything.
Co-authored-by: Sérgio Agostinho <sergio.r.agostinho@gmail.com>
kunaltyagi
added
the
changelog: enhancement
kunaltyagi
changed the title
pcl::index_t while compiling
4 participants
Add cmake configuration to change indices type and sign.