PointCloudLibrary / pcl

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

[pcl_visualizer] "Allow custom definition of non-dense point cloud" #5748

Open thorsten-ottosen opened 1 year ago

thorsten-ottosen commented 1 year ago

When profiling, it seems that quite some time is being used to test if each point in a point cloud is valid. Of course, this is necessary for non-dense point clouds. However, it could be done faster.

This happens when calling updatePointCloud() and addPointCloud() which both calls convertPointCloudToVTKPolyData().

Then function tests non-density with

     // Check if the point is invalid
      if (!std::isfinite ((*cloud)[i].x) ||
          !std::isfinite ((*cloud)[i].y) ||
          !std::isfinite ((*cloud)[i].z))
        continue;

Here it would be useful to be able to inject a custom definition:

    // Check if the point is invalid
      if ( isInvalidPoint((*cloud)[i]) )
        continue;

For example, it may be enough for certain applications to test if the x-component is NaN to determine this. Today we get 3 tests per point and each may results in a non-inlined function call.

The downside is that each of the above three functions needs an additional template parameter.

I hope it is something that you will consider in the future.

kind regards

Thorsten

mvieth commented 1 year ago

Hi, thanks for the suggestion. PCL actually has functions to test the validity of a point (see https://github.com/PointCloudLibrary/pcl/blob/master/common/include/pcl/common/point_tests.h ). These should be used instead of using std::isfinite on every component, and would also make it easier to switch to a custom test. However, these function are not yet used consistently everywhere in PCL. Regarding testing only the x-component for NaN: this may work in many cases, but it is also completely possible that the x-component is valid, but y and z are not and make the function fail. Please also note the test for cloud->is_dense a few lines earlier. If the cloud is guaranteed to only contain valid points, the test can be skipped completely. If speed is important, I would recommend to use a filter or similar to remove all invalid points at the beginning of your pipeline, then all further algorithms don't have to check again.

thorsten-ottosen commented 1 year ago

Hi Markus,

You're right, testing the x-component is not always enough. But we know it is enough in our case. The other components may be tested in assertions if one feels insecure about the state of one's data.

For our application it is difficult to work with dense point clouds as we rely on grid properties and the fact that some points are NaNs. But you're right, it is always good if one can get to a state where one has a dense point cloud.

kind regards

Thorsten