search

ros2 / rviz

ROS 3D Robot Visualizer
BSD 3-Clause Clear License
307 stars 214 forks source link

Use system-wise installed Ogre if available #161

Open mjbogusz opened 6 years ago

mjbogusz commented 6 years ago

This is similar to yaml-cpp issue mentioned in #152 (and fixed in #160): building rviz_ogre_vendor pulls whole ogre (which is big due to them always bundling in all samples, especially terrain ones are huge) and compiles it (which takes quite some time).

While I understand that we want to ensure ogre in version 1.10.10+ which is not available in Ubuntu LTS out-of-the-box I don't think we should assume it's not there.

However while checking specific version is easy in CMake there are other caveats compared to yaml-cpp, e.g. the downloaded and locally compiled version is static and rviz_ogre_vendor aliases some of ogre's libraries as it's additional targets that rviz_rendering explicitly depends upon.

Is there a specific reason for using ogre as static libraries? If switching to shared would not break anything it'd be much easier to use system-wise installed ogre if it's available.

wjwwood commented 6 years ago

Currently we're using a very specific version of ogre (it's just a certain commit on master which came after 1.10.10). Once we get to using a stable version again (1.10.11?) then I agree it's worth supporting an external Ogre instance.

We will need to update the "helper" logic I made in the rviz_ogre_vendor package which wraps them in exported targets (would be nice if Ogre did that for us).

the downloaded and locally compiled version is static and rviz_ogre_vendor aliases some of ogre's libraries as it's additional targets that rviz_rendering explicitly depends upon.

rviz_rendering is actually using exported targets, in which case the "aliased" libraries you're talking about are actually part of the target's interface, so I don't think that's inappropriate for that pattern.

Is there a specific reason for using ogre as static libraries? If switching to shared would not break anything it'd be much easier to use system-wise installed ogre if it's available.

Originally I wanted to make Ogre a non-transitive dependency, i.e. I wanted to use it only in cpp files (no mentions or includes in the public headers of rviz_rendering) and then hide the symbols. That way we wouldn't collide with other Ogre's on the system, but since then I've realized the work involved in providing an abstraction everywhere we use ogre in the plugins is unrealistic right now, though I am still working towards that. The real goal is to make it so that we could swap ogre out (maybe for Ogre 2.x or Qt3D or something) in the future, as I'm uncertain about the future of Ogre 1.x something like 5 years down the line.

Also, rather than loading the ogre plugins dynamically I'm loading thing explicitly (manually), which allows us to get away with not setting the Ogre plugin path and all that.

That being said, I am currently converting Ogre to build as shared libraries, but of course that broke everything on Windows 🙂.

So I think this should be delayed until we get things working on Windows again. See https://github.com/ros2/rviz/issues/144 and https://github.com/ros2/rviz/pull/143 for details.

mjbogusz commented 6 years ago

I'm bumping this issue as Windows support has (mostly) landed, the used Ogre version was bumped to 1.10.11 (though there are 2 newer releases, 1.10.12 and 1.11) and it's been switched to being built as shared libraries.