ros2 / rviz

ROS 3D Robot Visualizer
BSD 3-Clause Clear License
293 stars 211 forks source link

rviz_ogre_vendor package doesn't seem to try to `find_package(Ogre)` #723

Open wolfv opened 3 years ago

wolfv commented 3 years ago

It seems that the assimp vendor package correctly tries to find an installed assimp and uses that if available. However, the Ogre vendor CMakeLists doesn't attempt to find Ogre (if I am reading the CMake correctly). It would simplify our builds in the RoboStack project quite a bit if we could use the pre-built binaries from conda-forge. (some context here: https://medium.com/robostack/cross-platform-conda-packages-for-ros-fa1974fd1de3).

clalancette commented 3 years ago

I don't recall now why we decided to unconditionally vendor OGRE. @wjwwood do you remember?

wjwwood commented 3 years ago

I think that we vendored it because the versions below it didn't work for us, the ones in Ubuntu, and later versions had bugs too. I imagine this has been resolved by now, at least partially. But I know that the version used by rviz in ROS 1 and ignition/gazebo didn't work for us.

One reason we didn't have the conditional "find it upstream first" is that the cmake files that are provided by ogre were broken in some way and so we have a lot of custom logic for loading them as imported targets. I don't know if this logic can be used if the libraries are being provided separately. But maybe they can and we just didn't do that because it was extra work that we didn't have time to implement.

wolfv commented 3 years ago

Thanks for the reply. We've got a bunch of patches in the ros-galactic build-scripts repo: https://github.com/RoboStack/ros-galactic/tree/main/patch

I think some are very specific to our use-case, but we could try to generalize them enough to get them into a merge-able format. Would you be interested in that?

wolfv commented 3 years ago

PS: rviz2 should work as a conda-package on linux: {conda or mamba} create -n ros2 ros-galactic-rviz2 -c robostack -c conda-forge if you want to try it out :)