Re: [PATCH 2/2] hid: Don't sign-extend 16-bit values.

[Brendan McGrath]

I just ran in to this issue too when testing with the latest. Same 
symptoms for me: it only affected the x-axis (on both the left and right 
stick of my controller) and only occurred in the most right-most 
position (where the input would flip to become a left movement).

I thought it might be an overflow, so I changed set_axis_value (in 
bus_sdl.c) to add 32767 (instead of 32768). This fixed the x-axis but 
introduced the issue with the y-axis (where the up-most position become 
a down input).

So I tried the patch below and this seems to fix it for both axis. I'm 
not sure why it has this behaviour, so I'm not suggesting this as a 
solution - but as an observation which might help highlight the root cause.

diff --git a/dlls/winebus.sys/bus_sdl.c b/dlls/winebus.sys/bus_sdl.c
index 55891138c8..b2e7ec981f 100644
--- a/dlls/winebus.sys/bus_sdl.c
+++ b/dlls/winebus.sys/bus_sdl.c
@@ -248,12 +248,14 @@ static void set_axis_value(struct platform_private 
*ext, int index, short value)

switch (index)
      {
-    case SDL_CONTROLLER_AXIS_LEFTX:
      case SDL_CONTROLLER_AXIS_LEFTY:
-    case SDL_CONTROLLER_AXIS_RIGHTX:
      case SDL_CONTROLLER_AXIS_RIGHTY:
          *((WORD*)&ext->report_buffer[offset]) = LE_WORD(value) + 32768;
          break;
+    case SDL_CONTROLLER_AXIS_LEFTX:
+    case SDL_CONTROLLER_AXIS_RIGHTX:
+        *((WORD*)&ext->report_buffer[offset]) = LE_WORD(value) + 32767;
+        break;
      case SDL_CONTROLLER_AXIS_TRIGGERLEFT:
      case SDL_CONTROLLER_AXIS_TRIGGERRIGHT:
          *((WORD*)&ext->report_buffer[offset]) = LE_WORD(value);

On 12/2/19 9:16 am, Kai Krakow wrote:
> Am Mo., 11. Feb. 2019 um 22:26 Uhr schrieb Kai Krakow :
> > Hey Zeb!
> > > > > 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.
> > >
> > > Log should show this, yes.
> > > > > 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. ;-)
> > >
> > > Well, it's close to an explanation. INSIDE, which prompted these
> > > patches, showed very similar symptoms: the game assumed that the
> > > controller had a range of 0-32767, and so mapped 0-16383 to "left" and
> > > 16384-32767 to "right", and negative values were interpreted as above
> > > 16384 and therefore also "right". Thus the effect was basically the same
> > > as you describe, except inverted.
> > > > > 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?
> > >
> > > Just without any patches reverted (so with the bug visible) will
> > > hopefully be enough.
> >
> > Here's the log:
> > https://gist.github.com/a9b51a975f933482a7125792a66bf3cd
>
> This looks correct:
>
> 0022:trace:hid:debug_print_value_cap INPUT Value: 0x1/0x30: ReportId
> 0, IsAbsolute 1, HasNull 0, Bit Size 16, ReportCount 1, UnitsExp 0,
> Units 0, LogicalMin 0, Logical Max 65535, PhysicalMin 0, PhysicalMax
> 65535 -- StartBit 24/16
> 0022:trace:hid:debug_print_value_cap INPUT Value: 0x1/0x31: ReportId
> 0, IsAbsolute 1, HasNull 0, Bit Size 16, ReportCount 1, UnitsExp 0,
> Units 0, LogicalMin 0, Logical Max 65535, PhysicalMin 0, PhysicalMax
> 65535 -- StartBit 40/16
> 0022:trace:hid:debug_print_value_cap INPUT Value: 0x1/0x33: ReportId
> 0, IsAbsolute 1, HasNull 0, Bit Size 16, ReportCount 1, UnitsExp 0,
> Units 0, LogicalMin 0, Logical Max 65535, PhysicalMin 0, PhysicalMax
> 65535 -- StartBit 56/16
> 0022:trace:hid:debug_print_value_cap INPUT Value: 0x1/0x34: ReportId
> 0, IsAbsolute 1, HasNull 0, Bit Size 16, ReportCount 1, UnitsExp 0,
> Units 0, LogicalMin 0, Logical Max 65535, PhysicalMin 0, PhysicalMax
> 65535 -- StartBit 72/16
> 0022:trace:hid:debug_print_value_cap INPUT Value: 0x1/0x32: ReportId
> 0, IsAbsolute 1, HasNull 0, Bit Size 16, ReportCount 1, UnitsExp 0,
> Units 0, LogicalMin 0, Logical Max 32767, PhysicalMin 0, PhysicalMax
> 32767 -- StartBit 88/16
> 0022:trace:hid:debug_print_value_cap INPUT Value: 0x1/0x35: ReportId
> 0, IsAbsolute 1, HasNull 0, Bit Size 16, ReportCount 1, UnitsExp 0,
> Units 0, LogicalMin 0, Logical Max 32767, PhysicalMin 0, PhysicalMax
> 32767 -- StartBit 104/16
>
> Confirming with evtest:
>
> Event type 3 (EV_ABS)
>     Event code 0 (ABS_X)
>       Value      0
>       Min   -32768
>       Max    32767
>       Flat    4095
>     Event code 1 (ABS_Y)
>       Value      0
>       Min   -32768
>       Max    32767
>       Flat    4095
>     Event code 2 (ABS_Z)
>       Value      0
>       Min        0
>       Max     1023
>       Fuzz       3
>       Flat      63
>     Event code 3 (ABS_RX)
>       Value      0
>       Min   -32768
>       Max    32767
>       Fuzz     255
>       Flat    4095
>     Event code 4 (ABS_RY)
>       Value      0
>       Min   -32768
>       Max    32767
>       Fuzz     255
>       Flat    4095
>     Event code 5 (ABS_RZ)
>       Value      0
>       Min        0
>       Max     1023
>       Fuzz       3
>       Flat      63
>
> Thanks,
> Kai