-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Controller model fallbacks #10568
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
Controller model fallbacks #10568
Conversation
|
/azp run |
|
Azure Pipelines successfully started running 2 pipeline(s). |
Assets/MRTK/SDK/Features/UX/Scripts/Controllers/MixedRealityControllerVisualizer.cs
Outdated
Show resolved
Hide resolved
|
I agree with @keveleigh , Controller model fallback is built in to the architecture for how the system retrieves the actual model to use:
If no controller override is set, the global is used. So what is the actual issue being resolved? Plus most of the fixes are changing controller poses and other interactive data, not the model (other than adding new models in to source, which should be avoided, this is at the discretion of the developer in their own project.) |
Co-authored-by: Kurtis <kurtie@microsoft.com>
The fallback logic is sound and works fine. This PR fixes one bug where the Global Default would be used even if the Controller override was specified. Beyond that, it adds some open source models as representations of Generic Motion Controllers/Quest Motion controllers. This was requested by our users, who have expressed some dissatisfaction with our default gizmos. It also helps bridge the Oculus Quest story for Open XR, since controller models are not yet provided on the platform level for Oculus Devices. Quest Controller models are important for developers considering moving to the OpenXR backend. |
|
/azp run |
|
Azure Pipelines successfully started running 2 pipeline(s). |
…it-Unity into controllerModelFallbacks
|
/azp run |
|
Azure Pipelines successfully started running 2 pipeline(s). |
david-c-kline
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A couple minor bits. Otherwise looks good.
Assets/MRTK/SDK/Features/UX/Scripts/Controllers/MixedRealityControllerVisualizer.cs
Outdated
Show resolved
Hide resolved
| @@ -0,0 +1,21 @@ | |||
| MIT License | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please be sure to update the NOTICES.md file to indicate our usage of these models.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cgmanifest.json as well, which can be used to generate parts of the Notices file.
d67fb9b to
ac0aefc
Compare
…it-Unity into controllerModelFallbacks
|
/azp run |
|
Azure Pipelines successfully started running 2 pipeline(s). |
| @@ -0,0 +1,8 @@ | |||
| fileFormatVersion: 2 | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not: should we rename this folder? I feel like the WebXR part of the name can be misleading, and the license itself clarifies the source.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure what we'd rename it to. Would it be PlayCanvas, since that seems to be the ultimate parent project.
https://developer.playcanvas.com/en/tutorials/webxr-controllerhand-models/
The license mentions Amazon, but I can't quite determine where their involvement started/ended.
https://github.com/immersive-web/webxr-input-profiles/blob/main/packages/assets/LICENSE.md
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually looking deeper, it does seem to all originate from WebXR, so I'd say we should stick to that name
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My comment was less around where they originate from (the license succeeds for that, I think?) and more about relating what they mean to MRTK in the folder structure. They aren't specific to WebXR in the way we use them, so I fear that naming could be confusing.
Overview
Added vr controller model fallbacks to replaces our previously primitive gizmo visuals.
This PR also makes improvements to the base visualizer, allowing users to specify rotation offsets
Changes