[PATCH 1/5] hid: Correctly sign-extend all values in HidP_GetScaledUsageValue().

[Zebediah Figura]

Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com>
---
 dlls/hid/hidp.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/dlls/hid/hidp.c b/dlls/hid/hidp.c
index 15d827edf1..ac5f22203d 100644
--- a/dlls/hid/hidp.c
+++ b/dlls/hid/hidp.c
@@ -259,6 +259,19 @@ static NTSTATUS find_value(HIDP_REPORT_TYPE ReportType, USAGE UsagePage, USHORT
     return HIDP_STATUS_USAGE_NOT_FOUND;
 }
 
+static LONG sign_extend(ULONG value, const WINE_HID_ELEMENT *element)
+{
+    UINT bit_count = element->bitCount;
+
+    if ((value & (1 << (bit_count - 1)))
+            && element->ElementType == ValueElement
+            && element->caps.value.LogicalMin < 0)
+    {
+        value -= (1 << bit_count);
+    }
+    return value;
+}
+
 NTSTATUS WINAPI HidP_GetScaledUsageValue(HIDP_REPORT_TYPE ReportType, USAGE UsagePage,
                                          USHORT LinkCollection, USAGE Usage, PLONG UsageValue,
                                          PHIDP_PREPARSED_DATA PreparsedData, PCHAR Report, ULONG ReportLength)
@@ -277,7 +290,7 @@ NTSTATUS WINAPI HidP_GetScaledUsageValue(HIDP_REPORT_TYPE ReportType, USAGE Usag
                              element->valueStartBit, element->bitCount, &rawValue);
         if (rc != HIDP_STATUS_SUCCESS)
             return rc;
-        *UsageValue = rawValue;
+        *UsageValue = sign_extend(rawValue, element);
     }
 
     return rc;
-- 
2.20.1

[PATCH 2/5] hid: Scale from logical to physical coördinates in HidP_GetScaledUsageValue().

[Zebediah Figura]

This is not related to the recent regression.

Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com>
---
 dlls/hid/hidp.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/dlls/hid/hidp.c b/dlls/hid/hidp.c
index ac5f22203d..7c58426179 100644
--- a/dlls/hid/hidp.c
+++ b/dlls/hid/hidp.c
@@ -272,6 +272,18 @@ static LONG sign_extend(ULONG value, const WINE_HID_ELEMENT *element)
     return value;
 }
 
+static LONG logical_to_physical(LONG value, const WINE_HID_ELEMENT *element)
+{
+    if (element->caps.value.PhysicalMin || element->caps.value.PhysicalMax)
+    {
+        value = (((ULONGLONG)(value - element->caps.value.LogicalMin)
+                * (element->caps.value.PhysicalMax - element->caps.value.PhysicalMin))
+                / (element->caps.value.LogicalMax - element->caps.value.LogicalMin))
+                + element->caps.value.PhysicalMin;
+    }
+    return value;
+}
+
 NTSTATUS WINAPI HidP_GetScaledUsageValue(HIDP_REPORT_TYPE ReportType, USAGE UsagePage,
                                          USHORT LinkCollection, USAGE Usage, PLONG UsageValue,
                                          PHIDP_PREPARSED_DATA PreparsedData, PCHAR Report, ULONG ReportLength)
@@ -290,7 +302,7 @@ NTSTATUS WINAPI HidP_GetScaledUsageValue(HIDP_REPORT_TYPE ReportType, USAGE Usag
                              element->valueStartBit, element->bitCount, &rawValue);
         if (rc != HIDP_STATUS_SUCCESS)
             return rc;
-        *UsageValue = sign_extend(rawValue, element);
+        *UsageValue = logical_to_physical(sign_extend(rawValue, element), element);
     }
 
     return rc;
-- 
2.20.1

[PATCH 2/5] hid: Scale from logical to physical coördinates in HidP_GetScaledUsageValue().

[Aric Stewart]

Signed-off-by: Aric Stewart <aric(a)codeweavers.com>

On 2/12/19 12:06 AM, Zebediah Figura wrote:
> This is not related to the recent regression.
>
> Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com>
> ---
>   dlls/hid/hidp.c | 14 +++++++++++++-
>   1 file changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/dlls/hid/hidp.c b/dlls/hid/hidp.c
> index ac5f22203d..7c58426179 100644
> --- a/dlls/hid/hidp.c
> +++ b/dlls/hid/hidp.c
> @@ -272,6 +272,18 @@ static LONG sign_extend(ULONG value, const WINE_HID_ELEMENT *element)
>       return value;
>   }
>
> +static LONG logical_to_physical(LONG value, const WINE_HID_ELEMENT *element)
> +{
> +    if (element->caps.value.PhysicalMin || element->caps.value.PhysicalMax)
> +    {
> +        value = (((ULONGLONG)(value - element->caps.value.LogicalMin)
> +                * (element->caps.value.PhysicalMax - element->caps.value.PhysicalMin))
> +                / (element->caps.value.LogicalMax - element->caps.value.LogicalMin))
> +                + element->caps.value.PhysicalMin;
> +    }
> +    return value;
> +}
> +
>   NTSTATUS WINAPI HidP_GetScaledUsageValue(HIDP_REPORT_TYPE ReportType, USAGE UsagePage,
>                                            USHORT LinkCollection, USAGE Usage, PLONG UsageValue,
>                                            PHIDP_PREPARSED_DATA PreparsedData, PCHAR Report, ULONG ReportLength)
> @@ -290,7 +302,7 @@ NTSTATUS WINAPI HidP_GetScaledUsageValue(HIDP_REPORT_TYPE ReportType, USAGE Usag
>                                element->valueStartBit, element->bitCount, &rawValue);
>           if (rc != HIDP_STATUS_SUCCESS)
>               return rc;
> -        *UsageValue = sign_extend(rawValue, element);
> +        *UsageValue = logical_to_physical(sign_extend(rawValue, element), element);
>       }
>
> return rc;

