Add Metal support to MinimalScene and Qt Application #323

srmainwaring · 2021-11-30T19:30:55Z

🎉 New feature

Partially completes #314

Summary

This PR allows MinimalScene to use either OpenGL or Metal render systems.

Approach

Additional classes are introduced to the MinimalScene plugin folder to abstract the render hardware interfaces. These classes are declared in MinimalSceneRhi.hh:

IgnCameraTextureRhi
RenderThreadRhi
TextureNodeRhi

The suffix abbreviation Rhi for render hardware interface is borrowed from Qt.

Implementations for OpenGL and Metal are declared in:

MinimalSceneRhiOpenGL.hh
MinimalSceneRhiMetal.hh

The existing classes in MinimalScene.hh are modified to forward rendering calls to the (virtual) render hardware interface functions. There are some ABI breaking changes required in order to handle both OpenGL and Metal GPU texture objects.

We used forwarding rather than abstracting and subclassing the existing classes in MinimalScene.hh as it does not require introducing and registering additional Qt objects.

Platform configuration

Application.cc is modified to support either OpenGL or Metal. If the platform is Metal the graphics API is selected to be Metal, otherwise it is OpenGL.

Test it

Still to do - provide a standalone test in ign-gui for Metal support.

The feature can be tested using ignition gazebo by also building the PR that adds Metal support to Gazebo:

Add Metal support to Gazebo for macOS gz-sim#1225

Note: this PR does not provide a Metal version of the gamma adjustment (to follow). As a result the scene looks darker than the OpenGL version. If you want to lighten the scene you can cherry-pick this commit into ign-gui.

A working document describing how to build Ignition for Metal support is here: https://github.com/srmainwaring/ign-rendering/wiki. The main change to note is that the MinimalScene plugin element to override the default graphics interface is now <graphics_api>metal</graphics_api>

Tests

Most of the tests are disabled on macOS. The tests passed on an Ubuntu 20.04 VM running the llvmpipe driver (software rasterizer).

Code check

There is a code check failure concerning the use of static_cast vs reinterpret_cast in MinimalSceneRhi.cc. The code has been updated but the CI test does not appear to be using latest version of the code?

Checklist

Signed all commits for DCO
Added tests
Added example and/or tutorial
Updated documentation (as needed)
Updated migration guide (as needed)
codecheck passed (See contributing)
All tests passed (See test coverage)
While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge

src/Application.cc

src/plugins/minimal_scene/MinimalScene.hh

src/plugins/minimal_scene/MinimalSceneRhi.hh

src/plugins/minimal_scene/MinimalScene.hh

src/plugins/minimal_scene/MinimalSceneRhi.hh

srmainwaring · 2021-12-04T15:18:59Z

@ahcorde the latest changes should fix the OpenGL context issue. The renderer needs to be quite careful to ensure the OpenGL context is associated with the correct thread at the correct time. Both Metal and OpenGL should now be able to use the "threaded" OSG render engine - which will improve performance.

Here's my take on the thread affinity

Threads

Application: QThread *mainThread = QApplication::instance()->thread();
Current: QThread *currentThread = QThread::currentThread();
Ign Render: QThread *renderThread = this->dataPtr->renderThread;
Context: QThread *contextThread = this->dataPtr->renderThread->Context()->thread();

Initialisation

The application runs on the Application thread
Initialisation starts in RenderWindowItem::updatePaintNode which runs on the QSGRenderThread. The context is created here and must be moved to the Application thread before "Ready" is invoked
RenderWindowItem::Ready is a slot and if invoked using the Qt Meta system with Qt::QueuedConnection will run on the Application thread. This is a firm requirement for macOS where all render windows must be initialised on the GUI thread.
After the render thread is initialised both the OpenGL context and the render thread affinity must be set to the RenderThread where it will remain while the scene is rendered.

As there are a lot of small changes and a couple of GitHub edits which are missing signoff I'm going to rebase and squash to clean everything up.

iche033 · 2021-12-07T23:23:17Z

I'm testing this on Ubuntu with OpenGL. It working mostly fine. However, when I tried running ign-gazebo with sensors_demo.sdf, I'm getting a segfault. Backtrace below. The Ogre::v1::GL3PlusHardwareBufferManagerBase::createVertexBuffer backtrace usually suggests that it's being called outside the ogre rendering thread.

backtrace

