[webvr] Support HTC Vive controller hand through the Gamepad API

RESOLVED FIXED in Firefox 53

Status

()

defect
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: daoshengmu, Assigned: daoshengmu)

Tracking

unspecified
mozilla53
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox53 fixed)

Details

(Whiteboard: [webvr])

Attachments

(3 attachments)

(Assignee)

Description

3 years ago
In the gamepad extensions API in https://w3c.github.io/gamepad/extensions.html#gamepadhand-enum, it defines a string for indicating the hand that is holding the gamepad.

OpenVR API already provides its hand enum. (https://github.com/ValveSoftware/openvr/blob/5bc41e4b55d11dfc8fb4b958a6600aa7f8cee051/headers/openvr.h#L177)
(Assignee)

Updated

3 years ago
Blocks: 1299926
Whiteboard: [webvr]
(Assignee)

Updated

3 years ago
Assignee: nobody → dmu
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Comment 10

3 years ago
These patches are for supporting hand attribute in Gamepad API as Comment 1 mentioned.

As our expectation, there are some static casting because we have to support non MOZ_GAMEPAD platforms. We look forward Bug1315896 would help us resolve them in our future work.

Try looks good. https://treeherder.mozilla.org/#/jobs?repo=try&revision=416963530995d21b27bdee2ce6355199d84354e7

Comment 11

3 years ago
mozreview-review
Comment on attachment 8809330 [details]
Bug 1313581 - Part 2: Support hand attribute in GamepadManager;

https://reviewboard.mozilla.org/r/91926/#review92980
Attachment #8809330 - Flags: review?(cleu) → review+
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment on attachment 8809329 [details]
Bug 1313581 - Part 1: Support hand attribute in GamepadAPI;

Moving review to cleu. I'm busy with some other stuff right now and this doesn't really need DOM peer approval.

cleu, if you've got to many reviews, let me know and I can take this one back.
Attachment #8809329 - Flags: review?(kyle) → review?(cleu)
Attachment #8809330 - Flags: review?(kyle)

Comment 15

3 years ago
mozreview-review
Comment on attachment 8809331 [details]
Bug 1313581 - Part 3: Support hand attribute for OpenVR controllers;

https://reviewboard.mozilla.org/r/91928/#review93276

r+, but please increase the scope of bug 1315896 to deal with the uint32_t casting here for the GamepadHand type.
Attachment #8809331 - Flags: review?(gwright) → review+

Comment 16

3 years ago
mozreview-review
Comment on attachment 8809329 [details]
Bug 1313581 - Part 1: Support hand attribute in GamepadAPI;

https://reviewboard.mozilla.org/r/91924/#review93482

LGTM
Attachment #8809329 - Flags: review?(cleu) → review+
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Comment 20

3 years ago
qdot, sorry for bothering again. I need you help grant my part 1 & 2 for Mozreview landable because both cleu and me are not Lv 3. If you are still busy in something. I can wait for Kip's back for moving forward. Thanks.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Comment 24

2 years ago
(In reply to Daosheng Mu[:daoshengmu] from comment #21)
> Comment on attachment 8809329 [details]
> Bug 1313581 - Part 1: Support hand attribute in GamepadAPI;
> 
> Review request updated; see interdiff:
> https://reviewboard.mozilla.org/r/91924/diff/4-5/
(In reply to Daosheng Mu[:daoshengmu] from comment #20)
> qdot, sorry for bothering again. I need you help grant my part 1 & 2 for
> Mozreview landable because both cleu and me are not Lv 3. If you are still
> busy in something. I can wait for Kip's back for moving forward. Thanks.

Due yo Kip's back, I forward the r? to kip. Thanks again.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 28

2 years ago
mozreview-review
Comment on attachment 8809329 [details]
Bug 1313581 - Part 1: Support hand attribute in GamepadAPI;

https://reviewboard.mozilla.org/r/91924/#review95378

LGTM, thanks!
Attachment #8809329 - Flags: review?(kgilbert) → review+

Comment 29

2 years ago
mozreview-review
Comment on attachment 8809330 [details]
Bug 1313581 - Part 2: Support hand attribute in GamepadManager;

https://reviewboard.mozilla.org/r/91926/#review95380

Looks good, thanks!
Attachment #8809330 - Flags: review?(kgilbert) → review+
(Assignee)

Updated

2 years ago
Keywords: checkin-needed
need dom peer review for the webidl changes
Flags: needinfo?(dmu)
Keywords: checkin-needed
(Assignee)

Comment 31

2 years ago
(In reply to Carsten Book [:Tomcat] from comment #30)
> need dom peer review for the webidl changes

Got it. I will wait qdot for helping review. Thanks.
Flags: needinfo?(dmu)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 35

2 years ago
mozreview-review
Comment on attachment 8809329 [details]
Bug 1313581 - Part 1: Support hand attribute in GamepadAPI;

https://reviewboard.mozilla.org/r/91924/#review96568
Attachment #8809329 - Flags: review?(kyle) → review+

Comment 36

2 years ago
mozreview-review
Comment on attachment 8809329 [details]
Bug 1313581 - Part 1: Support hand attribute in GamepadAPI;

https://reviewboard.mozilla.org/r/91924/#review96570

::: dom/webidl/Gamepad.webidl:5
(Diff revision 7)
>  /* -*- Mode: IDL; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
>  /* This Source Code Form is subject to the terms of the Mozilla Public
>   * License, v. 2.0. If a copy of the MPL was not distributed with this
>   * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
>  

Nit: Can you add the spec URLs for both the gamepad spec, and the VR extensions spec here?
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Comment 40

2 years ago
(In reply to Daosheng Mu[:daoshengmu] from comment #37)
> Comment on attachment 8809329 [details]
> Bug 1313581 - Part 1: Support hand attribute in GamepadAPI;
> 
> Review request updated; see interdiff:
> https://reviewboard.mozilla.org/r/91924/diff/7-8/

Follow the Comment 36 to add the spec URLs.
(Assignee)

Updated

2 years ago
Keywords: checkin-needed
Autoland couldn't rebase this for landing.
Keywords: checkin-needed
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Updated

2 years ago
Keywords: checkin-needed

Comment 45

2 years ago
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c98f0110352b
Part 1: Support hand attribute in GamepadAPI; r=kip,lenzak800,qdot
https://hg.mozilla.org/integration/autoland/rev/4a5a49522f28
Part 2: Support hand attribute in GamepadManager; r=kip,lenzak800
https://hg.mozilla.org/integration/autoland/rev/bacf701ce65c
Part 3: Support hand attribute for OpenVR controllers; r=gw280
Keywords: checkin-needed

Comment 46

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/c98f0110352b
https://hg.mozilla.org/mozilla-central/rev/4a5a49522f28
https://hg.mozilla.org/mozilla-central/rev/bacf701ce65c
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
You need to log in before you can comment on or make changes to this bug.