[PATCH 3/5] xinput: Use HidP_GetScaledUsageValue().

[Zebediah Figura]

Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com>
---
 dlls/xinput1_3/hid.c | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/dlls/xinput1_3/hid.c b/dlls/xinput1_3/hid.c
index d7e22483d4..f276fe3229 100644
--- a/dlls/xinput1_3/hid.c
+++ b/dlls/xinput1_3/hid.c
@@ -338,9 +338,8 @@ void HID_destroy_gamepads(xinput_controller *devices)
     LeaveCriticalSection(&hid_xinput_crit);
 }
 
-#define SIGN(v,b) ((b==8)?(BYTE)v:(b==16)?(SHORT)v:(INT)v)
-#define SCALE_SHORT(v,r) (SHORT)((((0xffff)*(SIGN(v,r[1]) - r[0]))/r[2])-32767)
-#define SCALE_BYTE(v,r) (BYTE)((((0xff)*(SIGN(v,r[1]) - r[0]))/r[2]))
+#define SCALE_SHORT(v,r) (SHORT)((((0xffff)*(v - r[0]))/r[2])-32767)
+#define SCALE_BYTE(v,r) (BYTE)((((0xff)*(v - r[0]))/r[2]))
 
 void HID_update_state(xinput_controller* device)
 {
@@ -351,7 +350,7 @@ void HID_update_state(xinput_controller* device)
 
     USAGE buttons[15];
     ULONG button_length;
-    ULONG value;
+    LONG value;
 
     if (!private->enabled)
         return;
@@ -402,22 +401,22 @@ void HID_update_state(xinput_controller* device)
         }
     }
 
-    HidP_GetUsageValue(HidP_Input, HID_USAGE_PAGE_GENERIC, 0, HID_USAGE_GENERIC_X, &value, private->ppd, target_report, private->report_length);
+    HidP_GetScaledUsageValue(HidP_Input, HID_USAGE_PAGE_GENERIC, 0, HID_USAGE_GENERIC_X, &value, private->ppd, target_report, private->report_length);
     device->state.Gamepad.sThumbLX = SCALE_SHORT(value, private->ThumbLXRange);
 
-    HidP_GetUsageValue(HidP_Input, HID_USAGE_PAGE_GENERIC, 0, HID_USAGE_GENERIC_Y, &value, private->ppd, target_report, private->report_length);
+    HidP_GetScaledUsageValue(HidP_Input, HID_USAGE_PAGE_GENERIC, 0, HID_USAGE_GENERIC_Y, &value, private->ppd, target_report, private->report_length);
     device->state.Gamepad.sThumbLY = -SCALE_SHORT(value, private->ThumbLYRange);
 
-    HidP_GetUsageValue(HidP_Input, HID_USAGE_PAGE_GENERIC, 0, HID_USAGE_GENERIC_RX, &value, private->ppd, target_report, private->report_length);
+    HidP_GetScaledUsageValue(HidP_Input, HID_USAGE_PAGE_GENERIC, 0, HID_USAGE_GENERIC_RX, &value, private->ppd, target_report, private->report_length);
     device->state.Gamepad.sThumbRX = SCALE_SHORT(value, private->ThumbRXRange);
 
-    HidP_GetUsageValue(HidP_Input, HID_USAGE_PAGE_GENERIC, 0, HID_USAGE_GENERIC_RY, &value, private->ppd, target_report, private->report_length);
+    HidP_GetScaledUsageValue(HidP_Input, HID_USAGE_PAGE_GENERIC, 0, HID_USAGE_GENERIC_RY, &value, private->ppd, target_report, private->report_length);
     device->state.Gamepad.sThumbRY = -SCALE_SHORT(value, private->ThumbRYRange);
 
-    HidP_GetUsageValue(HidP_Input, HID_USAGE_PAGE_GENERIC, 0, HID_USAGE_GENERIC_RZ, &value, private->ppd, target_report, private->report_length);
+    HidP_GetScaledUsageValue(HidP_Input, HID_USAGE_PAGE_GENERIC, 0, HID_USAGE_GENERIC_RZ, &value, private->ppd, target_report, private->report_length);
     device->state.Gamepad.bRightTrigger = SCALE_BYTE(value, private->RightTriggerRange);
 
-    HidP_GetUsageValue(HidP_Input, HID_USAGE_PAGE_GENERIC, 0, HID_USAGE_GENERIC_Z, &value, private->ppd, target_report, private->report_length);
+    HidP_GetScaledUsageValue(HidP_Input, HID_USAGE_PAGE_GENERIC, 0, HID_USAGE_GENERIC_Z, &value, private->ppd, target_report, private->report_length);
     device->state.Gamepad.bLeftTrigger = SCALE_BYTE(value, private->LeftTriggerRange);
     LeaveCriticalSection(&private->crit);
 }
-- 
2.20.1

[PATCH 3/5] xinput: Use HidP_GetScaledUsageValue().

[Aric Stewart]

Signed-off-by: Aric Stewart <aric(a)codeweavers.com>

