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

Need to spec liveness of Gamepad objects #8

Open
luser opened this issue Apr 24, 2015 · 17 comments · May be fixed by #123
Open

Need to spec liveness of Gamepad objects #8

luser opened this issue Apr 24, 2015 · 17 comments · May be fixed by #123

Comments

@luser
Copy link
Contributor

luser commented Apr 24, 2015

Originally filed as https://www.w3.org/Bugs/Public/show_bug.cgi?id=21434:

Ted Mielczarek [:ted] 2013-03-29 13:34:49 UTC

The discussion in bug 17309 made it clear that Scott and I have implemented two different things in regards to the "liveness" of Gamepad objects. Scott's Chrome implementation returns a static snapshot of each Gamepad when you getGamepads(), whereas the Gamepad object attached to the "ongamepadconnected" event in my Firefox implementation is live: you can get the latest device state out of it at any time.

We need to decide what the correct behavior here is and spec it.

Obviously I'm biased in favor of my implementation, but I think it does have one large benefit: we're expecting users to poll Gamepad status in a requestAnimationFrame loop, so if you're generating new Gamepad objects for every call to getGamepads() you're going to be generating a lot of garbage.

[reply] [−] Comment 1 Scott Graham 2013-03-29 17:26:04 UTC

(In reply to comment #0)

The discussion in bug 17309 made it clear that Scott and I have implemented
two different things in regards to the "liveness" of Gamepad objects.
Scott's Chrome implementation returns a static snapshot of each Gamepad when
you getGamepads(), whereas the Gamepad object attached to the
"ongamepadconnected" event in my Firefox implementation is live: you can get
the latest device state out of it at any time.

We need to decide what the correct behavior here is and spec it.

Obviously I'm biased in favor of my implementation, but I think it does have
one large benefit: we're expecting users to poll Gamepad status in a
requestAnimationFrame loop, so if you're generating new Gamepad objects for
every call to getGamepads() you're going to be generating a lot of garbage.

(I'm honestly not sure precisely what I implemented.)

I agree on the reducing garbage concern, and so returning/modifying the same objects sounds right to me.

The part I don't like as much is that the data unexpectedly changes, and it's unclear when, and with what frequency it's updated. That's why I used getGamepads() as the sync point for "Please update the data in my gamepad objects to the most recent state".

[reply] [−] Comment 2 Ted Mielczarek [:ted] 2013-04-17 13:14:21 UTC

The HTML WebApp spec has concepts we can use to define this in a straightforward way:
http://www.whatwg.org/specs/web-apps/current-work/multipage/webappapis.html#concept-task

It basically specs thread event queues, so that you can say that the Gamepad state will be updated after the current script has finished executing.

[reply] [−] Comment 3 Ted Mielczarek [:ted] 2014-06-23 18:41:43 UTC

I chatted with another implementer and they were curious as to whether (assuming that Gamepad objects are snapshots):

navigator.getGamepads()[0] == navigator.getGamepads()[0]

ought to be true. It looks like it currently is in Chrome, but not in the IE developer preview.

We agreed that sensible spec language would allow that to be true, and in fact the spec should allow implementations to return the same snapshot for a Gamepad until the data changes (the gamepad has been interacted with in some way).

[reply] [−] Comment 4 Philip Jägenstedt 2014-12-18 15:25:35 UTC

Live Array-like objects like NodeList are more work to implement and test, we should avoid making more of them.

That navigator.getGamepads()[0] === navigator.getGamepads()[0] should always hold true seems like a given, otherwise you'll necessarily have multiple Gamepad objects representing the same underlying hardware, and the mess of updating all of them. Unless the idea is that a Gamepad object is a snapshot and you have to call getGamepads() every time you want to check for changes? Easy to implement, but a bit odd...

Finally, navigator.getGamepads() === navigator.getGamepads()? Returning a new array every time seems easier bindings-wise at least in Blink. It does require having a separate array internally to keep track of the Gamepad objects so that you don't create new one. Spec'ing and testing how the return value of getGamepads() is reused seems like an unnecessary burden to me.

[reply] [−] Comment 5 Ted Mielczarek [:ted] 2014-12-18 15:50:16 UTC

(In reply to Philip Jägenstedt from comment #4)

Live Array-like objects like NodeList are more work to implement and test,
we should avoid making more of them.

Yeah, I don't think anyone is arguing for that, thankfully.

That navigator.getGamepads()[0] === navigator.getGamepads()[0] should always
hold true seems like a given, otherwise you'll necessarily have multiple
Gamepad objects representing the same underlying hardware, and the mess of
Unless the idea is that a Gamepad object is a snapshot
and you have to call getGamepads() every time you want to check for changes?
Easy to implement, but a bit odd...

This is the entire crux of this bug: currently the Gamepad objects in Firefox always represent the most-recent-available state of the controller, whereas in Chrome and IE they represent a snapshot of the state as of the time you called navigator.getGamepads().

There was a long thread about this on public-webapps earlier this year:
http://lists.w3.org/Archives/Public/public-webapps/2014AprJun/0238.html

It didn't really reach any strong conclusions, there are good arguments to be made both ways. One compelling argument that was made is that if we do spec some sort of data change events in the future we will want snapshots for that:
http://lists.w3.org/Archives/Public/public-webapps/2014AprJun/0257.html

There's also a pretty good argument (that didn't come up on-list) that having snapshots makes it easier to compare gamepad state in the polling model, since you can hang on to the previous snapshot and compare vs. the next snapshot (using .timestamp, for one, and checking whether button states match etc).

[reply] [−] Comment 6 Philip Jägenstedt 2015-02-09 11:29:14 UTC

I'm trying to implement the Navigator.getGamepads() change in https://codereview.chromium.org/808643005/ but quickly realized that "Retrieve a snapshot of the data for the the currently connected and interacted-with gamepads." isn't a certainly until this spec bug is resolved.

Having getGamepads() return a new array of completly new Gamepad object every time is simple enough to implement, but it seems to make the GamepadEvent nonsensical, as the GamepadEvent.gamepad member will necessarily be a Gamepad object constructed only for that event and that will never appear in any array returned by Navigator.getGamepads().

What does Firefox do?

[reply] [−] Comment 7 Philip Jägenstedt 2015-02-09 13:32:22 UTC

(In reply to Philip Jägenstedt from comment #6)

What does Firefox do?

I don't have a gamepad to test with here, but judging by the code in gecko-dev and minor testing, it looks like Firefox always returns a new array, but the internal Gamepad objects are just allocated once. One point of doubt is |Gamepad::Clone|, is that called as part of getGamepads() so that a new object is seen by script every time?

@foolip
Copy link
Member

foolip commented Oct 14, 2019

@toji what is the status of this issue? It's come up in Intent to Ship: WebXR Gamepad Module on blink-dev.

If this isn't interoperable as this issue suggests it isn't, then resolving it in the spec seems fairly high priority.

@foolip
Copy link
Member

foolip commented Oct 14, 2019

FWIW, given the "The same object MUST be returned until the user agent needs to return different values (or values in a different order)" wording for axes and buttons, it seems like it was at one point the intention for a Gamepad object to be live in the sense that its attributes can start returning new values, and this does make sense to me. Gamepad objects do have an id, but to have to call navigator.getGamepads() for every animation frame seems odd given the naming of the API.

@toji
Copy link
Member

toji commented Oct 14, 2019

To clarify, I'm no longer an editor on this spec.

Speaking from the perspective of an editor within the Immersive Web Working Group, however, I understand the concern that our use of gamepad differs from how this spec text implies they should be used. It is worth noting that every Gamepad implementation I'm familiar with does update the gamepad objects in-place upon calling getGamepads(), which seems to contradict the spec text. I'm skeptical that the UA implementations will change to match the spec text any time soon, given that I believe it's already been previously identified by Microsoft as a content compatibility issue. As such, I would rather that WebXR be consistent with the observed behavior of gamepads rather than the single sentence in the Gamepad spec that states they should behave otherwise.

That said, I also don't think the spec and implementations should disagree. It would be best if either the spec updated to reflect the currently observed behavior accurately or we get agreement from all UAs that they will update getGamepads() to return actual snapshots. If this group takes the latter route then WebXR would be willing to change to match, especially given that backwards compatibility will already be broken at that point. In the meantime, since it's not clear which direction the Gamepad group wants to go, WebXR will continue with (and ship) a live attribute.

@annevk
Copy link
Member

annevk commented Oct 15, 2019

There's a couple issues here, right? Does getGamepads() return new objects or the same objects. What's the lifetime of those objects if they're the same (agent lifetime I guess). And if they are the same, when is their state updated and is that allowed to violate run-to-completion semantics? (Ideally not.)

@foolip
Copy link
Member

foolip commented Oct 15, 2019

@foolip
Copy link
Member

foolip commented Oct 16, 2019

There's some relevant discussion in Intent to Ship: WebXR Gamepad Module on blink-dev.

It sounds like because the number of buttons don't change, the buttons could keep returning the same array every time, and could be decorated with [SameObject]. The GamepadButton objects are what would change, at some well defined time. Maybe that's at every microtask checkpoint, top of the event loop or rendering frame, as long as it doesn't change while script is running anything is probably web compatible.

If axes needs to change it would however need to return a new array, so that can't be given the [SameObject] extended attribute.

@annevk
Copy link
Member

annevk commented Oct 16, 2019

As long as each get does not return a new array that is fine (same principle as the other getters). My main concern here is not so much compatibility by the way, but violating run-to-completion semantics.

@foolip
Copy link
Member

foolip commented Oct 16, 2019

From what I can tell this can be made sensible in terms of not changing state while script is running and FrozenArray<T> can be used.

I think any remaining warts after this might be with navigator.getGamepads() itself. It currently returns an array of fixed length where all entries can be null, presumably because originally the index was to be kept stable when the Gamepad instance might be replaced. If a Gamepad object is connected to the underlying gamepad and updated at well defined times, it seems to me that navigator.getGamepads() could just return an empty array if there are no gamepads. But there's some chance that's not web compatible.

@foolip
Copy link
Member

foolip commented Oct 16, 2019

@nondebug (Matt Reynolds) can you describe roughly the changes you'd like to make here?

@nondebug
Copy link
Collaborator

Sure, please take a look:

getGamepads() should return an array of Gamepad objects. The gamepad array is NOT a live array; the returned array will always hold the same Gamepad objects even if gamepads are connected and disconnected. The same gamepad array may be returned if no gamepads have been connected or disconnected. (But it's also fine to construct a new array for each call.)

Each non-null item in the array returned by getGamepads() corresponds to a single connected gamepad. As long as the gamepad remains connected, getGamepads() should return the same Gamepad object for that gamepad, at the same array index.

Gamepad objects are live. The id and index members should not be modified once the object is returned to script, but other members may be updated due to user interaction with the gamepad. (Should mapping or hand be allowed to change? This might be useful for devices that can be operated in multiple modes without a disconnection, eg Switch Joy-Cons.)

The connected member should return true as long as the gamepad remains connected, and false afterward.

The axes member is NOT a live array; it should hold the same array until any axis changes or the order of axes changes, after which it should return a new array with the updated axis state. (Should we continue to return the same array after disconnection?)

The buttons member is NOT a live array; it should hold the same array of GamepadButton objects as long as the gamepad remains connected and the order of buttons does not change. If the order of buttons changes, a new array should be returned. Each item in the array should hold the same GamepadButton object for the lifetime of the buttons array. (Should we continue to return the same array after disconnection?)

GamepadButton is live and its properties (value, pressed, touched) may change after the button has been returned to script.

The timestamp member should be updated whenever connected, axes, or buttons (also pose) is updated with new values. If there is no observable state change, the timestamp should not change. (This differs from the current spec language which describes the timestamp as "Last time the data for this gamepad was updated" which could include updates where the data received from the device matches the previous state. IMO this is not useful, API users should assume the browser is receiving updates continuously until the gamepad is disconnected.)

Live objects (Gamepad, GamepadButton) are updated when script is not running. If a change occurs while script is running, they will continue to hold the same values until the top of the event loop.

@annevk
Copy link
Member

annevk commented Oct 17, 2019

(But it's also fine to construct a new array for each call.)

That's not great, either store the JavaScript array object on the object and replace it when needed, or always create a new array based on the internal data.

should return

Must, right? (Same for other shoulds.)

other members may be updated due to user interaction with the gamepad

This will have to be defined as updating internal state on the Gamepad objects at a specific point in time to avoid violating run-to-completion semantics. (You're saying as much at the end, but I hope the eventual definition of this will fall out of the processing model.)

@luser
Copy link
Contributor Author

luser commented Oct 17, 2019

If both Firefox and Chrome's implementations do not match the spec text then I think the spec text should be changed to match the implementations. Someone should test Safari and Edge as well. I was under the impression that the Chrome implementation was snapshots, but perhaps that changed over time?

Regardless, last I knew the Firefox implementation actually implemented updates to Gamepad objects by doing the internal equivalent of queuing microtasks to update the state, so using some spec language like "Updates to Gamepad objects should be queued as microtasks in the current event loop" should both be sensible and not contradict the Firefox implementation. Note that there have been requests to add support for the Gamepad API to Web Workers so ideally the spec text could be written to not make it more difficult to add that in the future.

There are some existing manual web platform tests, it should be possible to write new tests for this behavior.

@foolip
Copy link
Member

foolip commented Oct 18, 2019

@nondebug thanks, that look great! On the various "new array" questions, this is my best understanding:

  • navigator.getGamepads() has to return a new each time, because the return value is sequence<T>. It was discussed earlier in the issue but I think this should be uncontroversial now.
  • axes should return the same frozen array for every call until its values need to change, at which point it starts returning another frozen array.
  • buttons is actually the same, that the values are objects and not numbers doesn't change much in terms of when a new array is returned.

For when updates happen, I think there are three choices:

  • The spec defines exactly where in the rendering loop it happens, i.e., it would only happen between animation frames. I don't think this is what anyone wants though, because the gamepads aren't actually synchronized with the rendering at the hardware level.
  • Allow updates to happen at the top of the event loop.
  • Allow updates to happen at any microtask checkpoint.

Between the last two, I think "top of the event loop" is probably fine? But just matching what is or will be implemented is more important than what I think :)

@Manishearth
Copy link
Member

FWIW in immersive-web/webxr-gamepads-module#18 (PR at immersive-web/webxr-gamepads-module#22 ) WebXR seems to be settling on making them live, but updated every XR frame. We already have a reliable frame tick which we can piggyback on for this (so we don't need to create new tasks).

@nondebug
Copy link
Collaborator

That's not great, either store the JavaScript array object on the object and replace it when needed, or always create a new array based on the internal data

navigator.getGamepads() has to return a new each time, because the return value is sequence<T>.

If Gamepad is a live object then the array returned by getGamepads() should only need to change when gamepads are added or removed from the list.

Must, right? (Same for other shoulds.)

Yes, apologies, I'm new to spec language.

This will have to be defined as updating internal state on the Gamepad objects at a specific point in time to avoid violating run-to-completion semantics. (You're saying as much at the end, but I hope the eventual definition of this will fall out of the processing model.)

Regardless, last I knew the Firefox implementation actually implemented updates to Gamepad objects by doing the internal equivalent of queuing microtasks to update the state, so using some spec language like "Updates to Gamepad objects should be queued as microtasks in the current event loop" should both be sensible and not contradict the Firefox implementation.

For when updates happen, I think there are three choices:

Let's go with "updates should be queued as microtasks in the current event loop" to match the Firefox implementation.

Note that there have been requests to add support for the Gamepad API to Web Workers so ideally the spec text could be written to not make it more difficult to add that in the future.

@luser, can you elaborate on this? (Is there an issue with tying the update to microtasks or the event queue in a Web Worker?)

nondebug added a commit to nondebug/gamepad that referenced this issue Oct 24, 2019
@nondebug nondebug linked a pull request Oct 24, 2019 that will close this issue
4 tasks
@foolip
Copy link
Member

foolip commented Oct 24, 2019

If Gamepad is a live object then the array returned by getGamepads() should only need to change when gamepads are added or removed from the list.

The objects in the arrays may be the same, but each time getGamepads() is called it has to return a new array. This is how sequence<T> conversion is defined, see "Let A be a new Array" at step 2 of "An IDL sequence value S of type sequence is converted to an ECMAScript Array object as follows" in https://heycam.github.io/webidl/#es-sequence.

@luser
Copy link
Contributor Author

luser commented Oct 25, 2019

@luser, can you elaborate on this? (Is there an issue with tying the update to microtasks or the event queue in a Web Worker?)

I just wanted to call that out to make sure that we don't wind up with spec text that ties the update rate to requestAnimationFrame or anything like that which would make it harder to make things work in a Web Worker in the future. I think your proposed text should be fine!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants