Move implementation of XR_FB_passthrough into godot_openxr_vendors

[dsnopek]

This depends on PR godotengine/godot#87630

Moving XR_FB_passthrough into 'godot_openxr_vendors' would allow us to expand the passthrough functionality to include more Meta-specific features, which wouldn't necessarily be accepted into Godot.

Note: This will fail CI until the Godot PR is merged, and we start using an extension_api.json from Godot master.

[BastiaanOlij]

Make sure you also add this to the PICO and HTC plugins.
PICO uses the meta vendor extension as is and HTC uses it on the Vive Cosmos.
(see https://github.khronos.org/OpenXR-Inventory/extension_support.html#XR_FB_passthrough )

I also believe we should add it to the Khronos plugin, other devices may support it in the future and if they use the Khronos plugin, they immediate piggy back on the existing support (though I hope they'll adopt the official spec approach).
As a rule I think we should make sure all extensions we implement, are added to the Khronos plugin just in case.

HTC has their own vendor extension but that is used by the HTC Focus (and I assume Elite, but they probably didn't update the list yet): https://github.khronos.org/OpenXR-Inventory/extension_support.html#XR_HTC_passthrough

This is the reason I'm not happy with us creating separate plugins for each device, multiple devices support the same extensions, support can be queried from OpenXR by seeing which extensions are available.

[dsnopek]

Regarding extension/plugin organization in the source repo:

What if we kept the source for all the extensions that are in the OpenXR spec in the same place in the source code (maybe common/src/main/cpp/extensions?), and then included each of the ones from that central location into the plugins that needed it?

And for any extensions that aren't in the OpenXR spec, which require special headers from the vendor, can be exclusively in the directory for that vendor?

So, XR_FB_passthrough would go into the common location, so the Meta, Pico, HTC and Khronos plugins can use it. But then if we had, for example, XR_META_body_tracking_full_body which isn't in the spec yet (and we don't have an implementation for yet, but just theoretically), that could live under godotopenxrmeta/src/main/cpp/.

[m4gr3d]

Regarding extension/plugin organization in the source repo:

What if we kept the source for all the extensions that are in the OpenXR spec in the same place in the source code (maybe common/src/main/cpp/extensions?), and then included each of the ones from that central location into the plugins that needed it?

And for any extensions that aren't in the OpenXR spec, which require special headers from the vendor, can be exclusively in the directory for that vendor?

So, XR_FB_passthrough would go into the common location, so the Meta, Pico, HTC and Khronos plugins can use it. But then if we had, for example, XR_META_body_tracking_full_body which isn't in the spec yet (and we don't have an implementation for yet, but just theoretically), that could live under godotopenxrmeta/src/main/cpp/.

That is the purpose of the common directory, so that sounds good to me!

Regarding XR_FB_passthrough, the implementation has an include on openxr/fb_scene_capture.h, is that header part of the openxr headers or is it specific to the Meta SDK?

[dsnopek]

Regarding XR_FB_passthrough, the implementation has an include on openxr/fb_scene_capture.h, is that header part of the openxr headers or is it specific to the Meta SDK?

Whoops! That's just a copy-paste mistake on my part - I started by copying the scene capture extension. :-) Fixed in my latest push.

However, in general, fb_scene_capture.h from the Meta SDK shouldn't be necessary anymore - XR_FB_scene_capture is part of openxr.h now. I imagine at an earlier point it was only in the Meta SDK?

So, even that one could theoretically be in the common code because it's in the OpenXR spec now.

[BastiaanOlij]

@dsnopek something I just wondered about, at some point Meta may actually implement this the OpenXR way and it will return the alpha blend mode as supported. At that time we probably need to have some check where the passthrough extension is only used as is when OpenXR reports alpha blend mode as not supported?

[dsnopek]

Make sure you also add this to the PICO and HTC plugins.
PICO uses the meta vendor extension as is and HTC uses it on the Vive Cosmos.

In my latest push, I've moved this extension to the "common" directory and added it to the Pico and Khronos plugins (I think the Vive Cosmos uses the Khronos loader?). If we are switching to having all the extensions in "common", we could certainly do more to organize things better, but that could be done all at once in a dedicated PR. Hopefully, this is OK for now?

something I just wondered about, at some point Meta may actually implement this the OpenXR way and it will return the alpha blend mode as supported. At that time we probably need to have some check where the passthrough extension is only used as is when OpenXR reports alpha blend mode as not supported?

Ah, that's a great point! I'm out of time today, but tomorrow I'll look at having the extension wrapper check if the the alpha blend mode is supported, and if so, to not do anything.

[dsnopek]

In my latest push, it'll now check if the runtime really supports ALPHA_BLEND before attempting to emulate it. Unfortunately, I had to add another method to OpenXRAPIExtension because we can't access XRServer early enough (it hasn't been registered yet) to call its method to list the supported blend modes.

I also re-ordered the methods in openxr_fb_passthrough_extension_wrapper.cpp to be sort of logically sequential, since I found myself getting a little lost in there. :-)

Anyway, at this point, I think all the review that directly pertains to this PR (as opposed to refactoring for follow-ups) has been addressed, either in a comment or with a code change. So, I'm going to take this out of draft now!

[dsnopek]

I just rebased this after PR #83 re-arranged the source code for the single GDExtension implementation. I retested and it's still working for me on Quest 3!

The upstream Godot changes have been merged (see PR godotengine/godot#87630), however, this will still fail tests here until we can use an updated extension_api.json for godot-cpp (so it'll get the new methods that we added in the upstream Godot PR). We should probably have a branch for Godot 4.3 for this.

[BastiaanOlij]

Unfortunately, I had to add another method to OpenXRAPIExtension because we can't access XRServer early enough (it hasn't been registered yet) to call its method to list the supported blend modes.

Not sure I'm happy about that, I know we love to hate the why and how about Reduz' views on this, but I don't think its acceptable to just plug around it.
OpenXRAPIExtension has direct access to OpenXRAPI which has access to the core list of blend modes. So I would just retrieve it that way.

[BastiaanOlij]

I'll find some time next week to also test this on PICO. PICO supposedly supports the blend mode approach and the meta approach, so I might hack a bit to see if it works both ways. Should also verify whether our overrule works correctly.

edit oh, and @dsnopek , we need to think of what happens when the vendor plugin is used with 4.2.1. I don't think we can get away with another "oh use this new vendor plugin only from 4.3 forward", especially because we don't have any form of versioning on the asset library. It's confusing enough as it is with the loader and vendor plugin.

[dsnopek]

Unfortunately, I had to add another method to OpenXRAPIExtension because we can't access XRServer early enough (it hasn't been registered yet) to call its method to list the supported blend modes.

