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

OpenXR: Add Meta touch plus interaction profile #86982

Merged
merged 1 commit into from
Jan 11, 2024

Conversation

BastiaanOlij
Copy link
Contributor

This PR adds support for Metas touch plus interaction profile.

It only adds the needed meta data in order to use this profile, it does not add this to the default action map. Any touch controller is already supported by the base touch interaction profile.

This new profile adds Quest Pro and Quest 3 specific controller inputs that can be used if developers choose to add support. For this the developer should create their own action map, as would be best practice in any case.

Depends on #86980

@BastiaanOlij BastiaanOlij added this to the 4.3 milestone Jan 9, 2024
@BastiaanOlij BastiaanOlij self-assigned this Jan 9, 2024
@BastiaanOlij BastiaanOlij requested a review from a team as a code owner January 9, 2024 02:39
@akien-mga
Copy link
Member

Just for my information, as I maintain the Godot package on Fedora and Mageia where it builds against system-provided OpenXR: What's the minimal OpenXR version to build Godot after this PR, 1.0.32 or 1.0.33?

@m4gr3d m4gr3d requested a review from dsnopek January 9, 2024 14:08
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.

Thanks!

This needs to be rebased on master, now that both PR #86980 and #86792 are merged.

I tested this on both the Quest 3 and Quest Pro, by creating some new actions and assigning them on a new "Touch controller plus" profile:

Selection_100

And then printing out some messages when signals on XRController3D were emitted.

This worked great on the Quest 3! However, on the Quest Pro, it picked up the "Touch controller" profile instead. I think this actually only applies to the Quest 3 controllers, not Quest Pro (which are technically called "Touch Pro" not "Touch Plus"). This is only represented in a comment though.

Otherwise, the code looks great!

@BastiaanOlij
Copy link
Contributor Author

@dsnopek good catch! For this we actually also need to add XR_FB_touch_controller_pro

So I guess we should add that here as well. Can't say I'm too happy about Metas approach here but alas...

@akien-mga honestly, I don't know. I'll ask Rylie if we keep track of that somewhere. My gut feeling says the pro controller was available for awhile, but the plus controller probably was added to 33

@BastiaanOlij BastiaanOlij force-pushed the openxr_touch_plus branch 2 times, most recently from 300d4d0 to e2a4014 Compare January 10, 2024 06:21
@BastiaanOlij
Copy link
Contributor Author

Rebased and now has support for both Pro and Plus controllers. @dsnopek if you don't mind giving it another try?

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.

With the latest updates, after creating a new profile for "Touch controller pro", it's now working for me on both Quest 3 and Quest Pro :-)

@dsnopek
Copy link
Contributor

dsnopek commented Jan 10, 2024

Or, should we try to sneak in support for this one too? It adds some new paths for the "Touch controller" profile.

We could do that in a follow-up too, though.

@BastiaanOlij
Copy link
Contributor Author

Or, should we try to sneak in support for this one too? It adds some new paths for the "Touch controller" profile.

Why not, might as well do the whole lot. It's in there but the DDOS seems to be causing some CI issues :)

@akien-mga akien-mga merged commit 4b305cd into godotengine:master Jan 11, 2024
15 checks passed
@akien-mga
Copy link
Member

Thanks!

@BastiaanOlij BastiaanOlij deleted the openxr_touch_plus branch February 27, 2024 23:42
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.

3 participants