PointCloudLibrary / pcl

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

CorrespondenceEstimation with different point types #329

Open aichim opened 11 years ago

aichim commented 11 years ago

Hi guys,

I think I just opened a can of worms in the CorrespondenceEstimation class and it's quite annoying.

So, I am trying to do the following:

  pcl::registration::CorrespondenceEstimation<PointXYZ, pcl::PointNormal> corresp_est;

This initially gave me an error of not being able to convert from PointXYZ to PointNormal in the PCL code, so I hunted for bugs and did the following changes:


diff --git a/registration/include/pcl/registration/correspondence_estimation.h b/registration/include/pcl/registration/correspondence_estimation.h
index ec5422c..ef94383 100644
--- a/registration/include/pcl/registration/correspondence_estimation.h
+++ b/registration/include/pcl/registration/correspondence_estimation.h
@@ -76,7 +76,7 @@ namespace pcl
         typedef typename KdTree::Ptr KdTreePtr;

         typedef pcl::search::KdTree<PointSource> KdTreeReciprocal;
-        typedef typename KdTree::Ptr KdTreeReciprocalPtr;
+        typedef typename KdTreeReciprocal::Ptr KdTreeReciprocalPtr;

         typedef pcl::PointCloud<PointSource> PointCloudSource;
         typedef typename PointCloudSource::Ptr PointCloudSourcePtr;
diff --git a/registration/include/pcl/registration/impl/correspondence_estimation.hpp b/registration/include/pcl/registration/impl/correspondence_estimation.hpp
index 4b02d68..4c2fb8c 100644
--- a/registration/include/pcl/registration/impl/correspondence_estimation.hpp
+++ b/registration/include/pcl/registration/impl/correspondence_estimation.hpp
@@ -142,7 +142,7 @@ pcl::registration::CorrespondenceEstimation<PointSource, PointTarget, Scalar>::d

   // Check if the template types are the same. If true, avoid a copy.
   // Both point types MUST be registered using the POINT_CLOUD_REGISTER_POINT_STRUCT macro!
-  if (isSamePointType<PointSource, PointTarget> ())
+  /*if (isSamePointType<PointSource, PointTarget> ())
   {
     // Iterate over the input set of source indices
     for (std::vector<int>::const_iterator idx = indices_->begin (); idx != indices_->end (); ++idx)
@@ -157,7 +157,7 @@ pcl::registration::CorrespondenceEstimation<PointSource, PointTarget, Scalar>::d
       correspondences[nr_valid_correspondences++] = corr;
     }
   }
-  else
+  else*/
   {
     PointTarget pt;

@@ -212,7 +212,7 @@ pcl::registration::CorrespondenceEstimation<PointSource, PointTarget, Scalar>::d

   // Check if the template types are the same. If true, avoid a copy.
   // Both point types MUST be registered using the POINT_CLOUD_REGISTER_POINT_STRUCT macro!
-  if (isSamePointType<PointSource, PointTarget> ())
+  /*if (isSamePointType<PointSource, PointTarget> ())
   {
     // Iterate over the input set of source indices
     for (std::vector<int>::const_iterator idx = indices_->begin (); idx != indices_->end (); ++idx)
@@ -233,7 +233,7 @@ pcl::registration::CorrespondenceEstimation<PointSource, PointTarget, Scalar>::d
       correspondences[nr_valid_correspondences++] = corr;
     }
   }
-  else
+  else*/
   {
     PointTarget pt_src;
     PointSource pt_tgt;

In the diff above you will also see that I took out the parts where we consider the point types to be the same. I think these tests are wrong, as they are runtime tests, and the compiler is going to compile everything anyway, and will produce errors.

After these changes, I am now getting pages and pages of errors of the form:

/usr/local/include/pcl-1.7/pcl/point_traits.h:137:12: note: definition of 'pcl::traits::datatype<pcl::_PointXYZ,
      pcl::fields::curvature>' is not complete until the closing '}'
    struct datatype : datatype<typename POD<PointT>::type, Tag>

/usr/local/include/pcl-1.7/pcl/registration/impl/correspondence_estimation.hpp:168:7: note: in instantiation of function template
      specialization 'pcl::for_each_type<boost::mpl::vector<pcl::fields::x, pcl::fields::y, pcl::fields::z, pcl::fields::normal_x,
      pcl::fields::normal_y, pcl::fields::normal_z, pcl::fields::curvature, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na,
      mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na>, pcl::NdConcatenateFunctor<pcl::PointXYZ, pcl::PointNormal>
      >' requested here
      pcl::for_each_type <FieldListTarget> (pcl::NdConcatenateFunctor <PointSource, PointTarget> (
jpapon commented 11 years ago

From what I can tell, no correspondence classes are actually instantiated under normal compilation (making the above changes compiles fine for me).

Adding a

#include <pcl/registration/correspondence_estimation.h>
template class pcl::registration::CorrespondenceEstimation<pcl::PointXYZ, pcl::PointNormal>;

to a random file caused the same errors you reported.

The errors also occur with or without your changes, I suspect nobody has really tried using two different point types like this.

aichim commented 11 years ago

But the changes do make sense, no? There is a clear error with the typedefs and the runtime check for the template arguments to be the same is pretty useless, no? :-)

jpapon commented 11 years ago

I've posted a fix, but it makes it less efficient for same-type correspondences (requires creating a full temporary copy even though it's not strictly necessary). Unfortunately checking to see if the pointtypes are the same is actually kind of a pain, since it would mean specializing the entire class. I don't think it's possible to do a compile time check for template parameter equality. Anyone know a good workaround for this? I don't think it can be done with a simple run-time check.

taketwo commented 10 years ago

We could use nearestKSearchT() here.

stale[bot] commented 4 years ago

Marking this as stale due to 30 days of inactivity. It will be closed in 7 days if no further activity occurs.

larshg commented 1 year ago

Is this one solved by #4901 or #4902 ?