[PATCH 2/2] hid: Don't sign-extend 16-bit values.
Kai Krakow
kai at kaishome.de
Sun Feb 10 19:16:33 CST 2019
Hey Zeb!
Am So., 10. Feb. 2019 um 22:57 Uhr schrieb Zebediah Figura
<z.figura12 at gmail.com>:
>
> Is the controller being reported through udev, then?
Not exactly sure, what you mean... In Steam, I disabled controller
configuration support for this controller because it messes with the
axis, converting them to mouse movement in some games. So it probably
doesn't go through the Steam uinput implementation.
"udevadm monitor" shows the controller when turning it on:
KERNEL[20453.657006] add
/devices/pci0000:00/0000:00:1d.0/usb2/2-1/2-1.6/2-1.6.3/2-1.6.3:1.0/bluetooth/hci0/hci0:71
(bluetooth)
UDEV [20453.679748] add
/devices/pci0000:00/0000:00:1d.0/usb2/2-1/2-1.6/2-1.6.3/2-1.6.3:1.0/bluetooth/hci0/hci0:71
(bluetooth)
KERNEL[20453.756728] add
/devices/pci0000:00/0000:00:1d.0/usb2/2-1/2-1.6/2-1.6.3/2-1.6.3:1.0/bluetooth/hci0/hci0:71/0005:045E:02FD.0013
(hid)
KERNEL[20453.757194] add
/devices/pci0000:00/0000:00:1d.0/usb2/2-1/2-1.6/2-1.6.3/2-1.6.3:1.0/bluetooth/hci0/hci0:71/0005:045E:02FD.0013/input/input19
(input)
KERNEL[20453.757408] add
/devices/pci0000:00/0000:00:1d.0/usb2/2-1/2-1.6/2-1.6.3/2-1.6.3:1.0/bluetooth/hci0/hci0:71/0005:045E:02FD.0013/input/input19/js0
(input)
KERNEL[20453.757459] add
/devices/pci0000:00/0000:00:1d.0/usb2/2-1/2-1.6/2-1.6.3/2-1.6.3:1.0/bluetooth/hci0/hci0:71/0005:045E:02FD.0013/input/input19/event17
(input)
KERNEL[20453.757491] add
/devices/pci0000:00/0000:00:1d.0/usb2/2-1/2-1.6/2-1.6.3/2-1.6.3:1.0/bluetooth/hci0/hci0:71/0005:045E:02FD.0013/hidraw/hidraw10
(hidraw)
KERNEL[20454.753078] bind
/devices/pci0000:00/0000:00:1d.0/usb2/2-1/2-1.6/2-1.6.3/2-1.6.3:1.0/bluetooth/hci0/hci0:71/0005:045E:02FD.0013
(hid)
KERNEL[20454.770809] change
/devices/pci0000:00/0000:00:1d.0/usb2/2-1/2-1.6/2-1.6.3/2-1.6.3:1.0/bluetooth/hci0/hci0:71/0005:045E:02FD.0013/power_supply/xpadneo_batt_3a:66:33:3a:38:63_0
(power_supply)
UDEV [20454.778571] add
/devices/pci0000:00/0000:00:1d.0/usb2/2-1/2-1.6/2-1.6.3/2-1.6.3:1.0/bluetooth/hci0/hci0:71/0005:045E:02FD.0013
(hid)
UDEV [20454.782705] add
/devices/pci0000:00/0000:00:1d.0/usb2/2-1/2-1.6/2-1.6.3/2-1.6.3:1.0/bluetooth/hci0/hci0:71/0005:045E:02FD.0013/input/input19
(input)
UDEV [20454.783105] add
/devices/pci0000:00/0000:00:1d.0/usb2/2-1/2-1.6/2-1.6.3/2-1.6.3:1.0/bluetooth/hci0/hci0:71/0005:045E:02FD.0013/hidraw/hidraw10
(hidraw)
UDEV [20454.792791] add
/devices/pci0000:00/0000:00:1d.0/usb2/2-1/2-1.6/2-1.6.3/2-1.6.3:1.0/bluetooth/hci0/hci0:71/0005:045E:02FD.0013/input/input19/js0
(input)
UDEV [20455.001865] add
/devices/pci0000:00/0000:00:1d.0/usb2/2-1/2-1.6/2-1.6.3/2-1.6.3:1.0/bluetooth/hci0/hci0:71/0005:045E:02FD.0013/input/input19/event17
(input)
UDEV [20455.004553] bind
/devices/pci0000:00/0000:00:1d.0/usb2/2-1/2-1.6/2-1.6.3/2-1.6.3:1.0/bluetooth/hci0/hci0:71/0005:045E:02FD.0013
(hid)
UDEV [20455.005809] change
/devices/pci0000:00/0000:00:1d.0/usb2/2-1/2-1.6/2-1.6.3/2-1.6.3:1.0/bluetooth/hci0/hci0:71/0005:045E:02FD.0013/power_supply/xpadneo_batt_3a:66:33:3a:38:63_0
(power_supply)
KERNEL[20455.739024] change
/devices/pci0000:00/0000:00:1d.0/usb2/2-1/2-1.6/2-1.6.3/2-1.6.3:1.0/bluetooth/hci0/hci0:71/0005:045E:02FD.0013/power_supply/xpadneo_batt_3a:66:33:3a:38:63_0
(power_supply)
UDEV [20455.740310] change
/devices/pci0000:00/0000:00:1d.0/usb2/2-1/2-1.6/2-1.6.3/2-1.6.3:1.0/bluetooth/hci0/hci0:71/0005:045E:02FD.0013/power_supply/xpadneo_batt_3a:66:33:3a:38:63_0
(power_supply)
"xpadneo" uses a trick to work around libSDL wrongly remapping its
buttons and controllers by faking a different interface serial/version
number. This is needed because "xpadneo" actually exposes compatible
button and axis mapping while the current firmware of the controller
puts the controller into "Android" mode when paired via BT in Linux.
While both Steam and SDL distribute a SDL_Mapping which works around
this, it still doesn't work for games which access /dev/input/js0
directly. But the fix implemented by "xpadneo" does work correctly
here but now has to sidestep "broken" SDL and Steam behavior. But
that's all it does, it shouldn't break the axis values itself.
Actually, this fake interface serial has been an idea developed by the
author of xpadneo and me. OTOH, this means libSDL or Steam do not do
any remapping with this driver.
> Does the attached patch improve anything?
Thanks, I'll try...
[some minutes later]
No, it's still the same.
Native Linux games running through Steam: Works just right, axis work
as usual. "evtest" shows no strange jumps in values.
Windows games running through SteamPlay: Axis are broken.
The following can be observed:
Vertical axis still work normally.
Horizontal axis work normally to the left but will flip over to left
when pushed further than maybe half-way to the right. According to
"evtest" that would be somewhere when we cross 24000-26000 (on a scale
-32768..32767)...
This is really strange, it tells me that there's an underlying problem
somewhere else. Shouldn't all axis be affected? Maybe there's a buffer
overflow or overwrite somewhere?
Reverting "hid: Don't sign-extend 16-bit values" does not fix the
problem as far as I could find out (I don't know why it did for Aric).
This is another clue that the problem was introduced by another
commit. Sorry for the confusion at this point.
I'm using Proton rebased to wine-4.1, so it includes all the Proton patches.
Thanks,
Kai
> On 2/10/19 1:10 PM, Kai Krakow wrote:
> > Hello again!
> >
> > I'm not sure if this commit is the problem, with it reverted I still
> > see strange effects: Moving the left stick only half-way right still
> > works correctly, moving it further turns it mirrored into left. Have
> > there been any other changes related to this? It worked fine in 4.0,
> > and the only other related change I could find here in my system was
> > updating from libSDL2-2.0.8 to 2.0.9 but I AFAIR this version still
> > worked with 4.0 correctly.
> >
> > Thanks,
> > Kai
> >
> > Am So., 10. Feb. 2019 um 19:02 Uhr schrieb Kai Krakow <kai at kaishome.de>:
> >>
> >> Hello!
> >>
> >> This change seems to introduce odd behavior in at least some games:
> >> Gamepad axis are mirrored to one side only now. This happens in Shadow
> >> of War (found by me) and Rocket League (reported by a user). I expect
> >> other games to fail in a similar way but didn't test those yet.
> >>
> >> I guess the clue is in "_may_ report a logical range"...
> >>
> >> I'm using the xpadneo driver with an Xbox One S wireless controller,
> >> and it reports -32768..32767 according to the driver (which follows
> >> the Linux spec on joysticks for this). This used to work perfectly
> >> before this change.
> >>
> >> Regards,
> >> Kai
> >>
> >> Am Mi., 6. Feb. 2019 um 19:42 Uhr schrieb Aric Stewart <aric at codeweavers.com>:
> >>>
> >>> Signed-off-by: Aric Stewart <aric at codeweavers.com>
> >>>
> >>> On 2/5/19 1:09 PM, Zebediah Figura wrote:
> >>>> From: Zebediah Figura <zfigura at codeweavers.com>
> >>>>
> >>>> Some controllers (including, with the previous patch, any reported
> >>>> through SDL) may report a logical range of [0,65535], which takes up
> >>>> 16 bits but should not be sign-extended.
> >>>>
> >>>> Signed-off-by: Zebediah Figura <zfigura at codeweavers.com>
> >>>> ---
> >>>> dlls/hid/hidp.c | 4 ----
> >>>> 1 file changed, 4 deletions(-)
> >>>>
> >>>> diff --git a/dlls/hid/hidp.c b/dlls/hid/hidp.c
> >>>> index f9978038e3..15d827edf1 100644
> >>>> --- a/dlls/hid/hidp.c
> >>>> +++ b/dlls/hid/hidp.c
> >>>> @@ -277,8 +277,6 @@ NTSTATUS WINAPI HidP_GetScaledUsageValue(HIDP_REPORT_TYPE ReportType, USAGE Usag
> >>>> element->valueStartBit, element->bitCount, &rawValue);
> >>>> if (rc != HIDP_STATUS_SUCCESS)
> >>>> return rc;
> >>>> - if (element->caps.value.BitSize == 16)
> >>>> - rawValue = (short)rawValue;
> >>>> *UsageValue = rawValue;
> >>>> }
> >>>>
> >>>> @@ -925,8 +923,6 @@ NTSTATUS WINAPI HidP_GetData(HIDP_REPORT_TYPE ReportType, HIDP_DATA *DataList, U
> >>>> element->valueStartBit, element->bitCount, &v);
> >>>> if (rc != HIDP_STATUS_SUCCESS)
> >>>> return rc;
> >>>> - if (element->caps.value.BitSize == 16)
> >>>> - v = (short)v;
> >>>> DataList[uCount].DataIndex = element->caps.value.u.NotRange.DataIndex;
> >>>> DataList[uCount].u.RawValue = v;
> >>>> }
> >>>>
> >>>
> >>>
> >
> >
>
More information about the wine-devel
mailing list