[Native] - Implement IsXInputDevice method in C# #1926

Jklawreszuk · 2023-10-11T15:29:55Z

PR Details

Description

My PR implements IsXInputDevice method in C#

Related Issue

#1394

Todo

- Need to check for regression

IXLLEGACYIXL · 2023-10-11T15:31:59Z

sources/engine/Stride.Input/Windows/InputSourceWindowsDirectInput.cs

 using SharpDX;
 using SharpDX.DirectInput;
-using Stride.Native.DirectInput;
+using Microsoft.Win32;


would that make a problem with linux migration?

Note that this file compiles only when STRIDE_UI_WINFORMS or STRIDE_UI_WPF and are set only when StrideFramework is Windows

so propably no

manio143 · 2023-10-11T16:13:07Z

While it's amazing to see so much C++ code replaced by so little C#, I'm worried we might not be able to accept code copied from a GPL licensed project into an MIT licensed project, unless with the explicit permission of the copyright holder to re-license the code.
This is because GPL is a viral license and GPL code can only be used in GPL projects.
The workaround is LGPL, where we would reference a compiled library that is an output of a GPL project.

Jklawreszuk · 2023-10-11T16:26:51Z

@manio143 Oh my. Good point, I forgot about the fact that GNU is not a completely "free" license. I'll try to investigate my own implementation then

tebjan · 2023-10-11T16:34:48Z

You could also ask the author if he allows us to use that part of the code. The author always has full control over his work, no matter the license the code is published with.

IXLLEGACYIXL · 2023-10-12T15:45:13Z

The problem is that the GPL code doesnt work with sharpdx

Sharpdx takes the HID path from a device and somehow builds an product ID ( GUID ) from it
what the GPL code does is just splitting the HID and takes the values and compares them which will never fit to the sharpdx guid

$devices = Get-WmiObject -Query 'SELECT * FROM Win32_PNPEntity WHERE DeviceID LIKE "%VID_%&PID_%&IG_%"'
this is how you can select the xinput devices
accordign to microsoft if it has IG_ in it then its an xinput device
https://learn.microsoft.com/en-us/windows/win32/xinput/xinput-and-directinput

to convert a sharpdx guid to a hid i only found that
https://stackoverflow.com/questions/67059441/how-to-map-directinput-sharpdx-device-to-its-corresponding-hid-c
but that mentioned class is internal

one option would be to accurately query with wql to get all xdevices and comparing the device path instead of the guid

the gpl code is just doing alot of fancy things but in the end it just converts this example hid path
HID\VID_045E&PID_028E&IG_00\3&8968588&0&0000
to that
045E028E
( not sure anymore aobut the leading zeros )
and this number is far away from a sharpdx guid

this shouldnt sound negative about the gpl code, i just couldnt find an alignment between sharpdx and the guid part of the code

if you need testing i can use my playstation controllers as xinput and help with testing

archanox · 2023-10-31T11:49:15Z

I haven't checked if this code works or not, but chatgpt says you can do this,

using SharpDX.XInput;

// Create a controller object for the first player
var controller = new Controller(UserIndex.One);

// Check if the controller is connected
if (controller.IsConnected)
{
    // Get the controller capabilities
    var capabilities = controller.GetCapabilities(DeviceQueryType.Any);

    // Check if the controller is an XInput device
    if (capabilities.Type == DeviceType.Gamepad)
    {
        // Do something with the controller
        Console.WriteLine("XInput device detected");
    }
    else
    {
        // The controller is not an XInput device
        Console.WriteLine("Non-XInput device detected");
    }
}
else
{
    // No controller is connected
    Console.WriteLine("No controller detected");
}

And

using SharpDX.DirectInput;

// Create a DirectInput object
var directInput = new DirectInput();

// Create a device instance from the product GUID
var deviceInstance = directInput.GetDevice(productGuid);

// Check if the device is an XInput device
if (deviceInstance.Type == DeviceType.Gamepad)
{
    // The device is an XInput device
    Console.WriteLine("XInput device detected");
}
else
{
    // The device is not an XInput device
    Console.WriteLine("Non-XInput device detected");
}

IXLLEGACYIXL · 2023-10-31T12:05:26Z

anything can be a xinput device, even a mouse, it depends how windows tries to use it

manio143 · 2023-10-31T21:14:43Z

sources/engine/Stride.Input/Windows/InputSourceWindowsDirectInput.cs

+
+            var mySession = CimSession.Create(null, DComOptions);
+
+            IEnumerable<CimInstance> allDevices = mySession.QueryInstances(@"root\cimv2", "WQL", "SELECT * FROM Win32_PNPEntity");


I imagine this query is not super cheap and results won't be different if called more than once a frame. Consider calling the query and transforming the IEnumerable with regex once, caching the result in the Scan() method, so that IsXInputDevice can be a simpler lookup.

@Jklawreszuk - do you think your WQL optimization is enough here? The Scan() method goes in a loop over the devices and checks for each one if it's XInput - are we confident to say there usually wouldn't be many devices returned by DirectInput and thus this isn't a big perf issue? Otherwise we make a Windows call a few times in a tight loop and I imagine those are usually slower. However, at the same time Scan is called on initialization and then only upon manual request it seems.

@manio143 Yea, Very good suggestion 👍. I guess, It is unlikely that something will change under the scan() method call. I updated changes, Is this what you had in mind?

archanox · 2023-11-02T06:15:25Z

anything can be a xinput device, even a mouse, it depends how windows tries to use it

Sorry, I might have misunderstood the purpose of the function being replaced.

Btw, cheers for the thumbs down @Jklawreszuk...

Jklawreszuk · 2023-11-02T13:28:26Z

Sorry @archanox, I didn't have time to reply. I want to thank you for your help, however, both examples do not compile , because API doesn't even exist. So, that's why I gave you thumbs down, nothing personal.. 😅

sources/engine/Stride.Input/Windows/InputSourceWindowsDirectInput.cs

manio143 · 2023-11-03T10:17:33Z

sources/engine/Stride.Input/Windows/InputSourceWindowsDirectInput.cs

@@ -18,11 +20,15 @@ internal class InputSourceWindowsDirectInput : InputSourceBase
        private readonly HashSet<Guid> devicesToRemove = new HashSet<Guid>();
        private InputManager inputManager;
        private DirectInput directInput;
+        private IEnumerable<CimInstance> allDevices;
+        private Regex regex;


Consider renaming it to xInputDeviceIdRegex

manio143 · 2023-11-03T10:19:13Z

sources/engine/Stride.Input/Windows/InputSourceWindowsDirectInput.cs

+            DComSessionOptions DComOptions = new DComSessionOptions();
+            DComOptions.Impersonation = ImpersonationType.Impersonate;
+
+            var mySession = CimSession.Create(null, DComOptions);


I believe all above and including this line has to be moved to GetAllXInputDevices()

sources/engine/Stride.Input/Windows/InputSourceWindowsDirectInput.cs

manio143

Code wise it's good. If you can confirm it got tested I'll merge it.

IXLLEGACYIXL · 2023-11-09T12:56:16Z

Code wise it's good. If you can confirm it got tested I'll merge it.

i can test it with a playstation controller and emulate that it should be treated as xinput, dunnno if thats sufficient

the query works so far

manio143 · 2023-11-09T21:09:08Z

There seems to be an issue with the build about no input files to clang - if there's no more cop files, we may need to turn off the property which triggers native lib compilation https://teamcity.stride3d.net/buildConfiguration/Engine_BuildWindowsOpenGL/23137?hideProblemsFromDependencies=false&hideTestsFromDependencies=false

Jklawreszuk · 2023-11-16T16:57:40Z

@manio143 I've managed to fix building my PR by simply checking if the input (c files collection) is empty.
I personally don't have a gamepad that supports xinput so it returned false oposed to @IXLLEGACYIXL when testing on XInput device, so it seems to work 👀

IXLLEGACYIXL · 2023-11-16T17:32:21Z

