Closed Bug 1313581 Opened 8 years ago Closed 8 years ago

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

Categories

(Core :: Graphics, defect)

Product:
Core
Component:
Graphics
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla53
Tracking Flags:
Tracking Status
firefox53 --- fixed

People

(Reporter: daoshengmu, Assigned: daoshengmu)

References

Details

(Whiteboard: [webvr])

Attachments

(3 files)

Bug 1313581 - Part 1: Support hand attribute in GamepadAPI;
58 bytes, text/x-review-board-request
kip
: review+
qdot
: review+
Details
Bug 1313581 - Part 2: Support hand attribute in GamepadManager;
58 bytes, text/x-review-board-request
cleu
: review+
kip
: review+
Details
Bug 1313581 - Part 3: Support hand attribute for OpenVR controllers;
58 bytes, text/x-review-board-request
gw280
: review+
Details 
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)
Blocks: 1299926
Whiteboard: [webvr]
Assignee: nobody → dmu
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 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 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)
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 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+
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.
(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 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 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+
Keywords: checkin-needed
need dom peer review for the webidl changes
Flags: needinfo?(dmu)
Keywords: checkin-needed
(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 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 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?
(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.
Keywords: checkin-needed
Autoland couldn't rebase this for landing.
Keywords: checkin-needed
Keywords: checkin-needed
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
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: