MarineRoboticsGroup / tonioviz

Finally, a nice C++ visualization tool.
4 stars 0 forks source link

Shouldn't have to find_package(GTSAM, Pangolin, etc) #5

Open keevindoherty opened 3 years ago

keevindoherty commented 3 years ago

Linking with tonioviz from another CMake project currently also requires find_packageing all of its other dependencies.

Seems like this is an issue with tonioviz's CMake package config, but I don't know enough about this to pinpoint exactly what needs to change.

tonioteran commented 3 years ago

trying to figure out how to correctly do this.

this is just a reference for me: https://cliutils.gitlab.io/modern-cmake/chapters/install/exporting.html

alanpapalia commented 3 years ago

Maybe RTABMap is a good place to look for this? Not 100% sure how it worked but I remember it hiding way code in something along the lines of #ifdef Tonioviz_GTSAM ... #endif to hide away anything that requires GTSAM. This file shows a way to do it for GTSAM.

(Disclaimer) I've found RTABMap to be pretty well written so just assumed it's a valid approach. Happy to talk about it more or take a crack at it (though maybe I'd take a bit longer than if someone else worked at it)

tonioteran commented 3 years ago

I think wrt to the include files and stuff, we shouldn't have much of a problem. I can template the CMakeLists.txt in a similar way as we do for the cpp-toolbox, and that way no GTSAM related files would be even included if they're not wanted.

What this issue mostly focuses on is the fact that we're currently not correctly exporting our imported targets, which seems to be a rather hard thing to do with CMake. I haven't found a good way to do this, with the main problem being that it is not possible to export a target that's not being build by the same project, so, for example, we cannot export GTSAM with Tonioviz since we aren't locally building it.

alanpapalia commented 3 years ago

Gotcha - sorry clearly wasn't thinking clearly when I stumbled across this. I realized my comment was addressing an entirely different thing going on in my head :)

Without asking too many stupid questions. Is there a reason why there is no use of the install command in the CMake file? I would think that would be necessary to pass dependencies on to libraries that link to tonioviz (which seems to be done by the find_dependency command, normally passed into a library.cmake.in file?).