#0  0x00007fffe8b731a9 in ?? () from /usr/lib/x86_64-linux-gnu/libGLdispatch.so.0
#1  0x00007fffa88ac69c in Ogre::v1::GL3PlusHardwareVertexBuffer::GL3PlusHardwareVertexBuffer(Ogre::v1::HardwareBufferManagerBase*, unsigned long, unsigned long, Ogre::v1::HardwareBuffer::Usage, bool) () from /usr/lib/x86_64-linux-gnu/OGRE-2.2/OGRE/RenderSystem_GL3Plus.so
#2  0x00007fffa889fab9 in Ogre::v1::GL3PlusHardwareBufferManagerBase::createVertexBuffer(unsigned long, unsigned long, Ogre::v1::HardwareBuffer::Usage, bool) () from /usr/lib/x86_64-linux-gnu/OGRE-2.2/OGRE/RenderSystem_GL3Plus.so
#3  0x00007fffa88b9cd6 in ?? () from /usr/lib/x86_64-linux-gnu/OGRE-2.2/OGRE/RenderSystem_GL3Plus.so
#4  0x00007fffb9673633 in ignition::rendering::v7::Ogre2MeshFactory::LoadImpl(ignition::rendering::v7::MeshDescriptor const&) ()
    at /usr/include/OGRE-2.2/OgreMemoryStdAlloc.h:80
#5  0x00007fffb96763a0 in ignition::rendering::v7::Ogre2MeshFactory::OgreItem(ignition::rendering::v7::MeshDescriptor const&) ()
    at /home/ian/code/ign_g_ws/src/ign-rendering/ogre2/src/Ogre2MeshFactory.cc:145
#6  0x00007fffb9676b29 in ignition::rendering::v7::Ogre2MeshFactory::Create(ignition::rendering::v7::MeshDescriptor const&) ()
    at /home/ian/code/ign_g_ws/src/ign-rendering/ogre2/src/Ogre2MeshFactory.cc:120
#7  0x00007fffb9692a80 in ignition::rendering::v7::Ogre2Scene::CreateMeshImpl(unsigned int, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, ignition::rendering::v7::MeshDescriptor const&) ()
    at /usr/include/c++/8/bits/shared_ptr_base.h:1018
#8  0x00007fffb9691b4f in ignition::rendering::v7::Ogre2Scene::CreateMeshImpl(unsigned int, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) ()
    at /home/ian/code/ign_g_ws/src/ign-rendering/ogre2/src/Ogre2Scene.cc:1118
#9  0x00007fffb9693837 in ignition::rendering::v7::Ogre2Scene::CreateBoxImpl(unsigned int, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) () at /usr/include/c++/8/bits/basic_string.h:252
#10 0x00007fffbb9e8a7e in ignition::rendering::v7::BaseScene::CreateBox() ()
    at /home/ian/code/ign_g_ws/src/ign-rendering/src/base/BaseScene.cc:1171
#11 0x00007fffbcf96f01 in ignition::gazebo::v7::SceneManager::LoadGeometry(sdf::v12::Geometry const&, ignition::math::v6::Vector3<double>&, ignition::math::v6::Pose3<double>&) () at /usr/include/c++/8/bits/shared_ptr_base.h:1018
#12 0x00007fffbcf989d2 in ignition::gazebo::v7::SceneManager::CreateVisual(unsigned long, sdf::v12::Visual const&, unsigned long) ()
    at /home/ian/code/ign_g_ws/src/ign-gazebo/src/rendering/SceneManager.cc:299
#13 0x00007fffbcef1c1b in ignition::gazebo::v7::RenderUtil::Update() ()
    at /home/ian/code/ign_g_ws/src/ign-gazebo/src/rendering/RenderUtil.cc:1122
#14 0x00007fffbd221810 in ignition::gazebo::v7::systems::SensorsPrivate::RunOnce() ()
    at /home/ian/code/ign_g_ws/src/ign-gazebo/src/systems/sensors/Sensors.cc:243
#15 0x00007fffbd221fd8 in ignition::gazebo::v7::systems::SensorsPrivate::RenderThread() ()
    at /home/ian/code/ign_g_ws/src/ign-gazebo/src/systems/sensors/Sensors.cc:314
#16 0x00007ffff24506df in ?? () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#17 0x00007ffff731b6db in start_thread (arg=0x7fffbb74d700) at pthread_create.c:463
#18 0x00007ffff7654a3f in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95

Do you get this issue?

srmainwaring · 2021-12-08T09:22:26Z

Seeing something similar with Metal in that example (on the server side) - let me take a look.

