[PATCH 1/2] winebus.sys: Translate SDL controller axes to unsigned 32-bit values.

[Zebediah Figura]

From: Zebediah Figura

INSIDE parses the HID report directly and assumes that the logical
minimum value is 0.

Signed-off-by: Zebediah Figura 
---
Testing on Windows with an extensive range of controllers yielded none that
used signed offsets.

dlls/winebus.sys/bus_sdl.c | 31 ++++++++++++++++++++++---------
 1 file changed, 22 insertions(+), 9 deletions(-)

diff --git a/dlls/winebus.sys/bus_sdl.c b/dlls/winebus.sys/bus_sdl.c
index 5cf5dddaf0..55891138c8 100644
--- a/dlls/winebus.sys/bus_sdl.c
+++ b/dlls/winebus.sys/bus_sdl.c
@@ -137,10 +137,10 @@ static inline struct platform_private *impl_from_DEVICE_OBJECT(DEVICE_OBJECT *de
 }

static const BYTE REPORT_AXIS_TAIL[] = {
-    0x16, 0x00, 0x80,   /* LOGICAL_MINIMUM (-32768) */
-    0x26, 0xff, 0x7f,   /* LOGICAL_MAXIMUM (32767) */
-    0x36, 0x00, 0x80,   /* PHYSICAL_MINIMUM (-32768) */
-    0x46, 0xff, 0x7f,   /* PHYSICAL_MAXIMUM (32767) */
+    0x17, 0x00, 0x00, 0x00, 0x00,   /* LOGICAL_MINIMUM (0) */
+    0x27, 0xff, 0xff, 0x00, 0x00,   /* LOGICAL_MAXIMUM (65535) */
+    0x37, 0x00, 0x00, 0x00, 0x00,   /* PHYSICAL_MINIMUM (0) */
+    0x47, 0xff, 0xff, 0x00, 0x00,   /* PHYSICAL_MAXIMUM (65535) */
     0x75, 0x10,         /* REPORT_SIZE (16) */
     0x95, 0x00,         /* REPORT_COUNT (?) */
     0x81, 0x02,         /* INPUT (Data,Var,Abs) */
@@ -170,10 +170,10 @@ static const BYTE CONTROLLER_AXIS [] = {
     0x09, 0x31,         /* USAGE (Y) */
     0x09, 0x33,         /* USAGE (RX) */
     0x09, 0x34,         /* USAGE (RY) */
-    0x16, 0x00, 0x80,   /* LOGICAL_MINIMUM (-32768) */
-    0x26, 0xff, 0x7f,   /* LOGICAL_MAXIMUM (32767) */
-    0x36, 0x00, 0x80,   /* PHYSICAL_MINIMUM (-32768) */
-    0x46, 0xff, 0x7f,   /* PHYSICAL_MAXIMUM (32767) */
+    0x17, 0x00, 0x00, 0x00, 0x00,   /* LOGICAL_MINIMUM (0) */
+    0x27, 0xff, 0xff, 0x00, 0x00,   /* LOGICAL_MAXIMUM (65535) */
+    0x37, 0x00, 0x00, 0x00, 0x00,   /* PHYSICAL_MINIMUM (0) */
+    0x47, 0xff, 0xff, 0x00, 0x00,   /* PHYSICAL_MAXIMUM (65535) */
     0x75, 0x10,         /* REPORT_SIZE (16) */
     0x95, 0x04,         /* REPORT_COUNT (4) */
     0x81, 0x02,         /* INPUT (Data,Var,Abs) */
@@ -245,7 +245,20 @@ static void set_axis_value(struct platform_private *ext, int index, short value)
 {
     int offset;
     offset = ext->axis_start + index * 2;
-    *((WORD*)&ext->report_buffer[offset]) = LE_WORD(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_TRIGGERLEFT:
+    case SDL_CONTROLLER_AXIS_TRIGGERRIGHT:
+        *((WORD*)&ext->report_buffer[offset]) = LE_WORD(value);
+        break;
+    }
 }

static void set_ball_value(struct platform_private *ext, int index, int value1, int value2)

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

[Zebediah Figura]

From: Zebediah Figura

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 
---
 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;
             }

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

[Aric Stewart]

Signed-off-by: Aric Stewart

On 2/5/19 1:09 PM, Zebediah Figura wrote:
> From: Zebediah Figura
>
> 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
>
> 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;
>         }

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

