Skip to content
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

Add a method to render targets and cameras to retrieve a Metal texture #477

Conversation

srmainwaring
Copy link
Contributor

🎉 New feature

Relates to: #461 and discussions in #422. It accompanies PR #463 but does not depend upon it.

Summary

This PR provides access to the underlying Metal texture object. The functions MetalId() and RenderTextureMetalId()
provide the Metal equivalent of GLId() and RenderTextureGLId() for OpenGL.

The functions expect a parameter void* _texturePtr which is the address of a pointer to void* as the object required for Metal is an Objective-C type id<MTLTexture> which we do not want exposed in the interface (the type is the id for an object
that implements the MTLTexture protocol).

Note: the functions do not add a dependency on Objective-C to the ignition rendering libraries, but they are intended to be used in Objective-C code as follows:

void *textureIdPtr = nullptr;
texture->RenderTextureMetalId(&textureIdPtr);
id<MTLTexture> metalTexture = CFBridgingRelease(textureIdPtr);

The translation unit containing the code should be compiled with ARC enabled and will need to link against the AppKit and Metal frameworks. When using cmake this is accomplished by CMakeLists.txt entries such as:

target_link_libraries(simple_demo_qml_metal PUBLIC
    ${IGNITION-RENDERING_LIBRARIES}
    ${OGRE2_LIBRARIES}
    ${OPENGL_LIBRARIES}
    Qt5::Core
    Qt5::Gui
    Qt5::Qml
    Qt5::Quick
    "-framework AppKit"
    "-framework Metal"
)

# Enable ARC on selected source files
set_source_files_properties(
    IgnitionRenderer.mm
    RenderThread.mm
    ThreadRenderer.mm
    PROPERTIES
    COMPILE_FLAGS
        "-fobjc-arc"
)

There is a runtime dependency on an upstream change to ogre-next: OGRECave/ogre-next@3b11873. Note that this PR does not require the upstream change to build as the Ogre interface used, getCustomAttribute, is available on the base class Ogre::TextureGpu and is used by the OpenGL functions.

Test it

These changes have no impact on existing functionality using the OpenGL render system.

To test these functions requires a version of ogre-next v2-2 that includes the change OGRECave/ogre-next@3b11873.

There is work in progress to enable the use of Metal for the example simple_demo_qml and for ign-gui and ign-gazebo. They depend upon this PR and provide a means to test the changes:

https://github.com/srmainwaring/ign-rendering/tree/feature/ign-rendering6-metal-simple_demo_qml
https://github.com/srmainwaring/ign-gui/tree/feature/ign-gui7-metal
https://github.com/srmainwaring/ign-gazebo/tree/feature/ign-gazebo7-metal

For more detail see comment: #463 (comment)

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
  • A number of tests fail on macOS - these are unrelated to this PR:
91% tests passed, 9 tests failed out of 102

Total Test time (real) = 138.35 sec

The following tests FAILED:
	 19 - UNIT_Heightmap_TEST (SEGFAULT)
	 59 - UNIT_RenderingIface_TEST (SEGFAULT)
	 83 - INTEGRATION_depth_camera (Failed)
	 85 - INTEGRATION_camera (Failed)
	 87 - INTEGRATION_render_pass (Failed)
	 89 - INTEGRATION_shadows (Failed)
	 93 - INTEGRATION_segmentation_camera (Failed)
	 95 - INTEGRATION_sky (Failed)
	 97 - INTEGRATION_thermal_camera (Failed)

Note to maintainers: Remember to use Squash-Merge

@srmainwaring srmainwaring requested a review from iche033 as a code owner October 21, 2021 12:31
@github-actions github-actions bot added 🌱 garden Ignition Garden 🏯 fortress Ignition Fortress labels Oct 21, 2021
@srmainwaring srmainwaring changed the title [Metal] Add a method to render targets and cameras to retrieve the Metal texture id Add a method to render targets and cameras to retrieve a Metal texture Oct 21, 2021
srmainwaring added a commit to srmainwaring/gz-gui that referenced this pull request Oct 21, 2021
- Fix build after changes in gazebosim/gz-rendering#477

Signed-off-by: Rhys Mainwaring <[email protected]>
@ahcorde
Copy link
Contributor

ahcorde commented Oct 21, 2021

This is breaking ABI, you should target main, isn't it @iche033 ?

@srmainwaring
Copy link
Contributor Author

srmainwaring commented Oct 21, 2021

This is breaking ABI, you should target main, isn't it @iche033 ?

You're right - I'll update the PR accordingly.

Closing in lieu of #478 which is based on main

@srmainwaring srmainwaring changed the base branch from ign-rendering6 to main October 21, 2021 15:13
chapulina and others added 21 commits October 21, 2021 16:16
Signed-off-by: Louise Poubel <[email protected]>
* Added macos instructions

Signed-off-by: Alejandro Hernández <[email protected]>

* Fix

Signed-off-by: Alejandro Hernández <[email protected]>

Co-authored-by: Ian Chen <[email protected]>
Signed-off-by: Ian Chen <[email protected]>

