[filters/VoxelGrid] Provide a way to override CentroidPoint or add my own Accumulators

[jackjansen]

Context

I have defined my own points, with x, y, z and color, and an extra field that is essentially a bit mask. I have been using this for years, and it works fine everywhere, with the exception of VoxelGrid.

VoxelGrid uses CentroidPoint to combine points, and it uses a set of predefined Accumulators to combine the fields within points according to the correct semantics for those fields.

This works fine for coordinates, normals, colours and a few other attribute types that pcl support, but there does not appear to be a way to extend the field types supported by supplying my own Accumulator type for my bit mask fields (which would is essence do a bitwise or on the fields in the incoming points).

With the current code structure it is impossible to override any of the implementations without copying everything up to VoxelGrid into my own code, which seems a bit gross.

Expected behavior

I would like to have full control over the way attributes of my points are treated, especially attributes that pcl doesn't know about as distributed because they are specific to my usage, but I can also see use cases for enabling use of different combination strategies for "standard" attributed like position or color.

Current Behavior

Currently I keep the leaf layout and then make an extra pass over the source points, call getCentroidIndex() and then manually modify the field.

Describe the solution you'd like

If I could override CentroidPoint when instantiating VoxelGrid I would only need to subclass CentroidPoint to do what I want. And I think it would allow all sorts of potentially useful other uses as well (for example create a CentroidPoint that uses a completely different way of combining points. I can think of random selection, for example, or modifying some attributes based on the values of some other attributes.

Describe alternatives you've considered

An alternative that would work for my use case is if I could add my own Accumulator implementation to the Accumulator.types list, but I'm not sure that there is a construct in C++ that would allow that.

Additional context

Add any other context or screenshots about the feature request here.

[mvieth]

Hi, your desire to customize the centroid computation is totally understandable. Looking at the way that CentroidPoint is currently used in VoxelGrid, I guess it would be necessary to add a CentroidPoint member in VoxelGrid that the user can replace via a setter-method before calling filter. But CentroidPoint, as well as all existing accumulators, would then also require a new method that resets the internal state. Additionally, to perhaps be able to parallelize VoxelGrid in the future, it would be nice to have some method to clone a CentroidPoint (so that each thread has its own CentroidPoint).

An alternative idea, that changes less of the existing code, could be to give the user the option to pass a function to VoxelGrid, which perhaps receives a point cloud and a vector of indices (the points to use for the centroid computation), and returns a single point (the centroid). This approach seems less invasive to me, while still giving total freedom to the user. Let me know what you think.

[jackjansen]

Your alternative idea sounds feasible, indeed. But wouldn't the signature of this function would become a bit clunky? Because you'll have to pass in input_, index_vector, first_index and last_index, and moreover index_vector is of a type that appears to be internal.... OTOH simply copying index_vector[first_index..lastindex] to a new temporary std::vector<int> isn't all that expensive and would make the signature a lot cleaner....

[jackjansen]

Oh, one more remark, sightly off-tangent, triggered by your mention of possible future parallelization of VoxelGrid: in my code I now use a mix of Octree for the higher levels of the point cloud and then VoxelGrid for the lower levels. For large point clouds this is orders of magnitude more space-efficient than using only VoxelGrid, at a small cost for runtime-efficiency. The resulting point cloud isn't 100% identical (because of the non-overlapping VoxelGrid matrices) but it was good enough for me. This might be a rather easy entry point for parallelisation....

The idea for this came from https://github.com/PointCloudLibrary/pcl/issues/5498 (credit where it is due:-). The implementation (which is not reusable for the general case directly because its tailored to my point data structures and more, but for reference) is at https://github.com/cwi-dis/cwipc_util/blob/9fb8f67dc722519c7f466769114938351ef123ae/src/cwipc_filters.cpp#L92

[mvieth]

Your alternative idea sounds feasible, indeed. But wouldn't the signature of this function would become a bit clunky? Because you'll have to pass in input_, index_vector, first_index and last_index, and moreover index_vector is of a type that appears to be internal.... OTOH simply copying index_vector[first_index..lastindex] to a new temporary std::vector<int> isn't all that expensive and would make the signature a lot cleaner....

Yes, that was my plan: using a function signature like void func(const pcl::PointCloud<PointT>&, const pcl::Indices&, PointT&) where pcl::Indices is by default identical to std::vector<int> but can be configured (when building PCL) to be a vector of some other index type. The last parameter would be the resulting centroid point.

Oh, one more remark, sightly off-tangent, triggered by your mention of possible future parallelization of VoxelGrid: in my code I now use a mix of Octree for the higher levels of the point cloud and then VoxelGrid for the lower levels. For large point clouds this is orders of magnitude more space-efficient than using only VoxelGrid, at a small cost for runtime-efficiency. The resulting point cloud isn't 100% identical (because of the non-overlapping VoxelGrid matrices) but it was good enough for me. This might be a rather easy entry point for parallelisation....

The idea for this came from #5498 (credit where it is due:-). The implementation (which is not reusable for the general case directly because its tailored to my point data structures and more, but for reference) is at https://github.com/cwi-dis/cwipc_util/blob/9fb8f67dc722519c7f466769114938351ef123ae/src/cwipc_filters.cpp#L92

Did you choose this approach because VoxelGrid was not space-efficient enough, or for some other reason?

[jackjansen]

My main reason is that I was running into the index limit of VoxelGrid (my point clouds are humans captured by four to eight RGBD cameras, 1M-2Mpoints in a space of say 1000*1000*2000mm but can be larger if there are more humans in the space). I usually want to downsample these to about 500Kpoints, which is a sweet spot for latency-versus-quality after compression.

So the limit of the indices was the primary reason, but the space efficiency definitely a secondary one.