PointCloudLibrary / pcl

Point Cloud Library (PCL)
https://pointclouds.org/
Other
9.86k stars 4.61k forks source link

[registration] Support use of search methods other than KdTreeFLANN #5619

Open themightyoarfish opened 1 year ago

themightyoarfish commented 1 year ago

The Registration class has a hardcoded assumption that the KdTree implementation is KdTreeFLANN. It uses an alias here:

https://github.com/PointCloudLibrary/pcl/blob/master/registration/include/pcl/registration/registration.h#L70

and does not expose the second template parameter to the KdTree base class, which then defaults to KdTreeFLANN

https://github.com/PointCloudLibrary/pcl/blob/master/search/include/pcl/search/kdtree.h#L60

Is it possible to support other search methods?

Context

I am currently trying unsuccessfully to shoehorn OrganizedNeighbor into my registration pipeline, since building the KdTree can take a lot of time for each new cloud, and mine are organized.

Expected behavior

Registration classes work with any search method exposing nearestKSearch() and radiusSearch() methods.

Current Behavior

Registration can only use KdTreeFLANN

Describe the solution you'd like

Encapsulating the use of setPointRepresentation() in some registration classes, and make the search method configurable with setSearchMethodSource(), setSearchMethodTarget().

Describe alternatives you've considered

n/a

Additional context

n/a

mvieth commented 1 year ago

It could make sense to use pcl::search::Search instead of pcl::search::KdTree, then other search methods such as organized search and octree would be possible. But I am not sure what to do with the point representation then, and I won't have time to look into this in the near future. If you manage to get something working, feel free to describe your solution here and/or open a pull request.

themightyoarfish commented 1 year ago

I am a bit confused anyway why the Registration class has methods to set the search method, while that should be the CorrespondenceEstimations job, shouldn't it? It seems that all Registration does with the point representation, is pass it to its tree_ kdTree member. I don't really get why that tree member exists, given it is used to set the search method on the correspondence estimation. Registration uses the tree to compute the fitness score and inform the search method for correspondence estimation, but I feel like the former could be part of the transformation estimation and the latter can just be set by the user if desired.

mvieth commented 1 year ago

I am a bit confused anyway why the Registration class has methods to set the search method, while that should be the CorrespondenceEstimations job, shouldn't it?

This might be a convenience feature (partially?)

It seems that all Registration does with the point representation, is pass it to its tree_ kdTree member. I don't really get why that tree member exists, given it is used to set the search method on the correspondence estimation.

Registration's tree_ member seems to also be used in some subclasses of Registration, e.g. ia_fpcs, ia_ransac, ia_kfpcs, sample_consensus_prerejective (mostly in functions that compute some kind of fitness or error metric, maybe there is a way to sensibly combine them with Registration::getFitnessScore?).

Registration uses the tree to compute the fitness score and inform the search method for correspondence estimation, but I feel like the former could be part of the transformation estimation and the latter can just be set by the user if desired.

I think having a function that computes a fitness score/error metric, independently from the transformation estimation, makes sense. So you can e.g. call it after the registration to check the result.