Closed Bug 1419344 Opened 2 years ago Closed 2 years ago

Adjust OpenVR input mapping for Microsoft Mixed Reality controllers

Categories

(Core :: WebVR, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla59
Tracking Status
firefox59 --- fixed

People

(Reporter: daoshengmu, Assigned: daoshengmu)

Details

Attachments

(1 file, 2 obsolete files)

Currently, the input mapping of OpenVR controllers for Windows MR is different from Windows MR in Edge browser. We have to do some adjustment to make the content could be consistent.

Axis (0, 1) <---> Axis(2, 3). And Axis 1 needs to be flipped. Axis 3 need to flip?

Button 3 <---> Button 4.
Assignee: nobody → dmu
Group: mozilla-employee-confidential, core-security
Compare with Edge, we have a wrong implementation for the vertical axis value. (https://www.w3.org/TR/gamepad/#dom-gamepad-axes) In order to not affect the current VR content, we add a workaround for yAxis.
Group: core-security → gfx-core-security
How could this be exploited as a security bug?
Flags: needinfo?(dmu)
This is not a security actually. It is just a confidential bug, but I don't know how to uncheck the `Security-Sensitive` option.
Flags: needinfo?(dmu)
Currently, we do not have too much information about Windows MR from the OpenVR runtime. I think it should be a temporary fix for supporting Windows MR devices in Firefox. Regarding to yAxisInvert, that is because we follow the implementation that Chromium did, we have argued about this, but it doesn't be changed.
Attachment #8931286 - Flags: review?(kgilbert)
Comment on attachment 8931286 [details] [diff] [review]
0001-Bug-1419344-Adjust-OpenVR-input-mapping-for-Microsof.patch

This LGTM.

Later, we may want to make similar adjustments of button orders for other OpenVR controllers.
Attachment #8931286 - Flags: review?(kgilbert) → review+
The controllers of Windows MR didn't support haptic feedback yet. I have checked it when running https://webvr.info/samples/XX-vr-controllers.html on Edge. Therefore, there is no response when calling aVRSystem->TriggerHapticPulse() from OpenVR. We can watch if there is any update from their side afterward.
Please help land it to m-c.
Keywords: checkin-needed
Needs a rebased patch.
Flags: needinfo?(dmu)
Keywords: checkin-needed
Carry the r+ from attachment 8931286 [details] [diff] [review]. I have rebased it with the current m-c, please help land it again. Thanks!
Attachment #8931286 - Attachment is obsolete: true
Flags: needinfo?(dmu)
Attachment #8932001 - Flags: review+
Attachment #8932001 - Flags: checkin?(ryanvm)
Comment on attachment 8932001 [details] [diff] [review]
0001-Bug-1419344-Adjust-OpenVR-input-mapping-for-Microsof.patch

Still doesn't apply to inbound tip for me :(
Flags: needinfo?(dmu)
Attachment #8932001 - Flags: checkin?(ryanvm)
Sorry for the apply failure.

I have rebased it with the current inbound/branches/default/tip, please help land it again. (Carry the r+ from attachment 8931286 [details] [diff] [review]) Thanks!
Attachment #8932001 - Attachment is obsolete: true
Flags: needinfo?(dmu)
Attachment #8932306 - Flags: review+
Attachment #8932306 - Flags: checkin?(ryanvm)
Comment on attachment 8932306 [details] [diff] [review]
0001-Bug-1419344-Adjust-OpenVR-input-mapping-for-Microsof.patch

The issue with the patch ended up being Windows vs. Unix line endings. Applies fine with that fixed :)

https://hg.mozilla.org/integration/mozilla-inbound/rev/fa7f2b1503e77f334039a3fe8f3646a8f34bada8
Attachment #8932306 - Flags: checkin?(ryanvm)
https://hg.mozilla.org/mozilla-central/rev/fa7f2b1503e7
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
Group: gfx-core-security
Group: mozilla-employee-confidential
You need to log in before you can comment on or make changes to this bug.