8000 Refactor input identification by toji · Pull Request #695 · immersive-web/webxr · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
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

Refactor input identification #695

Merged
merged 5 commits into from
Jun 26, 2019
Merged

Refactor input identification #695

merged 5 commits into from
Jun 26, 2019

Conversation

toji
Copy link
Member
@toji toji commented Jun 13, 2019

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.

@toji toji added this to the June 2019 milestone Jun 13, 2019
@toji toji requested a review from NellWaliczek June 13, 2019 18:51
Copy link
Contributor
@ddorwin ddorwin left a 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.

@NellWaliczek
Copy link
Member

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?

@toji
Copy link
Member Author
toji commented Jun 14, 2019

Addressed Davids' feedback outside of the name issue. Also haven't addressed Nell's comments. Need to give them both some more thought.

@ddorwin
Copy link
Contributor
ddorwin commented Jun 15, 2019

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:

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?

gamepadLayout is specific to Gamepad, so it would be weird to make obtaining renderId dependent on it in the spec. The xr-gamepad-mappings library could maybe make such an association, though.

@NellWaliczek
Copy link
Member

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.

@ddorwin
Copy link
Contributor
ddorwin commented Jun 18, 2019

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 gamepad further into the API.

Some possible examples:

  1. sequence<DOMString> getLayout(DOMString renderId);
  2. sequence<DOMString> getLayout(Gamepad gamepad);
  3. sequence<DOMString> getLayout(DOMString renderId, Gamepad gamepad);

Note on the parameters in these examples:

  • renderId would be one of the values from the renderIds attribute.
    • Something to think about is whether this would add complexity to implementations.
  • gamepad would be the gamepad attribute on the XRInputSource object.
    • This might not do much other than indicate that the layout is being requested for a Gamepad object vs. something else - see below.
  • In the future, the Gamepad parameter could become optional or a union in order to support future mechanisms that replace Gamepad.

@NellWaliczek
Copy link
Member

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?

@toji
Copy link
Member Author
toji commented Jun 24, 2019

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 profiles. This is intended to cover device appearance, gamepad layout, and any other needs that we might have in the future.

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:

  • Frequently appearance and layout will be linked. This is especially true when animating models based on the current input state, and even if you try to separate them developers will still try to infer one from the other. So a string like "oculus-touch" should be fine serving as both.
  • It prevents UAs from having multiple layouts for a single visual ID. Sounds like a bug, but I think it's a feature. This means that if you're going to use a particular profile string you have to ensure you're synced up with the rest of the UAs regarding how that string is used, which ultimately is better for devs. It also naturally encourages gamepad layouts to be more predictable, since we'll likely end up with a few distinct "tracks" of devices that all are working to maintain compatibility between devices.
  • You could actually mix and match "visual" and "layout" profiles if you wanted, but the reality of it is that they'll end up being both no matter what you try. Take, for example, the ["samsung-odyssey", "windows-mixed-reality", "touchpad-thumbstick-wand"] example in the PR: Samsung Odyssey really only describes the visual design of the device, while Windows mixed reality can be thought of only describing the layout, but the reality is if your app recognizes a Samsung Odyssey, it will know that it's using a Windows Mixed Reality layout (even if internally it's just a pointer to a common data set), and if you recognized Windows Mixed reality as a layout at all then you likely have an appropriate mesh for it, and even in the fallback case, which ostensibly only defines the layout, it's still extremely likely that developers will come up with a generic-looking "stick with a touchpad and a joystick" model to use in the case that this is the only thing present.
  • By not explicitly stating that it's a gamepad layout identifier or visual identifier we also leave things open for this to eventually help identify other aspects of the input device, like (for example) what style of hand tracking is being used.

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).

Copy link
Member
@NellWaliczek NellWaliczek left a 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.
Copy link
Member

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.

8000
@thetuvix
Copy link
Contributor

Very interesting!

These profiles are attempting to serve a dual purpose:

  1. To tell the app which render model to use.
  2. To tell the app how to interpret the button and axis arrays they get back from the Gamepad API.

