libpointmatcher icon indicating copy to clipboard operation
libpointmatcher copied to clipboard

Embedding libpointmatcher in Unity plugin

Open stephanemagnenat opened this issue 3 years ago • 6 comments

Dear all,

In the context of a research project on augmented reality for architecture and city planning, we are using Unity XR to generate a point cloud and then align it in background with libpointmatcher.

To do that, we need to embed libpointmatcher in a Unity plugin. @dhuettig has been working on that and has a functioning implementation, put the build process was not fully straightforward.

Thinking about it, I believe that it could be improved as some libpointmatcher dependencies are not necessary for that use case. In particular:

  • yaml-cpp could be optional for such embedded situation where no configuration file is parsed at runtime.
  • A part of boost (or all?) could be removed by updating the codebase to a more recent version of C++ (17 seems a reasonable baseline regarding compiler support nowadays).

But of course, I know from experience that sometimes in production, especially in research and industrial areas, the toolchains can be ancient, so before investing time to such an endeavour, I would like to have a feedback from the community of developers/users. So my questions are:

  • Is there interest beside our use case for contributions to reduce mandatory dependencies of libpointmatcher?
  • If so, what is the minimal acceptable version of C++?

stephanemagnenat avatar Oct 20 '20 11:10 stephanemagnenat

@AlexandreG87 and @simonpierredeschenes I know that you started removing dependency to Boost, any concerns or advices?

@peci1 any concerns for the DARPA infrastructure for C++17? Same question for @YoshuaNava.

pomerlef avatar Oct 21 '20 14:10 pomerlef

@pomerlef As far as I'm concerned, I've only replaced the BOOST_AUTO macros for the auto specifier from C++11 in the Registrar.h file. And I think that it could safely be done everywhere since libpointmatcher support C++11. For the other dependencies, I'm not familiar enough with boost to suggest a C++ version that would be the best to replace most of the them.

aguenette avatar Oct 21 '20 18:10 aguenette

I checked last year what needed to be done to remove our dependency on boost. Since you will not be using filesystem. It should be easy to do: https://github.com/ethz-asl/libpointmatcher/issues/306

PhiBabin avatar Oct 23 '20 02:10 PhiBabin

C++17 is okay ;)

peci1 avatar Oct 26 '20 13:10 peci1

Hi all, Thank you for tagging me.

I also think C++17 is quite good, and I see many advantages in it. For example: aligned allocators which could potentially speed-up our computations with minimal effort. Or parallel algorithms, optionals.

However, C++17 is not supported by default by ROS Noetic (released this year, last version until EOL of ROS1), neither by ROS2 Foxy Fitzroy link. Some developers from the codebase of libpointmatcher might be using ROS 1/2, and moving directly to C++17 could introduce some ABI compatibilities.

I would propose going first for C++14, or if we go for C++17, introduce a symbol so the library can still be compiled with C++14.

YoshuaNava avatar Oct 26 '20 13:10 YoshuaNava

It is not true that Noetic doesn't support C++17. It just doesn't allow any C++17-only feature to be present in the .h files (the API of the codebase). https://www.ros.org/reps/rep-0003.html#melodic

However, these restrictions are mostly for the "core" ROS packages. It is nice if other packages comply with these rules, too, but it is not a blocker - and the buildfarm definitely doesn't block building and releasing C++17 code. What's IMO important is whether all GCC versions used in the supported OS versions support C++17 (or at least the used features of it).

The oldest supported OS for Melodic is Debian Stretch with GCC 6.3. According to https://gcc.gnu.org/projects/cxx-status.html#cxx17, GCC supports about a half of C++17 features in 6.x, and almost all in 7.x. So using C++17 in PM would mean dropping support for compiling on Stretch (maybe, depending on the features used).

There is a workaround for many of the missing features, e.g. for std::optional. It autoselects either std::optional or std::experimental::optional based on the available compiler and language level: https://github.com/peci1/robot_body_filter/blob/95d8da4475283845066a7cbfcbe71b8b069009be/CMakeLists.txt#L4-L12 .

peci1 avatar Oct 26 '20 15:10 peci1