Add environment variable to locate plugins

[Levi-Armstrong]

I just copied how this was done in Ignition GUI

[Levi-Armstrong]

I am not sure why the tests are failing on windows and I do not have a way to test this locally.

[Levi-Armstrong]

This change solved the issue with locating the ignition rendering plugins within a snap package, but now at runtime Ogre does not have the OpenGL render plugin. I believe the Ignition Ogre Plugin is setting the path for ogre plugin but I am not certain. Does anyone have any thoughts on the issue?

[chapulina]

I am not sure why the tests are failing on windows and I do not have a way to test this locally.

They were failing before, not this PR's fault.

now at runtime Ogre does not have the OpenGL render plugin

Interesting, I wouldn't expect the existing plugins to be affected by this change. How are you loading Ogre? Would one of the existing examples in this package fail the same way? Are you loading Ogre using the environment variable?

[Levi-Armstrong]

It looks like the path it looks for ogre plugins is set at compile time here. I am testing a solution at the moment to address this similar to how this is done in ogre.

Another possible option would be to not provide the full path for the ogre plugins as done here but only include the names when loading ogre plugins. The only reason I can see to not make this change, is it would not support switching to the debug version. The reason behind only using the plugin name is that Ogre already supports bundling so it would detect that it inside a snap package and correctly locate its resources if the plugin name was only provided.

[Levi-Armstrong]

So with these changes I have been able to successfully create a snap package leveraging ignition gui. Below is a app configuration setting leveraging the environment variables so resources are located within the snap package.

apps:
  tesseract-setup-wizard:
    extensions:
      - kde-neon
    environment:
      IGN_GUI_PLUGIN_PATH: $SNAP/opt/ros/melodic/lib/
      IGN_RENDERING_PLUGIN_PATH: $SNAP/opt/ros/melodic/lib/
      IGN_RENDERING_RESOURCE_PATH: $SNAP/opt/ros/melodic/share/ignition/ignition-rendering4/
      OGRE_RESOURCE_PATH: $SNAP/usr/lib/x86_64-linux-gnu/OGRE-1.9.0/
    command: opt/ros/melodic/bin/demo_window

[Levi-Armstrong]

If you find these changes acceptable, I can go through and update OGRE2 plugin also.

[chapulina]

The approach looks reasonable to me. The only thing I'm wondering is whether we can avoid exposing engine specific configuration to downstream users.

So I could see OGRE_RESOURCE_PATH somehow combined with IGN_RENDERING_RESOURCE_PATH, or maybe using a more generic variable name such as IGN_RENDERING_ENGINE_RESOURCE_PATH which is shared by all rendering engines.

I'm not 100% sure this is a good idea though. Not sure what @iche033 thinks of this?

[iche033]

One small edge case issue I can think of for using a shared env variable like IGN_RENDERING_ENGINE_RESOURCE_PATH is when users load multiple render engines and need to specify a different resource path per engine. Ign-rendering is designed to support loading multiple rendering engines and switching between them at run time (at least that was the case when optix was working). So users may need to a way to specify different resource paths, and that may be easier to do with engine specific env variables.

[Levi-Armstrong]

Another option is how ogre does this. The only reason I had to expose the OGRE_RESOURCE_PATH environment variable was because I am bundling it using snap. In this case ogre looks for the environment variable SNAP and if it exists it prepends the path as shown here.

        // With Ubuntu snaps absolute paths are relative to the snap package.
        char* env_SNAP = getenv("SNAP");
        if(env_SNAP && !path.empty() && path[0] == '/') // only adjust absolute dirs
            path = env_SNAP + path;

[iche033]

Let's go with the OGRE_RESOURCE_PATH approach for now as it's working. The changes look good to me.

[chapulina]

Besides the inline comment, I'd like to clarify whether this is enough to close #100 (i.e. can we load rendering engine plugins other than Ogre and Optix?).

I suspect that the answer is no, which is fine and shouldn't block this PR.

[Levi-Armstrong]

Besides the inline comment, I'd like to clarify whether this is enough to close #100 (i.e. can we load rendering engine plugins other than Ogre and Optix?).

I suspect that the answer is no, which is fine and shouldn't block this PR.

I believe it should be enough to load plugins outside this package. Essential that is what is happening when I am bundling this into a snap. Everything is being loaded from the environment variables because the location of all resources are not in the location of the original install location.

[Levi-Armstrong]

@chapulina and @iche033 I added one more commit which makes the same change in the ogre2 plugin.

[chapulina]

I believe it should be enough to load plugins outside this package.

That sounds great, thanks!

Unrelated to this PR, but I'd say we need some documentation and a demo on how to load a custom engine before closing #100. Similar to the ign-physics examples. I haven't tried to do it myself, but looking at the code, I'm concerned about some assumptions made about the "default engines" in places like this:

https://github.com/ignitionrobotics/ign-rendering/blob/dfcf7c4a574b6b0e94419b7ba1ab3ebce64055f3/src/RenderEngineManager.cc#L287-L312

[Levi-Armstrong]

Is this good to merge?

[iche033]

merging!