Fix illegal memory acces in CUDA Octree builder

[ruslo]

Fixes: #2371

[kunaltyagi]

@ruslo Thanks for the nice fix 😄

Could you add a test which can be used to test that your fix works? The test will sadly not run on the CI, but will be available for improving the code to fix the second issue that you've reported (regarding normals not being NaN)

[ruslo]

Could you add a test which can be used to test that your fix works?

I've found few GPU tests here:

• https://github.com/PointCloudLibrary/pcl/tree/pcl-1.10.0/gpu/features/test

and added new case to test_normals.cpp.

The test will sadly not run on the CI

Yes, I've noticed that this C++ code is not tied to CMake. I did some hacks to run it on my side.

[kunaltyagi]

I did some hacks to run it on my side

Such as?

[ruslo]

I did some hacks to run it on my side

Such as?

It's expected that GTest is bundled, which is no longer true:

• https://github.com/PointCloudLibrary/pcl/blob/pcl-1.10.0/gpu/features/test/CMakeLists.txt#L5-L10

Sources are collected but not used anywhere:

• https://github.com/PointCloudLibrary/pcl/blob/pcl-1.10.0/gpu/features/test/CMakeLists.txt#L24-L27

I see that PCL using PCL_ADD_TEST wrapper for add_test. It can't be used
there because it needs custom tests target:

• https://github.com/PointCloudLibrary/pcl/blob/pcl-1.10.0/cmake/pcl_targets.cmake#L391

Which is created here:

• https://github.com/PointCloudLibrary/pcl/blob/pcl-1.10.0/test/CMakeLists.txt#L19

Since subdirectory gpu will be processed before subdirectory test this target will not be available:

• https://github.com/PointCloudLibrary/pcl/blob/pcl-1.10.0/CMakeLists.txt#L481-L483

Path to tested *.pcd file is hardcoded:

• https://github.com/PointCloudLibrary/pcl/blob/pcl-1.10.0/gpu/features/test/data_source.hpp#L82

Tested file office_chair_model.pcd is missing:

• https://github.com/PointCloudLibrary/pcl/tree/pcl-1.10.0/test

I found it, put it in the root folder and have to run the test from there, without using CTest.

I did some trivial tweaks to fix few places but most of the tests failing even without patch. List of tests I have to exclude:

PCL_FeaturesGPU.PrincipalCurvatures
PCL_FeaturesGPU.pfh_high_level1
PCL_FeaturesGPU.pfh_high_level2
PCL_FeaturesGPU.pfh_high_level3
PCL_FeaturesGPU.pfh_high_level4
PCL_FeaturesGPU.pfh_low_level
PCL_FeaturesGPU.pfhrgb
PCL_FeaturesGPU.ppf
PCL_FeaturesGPU.ppfrgb
PCL_FeaturesGPU.vfh1
PCL_FeaturesGPU.vfh_fill_size_component_true
PCL_FeaturesGPU.vfh_norm_bins_false
PCL_FeaturesGPU.vfh_norm_distance_true

[kunaltyagi]

I'll review the points raised and see what can be done to resolve them. Thanks @ruslo for your work 😄

[ruslo]

Build.Windows Failing after 94m
Build.Windows (Windows VS2017 Build x86) Failing after 94m

I don't think the failure is related to this change. @kunaltyagi Job restart needed?

[kunaltyagi]

@taketwo @SergioRAgostinho This changeset feels right to me after a dive in the octree module but I'm not confident.

[SergioRAgostinho]

Same opinion here. From my side, it's good to merge.

[ruslo]

Fixes: #2371

Line octree_builder.cu:403 was mentioned here too:

• GPU normal illegal memory access #3355
• Illegal memory access in gpu::octree #2103
• GPU octree error with big PointClouds.  #2008