Use of `XrSwapchainCreateFlags` for `OpenXRCompositionLayer` by bnjmntmm · Pull Request #108644 · godotengine/godot

bnjmntmm · 2025-07-15T20:41:05Z

Introduces SwapchainCreateFlags enum to control swapchain creation (NORMAL, STATIC, PROTECTED) in OpenXR composition layers.
This allows the user to set SwapChainCreate Flags to either have one static image on the composition layer or set the protected content flag which allows stuff like Widevine DRM protected videos to be displayed on the composition layer

As this is the first time ever creating something with c++, i am open for feedback, will try my best to answer to your guys' feedback and improve the code.

A question I have already: Should I rename the values in the enum, as "normal", "static" and "protected" are not that informative and "normal" is just not normal. it just sets a 0 as bit

Thanks ^^

dsnopek · 2025-07-15T20:50:57Z

Thanks!

I don't think it makes sense to have "static" as an option, because we already support that for Viewport-based composition layers automatically, and I don't think there is really a use case for it with Android surfaces. Just adding the ability to toggle "protected" should be enough.

Also, you need to make sure that this flag is used for Viewport-based layers as well. That would be done in OpenXRViewportCompositionLayerProvider::update_and_acquire_swapchain()

bnjmntmm · 2025-07-16T08:06:49Z

I've changed the enum to a simple bool. Also tested it and it still works. Off = pink screen, On = video is shown
Also added some basic documentation but that can be changed however you guys like :D

Also a question regarding the viewport based layers. Is there even the need to create a protected layer? I don't think it can be used properly but idk. I just added an extra if.
Maybe the if should be an extra check so it can't be static and protected.. idk if that even works

bnjmntmm · 2025-07-16T09:05:21Z

I think I have to change something in this pr.. as I rebased based on the docs and now it wanna also merge the other files.

dsnopek

Thanks!

At a high-level, this is looking good. I've mostly got notes about naming, and one functional note with regard to recreating the swapchain when the source of the content is a subviewport

Oh, and in the end, you'll need to squash everything into a single commit per Godot's pull request worflow. See https://docs.godotengine.org/en/latest/contributing/workflow/pr_workflow.html#the-interactive-rebase

modules/openxr/scene/openxr_composition_layer.h

modules/openxr/scene/openxr_composition_layer.cpp

modules/openxr/scene/openxr_composition_layer.h

modules/openxr/scene/openxr_composition_layer.cpp

modules/openxr/extensions/openxr_composition_layer_extension.h

modules/openxr/doc_classes/OpenXRCompositionLayer.xml

modules/openxr/extensions/openxr_composition_layer_extension.cpp

bnjmntmm · 2025-07-16T13:32:34Z

tried to address all of your feedback. Will squash if youre satisfied at the end

m4gr3d

Haven't tested the changes but the code looks good.

bnjmntmm · 2025-07-16T14:28:01Z

What does the one failed Linux / Editor with Mono mean?
Will squash later in the evening

dsnopek · 2025-07-16T15:06:26Z

What does the one failed Linux / Editor with Mono mean?

It wants you to change the order if the property in the XML docs - I think it alphabetizes them?

Here's the patch it gives:

diff --git a/modules/openxr/doc_classes/OpenXRCompositionLayer.xml b/modules/openxr/doc_classes/OpenXRCompositionLayer.xml
index 38ade44..8921f8e 100644
--- a/modules/openxr/doc_classes/OpenXRCompositionLayer.xml
+++ b/modules/openxr/doc_classes/OpenXRCompositionLayer.xml
@@ -49,6 +49,10 @@
 		<member name="layer_viewport" type="SubViewport" setter="set_layer_viewport" getter="get_layer_viewport">
 			The [SubViewport] to render on the composition layer.
 		</member>
+		<member name="protected_content" type="bool" setter="set_protected_content" getter="is_protected_content" default="false">
+			If enabled, the OpenXR swapchain will be created with the [code]XR_SWAPCHAIN_CREATE_PROTECTED_CONTENT_BIT[/code] flag, which will protect its contents from CPU access.
+			When used with an Android Surface, this may allow DRM content to be presented, and will only take effect when the Surface is first created; later changes to this property will have no effect.
+		</member>
 		<member name="sort_order" type="int" setter="set_sort_order" getter="get_sort_order" default="1">
 			The sort order for this composition layer. Higher numbers will be shown in front of lower numbers.
 			[b]Note:[/b] This will have no effect if a fallback mesh is being used.
@@ -102,10 +106,6 @@
 			See [method get_android_surface] for information about how to get the surface so that your application can draw to it.
 			[b]Note:[/b] This will only work in Android builds.
 		</member>
