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 support for Direct3D 12 OpenXR backend #104207

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

mbucchia
Copy link

@mbucchia mbucchia commented Mar 15, 2025

Add support for Direct3D 12 OpenXR backend and rendering with view
instancing (#86283). See godotengine/godot-proposals#10078.

This change adds support for running XR projects built with the d3d12
rendering backend. The XR backend hooks into the setup for the D3D12
render context in order to use the desired device and command queue for
submission to OpenXR. The XR backend takes care of importing the D3D12
swapchain images into the render context.

As part of this process, three issues are addressed:

  • Ensuring that resource state transitions are only done on textures
    that require them.
  • Enabling view instancing in the PSOs for multiview render passes.
  • Addressing a bug in the D3D12 runtime where PSO creation may fail
    when front face detection is used.

Please refer to #86283 for additional discussions on the implementation
details.

----8<----8<----8<----8<----8<----8<----8<----8<----8<----8<----8<----
(remove content below from commit message when squashing this pull request)

For testing, I have been using the https://github.com/GodotVR/godot-xr-template and set it up for D3D12 rendering:

--- a/project.godot
+++ b/project.godot
@@ -54,8 +54,12 @@ common/drop_mouse_on_gui_input_disabled=true

 [rendering]

-renderer/rendering_method="gl_compatibility"
-renderer/rendering_method.mobile="gl_compatibility"
+rendering_device/driver="d3d12"
+rendering_device/driver.windows="d3d12"
+;rendering_device/d3d12/agility_sdk_version=615
+rendering_device/fallback_to_vulkan=false
+rendering_device/fallback_to_opengl3=false
+renderer/rendering_method.mobile="forward_plus"
 textures/vram_compression/import_etc2_astc=true
 environment/defaults/default_environment="res://default_env.tres"

Testing:

  • Nvidia Ada Lovelace + godot-xr-template + Virtual Desktop (VDXR): PASS
  • Nvidia Ada Lovelace + godot-xr-template + Varjo: PASS
  • Nvidia Ada Lovelace + godot-xr-template + Meta XR Simulator: PASS
  • Nvidia Ada Lovelace + godot-xr-template + Meta Quest Link: PASS
  • Nvidia Ada Lovelace + godot-xr-template + SteamVR: PASS
  • Nvidia Ada Lovelace + godot-xr-template + Windows Mixed Reality: FAIL [1]
  • AMD RDNA2 + godot-xr-template + SteamVR: PENDING
  • Intel Arc + godot-xr-template + SteamVR: PENDING
  • godot-xr-template + Agility SDK 615: PASS
  • godot-xr-template + --gpu-validation: PASS

I will perform the PENDING scenarios above once we are closer to merging (these scenarios require me to swap GPU).

[1] Disappointingly, there is an issue with WMR and we crash in xrCreateSwapchain() pretty inexplicably.
Looking into it now, I do not believe it is a bug with this change however.

@mbucchia mbucchia requested review from a team as code owners March 15, 2025 22:53
@mbucchia
Copy link
Author

mbucchia commented Mar 15, 2025

Looking into workflow failures that seem to be due to the use of XrGraphicsRequirementsD3D12KHR.

Update: the clang and MingW build issues are resolved and base workflow passes.

@mbucchia mbucchia force-pushed the d3d12-xr branch 3 times, most recently from 33fdd8b to aa788f3 Compare March 16, 2025 07:17
@mbucchia
Copy link
Author

mbucchia commented Mar 16, 2025

I am a little confused by your CI.

This workflow with latest push passed all green:

https://github.com/mbucchia/godot/actions/runs/13881066387

However the checks below in the PR reflect a CSS error. The details show the formatter replacing #ifdef inclusion guards with #pragma once.

Meanwhile your CSS guide (https://docs.godotengine.org/en/latest/contributing/development/cpp_usage_guidelines.html#pragma-once-directive) says:

#pragma once directive
To follow the existing style, please use standard #ifdef-based include guards instead of #pragma once in new files

What am I missing?

@mbucchia
Copy link
Author

Oh I think I get it - the passing run is from my forked repo. And running the workflow on the Godot repo requires an approval, so the current results are not reflecting the latest push.

@lawnjelly
Copy link
Member

4.x very recently changed to #pragma once instead of header guards (#102298), the documentation may be out of date.

@mbucchia
Copy link
Author

4.x very recently changed to #pragma once instead of header guards (#102298), the documentation may be out of date.

Understood. I will change my new files to use the pragmas. Thanks!

This change adds support for d3d12 rendering devices with OpenXR. This
includes querying the adequate adapter to use for XR composition through
the use of hooks (similar to what is done in the Vulkan support) and
forwarding swapchain images from the OpenXR runtime to the engine.
The function is needed to import textures created by the OpenXR runtime
into the rendering backend. The code uses D3D12 introspection methods
along with the hints forwarded from the rendering backend to create the
necessary texture bookkeeping data.
Many OpenXR runtimes do not use D3D12 as their graphics backend internally
but instead import textures into the D3D12 device used for rendering by
the engine. These textures are marked with FLAG_ALLOW_SIMULTANEOUS_ACCESS
and by definition use automatic layout transitions. The engine shall not
use barriers to perform layout transitions on these textures.
Configure multi-view render passes to use a PSO with D3D12 view instancing
and output each view to a slice of the render target.
There is a bug in the Direct3D runtime during creation of a PSO with view
instancing. If a fragment shader uses front/back face detection built-in
variable (SV_IsFrontFace), its signature must include the pixel position
built-in variable (SV_Position), otherwise an Internal Runtime error will
occur.
@AThousandShips AThousandShips changed the title Adding support for Direct3D 12 OpenXR backend Add support for Direct3D 12 OpenXR backend Mar 16, 2025
@BastiaanOlij
Copy link
Contributor

Just giving this a first look and all seems pretty good. I don't know the DX12 backend well enough to give any insight on that so will need @clayjohn or @DarioSamo to verify the changes.

I'll find some time to do some testing on my end this week.

Re your comment here: #86283 (comment)
With WMR itself defuncts, do we want to put hacks like this in or just make note of them for anyone that wants to custom build for WMR?
I'm assuming here that SteamVR and other PC OpenXR runtimes will work with DX12 as well, the main goal here would be for those who wish to use the DX12 renderer on windows to be able to do VR with Godot.

@mbucchia
Copy link
Author

mbucchia commented Mar 17, 2025

Thank you Bastiaan! Please suggest some more testing as well if you want. I saw a couple of XR demo samples, maybe I will give those a shot. If you have a stress test let me know.

Re your comment here: #86283 (comment)
With WMR itself defuncts, do we want to put hacks like this in or just make note of them for anyone that wants to custom build for WMR?

I'm a little bit torn yeah. I think you really want to get rid of this warning message that calls specifically incompatibility with WMR today. D3D support was an avenue for that.

But also as I mentioned, I don't know if the issue will remain specific to WMR if a platform begins to use D3D12 as its internal gfx API in the future. There is none today AFAICT (most use D3D11 for composition inside the runtime). I might talk some more to the D3D team at Microsoft about these scenarios of "mixed usage" of Agility SDK. Definitely nothing actionable on the current PR though.

@AThousandShips AThousandShips added this to the 4.x milestone Mar 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants