[PATCH 2/2] hid: Don't sign-extend 16-bit values.
Kai Krakow
kai at kaishome.de
Sun Feb 10 20:34:07 CST 2019
Hi!
Am Mo., 11. Feb. 2019 um 03:27 Uhr schrieb Zebediah Figura
<z.figura12 at gmail.com>:
>
> Wine can pick up controllers from udev, either via hidraw or evdev, as
> well as from SDL. As of bd9e130ee, we shouldn't be using signed offsets
> anymore when reporting SDL controllers, so I assumed that your
> controller was coming through hidraw directly.
Not sure, does the log show this? I'm pretty sure it's going through
SDL because I can apply SDL controller mappings to it through
environment.
> The symptoms you describe imply that anything above 0x6000 (?) is
> somehow wrapping around to negative, though that doesn't necessarily
> make very much sense.
Indeed, I'm confused too. ;-)
> Can you provide a log with
> +hid,+hidp,+hid_report,+rawinput?
Sure, I'll do. But please wait, I really need some sleep now, this was
bothering me for a few hours now. ;-)
Just let me know: Two logs? One with patches reverted and one without?
Thanks,
Kai
> On 2/10/19 7:16 PM, Kai Krakow wrote:
> > 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