-		<member name="protected_content" type="bool" setter="set_protected_content" getter="is_protected_content" default="false">
-			If enabled, the OpenXR swapchain will be created with the [code]XR_SWAPCHAIN_CREATE_PROTECTED_CONTENT_BIT[/code] flag, which will protect its contents from CPU access.
-			When used with an Android Surface, this may allow DRM content to be presented, and will only take effect when the Surface is first created; later changes to this property will have no effect.
-		</member>
 	</members>
 	<constants>
 		<constant name="FILTER_NEAREST" value="0" enum="Filter">

So, just moving that section up to be below layer_viewport

dsnopek

Thanks, this looks great! Just two tiny notes, and this needs to be squashed, but after that I'd be happy to approve :-)

modules/openxr/extensions/openxr_composition_layer_extension.cpp

modules/openxr/scene/openxr_composition_layer.h

modules/openxr/scene/openxr_composition_layer.cpp

bnjmntmm · 2025-07-16T15:48:32Z

I'm stupid.. forgot to remove the old part in the docs

bnjmntmm · 2025-07-17T07:43:33Z

OKay.. squashing annoyed me so much because it reverted some changes, but I think every proposed change is in there.. I hope i didn't miss anything.

dsnopek

Thanks! I haven't tested, but the code looks great to me :-)

modules/openxr/scene/openxr_composition_layer.cpp

- Introduced SwapchainCreateFlags enum to control swapchain creation (NORMAL, STATIC, PROTECTED) in OpenXR composition layers. - Allows creation of static layers (never change after creation) and protected layers (for DRM-protected content). - Changed internal logic from enum to bool for simplicity; users now select "protected" or not. - Added support for protected content in viewport-based layers. - Refactored naming, documentation, and improved reusability. - Minor cleanup: removed unused variable and added missing space.

bnjmntmm · 2025-07-28T14:49:44Z

Done.. i hope 🫡

Repiteo · 2025-10-06T14:13:40Z

Thanks! Congratulations on your first merged contribution! 🎉

bnjmntmm requested a review from a team as a code owner July 15, 2025 20:41

dsnopek added this to the 4.x milestone Jul 15, 2025

dsnopek added enhancement topic:xr labels Jul 15, 2025

bnjmntmm requested review from a team as code owners July 16, 2025 08:03

bnjmntmm force-pushed the secure-android-surface branch from 0501d88 to a9c31b1 Compare July 16, 2025 09:19

dsnopek reviewed Jul 16, 2025

View reviewed changes

m4gr3d approved these changes Jul 16, 2025

View reviewed changes

dsnopek reviewed Jul 16, 2025

View reviewed changes

modules/openxr/extensions/openxr_composition_layer_extension.cpp Show resolved Hide resolved

modules/openxr/scene/openxr_composition_layer.h Outdated Show resolved Hide resolved

dsnopek modified the milestones: 4.x, 4.6 Jul 16, 2025

dsnopek reviewed Jul 16, 2025

View reviewed changes

modules/openxr/scene/openxr_composition_layer.cpp Outdated Show resolved Hide resolved

bnjmntmm force-pushed the secure-android-surface branch from 98e59cf to 7f7e96a Compare July 16, 2025 15:27

bnjmntmm force-pushed the secure-android-surface branch from 1bfb6d4 to bc60036 Compare July 17, 2025 07:42

dsnopek approved these changes Jul 17, 2025

View reviewed changes

AThousandShips changed the title ~~Usage of XrSwapchainCreateFlags for OpenXRCompositionLayer~~ Use of XrSwapchainCreateFlags for OpenXRCompositionLayer Jul 28, 2025

AThousandShips reviewed Jul 28, 2025

View reviewed changes

modules/openxr/scene/openxr_composition_layer.cpp Show resolved Hide resolved

bnjmntmm force-pushed the secure-android-surface branch from beef8e3 to ecfb962 Compare July 28, 2025 14:49

dsnopek mentioned this pull request Aug 8, 2025

Make OpenXRCompositionLayer and its children safe for multithreaded rendering #109431

Merged

Repiteo merged commit 788745e into godotengine:master Oct 6, 2025
20 checks passed

bnjmntmm mentioned this pull request Mar 5, 2026

Add support for creating protected content for the main swapchains #112273

Draft

Uh oh!

Conversation

bnjmntmm commented Jul 15, 2025

Uh oh!

dsnopek commented Jul 15, 2025

Uh oh!

bnjmntmm commented Jul 16, 2025

Uh oh!

bnjmntmm commented Jul 16, 2025

Uh oh!

dsnopek left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bnjmntmm commented Jul 16, 2025

Uh oh!

m4gr3d left a comment

Choose a reason for hiding this comment

Uh oh!

bnjmntmm commented Jul 16, 2025

Uh oh!

dsnopek commented Jul 16, 2025

Uh oh!

dsnopek left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bnjmntmm commented Jul 16, 2025

Uh oh!

bnjmntmm commented Jul 17, 2025

Uh oh!

dsnopek left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

bnjmntmm commented Jul 28, 2025

Uh oh!

Uh oh!

Repiteo commented Oct 6, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

dsnopek left a comment •

edited

Loading