[PATCH 1/4] user32: Uppercase the RawInput device names sans the GUID.
From: Arkadiusz Hiler <ahiler(a)codeweavers.com> This is a preparation for a patch that changes setupapi to return lowercased device paths, which is the source of RawInput device name here. On Windows RawInput returns mostly uppercase names (except the GUID) and we want to keep that behavior. Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- First two patches have been in Proton for a long while. They aren't directly related to the SDL issue described in PATCH 3, and as we're already using upper case device paths it's possible to skip them. However they fix some other kind of hotplug problem, and so I think they are related and worth having. Then, because SDL checks for uppercase "IG_", we need PATCH 1 before PATCH 2. I'm a bit unsure about PATCH 4, as creating a (msg) window in DllMain is probably not great, but I don't really how else we can make sure XInput gets notified first. It would be better to avoid refreshing the device list on every call. dlls/user32/rawinput.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/dlls/user32/rawinput.c b/dlls/user32/rawinput.c index f3dc4ae67de..a774f12231b 100644 --- a/dlls/user32/rawinput.c +++ b/dlls/user32/rawinput.c @@ -101,6 +101,7 @@ static struct device *add_device(HDEVINFO set, SP_DEVICE_INTERFACE_DATA *iface) struct device *device = NULL; UINT32 handle; HANDLE file; + WCHAR *pos; DWORD i, size, type; SetupDiGetDeviceInterfaceDetailW(set, iface, NULL, 0, &size, &device_data); @@ -125,6 +126,9 @@ static struct device *add_device(HDEVINFO set, SP_DEVICE_INTERFACE_DATA *iface) detail->cbSize = sizeof(SP_DEVICE_INTERFACE_DETAIL_DATA_W); SetupDiGetDeviceInterfaceDetailW(set, iface, detail, size, NULL, NULL); + /* upper case everything but the GUID */ + for (pos = detail->DevicePath; *pos && *pos != '{'; pos++) *pos = towupper(*pos); + file = CreateFileW(detail->DevicePath, GENERIC_READ | GENERIC_WRITE, FILE_SHARE_READ | FILE_SHARE_WRITE, NULL, OPEN_EXISTING, 0, 0); if (file == INVALID_HANDLE_VALUE) -- 2.34.1
From: Arkadiusz Hiler <ahiler(a)codeweavers.com> Some games are doing case sensitive matches on the device paths obtained from Setup API and expect them to be lowercase. This fixes Virginia not being able to discover DualShock 4. Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/joy.cpl/main.c | 2 +- dlls/setupapi/devinst.c | 3 +++ dlls/setupapi/tests/devinst.c | 13 +++++++++++++ 3 files changed, 17 insertions(+), 1 deletion(-) diff --git a/dlls/joy.cpl/main.c b/dlls/joy.cpl/main.c index a1308606f98..081dcf2dc74 100644 --- a/dlls/joy.cpl/main.c +++ b/dlls/joy.cpl/main.c @@ -100,7 +100,7 @@ static BOOL CALLBACK enum_callback(const DIDEVICEINSTANCEW *instance, void *cont joystick->num_effects = 0; IDirectInputDevice8_GetProperty(joystick->device, DIPROP_GUIDANDPATH, &prop_guid_path.diph); - joystick->is_xinput = wcsstr(prop_guid_path.wszPath, L"&IG_") != NULL; + joystick->is_xinput = wcsstr(prop_guid_path.wszPath, L"&ig_") != NULL; if (joystick->forcefeedback) data->num_ff++; diff --git a/dlls/setupapi/devinst.c b/dlls/setupapi/devinst.c index 29cfbbeb87f..ecc0b58ca5c 100644 --- a/dlls/setupapi/devinst.c +++ b/dlls/setupapi/devinst.c @@ -407,6 +407,9 @@ static LPWSTR SETUPDI_CreateSymbolicLinkPath(LPCWSTR instanceId, lstrcpyW(ret + printed + 1, ReferenceString); } } + + CharLowerW(ret); + return ret; } diff --git a/dlls/setupapi/tests/devinst.c b/dlls/setupapi/tests/devinst.c index 6f02acb9589..35823788605 100644 --- a/dlls/setupapi/tests/devinst.c +++ b/dlls/setupapi/tests/devinst.c @@ -1025,6 +1025,18 @@ static void test_register_device_info(void) SetupDiDestroyDeviceInfoList(set); } +static void check_all_lower_case(int line, const char* str) +{ + const char *cur; + + for (cur = str; *cur; cur++) + { + BOOL is_lower = (tolower(*cur) == *cur); + ok_(__FILE__, line)(is_lower, "Expected device path to be all lowercase but got %s.\n", str); + if (!is_lower) break; + } +} + static void check_device_iface_(int line, HDEVINFO set, SP_DEVINFO_DATA *device, const GUID *class, int index, DWORD flags, const char *path) { @@ -1046,6 +1058,7 @@ static void check_device_iface_(int line, HDEVINFO set, SP_DEVINFO_DATA *device, ret = SetupDiGetDeviceInterfaceDetailA(set, &iface, detail, sizeof(buffer), NULL, NULL); ok_(__FILE__, line)(ret, "Failed to get interface detail, error %#x.\n", GetLastError()); ok_(__FILE__, line)(!strcasecmp(detail->DevicePath, path), "Got unexpected path %s.\n", detail->DevicePath); + check_all_lower_case(line, detail->DevicePath); } else { -- 2.34.1
The SDL library calls GetRawInputDeviceList quickly after it has detected a new device, from a WM_DEVICECHANGE notification. It uses rawinput device information to find out whether it is an XInput device, when its name contains "IG_", or if the device should be opened through DInput. If the device is missing from the rawinput list, it considers it as a DInput device, and may end up with the same device used twice. Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/user32/input.c | 1 + 1 file changed, 1 insertion(+) diff --git a/dlls/user32/input.c b/dlls/user32/input.c index 08a00c0e647..7134ab0fbb2 100644 --- a/dlls/user32/input.c +++ b/dlls/user32/input.c @@ -1156,6 +1156,7 @@ BOOL WINAPI EnableMouseInPointer(BOOL enable) static DWORD CALLBACK devnotify_window_callback(HANDLE handle, DWORD flags, DEV_BROADCAST_HDR *header) { + rawinput_update_device_list(); SendMessageTimeoutW(handle, WM_DEVICECHANGE, flags, (LPARAM)header, SMTO_ABORTIFHUNG, 2000, NULL); return 0; } -- 2.34.1
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=104433 Your paranoid android. === debian11 (32 bit Chinese:China report) === user32: win.c:10433: Test failed: Expected foreground window 0, got 00F2005A win.c:10439: Test failed: Expected foreground window 00550126, got 00F2005A
Instead of doing it lazily, which is more likely to be after the game has registered theirs. As the message handling order is the same as the registration, XInput may end updating its device list only too late, every time. Some games register device notifications and call XInputGetCapabilities or XInputGetState from their window proc, expecting XInput controller list to be updated already. If the controller is missing they won't try calling XInput functions again until another WM_DEVICECHANGE message is received. Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/xinput1_3/main.c | 55 ++++++++++++++++++++++--------------------- 1 file changed, 28 insertions(+), 27 deletions(-) diff --git a/dlls/xinput1_3/main.c b/dlls/xinput1_3/main.c index 506802d142a..4536b027dc2 100644 --- a/dlls/xinput1_3/main.c +++ b/dlls/xinput1_3/main.c @@ -119,7 +119,8 @@ static struct xinput_controller controllers[XUSER_MAX_COUNT] = {{ &controller_critsect_debug[3], -1, 0, 0, 0, 0 }}, }; -static HMODULE xinput_instance; +static HDEVNOTIFY update_devnotify; +static HWND update_hwnd; static HANDLE start_event; static HANDLE stop_event; static HANDLE done_event; @@ -654,34 +655,14 @@ static DWORD WINAPI hid_update_thread_proc(void *param) struct xinput_controller *devices[XUSER_MAX_COUNT + 2]; HANDLE events[XUSER_MAX_COUNT + 2]; DWORD i, count = 2, ret = WAIT_TIMEOUT; - DEV_BROADCAST_DEVICEINTERFACE_W filter = - { - .dbcc_size = sizeof(DEV_BROADCAST_DEVICEINTERFACE_W), - .dbcc_devicetype = DBT_DEVTYP_DEVICEINTERFACE, - .dbcc_classguid = GUID_DEVINTERFACE_WINEXINPUT, - }; - WNDCLASSEXW cls = - { - .cbSize = sizeof(WNDCLASSEXW), - .hInstance = xinput_instance, - .lpszClassName = L"__wine_xinput_devnotify", - .lpfnWndProc = xinput_devnotify_wndproc, - }; - HDEVNOTIFY notif; - HWND hwnd; MSG msg; - RegisterClassExW(&cls); - hwnd = CreateWindowExW(0, cls.lpszClassName, NULL, 0, 0, 0, 0, 0, - HWND_MESSAGE, NULL, NULL, NULL); - notif = RegisterDeviceNotificationW(hwnd, &filter, DEVICE_NOTIFY_WINDOW_HANDLE); - update_controller_list(); SetEvent(start_event); do { - if (ret == count) while (PeekMessageW(&msg, hwnd, 0, 0, PM_REMOVE)) DispatchMessageW(&msg); + if (ret == count) while (PeekMessageW(&msg, update_hwnd, 0, 0, PM_REMOVE)) DispatchMessageW(&msg); if (ret == WAIT_TIMEOUT) update_controller_list(); if (ret < count - 2) read_controller_state(devices[ret]); @@ -704,10 +685,6 @@ static DWORD WINAPI hid_update_thread_proc(void *param) while ((ret = MsgWaitForMultipleObjectsEx(count, events, 2000, QS_ALLINPUT, MWMO_ALERTABLE)) < count - 1 || ret == count || ret == WAIT_TIMEOUT); - UnregisterDeviceNotification(notif); - DestroyWindow(hwnd); - UnregisterClassW(cls.lpszClassName, xinput_instance); - if (ret != count - 1) ERR("update thread exited unexpectedly, ret %u\n", ret); SetEvent(done_event); return ret; @@ -765,15 +742,39 @@ static void controller_unlock(struct xinput_controller *controller) BOOL WINAPI DllMain(HINSTANCE inst, DWORD reason, LPVOID reserved) { + DEV_BROADCAST_DEVICEINTERFACE_W filter = + { + .dbcc_size = sizeof(DEV_BROADCAST_DEVICEINTERFACE_W), + .dbcc_devicetype = DBT_DEVTYP_DEVICEINTERFACE, + .dbcc_classguid = GUID_DEVINTERFACE_WINEXINPUT, + }; + WNDCLASSEXW class = + { + .cbSize = sizeof(WNDCLASSEXW), + .hInstance = inst, + .lpszClassName = L"__wine_xinput_devnotify", + .lpfnWndProc = xinput_devnotify_wndproc, + }; + + TRACE("inst %p, reason %u, reserved %p.\n", inst, reason, reserved); + switch (reason) { case DLL_PROCESS_ATTACH: - xinput_instance = inst; DisableThreadLibraryCalls(inst); + + RegisterClassExW(&class); + update_hwnd = CreateWindowExW(0, class.lpszClassName, NULL, 0, 0, 0, 0, 0, + HWND_MESSAGE, NULL, NULL, NULL); + update_devnotify = RegisterDeviceNotificationW(update_hwnd, &filter, DEVICE_NOTIFY_WINDOW_HANDLE); break; case DLL_PROCESS_DETACH: if (reserved) break; stop_update_thread(); + + UnregisterDeviceNotification(update_devnotify); + DestroyWindow(update_hwnd); + UnregisterClassW(class.lpszClassName, class.hInstance); break; } return TRUE; -- 2.34.1
On 12/23/21 15:33, Rémi Bernon wrote:
Instead of doing it lazily, which is more likely to be after the game has registered theirs. As the message handling order is the same as the registration, XInput may end updating its device list only too late, every time.
Some games register device notifications and call XInputGetCapabilities or XInputGetState from their window proc, expecting XInput controller list to be updated already. If the controller is missing they won't try calling XInput functions again until another WM_DEVICECHANGE message is received.
Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> ---
Yeah, thanks for ignoring this. Having a better look it looks like this can better be solved through I_ScRegisterDeviceNotification filters. The internal XInput device should already be sending its WM_DEVICECHANGE message first, and only then the HID interface the game may be watching for. As we don't have filter implementation yet, the game reacts to the first message (and for some reason differently on the second one), before XInput can refresh its device list. -- Rémi Bernon <rbernon(a)codeweavers.com>
On 12/23/21 15:33, Rémi Bernon wrote:
From: Arkadiusz Hiler <ahiler(a)codeweavers.com>
This is a preparation for a patch that changes setupapi to return lowercased device paths, which is the source of RawInput device name here. On Windows RawInput returns mostly uppercase names (except the GUID) and we want to keep that behavior.
Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> ---
First two patches have been in Proton for a long while.
They aren't directly related to the SDL issue described in PATCH 3, and as we're already using upper case device paths it's possible to skip them.
However they fix some other kind of hotplug problem, and so I think they are related and worth having. Then, because SDL checks for uppercase "IG_", we need PATCH 1 before PATCH 2.
I'm a bit unsure about PATCH 4, as creating a (msg) window in DllMain is probably not great, but I don't really how else we can make sure XInput gets notified first. It would be better to avoid refreshing the device list on every call.
I really don't know how bad it is to do that in DllMain. As a possible (ugly?) workaround, after PATCH 3 which should keep it up to date, it may be possible to use user32 rawinput device list to tell whenever new devices have appeared instead. We'll still have to call it on every XInput call though, which is less ideal than handling the notifications. -- Rémi Bernon <rbernon(a)codeweavers.com>
participants (2)
-
Marvin -
Rémi Bernon