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

RESOLVED FIXED in Firefox 55

Status

()

defect
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: daoshengmu, Assigned: daoshengmu)

Tracking

({feature})

unspecified
mozilla55
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox55 fixed)

Details

(Whiteboard: [gfx-noted], )

Attachments

(4 attachments, 1 obsolete attachment)

Assignee

Updated

2 years ago
Blocks: 1299926
Assignee

Updated

2 years ago
Summary: [webvr] Support HTC Vive button touched → [webvr] Support HTC Vive button touched in the Gamepad API
Whiteboard: [gfx-noted]
Component: Graphics → WebVR
Assignee

Updated

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

Comment 7

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

Comment 11

2 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.

Comment 12

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

Updated

2 years ago
Attachment #8855366 - Attachment is obsolete: true
Attachment #8855366 - Flags: review?(kyle)
Assignee

Comment 17

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

Comment 18

2 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.

Comment 20

2 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+

Comment 21

2 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+
Comment on attachment 8855362 [details]
Bug 1336002 - Part 1: Support button touched in the Gamepad API;

https://reviewboard.mozilla.org/r/127214/#review131102
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+
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+

Comment 25

2 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
You need to log in before you can comment on or make changes to this bug.