Not sure I'm happy about that, I know we love to hate the why and how about Reduz' views on this, but I don't think its acceptable to just plug around it. OpenXRAPIExtension has direct access to OpenXRAPI which has access to the core list of blend modes. So I would just retrieve it that way.

Actually, after a few revisions, the new method that my quoted comment refers to was removed. It's not in the version of the PR that was merged into Godot and no longer relied upon here.

This is connected with the consolidation into OpenXRAPIExtension::is_environment_blend_mode_alpha_supported() which replaces a pre-existing method which existed to avoid two extensions both trying to emulate passthrough -- this set of changes was suggested by @m4gr3d.

Anyway, you don't have to worry about this anymore :-)

I'll find some time next week to also test this on PICO. PICO supposedly supports the blend mode approach and the meta approach, so I might hack a bit to see if it works both ways. Should also verify whether our overrule works correctly.

Awesome, thanks!

oh, and @dsnopek , we need to think of what happens when the vendor plugin is used with 4.2.1. I don't think we can get away with another "oh use this new vendor plugin only from 4.3 forward", especially because we don't have any form of versioning on the asset library. It's confusing enough as it is with the loader and vendor plugin.

There is a sort of way to handle this in the asset library, but it's a pain. If you set the version on an asset to "4.3" it'll only be shown in the editor when using Godot 4.3 and greater. So, you can have two assets, one set to "4.2" and the other set to "4.3", and folks on earlier Godots won't see the newer version.

Anyway, we do need to have a special 4.3+ build since this depends on new APIs added in Godot 4.3. If this build is attempted to be used with an older Godot, you'll get an error message about it -- since godot-cpp will compare the version of Godot it was compiled for (from the extension_api.json file used) with the version of Godot that's loading it -- but the editor won't crash or anything.

I think the trickier part is folks who are upgrading a Godot 4.2 project using passthrough to Godot 4.3. Passthrough will suddenly stop working, and they need to upgrade, but they won't get any message about it or anything.

[m4gr3d]

Now that godotengine/godot#87630 is merged, we should look into updating the godot-cpp dependency, merging this PR and releasing a 3.0.0-dev build for users to test with Godot 4.3.dev3.

Anyway, we do need to have a special 4.3+ build since this depends on new APIs added in Godot 4.3. If this build is attempted to be used with an older Godot, you'll get an error message about it -- since godot-cpp will compare the version of Godot it was compiled for (from the extension_api.json file used) with the version of Godot that's loading it -- but the editor won't crash or anything.

We've typically following the approach used in Godot core and use the master branch to track the tip of development. We can continue with that approach given we have the 2.x tags that we can use if we need to release a bug fix for the current 2.x version.

[m4gr3d]

@dsnopek Can you bump the compatibility_minimum value in plugin.gdextension to 4.3.

[dsnopek]

@dsnopek Can you bump the compatibility_minimum value in plugin.gdextension to 4.3.

Bumped!

[m4gr3d]

@dsnopek Ping for rebase. Now that #95 is merged, this PR should pass the checks.

[dsnopek]

@m4gr3d Rebased!

[m4gr3d]

@dsnopek Can you update the CHANGES.md file as well.

[dsnopek]

@m4gr3d Done!