possible bug or documentation issue with picked indices for non-dense clouds in PCLVisualizer

[martyvona]

What seemed to be happening in our case is that we were adding a point cloud
to the visualizer that contained some NaN points, i.e. I think in the PCL
terminology it was not a "dense" point cloud.

The visualizer seems to remove such invalid points internally. The indices
that are reported for picks (I think this applies to both single point and
area picks) refer to the point cloud after this mutation.

We are working around the issue with code like this (this is a
generalization of our actual code that I did not test):

vector &indices; //picked indices from visualizer

sort(indices.begin(), indices.end());

const int m = indices.size(), n = cloud.points.size();

//i is index in original cloud
//j is index in virtual sparse cloud
//k is index in indices
for (int i = 0, j = 0, k = 0; (i < n) && (k < m); i++) {
if (isFinite(cloud.points[i])) {
while ((k < m) && (indices[k] == j)) indices[k++] = i;
j++;
}
}

[taketwo]

You may consider using the function removeNaNFromPointCloud() to remove NaN points from the point cloud before displaying.

Although I would agree that this (non-obvious) behavior deserves a mention in the documentation.

[nizar-sallem]

This is due to the fact that NaN points can not really be picked from a 3D point of view: coordinates doesn't exist.
You can get their cell though if you want but you would need to rewrite the associated event.

[martyvona]

Hi -

The point here is that it might be considered reasonable for users to expect that picked point indices reported by PCLVisualizer actually correspond to indices in the cloud they added to it. Let's tentatively call this "the desired behavior". This is not about users expecting to be able to pick NaN points. Calling removeNaNFromPointCloud() is one way to achieve the desired behavior, but (a) the necessity of doing something like that might be documented and (b) users who have non-dense (for example, organized) point clouds may still want to know the indices of the picked points in their original NaN-containing clouds.

Best,
Marty

[jpapon]

This is probably due to how PCLVisualizer adds the points to VTK internally - I believe it simply skips NAN points.

I think this is something that should be added into the documentation, rather than changed in PCLVisualizer, since as far as I can see it would require wasting memory storing NAN points in the visualizer.

[taketwo]

There is a version of removeNaNFromPointCloud() that does a dry-run which only computes the mapping between indices of "full" and "nan-free" point clouds. PCLVisualizer may run it internally when a point cloud is added and store the map. When a point is clicked it would return the index that user expects by simply looking it up in the map.

The question is if this could break any existing code which relies on the current behavior of PCLVisualizer.

[jpapon]

Right, but then you would have to store a mapping for every point added to the visualizer - that seems pretty wasteful for something that can be avoided by simply sanitizing the input.

[taketwo]

Well if user preferred not to sanitize the input then he has to face the consequences. On the other hand, of course, not every user that visualizes an organized point cloud actually needs point-clicking...

[martyvona]

It looks to me like PCLVisualizer is already making a copy of all non-nan point coords, so you are already using at least 12 bytes per point just for visualization (this is in PCLVisualizer::convertPointCloudToVTKPolyData()). Very likely quite a few more bytes are also used per point for the VTK datastructures. You could probably implement the "desired behavior" with an additional 4 bytes storage per point (and this is still only for non-nan points). And probably someone who understands the VTK datastructures and the implementation of the point picking callbacks could do it in ~10 LoC or less.

Regarding whether some users would prefer not to "sanitize" their point clouds, perhaps a better way to put it is that some users might prefer not to "disorganize" their point clouds. While it is true that not all people trying to visualize organized clouds care about point picking, if you tell people that PCLVisualizer implements picking then of course you should expect people will start to use it with that capability in mind. If in fact that capability is broken or does not work as could be reasonably expected for organized clouds, then you might want to either fix it or let people know that beforehand.

Good luck.

[jpapon]

Wouldn't it be best to direct people in the documentation to the pp_callback in pcd_viewer.cpp?
It shows that there isn't a 1:1 correspondence and does a nearest K search for the closest point in the original cloud.

[martyvona]

pp_callback in pcd_viewer.cpp

That is some surprising code. I'm pretty sure that there is no guarantee of spatial locality between the indices reported by point picking events for nan-containing clouds and the indices of the actual picked points in the original cloud. (Also that code doesn't seem to consistently handle the two cases of getting a single point vs two points from the pick event. And there is the area pick event to consider as well.)

Not to say that this is a particularly hard problem to solve. The workaround I originally posted is fine for me, though it's O(N) for each pick where N is the original point cloud size. A variation on the idea could be made that works in O(M) per pick using a lookup table where M is the number of picked indices, at memory cost O(N). Though if the visualization datastructures could be amended to store the actual original cloud index for each non-nan point then that could solve it for everyone in O(1) time with O(K) additional storage where K is the number of non-nan points in the cloud.

[jpapon]

I'm pretty sure that there is no guarantee of spatial locality between the indices reported by point picking events for nan-containing clouds and the indices of the actual picked points in the original cloud.

It's not searching by index though - it's searching for the point in the original cloud closest to the selected point. Assuming the selected point was in the original cloud, it will always return the correct index. It doesn't matter if the points in the cloud's vector are ordered strangely.

I don't see any problem with this solution - if the point picked was in the original cloud, the search will return the correct index. Granted, it's not the most efficient method (since you need to build a KD-tree), but it only affects point picking, and I'm not sure we should be worrying about the efficiency of point picking.

On the other hand, I'd be concerned about any method that would increase load in all use cases of PCLVisualizer just to make point picking a little better. Lots of people are using PCLVisualizer to show clouds at 30FPS - it's unfair to add additional load for them just to make point picking easier. Keeping track of NAN points could easily mean 100k extra points per frame for Kinect data - that's a lot of extra overhead at 30FPS.

[rbrusu]

Is there a follow up to this issue?

[SergioRAgostinho]

This can probably be closed.