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

Give users more control regarding with which point types classes are … #6062

Merged
merged 1 commit into from
Jun 18, 2024

Conversation

mvieth
Copy link
Member

@mvieth mvieth commented Jun 8, 2024

…instantiated

Previously, users could only choose between all types, some types ("core types" although it is not transparent which types these are), and no types (PCL_NO_PRECOMPILE). PCL_XYZ_POINT_TYPES and PCL_NORMAL_POINT_TYPES are now configurable via CMake (these only take effect if PCL_ONLY_CORE_POINT_TYPES and PCL_NO_PRECOMPILE are OFF). This enables users on all platforms and compilers to reduce PCL compile times and library size. Additionally, for users of MSVC or MINGW, PCL_ONLY_CORE_POINT_TYPES is now ON by default and not statically added to the definitions in CMakeLists.txt. I believe this is behaviour is more transparent, and these users now have the option to set PCL_ONLY_CORE_POINT_TYPES to OFF and choose the instantiated types via PCL_XYZ_POINT_TYPES and PCL_NORMAL_POINT_TYPES instead. However one of these two ways to reduce the set of instantiations must be used on Windows to avoid the linker error fatal error LNK1189: library limit of 65535 objects exceeded when trying to link the features library.

Fixes #5928

…instantiated

Previously, users could only choose between all types, some types ("core types" although it is not transparent which types these are), and no types (`PCL_NO_PRECOMPILE`).
`PCL_XYZ_POINT_TYPES` and `PCL_NORMAL_POINT_TYPES` are now configurable via CMake (these only take effect if `PCL_ONLY_CORE_POINT_TYPES` and `PCL_NO_PRECOMPILE` are OFF). This enables users on all platforms and compilers to reduce PCL compile times and library size.
Additionally, for users of MSVC or MINGW, `PCL_ONLY_CORE_POINT_TYPES` is now ON by default and not statically added to the definitions in CMakeLists.txt. I believe this is behaviour is more transparent, and these users now have the option to set `PCL_ONLY_CORE_POINT_TYPES` to OFF and choose the instantiated types via `PCL_XYZ_POINT_TYPES` and `PCL_NORMAL_POINT_TYPES` instead. However one of these two ways to reduce the set of instantiations must be used on Windows to avoid the linker error `fatal error LNK1189: library limit of 65535 objects exceeded` when trying to link the features library.
@mvieth mvieth added changelog: enhancement Meta-information for changelog generation module: cmake labels Jun 8, 2024
@mvieth
Copy link
Member Author

mvieth commented Jun 18, 2024

One addition: there currently seem to be some constraints regarding which point types can be removed from/need to be present in PCL_XYZ_POINT_TYPES and PCL_NORMAL_POINT_TYPES (depending on which PCL modules are built, and whether tests are built or not). Perhaps we can improve this in a future pull request. Currently, in my tests, PCL_NORMAL_POINT_TYPES=(pcl::Normal)(pcl::PointNormal) and PCL_XYZ_POINT_TYPES=(pcl::PointXYZ)(pcl::PointXYZI)(pcl::PointXYZL)(pcl::PointXYZRGB)(pcl::PointXYZRGBA)(pcl::PointXYZRGBL)(pcl::PointNormal)(pcl::PointXYZINormal)(pcl::PointXYZLNormal)(pcl::PointXYZRGBNormal)(pcl::PointWithRange) seem to work.

@mvieth mvieth merged commit b0b2519 into PointCloudLibrary:master Jun 18, 2024
13 checks passed
@mvieth mvieth deleted the configurable_instantiations branch June 18, 2024 12:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog: enhancement Meta-information for changelog generation module: cmake
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[compile error] "PCL_ONLY_CORE_POINT_TYPES not working as expected"
2 participants