On 2/12/19 12:06 AM, Zebediah Figura wrote:
> Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com>
> ---
>   dlls/xinput1_3/hid.c | 19 +++++++++----------
>   1 file changed, 9 insertions(+), 10 deletions(-)
>
> diff --git a/dlls/xinput1_3/hid.c b/dlls/xinput1_3/hid.c
> index d7e22483d4..f276fe3229 100644
> --- a/dlls/xinput1_3/hid.c
> +++ b/dlls/xinput1_3/hid.c
> @@ -338,9 +338,8 @@ void HID_destroy_gamepads(xinput_controller *devices)
>       LeaveCriticalSection(&hid_xinput_crit);
>   }
>
> -#define SIGN(v,b) ((b==8)?(BYTE)v:(b==16)?(SHORT)v:(INT)v)
> -#define SCALE_SHORT(v,r) (SHORT)((((0xffff)*(SIGN(v,r[1]) - r[0]))/r[2])-32767)
> -#define SCALE_BYTE(v,r) (BYTE)((((0xff)*(SIGN(v,r[1]) - r[0]))/r[2]))
> +#define SCALE_SHORT(v,r) (SHORT)((((0xffff)*(v - r[0]))/r[2])-32767)
> +#define SCALE_BYTE(v,r) (BYTE)((((0xff)*(v - r[0]))/r[2]))
>
> void HID_update_state(xinput_controller* device)
>   {
> @@ -351,7 +350,7 @@ void HID_update_state(xinput_controller* device)
>
> USAGE buttons[15];
>       ULONG button_length;
> -    ULONG value;
> +    LONG value;
>
> if (!private->enabled)
>           return;
> @@ -402,22 +401,22 @@ void HID_update_state(xinput_controller* device)
>           }
>       }
>
> -    HidP_GetUsageValue(HidP_Input, HID_USAGE_PAGE_GENERIC, 0, HID_USAGE_GENERIC_X, &value, private->ppd, target_report, private->report_length);
> +    HidP_GetScaledUsageValue(HidP_Input, HID_USAGE_PAGE_GENERIC, 0, HID_USAGE_GENERIC_X, &value, private->ppd, target_report, private->report_length);
>       device->state.Gamepad.sThumbLX = SCALE_SHORT(value, private->ThumbLXRange);
>
> -    HidP_GetUsageValue(HidP_Input, HID_USAGE_PAGE_GENERIC, 0, HID_USAGE_GENERIC_Y, &value, private->ppd, target_report, private->report_length);
> +    HidP_GetScaledUsageValue(HidP_Input, HID_USAGE_PAGE_GENERIC, 0, HID_USAGE_GENERIC_Y, &value, private->ppd, target_report, private->report_length);
>       device->state.Gamepad.sThumbLY = -SCALE_SHORT(value, private->ThumbLYRange);
>
> -    HidP_GetUsageValue(HidP_Input, HID_USAGE_PAGE_GENERIC, 0, HID_USAGE_GENERIC_RX, &value, private->ppd, target_report, private->report_length);
> +    HidP_GetScaledUsageValue(HidP_Input, HID_USAGE_PAGE_GENERIC, 0, HID_USAGE_GENERIC_RX, &value, private->ppd, target_report, private->report_length);
>       device->state.Gamepad.sThumbRX = SCALE_SHORT(value, private->ThumbRXRange);
>
> -    HidP_GetUsageValue(HidP_Input, HID_USAGE_PAGE_GENERIC, 0, HID_USAGE_GENERIC_RY, &value, private->ppd, target_report, private->report_length);
> +    HidP_GetScaledUsageValue(HidP_Input, HID_USAGE_PAGE_GENERIC, 0, HID_USAGE_GENERIC_RY, &value, private->ppd, target_report, private->report_length);
>       device->state.Gamepad.sThumbRY = -SCALE_SHORT(value, private->ThumbRYRange);
>
> -    HidP_GetUsageValue(HidP_Input, HID_USAGE_PAGE_GENERIC, 0, HID_USAGE_GENERIC_RZ, &value, private->ppd, target_report, private->report_length);
> +    HidP_GetScaledUsageValue(HidP_Input, HID_USAGE_PAGE_GENERIC, 0, HID_USAGE_GENERIC_RZ, &value, private->ppd, target_report, private->report_length);
>       device->state.Gamepad.bRightTrigger = SCALE_BYTE(value, private->RightTriggerRange);
>
> -    HidP_GetUsageValue(HidP_Input, HID_USAGE_PAGE_GENERIC, 0, HID_USAGE_GENERIC_Z, &value, private->ppd, target_report, private->report_length);
> +    HidP_GetScaledUsageValue(HidP_Input, HID_USAGE_PAGE_GENERIC, 0, HID_USAGE_GENERIC_Z, &value, private->ppd, target_report, private->report_length);
>       device->state.Gamepad.bLeftTrigger = SCALE_BYTE(value, private->LeftTriggerRange);
>       LeaveCriticalSection(&private->crit);
>   }

[PATCH 4/5] xinput: Use a struct instead of an array to store axis information.

[Zebediah Figura]

Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com>
---
 dlls/xinput1_3/hid.c | 80 +++++++++++++++++---------------------------
 1 file changed, 30 insertions(+), 50 deletions(-)

diff --git a/dlls/xinput1_3/hid.c b/dlls/xinput1_3/hid.c
index f276fe3229..7553aaba63 100644
--- a/dlls/xinput1_3/hid.c
+++ b/dlls/xinput1_3/hid.c
@@ -59,6 +59,13 @@ static CRITICAL_SECTION_DEBUG hid_critsect_debug =
 };
 static CRITICAL_SECTION hid_xinput_crit = { &hid_critsect_debug, -1, 0, 0, 0, 0 };
 
