-
Notifications
You must be signed in to change notification settings - Fork 394
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
Refactor input identification #695
Conversation
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.
Thanks. The approach in the explainer LG. I left some comments on specifics there.
Thanks for putting this together! I read it over and I think my main question/concern is how, as a developer, I'm expected to use the layout and render parts differently? For example, right now the xr-gamepad-mappings library uses a single ID to pull a json file with the details of the controller. This file includes how to map elements in the buttons/axes array into specific physical controller parts (i.e. thumbsticks). But it also contains information about how to animate the virtual model to match the behavior of the physical gamepad. Under this design I'm having a bit of a hard time picturing the correct way to update the library. Do I start by walking through the fallbacks for the layouts until I find a match? Once I have that match, am I guaranteed that all the renderIds will be compatible with all the layouts? I'd guess not? Or am I missing something here? If I'm not misunderstanding, perhaps an alternative approach would be to make the retrieval of renderId a function call that takes a layoutId as a parameter? |
Addressed Davids' feedback outside of the name issue. Also haven't addressed Nell's comments. Need to give them both some more thought. |
Replying to @NellWaliczek's question, perhaps applications (or libraries) would just not animate button presses, etc. if they don't recognize both values or do not recognize them both as a valid pair together. The unrecognized pairs shouldn't be much more common than not recognizing one of the values. The registry (#578) should help ensure this. @NellWaliczek wrote:
|
Yeah I see what you mean about that being a bit weird. In theory a renderId can exist for XRInputSources without a Gamepad associated with them. My concern is that we'll end up with a mismatch. As in, you end up with a layout that doesn't match with the model that is being rendered. Not sure if that's a real concern... @thetuvix do you have an opinion on this? And for what its worth, not animating is a pretty bad experience since the user will have no idea if the input source is working if they can't see a visual response. |
I'm not sure if this solves the problem, but another way the layouts could be exposed would be to have the Gamepad layout list returned by a method. This might also be a bit more future-compatible since it wouldn't bake Some possible examples:
Note on the parameters in these examples:
|
After thinking on this a bit more and rereading... I'm still kinda lost. I really don't understand the relationship between renderId and layoutId....Would it be possible to update this PR with a few additional bits of sample code? For example, should I check layoutId or renderId first? If I only recognize the 2nd entry in layoutIds, can I be sure that all of the renderIds will work with it? What about if I check renderId first? Will all the layoutIds work with it? Relatedly, does a layoutId need to be limited to things that have gamepads? Or will it work for things with articulated hand tracking? |
After talking with Nell about this on Friday and doing some additional research, I've updated this proposal to hopefully be a bit more straightforward. The latest change combines the two previous lists of IDs into a single list, now called It might feel more limiting at first glance, but after giving some thought I feel like this approach is actually a bit more flexible and can handle a variety of situations. This is based on a few realizations:
Finally, it seems as though OpenXR's "Interaction profiles" concept could be exposed reasonably via this mechanic. We would need a simple mapping from their path names to an appropriately formatted profile name and gamepad layout, but fundamentally they both provide a description of how the device expects to be used. (Would love any comments @thetuvix has in that regard). |
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.
Oh yay! I'm super happy with this approach now. It's very clear to me how I can update the xr-gamepad-mappings library. Heck, now I even want to rename it xr-gamepad-profiles!
A handful of minor comments. I suspect I'll be able to approve w/in one or two more iterations.
index.bs
Outdated
|
||
An <dfn for="XRInputSource">input profile name</dfn> is a lowercase {{DOMString}} containing no spaces, with separate words concatenated with a hyphen (<code>-</code>) character. A descriptive name should be chosen, using the preferred verbiage of the device vendor when possible. If the platform provides an appropriate identifier, such as a USB vendor and product ID, it MAY be used. Values that uniquely identify a single device, such as serial numbers, MUST NOT be used. The [=input profile name=] MUST NOT contain an indication of device handedness. If multiple user agents expose the same device, they SHOULD make an effort to report the same [=input profile name=]. | ||
|
||
Any [=input profile name=]s given after the first should provide fallback values that represent alternative visual representations and/or {{XRInputSource/gamepad}} layouts of the device. This may include a more generic or prior version of the device, a more widely recognized device that is sufficently similar, or a broad description of the device type (such as "touchpad-wand"). If multiple profiles are given the {{XRInputSource/gamepad}} layouts they describe must all represent a superset or subset of every other profile in the list. |
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.
Probably need to call out that the list should be in order of decreasing specificity where possible. We probably also want to state that the last item in the list should not contain any vendor specific information. It doesn't have to be this PR, but we probably want to enumerate a concrete list of allowable "last item in the list" profile names that matches what's supported by the "xr-mapping". Though that could be defined in the registry if necessary.
Very interesting! These profiles are attempting to serve a dual purpose:
However, purpose 2 is only fulfilled if the app is given Some possible solutions:
|
@thetuvix, perhaps I'm misunderstanding something here, but it feels like we can solve this without any signature changes but just ensuring we have the correct set of pre-enumerated "default profiles" for input sources with 'xr-standard' mappings. (Which is actually the suggestion I'd made to @toji before he posted this PR because I was concerned it would be confusing otherwise....) To address your concern more specifically, is there a reason that the final fallback for a windows mixed reality controller wouldn't just be 'thumbstick-touchpad-wand' or whatever we end up calling it? And for hardware with the primary mechanic being the touchpad rather than the thumbstick, then we could define a 'touchpad-thumbstick-wand'? Without being super thorough just yet, I was roughly thinking about this list:
For what it's worth, this approach doesn't preclude our adjusting the ordering of elements in the buttons/axes array. I'm just still a bit skeptical that "fixing" the ordering will create the fallback behavior you're looking for. If I'm understanding correctly, you're concerned that people will always assume that the first element is, say, a touchpad and will get messed up interactions because in some cases it is actually a thumbstick. The counterpoint to that is, if that array entry is empty, developers may not look at the next entry which would leave end users with no way to interact with the content as opposed to a less than ideal way to interact. But also! I was kinda assuming that profiles shouldn't be limited to Gamepads! When we get into hand tracking we'll probably want to distinguish between simple hand gesture selection and articulated hands, yes? I was sort of assuming that this function would serve the same purpose, but perhaps I'm oversimplifying? |
To me, it seemed like a strong API smell for trying to solve forward-compat at this layer if an app supported For example, if we were to do that, |
Note that the list of 5 profiles you listed actually stresses the key reason I'm interested to align on IDs here:
All 5 of these profiles could be comfortably mapped to a subset of a Windows Mixed Reality controller. However, imagine we proceed with this PR and some web app ends up authored for just one of those profiles. There is only a 1 in 5 chance that the profile the app chose happens to be the one that is ID-compatible with the profile chosen for Windows Mixed Reality controllers. If, instead, we assign well-known IDs to touchpad axes and thumbstick axes, the first 4 profiles will all map identically to Windows Mixed Reality controllers, increasing app compat across the web. For |
I'll try to capture some of the discussion/conclusions from the call today: Addressing the possible directions Alex laid out in his comment above intent was to have each profile represent a subset or superset of every other profile in the array. This will definitely be easier to pull off for if we re-organize the In terms of the base "generic" profiles, it's apparent that we should have a list written down for what the expected set is, and that said list should not be part of the spec proper since it's likely to evolve over time. I'll work on producing a starting point for that, possibly as a separate PR. Related to making the Additionally, it's not my expectation that we would have more than one generic profile in the list at any point. ie: While There's an argument to be made that controller interactions can be handled more robustly by the platform if the API communicates how it wants to use the controller. For example: Touchpads can be silently remapped to buttons if necessary. As such it might be compelling to let the app tell the API what profiles it wants to support and allow the API to report back what it was able to conform to. It's my view currently that while this kind of capability could indeed be useful we should stick to trying to expose as much raw data as we can via the Gamepad API for our initial release and enable libraries to provide that sort of mapping initially. That would allow our group more time to see how OpenXR and similar systems evolve and how well they are adopted by the dev community before committing to a more sophisticated path, likely in a new module or a level 2 of the proposed gamepad module. |
Opened #735 to remove any ambiguities from the |
Overall, great summary. There's one thing that differed in my recollection
For this, I think the suggestion was to keep the let profiles = xrInputSource.profiles;
// use some heuristic that isn't this, but is actually meaningful
let selectedProfile = profiles[profiles.length - 1];
xrInputSource.setSelectedProfile(selectedProfile); @thetuvix, does that match your recollection? |
I don't recall that we reached the point of agreeing on that recommendation on the call, and I"m not in favor of it at this time. That feels more appropriate for a new input module or a level 2 gamepad module down the road. (I can generally see that sort of API being most practical to pair with an action-mapping API, like OpenXR has.) |
Ah, sorry I didn't mean we'd made that a recommendation. Just that it was what we were discussing as an option. |
Also opened #738 to outline an initial set of generic profiles. |
|
||
If the {{XRSession}}'s [=mode=] is {{XRSessionMode/inline}}, {{XRInputSource/profiles}} MUST be an empty list. | ||
|
||
If the input device cannot be reliably identified, or the user agent wishes to mask the input device being used, it MAY choose to only report generic [=input profile name=]s or an empty list. |
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.
Given that we're now enumerating generic profile names in PR #738, how do we feel about requiring the profiles list to have at least one generic profile name in the list?
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.
We should absolutely strongly recommend it, but I'm not sure if I'm comfortable requiring that for a few reasons:
- If we're going to require that we really should put the profile names in the spec.
- I don't think we'll have a generic profile for everything that might be exposed. (Hands, tracking pucks, stylus, body tracking markers...)
- It's possible that there's some situations in which the underlying native API doesn't give you a sense of what the device looks like? (Especially true if you're using some custom attachment to a tracking puck.)
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.
That makes sense, though I do feel like we could reasonably bend the rules on the first point if we wanted to require a generic profile name for "masked" hardware. Not a blocking issue, though. If this turns out to be a problem we can always add it later.
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.
LGTM! Only had one question for Alex (which can totally be fixed in a subsequent PR)
index.bs
Outdated
@@ -1353,6 +1354,24 @@ The <dfn attribute for="XRInputSource">gripSpace</dfn> attribute is an {{XRSpace | |||
|
|||
The {{gripSpace}} MUST be <code>null</code> if the input source isn't inherently trackable such as for input sources with a {{targetRayMode}} of {{XRTargetRayMode/gaze}} or {{XRTargetRayMode/screen}}. | |||
|
|||
The <dfn attribute for="XRInputSource">profiles</dfn> attribute is a [=/list=] of [=input profile name=]s indicating both the preffered visual representation and behavior of the input source. For example, if the {{XRInputSource/gamepad}} attribute is not <code>null</code> the profile identifies the mapping of the buttons and axes of the attribute of the {{XRInputSource}}. Profiles are given in descending order of preference. |
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.
Profiles are given in descending order of preference.
Seems like this might make more sense at the end of line 1361, but feel free to ignore
|
||
```js | ||
// Exact strings are examples only. | ||
["samsung-odyssey", "windows-mixed-reality"] | ||
["samsung-odyssey", "windows-mixed-reality", "touchpad-thumbstick-controller"] |
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.
@thetuvix , given that this is your hardware, would you prefer the example strings to be different?
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.
I'm curious to hear Alex's take on this, but given the time zone differences at the moment I'm going to assume that we can safely handle any concerns that do come up as a follow-up PR.
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.
"Samsung Odyssey" is a headset and "Windows Mixed Reality" is a platform, so I'd probably go with samsung-odyssey-controller
and windows-mixed-reality-controller
, to differentiate from windows-mixed-reality-clicker
, etc.
However, when we populate the initial registry with the identifiers we expect to be used at WebXR 1.0, we can shake out this example to match the registry then.
Fixes #550. Removed any logic regarding using the gamepad id for rendering or layout identification. Introduced the `renderId` and `gamepadLayout` attributes to `XRInputSource` in it's place.
Fixes #550.
Removed any logic regarding using the gamepad id for rendering or
layout identification. Introduced the
renderId
andgamepadLayout
attributes to
XRInputSource
in it's place.