@manio143 I've managed to fix building my PR by simply checking if the input (c files collection) is empty. I personally don't have a gamepad that supports xinput so it returned false oposed to @IXLLEGACYIXL when testing on XInput device, so it seems to work 👀

i tried your query and it works, but i cant tell if my xinput is even valid, as the playstation controller is emulated and my mouse too .. .i get with your query ( your test script ) these two devices... i think a real xinput device is needed which i dont have , only the emulated one so dont count on my test

the emulator converts falsely everything to xinput for reasons..so i cant test if a device is not an xinput

manio143 · 2023-11-16T21:49:05Z

Build looks good. I'll see if this works with my Xbox controller.

manio143 · 2023-11-16T22:14:46Z

Ok, so this doesn't fully work for me.
The device's productGuid is 02ff045e-0000-0000-0000-504944564944, but the deviceId received from the Cim query is "045e02ff" so it didn't recognize it correctly.
Seems the bug is in the string concatenation.
Instead of

match.Groups[1].ToString() + match.Groups[2].ToString()

should be

match.Groups[2].ToString() + match.Groups[1].ToString()

@IXLLEGACYIXL you mentioned this was working for you, can you provide what was the device Id in your case?

Also, there were duplicates in the deviceId list from Cim query - maybe we can run a .Distinct() before ToList()?

Jklawreszuk · 2023-11-16T23:31:10Z

Alright, good to hear 👍. Anyway, yes, let's filter duplicates. I hope that string concatenation is deterministic (i.e. PID + VID). Otherwise method needs to exctract PID and VID separately(??)

IXLLEGACYIXL · 2023-11-16T23:34:21Z

Ok, so this doesn't fully work for me.
The device's productGuid is 02ff045e-0000-0000-0000-504944564944, but the deviceId received from the Cim query is "045e02ff" so it didn't recognize it correctly.
Seems the bug is in the string concatenation.
Instead of
match.Groups[1].ToString() + match.Groups[2].ToString()
should be
match.Groups[2].ToString() + match.Groups[1].ToString()
@IXLLEGACYIXL you mentioned this was working for you, can you provide what was the device Id in your case?

Also, there were duplicates in the deviceId list from Cim query - maybe we can run a .Distinct() before ToList()?

ive only tested the query itself, not the entire thing

the guid is built but hidden and internal thats what ive written earlier

manio143 · 2023-11-17T08:07:53Z

I hope that string concatenation is deterministic (i.e. PID + VID)

The way your regex is written guarantees determinism here.

IXLLEGACYIXL · 2023-11-17T12:34:33Z

Ok, so this doesn't fully work for me. The device's productGuid is 02ff045e-0000-0000-0000-504944564944, but the deviceId received from the Cim query is "045e02ff" so it didn't recognize it correctly. Seems the bug is in the string concatenation. Instead of
match.Groups[1].ToString() + match.Groups[2].ToString()
should be
match.Groups[2].ToString() + match.Groups[1].ToString()
@IXLLEGACYIXL you mentioned this was working for you, can you provide what was the device Id in your case?

Also, there were duplicates in the deviceId list from Cim query - maybe we can run a .Distinct() before ToList()?

so no for me its not working and i cant try as i lose my mouse with the emulator
ive only tried the wql

IXLLEGACYIXL reviewed Oct 11, 2023

View reviewed changes

Eideren marked this pull request as draft October 11, 2023 19:27

Jklawreszuk force-pushed the xinput branch from 62014ae to 30942e8 Compare October 31, 2023 14:50

Jklawreszuk marked this pull request as ready for review October 31, 2023 14:53

Jklawreszuk changed the title ~~[WIP] - [Native] - Implement IsXInputDevice method in C#~~ [Native] - Implement IsXInputDevice method in C# Oct 31, 2023

Jklawreszuk force-pushed the xinput branch 2 times, most recently from dcaa8a5 to 08269c3 Compare October 31, 2023 15:11

manio143 reviewed Oct 31, 2023

View reviewed changes

manio143 reviewed Nov 2, 2023

View reviewed changes

sources/engine/Stride.Input/Windows/InputSourceWindowsDirectInput.cs Outdated Show resolved Hide resolved

Jklawreszuk force-pushed the xinput branch 2 times, most recently from c774cb4 to 62abf51 Compare November 2, 2023 22:15

manio143 reviewed Nov 3, 2023

View reviewed changes

Jklawreszuk force-pushed the xinput branch 3 times, most recently from f071bb4 to 37f8135 Compare November 8, 2023 16:06

manio143 approved these changes Nov 9, 2023

View reviewed changes

xen2 force-pushed the master branch from 69dcfb7 to eedbdf2 Compare November 10, 2023 11:24

Jklawreszuk changed the title ~~[Native] - Implement IsXInputDevice method in C#~~ [WIP] - [Native] - Implement IsXInputDevice method in C# Nov 11, 2023

Eideren marked this pull request as draft November 12, 2023 15:46

[Native] - Implement IsXInputDevice in C#

9144c68

Jklawreszuk added 4 commits November 15, 2023 23:47

Optimize WQL query

9b25816

Extract query to separate method

d4a14a0

Code review fixes

291c006

Build fix

945ffda

Jklawreszuk force-pushed the xinput branch from 37f8135 to 945ffda Compare November 15, 2023 23:48

Jklawreszuk changed the title ~~[WIP] - [Native] - Implement IsXInputDevice method in C#~~ [Native] - Implement IsXInputDevice method in C# Nov 15, 2023

Jklawreszuk marked this pull request as ready for review November 15, 2023 23:49

Correct guid concatenation, apply distinct for xinput devices

9e5453d

manio143 merged commit bcd1a04 into stride3d:master Nov 17, 2023

Jklawreszuk deleted the xinput branch November 17, 2023 19:05

Kryptos-FR mentioned this pull request Nov 18, 2023

Rollback changes to Stride.Native.targets and add StrideNative.cpp #2047

Merged

8 tasks


		var mySession = CimSession.Create(null, DComOptions);

		IEnumerable<CimInstance> allDevices = mySession.QueryInstances(@"root\cimv2", "WQL", "SELECT * FROM Win32_PNPEntity");

Uh oh!

[Native] - Implement IsXInputDevice method in C# #1926

[Native] - Implement IsXInputDevice method in C# #1926

Uh oh!

Conversation

Jklawreszuk commented Oct 11, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Details

Description

Related Issue

Todo

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

manio143 commented Oct 11, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Jklawreszuk commented Oct 11, 2023

Uh oh!

tebjan commented Oct 11, 2023

Uh oh!

IXLLEGACYIXL commented Oct 12, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

archanox commented Oct 31, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

IXLLEGACYIXL commented Oct 31, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Jklawreszuk Nov 3, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

archanox commented Nov 2, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Jklawreszuk commented Nov 2, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

manio143 left a comment

Choose a reason for hiding this comment

Uh oh!

IXLLEGACYIXL commented Nov 9, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

manio143 commented Nov 9, 2023

Uh oh!

Jklawreszuk commented Nov 16, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

IXLLEGACYIXL commented Nov 16, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

manio143 commented Nov 16, 2023

Uh oh!

manio143 commented Nov 16, 2023

Uh oh!

Jklawreszuk commented Nov 16, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

IXLLEGACYIXL commented Nov 16, 2023

Uh oh!

manio143 commented Nov 17, 2023

Uh oh!

Jklawreszuk commented Oct 11, 2023 •

edited

Loading

manio143 commented Oct 11, 2023 •

edited

Loading

IXLLEGACYIXL commented Oct 12, 2023 •

edited

Loading

archanox commented Oct 31, 2023 •

edited

Loading

Jklawreszuk Nov 3, 2023 •

edited

Loading

archanox commented Nov 2, 2023 •

edited

Loading

Jklawreszuk commented Nov 2, 2023 •

edited

Loading

IXLLEGACYIXL commented Nov 9, 2023 •

edited

Loading

Jklawreszuk commented Nov 16, 2023 •

edited

Loading

IXLLEGACYIXL commented Nov 16, 2023 •

edited

Loading

Jklawreszuk commented Nov 16, 2023 •

edited

Loading