Closed Bug 1336002 Opened 4 years ago Closed 3 years ago

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

Categories

(Core :: WebVR, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: daoshengmu, Assigned: daoshengmu)

References

()

Details

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

Attachments

(4 files, 1 obsolete file)

Blocks: 1299926
Summary: [webvr] Support HTC Vive button touched → [webvr] Support HTC Vive button touched in the Gamepad API
Whiteboard: [gfx-noted]
Component: Graphics → WebVR
Assignee: nobody → dmu
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 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 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+
Attachment #8855366 - Attachment is obsolete: true
Attachment #8855366 - Flags: review?(kyle)
I move the patch of updateTimeStamp for gamepadpose changes to an another separate bug, Bug 1354977.
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 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 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+
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.