Controller model fallbacks#10568
Conversation
RogPodge
requested review from
cre8ivepark,
david-c-kline,
julenka and
keveleigh
as code owners
|
/azp run |
|
Azure Pipelines successfully started running 2 pipeline(s). |
|
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 <[email protected]>
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). |
|
/azp run |
|
Azure Pipelines successfully started running 2 pipeline(s). |
david-c-kline
left a comment
david-c-kline
left a comment
There was a problem hiding this comment.
A couple minor bits. Otherwise looks good.
| @@ -0,0 +1,21 @@ | |||
| MIT License | |||
There was a problem hiding this comment.
Please be sure to update the NOTICES.md file to indicate our usage of these models.
There was a problem hiding this comment.
cgmanifest.json as well, which can be used to generate parts of the Notices file.
RogPodge
force-pushed
the
controllerModelFallbacks
branch
from
d67fb9b to
ac0aefc
Compare
|
/azp run |
|
Azure Pipelines successfully started running 2 pipeline(s). |
| @@ -0,0 +1,8 @@ | |||
| fileFormatVersion: 2 | |||
There was a problem hiding this comment.
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.
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.
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.
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.
4 participants
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