1318586 - [webvr] Adjust OpenVR controller button mapping

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Daosheng Mu[:daoshengmu]]

Our current button mapping follows OpenVR sdk. But, it is not mapped to other WebVR projects, like https://github.com/aframevr/aframe/blob/03139adbc3567b7116fc4f8b92d6aae6794d97a2/src/components/vive-controls.js#L32. Chromium is also do the same thing, https://chromium.googlesource.com/experimental/chromium/src/+/60e635ba381f28aeb880fe737e003af4b47b1aa7/device/vr/openvr/open_vr_gamepad_data_fetcher.cc#115.

Our current implementation follows OpenVR's header, https://github.com/ValveSoftware/openvr/blob/5bc41e4b55d11dfc8fb4b958a6600aa7f8cee051/headers/openvr.h#L540. One thing we could enhance is checking SupportedButtons property for different devices' button support.
uint64_t supported_buttons = vr_system_->GetUint64TrackedDeviceProperty(deviceId, vr::Prop_SupportedButtons_Uint64);

Comment 1

[Daosheng Mu[:daoshengmu]]

Attachment: Bug 1318586 - Adjust OpenVR controller button mapping to be consistency;

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

Comment 2

[Daosheng Mu[:daoshengmu]]

After discussing with MozVR team, we hope to make the button mapping to be consistency with the current WebVR projects in this short term. https://github.com/aframevr/aframe/blob/03139adbc3567b7116fc4f8b92d6aae6794d97a2/src/components/vive-controls.js#L32.

And we will keep discussing with other implementers to finalize the decision about the mapping.

Comment 3

[Daosheng Mu[:daoshengmu]]

Comment on attachment 8812107 [details]
Bug 1318586 - Adjust OpenVR controller button mapping to be consistency;

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

Comment 4

[Daosheng Mu[:daoshengmu]]

Because Kip's back, I change the r? to kip.
gw280, thanks for your help.

Comment 5

[:kip (Kearwood Gilbert)]

Comment on attachment 8812107 [details]
Bug 1318586 - Adjust OpenVR controller button mapping to be consistency;

https://reviewboard.mozilla.org/r/93992/#review95374

This looks good to me.  r=me with a small comment to warn people that changing the order of the buttons could break web content

::: gfx/vr/gfxVROpenVR.cpp:64
(Diff revision 2)
>  static pfn_VR_GetGenericInterface vr_GetGenericInterface = nullptr;
>  
>  // EButton_System, EButton_DPad_xx, and EButton_A
>  // can not be triggered in Steam Vive in OpenVR SDK 1.0.3.
>  const uint64_t gOpenVRButtonMask[] = {
>    // vr::ButtonMaskFromId(vr::EVRButtonId::k_EButton_System),

Perhaps a short comment here stating that changing the order of these buttons may break web content could be useful.

Comment 6

[Daosheng Mu[:daoshengmu]]

Comment on attachment 8812107 [details]
Bug 1318586 - Adjust OpenVR controller button mapping to be consistency;

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

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/809655f44e19
Adjust OpenVR controller button mapping to be consistency; r=kip

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/809655f44e19