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

Add gamepad input events #152

Open
wants to merge 4 commits into
base: gh-pages
Choose a base branch
from
Open

Add gamepad input events #152

wants to merge 4 commits into from

Conversation

@nondebug
Copy link
Collaborator

@nondebug nondebug commented Jun 10, 2021

Closes #4

Closes #22 ?

The following tasks have been completed:

  • Modified Web platform tests (link to pull request)

Implementation commitment:


Preview | Diff

Base automatically changed from algorithms to gh-pages Jul 8, 2021
@nondebug nondebug force-pushed the button-axis-events branch from 04884c3 to e93ef99 Jul 20, 2021
@reillyeon
Copy link
Member

@reillyeon reillyeon commented Jul 21, 2021

I've updated the description to reference crbug/856290 as indication of implementation commitment from Chromium. Does this change completely replace #15?

index.html Outdated Show resolved Hide resolved
index.html Outdated
|button|.{{GamepadButton/touched}}.
</li>
<li>[=Queue a task=] on the [=gamepad task source=] to [=fire
an event=] named {{buttonchange}} at |gamepad| using
Copy link
Member

@reillyeon reillyeon Jul 21, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should buttonchange also fire when a button is touched but its value has not changed or do we want to save that for separate buttontouchdown and buttontouchup events?

Copy link
Collaborator Author

@nondebug nondebug Jul 27, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think buttonchange should only fire for value changes and we should add touch events if needed.

The current gamepad market doesn't have many gamepads that detect touch separately from pressure so I think the potential for a gamepad to fire buttontouchdown without buttonchange is pretty low.

Copy link
Member

@reillyeon reillyeon Jul 28, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense to me. Do we want to specify that now or leave it as a TODO for later?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am happy either way. I would say let's do the thing that would be easiest from a standards approval perspective. I think that would be to add it here and get it all done in one go.

index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
Copy link
Member

@reillyeon reillyeon left a comment

LGTM

index.html Outdated
<dfn>axisIndex</dfn> member
</dt>
<dd>
The button index for the event.
Copy link
Member

@reillyeon reillyeon Jul 28, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
The button index for the event.
The axis index for the event.

index.html Outdated
<dfn>axisSnapshot</dfn> member
</dt>
<dd>
A copy of the current button state.
Copy link
Member

@reillyeon reillyeon Jul 28, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
A copy of the current button state.
A copy of the current axis state.

Copy link

@jameshollyergoogle jameshollyergoogle left a comment

This looks good. However, I don't see anything about coalescing events as we had discussed. Are we still planning on adding that in?

@reillyeon
Copy link
Member

@reillyeon reillyeon commented Jul 28, 2021

This looks good. However, I don't see anything about coalescing events as we had discussed. Are we still planning on adding that in?

Considering event coalescing does seem important because I am concerned that the current design will result in lots of events being fired for each "update" of the gamepad. For example, when a control stick is moved diagonally that will generate two axis events with identical timestamps. This seems like it will cause more work to be done on the main thread. It should be possible to handle these events simultaneously. Doing this actually seems like a slightly different problem than what is solved by APIs like PointerEvent.getCoalescedEvents() because in that case the coalesced events would otherwise be discarded while for the Gamepad API they would otherwise be fired next.

I can see a couple ways to avoid this problem:

  1. Use a more generic change event which provides a list of buttons and axis that have changed.
  2. Add a dequeueEvents() method to Gamepad which will return the GamepadAxisEvent and GamepadButtonEvents which will be dispatched next according to the algorithms defined here and perhaps cancel dispatching them.

Maybe the issue only really affects axis (since movement of a joystick with X and Y axis means that there is a high likelihood of paired events) and so only applying proposal (1) to GamepadAxisEvents would be enough and GamepadButtonEvents can be left as specified here. This would also make GamepadAxisEvents similar to PointerEvents in having both X and Y axis (or an arbitrary number of axis) updates in the same event.

@nondebug
Copy link
Collaborator Author

@nondebug nondebug commented Jul 29, 2021

I like proposal 1 (a single change event) since most gamepads deliver all button and axis state in one report and many applications would prefer to consume inputs that way. I think buttons and axes should be handled the same way.

interface GamepadChangeEvent : Event {
  // A copy of the gamepad state when this event was created.
  readonly attribute Gamepad gamepadSnapshot;
  // Indices of axes for which the value has changed since the previously dispatched event.
  readonly attribute FrozenArray<long> axesChanged;
  // Indices of buttons for which the value has changed since the previously dispatched event.
  readonly attribute FrozenArray<long> buttonsChanged;
  // Indices of buttons that were pressed since the previously dispatched event.
  readonly attribute FrozenArray<long> buttonsDown;
  // Indices of buttons that were released since the previously dispatched event.
  readonly attribute FrozenArray<long> buttonsUp;

  // Returns the sequence of GamepadChangeEvent instances that were coalesced into this event,
  // or an empty sequence if this instance is not a coalesced event.
  sequence<GamepadChangeEvent> getCoalescedEvents();
};

When an input frame is received from the gamepad, if a change event listener is registered on the Gamepad then create a GamepadChangeEvent from the input frame and queue it to an internal slot on the Gamepad. Then, queue a task on the main thread to (maybe) dispatch the event.

When dispatching, dequeue all the queued instances. If there were no queued instances, don't dispatch an event. Otherwise, create a new GamepadChangeEvent and stash the queued instances in an internal slot so they can be returned by getCoalescedEvents(). Initialize its gamepadSnapshot attribute to the gamepadSnapshot of the most recent instance. Initialize the button and axis index arrays to the set union of the corresponding arrays in the dequeued instances. If a button or axis changed in any coalesced event then its index should be in the buttonsChanged or axesChanged arrays. If a button is pressed and released in separate coalesced events then its index should be in both buttonsDown and buttonsUp.

@reillyeon
Copy link
Member

@reillyeon reillyeon commented Jul 29, 2021

I'm interested in the TAG's opinion on the best design here. I think you're about to submit a review request?

A single event does seem more in line with the PointerEvent interface, which includes a number of different pointer properties (comparable to axis and buttons) in a single event. From that perspective coalescing events is an orthogonal concern and depends on whether we want to specify when the user agent may choose not to fire a GamepadChangeEvent. For example, it could discard events that arrive while the main thread is currently handling another event. (Actually implementing this in Chromium will be non-trivial but could be worthwhile.)

As specified in this PR multiple input reports would be queued and delivered as separate events, while with the current polling-based system some input reports would simply be missed because the page was busy and didn't poll quickly enough. Coalescing events while the main thread is busy seems better than queuing them and delivering a cluster of events at a later time.

index.html Outdated Show resolved Hide resolved
@marcoscaceres
Copy link
Member

@marcoscaceres marcoscaceres commented Aug 3, 2021

Note also garbage collection text might be needed: whatwg/dom#1003

Co-authored-by: Marcos Cáceres <marcos@marcosc.com>
@jakearchibald
Copy link

@jakearchibald jakearchibald commented Aug 9, 2021

Did you folks see #4 (comment)? I propose an axischange event there.

It feels like this matches pointermove, where you're generally interested in the latest value, sync'd to raf. Whereas button presses are more immediate (and must be preceded by events to update the axis state if changes are pending).

Event coalescing can be used to get more detailed axis changes, but even then I wouldn't expect multiple events with the same type and timestamp.

Having an equivalent of pointerrawupate also seems ok.

@nondebug
Copy link
Collaborator Author

@nondebug nondebug commented Aug 25, 2021

@jakearchibald

Event coalescing can be used to get more detailed axis changes, but even then I wouldn't expect multiple events with the same type and timestamp.

Actually, this would be very common. Gamepads are usually implemented using the HID protocol or something similar where all button and axis values are delivered in a single packet. If you're moving a joystick then it's likely you'll see updated X and Y axis values in the same packet, which would result in two axischange events with the same timestamp (but different axis indices).

Having an equivalent of pointerrawupdate also seems ok.

Perhaps there should be a gamepadchange event that fires at some reasonable time synchronized to rAF and provides coalesced events, and a rawgamepadchange event that fires for every change and doesn't coalesce?

I'm looking at the pointermove spec and it looks there's no guidance on when the event should be fired if it's delayed. For gamepadchange I think it would be good to explicitly specify that if there's a pending gamepadchange event then it must be fired before rAF. That lets applications synchronize on rAF if they need to consume some inputs through getGamepads() polling and some through events.

@jakearchibald
Copy link

@jakearchibald jakearchibald commented Aug 26, 2021

which would result in two axischange events with the same timestamp (but different axis indices).

Could/should we combine those into a single event?

Perhaps there should be a gamepadchange event that fires at some reasonable time synchronized to rAF and provides coalesced events, and a rawgamepadchange event that fires for every change and doesn't coalesce?

Yeah, that's what I'm thinking too. That seems to fit with pointer events nicely.

I'm looking at the pointermove spec and it looks there's no guidance on when the event should be fired if it's delayed.

Yeah sigh I'm sad that they haven't done that.

For gamepadchange I think it would be good to explicitly specify that if there's a pending gamepadchange event then it must be fired before rAF.

Agreed! It'd be good to lead the way here and show pointerevents how it should be done 😄.

https://html.spec.whatwg.org/multipage/webappapis.html#event-loop-processing-model steps 11.7-11.12 handle various events that shouls be sync'd to rAF, so I guess the gamepad stuff should go in there.

What about button presses? Do you think those should happen immediately? Mouse clicks do. It's something like this:

On mouse button down:

  1. If the mouse position has changed since the last mousemove event, dispatch a mousemove event.
  2. Dispatch a mousedown event.

So the mouse-down flushes the queued mousemove event.

@nondebug
Copy link
Collaborator Author

@nondebug nondebug commented Sep 4, 2021

The latest patch removes buttonchange/axischange and replaces them with gamepadchange/rawgamepadchange. I've added a "run the gamepad steps" algorithm to integrate with the event loop's pre-animation-frame steps which flushes any pending gamepadchange events.

What about button presses? Do you think those should happen immediately?

Yes, they should happen immediately. I've added a step to flush gamepadchange before firing buttondown/buttonup. Currently it flushes on either event, maybe it should only flush for buttondown?

@marcoscaceres marcoscaceres self-requested a review Sep 9, 2021
@nondebug nondebug changed the title Add buttondown, buttonup, buttonchange, axischange events Add gamepad input events Oct 21, 2021
@jharris1993
Copy link

@jharris1993 jharris1993 commented Jan 8, 2022

Is this still happening?

Is this something that mere mortals like myself can experiment with?

Is this likely to happen any time soon, or should I continue to work on my half-@$$ed hack-job to shim an event?

My (non-expert) two cents:

  1. We should be looking at the larger picture - the joystick/gamepad as a generic user interface device instead of being a "game controller"
  2. I'd be thrilled if I could get a generic event that fires and returns the existing gamepad data structure as it already exists in the gamepad interface.  If the data needs any massaging/smoothing, that can be done after the event is fired.

Ref:
#4 (comment)

@reillyeon
Copy link
Member

@reillyeon reillyeon commented Jan 10, 2022

@jharris1993, this work is still in progress. If I'm interpreting your feedback correctly it sounds like this change is exactly what you're looking for. We would appreciate feedback on specific changes proposed in this PR which you can preview as a rendered specification here. Feedback from developers like you is what helps us prioritize our work and move designs in the right direction. Chromium is planning on implementing these changes but there isn't a prototype ready yet. If you subscribe to the Chromium issue you'll get updates when code for that lands.

@jharris1993
Copy link

@jharris1993 jharris1993 commented Jan 12, 2022

@reillyeon

Ref: https://w3c.github.io/gamepad/#other-events
I don't see much to comment on here except for the fact that they say it "requires more discussion".

My two kopeks:

At least initially, a generic gamepad event that returns the entire gamepad data-frame (gamepad dictionary?) would be sufficient.  (Ref: KISS rule)

As far as data processing is concerned, we should be concentrating on getting the DATA to the developer/application.&nbsp: Any additional data-massaging can be done to the returned data on the server or within the JavaScript on the client..  (i.e Qualifying axis inputs with a button press, normalizing or constraining data, etc.)

After that has been implemented, the question of adding additional gamepad events can be addressed based on need.

P.S.

Where can I comment on that specification?  Note that I am not able to provide specific comments, (i.e. GampadButtonEvent should be a float/int/bool), but I can provide general comments based on need, (i.e. We need event such-and-so and this is why).

P.P.S.

Where can I comment on the W3C/Browser security model requiring a secure context?

@reillyeon
Copy link
Member

@reillyeon reillyeon commented Jan 12, 2022

Thanks for this feedback. I will let @nondebug take it from here. I'm just managing things while he's out of the office.

Where can I comment on that specification?  Note that I am not able to provide specific comments, (i.e. GampadButtonEvent should be a float/int/bool), but I can provide general comments based on need, (i.e. We need event such-and-so and this is why).

"That specification" is this specification, and the issue tracker is here on GitHub.

Where can I comment on the W3C/Browser security model requiring a secure context?

You've already found the issue related to the Gamepad API in particular moving to a secure context requirement. In terms of the overall direction the web security model is moving I think commenting on https://github.com/w3c/webappsec-secure-contexts where "secure context" is defined.

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