+struct axis_info
+{
+    LONG min;
+    LONG range;
+    USHORT bits;
+};
+
 struct hid_platform_private {
     PHIDP_PREPARSED_DATA ppd;
     HANDLE device;
@@ -71,50 +78,23 @@ struct hid_platform_private {
     BYTE current_report;
     CHAR *reports[2];
 
-    LONG ThumbLXRange[3];
-    LONG ThumbLYRange[3];
-    LONG LeftTriggerRange[3];
-    LONG ThumbRXRange[3];
-    LONG ThumbRYRange[3];
-    LONG RightTriggerRange[3];
+    struct axis_info lx, ly, ltrigger, rx, ry, rtrigger;
 };
 
 static DWORD last_check = 0;
 
 static void MarkUsage(struct hid_platform_private *private, WORD usage, LONG min, LONG max, USHORT bits)
 {
+    struct axis_info info = {min, max-min, bits};
+
     switch (usage)
     {
-        case HID_USAGE_GENERIC_X:
-            private->ThumbLXRange[0] = min;
-            private->ThumbLXRange[1] = bits;
-            private->ThumbLXRange[2] = max - min;
-        break;
-        case HID_USAGE_GENERIC_Y:
-            private->ThumbLYRange[0] = min;
-            private->ThumbLYRange[1] = bits;
-            private->ThumbLYRange[2] = max - min;
-        break;
-        case HID_USAGE_GENERIC_Z:
-            private->LeftTriggerRange[0] = min;
-            private->LeftTriggerRange[1] = bits;
-            private->LeftTriggerRange[2] = max - min;
-        break;
-        case HID_USAGE_GENERIC_RX:
-            private->ThumbRXRange[0] = min;
-            private->ThumbRXRange[1] = bits;
-            private->ThumbRXRange[2] = max - min;
-        break;
-        case HID_USAGE_GENERIC_RY:
-            private->ThumbRYRange[0] = min;
-            private->ThumbRYRange[1] = bits;
-            private->ThumbRYRange[2] = max - min;
-        break;
-        case HID_USAGE_GENERIC_RZ:
-            private->RightTriggerRange[0] = min;
-            private->RightTriggerRange[1] = bits;
-            private->RightTriggerRange[2] = max - min;
-        break;
+        case HID_USAGE_GENERIC_X: private->lx = info; break;
+        case HID_USAGE_GENERIC_Y: private->ly = info; break;
+        case HID_USAGE_GENERIC_Z: private->ltrigger = info; break;
+        case HID_USAGE_GENERIC_RX: private->rx = info; break;
+        case HID_USAGE_GENERIC_RY: private->ry = info; break;
+        case HID_USAGE_GENERIC_RZ: private->rtrigger = info; break;
     }
 }
 
@@ -166,27 +146,27 @@ static BOOL VerifyGamepad(PHIDP_PREPARSED_DATA ppd, XINPUT_CAPABILITIES *xinput_
     }
     HeapFree(GetProcessHeap(), 0, value_caps);
 
-    if (private->LeftTriggerRange[1])
+    if (private->ltrigger.bits)
         xinput_caps->Gamepad.bLeftTrigger = (1u << (sizeof(xinput_caps->Gamepad.bLeftTrigger) + 1)) - 1;
     else
         WARN("Missing axis LeftTrigger\n");
-    if (private->RightTriggerRange[1])
+    if (private->rtrigger.bits)
         xinput_caps->Gamepad.bRightTrigger = (1u << (sizeof(xinput_caps->Gamepad.bRightTrigger) + 1)) - 1;
     else
         WARN("Missing axis RightTrigger\n");
-    if (private->ThumbLXRange[1])
+    if (private->lx.bits)
         xinput_caps->Gamepad.sThumbLX = (1u << (sizeof(xinput_caps->Gamepad.sThumbLX) + 1)) - 1;
     else
         WARN("Missing axis ThumbLX\n");
-    if (private->ThumbLYRange[1])
+    if (private->ly.bits)
         xinput_caps->Gamepad.sThumbLY = (1u << (sizeof(xinput_caps->Gamepad.sThumbLY) + 1)) - 1;
     else
         WARN("Missing axis ThumbLY\n");
-    if (private->ThumbRXRange[1])
+    if (private->rx.bits)
         xinput_caps->Gamepad.sThumbRX = (1u << (sizeof(xinput_caps->Gamepad.sThumbRX) + 1)) - 1;
     else
         WARN("Missing axis ThumbRX\n");
-    if (private->ThumbRYRange[1])
+    if (private->ry.bits)
         xinput_caps->Gamepad.sThumbRY = (1u << (sizeof(xinput_caps->Gamepad.sThumbRY) + 1)) - 1;
     else
         WARN("Missing axis ThumbRY\n");
@@ -338,8 +318,8 @@ void HID_destroy_gamepads(xinput_controller *devices)
     LeaveCriticalSection(&hid_xinput_crit);
 }
 
-#define SCALE_SHORT(v,r) (SHORT)((((0xffff)*(v - r[0]))/r[2])-32767)
-#define SCALE_BYTE(v,r) (BYTE)((((0xff)*(v - r[0]))/r[2]))
+#define SCALE_SHORT(v,r) (SHORT)((((0xffff)*(v - (r).min))/(r).range)-32767)
+#define SCALE_BYTE(v,r) (BYTE)((((0xff)*(v - (r).min))/(r).range))
 
 void HID_update_state(xinput_controller* device)
 {
@@ -402,22 +382,22 @@ void HID_update_state(xinput_controller* device)
     }
 
     HidP_GetScaledUsageValue(HidP_Input, HID_USAGE_PAGE_GENERIC, 0, HID_USAGE_GENERIC_X, &value, private->ppd, target_report, private->report_length);
