Add support for OpenXR composition layers

[dsnopek]

Supersedes #76877

This builds on the work that @BastiaanOlij did on PR #76877 and makes a few changes:

• Rather than sub-classing SubViewport, it sub-classes Node3D and you refer to a SubViewport to use. This allows positioning the composition layer relative to the XROrigin.
• Rather than using a single class, it makes a parent OpenXRCompositionLayer class, and then several child classes for different types of composition layers: OpenXRCompositionLayerQuad, OpenXRCompositionLayerCylinder and OpenXRCompositionLayerEquirect
• It's able to correctly support SubViewports which are set to an "Update Mode" of "Once" by creating a static OpenXR swapchain. (This requires a tiny change to RenderingServer so that we can see the viewport's update mode flip from "Once" to "Disabled".)

In addition to those changes, it also has the following additional features:

• It will show a "fallback mesh" in the editor, so that you can configure the size/shape of the composition layer and see it represented there
• If the given composition layer isn't supported, it will use the same "fallback mesh" at runtime with a ViewportTexture, in order to emulate the composition layer

I've tested this on the Meta Quest 3 (both real composition layers, and the fallback meshes) and it's working for me :-)

[m4gr3d]

The code looks good to me!

I still need to perform a few rounds of testing.

[BastiaanOlij]

• It's able to correctly support SubViewports which are set to an "Update Mode" of "Once" by creating a static OpenXR swapchain. (This requires a tiny change to RenderingServer so that we can see the viewport's update mode flip from "Once" to "Disabled".)

I need to check the details but reading this, you can't rely on this. Remember that the rendering server can run in a different thread and we're working towards using that on Quest to get extra performance. That means that when reading back this information from the "node" side of Godot you either create a sync point, or you're potentially reading back old data.

[devloglogan]

A couple notes from testing this out for a bit:

1. If multiple OpenXRCompositionLayer nodes use the same SubViewport, only one of them will be able to render it. If this is expected, a warning/error for when someone is trying to do this could be nice.

2. If you set the assigned SubViewport such that transparent_bg is true, I don't see an easy way to make it so that the OpenXRCompositionLayer could become transparent as well. I feel like this would be handy.

3. Scaling an OpenXRCompositionLayer node in the editor throws a lot of errors like this:

Basis [X: (2.007861, 0, 0), Y: (0, 1, 0), Z: (0, 0, 4.475057)] must be normalized in order to be casted to a Quaternion. Use get_rotation_quaternion() or call orthonormalized() if the Basis contains linearly independent vectors.

Though the fallback mesh will be scaled in the editor, and then that scale not applied at runtime.

[BastiaanOlij]

Yeah looking further at this code the way we handle the swapchains needs to change. This is going to cause big problems once we're multithreading. You need to draw a big wall between what happens on the node side, and what happens on the rendering side of Godot. As soon as you're mixing code here, things fall over. On the node side you should only be setting data on the rendering server and assume that code is delayed until rendering starts. You can't read back data here.

Similarly, you don't want to read data from the node side from rendering code.

So first this means, don't store the SubViewport pointer in OpenXRViewportCompositionLayerProvider, store the RID of the viewport here. RIDs are special in that they can be allocated on the main thread before actually being properly updated on the rendering thread. Then in OpenXRCompositionLayer::set_layer_viewport, when you get the RID, store that in the provider.

Then second, the code that updates the swapchain shouldn't happen on process, it should happen as part of XRServer::pre_render which is called from the rendering thread. This calls OpenXRInterface::pre_render which call on_pre_render on our extensions but not on the providers.

It's going to be difficult to test if we get this right because running the renderer on a separate thread doesn't currently work well with XR, I'm working on fixing a number of the more pressing issues around this but when I designed the XRServer and XRInterface many moons ago, this wasn't on the radar at all. So I expect we'll have a bit of cleanup to do as we slowly fix thing up. Best to make sure that anything new we build does this right ;)

Speaking of doing things right, the whole override approach is flawed, another thing you need to add into your code for updating the swapchain is checking whether settings on the viewport have changed (size/MSAA/etc), there are many changes that we don't have hooks for that will undo the override and have Godot rebuild its internal textures.

