PointCloudLibrary / pcl

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

SuperVoxel Clustering min threshold bug #622

Closed phil0stine closed 7 years ago

phil0stine commented 10 years ago

I could do a pull request, but I figure this is so simple I can just explain it here.

From the VCCS paper, filtering of sparse seed points is achieved by

establishing a small search radius around each seed, and deleting seeds which do not have at least as many voxels as would be occupied by a planar surface intersecting with half of the search volume

In supervoxel_clustering.hpp, lines 402-405, min_points is being set to be 1/20th of the intersecting area, not 1/2, by the 0.05f factor:

  float search_radius = 0.5f*seed_resolution_;
  // This is number of voxels which fit in a planar slice through search volume
  // Area of planar slice / area of voxel side
  float min_points = 0.05f * (search_radius)*(search_radius) * 3.1415926536f / (resolution_*resolution_);

The last line should instead read :

float min_points = 0.5f * (search_radius)*(search_radius) * 3.1415926536f / (resolution_*resolution_);
jpapon commented 10 years ago

Yes, this actually makes it so only seeds in very sparse areas are deleted. I know it doesn't exactly match the version from the paper, but at some point I decided that it was better to keep most supervoxel seeds, as it tended to work better for many scenes. The version in PCL also doesn't use CieLab color space, but this was also intentional - the conversion added to runtime while giving very little performance increase.

The seeding strategy should probably be completely revised at some point - here it just divides the space and removes isolated areas. It would be better to avoid placing seeds on edges.

phil0stine commented 10 years ago

at some point I decided that it was better to keep most supervoxel seeds, as it tended to work better for many scenes.

Fair enough, it just looked suspiciously like an extra zero might have been accidentally inserted. It might make sense to give the user some control over this factor as a parameter.

Thanks for the heads up on the color space, I noticed that the PCL version doesn't make use of the FPFH, which presumably was left out for the same reason.

I think the strategy of dividing the space evenly for seeding makes sense, but perhaps adding small set of additional filters would be sufficient.

Also, I wonder if "bad seeds" could be determined by the resulting supervoxels they produce. ie, if a seed hasnt grown much (or at all), it shouldn't still be used to represent a supervoxel.

jpapon commented 10 years ago

Thanks for the heads up on the color space, I noticed that the PCL version doesn't make use of the FPFH, which presumably was left out for the same reason.

Yeah, sorry I forgot to mention that as well.

Also, I wonder if "bad seeds" could be determined by the resulting supervoxels they produce. ie, if a seed hasnt grown much (or at all), it shouldn't still be used to represent a supervoxel.

This is a great idea! A seed pruning step would definitely be useful. I'll play around with it this afternoon.

jspricke commented 10 years ago

I would propose to add a comment to the line in question.