-    device->state.Gamepad.sThumbLX = SCALE_SHORT(value, private->ThumbLXRange);
+    device->state.Gamepad.sThumbLX = SCALE_SHORT(value, private->lx);
 
     HidP_GetScaledUsageValue(HidP_Input, HID_USAGE_PAGE_GENERIC, 0, HID_USAGE_GENERIC_Y, &value, private->ppd, target_report, private->report_length);
-    device->state.Gamepad.sThumbLY = -SCALE_SHORT(value, private->ThumbLYRange);
+    device->state.Gamepad.sThumbLY = -SCALE_SHORT(value, private->ly);
 
     HidP_GetScaledUsageValue(HidP_Input, HID_USAGE_PAGE_GENERIC, 0, HID_USAGE_GENERIC_RX, &value, private->ppd, target_report, private->report_length);
-    device->state.Gamepad.sThumbRX = SCALE_SHORT(value, private->ThumbRXRange);
+    device->state.Gamepad.sThumbRX = SCALE_SHORT(value, private->rx);
 
     HidP_GetScaledUsageValue(HidP_Input, HID_USAGE_PAGE_GENERIC, 0, HID_USAGE_GENERIC_RY, &value, private->ppd, target_report, private->report_length);
-    device->state.Gamepad.sThumbRY = -SCALE_SHORT(value, private->ThumbRYRange);
+    device->state.Gamepad.sThumbRY = -SCALE_SHORT(value, private->ry);
 
     HidP_GetScaledUsageValue(HidP_Input, HID_USAGE_PAGE_GENERIC, 0, HID_USAGE_GENERIC_RZ, &value, private->ppd, target_report, private->report_length);
-    device->state.Gamepad.bRightTrigger = SCALE_BYTE(value, private->RightTriggerRange);
+    device->state.Gamepad.bRightTrigger = SCALE_BYTE(value, private->rtrigger);
 
     HidP_GetScaledUsageValue(HidP_Input, HID_USAGE_PAGE_GENERIC, 0, HID_USAGE_GENERIC_Z, &value, private->ppd, target_report, private->report_length);
-    device->state.Gamepad.bLeftTrigger = SCALE_BYTE(value, private->LeftTriggerRange);
+    device->state.Gamepad.bLeftTrigger = SCALE_BYTE(value, private->ltrigger);
     LeaveCriticalSection(&private->crit);
 }
 
-- 
2.20.1

[PATCH 4/5] xinput: Use a struct instead of an array to store axis information.

[Aric Stewart]

Signed-off-by: Aric Stewart <aric(a)codeweavers.com>

