Add precompiled computeApproximateCovariances; fix compilation error for the same

[Morwenn]

A colleague told me that computeApproximateCovariances failed to compile: it seems that the automatic change from old for loops to foreach loops introduced that bug.

This this an obvious albeit untested fix since I don't have a test case. I guess that it could ship in 1.10.2 considering how simple and breaking it is.

[kunaltyagi]

The green CI implies that there is no cpp file (no test, no lib) including this at all. Is there any way to fix the lack of compilation?

[Morwenn]

If I actually knew what the function did, how to use it, etc. I'd gladly contribute a test but I'm not the user myself so I unfortunately don't have a ready-made test :/

[kunaltyagi]

To clarify, I don't think a test is needed, just some way we can compile this header. A test is good, but for 1.10.1, simply compiling it on CI would be great

[Morwenn]

Hum, then maybe it would be worth creating .cpp in the test suite files with explicit instantiations of function templates and class templates with some template parameters? Something along these lines:

template void pcl::features::computeApproximateCovariances<pcl::PointXYZ, pcl::PointXYZ>(const pcl::PointCloud<pcl::PointXYZ>&, const pcl::PointCloud<pcl::PointXYZ>&, std::vector<Eigen::Matrix3d, Eigen::aligned_allocator<Eigen::Matrix3d>>&, double);

It would take care of the instantiation and compilation without having to find valid use cases or how to call it correctly. Such a file would also be a good list of functions that lack tests - which can be added later.

[kunaltyagi]

What about an cpp file with template instantiation instead of a test?

[Morwenn]

Well, that's what I was proposing :p

[kunaltyagi]

Oops. My bad again. I meant a cpp file "not in the test suite". There are such template instantiations in a lot of places in PCL

[Morwenn]

Oh, Ill try to find an example of what you already have then.

[Morwenn]

So basically I need to add PCL_INSTANTIATE_computeApproximateCovariances at the end of from_meshes.h and I need to create a from_meshes.cpp where I call PCL_INSTANTIATE(computeApproximateCovariances, /* foo */); depending on the precompilation options, where foo corresponds to the types of points I need to instantiate the template with? From what I'm gathering I will probably need a product instantiation but I'm not sure which kind of points I'm supposed to handle.

Concerning the type of points: the function takes normals from which we use normal_x, normal_y and normal_z so I guess that we should pass it all point types that support those variables, but cloud is only passed to check its size against that of normals so I'm not sure for which kind of cloud points the function should be instantiated.

[kunaltyagi]

So basically ...?

Yes. That's the right track. The cpp file will need 2 modes: core_points_only and all points.

From what I'm gathering I will probably need a product instantiation but I'm not sure which kind of points I'm supposed to handle

The first template argument are points that have xyz, and the second are those that have normal. There are macros for both in PCL, which might help here. This will be the non-core-points mode. For core points mode, I know it's a handpicked selection, but I'm not sure which ones. @taketwo or @SergioRAgostinho might know better

[SergioRAgostinho]

@taketwo or @SergioRAgostinho might know better

No big intuition, I would replicate what I've seen in other classes of the same module, related to this one.

// Instantiations of specific point types
#ifdef PCL_ONLY_CORE_POINT_TYPES
  PCL_INSTANTIATE_PRODUCT(PrincipalCurvaturesEstimation, ((pcl::PointXYZ)(pcl::PointXYZI)(pcl::PointXYZRGBA))((pcl::Normal))((pcl::PrincipalCurvatures)))
#else
  PCL_INSTANTIATE_PRODUCT(PrincipalCurvaturesEstimation, (PCL_XYZ_POINT_TYPES)(PCL_NORMAL_POINT_TYPES)((pcl::PrincipalCurvatures)))
#endif

In your case you're only interest the first two operands of the product. This seems to be the common instantiation in other classes of this module which need covariance information.