FIX: Incorrect HID Stick values for LogicalMinimum with negative values (ISXB-1735)

Assets/Tests/InputSystem/Plugins/HIDTests.cs

@@ -218,27 +218,9 @@
 
         // The HID report descriptor is fetched from the device via an IOCTL.
         var deviceId = runtime.AllocateDeviceId();
-        unsafe
-        {
-            runtime.SetDeviceCommandCallback(deviceId,
-                (id, commandPtr) =>
-                {
-                    if (commandPtr->type == HID.QueryHIDReportDescriptorSizeDeviceCommandType)
-                        return reportDescriptor.Length;
 
-                    if (commandPtr->type == HID.QueryHIDReportDescriptorDeviceCommandType
-                        && commandPtr->payloadSizeInBytes >= reportDescriptor.Length)
-                    {
-                        fixed(byte* ptr = reportDescriptor)
-                        {
-                            UnsafeUtility.MemCpy(commandPtr->payloadPtr, ptr, reportDescriptor.Length);
-                            return reportDescriptor.Length;
-                        }
-                    }
+        SetDeviceCommandCallbackToReturnReportDescriptor(deviceId, reportDescriptor);
 
-                    return InputDeviceCommand.GenericFailure;
-                });
-        }
         // Report device.
         runtime.ReportNewInputDevice(
             new InputDeviceDescription
@@ -309,6 +291,130 @@
         ////TODO: check hat switch
     }
 