On 2/12/19 12:06 AM, Zebediah Figura wrote:
> Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com>
> ---
>   dlls/xinput1_3/hid.c | 80 +++++++++++++++++---------------------------
>   1 file changed, 30 insertions(+), 50 deletions(-)
>
> diff --git a/dlls/xinput1_3/hid.c b/dlls/xinput1_3/hid.c
> index f276fe3229..7553aaba63 100644
> --- a/dlls/xinput1_3/hid.c
> +++ b/dlls/xinput1_3/hid.c
> @@ -59,6 +59,13 @@ static CRITICAL_SECTION_DEBUG hid_critsect_debug =
>   };
>   static CRITICAL_SECTION hid_xinput_crit = { &hid_critsect_debug, -1, 0, 0, 0, 0 };
>
> +struct axis_info
> +{
> +    LONG min;
> +    LONG range;
> +    USHORT bits;
> +};
> +
>   struct hid_platform_private {
>       PHIDP_PREPARSED_DATA ppd;
>       HANDLE device;
> @@ -71,50 +78,23 @@ struct hid_platform_private {
>       BYTE current_report;
>       CHAR *reports[2];
>
> -    LONG ThumbLXRange[3];
> -    LONG ThumbLYRange[3];
> -    LONG LeftTriggerRange[3];
> -    LONG ThumbRXRange[3];
> -    LONG ThumbRYRange[3];
> -    LONG RightTriggerRange[3];
> +    struct axis_info lx, ly, ltrigger, rx, ry, rtrigger;
>   };
>
> static DWORD last_check = 0;
>
> static void MarkUsage(struct hid_platform_private *private, WORD usage, LONG min, LONG max, USHORT bits)
>   {
> +    struct axis_info info = {min, max-min, bits};
> +
>       switch (usage)
>       {
> -        case HID_USAGE_GENERIC_X:
> -            private->ThumbLXRange[0] = min;
> -            private->ThumbLXRange[1] = bits;
> -            private->ThumbLXRange[2] = max - min;
> -        break;
> -        case HID_USAGE_GENERIC_Y:
> -            private->ThumbLYRange[0] = min;
> -            private->ThumbLYRange[1] = bits;
> -            private->ThumbLYRange[2] = max - min;
> -        break;
> -        case HID_USAGE_GENERIC_Z:
> -            private->LeftTriggerRange[0] = min;
> -            private->LeftTriggerRange[1] = bits;
> -            private->LeftTriggerRange[2] = max - min;
> -        break;
> -        case HID_USAGE_GENERIC_RX:
> -            private->ThumbRXRange[0] = min;
> -            private->ThumbRXRange[1] = bits;
> -            private->ThumbRXRange[2] = max - min;
> -        break;
> -        case HID_USAGE_GENERIC_RY:
> -            private->ThumbRYRange[0] = min;
> -            private->ThumbRYRange[1] = bits;
> -            private->ThumbRYRange[2] = max - min;
> -        break;
> -        case HID_USAGE_GENERIC_RZ:
> -            private->RightTriggerRange[0] = min;
> -            private->RightTriggerRange[1] = bits;
> -            private->RightTriggerRange[2] = max - min;
> -        break;
> +        case HID_USAGE_GENERIC_X: private->lx = info; break;
> +        case HID_USAGE_GENERIC_Y: private->ly = info; break;
> +        case HID_USAGE_GENERIC_Z: private->ltrigger = info; break;
> +        case HID_USAGE_GENERIC_RX: private->rx = info; break;
> +        case HID_USAGE_GENERIC_RY: private->ry = info; break;
> +        case HID_USAGE_GENERIC_RZ: private->rtrigger = info; break;
>       }
>   }
>
> @@ -166,27 +146,27 @@ static BOOL VerifyGamepad(PHIDP_PREPARSED_DATA ppd, XINPUT_CAPABILITIES *xinput_
>       }
>       HeapFree(GetProcessHeap(), 0, value_caps);
>
> -    if (private->LeftTriggerRange[1])
> +    if (private->ltrigger.bits)
>           xinput_caps->Gamepad.bLeftTrigger = (1u << (sizeof(xinput_caps->Gamepad.bLeftTrigger) + 1)) - 1;
>       else
>           WARN("Missing axis LeftTrigger\n");
> -    if (private->RightTriggerRange[1])
> +    if (private->rtrigger.bits)
>           xinput_caps->Gamepad.bRightTrigger = (1u << (sizeof(xinput_caps->Gamepad.bRightTrigger) + 1)) - 1;
>       else
>           WARN("Missing axis RightTrigger\n");
> -    if (private->ThumbLXRange[1])
> +    if (private->lx.bits)
>           xinput_caps->Gamepad.sThumbLX = (1u << (sizeof(xinput_caps->Gamepad.sThumbLX) + 1)) - 1;
>       else
>           WARN("Missing axis ThumbLX\n");
> -    if (private->ThumbLYRange[1])
> +    if (private->ly.bits)
>           xinput_caps->Gamepad.sThumbLY = (1u << (sizeof(xinput_caps->Gamepad.sThumbLY) + 1)) - 1;
>       else
>           WARN("Missing axis ThumbLY\n");
> -    if (private->ThumbRXRange[1])
> +    if (private->rx.bits)
>           xinput_caps->Gamepad.sThumbRX = (1u << (sizeof(xinput_caps->Gamepad.sThumbRX) + 1)) - 1;
>       else
>           WARN("Missing axis ThumbRX\n");
> -    if (private->ThumbRYRange[1])
> +    if (private->ry.bits)
>           xinput_caps->Gamepad.sThumbRY = (1u << (sizeof(xinput_caps->Gamepad.sThumbRY) + 1)) - 1;
>       else
>           WARN("Missing axis ThumbRY\n");
> @@ -338,8 +318,8 @@ void HID_destroy_gamepads(xinput_controller *devices)
>       LeaveCriticalSection(&hid_xinput_crit);
>   }
>
> -#define SCALE_SHORT(v,r) (SHORT)((((0xffff)*(v - r[0]))/r[2])-32767)
> -#define SCALE_BYTE(v,r) (BYTE)((((0xff)*(v - r[0]))/r[2]))
> +#define SCALE_SHORT(v,r) (SHORT)((((0xffff)*(v - (r).min))/(r).range)-32767)
> +#define SCALE_BYTE(v,r) (BYTE)((((0xff)*(v - (r).min))/(r).range))
>
> void HID_update_state(xinput_controller* device)
>   {
> @@ -402,22 +382,22 @@ void HID_update_state(xinput_controller* device)
>       }
>
> HidP_GetScaledUsageValue(HidP_Input, HID_USAGE_PAGE_GENERIC, 0, HID_USAGE_GENERIC_X, &value, private->ppd, target_report, private->report_length);
> -    device->state.Gamepad.sThumbLX = SCALE_SHORT(value, private->ThumbLXRange);
> +    device->state.Gamepad.sThumbLX = SCALE_SHORT(value, private->lx);
>
> HidP_GetScaledUsageValue(HidP_Input, HID_USAGE_PAGE_GENERIC, 0, HID_USAGE_GENERIC_Y, &value, private->ppd, target_report, private->report_length);
> -    device->state.Gamepad.sThumbLY = -SCALE_SHORT(value, private->ThumbLYRange);
> +    device->state.Gamepad.sThumbLY = -SCALE_SHORT(value, private->ly);
>
> HidP_GetScaledUsageValue(HidP_Input, HID_USAGE_PAGE_GENERIC, 0, HID_USAGE_GENERIC_RX, &value, private->ppd, target_report, private->report_length);
> -    device->state.Gamepad.sThumbRX = SCALE_SHORT(value, private->ThumbRXRange);
> +    device->state.Gamepad.sThumbRX = SCALE_SHORT(value, private->rx);
>
> HidP_GetScaledUsageValue(HidP_Input, HID_USAGE_PAGE_GENERIC, 0, HID_USAGE_GENERIC_RY, &value, private->ppd, target_report, private->report_length);
> -    device->state.Gamepad.sThumbRY = -SCALE_SHORT(value, private->ThumbRYRange);
> +    device->state.Gamepad.sThumbRY = -SCALE_SHORT(value, private->ry);
>
> HidP_GetScaledUsageValue(HidP_Input, HID_USAGE_PAGE_GENERIC, 0, HID_USAGE_GENERIC_RZ, &value, private->ppd, target_report, private->report_length);
> -    device->state.Gamepad.bRightTrigger = SCALE_BYTE(value, private->RightTriggerRange);
> +    device->state.Gamepad.bRightTrigger = SCALE_BYTE(value, private->rtrigger);
>
> HidP_GetScaledUsageValue(HidP_Input, HID_USAGE_PAGE_GENERIC, 0, HID_USAGE_GENERIC_Z, &value, private->ppd, target_report, private->report_length);
> -    device->state.Gamepad.bLeftTrigger = SCALE_BYTE(value, private->LeftTriggerRange);
> +    device->state.Gamepad.bLeftTrigger = SCALE_BYTE(value, private->ltrigger);
>       LeaveCriticalSection(&private->crit);
>   }