Ideally, but this is long term thinking, we change the viewport logic so that it can actually switch between a single render target and using a swapchain, by default using a normal VkSwapchain, but then using VulkanHooks to override that logic and redirect it to an XrSwapchain. But I think that is Godot 5 territory ;)

[BastiaanOlij]

Finally, other than this swapchain/renderthread mess, I really like the overall approach here. Really cool that you managed to add in a fallback solution. Can't wait to put it through its proper paces :)

[dsnopek]

@BastiaanOlij Thanks for all the notes!

So first this means, don't store the SubViewport pointer in OpenXRViewportCompositionLayerProvider, store the RID of the viewport here. RIDs are special in that they can be allocated on the main thread before actually being properly updated on the rendering thread. Then in OpenXRCompositionLayer::set_layer_viewport, when you get the RID, store that in the provider.

Then second, the code that updates the swapchain shouldn't happen on process, it should happen as part of XRServer::pre_render which is called from the rendering thread. This calls OpenXRInterface::pre_render which call on_pre_render on our extensions but not on the providers.

This all sounds doable. Would making these changes be sufficient for now? There's more things you describe in order to "do things right" but it sounds like we could perhaps continue to work on that over time, since it would require refactoring a number of systems.

[BastiaanOlij]

So first this means, don't store the SubViewport pointer in OpenXRViewportCompositionLayerProvider, store the RID of the viewport here. RIDs are special in that they can be allocated on the main thread before actually being properly updated on the rendering thread. Then in OpenXRCompositionLayer::set_layer_viewport, when you get the RID, store that in the provider.
Then second, the code that updates the swapchain shouldn't happen on process, it should happen as part of XRServer::pre_render which is called from the rendering thread. This calls OpenXRInterface::pre_render which call on_pre_render on our extensions but not on the providers.

This all sounds doable. Would making these changes be sufficient for now? There's more things you describe in order to "do things right" but it sounds like we could perhaps continue to work on that over time, since it would require refactoring a number of systems.

Yes, I think the blurb up above here catches the two main things that stood out for me, the rest is all future music for improvements down the line.

The major thing to change is to make sure we're doing things on the right thread and understand how things work together. As a rule the following two criteria should always be met:

• We should never be asking data from the rendering system from the main thread, this is a clear indication code is in the wrong place.
• We should never directly access node information from the rendering thread

For that 2nd one, if there is information in the node system needed in code on the rendering thread, it has to be duplicated or made thread safe in some other way.

XR is weird here because the logic in the XRServer and XRInterface has so much overlap with rendering. It doesn't fit super well into the way the rendering server shields threading issues with logic effecting both main threads and the rendering thread. Plus when I designed it, I was too new with Godot to know how these things were organised.

Keep an eye on #89734 where I've started solving the most immediate issues around this.

[dsnopek]

@BastiaanOlij I think I've addressed all your feedback, moving the interactions with the rendering server into OpenXRCompositionLayerExtension::on_pre_render(), and using the RID rather than a pointer to the viewport node. Please let me know what you think!

@devloglogan I've added warnings and errors for all the edge cases that you found. And, I've also added an alpha_blend property to the composition layers that can allow using a viewport with a transparent background.

[BastiaanOlij]

This is looking real good, just a few small nitpicks and some remarks for the future.
I'm happy for this to be merged once the nitpicks have been addressed.

[dsnopek]

@BastiaanOlij Thanks for the review! I've updated the PR for all but the note about cleaning up the old viewport in OpenXRViewportCompositionLayerProvider::set_viewport(). I'm not sure there's a better way to handle that (see above for more details), but perhaps it's OK as-is because it's about clean up, which shouldn't need to happen very often? In most cases, only when switching scenes or shutting down the app altogether.

[BastiaanOlij]

@BastiaanOlij Thanks for the review! I've updated the PR for all but the note about cleaning up the old viewport in OpenXRViewportCompositionLayerProvider::set_viewport(). I'm not sure there's a better way to handle that (see above for more details), but perhaps it's OK as-is because it's about clean up, which shouldn't need to happen very often? In most cases, only when switching scenes or shutting down the app altogether.

I agree, I think it's just important to have this noted so its something we can think more about and know is a weakness.

