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 the debug utils extension in OpenXR #95156

Merged
merged 1 commit into from
Aug 27, 2024

Conversation

BastiaanOlij
Copy link
Contributor

@BastiaanOlij BastiaanOlij commented Aug 5, 2024

Adds support for the debug utils extension in OpenXR.

This extension gives access to validation layers and getting further feedback from the XR runtime.

For supported platforms see: https://github.khronos.org/OpenXR-Inventory/extension_support.html#XR_EXT_debug_utils
For specification see: https://registry.khronos.org/OpenXR/specs/1.0/html/xrspec.html#XR_EXT_debug_utils

Todos:

  • Output messages to Godot log
  • Restrict message types (right now all are reported)
  • Set OpenXR object names for more easy debugging
  • Set labels for more easy debugging

Note: This PR currently just logs the debug messages coming from the XR runtime and is thus mostly useful for finding faults while implementing new APIs. In the future we may want to investigate whether we can use the callbacks for profiling as well but that is out of scope for this PR.

@BastiaanOlij BastiaanOlij added this to the 4.4 milestone Aug 5, 2024
@BastiaanOlij BastiaanOlij requested a review from a team August 5, 2024 07:29
@BastiaanOlij BastiaanOlij self-assigned this Aug 5, 2024
@BastiaanOlij
Copy link
Contributor Author

cc @dsnopek, I tested this with the Meta XR Simulator, so far only got feedback from verbose messages (and flooded the logs :P) but will be useful with further testing.

@BastiaanOlij BastiaanOlij force-pushed the add_openxr_debug_utils branch from 4fe9a79 to 5d81ff9 Compare August 5, 2024 07:31
@BastiaanOlij BastiaanOlij force-pushed the add_openxr_debug_utils branch 3 times, most recently from 623ef95 to 57e8725 Compare August 6, 2024 07:28
@BastiaanOlij
Copy link
Contributor Author

Ok, debug label support is in as well. Just needs a few small tweaks.

However there is a bug when using Metas XR simulator, it seems to require us to not free memory of the labels we provide. This is likely a bug on their side but not ruling anything out just yet.

@BastiaanOlij BastiaanOlij requested a review from a team August 6, 2024 07:33
if (debug_utils_ext) {
set_object_name(XR_OBJECT_TYPE_SESSION, uint64_t(p_instance), "Main Godot OpenXR Session");

begin_debug_label_region("Godot session active");
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this extension need to be invoked first before all the others?

Copy link
Contributor

Choose a reason for hiding this comment

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

After catching up with the code in openxr_api, it looks this should be invoked from openxr_api instead to ensure it runs before all the other extensions. Same thing for set_object_name in on_instance_created

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 can't, because at that point we haven't initialised it yet, it leads to a nice crash :P

We'd need some way to give priority to this extension over all the other extensions so it runs first.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Owh this is the session call, I mixed up with instance because the parameter name is wrong, oops :) I'll fix that up.

I guess we could move it into openxr_api itself to ensure it runs ASAP.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok changed this, but with the footnote that this requires a fix in the Meta XR Simulator before it works properly as the memory holding the string gets destroyed after the call ends.

@BastiaanOlij BastiaanOlij force-pushed the add_openxr_debug_utils branch 2 times, most recently from 02b1fca to 974b33c Compare August 8, 2024 00:38
@BastiaanOlij BastiaanOlij force-pushed the add_openxr_debug_utils branch 2 times, most recently from 7700dbe to a4dee6c Compare August 19, 2024 05:00
@BastiaanOlij BastiaanOlij marked this pull request as ready for review August 19, 2024 05:58
@BastiaanOlij BastiaanOlij requested review from a team as code owners August 19, 2024 05:58
@BastiaanOlij BastiaanOlij requested a review from dsnopek August 19, 2024 06:01
@BastiaanOlij BastiaanOlij force-pushed the add_openxr_debug_utils branch from a4dee6c to d8a76c3 Compare August 21, 2024 23:54
Copy link
Contributor

@dsnopek dsnopek left a comment

Choose a reason for hiding this comment

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

I haven't had a chance to test this, but skimming through the code it looks great to me! (Aside from the couple of small language things that ATS pointed out.)

@BastiaanOlij BastiaanOlij force-pushed the add_openxr_debug_utils branch from d8a76c3 to 08ffa5d Compare August 27, 2024 02:07
@akien-mga akien-mga merged commit c2daec1 into godotengine:master Aug 27, 2024
18 checks passed
@akien-mga
Copy link
Member

Thanks!

@BastiaanOlij BastiaanOlij deleted the add_openxr_debug_utils branch March 13, 2025 07:01
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.

5 participants