[PATCH 5/5] xinput: Fix axis scaling arithmetic.

[Zebediah Figura]

Cast difference to ULONGLONG to ensure unsigned division, and correct
off-by-one error present when inverting axes (previously hidden by signed
division bug).

Fixes a regression introduced by bd9e130ee5b569433f2c847c5b2b562f0ae35ac5.

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=46623

Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com>
---
 dlls/xinput1_3/hid.c | 23 +++++++++++++++--------
 1 file changed, 15 insertions(+), 8 deletions(-)

diff --git a/dlls/xinput1_3/hid.c b/dlls/xinput1_3/hid.c
index 7553aaba63..c0c405fbdb 100644
--- a/dlls/xinput1_3/hid.c
+++ b/dlls/xinput1_3/hid.c
@@ -318,8 +318,15 @@ void HID_destroy_gamepads(xinput_controller *devices)
     LeaveCriticalSection(&hid_xinput_crit);
 }
 
-#define SCALE_SHORT(v,r) (SHORT)((((0xffff)*(v - (r).min))/(r).range)-32767)
-#define SCALE_BYTE(v,r) (BYTE)((((0xff)*(v - (r).min))/(r).range))
+static SHORT scale_short(LONG value, const struct axis_info *axis)
+{
+    return ((((ULONGLONG)(value - axis->min)) * 0xffff) / axis->range) - 32768;
+}
+
+static BYTE scale_byte(LONG value, const struct axis_info *axis)
+{
+    return (((ULONGLONG)(value - axis->min)) * 0xff) / axis->range;
+}
 
 void HID_update_state(xinput_controller* device)
 {
@@ -382,22 +389,22 @@ void HID_update_state(xinput_controller* device)
     }
 
     HidP_GetScaledUsageValue(HidP_Input, HID_USAGE_PAGE_GENERIC, 0, HID_USAGE_GENERIC_X, &value, private->ppd, target_report, private->report_length);
-    device->state.Gamepad.sThumbLX = SCALE_SHORT(value, private->lx);
+    device->state.Gamepad.sThumbLX = scale_short(value, &private->lx);
 
     HidP_GetScaledUsageValue(HidP_Input, HID_USAGE_PAGE_GENERIC, 0, HID_USAGE_GENERIC_Y, &value, private->ppd, target_report, private->report_length);
-    device->state.Gamepad.sThumbLY = -SCALE_SHORT(value, private->ly);
+    device->state.Gamepad.sThumbLY = -scale_short(value, &private->ly) - 1;
 
     HidP_GetScaledUsageValue(HidP_Input, HID_USAGE_PAGE_GENERIC, 0, HID_USAGE_GENERIC_RX, &value, private->ppd, target_report, private->report_length);
-    device->state.Gamepad.sThumbRX = SCALE_SHORT(value, private->rx);
+    device->state.Gamepad.sThumbRX = scale_short(value, &private->rx);
 
     HidP_GetScaledUsageValue(HidP_Input, HID_USAGE_PAGE_GENERIC, 0, HID_USAGE_GENERIC_RY, &value, private->ppd, target_report, private->report_length);
-    device->state.Gamepad.sThumbRY = -SCALE_SHORT(value, private->ry);
+    device->state.Gamepad.sThumbRY = -scale_short(value, &private->ry) - 1;
 
     HidP_GetScaledUsageValue(HidP_Input, HID_USAGE_PAGE_GENERIC, 0, HID_USAGE_GENERIC_RZ, &value, private->ppd, target_report, private->report_length);
-    device->state.Gamepad.bRightTrigger = SCALE_BYTE(value, private->rtrigger);
+    device->state.Gamepad.bRightTrigger = scale_byte(value, &private->rtrigger);
 
     HidP_GetScaledUsageValue(HidP_Input, HID_USAGE_PAGE_GENERIC, 0, HID_USAGE_GENERIC_Z, &value, private->ppd, target_report, private->report_length);
-    device->state.Gamepad.bLeftTrigger = SCALE_BYTE(value, private->ltrigger);
+    device->state.Gamepad.bLeftTrigger = scale_byte(value, &private->ltrigger);
     LeaveCriticalSection(&private->crit);
 }
 
-- 
2.20.1

[PATCH 5/5] xinput: Fix axis scaling arithmetic.

[Aric Stewart]

Signed-off-by: Aric Stewart <aric(a)codeweavers.com>

On 2/12/19 12:06 AM, Zebediah Figura wrote:
> Cast difference to ULONGLONG to ensure unsigned division, and correct
> off-by-one error present when inverting axes (previously hidden by signed
> division bug).
>
> Fixes a regression introduced by bd9e130ee5b569433f2c847c5b2b562f0ae35ac5.
>
> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=46623
>
> Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com>
> ---
>   dlls/xinput1_3/hid.c | 23 +++++++++++++++--------
>   1 file changed, 15 insertions(+), 8 deletions(-)
>
> diff --git a/dlls/xinput1_3/hid.c b/dlls/xinput1_3/hid.c
> index 7553aaba63..c0c405fbdb 100644
> --- a/dlls/xinput1_3/hid.c
> +++ b/dlls/xinput1_3/hid.c
> @@ -318,8 +318,15 @@ void HID_destroy_gamepads(xinput_controller *devices)
>       LeaveCriticalSection(&hid_xinput_crit);
>   }
>
> -#define SCALE_SHORT(v,r) (SHORT)((((0xffff)*(v - (r).min))/(r).range)-32767)
> -#define SCALE_BYTE(v,r) (BYTE)((((0xff)*(v - (r).min))/(r).range))
> +static SHORT scale_short(LONG value, const struct axis_info *axis)
> +{
> +    return ((((ULONGLONG)(value - axis->min)) * 0xffff) / axis->range) - 32768;
> +}
> +
> +static BYTE scale_byte(LONG value, const struct axis_info *axis)
> +{
> +    return (((ULONGLONG)(value - axis->min)) * 0xff) / axis->range;
> +}
>
> void HID_update_state(xinput_controller* device)
>   {
> @@ -382,22 +389,22 @@ void HID_update_state(xinput_controller* device)
>       }
>
> HidP_GetScaledUsageValue(HidP_Input, HID_USAGE_PAGE_GENERIC, 0, HID_USAGE_GENERIC_X, &value, private->ppd, target_report, private->report_length);
> -    device->state.Gamepad.sThumbLX = SCALE_SHORT(value, private->lx);
> +    device->state.Gamepad.sThumbLX = scale_short(value, &private->lx);
>
> HidP_GetScaledUsageValue(HidP_Input, HID_USAGE_PAGE_GENERIC, 0, HID_USAGE_GENERIC_Y, &value, private->ppd, target_report, private->report_length);
> -    device->state.Gamepad.sThumbLY = -SCALE_SHORT(value, private->ly);
> +    device->state.Gamepad.sThumbLY = -scale_short(value, &private->ly) - 1;
>
> HidP_GetScaledUsageValue(HidP_Input, HID_USAGE_PAGE_GENERIC, 0, HID_USAGE_GENERIC_RX, &value, private->ppd, target_report, private->report_length);
> -    device->state.Gamepad.sThumbRX = SCALE_SHORT(value, private->rx);
> +    device->state.Gamepad.sThumbRX = scale_short(value, &private->rx);
>
> HidP_GetScaledUsageValue(HidP_Input, HID_USAGE_PAGE_GENERIC, 0, HID_USAGE_GENERIC_RY, &value, private->ppd, target_report, private->report_length);
> -    device->state.Gamepad.sThumbRY = -SCALE_SHORT(value, private->ry);
> +    device->state.Gamepad.sThumbRY = -scale_short(value, &private->ry) - 1;
>
> HidP_GetScaledUsageValue(HidP_Input, HID_USAGE_PAGE_GENERIC, 0, HID_USAGE_GENERIC_RZ, &value, private->ppd, target_report, private->report_length);
> -    device->state.Gamepad.bRightTrigger = SCALE_BYTE(value, private->rtrigger);
> +    device->state.Gamepad.bRightTrigger = scale_byte(value, &private->rtrigger);
>
> HidP_GetScaledUsageValue(HidP_Input, HID_USAGE_PAGE_GENERIC, 0, HID_USAGE_GENERIC_Z, &value, private->ppd, target_report, private->report_length);
> -    device->state.Gamepad.bLeftTrigger = SCALE_BYTE(value, private->ltrigger);
> +    device->state.Gamepad.bLeftTrigger = scale_byte(value, &private->ltrigger);
>       LeaveCriticalSection(&private->crit);
>   }

[Aric Stewart]

Signed-off-by: Aric Stewart <aric(a)codeweavers.com>

On 2/12/19 12:06 AM, Zebediah Figura wrote:
> Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com>
> ---
>   dlls/hid/hidp.c | 15 ++++++++++++++-
>   1 file changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/dlls/hid/hidp.c b/dlls/hid/hidp.c
> index 15d827edf1..ac5f22203d 100644
> --- a/dlls/hid/hidp.c
> +++ b/dlls/hid/hidp.c
> @@ -259,6 +259,19 @@ static NTSTATUS find_value(HIDP_REPORT_TYPE ReportType, USAGE UsagePage, USHORT
>       return HIDP_STATUS_USAGE_NOT_FOUND;
>   }
>
> +static LONG sign_extend(ULONG value, const WINE_HID_ELEMENT *element)
> +{
> +    UINT bit_count = element->bitCount;
> +
> +    if ((value & (1 << (bit_count - 1)))
> +            && element->ElementType == ValueElement
> +            && element->caps.value.LogicalMin < 0)
> +    {
> +        value -= (1 << bit_count);
> +    }
> +    return value;
> +}
> +
>   NTSTATUS WINAPI HidP_GetScaledUsageValue(HIDP_REPORT_TYPE ReportType, USAGE UsagePage,
>                                            USHORT LinkCollection, USAGE Usage, PLONG UsageValue,
>                                            PHIDP_PREPARSED_DATA PreparsedData, PCHAR Report, ULONG ReportLength)
> @@ -277,7 +290,7 @@ NTSTATUS WINAPI HidP_GetScaledUsageValue(HIDP_REPORT_TYPE ReportType, USAGE Usag
>                                element->valueStartBit, element->bitCount, &rawValue);
>           if (rc != HIDP_STATUS_SUCCESS)
>               return rc;
> -        *UsageValue = rawValue;
> +        *UsageValue = sign_extend(rawValue, element);
>       }
>
> return rc;