Open srmainwaring opened 2 years ago
We discussed the idea of letting gz-rendering load its own plugins in https://github.com/gazebosim/gz-rendering/issues/482 for making gazebo visual plugins work but opted to implement it in gz-gazebo instead. I think it's still valuable to support loading plugins in gz-rendering. The proposed implementation approach sounds good.
From the use case described, it's sounds like it's different from the type of plugins in gz-gazebo or gz-gui, where the proposed gz-rendering plugins don't have standard interfaces (e.g. Configure
, Update
) that need to be implemented?
Some more detail about the use case:
One of the wave rendering approaches modifies a set of three textures each update step to encode the wave displacements and normal and tangent vectors. These are pushed from the CPU to the GPU using ogre staging textures and then applied to the ocean grid in the vertex shader. This part of the code requires access to the classes in ogre-next
and the gz-rendering-ogre2
plugin library.
There is a system plugin analogous to the ShaderParam
plugin in gz-sim
that is declared in the <visual>
element of the model. This plugin implements the usual ISystemConfigure
and ISystemPreUpdate
interfaces. The problem is that if this plugin links to gz-rendering-ogre2
directly the render library may be loaded before it is correctly loaded and initialised by MinimalScene
. This was causing segfaults on some platforms (macOS arm64).
The approach is to use two plugins for the visual: A system plugin as described in (2), an interface component that defines extensions to gz-rendering
and a plugin loader, and a render engine extension plugin that links to ogre2 and gz-rendering-ogre2
that contains the implementation. The ogre2 extension plugin is not loaded by the system visual's PreUpdate
method in (2) until the scene created by MinimalScene
is valid.
There's a draft PR here with an implementation: https://github.com/srmainwaring/asv_wave_sim/pull/61. It works as intended however the code needs to be refactored and simplified as most of the functionality adapted from the gz::rendering::RenderEngineManager
and gz::rendering::RenderEngine
is not required, and we are not really creating a new type of Scene
but rather creating an object factory for extending rendering types so the current naming is misleading.
As an aside there is also something unusual going on with the render engine singletons. The following are called after MinimalScene
loads and initialises the metal render engine:
{
auto engine = rendering::Ogre2RenderEngine::Instance();
auto graphicsAPI = engine->GraphicsAPI();
gzdbg << "Using graphicsAPI: "
<< GraphicsAPIUtils::Str(graphicsAPI) << "\n";
}
// outputs
// Using graphicsAPI: OPENGL
Which is the default, but incorrect as the Metal render engine has already been initialised and loaded.
{
auto engine = rendering::engine("ogre2");
auto graphicsAPI = engine->GraphicsAPI();
gzdbg << "Using graphicsAPI: "
<< GraphicsAPIUtils::Str(graphicsAPI) << "\n";
}
// outputs
// Using graphicsAPI: METAL
Which is correct when using the Metal render engine.
The first case is used in gz-rendering
in the ShaderParam code, and works there. It does not seem to work when used in a separate plugin. It is almost as if there are separate copies of the SingletonT<> in each plugin library?
thanks for explaining the use case and how the extension plugin is implemented. So My understanding is:
MinimalScene
(which dynamically loads gz-rendering-ogre2
)WaveVisual
) which has a RenderEngineExtensionManager
RenderinEngineExtensionManager
loads the extension plugin once a valid scene
is found (extension plugin is linked against gz-rendering-ogre2
)Load
, Init
, ScenenodeFactory
// outputs // Using graphicsAPI: OPENGL Which is the default, but incorrect as the Metal render engine has already been initialised and loaded.
that's weird. The minimal scene is created and initialized using metal backend and this happens before the extension plugin is loaded or after?
It is almost as if there are separate copies of the SingletonT<> in each plugin library?
Note that I'm not exactly sure what the behavior is when the gz-rendering-ogre2
library is first dynamically loaded into memory and then we load another library that links against gz-rendering-ogre2
. We had a bit of an issue with a similar situation in gz-sensors and ended up refactoring the code and removing the plugin loading functionality in https://github.com/gazebosim/gz-sensors/pull/90. But it seems like in this case, the render engine extension plugin actually loads and run but it appears to be accessing different singletons.
Yes, that's the gist of it.
In step 1. it's not guaranteed that gz-rendering-ogre2
will be loaded by MinimalScene
before step 2 loads the WaveVisual
as these may run on different threads (actually it's not assured that step 1 occurs before step 2 either).
I'm wondering whether the problem with the singletons is caused by the fact that RenderEngine
does not have a virtual method anchored in the translation unit (it has all it's methods defined in the header - most are abstract but the destructor is defined inline). This can result a copy of RenderEngine::~RenderEngine() being created in each library / plugin that includes RenderEngine.hh
(https://llvm.org/docs/CodingStandards.html#provide-a-virtual-method-anchor-for-classes-in-headers).
The fix is to add a .cc
file for each interface class that defines one virtual method (say the destructor) so it could be as simple as.
#include "RenderEngine.hh"
RenderEngine::~RenderEngine() = default;
The fix is to add a .cc file for each interface class that defines one virtual method (say the destructor) so it could be as simple as.
Oh, I believe this may fix some other things I have been running in to. I'm going to open a PR with just this fix.
Desired behaviour
Add a mechanism for external projects to extend the render engine interface and implementation.
The present render engine plugin mechanism allows external projects to add custom render engines that implement part or all of the current render engine interface, but it does not allow the interface to be extended via plugins.
For example if you wanted to add a
ForceVisual
the current approach would be to add classes to the existinggz-rendering
library and add aCreateForceVisual
method toScene
(and equivalents for each render engine implementation). It would be useful to be able to add such objects in external projects without having to change the core libraries.The use case I have in mind is for an plugin for an OceanVisual. The visual plugin requires a number of custom rendering objects that subclass from
rendering::Visual
andrendering::Geometry
. My current implementation defines these directly in the system plugin, however it turns out that on some platforms the approach is sensitive to the order in which the plugins are loaded which can result in either the render engine being improperly initialised or the loader reporting missing symbols.Alternatives considered
Define extensions directly in system plugins. This can be made to work, but is not robust and may fail on some platforms. It is also not very modular and does not enable custom rendering extensions to be easily shared.
Implementation suggestion
Add a mechanism analogous to RenderEngine, RenderEnginePlugin, RenderEngineManger that allows extension interfaces and their implementation to be defined in plugins and loaded at runtime. The extension manager should have a mechanism to check that required (render engine) dependencies are available and have been loaded before attempting to load extensions.
Additional context
I'll attempt to implement this approach in an external project (asv_wave_sim). If it works I'll factor out the extension mechanism and submit it as a PR. In the meanwhile if the team have any suggestions of recommendations on an approach that would be welcomed!