1336002 - [webvr] Support HTC Vive button touched in the Gamepad API

Status: RESOLVED FIXED

(Core :: WebVR, defect)

Description

[Daosheng Mu[:daoshengmu]]

https://w3c.github.io/gamepad/#dom-gamepadbutton

Comment 1

[Daosheng Mu[:daoshengmu]]

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

Comment 2

[Daosheng Mu[:daoshengmu]]

Attachment: Bug 1336002 - Part 1: Support button touched in the Gamepad API;

Review commit: https://reviewboard.mozilla.org/r/127214/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/127214/

Comment 3

[Daosheng Mu[:daoshengmu]]

Attachment: Bug 1336002 - Part 4: Add gamepad button touched tests;

Review commit: https://reviewboard.mozilla.org/r/127216/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/127216/

Comment 4

[Daosheng Mu[:daoshengmu]]

Attachment: Bug 1336002 - Part 2: Support button touched in GamepadManager;

Review commit: https://reviewboard.mozilla.org/r/127218/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/127218/

Comment 5

[Daosheng Mu[:daoshengmu]]

Attachment: Bug 1336002 - Part 3: Support button touched in OpenVR and Puppet;

Review commit: https://reviewboard.mozilla.org/r/127220/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/127220/

Comment 6

[Daosheng Mu[:daoshengmu]]

Attachment: Bug 1336002 - Part 5: Add gamepad button touched tests;

Review commit: https://reviewboard.mozilla.org/r/127222/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/127222/

Comment 7

[:kip (Kearwood Gilbert)]

Comment on attachment 8855365 [details]
Bug 1336002 - Part 3: Support button touched in OpenVR and Puppet;

https://reviewboard.mozilla.org/r/127220/#review129986

This LGTM, Thanks!

Comment 8

[Daosheng Mu[:daoshengmu]]

Comment on attachment 8855364 [details]
Bug 1336002 - Part 2: Support button touched in GamepadManager;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/127218/diff/1-2/

Comment 9

[Daosheng Mu[:daoshengmu]]

Comment on attachment 8855365 [details]
Bug 1336002 - Part 3: Support button touched in OpenVR and Puppet;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/127220/diff/1-2/

Comment 10

[Daosheng Mu[:daoshengmu]]

Comment on attachment 8855366 [details]
Bug 1336002 - Part 5: Add gamepad button touched tests;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/127222/diff/1-2/

Comment 11

[Michael Leu[:Lenzak](UTC+8)]

Comment on attachment 8855364 [details]
Bug 1336002 - Part 2: Support button touched in GamepadManager;

https://reviewboard.mozilla.org/r/127218/#review130148

::: dom/gamepad/android/AndroidGamepad.cpp:50
(Diff revision 2)
>          GamepadPlatformService::GetParentService();
>      if (!service) {
>        return;
>      }
>  
> -    service->NewButtonEvent(aID, aButton, aPressed, aValue);
> +    service->NewButtonEvent(aID, aButton, aPressed, aPressed, aValue);

It looks a bit awkward when you need to pass duplicate argument reduntantly.

I think you should add another "NewButtonEvent" with touch event support.

Comment 12

[Michael Leu[:Lenzak](UTC+8)]

Comment on attachment 8855366 [details]
Bug 1336002 - Part 5: Add gamepad button touched tests;

https://reviewboard.mozilla.org/r/127222/#review130720

LGTM, although it's a bit awkward to pass duplicate argument redundantly in NewButtonEvent for now, we may add touch support for these tests in the future.

Comment 13

[Daosheng Mu[:daoshengmu]]

Comment on attachment 8855362 [details]
Bug 1336002 - Part 1: Support button touched in the Gamepad API;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/127214/diff/1-2/

Comment 14

[Daosheng Mu[:daoshengmu]]

Comment on attachment 8855364 [details]
Bug 1336002 - Part 2: Support button touched in GamepadManager;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/127218/diff/2-3/

Comment 15

[Daosheng Mu[:daoshengmu]]

Comment on attachment 8855365 [details]
Bug 1336002 - Part 3: Support button touched in OpenVR and Puppet;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/127220/diff/2-3/

Comment 16

[Daosheng Mu[:daoshengmu]]

Comment on attachment 8855363 [details]
Bug 1336002 - Part 4: Add gamepad button touched tests;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/127216/diff/1-2/

Comment 17

[Daosheng Mu[:daoshengmu]]

I move the patch of updateTimeStamp for gamepadpose changes to an another separate bug, Bug 1354977.

Comment 18

[Daosheng Mu[:daoshengmu]]

Comment on attachment 8855364 [details]
Bug 1336002 - Part 2: Support button touched in GamepadManager;

https://reviewboard.mozilla.org/r/127218/#review130148

> It looks a bit awkward when you need to pass duplicate argument reduntantly.
> 
> I think you should add another "NewButtonEvent" with touch event support.

Good idea! thanks.

Comment 19

[Daosheng Mu[:daoshengmu]]

try looks good, https://treeherder.mozilla.org/#/jobs?repo=try&revision=a2436986ed84b4f68af36ceecba8326aa45bc72e

Comment 20

[Michael Leu[:Lenzak](UTC+8)]

Comment on attachment 8855364 [details]
Bug 1336002 - Part 2: Support button touched in GamepadManager;

https://reviewboard.mozilla.org/r/127218/#review130838

Looks nice!

Comment 21

[Michael Leu[:Lenzak](UTC+8)]

Comment on attachment 8855363 [details]
Bug 1336002 - Part 4: Add gamepad button touched tests;

https://reviewboard.mozilla.org/r/127216/#review130840

Comment 22

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Comment on attachment 8855362 [details]
Bug 1336002 - Part 1: Support button touched in the Gamepad API;

https://reviewboard.mozilla.org/r/127214/#review131102

Comment 23

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Comment on attachment 8855362 [details]
Bug 1336002 - Part 1: Support button touched in the Gamepad API;

https://reviewboard.mozilla.org/r/127214/#review131104

Comment 24

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Comment on attachment 8855363 [details]
Bug 1336002 - Part 4: Add gamepad button touched tests;

https://reviewboard.mozilla.org/r/127216/#review131106

Comment 25

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fafb04b95d0f
Part 1: Support button touched in the Gamepad API; r=qdot
https://hg.mozilla.org/integration/autoland/rev/20750f1dbcaf
Part 2: Support button touched in GamepadManager; r=Lenzak
https://hg.mozilla.org/integration/autoland/rev/961f1f9a522e
Part 3: Support button touched in OpenVR and Puppet; r=kip
https://hg.mozilla.org/integration/autoland/rev/4d1d356ad1b3
Part 4: Add gamepad button touched tests; r=Lenzak,qdot

Comment 26

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/fafb04b95d0f
https://hg.mozilla.org/mozilla-central/rev/20750f1dbcaf
https://hg.mozilla.org/mozilla-central/rev/961f1f9a522e
https://hg.mozilla.org/mozilla-central/rev/4d1d356ad1b3