However, purpose 2 is only fulfilled if the app is given buttons and axes arrays that match the particular profile that the app then happens to choose. If any of the "touchpad-thumbstick-wand" profile, the "windows-mixed-reality" profile and the "valve-index" profile differ in whether they choose the touchpad or thumbstick as their primary axis, the app cannot correctly use the gamepad attribute across both the Valve Index and Samsung Odyssey controllers.

Some possible solutions:

  • Enforce a rule that all profiles returned must provide an equivalent meaning for any button or axis IDs that overlap. However, that would prevent us from ever implementing both a generic "touchpad-wand" profile for HTC Vive-like controllers and a "thumbstick-wand" profile for Oculus Touch-like controllers, and then supporting them both on a Windows Mixed Reality controller. We could decide that kind of thing is out of scope, delegating that fallback support to @NellWaliczek's app library, but then it seems odd to bother with the halfway measure of "thumbstick-wand" fallback support in WebXR itself.
  • Define stronger bindings from well-known controller parts to numerical IDs in the "xr-standard" mapping. For example, we could decide that axis 0 and 1 are always a primary touchpad and axis 2 and 3 are always a primary thumbstick, and so on. If a controller only has one or the other control, the intervening array elements would be left blank. This would also have the positive side effect of giving far more predictable button/axis IDs for well-known controller parts for those users that do not use an extra controller mapping library on top, while giving up fairly little in app compat - my experience is that an app which built its logic assuming thumbstick will not behave correctly when presented with touchpad input, and vice versa. My recollection of why we didn't go that strongly-mapped route was that we wanted to avoid gaps in the gamepad array. However, per the spec's current gamepad image, one of our example controllers leaves buttons[3] blank, and so we're already down that road.
  • Change the XRInputSource.gamepad attribute to a getGamepad(profile) method, enabling the UA to adapt the controller that's in use to the controller that the app actually tested against. This enables the UA to renumber the buttons and axes as needed to meet the app's expectation, even when the set of fallback profiles evolves organically over time in a mutually-exclusive manner. This could even enable a UA to support a "touchpad-only" app running on a "thumbstick-only" controller, e.g. by injecting synthetic touch/untouch events when the thumbstick is moved off-center. This is far closer to the similar OpenXR concept of "interaction profiles", which involve an explicit negotiation with the app, although it still allows the app to make the final selection of profile.
  • Change the XRSession.inputSources attribute to a XRSession.getInputSources(supportedProfiles) method, enabling the UA to adapt all controllers in advance to the set of controllers that the app actually tested against. This would enable the UA to be in control of selecting among the profiles that the app supports, expressing the UA's forward-compatible preference for best fit rather than the app's preference.
  • Expose only the UA's single most-preferred profile for a given input source, moving away from the WebXR API supporting multiple profiles. This relies on apps to use a higher-layer library like the one @NellWaliczek is building to support forward-compatibility to future input devices. While this provides the least forward-compatibility in the WebXR spec itself, it also avoids some of the problems explored above if the subset of forward-compatibility we're able to tackle in WebXR 1.0 becomes awkward when we then attempt to compose in the other half of forward-compatibility in the app library.

@NellWaliczek
Copy link
Member

@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:

  • touchpad-thumbstick-wand
  • thumbstick-touchpad-wand
  • touchpad-wand
  • thumbstick-wand
  • button-wand

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?

@NellWaliczek NellWaliczek added the agenda Request discussion in the next telecon/FTF label Jun 25, 2019
@thetuvix
Copy link
Contributor
thetuvix commented Jun 25, 2019

To me, it seemed like a strong API smell for trying to solve forward-compat at this layer if an app supported "touchpad-thumbstick-wand" and was then incompatible with devices that supported "thumbstick-touchpad-wand" instead. That kind of thing strongly suggests to me that we made the wrong tradeoff in allowing touchpads and thumbsticks to differ in their axis IDs across profiles rather than just assigning well-known IDs to "primary touchpad" and "primary thumbstick".

