-
Notifications
You must be signed in to change notification settings - Fork 26
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 scene capture api to the Godot OpenXR loader plugin #40
Conversation
@m4gr3d Seems good! Actually it's the exact same changes that I made locally when I realized that you already created the PR :) |
@m4gr3d Sorry, by "locally" i meant that I've implemented the exact same changes, before seeing that you have already made them :) So this PR should still be merged |
Sounds good! I need to rebase the PR, then I'll merge it for the 2.0 update. @konczg @kisg On that topic, are you okay with moving the anchor / scene APIs gdextension implementation to this repo. |
7118479
to
43bf85d
Compare
Sorry guys, missed this discussion for some reason. I'm on the fence here. I think it doesn't make sense to include options here for features that aren't explicitly supported in this plugin or in the core. We could easily end up misleading users who download the plugin, see the option and think they can enable support for this not knowing that another plugin is needed. To me it makes more sense that these options are exposed through the plugin that implements the feature. Unless there is a technical limitation here that does not make this possible. That said, the ideal situation as Fredia already eludes to in his last comment, is to have the scene API logic embedded in this plugin. However this is up to Migeran whether they are ok with donating their logic to this project. I don't know their standpoint on whether they purely implemented the logic in a plugin because we can't (currently) be accepted it in core, or whether they also want to retain ownership of the implementation. |
We (Migeran company leadership) discussed it internally. The Migeran company can agree to donating the Scene Capture API implementation to Godot, provided that the contribution is attributed properly to Migeran - Software Development & Consulting (https://migeran.com) as Migeran designed and implemented the feature - and paid the costs of development. I can send a PR for this. However, it might make sense to rename this repository to either e.g. godot_openxr_device_support or to create separate repositories for each headset brand, (e.g. godot_openxr_meta) that would host the device support code for the specific brand. (OpenXR loader, device specific OpenXR extensions ... etc.) What do you think? |
43bf85d
to
80712d3
Compare
Thanks @kisg, we appreciate the donation!
This is a topic we've discussed a few times especially as the scope of the repo and plugin grows beyond just providing loader capabilities, and something I'm in favor of. I think it makes sense to rename the plugin following the inclusion of the Scene API extension as it'll be the first vendor extension offered and supported by this plugin. With regard to creating a separate repo for each headset branch, this is not on the table at the moment due to the overhead if would create for what would mostly be identical work for each vendor, but were a vendor to step forward and device they want to handle support for their headset, we would not be opposed to let them fork from this repo and point users to their 'official' plugin. |
Superseded by #53 |
Follow up to my comment in godotengine/godot#68259 (comment) about moving the anchor api options to the Godot OpenXR loader plugin.
This is set as a draft for now because the OpenXR Extension Wrapper needs to be added to the Godot OpenXR loader plugin to actually provide the functionality.
cc @konczg