Closed Bug 1336002 Opened 7 years ago Closed 7 years ago

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

Tracking

()

Status:

RESOLVED FIXED

Milestone:

mozilla55

Tracking Flags:

Tracking

Status

firefox55

---

fixed

People

(Reporter: daoshengmu, Assigned: daoshengmu)

References

(
URL
)

Details

(Keywords: feature, Whiteboard: [gfx-noted])

Attachments

(4 files, 1 obsolete file)

Bug 1336002 - Part 1: Support button touched in the Gamepad API; 7 years ago Daosheng Mu[:daoshengmu] 59 bytes, text/x-review-board-request	qdot : review+	Details
Bug 1336002 - Part 4: Add gamepad button touched tests; 7 years ago Daosheng Mu[:daoshengmu] 59 bytes, text/x-review-board-request	qdot : review+ cleu : review+	Details
Bug 1336002 - Part 2: Support button touched in GamepadManager; 7 years ago Daosheng Mu[:daoshengmu] 59 bytes, text/x-review-board-request	cleu : review+	Details
Bug 1336002 - Part 3: Support button touched in OpenVR and Puppet; 7 years ago Daosheng Mu[:daoshengmu] 59 bytes, text/x-review-board-request	kip : review+	Details
Bug 1336002 - Part 5: Add gamepad button touched tests; 7 years ago Daosheng Mu[:daoshengmu] 59 bytes, text/x-review-board-request	cleu : review+	Details

Daosheng Mu[:daoshengmu]

Assignee

Description

•

7 years ago

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

Daosheng Mu[:daoshengmu]

Assignee

Updated

•

7 years ago

Blocks: 1299926

Daosheng Mu[:daoshengmu]

Assignee

Updated

•

7 years ago

Summary: [webvr] Support HTC Vive button touched → [webvr] Support HTC Vive button touched in the Gamepad API

George Wright (:gw280) (needinfo me!)

Updated

•

7 years ago

Whiteboard: [gfx-noted]

George Wright (:gw280) (needinfo me!)

Updated

•

7 years ago

Keywords: feature

:kip (Kearwood Gilbert)

Updated

•

7 years ago

Component: Graphics → WebVR

Daosheng Mu[:daoshengmu]

Assignee

Updated

•

7 years ago

Assignee: nobody → dmu

Daosheng Mu[:daoshengmu]

Assignee

Updated

•

7 years ago

URL: https://w3c.github.io/gamepad/#gamepa...

Daosheng Mu[:daoshengmu]

Assignee

Comment 1

•

7 years ago

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

Comment hidden (mozreview-request)

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

Comment hidden (mozreview-request)

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

Comment hidden (mozreview-request)

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

Comment hidden (mozreview-request)

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

Comment hidden (mozreview-request)

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

:kip (Kearwood Gilbert)

Comment 7

•

7 years ago

mozreview-review

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!

Attachment #8855365 - Flags: review?(kgilbert) → review+

Comment hidden (mozreview-request)

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 hidden (mozreview-request)

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 hidden (mozreview-request)

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/

Michael Leu[:Lenzak](UTC+8)

Comment 11

•

7 years ago

mozreview-review

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.

Michael Leu[:Lenzak](UTC+8)

Comment 12

•

7 years ago

mozreview-review

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.

Attachment #8855366 - Flags: review?(cleu) → review+

Comment hidden (mozreview-request)

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 hidden (mozreview-request)

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 hidden (mozreview-request)

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 hidden (mozreview-request)

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/

Daosheng Mu[:daoshengmu]

Assignee

Updated

•

7 years ago

Attachment #8855366 - Attachment is obsolete: true

Attachment #8855366 - Flags: review?(kyle)

Daosheng Mu[:daoshengmu]

Assignee

Comment 17

•

7 years ago

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

Daosheng Mu[:daoshengmu]

Assignee

Comment 18

•

7 years ago

mozreview-review-reply

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.

Daosheng Mu[:daoshengmu]

Assignee

Comment 19

•

7 years ago

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

Michael Leu[:Lenzak](UTC+8)

Comment 20

•

7 years ago

mozreview-review

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

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

Looks nice!

Attachment #8855364 - Flags: review?(cleu) → review+

Michael Leu[:Lenzak](UTC+8)

Comment 21

•

7 years ago

mozreview-review

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

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

Attachment #8855363 - Flags: review?(cleu) → review+

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

Comment 22

•

7 years ago

mozreview-review

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

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

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

Comment 23

•

7 years ago

mozreview-review

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

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

Attachment #8855362 - Flags: review?(kyle) → review+

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

Comment 24

•

7 years ago

mozreview-review

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

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

Attachment #8855363 - Flags: review?(kyle) → review+

Pulsebot

Comment 25

•

7 years ago

Pushed by dmu@mozilla.com:
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

Carsten Book [:Tomcat]

Comment 26

•

7 years ago

bugherder

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

Status: NEW → RESOLVED

Closed: 7 years ago

status-firefox55: --- → fixed

Resolution: --- → FIXED

Target Milestone: --- → mozilla55

You need to log in before you can comment on or make changes to this bug.