PointCloudLibrary / PointCloudLibrary/pcl

Adjust code to match current FLANN API

Open
#3,234 5 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

needs: author reply status: stale
Dominant language
C++
Stars
11.1k
Forks
4.7k
Avg merge
4d 10h
Merged PRs (30d)
6

Description

With FLANN 1.8 they switched from int to size_t. PCL is still using int, so there is always an implicit conversion.

I tried to fix it in context of CLang-Tidy warning performance-implicit-conversion-in-loop here:
https://github.com/PointCloudLibrary/pcl/blob/816391762918f3a026b1822fe42c74bb83129eb4/registration/include/pcl/registration/impl/ppf_registration.hpp#L108

But after this I had to change a lot of further PCL code, so I stopped my work after I had already ~100 lines adjusted, because I don't know if you want such a big change before release of 1.10.

Necessary adjustments:

 size_t  
 nearestKSearch (const PointT &point, size_t k,  
                 std::vector<size_t> &k_indices, std::vector<float> &k_sqr_distances) const override;
int 
      radiusSearch (const PointT &point, double radius, std::vector<size_t> &k_indices,
                    std::vector<float> &k_sqr_distances, int max_nn = 0) const override;

Some things you should notice about this:

  • max_nn is an int instead of unsigned int. We should think if we adjust meaning of 0, because -1 means to return all points in FLANN during I don't know what FLANN is doing with 0.
  • radiusSearch returns int during nearestKSearch returns size_t, as nearestKSearch just return k. Maybe we should not return k, but return value of knnSearch.

And in general: Increase required FLANN version in CMake from 1.7 to 1.8.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with kdtree/include/pcl/kdtree/kdtree_flann.h and the ppf_registration.hpp loop, then inspect the related PCL call sites affected by FLANN 1.8's size_t API. Decide the intended max_nn and search-result semantics, update the CMake minimum FLANN version, and confirm all affected declarations and uses are consistent.

Written by the indexing model from the issue text.

Assessment

Tech stack
cmake, cpp
Domain
build-system, computer-vision
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.