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.

Sign up for GitHub

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

OpenXR: Allow moving vendor passthrough extensions to GDExtension #87630

Merged
merged 1 commit into from
Feb 3, 2024
Merged

OpenXR: Allow moving vendor passthrough extensions to GDExtension #87630

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions doc/classes/XRInterface.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,16 +102,18 @@
Is [code]true[/code] if this interface has been initialized.
</description>
</method>
<method name="is_passthrough_enabled">
<method name="is_passthrough_enabled" is_deprecated="true">
<return type="bool" />
<description>
Is [code]true[/code] if passthrough is enabled.
[i]Deprecated.[/i] Check if [member environment_blend_mode] is [constant XRInterface.XR_ENV_BLEND_MODE_ALPHA_BLEND], instead.
</description>
</method>
<method name="is_passthrough_supported">
<method name="is_passthrough_supported" is_deprecated="true">
<return type="bool" />
<description>
Is [code]true[/code] if this interface supports passthrough.
[i]Deprecated.[/i] Check that [constant XRInterface.XR_ENV_BLEND_MODE_ALPHA_BLEND] is supported using [method get_supported_environment_blend_modes], instead.
</description>
</method>
<method name="set_environment_blend_mode">
Expand Down Expand Up @@ -143,17 +145,19 @@
Sets the active play area mode, will return [code]false[/code] if the mode can't be used with this interface.
</description>
</method>
<method name="start_passthrough">
<method name="start_passthrough" is_deprecated="true">
<return type="bool" />
<description>
Starts passthrough, will return [code]false[/code] if passthrough couldn't be started.
[b]Note:[/b] The viewport used for XR must have a transparent background, otherwise passthrough may not properly render.
[i]Deprecated.[/i] Set the [member environment_blend_mode] to [constant XRInterface.XR_ENV_BLEND_MODE_ALPHA_BLEND], instead.
</description>
</method>
<method name="stop_passthrough">
<method name="stop_passthrough" is_deprecated="true">
<return type="void" />
<description>
Stops passthrough.
[i]Deprecated.[/i] Set the [member environment_blend_mode] to [constant XRInterface.XR_ENV_BLEND_MODE_OPAQUE], instead.
</description>
</method>
<method name="supports_play_area_mode">
Expand Down
38 changes: 38 additions & 0 deletions modules/openxr/doc_classes/OpenXRAPIExtension.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,12 @@
Returns the id of the system, which is a [url=https://registry.khronos.org/OpenXR/specs/1.0/man/html/XrSystemId.html]XrSystemId[/url] cast to an integer.
</description>
</method>
<method name="is_environment_blend_mode_alpha_supported">
<return type="int" enum="OpenXRAPIExtension.OpenXRAlphaBlendModeSupport" />
<description>
Returns [enum OpenXRAPIExtension.OpenXRAlphaBlendModeSupport] denoting if [constant XRInterface.XR_ENV_BLEND_MODE_ALPHA_BLEND] is really support, emulated or not supported at all.
dsnopek marked this conversation as resolved.
Show resolved Hide resolved
</description>
</method>
<method name="is_initialized">
<return type="bool" />
<description>
Expand All @@ -94,13 +100,34 @@
Returns [code]true[/code] if OpenXR is enabled.
</description>
</method>
<method name="register_composition_layer_provider">
<return type="void" />
<param index="0" name="extension" type="OpenXRExtensionWrapperExtension" />
<description>
Registers the given extension as a composition layer provider.
</description>
</method>
<method name="set_emulate_environment_blend_mode_alpha_blend">
<return type="void" />
<param index="0" name="enabled" type="bool" />
<description>
If set to [code]true[/code], an OpenXR extension is loaded which is capable of emulating the [constant XRInterface.XR_ENV_BLEND_MODE_ALPHA_BLEND] blend mode.
</description>
</method>
Comment on lines +110 to +116
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can do this automatically on our end instead of letting the gdextension dictates whether this should be set or not, by going through the list of gdextensions and checking their return values to is_environment_blend_mode_alpha_blend_supported().

This would mitigate the issue where multiple gdextensions can be loaded each with different level of support. For example, one gdextension may have full support while the other may only be able to emulate support. The latter would set this flag which would cause the logic to assume that emulation is required when it's not.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't entirely understand this suggestion.

We can do this automatically on our end instead of letting the gdextension dictates whether this should be set or not, by going through the list of gdextensions and checking their return values to is_environment_blend_mode_alpha_blend_supported().

Are you saying that we should add some method on OpenXRExtensionWrapperExtension which allows the extension to tell Godot that it's going to emulate ALPHA_BLEND and we'll loop over all the extensions and call it? At first glance, that doesn't seem any simpler. But I'm probably misunderstanding?

This would mitigate the issue where multiple gdextensions can be loaded each with different level of support. For example, one gdextension may have full support while the other may only be able to emulate support. The latter would set this flag which would cause the logic to assume that emulation is required when it's not.

I don't think we'll encounter situations with multiple extensions that have "full" vs "emulated" support. The only way "full" support happens is if the runtime reports that it supports ALPHA_BLEND, in which case no extension is needed.

The situation with multiple extensions that I forsee is where the runtime supports multiple passthrough extensions for compatibility, for example, maybe HTC devices would support their passthrough extension but then also the Meta one to make it easy for developers to port their apps. So, we just need to make sure that both extensions don't try to do emulation.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it, thanks for the clarification!

<method name="transform_from_pose">
<return type="Transform3D" />
<param index="0" name="pose" type="const void*" />
<description>
Creates a [Transform3D] from an [url=https://registry.khronos.org/OpenXR/specs/1.0/man/html/XrPosef.html]XrPosef[/url].
</description>
</method>
<method name="unregister_composition_layer_provider">
<return type="void" />
<param index="0" name="extension" type="OpenXRExtensionWrapperExtension" />
<description>
Unregisters the given extension as a composition layer provider.
</description>
</method>
<method name="xr_result">
<return type="bool" />
<param index="0" name="result" type="int" />
Expand All @@ -111,4 +138,15 @@
</description>
</method>
</methods>
<constants>
<constant name="OPENXR_ALPHA_BLEND_MODE_SUPPORT_NONE" value="0" enum="OpenXRAlphaBlendModeSupport">
Means that [constant XRInterface.XR_ENV_BLEND_MODE_ALPHA_BLEND] isn't supported at all.
</constant>
<constant name="OPENXR_ALPHA_BLEND_MODE_SUPPORT_REAL" value="1" enum="OpenXRAlphaBlendModeSupport">
Means that [constant XRInterface.XR_ENV_BLEND_MODE_ALPHA_BLEND] is really supported.
</constant>
<constant name="OPENXR_ALPHA_BLEND_MODE_SUPPORT_EMULATING" value="2" enum="OpenXRAlphaBlendModeSupport">
Means that [constant XRInterface.XR_ENV_BLEND_MODE_ALPHA_BLEND] is emulated.
</constant>
</constants>
</class>
6 changes: 6 additions & 0 deletions modules/openxr/doc_classes/OpenXRExtensionWrapperExtension.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,12 @@
<tutorials>
</tutorials>
<methods>
<method name="_get_composition_layer" qualifiers="virtual">
<return type="int" />
<description>
Returns a pointer to a [code]XrCompositionLayerBaseHeader[/code] struct to provide a composition layer. This will only be called if the extension previously registered itself with [method OpenXRAPIExtension.register_composition_layer_provider].
</description>
</method>
<method name="_get_requested_extensions" qualifiers="virtual">
<return type="Dictionary" />
<description>
Expand Down
11 changes: 11 additions & 0 deletions modules/openxr/extensions/openxr_extension_wrapper_extension.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ void OpenXRExtensionWrapperExtension::_bind_methods() {
GDVIRTUAL_BIND(_set_session_create_and_get_next_pointer, "next_pointer");
GDVIRTUAL_BIND(_set_swapchain_create_info_and_get_next_pointer, "next_pointer");
GDVIRTUAL_BIND(_set_hand_joint_locations_and_get_next_pointer, "hand_index", "next_pointer");
GDVIRTUAL_BIND(_get_composition_layer);
GDVIRTUAL_BIND(_on_register_metadata);
GDVIRTUAL_BIND(_on_before_instance_created);
GDVIRTUAL_BIND(_on_instance_created, "instance");
Expand Down Expand Up @@ -128,6 +129,16 @@ void *OpenXRExtensionWrapperExtension::set_hand_joint_locations_and_get_next_poi
return nullptr;
}

XrCompositionLayerBaseHeader *OpenXRExtensionWrapperExtension::get_composition_layer() {
uint64_t pointer;

if (GDVIRTUAL_CALL(_get_composition_layer, pointer)) {
return reinterpret_cast<XrCompositionLayerBaseHeader *>(pointer);
}

return nullptr;
}

void OpenXRExtensionWrapperExtension::on_register_metadata() {
GDVIRTUAL_CALL(_on_register_metadata);
}
Expand Down
4 changes: 3 additions & 1 deletion modules/openxr/extensions/openxr_extension_wrapper_extension.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@
#include "core/os/thread_safe.h"
#include "core/variant/native_ptr.h"

class OpenXRExtensionWrapperExtension : public Object, OpenXRExtensionWrapper {
class OpenXRExtensionWrapperExtension : public Object, public OpenXRExtensionWrapper, public OpenXRCompositionLayerProvider {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it safe to always add OpenXRCompositionLayerProvider here? Most extensions won't be a composition layer provider.

I think we need a OpenXRCompositionLayerProviderWrapperExtension (damn thats a mouth full) wrapper that inherits from OpenXRExtensionWrapperExtension and OpenXRCompositionLayerProvider?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could make a new extension class, however, nothing special is done with an OpenXRCompositionLayerProvider until you call openxr_api->register_composition_layer_provider(), so I think it's safe for them to always be OpenXRCompositionLayerProvider, since only the extensions that really need to do anything for that will register themselves.

GDCLASS(OpenXRExtensionWrapperExtension, Object);

protected:
Expand All @@ -59,13 +59,15 @@ class OpenXRExtensionWrapperExtension : public Object, OpenXRExtensionWrapper {
virtual void *set_session_create_and_get_next_pointer(void *p_next_pointer) override;
virtual void *set_swapchain_create_info_and_get_next_pointer(void *p_next_pointer) override;
virtual void *set_hand_joint_locations_and_get_next_pointer(int p_hand_index, void *p_next_pointer) override;
virtual XrCompositionLayerBaseHeader *get_composition_layer() override;

//TODO workaround as GDExtensionPtr<void> return type results in build error in godot-cpp
GDVIRTUAL1R(uint64_t, _set_system_properties_and_get_next_pointer, GDExtensionPtr<void>);
GDVIRTUAL1R(uint64_t, _set_instance_create_info_and_get_next_pointer, GDExtensionPtr<void>);
GDVIRTUAL1R(uint64_t, _set_session_create_and_get_next_pointer, GDExtensionPtr<void>);
GDVIRTUAL1R(uint64_t, _set_swapchain_create_info_and_get_next_pointer, GDExtensionPtr<void>);
GDVIRTUAL2R(uint64_t, _set_hand_joint_locations_and_get_next_pointer, int, GDExtensionPtr<void>);
GDVIRTUAL0R(uint64_t, _get_composition_layer);

virtual void on_register_metadata() override;
virtual void on_before_instance_created() override;
Expand Down
Loading
Loading