The reality is that these are very unlikely scenarios.

[BastiaanOlij]

One more thing I thought off that would be a nice to add feature, it would be good if OpenXRCompositionLayer had a virtual ray_intersect(origin, direction) function that would provide a UV for the viewport that this ray intersects with. Since all composition layers have curved shapes, and will generally be used for UIs, it makes more sense to have this logic implemented here and make it easy to create simulate input on the viewport.

Also thinking forward, seeing you have the fallback logic I think we need to consider making these XR classes in a future PR and figure out a way to make composition layers something that the XR interface exposes.

[dsnopek]

One more thing I thought off that would be a nice to add feature, it would be good if OpenXRCompositionLayer had a virtual ray_intersect(origin, direction) function that would provide a UV for the viewport that this ray intersects with.

That would be great! However, could we save that for a follow-up PR? I know that both you and I have a few PRs planned that will build on what's here, so it'd be nice to get this in as soon as possible so it doesn't block/conflict with other work.

Also thinking forward, seeing you have the fallback logic I think we need to consider making these XR classes in a future PR and figure out a way to make composition layers something that the XR interface exposes.

Yes, I agree. WebXR also supports similar composition layers, and having a way to unify those would be great.

The challenge is that the settings for composition layers will likely be different between the different XR interfaces, which will make it a little challenging to have a single set of nodes. In the case of OpenXR vs WebXR, all the size and shape settings are the same, but everything beyond that is different, and both OpenXR and WebXR have settings that the other doesn't support.

OpenXR also has a whole bunch of extensions that add even more settings to composition layers (which we'll need to be able to support from GDExtension, because many of them are vendor extensions), and there are no equivalents in WebXR. It may even be hard/impossible to emulate all of the possible settings with the fallback logic.

So, for now, I think it probably does make sense to try and accommodate OpenXR first, before trying to build a generic system.

[BastiaanOlij]

@dsnopek Agree this should all be follow up PRs. I do think the ray intersection code should be done fairly quickly because without it, there is a lot of math required by the user to actually interact with any UI you draw on composition layers.

Making this a more generic solution is future music however, I think it's something we need to start thinking off as you've got many more ingredients in place than I had with my initial attempt, but I agree we should first get experience just handling this for OpenXR and then see how we can broaden it later.

[BastiaanOlij]

Ok, I've tested this using my https://github.com/BastiaanOlij/godot_openxr_demo project and adding a composition layer to it.

1. Using a Valve Index in PCVR mode, shows up fine but I think it is using the fallback option here (good test of fallback ;) ).
2. Using a Quest 3, both with compatibility and mobile renderers, works perfectly
3. Using a HTC Elite XR, works perfectly with compatibility but crashes on mobile renderer. Seeing I only just started testing with the Elite XR, it's likely the crash is unrelated to this PR.

The only thing that came up today that we may want to do before this is ready for merge, is to make sure the material used to render the fallback mode is:

• set to unshaded (I think it is)
• has linear filtering
• does not cast shadows
• does not do a depth test/overrides POSITION so the z component becomes -1.0/0.0 (or 1.0 once reverse-Z is merged) so it renders over current content just like composition layers do.

[dsnopek]

@BastiaanOlij Thanks for testing!

(I've removed the "needs testing" tag, since Bastiaan and Logan have both tested it.)

• set to unshaded (I think it is)
• has linear filtering
• does not cast shadows
• does not do a depth test/overrides POSITION so the z component becomes -1.0/0.0 (or 1.0 once reverse-Z is merged) so it renders over current content just like composition layers do.

Both unshaded and linear filtering were already enabled. In my latest push, I've set it to not cast shadows and not do the depth test. (I think not doing the depth test will be sufficient in almost all cases, but if users find it isn't, we can do a follow-up to use a shader to directly manipulate the depth values.)

[akien-mga]

Thanks!

[m4gr3d]

@dsnopek nit: these methods have the same name as the ones defined in OpenXRAPI; can we rename them to [un]register_viewport_composition_layer_provider instead.

[dsnopek]

Sure, I could do a follow-up to rename them. I've got 2 other follow-up's already in the works :-)

[dsnopek]

I snuck the requested change into my first follow-up PR :-) - see #90237