+    // This is used to mock out the IOCTL the HID device driver would use to return
+    // the report descriptor and its size.
+    unsafe void SetDeviceCommandCallbackToReturnReportDescriptor(int deviceId, byte[] reportDescriptor)
+    {
+        runtime.SetDeviceCommandCallback(deviceId,
+            (id, commandPtr) =>
+            {
+                if (commandPtr == null)
+                    return InputDeviceCommand.GenericFailure;
+
+                if (commandPtr->type == HID.QueryHIDReportDescriptorSizeDeviceCommandType)
+                    return reportDescriptor.Length;
+
+                if (commandPtr->type == HID.QueryHIDReportDescriptorDeviceCommandType
+                    && commandPtr->payloadSizeInBytes >= reportDescriptor.Length)
+                {
+                    fixed(byte* ptr = reportDescriptor)
+                    {
+                        UnsafeUtility.MemCpy(commandPtr->payloadPtr, ptr, reportDescriptor.Length);
+                        return reportDescriptor.Length;
+                    }
+                }
+
+                return InputDeviceCommand.GenericFailure;
+            });
+    }
+
+    [Test]
+    [Category("HID Devices")]
+    // These descriptor values were generated with the Microsoft HID Authoring descriptor tool in
+    // https://github.com/microsoft/hidtools for the expected value:
+    // Logical min 0, logical max 65535
+    [TestCase(16, new byte[] {0x16, 0x00, 0x00}, new byte[] { 0x27, 0xFF, 0xFF, 0x00, 0x00 }, 0, 65535)]
+    // Logical min -32768, logical max 32767
+    [TestCase(16, new byte[] {0x16, 0x00, 0x80}, new byte[] {0x26, 0xFF, 0x7F}, -32768, 32767)]
+    // Logical min 0, logical max 255
+    [TestCase(8, new byte[] {0x15, 00}, new byte[] {0x26, 0xFF, 0x00}, 0, 255)]
+    // Logical min -128, logical max 127
+    [TestCase(8, new byte[] {0x15, 0x80}, new byte[] {0x25, 0x7F}, -128, 127)]
+    // Logical min -16, logical max 15 (below 8 bit boundary)
+    [TestCase(5, new byte[] {0x15, 0xF0}, new byte[] {0x25, 0x0F}, -16, 15)]
+    // Logical min 0, logical max 31 (below 8 bit boundary)
+    [TestCase(5, new byte[] {0x15, 0x00}, new byte[] {0x25, 0x1F}, 0, 31)]
+    // Logical min -4096, logical max 4095 (crosses byte boundary)
+    [TestCase(13, new byte[] {0x16, 0x00, 0xF0}, new byte[] {0x26, 0xFF, 0x0F}, -4096, 4095)]
+    // Logical min 0, logical max 8191 (crosses byte boundary)
+    [TestCase(13, new byte[] {0x15, 0x00}, new byte[] {0x26, 0xFF, 0x1F}, 0, 8191)]
+    // Logical min 0, logical max 16777215 (24 bit)
+    [TestCase(24, new byte[] {0x15, 0x00}, new byte[] {0x27, 0xFF, 0xFF, 0xFF, 0x00}, 0, 16777215)]
+    // Logical min -8388608, logical max 8388607 (24 bit)
+    [TestCase(24, new byte[] {0x17, 0x00, 0x00, 0x80, 0xFF}, new byte[] {0x27, 0xFF, 0xFF, 0x7F, 0x00}, -8388608, 8388607)]
+    // Logical min -72, logical max -35
+    [TestCase(8, new byte[] {0x15, 0xB8}, new byte[] {0x25, 0xDD}, -72, -35)]
+    // Logical min 30, logical max 78
+    [TestCase(8, new byte[] {0x15, 0x1E}, new byte[] {0x25, 0x4E}, 30, 78)]
+
+    public void Devices_CanParseHIDDescriptor_WithSignedLogicalMinAndMaxValues(byte reportSizeBits, byte[] logicalMinBytes, byte[] logicalMaxBytes, int logicalMinExpected, int logicalMaxExpected)
+    {
+        // Dynamically create HID report descriptor for one X-axis with parameterized logical min/max
+        var reportDescriptorStart = new byte[]
+        {
+            0x05, 0x01,        // Usage Page (Generic Desktop Ctrls)
+            0x09, 0x05,        // Usage (Game Pad)
+            0xA1, 0x01,        // Collection (Application)
+            0x85, 0x01,        //   Report ID (1)
+            0x05, 0x01,        //   Usage Page (Generic Desktop Ctrls)
+            0x09, 0x30,        //   Usage (X)
+        };
+
+        var reportDescriptorEnd = new byte[]
+        {
+            0x75, reportSizeBits,   //   Report Size (8)
+            0x95, 0x01,             //   Report Count (1)
+            0x81, 0x02,             //   Input (Data,Var,Abs)
+            0xC0,                   //   End Collection
+        };
+
+        // Concatenate to form final descriptor based on test parameters where logical min/max bytes
+        // are inserted in the middle.
+        var reportDescriptor = reportDescriptorStart.Concat(logicalMinBytes).
+            Concat(logicalMaxBytes).
+            Concat(reportDescriptorEnd).
+            ToArray();
+
+        // The HID report descriptor is fetched from the device via an IOCTL.
+        var deviceId = runtime.AllocateDeviceId();
+
+        // Callback to return the desired report descriptor.
+        SetDeviceCommandCallbackToReturnReportDescriptor(deviceId, reportDescriptor);
+
+        // Report device.
+        runtime.ReportNewInputDevice(
+            new InputDeviceDescription
+            {
+                interfaceName = HID.kHIDInterface,
+                manufacturer = "TestLogicalMinMaxParsing",
+                product = "TestHID",
+                capabilities = new HID.HIDDeviceDescriptor
+                {
+                    vendorId = 0x321,
+                    productId = 0x432
+                }.ToJson()
+            }.ToJson(), deviceId);
+
+        InputSystem.Update();
+
+        var device = InputSystem.GetDeviceById(deviceId);
+        Assert.That(device, Is.Not.Null);
+
+        var parsedDescriptor = JsonUtility.FromJson<HID.HIDDeviceDescriptor>(device.description.capabilities);
+
+        // Check we parsed the values as expected
+        foreach (var element in parsedDescriptor.elements)
+        {
+            if (element.usage == (int)HID.GenericDesktop.X)
+            {
+                Assert.That(element.logicalMin, Is.EqualTo(logicalMinExpected));
+                Assert.That(element.logicalMax, Is.EqualTo(logicalMaxExpected));
+            }
+            else
+                Assert.Fail("Could not find X element in descriptor");
+        }
+    }
+
     [Test]
     [Category("Devices")]
     public void Devices_CanCreateGenericHID_FromDeviceWithParsedReportDescriptor()
@@ -1026,7 +1132,7 @@
     }
 
     [StructLayout(LayoutKind.Explicit)]
