Made it so that PointCloudColorHandlerRGBField uses pcl::getFieldInde…

[indianajohn-aemass]

…x, allowing PointCloudColorHandlerRGBField to be instantiated by clouds without RGB fields. In the absence of RGB fields, clouds are colored white. This was done to prevent a compilation error for the next change: made it so that PCLVisualizer::addPointCloud(cloud) uses a PointCloudColroHandlerRGBField by default if an RGB field is present in the custom point type. If an RGB field is not present, the previous behavior prevails. Previously, if addPointCloud(cloud) were called, a cryptic runtime error stating that the RGB field was not capable was given; these changes seem to implement a more intuitive behavior.

To reproduce the erroneous behavior in the previous versions of PCL, create a custom point type with fields identical to pcl::PointXYZRGBA and add try the following snippet:

PCLVisualizer myVisualizer;
...
pcl::PointCloud<MyCustomPointType>::Ptr cloud;
...
myVisualizer.addPointCloud<MyCustomPointType>(cloud,"test");

The above code results in a compilation error.

[taketwo]

I think this is a reasonable change in default behavior. I have a couple of points though:

• In the addPointCloud function we could as well check if rgba field exists and instantiate corresponding color handler;
• In the getColors function you have else cases for the situation "If no RGB field is available, color the point white". This should never happen anyway, because if there is no RGB field, then capable_ will be set to false, and the function will return immediately.

[indianajohn-aemass]

Hi Sergey,

I addressed your comments in subsequent commit 0f5f3ff. Let me know if I can be of further help.

John

[taketwo]

Looking at the getColors again, I don't really understand why you need to access color data through field index offset. Could you please explain why the old code (accessing r, g, and b fields separately) doesn't work?

[indianajohn-aemass]

addPointCloud (cloud,label) is instantiated by some examples
(and likely users code), and if I make the changes I made to
pcl_visualizer.hpp without changing point_cloud_color_handlers.hpp, the
accessors cloud.r, cloud.g, and cloud.b cause compile time errors for that
template instantiation. I'm phone posting now but I can give you the exact
error in a few hours.
On Aug 12, 2015 2:22 AM, "Sergey Alexandrov" [email protected]
wrote:

Looking at the getColors again, I don't really understand why you need to
access color data through field index offset. Could you please explain why
the old code (accessing r, g, and b fields separately) doesn't work?

—
Reply to this email directly or view it on GitHub
#1295 (comment)
.

[indianajohn-aemass]

As promised, here is the error message for the build that includes the changes to pcl_visualizer.hpp but not point_cloud_color_handlers.hpp (Clang on MacOSX Yosemite):

[ 44%] Building CXX object visualization/CMakeFiles/pcl_visualization.dir/src/pcl_visualizer.cpp.o
In file included from /Users/aemass/dev/pcl/visualization/src/pcl_visualizer.cpp:97:
In file included from /Users/aemass/dev/pcl/visualization/include/pcl/visualization/pcl_visualizer.h:48:
In file included from /Users/aemass/dev/pcl/visualization/include/pcl/visualization/common/actor_map.h:40:
In file included from /Users/aemass/dev/pcl/visualization/include/pcl/visualization/point_cloud_handlers.h:42:
In file included from /Users/aemass/dev/pcl/visualization/include/pcl/visualization/point_cloud_color_handlers.h:947:
/Users/aemass/dev/pcl/visualization/include/pcl/visualization/impl/point_cloud_color_handlers.hpp:165:42: error: no member named 'r' in 'pcl::PointXYZ'
      colors[j    ] = cloud_->points[cp].r;
                      ~~~~~~~~~~~~~~~~~~ ^
/Users/aemass/dev/pcl/visualization/include/pcl/visualization/impl/pcl_visualizer.hpp:93:44: note: in instantiation of member function
      'pcl::visualization::PointCloudColorHandlerRGBField<pcl::PointXYZ>::getColor' requested here
    PointCloudColorHandlerRGBField<PointT> color_handler_rgb_field (cloud);
                                           ^
/Users/aemass/dev/pcl/visualization/include/pcl/visualization/impl/pcl_visualizer.hpp:75:11: note: in instantiation of function template specialization
      'pcl::visualization::PCLVisualizer::addPointCloud<pcl::PointXYZ>' requested here
  return (addPointCloud<PointT> (cloud, geometry_handler, id, viewport));
          ^
/Users/aemass/dev/pcl/visualization/include/pcl/visualization/pcl_visualizer.h:868:19: note: in instantiation of function template specialization
      'pcl::visualization::PCLVisualizer::addPointCloud<pcl::PointXYZ>' requested here
          return (addPointCloud<pcl::PointXYZ> (cloud, id, viewport));
                  ^
In file included from /Users/aemass/dev/pcl/visualization/src/pcl_visualizer.cpp:97:
In file included from /Users/aemass/dev/pcl/visualization/include/pcl/visualization/pcl_visualizer.h:48:
In file included from /Users/aemass/dev/pcl/visualization/include/pcl/visualization/common/actor_map.h:40:
In file included from /Users/aemass/dev/pcl/visualization/include/pcl/visualization/point_cloud_handlers.h:42:
In file included from /Users/aemass/dev/pcl/visualization/include/pcl/visualization/point_cloud_color_handlers.h:947:
/Users/aemass/dev/pcl/visualization/include/pcl/visualization/impl/point_cloud_color_handlers.hpp:166:42: error: no member named 'g' in 'pcl::PointXYZ'
      colors[j + 1] = cloud_->points[cp].g;
                      ~~~~~~~~~~~~~~~~~~ ^
/Users/aemass/dev/pcl/visualization/include/pcl/visualization/impl/point_cloud_color_handlers.hpp:167:42: error: no member named 'b' in 'pcl::PointXYZ'
      colors[j + 2] = cloud_->points[cp].b;
                      ~~~~~~~~~~~~~~~~~~ ^

Is there maybe a better way to address this error? My change seems to work and I can't think of any side effects that it could have.

[taketwo]

Sorry for the delay. Okay, the compile error actually makes sense. One other possible way to get over it is to use SFINAE to prevent instantiation of PointCloudColorHandlerRGBField for point types without color... But I think your solution looks better.

By the way, there is a meta-function specifically to check if there is rgb or rgba field, it's called has_color. We can use it in the addPointCloud function.

Finally, can you squash the commits into one? Then I'll merge.

[indianajohn-aemass]

Sergey,

I changed pcl::has_field<PointT,rgba/rgb> to pcl::has_color in addPointCloud, updated my fork, and incorporated all of my changes into a single commit. It should be ready to merge. Thanks for keeping the torch lit on a library that I personally get a lot of use out of, and have a good day.

[taketwo]

Thanks for contributing and for your kind words :) Have a nice day as well.