[Features] our_cvfh use own impl of extract clusters

[larshg]

Describe the bug

OURCVFHEstimation used its own impl of extractEuclideanClustersSmooth (ie. using normals also) See: https://github.com/PointCloudLibrary/pcl/blob/70047873577e4d9b6d6d6e675eafaff3216492bc/features/include/pcl/features/impl/our_cvfh.hpp#L73-L157

Context

Looking into the various cluster algorithms.

Expected behavior

Let our_cvfh use the cluster algorithms in segmentation module.

Current Behavior

It has its own implementation which doesn't get fixed. ie. latest commit about abs/acos to the dot_p.

Your Environment (please complete the following information):

• OS: Windows
• Compiler: VS2019
• PCL Version Head

Possible Solution

Remove the local implementation and use the segmentation method from the segmentation module.

Additional context

Wonder if there are other places where this happens or something similiar😄

[kunaltyagi]

Remove the local implementation and use the segmentation method from the segmentation module

Are the implementations significantly similar (or dare I say: same)?

[larshg]

Except from the bug fix it is the same 😄 Unless I have missed some, but I don't think so. Even the comments are exact the same, so it seems like a direct copy.

[larshg]

Ahh.. had another look: It actually seems to include the fix which is proposed here: #3842 ie.

double dot_p = normals.points[seed_queue[sq_idx]].normal[0] * normals.points[nn_indices[j]].normal[0]
            + normals.points[seed_queue[sq_idx]].normal[1] * normals.points[nn_indices[j]].normal[1] + normals.points[seed_queue[sq_idx]].normal[2]
            * normals.points[nn_indices[j]].normal[2];

which is not yet present in the current extract_clusters in the segmentation module.

[kunaltyagi]

That fix is in another PR IIRC

If this is the case, I'd say: go ahead and merge the 2 implementations