-    struct SimpleJoystickLayout : IInputStateTypeInfo
+    struct SimpleJoystickLayoutWithStick : IInputStateTypeInfo
     {
         [FieldOffset(0)] public byte reportId;
         [FieldOffset(1)] public ushort x;
@@ -1069,7 +1175,7 @@
         Assert.That(device, Is.TypeOf<Joystick>());
         Assert.That(device["Stick"], Is.TypeOf<StickControl>());
 
-        InputSystem.QueueStateEvent(device, new SimpleJoystickLayout { reportId = 1, x = ushort.MaxValue, y = ushort.MinValue });
+        InputSystem.QueueStateEvent(device, new SimpleJoystickLayoutWithStick { reportId = 1, x = ushort.MaxValue, y = ushort.MinValue });
         InputSystem.Update();
 
         Assert.That(device["stick"].ReadValueAsObject(),

Packages/com.unity.inputsystem/CHANGELOG.md

@@ -22,6 +22,9 @@ however, it has to be formatted properly to pass verification tests.
 - Fixed an issue in `DeltaStateEvent.From` where unsafe code would throw exception or crash if internal pointer `currentStatePtr` was `null`. [ISXB-1637](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1637).
 - Fixed an issue in `InputTestFixture.Set` where attempting to change state of a device not belonging to the test fixture context would result in null pointer exception or crash. [ISXB-1637](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1637).
 - Fixed an issue where input action parameter overrides applied via `InputActionRebindingExtensions.ApplyParameterOverride` would not be applied if the associated binding has the empty string as `InputBinding.name`and the binding mask also has the empty string as name. (ISXB-1721).
+- Fixed HID parsing not handling logical minimum and maximum values correctly when they are negative. This applies for platforms that parse HID descriptors in the package, e.g. macOS at the moment.
+- Fix usage of correct data format for stick axes in HID Layout Builder ([User contribution](https://github.com/Unity-Technologies/InputSystem/pull/2245))
+
 
 ## [1.15.0] - 2025-10-03
 

Packages/com.unity.inputsystem/InputSystem/Plugins/HID/HID.cs

@@ -385,7 +385,7 @@ public InputControlLayout Build()
                     var yElementParameters = yElement.DetermineParameters();
 
                     builder.AddControl(stickName + "/x")
-                        .WithFormat(xElement.isSigned ? InputStateBlock.FormatSBit : InputStateBlock.FormatBit)
+                        .WithFormat(xElement.DetermineFormat())
                         .WithByteOffset((uint)(xElement.reportOffsetInBits / 8 - byteOffset))
                         .WithBitOffset((uint)(xElement.reportOffsetInBits % 8))
                         .WithSizeInBits((uint)xElement.reportSizeInBits)
@@ -394,7 +394,7 @@ public InputControlLayout Build()
                         .WithProcessors(xElement.DetermineProcessors());
 
                     builder.AddControl(stickName + "/y")
-                        .WithFormat(yElement.isSigned ? InputStateBlock.FormatSBit : InputStateBlock.FormatBit)
+                        .WithFormat(yElement.DetermineFormat())
                         .WithByteOffset((uint)(yElement.reportOffsetInBits / 8 - byteOffset))
                         .WithBitOffset((uint)(yElement.reportOffsetInBits % 8))
                         .WithSizeInBits((uint)yElement.reportSizeInBits)

Packages/com.unity.inputsystem/InputSystem/Plugins/HID/HIDParser.cs

@@ -267,7 +267,7 @@ private unsafe static int ReadData(int itemSize, byte* currentPtr, byte* endPtr)
             {
                 if (currentPtr >= endPtr)
                     return 0;
-                return *currentPtr;
+                return (sbyte)*currentPtr;
             }
 
             // Read short.
@@ -277,7 +277,7 @@ private unsafe static int ReadData(int itemSize, byte* currentPtr, byte* endPtr)
                     return 0;
                 var data1 = *currentPtr;
                 var data2 = *(currentPtr + 1);
-                return (data2 << 8) | data1;
+                return (short)((data2 << 8) | data1);
             }
 
             // Read int.
@@ -291,7 +291,7 @@ private unsafe static int ReadData(int itemSize, byte* currentPtr, byte* endPtr)
                 var data3 = *(currentPtr + 2);
                 var data4 = *(currentPtr + 3);
 
-                return (data4 << 24) | (data3 << 24) | (data2 << 8) | data1;
+                return (data4 << 24) | (data3 << 16) | (data2 << 8) | data1;
             }
 
             Debug.Assert(false, "Should not reach here");