Add Metal support for the Ogre2 Render Engine #463
Conversation
|
@ahcorde I have put together an example to test the depth camera based on the examples I'm wondering whether to push it to this PR or to a create new one and then cherry pick it in later, as there is quite a lot here already?
|
+1 on creating a new PR for this. Makes it easier to review. Thanks! |
|
I'm testing your code but I found this issue: I commented out theses lines: But I'm still getting the segfault. any ideas ? should I cherry-pick some other commit ? |
|
Hi @ahcorde - thanks for reviewing. I'm wondering if the issue you are seeing is related to #456? The selection buffer material (and a few others) reference other shader aliases. The error message is a bit cryptic but it looks like it may not be finding Permalink does not seem to be working... L52 is the reference // Metal shaders
fragment_program selection_buffer_fs_Metal metal
{
source selection_buffer_fs.metal
shader_reflection_pair_hint DepthCameraVS_Metal
}
// Unified shaders
fragment_program selection_buffer_fs unified
{
delegate selection_buffer_fs_GLSL
delegate selection_buffer_fs_Metal
}
material SelectionBuffer
{
// Material has one technique
technique
{
// This technique has one pass
pass
{
// Make this pass use the vertex shader defined above
vertex_program_ref DepthCameraVS
{
}Also I've found Metal to be unforgiving if anything fails while setting up the command buffer, the OpenGL version will usually carry on and render whatever works and only cause a segmentation fault on compile errors. Edit: as this looks to be a potential cause of error, I'll update the material scripts to only reference locally defined vertex and fragment program definitions. Edit2: the |
|
Thank you for the quick patch. I'm not getting the segfault, but the window is completely black. |
That's not good. What example are you running ( |
|
macOS Big Sur 11.6 and XCODE Version 12.5.1 (I think I can update to version 13.0.0) |
|
I'm on Big Sur 11.4 with Xcode 12.5, so I don't think that's it. I'm using a later version of Ogre v2.2 than the one in the osrf/simulation/ogre2.2.rb formula so that may be a cause? I'll roll back and check that next. Other than that I'm a bit stumped. Does anything show up when you look at the frame debugger in Xcode?
|
|
I tried this other PR #467 and I'm able to visualize the depth camera. I have never run the code with xcode, let me try it |
|
Is the depth_camera example running with To use Xcode with the examples I create another build folder in the example folder, say This is the frame debugger guide: https://developer.apple.com/documentation/metal/frame_capture_debugging_tools/enabling_frame_capture I found I had to set GPU Frame Capture to Metal aa otherwise the camera icon will not appear in the debug window. The command line and environment variables can be set in the same Edit Scheme window. |
|
This is the output. It looks different
|
|
That is odd. The depth buffer is rendering, and it's hard to see at the zoom level, but it looks like the second to last render of Command Buffer 2 has a valid colour texture (expand Command Buffer 2 and look at Render 9) - although I'm seeing 4 attachments and it looks like there are only 3 in your case. This could indicate a different workflow (i.e. different versions of Ogre v2.2). If you expand Render 9 further and hover your mouse over the calls you will get a preview showing the progress of the render as you mouse down. For some reason that's failing for you. I'll install the |
|
I checked the image again and I added the last part of the diagram.
|
|
I've rolled back Ogre2.2 to match the osrf/simulation brew version and cannot replicate the issue you're seeing. Could you export the gpu trace and make it available on a shared drive (dropbox or equivalent). To export: highlight the project in the left-hand pane, then from the menu: File > Export and you should get a Are you able to get any of the other examples working using In the meanwhile I will try to get this project set up on another machine - a MacBook this time to see if there's a hardware issue, and I'll also look into moving to Big Sur 11.6 and Xcode 12.5.1 on that machine so see if there is anything in that. |
|
here we go: |
|
Thanks for the trace. The first thing I can see is that the trace will not run on my Mac Pro as it says Edit: ok, so the trace won't play on my MacBook Pro (2013) either, but it has a very similar spec to your machine (same card and model is MacBookPro11,1). Perhaps it the OS and Xcode version? |
Graphics: |
|
@ahcorde I can replicate the behaviour on my MacBook Pro. The camera examples work:
But the other examples that use the regular camera are not working. I think the problem might be that the Ogre HlmsUnlit materials use Metal 2 syntax in the vertex shader. Both our MacBook Pro's only support Metal GPUFamily macOS 1. My Mac Pro workstation supports Metal GPUFamily macOS 2. I'll need to check with @darksylinc to confirm. |
|
I stick to the earliest syntax I can for best iOS compatibility. Anyway, syntax support boils down to macOS version, not GPU version (*). If we're using a newer syntax it will fail to compile and a new OS version is needed (*) As long as we're not using a feature specific for nee GPUs, which afaik we don't |
|
One simple cause could be load/store actions. If they're incorrectly set, some GPUs will completely ignore it while on others rendering will be garbage. Since we support load/store in all APIs try hooking rendedoc+gl+linux because RenserDoc has debugging facilities to easily spot incorrect use of dont_care. The debug looks like this: |
|
Bte I suspect msaa is involved (call it a sixth sense) try disabling msaa, iirc it's on by default in ignition |
- Revert debug output in Ogre2RenderEngine - Fix codecheck issue with variable declaration shadows previous local - Revert changes to simple_demo_qml - Move useMetalRenderSystem to Ogre2RenderEnginePrivate Signed-off-by: Rhys Mainwaring <[email protected]>
- Add enum class GraphicsAPI to RenderEngine.hh - Add utils class GraphicsAPIUtils for setting enum from string - Update Ogre2RenderEngine to use GraphicsAPI enum - Update examples to use GraphicsAPI enum Signed-off-by: Rhys Mainwaring <[email protected]>
- Move GraphicsAPI to own file Signed-off-by: Rhys Mainwaring <[email protected]>
- Fix custom_scene_viewer Signed-off-by: Rhys Mainwaring <[email protected]>
- Fix code check - Change GraphicsAPIUtils class documentation - Improve graphics API check in Ogre2RenderEngine Signed-off-by: Rhys Mainwaring <[email protected]>
- Fix code check - uninitialised variable warning Signed-off-by: Rhys Mainwaring <[email protected]>
|
Hi @srmainwaring, I'm trying to compile your changes but I'm not able.
But I'm getting this error: minimal_scene/MinimalSceneRhiMetal.mm:86:12: error: no member named 'RenderTextureMetalId' in 'ignition::rendering::Camera'
_camera->RenderTextureMetalId(&texturePtr);
~~~~~~~~~^Are these the right branches ? |
|
@ahcorde you need to use main for ign-rendering as the changes to expose the Metal texture were ABI breaking. Let me know if there are any issues getting the gui and gazebo branches to build and I’ll patch them up. They should be ok as I checked them on an old macbook as well as a workstation. I have to update the gui changes to use the GraphicsAPI enum, but would like to do this once this PR has been merged and bumped to main. |
|
@ahcorde do you want give this another test on your mac machine? As of now, you'll need to test this branch by itself. The other PRs for full ign-gazebo + Metal integration will likely target Garden and not Fortress. |
|
yes @iche033, I will test it today |
|
I was able to compile it but I'm getting this trace when I try to run it: ign gazebo -v 4 -g```bashign gazebo -v 4 -g |
Hi @ahcorde it looks like you may be missing the <plugin filename="MinimalScene" name="3D View">
<ignition-gui>
<title>3D View</title>
<property type="bool" key="showTitleBar">false</property>
<property type="string" key="state">docked</property>
</ignition-gui>
<engine>ogre2</engine>
<scene>scene</scene>
<ambient_light>0.4 0.4 0.4</ambient_light>
<background_color>0.8 0.8 0.8</background_color>
<camera_pose>-6 0 6 0 0.5 0</camera_pose>
<render_system>metal</render_system>
</plugin>How about we move the discussion of the ign-gui and ign-gazebo support to gazebosim/gz-gui#314 as they build upon this PR but don't affect it directly (I've probably confused issues by referencing the gui / gazebo work in this thread). |
I don't see any API/ABI changes in public headers, so we're good on that front. Does it break behavior or affect users negatively any other way? Maybe we should revert #407 and see if the Ogre 2 tests pass on macOS CI with this PR? |
The PR was set up to not change the default behaviour for macOS, so it will not alter the current CI tests for macOS (which will still use the OpenGL graphics API with Ogre2). I do have a local branch with a set of proposed changes to the tests which allow an additional graphics API parameter to be passed to the gtest runners, but thought it better to leave that for a separate PR as it has a wider impact and there is already a lot in this one. |
|
@srmainwaring awesome contribution! Thank you! |
🎉 New feature
Summary
This PR adds Metal Render System support to the ogre2 render engine.
Relates to: #461 and discussions in #422.
Marked as draft as not all shaders have been ported to Metal and some examples are not working correctly.
Test it
A number of the examples have been modified to allow the Metal to be used with the
ogre2render engine instead of GL3+. For example to use Metal with thesimple_demoexample run:For a list of tasks and the status of the examples see the accompanying feature request: #461
Checklist
codecheckpassed (See contributing)Note to maintainers: Remember to use Squash-Merge
🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