Closed srmainwaring closed 9 months ago
This branch (https://github.com/srmainwaring/ign-gazebo/tree/feature/ign-gazebo5-macos) contains proof of concept changes to support the sensors system plugin and GUI rendering on macOS.
There is some platform / render engine specific code dealing with OpenGL contexts that needs to be moved elsewhere but the changes illustrate the proposed approach and does render a camera view in the image display (but still seems quite fragile as seemingly minor change to a SDF file will cause a crash).
Nice!
A suggestion that would keep deferred initialisation and avoid blocking in the main thread is to provide another system interface, say ISystemInit that the SimulationRunner would guarantee to call on the main thread after the system was configured.
Another option is to use event callbacks. The SimulationRunner would emit this new init event on the main thread. Here's an example of connecting to an event.
It looks like all your changes are done on top of Edifice so the the advantage of adding a new event is that it should not break ABI.
Thanks, I'll look into the events callback example. I wound up using Sensors::Update
to initialise the RenderUtil
instance which does not break the ABI either. That should be safe as it's also on the main thread and should not conflict with access to shared resources (i.e. the initialized
flag) from the Sensors::PostUpdate
worker threads.
The main thing that I'm not sure how to handle cleanly is the management the OpenGL context. This is first created on the main thread (by Ogre2 in this case). I capture the context in SensorsPrivate
then deactivate it after this->dataPtr->renderUtil.Init()
on the main thread and then set the context as current on the render thread in the remainder of the initialisation sequence which happens once the condition variable in SensorsPrivate::WaitForInit
is released. In my example I'm using the Apple CGL calls directly which is clearly a no-go for general release.
I think what may be required is something like Qt's moveToThread
on RenderUtil
which delegates to the render engine implementation so this can be carried out in a platform and render engine independent way.
I found this while trying to understand if there's any way I can get Ignition Gazebo running (via https://github.com/ignitionrobotics/ign-gazebo/issues/44). It looks like there's still something to be sorted out, but regardless, thank you so much for your contributions! Ignition looks like a really great step into the future, and you're helping make robotics more accessible to folks like me. 🎖
I found this while trying to understand if there's any way I can get Ignition Gazebo running (via #44). It looks like there's still something to be sorted out, but regardless, thank you so much for your contributions! Ignition looks like a really great step into the future, and you're helping make robotics more accessible to folks like me. 🎖
@johntron thanks for the feedback! If you're interested in getting Gazebo running on macOS and are prepared to have a go at building from development forks there are some details in this issue https://github.com/ignitionrobotics/ign-gui/issues/314 on what's required. I plan to submit the prototype branches as PR's once I've cleaned them up and their dependencies have been merged. In the meanwhile if you need any help let me know - I'd appreciate any additional feedback from other macOS users while drafting the PR on how well these changes work and what can be improved.
fixed by #1225
Desired behavior
Would like the sensors system plugin to run on macOS using the ogre2 render engine.
In the following we'll use the
camera_sensor.sdf
world as an example, but the discussion applies to any model loading the sensor system plugin with the SDF element:If you run the following command on Ignition Ediface you'll most likely see a crash on macOS:
By including additional exception handling in Ogre2 we see that the reason is that the
Sensors
class is trying to create a new render window on a render thread. The requested operation is not allowed in the Cocoa window system, where there are some restrictions that require the initialisation ofNSWindow
and manipulationNSView
to occur on the main thread. A full stack trace is included below in the Additional context section.The new feature then is the ability to have some of the initialisation of the sensor system occur on the main thread before control is ceded to the render thread.
Alternatives considered
Modify the Ogre2
CocoaWindow
class to support creating windows on secondary threads. The Apple developer guide has this document on Thread Safety. It suggests that aNSWindow
can be created on a secondary thread (but is not generally thread-safe). However all operations on aNSView
must occur on the main thread. Ogre makes use of both objects.The function call raising the exception in Ogre is:
OgreGL3PlusWindow
is aNSWindow
, so the initialisation of aNSWindow
is failing in this case contrary to the comments in the Apple developer guide, and it's not clear that there is an alternative way to initialise a new window for the case at hand.Implementation suggestion
The challenge is that the initialisation of the render system in
Sensors
is deferred and occurs in:https://github.com/ignitionrobotics/ign-gazebo/blob/9fbb9b988c20711fcff706fb14d034516e027e6b/src/systems/sensors/Sensors.cc#L180-L206
which is already running on a render thread.
It is ultimately triggered by
this->dataPtr->doInit
being set totrue
in:https://github.com/ignitionrobotics/ign-gazebo/blob/9fbb9b988c20711fcff706fb14d034516e027e6b/src/systems/sensors/Sensors.cc#L427-L451
which in turn is called from a worker thread in the
SimulationRunner
.A suggestion that would keep deferred initialisation and avoid blocking in the main thread is to provide another system interface, say
ISystemInit
that theSimulationRunner
would guarantee to call on the main thread after the system was configured.I'm happy to attempt to implement and test this proposal but wanted to sound out the development team on the design in case there are major issues with what is being suggested.
Update
Since
SimulationRunner::UpdateSystems
calls theISystemPreUpdate
andISystemUpdate
interfaces from the main thread we could initialise the render engine fromSensors::Update
or subclassSensors
fromISystemPreUpdate
then useSensors::PreUpdate
rather than introduce a new interface.Additional context
This is the
camera_sensors.sdf
example running on macOS with the sensor system plugin disabled:This is the log output from the server process with the plugin enabled. The code is from the
ign-gazebo5
branch modified to support macOS with additional error reporting enabled inign-rendering5
andogre-next2.1
: