1606562 - Axes on XInput (Gamepad) on Windows are out of the allowed range

Status: VERIFIED FIXED

(Core :: DOM: Device Interfaces, defect, P1)

Description

[emil.lenngren]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/78.0.3904.108 Safari/537.36

Steps to reproduce:

Plug in an Xbox One controller on your Windows computer. Then navigate to https://html5gamepad.com/ to try it out and inspect the axis values when tilting the analog sticks maximally.

Actual results:

Minimum left is -1.00003 and maximum down is 1.00003.

Expected results:

According to https://w3c.github.io/gamepad/#dom-gamepad-axes the values must be normalized to the range [-1.0 .. 1.0]. So minimum left should be -1.0 and maximum down should be 1.0.

The bug is here: https://github.com/mozilla/gecko-dev/blob/b222bd7a3952a54fb0b8c965189dfb779caf7f23/dom/gamepad/windows/WindowsGamepad.cpp#L531. The int16_t value is converted to a float by dividing by 32767. But the minimum int16_t value is -32768, and hence the value -1.00003 arises.

Comment 1

[emil.lenngren]

This currently breaks games compiled with Emscripten and SDL2. SDL2 gives controller axes as int16_t values. It converts the Gamepad value by multiplying by 32767, followed by a cast to int16_t (http://hg.libsdl.org/SDL/file/0f7577d28c6c/src/joystick/emscripten/SDL_sysjoystick.c#l366). This causes maximum down to become -32768 (which means maximum up), rather than 32767.

Comment 2

[Pablo]

Confirmed issue on windows10 64bit using:
Firefox Release 71 affected
Firefox beta 72 affected
Firefox nightly 73.0a1 affected

With chrome this issue is not happening

Steps:
-go to https://html5gamepad.com/
-pressing down the left and right analog stick with xbox one controller
-then pressing left the left and right analog stick with xbox one controller

left analog stick:
left: axis 0 -1.00003
down: axis 1 1.00003

right analog stick:
left: axis 2 -1.00003
down: axis 3 1.00003

Comment 3

[Daosheng Mu[:daoshengmu]]

It is a bug definitely. I wanna make sure if

left analog stick:
right: axis 0
up: axis 1

right analog stick:
right: axis 2
up: axis 3

work as we expect [-1, 1]? Because SHORT range is [-32768, 32767], it should work correctly when the sThumbXXX is positive.

[1] https://docs.microsoft.com/en-us/windows/win32/api/xinput/ns-xinput-xinput_gamepad

Comment 4

[emil.lenngren]

Yep, up/right work fine, since those come from positive sThumbXXX.

Comment 5

[Pablo]

Attachment: 3html5_gamepad_tester_firefox_nightly_2020_01_15_10_29_14 - Copy.mp4

Comment 6

[Pablo]

Daosheng Mu

I uploaded a video,
left stick > up, right, down, left
right stick> up, right, down, left

please let me know if you need anything else.
Regards

Comment 7

[Daosheng Mu[:daoshengmu]]

Attachment: Bug 1606562 - Fixes XInput thumb value ranges.

sThumbXXX in XInput is a short and its value range is between -32768 and 32767.

Comment 8

[emil.lenngren]

Shouldn't const short div = state.Gamepad.sThumbLX > 0 ? 32767 : -32768 be const int div = state.Gamepad.sThumbLX > 0 ? 32767 : 32768 in the patch? Otherwise only the absolute value is used.

Comment 9

[emil.lenngren]

div should also be a double by the way otherwise you get integer by integer division which results in 0

Comment 10

[Daosheng Mu[:daoshengmu]]

(In reply to emil.lenngren from comment #9)

div should also be a double by the way otherwise you get integer by integer division which results in 0

Yes! you're right.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/504b34d8043d
Fixes XInput thumb value ranges. r=jmathies

Comment 12

[Razvan Maries]

https://hg.mozilla.org/mozilla-central/rev/504b34d8043d

Comment 13

[BugBot [:suhaib / :marco/ :calixte]]

Since the status are different for nightly and release, what's the status for beta?
For more information, please visit auto_nag documentation.

Comment 14

[Ryan VanderMeulen [:RyanVM]]

Is this something we should consider for Beta uplift?

Comment 15

[Daosheng Mu[:daoshengmu]]

Comment on attachment 9122726 [details]
Bug 1606562 - Fixes XInput thumb value ranges.

Beta/Release Uplift Approval Request

• User impact if declined: axis values of gamepads will not right (1.0 -> 1.00003)
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: :Pablo can you help verify it if it works properly? Follow Comment 2 and make sure the values are 1.0 instead of 1.00003.
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This is just a value change to avoid size overflow, so it has no risk.
• String changes made/needed:

Comment 16

[Pascal Chevrel:pascalc (PTO until April 26)]

Comment on attachment 9122726 [details]
Bug 1606562 - Fixes XInput thumb value ranges.

Landed 2 weeks ago with no regression, looks low risk, uplift approved for 74.0b8, thanks.

Comment 17

[Razvan Maries]

https://hg.mozilla.org/releases/mozilla-beta/rev/162bdffd5691

Comment 18

[Pablo]

Verified fix on Windows 10 64bit Using Firefox beta 74.0b8 and Nightly 75.0a1

Left stick:
up axis1 = -1.00000
right axis0 = 1.00000
down axis1 = 1.00000
left axis0 = -1.00000

Right stick:
up axis3 = -1.00000
right axis2 = 1.00000
down axis3 = 1.00000
left axis2 = -1.00000