[Kai Krakow]

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 :
> Signed-off-by: Aric Stewart
>
> On 2/5/19 1:09 PM, Zebediah Figura wrote:
> > From: Zebediah Figura
> >
> > 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
> >
> > 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;
> >         }

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

[Kai Krakow]

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 :
> 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 :
> > Signed-off-by: Aric Stewart
> >
> > On 2/5/19 1:09 PM, Zebediah Figura wrote:
> > > From: Zebediah Figura
> > >
> > > 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
> > >
> > > 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;
> > >         }

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

[Aric Stewart]

I am noticing this as well with one of my games and a ps4 pad. I have brought it up to Zeb but have not had a chance to dig.

And my mirror problem did go away with this patch reverted.

-aric
> On Feb 10, 2019, at 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 :
> >
> > 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 :
> > >
> > > Signed-off-by: Aric Stewart
> > > > On 2/5/19 1:09 PM, Zebediah Figura wrote:
> > > > From: Zebediah Figura
> > > >
> > > > 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
> > > >
> > > > 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;
> > > >        }

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

[Kai Krakow]

FWIW, this only shows on the horizontal axis, not the vertical. This makes
it even stranger...

Am So., 10. Feb. 2019, 22:04 hat Aric Stewart 
geschrieben:
> I am noticing this as well with one of my games and a ps4 pad. I have
> brought it up to Zeb but have not had a chance to dig.
>
> And my mirror problem did go away with this patch reverted.
>
> -aric
> > On Feb 10, 2019, at 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 <[e-mail]
> >
> > :
> > > 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 <
>
> [e-mail]>:
> > > > Signed-off-by: Aric Stewart
> > > > > On 2/5/19 1:09 PM, Zebediah Figura wrote:
> > > > > From: Zebediah Figura
> > > > >
> > > > > 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
> > > > >
> > > > > 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;
> > > > >          }

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

[Zebediah Figura]

Is the controller being reported through udev, then?

Does the attached patch improve anything?

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 :
> > 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 :
> > > Signed-off-by: Aric Stewart
> > >
> > > On 2/5/19 1:09 PM, Zebediah Figura wrote:
> > > > From: Zebediah Figura
> > > >
> > > > 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
> > > >
> > > > 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;
> > > >          }

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

[Kai Krakow]

Hey Zeb!

Am So., 10. Feb. 2019 um 22:57 Uhr schrieb Zebediah Figura
:
> 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 :
> > > 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 :
> > > > Signed-off-by: Aric Stewart
> > > >
> > > > On 2/5/19 1:09 PM, Zebediah Figura wrote:
> > > > > From: Zebediah Figura
> > > > >
> > > > > 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
> > > > >
> > > > > 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;
> > > > >          }

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

[Zebediah Figura]

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.

The symptoms you describe imply that anything above 0x6000 (?) is 
somehow wrapping around to negative, though that doesn't necessarily 
make very much sense. Can you provide a log with 
+hid,+hidp,+hid_report,+rawinput?

On 2/10/19 7:16 PM, Kai Krakow wrote:
> Hey Zeb!
>
> Am So., 10. Feb. 2019 um 22:57 Uhr schrieb Zebediah Figura
> :
> > 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 :
> > > > 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 :
> > > > > Signed-off-by: Aric Stewart
> > > > >
> > > > > On 2/5/19 1:09 PM, Zebediah Figura wrote:
> > > > > > From: Zebediah Figura
> > > > > >
> > > > > > 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
> > > > > > 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;
> > > > > >           }

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

[Kai Krakow]

Hi!

Am Mo., 11. Feb. 2019 um 03:27 Uhr schrieb Zebediah Figura
:
> 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
> > :
> > > 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 :
> > > > > 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 :
> > > > > > Signed-off-by: Aric Stewart
> > > > > >
> > > > > > On 2/5/19 1:09 PM, Zebediah Figura wrote:
> > > > > > > From: Zebediah Figura 
> > > > > > >
> > > > > > > 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 
> > > > > > > ---
> > > > > > >     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;
> > > > > > >                 }
> > > > > > >

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