Co-authored-by: Alejandro Hernández Cordero <[email protected]>
…_buffer_range is not available (gazebosim#462)

- The definition of bufferFetch1 was failing for OpenGL < 4.2 resulting in compile errors for lighting shaders

Signed-off-by: Rhys Mainwaring <[email protected]>
- Fix a bug where the warning of an invalid anti-aliasing level in Ogre2RenderTarget was not being output
- Set the FSAA level to 0 if the target level is invalid
- Provide a list of valid options in the warning

Signed-off-by: Rhys Mainwaring <[email protected]>
* fix ray query dist calc

Signed-off-by: Ian Chen <[email protected]>

* disable test

Signed-off-by: Ian Chen <[email protected]>
Signed-off-by: GitHub <[email protected]>

Co-authored-by: Alejandro Hernández Cordero <[email protected]>
Co-authored-by: Jose Luis Rivero <[email protected]>
* Avoid symbol redefiniition on armel builds

Signed-off-by: Jose Luis Rivero <[email protected]>
…m#465)

* Fix logic on warning for ogre versions different than 1.9.x

Signed-off-by: Jose Luis Rivero <[email protected]>
…tal texture id

- These functions provide the Metal equivalent of GLId() and RenderTextureGLId() for OpenGL.
- The argument is the address of a pointer to void* as the object required for Metal is an objective-c type id<MTLTexture> which we do not want exposed in the interface
- There is a runtime dependency on an upstream change to ogre-next: OGRECave/ogre-next@3b11873

Signed-off-by: Rhys Mainwaring <[email protected]>
- Ensure line length limits are adhered to.

Signed-off-by: Rhys Mainwaring <[email protected]>
@srmainwaring srmainwaring force-pushed the feature/ign-rendering6-metal-texture branch from 3a8fc90 to c3a25b6 Compare October 21, 2021 15:18
@srmainwaring srmainwaring deleted the feature/ign-rendering6-metal-texture branch October 21, 2021 15:29
@srmainwaring srmainwaring restored the feature/ign-rendering6-metal-texture branch October 21, 2021 15:30
srmainwaring added a commit to srmainwaring/gz-gui that referenced this pull request Nov 25, 2021
- Fix build after changes in gazebosim/gz-rendering#477

Signed-off-by: Rhys Mainwaring <[email protected]>
srmainwaring added a commit to srmainwaring/gz-gui that referenced this pull request Nov 30, 2021
- Set ogre2 render engine as default and ensure the Qt surface format supports OpenGL 4.1
- Default ogre render engine to version to ogre2 in CMakeLists.txt, scene3d.config and Scene3D.
- Set default QSurfaceFormat to OpenGL 4.1 in applications
- cherry-pick 066d276
- Experimental support for Metal in Scene3D and MinimalScene plugins (OpenGL is disabled)
- Modify examples/standalone/window to use Metal backend in the Qt scene graph and force render loop to be single threaded
- Modify ign.cc commandlets to use Metal backend in the Qt scene graph and force render loop to be single threaded
- Use CFBridgingRetain / CFBridgingRelease to manage access to Metal texture objects.
- Force render engine to initialise on the main thread.
- Fix build after interface changes
- Fix build after changes in gazebosim/gz-rendering#477
- Reverse changes to isolate issues in transform control
- Add cross-platform support to MinimalScene TextureNode
- Introduce abstract delegate class for the TextureNode
- Add implementations for OpenGL and Metal
- Add cross-platform support to MinimalScene RenderThread
- Introduce abstract delegate class for the RenderThread
- Add implementations for OpenGL and Metal
- Update CMakeLists.txt for cross-platform
- Only include objective-c and Apple frameworks for macOS
- Add render interface for a camera texture
- Change name of render interface files
- Refactor file names to use `Rhi` suffix
- Refactor names of render interface classes
- Replace Delegate with Rhi (render hardware interface - from Qt)
- Add render hardware interface for the ignition renderer
- Update MinimalScene to use a render hardware interface to retrieve the camera texture
- Convert MinimalScene back to c++
- All Metal specific code moved out of MinimalScene to render interface classes
- Rename MinimalScene and compile as c++
- Allow render system to be switched between Metal and OpenGL
  - Add method SetRenderSystem
  - Update classes in MinimalScene to allow the render system to be set in the plugin XML
  - Move all scene graph backend config to Application
- Set textureDirty flag true on initialisation
- Fix an issue where the initial texture is not sized correctly on initialisation leading to aliasing artefacts
- Fix after rebasing onto ign-gui6

[QML] update function call syntax in Connections blocks
- Change syntax to address warning: QML Connections: Implicitly defined onFoo properties in Connections are deprecated
- Qt 5.15, see: https://doc.qt.io/qt-5/qml-qtqml-connections.html
- Add missing properties for tooltopDelay
- cherry-pick ac51e07

Signed-off-by: Rhys Mainwaring <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏯 fortress Ignition Fortress 🌱 garden Ignition Garden
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants