Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CMake] Add option to choose pcl::index_t while compiling #4166

Merged
merged 18 commits into from
Jun 10, 2020
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions cmake/pcl_options.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -64,3 +64,14 @@ set_property(GLOBAL PROPERTY USE_FOLDERS ON)
option(BUILD_tools "Useful PCL-based command line tools" ON)

option(WITH_DOCS "Build doxygen documentation" OFF)

# set index size
set(PCL_INDEX_SIZE -1 CACHE STRING "Set index size. Available options are: 8 16 32 64")
kunaltyagi marked this conversation as resolved.
Show resolved Hide resolved

#set whether indices are signed or unsigned
set(PCL_INDEX_SIGNED true CACHE BOOL "Set whether indices need to be signed or unsigned. Signed by default.")
if (PCL_INDEX_SIGNED)
set(PCL_INDEX_SIGNED_STR "true")
else()
set (PCL_INDEX_SIGNED_STR "false")
endif()
17 changes: 1 addition & 16 deletions common/include/pcl/types.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@
* \ingroup common
*/

#include <pcl/pcl_config.h>
#include <pcl/pcl_macros.h>

#include <vector>

#include <cstdint>
Expand All @@ -61,21 +61,6 @@ namespace pcl
using int64_t PCL_DEPRECATED(1, 12, "use std::int64_t instead of pcl::int64_t") = std::int64_t;
using int_fast16_t PCL_DEPRECATED(1, 12, "use std::int_fast16_t instead of pcl::int_fast16_t") = std::int_fast16_t;

// temporary macros for customization. Only use for PCL < 1.12
// Aim is to remove macros and instead allow multiple index types to coexist together
#ifndef PCL_INDEX_SIZE
#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 // PCL_INDEX_SIZE

#ifndef PCL_INDEX_SIGNED
#define PCL_INDEX_SIGNED true
#endif

namespace detail {
/**
* \brief int_type::type refers to an integral type that satisfies template parameters
Expand Down
16 changes: 16 additions & 0 deletions pcl_config.h.in
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,22 @@
#define PCL_VERSION_COMPARE(OP, MAJ, MIN, PATCH) \
(PCL_VERSION*10+PCL_DEV_VERSION OP PCL_VERSION_CALC(MAJ, MIN, PATCH)*10)

/* Index type and signed/unsigned property */
#define PCL_INDEX_SIGNED ${PCL_INDEX_SIGNED_STR}
#define PCL_INDEX_SIZE_TEMP ${PCL_INDEX_SIZE}

#if (PCL_INDEX_SIZE_TEMP > 0)
#define PCL_INDEX_SIZE ${PCL_INDEX_SIZE}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using PCL_INDEX_SIZE_TEMP might be better instead of ${PCL_INDEX_SIZE}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, yeah. Can we just remove the temporary variable in that case? It's sole use seems to be comparison with 0

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm unresolving the conversation, because PCL_INDEX_SIZE_TEMP still needs to be removed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 

Copy link
Member

@SergioRAgostinho SergioRAgostinho Jun 10, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's what I interpreted, because there's no need to actually define anything.

#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 //PCL_INDEX_SIZE_TEMP
kunaltyagi marked this conversation as resolved.
Show resolved Hide resolved
#undef PCL_INDEX_SIZE_TEMP

#cmakedefine HAVE_TBB 1
SergioRAgostinho marked this conversation as resolved.
Show resolved Hide resolved

#cmakedefine HAVE_OPENNI 1
Expand Down