% ign gazebo -v4 -s -r sensors_demo.sdf
[Msg] Ignition Gazebo Server v7.0.0~pre1
[Msg] Loading SDF world file[/usr/local/Cellar/ignition-gazebo7/7.0.0/share/ignition/ignition-gazebo7/worlds/sensors_demo.sdf].
[Dbg] [Physics.cc:774] Loaded [ignition::physics::dartsim::Plugin] from library [/usr/local/Cellar/ignition-physics5/5.1.0/lib/ign-physics-5/engine-plugins/libignition-physics-dartsim-plugin.dylib]
[Dbg] [SimulationRunner.cc:891] Loaded system [ignition::gazebo::systems::Physics] for entity [1]
[Dbg] [Sensors.cc:397] Configuring Sensors system
[Dbg] [Sensors.cc:327] SensorsPrivate::Run
[Dbg] [SimulationRunner.cc:891] Loaded system [ignition::gazebo::systems::Sensors] for entity [1]
[Dbg] [Sensors.cc:307] SensorsPrivate::RenderThread started
[Dbg] [Sensors.cc:196] Waiting for init
[Msg] Create service on [/world/lidar_sensor/create]
[Msg] Remove service on [/world/lidar_sensor/remove]
[Msg] Pose service on [/world/lidar_sensor/set_pose]
[Msg] Light configuration service on [/world/lidar_sensor/light_config]
[Msg] Physics service on [/world/lidar_sensor/set_physics]
[Msg] SphericalCoordinates service on [/world/lidar_sensor/set_spherical_coordinates]
[Msg] Enable collision service on [/world/lidar_sensor/enable_collision]
[Msg] Disable collision service on [/world/lidar_sensor/disable_collision]
[Msg] Material service on [/world/lidar_sensor/visual_config]
[Dbg] [SimulationRunner.cc:891] Loaded system [ignition::gazebo::systems::UserCommands] for entity [1]
[Dbg] [SimulationRunner.cc:891] Loaded system [ignition::gazebo::systems::SceneBroadcaster] for entity [1]
[Msg] Loaded level [3]
[Msg] Serving world controls on [/world/lidar_sensor/control], [/world/lidar_sensor/control/state] and [/world/lidar_sensor/playback/control]
[Msg] Serving GUI information on [/world/lidar_sensor/gui/info]
[Msg] World [lidar_sensor] initialized with [1ms] physics profile.
[Msg] Serving world SDF generation service on [/world/lidar_sensor/generate_world_sdf]
[Msg] Serving world names on [/gazebo/worlds]
[Msg] Resource path add service on [/gazebo/resource_paths/add].
[Msg] Resource path get service on [/gazebo/resource_paths/get].
[Msg] Resource paths published on [/gazebo/resource_paths].
[Msg] Found no publishers on /stats, adding root stats topic
[Msg] Found no publishers on /clock, adding root clock topic
[Dbg] [SimulationRunner.cc:522] Creating PostUpdate worker threads: 3
[Dbg] [SimulationRunner.cc:535] Creating postupdate worker thread (0)
[Dbg] [SimulationRunner.cc:535] Creating postupdate worker thread (1)
[Dbg] [Sensors.cc:472] Initialization needed
[Msg] Loading plugin [ignition-rendering-ogre2]
[Dbg] [RenderUtil.cc:2566] Create scene [scene]
[Dbg] [Sensors.cc:510] Initialization needed
[Dbg] [Sensors.cc:208] Initializing render context
[Dbg] [Sensors.cc:222] Rendering Thread initialized
[Msg] Serving scene information on [/world/lidar_sensor/scene/info]
[Msg] Serving graph information on [/world/lidar_sensor/scene/graph]
[Msg] Serving full state on [/world/lidar_sensor/state]
[Msg] Serving full state (async) on [/world/lidar_sensor/state_async]
[Msg] Publishing scene information on [/world/lidar_sensor/scene/info]
[Msg] Publishing entity deletions on [/world/lidar_sensor/scene/deletion]
[Msg] Publishing state changes on [/world/lidar_sensor/state]
[Msg] Publishing pose messages on [/world/lidar_sensor/pose/info]
[Msg] Publishing dynamic pose messages on [/world/lidar_sensor/dynamic_pose/info]
[Dbg] [CameraSensor.cc:271] Camera images for [cameras_alone::link::cameras_alone] advertised on [camera_alone]
[Dbg] [CameraSensor.cc:524] Camera info for [cameras_alone::link::cameras_alone] advertised on [/camera_info]
[Dbg] [CameraSensor.cc:271] Camera images for [camera_with_lidar::link::camera] advertised on [camera]
[Dbg] [CameraSensor.cc:524] Camera info for [camera_with_lidar::link::camera] advertised on [/camera_info]
[Dbg] [DepthCameraSensor.cc:287] Depth images for [cameras_alone::link::depth_camera1] advertised on [depth_camera]
[Dbg] [CameraSensor.cc:524] Camera info for [cameras_alone::link::depth_camera1] advertised on [/camera_info]
[Dbg] [DepthCameraSensor.cc:304] Points for [cameras_alone::link::depth_camera1] advertised on [depth_camera/points]
[Dbg] [RgbdCameraSensor.cc:222] RGB images for [rgbd_camera::link::rgbd_camera] advertised on [rgbd_camera/image]
[Dbg] [RgbdCameraSensor.cc:236] Depth images for [rgbd_camera::link::rgbd_camera] advertised on [rgbd_camera/depth_image]
[Dbg] [RgbdCameraSensor.cc:250] Points for [rgbd_camera::link::rgbd_camera] advertised on [rgbd_camera/points]
[Dbg] [CameraSensor.cc:524] Camera info for [rgbd_camera::link::rgbd_camera] advertised on [rgbd_camera/camera_info]
[Dbg] [Lidar.cc:122] Laser scans for [camera_with_lidar::link::gpu_lidar] advertised on [lidar]
[Dbg] [GpuLidarSensor.cc:149] Lidar points for [camera_with_lidar::link::gpu_lidar] advertised on [lidar/points]
[Dbg] [ThermalCameraSensor.cc:223] Thermal images for [thermal_camera::link::thermal_camera] advertised on [thermal_camera]
[Dbg] [CameraSensor.cc:524] Camera info for [thermal_camera::link::thermal_camera] advertised on [/camera_info]
[Msg] Setting ambient temperature to 288.15 Kelvin and gradient to -0.0065 K/m. The resulting temperature range is: 0.750384 Kelvin.
Stack trace (most recent call last) in thread 123145355350016:
#13   Object "libsystem_pthread.dylib", at 0x7fff204e58fc, in _pthread_start + 224
#12   Object "libignition-gazebo7-sensors-system.", at 0x12dc751be, in void* std::__1::__thread_proxy<std::__1::tuple<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct> >, void (ignition::gazebo::v7::systems::SensorsPrivate::*)(), ignition::gazebo::v7::systems::SensorsPrivate*> >(void*) + 62
#11   Object "libignition-gazebo7-sensors-system.", at 0x12dc5b305, in ignition::gazebo::v7::systems::SensorsPrivate::RenderThread() + 277
#10   Object "libignition-gazebo7-sensors-system.", at 0x12dc5aa50, in ignition::gazebo::v7::systems::SensorsPrivate::RunOnce() + 576
#9    Object "libignition-sensors7.7.0.0~pre1.dyl", at 0x12e35ed67, in ignition::sensors::v7::Manager::RunOnce(std::__1::chrono::duration<long long, std::__1::ratio<1l, 1000000000l> > const&, bool) + 71
#8    Object "libignition-sensors7.7.0.0~pre1.dyl", at 0x12e36a580, in ignition::sensors::v7::Sensor::Update(std::__1::chrono::duration<long long, std::__1::ratio<1l, 1000000000l> > const&, bool) + 80
#7    Object "libignition-sensors7-depth_camera.7", at 0x12e0d0875, in ignition::sensors::v7::DepthCameraSensor::Update(std::__1::chrono::duration<long long, std::__1::ratio<1l, 1000000000l> > const&) + 53
#6    Object "libignition-sensors7-rendering.7.0.", at 0x12e244574, in ignition::sensors::v7::RenderingSensor::Render() + 212
#5    Object "libignition-rendering7-ogre2.7.0.0~", at 0x12e6864e6, in ignition::rendering::v7::Ogre2DepthCamera::Render() + 22
#4    Object "libGL.dylib", at 0x7fff6bde6678, in glEnable + 16
#3    Object "libsystem_platform.dylib", at 0x7fff2052ad7c, in _sigtramp + 28
#2    Object "libignition-tools-backward.dylib", at 0x10d89f7f1, in backward::SignalHandling::sig_handler(int, __siginfo*, void*) + 13
#1    Object "libignition-tools-backward.dylib", at 0x10d89f85f, in backward::SignalHandling::handleSignal(int, __siginfo*, void*) + 79
#0    Object "libignition-tools-backward.dylib", at 0x10d89f9ed, in backward::StackTraceImpl<backward::system_tag::darwin_tag>::load_from(void*, unsigned long, void*, void*) + 33
zsh: segmentation fault  ign gazebo -v4 -s -r sensors_demo.sdf

