-
Notifications
You must be signed in to change notification settings - Fork 269
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support the sensors system on macOS #960
Comments
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!
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 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 I think what may be required is something like Qt's |
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 gazebosim/gz-gui#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:
# Run the camera sensor example (assuming the current working directory is the colcon workspace) ign gazebo -v 4 -s src/ign-gazebo/examples/worlds/camera_sensor.sdf
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
:The text was updated successfully, but these errors were encountered: