Skip to content

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

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.

Sign up for GitHub

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

Merged
merged 18 commits into from
Jun 10, 2020
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
3b11a2a
add cmake variables
haritha-j Jun 4, 2020
b78a2ad
add cmake variables
haritha-j Jun 4, 2020
2810fda
add cmake variables
haritha-j Jun 4, 2020
60cc20f
add index size options
haritha-j Jun 4, 2020
c8efa4d
add index size options
haritha-j Jun 4, 2020
08e6a73
update pcl_config
haritha-j Jun 4, 2020
0a9670a
fix pcl_config
haritha-j Jun 4, 2020
3922340
Delete settings.json
haritha-j Jun 4, 2020
9000c58
add cmake integration for indices type
haritha-j Jun 5, 2020
f10d8c2
Merge remote-tracking branch 'origin/indices' into indices
haritha-j Jun 5, 2020
1910570
rename PCL_INDEX_SIGNED
haritha-j Jun 5, 2020
6861e24
add HAVE_TBB
haritha-j Jun 5, 2020
3ec2691
change default behalt behaviour
haritha-j Jun 6, 2020
79e7e2b
undefine unused variable
haritha-j Jun 7, 2020
7257576
update cmake help string
haritha-j Jun 9, 2020
dd9a404
Update cmake/pcl_options.cmake
haritha-j Jun 9, 2020
7912d9f
remove PCL_INDEX_SIZE_TEMP
haritha-j Jun 10, 2020
9059add
Merge branch 'indices' of https://github.com/haritha-j/pcl into indices
haritha-j Jun 10, 2020
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
View file
Open in desktop
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. A negative value indicates default size (32 for PCL >= 1.12, 8*sizeof(int) i.e., the number of bits in int, otherwise)")

#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()
24 changes: 4 additions & 20 deletions common/include/pcl/types.h
View file
Open in desktop
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 Expand Up @@ -112,17 +97,16 @@ namespace pcl
/**
* \brief number of bits in PCL's index type
*
* For PCL 1.11, please use PCL_INDEX_SIZE to choose a size best suited for your needs.
* PCL 1.12 will come with default 32, along with client code compile time choice
* Please use PCL_INDEX_SIZE when building PCL to choose a size best suited for your needs.
* PCL 1.12 will come with default 32
*
* PCL 1.11 has a default size = sizeof(int)
*/
constexpr std::uint8_t index_type_size = PCL_INDEX_SIZE;

/**
* \brief signed/unsigned nature of PCL's index type
* For PCL 1.11, please use PCL_INDEX_SIGNED to choose a type best suited for your needs.
* PCL 1.12 will come with default signed, along with client code compile time choice
* Please use PCL_INDEX_SIGNED when building PCL to choose a type best suited for your needs.
* Default: signed
*/
constexpr bool index_type_signed = PCL_INDEX_SIGNED;
Expand Down
14 changes: 14 additions & 0 deletions pcl_config.h.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,20 @@
#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}

#if (${PCL_INDEX_SIZE} > 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 •
edited
Loading

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

#cmakedefine HAVE_TBB 1

#cmakedefine HAVE_OPENNI 1
Expand Down