immersive-web / webxr-gamepads-module

Repository for the WebXR Gamepads Module
https://immersive-web.github.io/webxr-gamepads-module
Other
30 stars 10 forks source link

Ensure that the returned gamepad object is live #18

Closed kearwood closed 4 years ago

kearwood commented 4 years ago

During TPAC 2019 we were made aware that not all UA's are consistent with the gamepad object behavior.

For the Gamepad objects returned by WebXR, we should ensure that the objects are "live", for all implementing UA's.

NellWaliczek commented 4 years ago

IIRC, it was more that the Gamepad folks were asking us to revisit this decision in light of the fact that the behavior mismatches with the Gamepad spec. (of course in practice, this the Gampad implementations aren't consistent in this behavior).

NellWaliczek commented 4 years ago

Hold on... I'm a bit unsure this was the conclusion from that conversation. In fact, from my recollection it was the opposite thing they were asking us to do. To make sure that the Gamepad object on an XRInputSource was different every time it was requested. The implication of which would be revisiting if the gamepad should be an attribute or have a getter function.

/agenda To confirm the expected behavior from the Gamepad folks and get consensus on which approach we intend to take.

Manishearth commented 4 years ago

We could do a getter function like inputSource.getGamepad(xrFrame) which is more in line with all the other per-frame data that we obtain. It would also make the lifecycle much more explicit. The alternative would be a getter function which operates on the "recentmost frame" which seems tricky to pin down (and potentially inconsistent).

The normal Gamepad API has no such frame driver which makes this way more confusing and is probably why the implementations are much more inconsistent.

Manishearth commented 4 years ago

The editors discussed this a bit today.

While the Gamepad spec doesn't really make the objects live, this doesn't match with the implementations, and we'd like for gamepad objects to be live here.

I brought up that multithreaded and multiprocess browsers (ideally your code doing weird device stuff is in a separate process) are going to have problems with "live" since in such a scenario nothing is truly "live", and you don't want blocking IPC calls on gamepad access. We need to specify when it updates.

The natural thing to do is to update it on every frame. Not just on every animation frame, but instead every time any kind of frame (be it input or animation) is created. We do similar things for renderState right now, though that's trickier since it's about both writing and reading.

Note that in the spec an animation frame can happen even if rAF() isn't called, animation frames when there is no rAF mostly do nothing other than updating render state right now.

foolip commented 4 years ago

Related: https://github.com/w3c/gamepad/issues/8

probot-label[bot] commented 4 years ago

This issue is fixed by PR #22

AdaRoseCannon commented 4 years ago

This is still marked with the agenda label, even though a PR has been made to fix it, does it still need to be discussed?

If you want to update the topic just use '/agenda' again.

luser commented 4 years ago

The natural thing to do is to update it on every frame. Not just on every animation frame, but instead every time any kind of frame (be it input or animation) is created. We do similar things for renderState right now, though that's trickier since it's about both writing and reading.

I'm pretty sure this is what happens in practice in all the existing Gamepad implementations anyway--updates to Gamepad state are effectively queued as microtasks that get run at the end of the current task, so updates don't actually violate JS run-to-completion semantics.

Manishearth commented 4 years ago

@luser I'm curious: what's the heartbeat for that? WebXR has a natural heartbeat but non-XR gamepads don't have any intrinsic notion of frame, aside from perhaps window.rAF

@AdaRoseCannon I think folks might have opinions on whether that pull request is the correct way of doing things, so best to discuss it in the call? I'm also okay with merging as is if there seems to be enough consensus already.

luser commented 4 years ago

I don't think there's any particular "heartbeat" here. Some platform APIs provide an event-based model where you get notified on new data, some require polling. Either way it's reasonable to send data updates to pages that have used the Gamepad API as often as they arrive.

luser commented 4 years ago

I should say that given that the Gamepad API only provides a polling interface via getGamepads() it's very likely that all of the callers will be using rAF anyway.

toji commented 4 years ago

@Manishearth: In Chrome, at least, the gamepads were polled at 60Hz by a background thread and the values polled were placed into shared memory for the next call to getGamepads() to pick up. I think the polling rate may have been upped to 100Hz when WebVR came along to mitigate jank in the controller movement. Regardless, that's not something that's dictated by the spec and the pages still needed to explicitly poll for new values to be populated into the live objects anyway.

Manishearth commented 4 years ago

Ah, I see. Makes sense. Glad we have a natural heartbeat here so that we can neatly spec this.

Artyom17 commented 4 years ago

A bit of off-topic, but important in the context. @toji - the issue with the gamepad polling approach in Chrome which I recently stepped into: the gamepad poller doesn't know anything about the frame which is currently being rendered and the pose in the gamepad object is super sensitive to this matter: if the wrong pose is being fetched for the frame then you get judder. For example, for PC side it is wrong to use ovr_GetInputState; you must use ovr_GetTrackingState and extract HandPoses from it which are frame-specific. But it is a moot point now.

Artyom17 commented 4 years ago

What is the status of this issue? Isn't it fixed by PR #22 ? If so, why the issue is still opened? And more importantly - is it safe to ship gamepad module with this change, are there any other breaking changes coming?

Manishearth commented 4 years ago

22 hasn't landed yet, it's waiting on https://github.com/immersive-web/webxr/pull/897 which needs a review from @NellWaliczek

Note that this is not a breaking change, potential resolutions of this issue involved breaking changes but we did not end up choosing that route.