add explicit instantiations to transformation estimation LM and one to ICP

[themightyoarfish]

based on #5894
this class takes a lot of time to instantiate in my program.

[mvieth]

Hi, why did you add the change in icp.cpp to this pull request? It seems unrelated to the other changes and does not even have any effect without a corresponding update in icp.h.
Please also have a look at the formatting of the files. The formatting check currently does not pass.

[themightyoarfish]

Will fix formatting.

I think using ICP with PointNormal as the target type is not so unusual, you need it for using the Point-to-Plane error. In my work, i often have normals for the target, as a 3D model is available. Hence why i added the instantiation. missed the change in the header.

[mvieth]

Can you try changing this:

pcl/registration/include/pcl/registration/registration.h

Line 668 in ba3ea48

int k = tree_->nearestKSearch(cloud, index, 1, indices, distances);

to
int k = tree_->nearestKSearchT(cloud[index], 1, indices, distances); ?
That should hopefully solve the build error on Windows.

[themightyoarfish]

done in cf13771

[mvieth]

You missed adding the T in nearestKSearchT. When using nearestKSearchT, the first argument can be any point type.

[mvieth]

@themightyoarfish There are still (additional) build errors on Windows. Please either find solutions for them until all checks are successful, or remove the explicit instantiation of IterativeClosestPoint<pcl::PointXYZI, pcl::PointNormal> from this pull request. I currently do not have the time to offer detailed suggestions how to fix the errors.

[themightyoarfish]

It looks like the CI passed now.

[mvieth]

Suggested change

	extern template class pcl::IterativeClosestPoint<pcl::PointXYZI, pcl::PointNormal>;
	extern template class pcl::IterativeClosestPoint<pcl::PointXYZ, pcl::PointNormal>;

To match the instantiation in icp.cpp

[themightyoarfish]

made all match in 98bb945

[mvieth]

To be honest, I am against adding instantiations for both pcl::IterativeClosestPoint<pcl::PointXYZ, pcl::PointNormal> and pcl::IterativeClosestPoint<pcl::PointXYZI, pcl::PointNormal>. I would for now only add the former (PointXYZ), but not the latter (PointXYZI). Hopefully you agree with me that the number of explicit template instantiations should be kept low, otherwise we increase both the build time of PCL as well as the binary size of PCL (both affecting everyone, regardless whether they use the instantiations or not). I see your point that people might use PointXYZ and PointNormal for ICP with point-to-plane estimation because only the target cloud needs normals, even though I believe that people using different point types for source and target clouds is rare. So, with PointXYZ for source and PointNormal for target, both point types have the fewest fields necessary to still work with point-to-plane ICP (no intensity, rgb, labels, etc necessary). But if we add pcl::IterativeClosestPoint<pcl::PointXYZI, pcl::PointNormal>, we could just as well also add pcl::IterativeClosestPoint<pcl::PointXYZL, pcl::PointNormal> and pcl::IterativeClosestPoint<pcl::PointXYZRGB, pcl::PointNormal> and pcl::IterativeClosestPoint<pcl::PointXYZ, pcl::PointXYZINormal> and pcl::IterativeClosestPoint<pcl::PointXYZI, pcl::PointXYZINormal> and so on. I think only for pcl::IterativeClosestPoint<pcl::PointXYZ, pcl::PointNormal> there is a good reason to have it. But for all the other source type/target type combinations (where source type != target type), there is no real reason to have some but not others, and overall I think these are not used by enough people to justify adding explicit instantiations for them.

As a side note, I think it could be an option to add an explicit instantiation for any point types you like in your own project, then reuse that. That way, you do not have to recompile the heavy instantiations when you rebuild other parts of your project.

[themightyoarfish]

yeah ok i have removed the XYZI-PointNormal instantiation.

[mvieth]

Looks good to me now. Thanks for bearing with my change requests 🙂