I'm a bit stuck for testing on Ubuntu at the moment - I had been testing using a VMware Fusion Ubuntu VM which had worked well with Edifice (which actually runs with hardware GPU acceleration enabled on the VM). But with ogre2.2 dropping OpenGL < 4.2 support that no longer works and software (llvmpipe) rendering is not functioning properly for Fortress and above either.

Update

Looks like there are some OpenGL calls in ign-rendering that are a problem for Metal - not sure why they don't show up in the ign-rendering examples?

https://github.com/ignitionrobotics/ign-rendering/blob/6c463bdf2e33f5627d027126f34bf830281b9864/ogre2/src/Ogre2DepthCamera.cc#L974-L981

//////////////////////////////////////////////////
void Ogre2DepthCamera::Render()
{
  // GL_DEPTH_CLAMP was disabled in later version of ogre2.2
  // however our shaders rely on clamped values so enable it for this sensor
#ifndef _WIN32
  glEnable(GL_DEPTH_CLAMP);
#endif

and similar blocks for other camera sensors. The PR does change how the OpenGL context is initialised and moved - so it could be that.

srmainwaring · 2021-12-08T19:47:06Z

Disabling the glEnable and glDisable calls in ign-rendering:

Ogre2DepthCamera.cc
Ogre2LidarVisual.cc
Ogre2Marker.cc
Ogre2ThermalCamera.cc

fixes the issue I was seeing with sensors_demo.sdf when using Metal:

Will need a PR for ign-rendering to address that, but it shouldn't be a block for this PR.

Still looking into what might be going on with OpenGL...

iche033 · 2021-12-09T05:59:40Z

Disabling the glEnable and glDisable calls in ign-rendering:

ah yes. Here is a PR to fix the issue: gazebosim/gz-rendering#509

I'm a bit stuck for testing on Ubuntu at the moment

hmm ok I'll play around a bit and see what's causing the issue

srmainwaring · 2022-01-13T18:04:03Z

Hi @iche033 - I've just been through updating all my dependencies on the latest for garden and I think all the upstream changes are in for this PR now.

Question: the issue noted above re. sensors has to do with the accompanying PR in ign-gazebo rather than this one (I think?). Is your preference to get them both PRs in at the same time, or would it be ok to have this change accepted first and then get the remaining issue with the downstream PR gazebosim/gz-sim#1225 resolved in a second step?

iche033 · 2022-01-13T21:51:03Z

we can get this in first since it looks like the issue is mainly in ign-gazebo. I'll take another look at the changes

srmainwaring · 2022-01-13T23:08:43Z

Thank you, that would be great.

Is there an update to the ogre2.2 version required for garden? I've noticed this exception after pulling changes into rendering, gui, sensors and gazebo.

libc++abi: terminating with uncaught exception of type Ogre::InvalidParametersException: OGRE EXCEPTION(2:InvalidParametersException): Named constants have not been initialised, perhaps a compile error. in GpuProgramParameters::_findNamedConstantDefinition at /Users/rhys/Code/ogre/ogre-next2.2/OgreMain/src/OgreGpuProgramParams.cpp (line 2210)
Stack trace (most recent call last) in thread 123145547399168:
#2    Object "libsystem_platform.dylib", at 0x7fff204f9d7c, in _sigtramp + 28
#1    Object "libignition-tools-backward.dylib", at 0x10da90fad, in backward::SignalHandling::sig_handler(int, __siginfo*, void*) + 13
#0    Object "libignition-tools-backward.dylib", at 0x10da91016, in backward::SignalHandling::handleSignal(int, __siginfo*, void*) + 70
zsh: abort      ign gazebo -v4 -r -s src/ign-gazebo/examples/worlds/depth_camera_sensor.sdf

At first I though it might be an Xcode version, as I was linking against the osrf/simulation tap and had upgraded to Xcode 13.1, but I've rebuilt ogre2.2 from source and same error.

There is also a LOD type issue that has crept in which I'm trying to find the cause of.

gazebo_rendering_lod_2022-01.mov

srmainwaring · 2022-02-16T09:21:45Z

Thanks @scpeters, I think that patch was intended to go in osrf/homebrew-simulation#1645 but was accidentally missed. That should complete all the upstream dependencies for this PR.

iche033 · 2022-02-26T05:22:55Z

I was finally able to test this on an M1 macbook running Monterey. After a few patches here and there and working around rpath issues, it's working!

srmainwaring · 2022-02-26T11:30:26Z

I was finally able to test this on an M1 macbook running Monterey.

Great to know it's running on the new macbooks.

I build on macOS leaving SIP enabled using the following build command:

colcon build --merge-install --cmake-args \
  -DCMAKE_BUILD_TYPE=RelWithDebInfo \
  -DCMAKE_MACOSX_RPATH=FALSE \
  -DCMAKE_INSTALL_NAME_DIR=$(pwd)/install/lib

The binaries are not relocatable, but that isn't so much of an issue. Are the other RPATH patches to deal with the different location that brew installs to on arm64 machines?

iche033 · 2022-02-28T19:25:40Z

yeah I saw that you mentioned the -DCMAKE_MACOSX_RPATH arg in one the ign-gazebo PRs and that helped. I also had to do the same thing in the ogre2.2.rb homebrew formula, see osrf/homebrew-simulation#1823 (comment). I can create a PR for this if that doesn't affect other macOS systems.

scpeters

I've managed to get this working on my Mac laptop. Is it possible to automatically set the graphics_api parameter to metal on macOS? I don't think we should require users to modify their gui.config file in order to use it.

<graphics_api>metal</graphics_api>

src/plugins/minimal_scene/MinimalScene.hh

- Add list of available graphics APIs - Set default graphics API to 'metal' if the platform is Apple, otherwise use 'opengl'. Signed-off-by: Rhys Mainwaring <[email protected]>

srmainwaring · 2022-03-01T13:59:05Z

I don't think we should require users to modify their gui.config file in order to use it.

I've changed the default graphics_api in MinimalScene to be platform specific. It can still be customised if needed (in case OpenGL is re-enabled on macOS or other choices such as Vulkan become available). An alternative would be to provide a DefaultGraphicsAPI method in ign-rendering, which might be nice but would add an upstream dependency to this PR.

ahcorde

LGTM, I only have some suggestions related with the style

src/plugins/minimal_scene/MinimalScene.hh

src/plugins/minimal_scene/MinimalScene.cc

src/plugins/minimal_scene/MinimalSceneRhiOpenGL.cc

src/plugins/minimal_scene/MinimalSceneRhiMetal.mm

src/plugins/minimal_scene/MinimalSceneRhiMetal.hh

src/plugins/minimal_scene/MinimalSceneRhi.hh

src/plugins/minimal_scene/MinimalSceneRhiOpenGL.cc

darksylinc · 2022-03-04T15:25:28Z

This PR likely overlaps / conflicts with mine (#357) but that's not a problem.

This one should be higher priority and I'll fix the problems in my PR after this one is merged.

srmainwaring · 2022-03-04T15:39:17Z

some suggestions related with the style

Thanks @ahcorde, your suggestions have been adopted.

ahcorde · 2022-03-04T15:42:56Z

some suggestions related with the style

Thanks @ahcorde, your suggestions have been adopted.

Thank you for your great work, I will test this locally again and I will merge it

ahcorde · 2022-03-04T15:43:31Z

Last 2 commits are not signed, do you mind to fix it ?

ahcorde · 2022-03-04T16:49:35Z

I'm able to run shapes.sdf but I'm not able to run camera_sensor.sdf. I think this is not an error of this PR.

Here you can find the backtrace:

``` Stack trace (most recent call last) in thread 123145463980032: #26 Object "libsystem_pthread.dylib", at 0x7fff208da8fc, in _pthread_start + 224 #25 Object "libignition-gazebo7-sensors-system.", at 0x12020ff85, in void* std::__1::__thread_proxy >, void (ignition::gazebo::v7::systems::SensorsPrivate::*)(), ignition::gazebo::v7::systems::SensorsPrivate*> >(void*) + 101 #24 Object "libignition-gazebo7-sensors-system.", at 0x1202107ae, in void std::__1::__thread_execute >, void (ignition::gazebo::v7::systems::SensorsPrivate::*)(), ignition::gazebo::v7::systems::SensorsPrivate*, 2ul>(std::__1::tuple >, void (ignition::gazebo::v7::systems::SensorsPrivate::*)(), ignition::gazebo::v7::systems::SensorsPrivate*>&, std::__1::__tuple_indices<2ul>) + 62 #23 Object "libignition-gazebo7-sensors-system.", at 0x12021086e, in decltype(*(std::__1::forward(fp0)).*fp()) std::__1::__invoke(void (ignition::gazebo::v7::systems::SensorsPrivate::*&&)(), ignition::gazebo::v7::systems::SensorsPrivate*&&) + 110 #22 Object "libignition-gazebo7-sensors-system.", at 0x1201d36eb, in ignition::gazebo::v7::systems::SensorsPrivate::RenderThread() + 187 #21 Object "libignition-gazebo7-sensors-system.", at 0x1201d2b8a, in ignition::gazebo::v7::systems::SensorsPrivate::RunOnce() + 202 #20 Object "libignition-gazebo7-rendering.7.0.0", at 0x12066d211, in ignition::gazebo::v7::RenderUtil::Update() + 4273 #19 Object "libignition-gazebo7-rendering.7.0.0", at 0x1209df279, in ignition::gazebo::v7::SceneManager::CreateVisual(unsigned long long, sdf::v13::Visual const&, unsigned long long) + 2121 #18 Object "libignition-gazebo7-rendering.7.0.0", at 0x1209e0882, in ignition::gazebo::v7::SceneManager::LoadGeometry(sdf::v13::Geometry const&, ignition::math::v7::Vector3&, ignition::math::v7::Pose3&) + 1826 #17 Object "libignition-rendering7.7.0.0~pre1.d", at 0x1234a06e7, in ignition::rendering::v7::BaseScene::CreatePlane() + 151 #16 Object "libignition-rendering7-ogre.7.0.0~p", at 0x1250e5fb2, in ignition::rendering::v7::OgreScene::CreatePlaneImpl(unsigned int, std::__1::basic_string, std::__1::allocator > const&) + 114 #15 Object "libignition-rendering7-ogre.7.0.0~p", at 0x1250e6171, in ignition::rendering::v7::OgreScene::CreateMeshImpl(unsigned int, std::__1::basic_string, std::__1::allocator > const&, std::__1::basic_string, std::__1::allocator > const&) + 113 #14 Object "libignition-rendering7-ogre.7.0.0~p", at 0x1250e6207, in ignition::rendering::v7::OgreScene::CreateMeshImpl(unsigned int, std::__1::basic_string, std::__1::allocator > const&, ignition::rendering::v7::MeshDescriptor const&) + 87 #13 Object "libignition-rendering7-ogre.7.0.0~p", at 0x125050b3f, in ignition::rendering::v7::OgreMeshFactory::Create(ignition::rendering::v7::MeshDescriptor const&) + 175 #12 Object "libignition-rendering7-ogre.7.0.0~p", at 0x125051062, in ignition::rendering::v7::OgreMeshFactory::OgreEntity(ignition::rendering::v7::MeshDescriptor const&) + 34 #11 Object "libignition-rendering7-ogre.7.0.0~p", at 0x1250511a5, in ignition::rendering::v7::OgreMeshFactory::Load(ignition::rendering::v7::MeshDescriptor const&) + 101 #10 Object "libignition-rendering7-ogre.7.0.0~p", at 0x125053052, in ignition::rendering::v7::OgreMeshFactory::LoadImpl(ignition::rendering::v7::MeshDescriptor const&) + 6418 #9 Object "RenderSystem_GL.1.9.0.dylib", at 0x126fd984e, in Ogre::HardwareBufferManager::createVertexBuffer(unsigned long, unsigned long, Ogre::HardwareBuffer::Usage, bool) + 22 #8 Object "RenderSystem_GL.1.9.0.dylib", at 0x126fcaa58, in Ogre::GLHardwareBufferManagerBase::createVertexBuffer(unsigned long, unsigned long, Ogre::HardwareBuffer::Usage, bool) + 80 #7 Object "RenderSystem_GL.1.9.0.dylib", at 0x126fcf0ac, in Ogre::GLHardwareVertexBuffer::GLHardwareVertexBuffer(Ogre::HardwareBufferManagerBase*, unsigned long, unsigned long, Ogre::HardwareBuffer::Usage, bool) + 64 #6 Object "libGL.dylib", at 0x7fff6bff1e67, in glGenBuffersARB + 19 #5 Object "libsystem_platform.dylib", at 0x7fff2091fd7c, in _sigtramp + 28 #4 Object "libignition-tools-backward.dylib", at 0x10ac60422, in backward::SignalHandling::sig_handler(int, __siginfo*, void*) + 34 #3 Object "libignition-tools-backward.dylib", at 0x10ac6066a, in backward::SignalHandling::handleSignal(int, __siginfo*, void*) + 106 #2 Object "libignition-tools-backward.dylib", at 0x10ac60788, in backward::StackTraceImpl::load_from(void*, unsigned long, void*, void*) + 56 #1 Object "libignition-tools-backward.dylib", at 0x10ac608e1, in backward::StackTraceImpl::load_here(unsigned long, void*, void*) + 129 #0 Object "libignition-tools-backward.dylib", at 0x10ac620f0, in unsigned long backward::details::unwind::callback>(backward::StackTraceImpl::callback, unsigned long) + 32 Segmentation fault: 11 ```

ahcorde · 2022-03-04T17:04:09Z

my bad, I forgot to use only ogre2. All good!

Co-authored-by: Alejandro Hernández Cordero <[email protected]> Signed-off-by: Rhys Mainwaring <[email protected]>

srmainwaring · 2022-03-04T17:45:47Z

Last 2 commits are not signed, do you mind to fix it ?

Should be good now, btw is there an automated way to add the sign-off when committing from the review page in GitHub?

scpeters · 2022-03-04T18:11:47Z

Last 2 commits are not signed, do you mind to fix it ?

Should be good now, btw is there an automated way to add the sign-off when committing from the review page in GitHub?

sadly no, I copy and paste it into the commit message when using the web UI

srmainwaring requested review from chapulina and jennuine as code owners November 30, 2021 19:30

github-actions bot added the 🌱 garden Ignition Garden label Nov 30, 2021

srmainwaring marked this pull request as draft November 30, 2021 19:31

srmainwaring changed the title ~~Add Metal support to MinimalScene and Qt Application [Draft - Do Not Merge]~~ Add Metal support to MinimalScene and Qt Application Nov 30, 2021

srmainwaring commented Nov 30, 2021

View reviewed changes

src/Application.cc Show resolved Hide resolved

srmainwaring commented Nov 30, 2021

View reviewed changes

src/plugins/minimal_scene/MinimalScene.hh Show resolved Hide resolved

srmainwaring commented Nov 30, 2021

View reviewed changes

src/plugins/minimal_scene/MinimalScene.hh Show resolved Hide resolved

srmainwaring commented Nov 30, 2021

View reviewed changes

src/plugins/minimal_scene/MinimalSceneRhi.hh Show resolved Hide resolved

chapulina added the macOS macOS support label Dec 1, 2021

chapulina requested review from ahcorde and iche033 December 1, 2021 06:19

srmainwaring mentioned this pull request Dec 1, 2021

Add Metal support to Gazebo for macOS gazebosim/gz-sim#1225

Merged

8 tasks

ahcorde reviewed Dec 3, 2021

View reviewed changes

src/plugins/minimal_scene/MinimalScene.hh Outdated Show resolved Hide resolved

src/plugins/minimal_scene/MinimalSceneRhi.hh Outdated Show resolved Hide resolved

srmainwaring force-pushed the feature/ign-gui7-metal-minimalscene branch from b96632f to c91214a Compare December 4, 2021 18:40

srmainwaring marked this pull request as ready for review December 4, 2021 18:46

srmainwaring requested a review from ahcorde December 4, 2021 18:56

srmainwaring mentioned this pull request Dec 7, 2021

Examples not working on macOS with Ogre2.2 gazebosim/gz-rendering#422

Closed

iche033 mentioned this pull request Dec 9, 2021

Check for OpenGL backend before calling GL APIs gazebosim/gz-rendering#509

Merged

7 tasks

chapulina added the needs upstream release Blocked by a release of an upstream library label Dec 10, 2021

chapulina requested a review from scpeters February 28, 2022 19:36

scpeters reviewed Mar 1, 2022

View reviewed changes

src/plugins/minimal_scene/MinimalScene.hh Outdated Show resolved Hide resolved

[Metal] incorporate feedback from review

6749a92

- Add list of available graphics APIs - Set default graphics API to 'metal' if the platform is Apple, otherwise use 'opengl'. Signed-off-by: Rhys Mainwaring <[email protected]>

scpeters approved these changes Mar 1, 2022

View reviewed changes

Merge branch 'main' into feature/ign-gui7-metal-minimalscene

5f7e81d

ahcorde approved these changes Mar 4, 2022

View reviewed changes

srmainwaring and others added 2 commits March 4, 2022 17:39

Update src/plugins/minimal_scene/MinimalScene.hh

5b0e214

Co-authored-by: Alejandro Hernández Cordero <[email protected]> Signed-off-by: Rhys Mainwaring <[email protected]>

Apply suggestions from code review

f22261c

Co-authored-by: Alejandro Hernández Cordero <[email protected]> Signed-off-by: Rhys Mainwaring <[email protected]>

srmainwaring force-pushed the feature/ign-gui7-metal-minimalscene branch from d5cc37e to f22261c Compare March 4, 2022 17:42

ahcorde merged commit 63b3599 into gazebosim:main Mar 4, 2022

srmainwaring deleted the feature/ign-gui7-metal-minimalscene branch March 5, 2022 14:21

srmainwaring mentioned this pull request Mar 18, 2022

Add Metal support for the Ogre2 Render Engine gazebosim/gz-rendering#461

Closed

18 tasks

iche033 mentioned this pull request Apr 6, 2022

ogre formulae don't compile on Macs with M1 arm processor osrf/homebrew-simulation#1628

Closed

srmainwaring mentioned this pull request Aug 7, 2022

Add Metal support to MinimalScene and Qt Applications #314

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Metal support to MinimalScene and Qt Application #323

Add Metal support to MinimalScene and Qt Application #323

srmainwaring commented Nov 30, 2021 •

edited

Loading

srmainwaring commented Dec 4, 2021

iche033 commented Dec 7, 2021

srmainwaring commented Dec 8, 2021 •

edited

Loading

srmainwaring commented Dec 8, 2021

iche033 commented Dec 9, 2021

srmainwaring commented Jan 13, 2022

iche033 commented Jan 13, 2022

srmainwaring commented Jan 13, 2022

srmainwaring commented Feb 16, 2022

iche033 commented Feb 26, 2022

srmainwaring commented Feb 26, 2022 •

edited

Loading

iche033 commented Feb 28, 2022

scpeters left a comment

srmainwaring commented Mar 1, 2022

ahcorde left a comment

darksylinc commented Mar 4, 2022

srmainwaring commented Mar 4, 2022

ahcorde commented Mar 4, 2022

ahcorde commented Mar 4, 2022

ahcorde commented Mar 4, 2022

ahcorde commented Mar 4, 2022

srmainwaring commented Mar 4, 2022

scpeters commented Mar 4, 2022

Add Metal support to MinimalScene and Qt Application #323

Add Metal support to MinimalScene and Qt Application #323

Conversation

srmainwaring commented Nov 30, 2021 • edited Loading

🎉 New feature

Summary

Approach

Platform configuration

Test it

Tests

Code check

Checklist

srmainwaring commented Dec 4, 2021

Threads

Initialisation

iche033 commented Dec 7, 2021

srmainwaring commented Dec 8, 2021 • edited Loading

srmainwaring commented Dec 8, 2021

iche033 commented Dec 9, 2021

srmainwaring commented Jan 13, 2022

iche033 commented Jan 13, 2022

srmainwaring commented Jan 13, 2022

srmainwaring commented Feb 16, 2022

iche033 commented Feb 26, 2022

srmainwaring commented Feb 26, 2022 • edited Loading

iche033 commented Feb 28, 2022

scpeters left a comment

Choose a reason for hiding this comment

srmainwaring commented Mar 1, 2022

ahcorde left a comment

Choose a reason for hiding this comment

darksylinc commented Mar 4, 2022

srmainwaring commented Mar 4, 2022

ahcorde commented Mar 4, 2022

ahcorde commented Mar 4, 2022

ahcorde commented Mar 4, 2022

ahcorde commented Mar 4, 2022

srmainwaring commented Mar 4, 2022

scpeters commented Mar 4, 2022

srmainwaring commented Nov 30, 2021 •

edited

Loading

srmainwaring commented Dec 8, 2021 •

edited

Loading

srmainwaring commented Feb 26, 2022 •

edited

Loading