[Zebediah Figura]

On 2/10/19 8:34 PM, Kai Krakow wrote:
> Hi!
>
> Am Mo., 11. Feb. 2019 um 03:27 Uhr schrieb Zebediah Figura
> :
> > 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.
> Thanks,
> Kai

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

[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

After starting Shadow of the Tomb Raider and at the main menu, I
slowly moved the right stick to the right, then to the left. I
repeated this for the left stick. Then exited the game with dpad and A
button.

Thanks,
Kai

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

[Kai Krakow]

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

[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

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

[Zebediah Figura]

Okay, through playing around with hid and xinput tests I've managed to 
determine the root cause, which is that xinput's scaling logic is kind 
of broken. In particular it scales X value coordinates to the range 
[-32767, 32768], which breaks when the controller is held all the way to 
the right (which often is not physically "all the way to the right") 
because 32768 wraps around to negative. The reason this worked before is 
because of some quirks involving signed multiply. I've sent some patches 
which should address this.

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

[Kai Krakow]

Yep, thanks Zeb. It works properly now.

Am Di., 12. Feb. 2019 um 07:06 Uhr schrieb Zebediah Figura
:
> Okay, through playing around with hid and xinput tests I've managed to
> determine the root cause, which is that xinput's scaling logic is kind
> of broken. In particular it scales X value coordinates to the range
> [-32767, 32768], which breaks when the controller is held all the way to
> the right (which often is not physically "all the way to the right")
> because 32768 wraps around to negative. The reason this worked before is
> because of some quirks involving signed multiply. I've sent some patches
> which should address this.

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

[Roderick Colenbrander]

Hi,

I haven't followed the discussion too closely, nor the exact details
of Wine's implementation. I do work a lot on Linux kernel, evdev and
HID. To me the report below is strange. In particular the logical
minimum and maximum don't look right. It should reflect what is
reported over evdev, so for your device -32768 to +32767.

The HID spec is a bit weird about signed/unsigned int for the input
reports. The interpretation depends on logical minimum / maximum. For
reference see section 5.8
https://www.usb.org/sites/default/files/documents/hid1_11.pdf.

I would think Wine's handling should follow the same guidelines. We
shouldn't do any interpretation of the data until we really consume it
I guess. I'm not too sure about how this part of Wine currently works.

Thanks,
Roderick

On Mon, Feb 11, 2019 at 2:17 PM 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

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

[Kai Krakow]

Hey Zeb!

Reverting also "winebus.sys: Translate SDL controller axes to unsigned
32-bit values" gets rid of the problem. I think this is what Aric did:
He reverted both patches (I only realized lately that this patch was
made from two commits).

So the problem seems to be in the first commit.

Looking at the code, it probably works the same for X and Y axis. But
still the problem only shows for the X axis. I don't quite understand
the code and how BYTE_REPORT_TAIL[] works but:

1. Isn't the switch statement missing a default case? Previously the
function did set any index. Now it doesn't.

2. Is it possible that bytes are overwritten? Y usually becomes
written after X and could overwrite some index of X.
(sidenote: I was seeing some random jumps while moving the sticks more
or less random but I couldn't reproduce it later)

3. Could LE_WORD(value)+32768 have some problem with the signed-bit?
(but then it should affect both X and Y)

4. Is it safe to add 32768 to LE_WORD() (which is probably 16-bit),
plus "value" is of type short
(but I'm always confused by C types and its sizes/signs, it's probably
signed 16-bit on Linux64)

Thanks,
Kai

Am Mo., 11. Feb. 2019 um 02:16 Uhr schrieb Kai Krakow :
> Hey Zeb!
>
> Am So., 10. Feb. 2019 um 22:57 Uhr schrieb Zebediah Figura
> :
> > 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 :
> > > > 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 :
> > > > > Signed-off-by: Aric Stewart
> > > > >
> > > > > On 2/5/19 1:09 PM, Zebediah Figura wrote:
> > > > > > From: Zebediah Figura
> > > > > >
> > > > > > 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
> > > > > >
> > > > > > 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;
> > > > > >          }