For example, if we were to do that, "touchpad-thumbstick-wand" and "thumbstick-touchpad-wand" would collapse to the same profile, and we'd avoid that entire class of app compat issue. As you point out, the primary concern in doing so is that we would have gaps in the axes array, which could cause some apps enumerating axes to do the wrong thing if they stop prematurely. However, per the image below, we are coming out of the gate with gaps in well-known controllers like Oculus Touch - at that point, WebXR content will be forced to account for such gaps. Once content has to account for those gaps anyway, we should just go all-in and assign well-known IDs for well-known parts within the xr-standard mapping, avoiding the "touchpad-thumbstick-wand"/"thumbstick-touchpad-wand" gotchas in the approach above. Note that this requires no signature changes to the WebXR or Gamepad APIs - it only requires a table in the spec to designate IDs.

xr-standard mapping

@thetuvix
Copy link
Contributor

Note that the list of 5 profiles you listed actually stresses the key reason I'm interested to align on IDs here:

  • touchpad-thumbstick-wand
  • thumbstick-touchpad-wand
  • touchpad-wand
  • thumbstick-wand
  • button-wand

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 "button-wand", I'd encourage us to see if we really need that one - we already have the "onselect" event as the mechanism we introduced to represent a "primary action" across disparate input forms. I'm not sure we're buying enough for the tradeoffs involved in having buttons[0] on each Gamepad also represent the same thing.

@toji
Copy link
Member Author
toji commented Jun 25, 2019

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 xr-standard mapping to give joysticks and touchpads static mappings, so my intent is to do that in a separate PR.

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 xr-standard mapping above more rigid, it's also my intent to not support both a "touch 628C pad-thumbstick-controller" and a "thumbstick-touchpad-controller" profile, as that would imply that we're trying to communicate a precedence that may not actually be there, and is unlikely to be meaningful to developers who, in the presence of both types of input, will likely make app-specific decisions about how to use them.

Additionally, it's not my expectation that we would have more than one generic profile in the list at any point. ie: While ["touchpad-thumbstick-controller", "touchpad-controller", "thumbstick-controller"] can certainly be argued to be accurate, it shouldn't be necessary as long as the generic profile set is reasonably sized and well defined. This is something that we can continue to evaluate, though, and also enable after the fact if it becomes necessary.

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.

@toji
Copy link
Member Author
toji commented Jun 25, 2019

Opened #735 to remove any ambiguities from the xr-standard mapping and thus make the profiles a bit more practical.

@NellWaliczek
Copy link
Member

Overall, great summary. There's one thing that differed in my recollection

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.

For this, I think the suggestion was to keep the profiles array as is, and add an additional API developers can call to ensure that the buttons behave in alignment with the profile they will actually be using. In otherwords:

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?

@toji
Copy link
Member Author
toji commented Jun 25, 2019

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.)

@NellWaliczek
Copy link
Member

Ah, sorry I didn't mean we'd made that a recommendation. Just that it was what we were discussing as an option.

@toji
Copy link
Member Author
toji commented Jun 25, 2019

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.
Copy link
Member

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?

Copy link
Member Author

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.)

Copy link
Member

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.

Copy link
Member
@NellWaliczek NellWaliczek left a 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.
Copy link
Member

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"]
Copy link
Member

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?

Copy link
Member Author

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.

Copy link
Contributor

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.

toji added 5 commits June 26, 2019 14:42
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.
@toji toji merged commit b68280b into master Jun 26, 2019
@toji toji deleted the input-id branch June 26, 2019 21:56
@AdaRoseCannon AdaRoseCannon added ed:explainer Include in newsletter, explainer change ed:spec Include in newsletter, spec change and removed agenda Request discussion in the next telecon/FTF labels Sep 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ed:explainer Include in newsletter, explainer change ed:spec Include in newsletter, spec change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Format of Gamepad identifiers
5 participants
0