Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CorrespondenceEstimation with different point types #329

Open
aichim opened this issue Oct 16, 2013 · 6 comments · May be fixed by #1476
Open

CorrespondenceEstimation with different point types #329

aichim opened this issue Oct 16, 2013 · 6 comments · May be fixed by #1476

Comments

@aichim
Copy link
Member

aichim commented Oct 16, 2013

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> (
@ghost ghost assigned jspricke Oct 16, 2013
@jpapon
Copy link
Contributor

jpapon commented Oct 16, 2013

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
Copy link
Member Author

aichim commented Oct 16, 2013

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 added a commit to jpapon/pcl that referenced this issue Oct 16, 2013
@jpapon
Copy link
Contributor

jpapon commented Oct 16, 2013

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
Copy link
Member

taketwo commented Jan 24, 2014

We could use nearestKSearchT() here.

SergioRAgostinho added a commit to SergioRAgostinho/pcl that referenced this issue Dec 13, 2015
@jspricke jspricke removed their assignment Jul 9, 2017
SergioRAgostinho pushed a commit to SergioRAgostinho/pcl that referenced this issue Dec 14, 2017
@stale
Copy link

stale bot commented May 19, 2020

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

@larshg
Copy link
Contributor

larshg commented Jun 14, 2023

Is this one solved by #4901 or #4902 ?

@stale stale bot removed the status: stale label Jun 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants