New issue
Advanced search Search tips

Issue 870920 link

Starred by 1 user

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

Improve gamepad.mojom

Project Member Reported by dcheng@chromium.org, Aug 3

Issue description

In general, this file could use a lot more comments.

Also:
//ui/gfx/geometry has Quarternion and Vector3dF types, so we should use those.
timestamps should use mojo_base.mojom.TimeDelta or the like
 
Cc: bajones@chromium.org
Also:
- understand why GamepadObserver sends over a gamepad index and also a clone of the gamepad data
- see if the gamepad shared memory helpers can use span<Gamepad>
Duplicating your comment from https://chromium-review.googlesource.com/c/chromium/src/+/1161492/4/device/gamepad/public/mojom/gamepad.mojom#115 so it doesn't get forgotten:

(Re: GamepadHapticsManager interface)

> Is this something that also lives in the browser process? Might be nice to document it as such (in a followup)
> //ui/gfx/geometry has Quarternion and Vector3dF types, so we should use those.

Sounds good to me. Brandon is the editor for the Gamepad Extensions draft that defines the pose member. As far as I know, WebVR/XR is the only code in Chrome that sets the gamepad pose.

https://w3c.github.io/gamepad/extensions.html

Brandon, do you see any issue with changing these types in the extensions proposal?

> timestamps should use mojo_base.mojom.TimeDelta or the like

Nice, I didn't know about this. We can change from int64_t to mojo_base.mojom.TimeTicks. int64_t was chosen because it's the type of the tick count for base::TimeTicks (which is also WTF::TimeTicks).

> understand why GamepadObserver sends over a gamepad index and also a clone of the gamepad data

Neither of these parameters are needed anymore, they are only used in DCHECKs. Let's remove them and consider possibly combining connection/disconnection/button/axis events into a single gamepadstatechange signal since they are all treated the same once they reach GamepadDispatcher.

> see if the gamepad shared memory helpers can use span<Gamepad>

I think this would work. The Gamepads type wraps an array of #pragma packed Gamepad objects. The Gamepads type itself is also packed but this should not affect padding within the span<Gamepad>

Sign in to add a comment