search

patrykcieslak / stonefish

Stonefish - an advanced C++ simulation library designed for (but not limited to) marine robotics.
https://stonefish.readthedocs.io/
GNU General Public License v3.0
126 stars 32 forks source link

Allow configuring shader dir at runtime #14

Closed nilsbore closed 3 years ago

nilsbore commented 3 years ago

This change adds an optional argument for the shader dir path. If not used, the API is identical to before, as well as the behavior. This allows using a different install path, as enabled by https://github.com/patrykcieslak/stonefish/pull/13 .

nilsbore commented 3 years ago

For the record, these two changes allowed us to get binary debian releases of stonefish_ros going. This is valuable to us as it allows collaborators to quickly get going with the simulator.

patrykcieslak commented 3 years ago

I did not see it necessary to modify the code because the path is already set up using a define. I just defined the shader path with the install prefix in cmake.

nilsbore commented 3 years ago

Yes, this is the solution that we've been using before also. However, we now want to be able to use the shaders as part of an installed ROS workspace or to create a debian package. When doing so, AFAIK it is not possible to know beforehand if the installed path will be e.g. catkin_ws/install, /opt/ros/melodic or /opt/ros/noetic. However, it is very easy to check at runtime within roslaunch. Therefore the runtime argument is useful.

nilsbore commented 3 years ago

I'll investigate a bit more though, maybe it is possible to extract this path when doing the release build.

patrykcieslak commented 3 years ago

I think there is a simpler solution - embedding the resources inside the library binary. I will then make an option in CMake to enable this functionality so you can compile an easily distributable version. I have an idea how to implement it in a transparent way. I should be able to work on it this week.

nilsbore commented 3 years ago

That sounds like a good plan! We can use our fork of Stonefish in the meanwhile, but I'd prefer to keep it to tracking upstream Stonefish in the long run. Somewhat related to this (enough to not open another issue): what would you think about putting the contents of the include folder Library/include inside Library/include/Stonefish instead? That'd make it easier to work with includes in the source folder.

nilsbore commented 3 years ago

It looks like you added something like you described already. Would you say it is functional?