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 Meta configs for the eye gaze interaction extension #48

Merged

Conversation

m4gr3d
Copy link
Collaborator

@m4gr3d m4gr3d commented Oct 1, 2023

The first commit is a small change to address and fix #46

The second commit contains the majority of the changes and is the counterpart to godotengine/godot#82614:

  • Adds the necessary config option to enable eye tracking for Meta devices
  • Defines XR_EXT_eye_gaze_interaction as a feature tag to set when eye tracking is enabled
    • On mobile xr devices, this feature tag is used to gate the eye gaze interaction extension
  • Adds the necessary Meta permissions when eye tracking is enabled
  • Updates the demo project to showcase eye tracking (tested on Quest Pro device)
  • Updates Meta's GodotPlugin instance to return true for the PERMISSION_XR_EXT_eye_gaze_interaction feature tag when the necessary permissions are granted.

@BastiaanOlij
Copy link
Member

Owh, I'm starting to understand the approach you were planning now. Makes a lot of sense to have a platform agnostic feature for the permission, that we then implement for each platform that supports it.

We might want to add stubs to the other platforms that implement the supportsFeature function and just returns false.

The only bit that I don't completely see through, when I'm looking at also adding support on PICO, is selecting the setting itself in the export interface. I'm assuming we'll have a PICO section where we select whether eye gaze is required on PICO and it adds the same feature tag?

@BastiaanOlij
Copy link
Member

Owh I noticed in the PR on the Godot side the supportsFeature already has a base implementation and we're overriding it :)

@m4gr3d
Copy link
Collaborator Author

m4gr3d commented Oct 1, 2023

I'm assuming we'll have a PICO section where we select whether eye gaze is required on PICO and it adds the same feature tag?

Yeah I can't test so I skipped that section but the changes will be similar (if not identical to the Meta changes):

  • In godot_openxr_pico_editor_export_plugin.gd, an additional option will be added which will be returned by _get_export_options as done for Meta. Unlike Meta, Pico eye tracking may be more of a boolean value since they don't have the required attribute
  • _get_export_features will need to be overridden for Pico to include the feature tag when Pico eye tracking is enabled
  • _get_android_manifest_element_contents will also need to be overridden for Pico to include the proper Pico permission in the manifest.

@BastiaanOlij
Copy link
Member

I'm kind of in the same boat as my PICO 4 doesn't have eye tracking. But I'm hoping I can test the extension with a Varjo headset at least.

I might add the PICO changes as I think they're needed and sent a test build off to PICO. But I think we can approve this work as is after I get a chance to test it on my Quest. Hopefully the production team will merge the upstream change in Godot for 4.2

@m4gr3d
Copy link
Collaborator Author

m4gr3d commented Oct 1, 2023

I'm kind of in the same boat as my PICO 4 doesn't have eye tracking. But I'm hoping I can test the extension with a Varjo headset at least.

I might add the PICO changes as I think they're needed and sent a test build off to PICO. But I think we can approve this work as is after I get a chance to test it on my Quest. Hopefully the production team will merge the upstream change in Godot for 4.2

We can also use the gating logic to validate this no longer causes a crash on the Pico 4.

@m4gr3d m4gr3d force-pushed the add_eye_gaze_interaction_configs branch from 1b206c9 to 0ed1a8e Compare October 2, 2023 20:48
@m4gr3d
Copy link
Collaborator Author

m4gr3d commented Oct 3, 2023

@BastiaanOlij This can be merged now that godotengine/godot#82614 has been merged.

Copy link
Member

@BastiaanOlij BastiaanOlij left a comment

Choose a reason for hiding this comment

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

Approving this as is, we'll look at adding PICO support separately

@BastiaanOlij BastiaanOlij merged commit 860177c into GodotVR:master Oct 4, 2023
1 check passed
@m4gr3d m4gr3d deleted the add_eye_gaze_interaction_configs branch October 4, 2023 07:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Warning if multiple loaders are selected
2 participants