[PATCH 0/4] MR2861: dinput: Test and fix loading of action maps to the registry.
From: Rémi Bernon <rbernon(a)codeweavers.com> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=54917 --- dlls/dinput/tests/device8.c | 118 ++++++++++++++++++++++++++++++++++++ 1 file changed, 118 insertions(+) diff --git a/dlls/dinput/tests/device8.c b/dlls/dinput/tests/device8.c index 69106749528..dd606dce2a5 100644 --- a/dlls/dinput/tests/device8.c +++ b/dlls/dinput/tests/device8.c @@ -34,6 +34,10 @@ #include "dinput_test.h" +#include "initguid.h" + +DEFINE_GUID(GUID_keyboard_action_mapping,0x00000001,0x0002,0x0003,0x04,0x05,0x06,0x07,0x08,0x09,0x0a,0x0b); + struct enum_data { DWORD version; @@ -1712,6 +1716,60 @@ static void test_dik_codes( IDirectInputDevice8W *device, HANDLE event, HWND hwn ok( hr == DI_OK, "Unacquire returned %#lx\n", hr ); } +#define check_member_str_( file, line, val, exp, member ) \ + ok_(file, line)( !strcmp( (val).member, (exp).member ), "got " #member " %s\n", \ + debugstr_a((val).member) ) +#define check_member_str( val, exp, member ) \ + check_member_str_( __FILE__, __LINE__, val, exp, member ) + +#define check_diactionA( a, b ) check_diactionA_( __LINE__, a, b ) +static void check_diactionA_( int line, const DIACTIONA *actual, const DIACTIONA *expected ) +{ + check_member_( __FILE__, line, *actual, *expected, "%#Ix", uAppData ); + check_member_( __FILE__, line, *actual, *expected, "%#lx", dwSemantic ); + todo_wine_if( expected->dwSemantic == 0x810004c8 ) + check_member_( __FILE__, line, *actual, *expected, "%#lx", dwFlags ); + if (actual->lptszActionName && expected->lptszActionName) + check_member_str_( __FILE__, line, *actual, *expected, lptszActionName ); + else + check_member_( __FILE__, line, *actual, *expected, "%p", lptszActionName ); + todo_wine_if( expected->dwSemantic == 0x810004c8 ) + check_member_guid_( __FILE__, line, *actual, *expected, guidInstance ); + todo_wine_if( expected->dwSemantic == 0x810004c8 ) + check_member_( __FILE__, line, *actual, *expected, "%#lx", dwObjID ); + todo_wine_if( expected->dwSemantic == 0x810004c8 ) + check_member_( __FILE__, line, *actual, *expected, "%#lx", dwHow ); +} + +#define check_diactionformatA( a, b ) check_diactionformatA_( __LINE__, a, b ) +static void check_diactionformatA_( int line, const DIACTIONFORMATA *actual, const DIACTIONFORMATA *expected ) +{ + DWORD i; + check_member_( __FILE__, line, *actual, *expected, "%#lx", dwSize ); + check_member_( __FILE__, line, *actual, *expected, "%#lx", dwActionSize ); + check_member_( __FILE__, line, *actual, *expected, "%#lx", dwDataSize ); + check_member_( __FILE__, line, *actual, *expected, "%#lx", dwNumActions ); + for (i = 0; i < min( actual->dwNumActions, expected->dwNumActions ); ++i) + { + winetest_push_context( "action[%lu]", i ); + check_diactionA_( line, actual->rgoAction + i, expected->rgoAction + i ); + winetest_pop_context(); + if (expected->dwActionSize != sizeof(DIACTIONA)) break; + if (actual->dwActionSize != sizeof(DIACTIONA)) break; + } + check_member_guid_( __FILE__, line, *actual, *expected, guidActionMap ); + check_member_( __FILE__, line, *actual, *expected, "%#lx", dwGenre ); + check_member_( __FILE__, line, *actual, *expected, "%#lx", dwBufferSize ); + check_member_( __FILE__, line, *actual, *expected, "%+ld", lAxisMin ); + check_member_( __FILE__, line, *actual, *expected, "%+ld", lAxisMax ); + check_member_( __FILE__, line, *actual, *expected, "%p", hInstString ); + check_member_( __FILE__, line, *actual, *expected, "%ld", ftTimeStamp.dwLowDateTime ); + check_member_( __FILE__, line, *actual, *expected, "%ld", ftTimeStamp.dwHighDateTime ); + todo_wine + check_member_( __FILE__, line, *actual, *expected, "%#lx", dwCRC ); + check_member_str_( __FILE__, line, *actual, *expected, tszActionMap ); +} + static void test_sys_keyboard( DWORD version ) { const DIDEVCAPS expect_caps = @@ -2213,6 +2271,64 @@ skip_key_tests: localized = old_localized; } +static void test_sys_keyboard_action_format(void) +{ + DIACTIONA actions[] = + { + {.uAppData = 0x1, .dwSemantic = 0x810004c8, .dwFlags = DIA_APPNOMAP}, + {.uAppData = 0x1, .dwSemantic = 0x81000448, .dwFlags = 0}, + }; + const DIACTIONA expect_actions[ARRAY_SIZE(actions)] = + { + {.uAppData = 0x1, .dwSemantic = 0x810004c8, .dwFlags = 0, .guidInstance = GUID_SysKeyboard, .dwObjID = 0xc804, .dwHow = DIAH_DEFAULT}, + {.uAppData = 0x1, .dwSemantic = 0x81000448, .dwFlags = 0, .guidInstance = GUID_SysKeyboard, .dwObjID = 0x4804, .dwHow = DIAH_DEFAULT}, + }; + DIACTIONFORMATA action_format = + { + .dwSize = sizeof(DIACTIONFORMATA), + .dwActionSize = sizeof(DIACTIONA), + .dwNumActions = ARRAY_SIZE(actions), + .dwDataSize = ARRAY_SIZE(actions) * 4, + .rgoAction = actions, + .dwGenre = 0x1000000, + .guidActionMap = GUID_keyboard_action_mapping, + }; + const DIACTIONFORMATA expect_action_format = + { + .dwSize = sizeof(DIACTIONFORMATA), + .dwActionSize = sizeof(DIACTIONA), + .dwNumActions = ARRAY_SIZE(expect_actions), + .dwDataSize = ARRAY_SIZE(expect_actions) * 4, + .rgoAction = (DIACTIONA *)expect_actions, + .dwGenre = 0x1000000, + .guidActionMap = GUID_keyboard_action_mapping, + .dwCRC = 0x68e7e227, + }; + IDirectInputDevice8A *device; + IDirectInput8A *dinput; + HRESULT hr; + LONG ref; + + hr = DirectInput8Create( instance, 0x800, &IID_IDirectInput8A, (void **)&dinput, NULL ); + ok( hr == DI_OK, "CreateDevice returned %#lx\n", hr ); + hr = IDirectInput_CreateDevice( dinput, &GUID_SysKeyboard, &device, NULL ); + ok( hr == DI_OK, "CreateDevice returned %#lx\n", hr ); + ref = IDirectInput_Release( dinput ); + ok( ref == 0, "Release returned %ld\n", ref ); + + hr = IDirectInputDevice8_BuildActionMap( device, &action_format, NULL, 2 ); + ok( hr == DI_OK, "BuildActionMap returned %#lx\n", hr ); + check_diactionformatA(&action_format, &expect_action_format); + + hr = IDirectInputDevice8_SetActionMap( device, &action_format, NULL, 2 ); + ok( hr == DI_SETTINGSNOTSAVED, "SetActionMap returned %#lx\n", hr ); + hr = IDirectInputDevice8_SetActionMap( device, &action_format, NULL, 0 ); + ok( hr == DI_OK, "SetActionMap returned %#lx\n", hr ); + + ref = IDirectInputDevice8_Release( device ); + ok( ref == 0, "Release returned %ld\n", ref ); +} + START_TEST(device8) { dinput_test_init(); @@ -2233,6 +2349,8 @@ START_TEST(device8) test_sys_keyboard( 0x700 ); test_sys_keyboard( 0x800 ); + test_sys_keyboard_action_format(); + test_mouse_keyboard(); test_keyboard_events(); test_appdata_property(); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2861
From: Rémi Bernon <rbernon(a)codeweavers.com> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=54917 --- dlls/dinput/device.c | 25 +++++++++++-------------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/dlls/dinput/device.c b/dlls/dinput/device.c index 4db8bd943cb..4f76168a1a3 100644 --- a/dlls/dinput/device.c +++ b/dlls/dinput/device.c @@ -323,19 +323,20 @@ int dinput_device_object_index_from_id( IDirectInputDevice8W *iface, DWORD id ) * Retrieves an open registry key to save the mapping, parametrized for an username, * specific device and specific action mapping guid. */ -static HKEY get_mapping_key(const WCHAR *device, const WCHAR *username, const WCHAR *guid) +static HKEY get_mapping_key( const WCHAR *device, const WCHAR *username, const WCHAR *guid, BOOL delete ) { - static const WCHAR *subkey = L"Software\\Wine\\DirectInput\\Mappings\\%s\\%s\\%s"; - HKEY hkey; + static const WCHAR format[] = L"Software\\Wine\\DirectInput\\Mappings\\%s\\%s\\%s"; + SIZE_T len = wcslen( format ) + wcslen( username ) + wcslen( device ) + wcslen( guid ) + 1; WCHAR *keyname; + HKEY hkey; - SIZE_T len = wcslen( subkey ) + wcslen( username ) + wcslen( device ) + wcslen( guid ) + 1; - keyname = malloc( sizeof(WCHAR) * len ); - swprintf( keyname, len, subkey, username, device, guid ); + if (!(keyname = malloc( sizeof(WCHAR) * len ))) return 0; /* The key used is HKCU\Software\Wine\DirectInput\Mappings\[username]\[device]\[mapping_guid] */ - if (RegCreateKeyW(HKEY_CURRENT_USER, keyname, &hkey)) - hkey = 0; + swprintf( keyname, len, format, username, device, guid ); + + if (delete) RegDeleteTreeW( HKEY_CURRENT_USER, keyname ); + if (RegCreateKeyW( HKEY_CURRENT_USER, keyname, &hkey )) hkey = 0; free( keyname ); @@ -355,9 +356,7 @@ static HRESULT save_mapping_settings(IDirectInputDevice8W *iface, LPDIACTIONFORM if (StringFromCLSID(&lpdiaf->guidActionMap, &guid_str) != S_OK) return DI_SETTINGSNOTSAVED; - hkey = get_mapping_key(didev.tszInstanceName, lpszUsername, guid_str); - - if (!hkey) + if (!(hkey = get_mapping_key( didev.tszInstanceName, lpszUsername, guid_str, TRUE ))) { CoTaskMemFree(guid_str); return DI_SETTINGSNOTSAVED; @@ -398,9 +397,7 @@ static BOOL load_mapping_settings( struct dinput_device *This, LPDIACTIONFORMATW if (StringFromCLSID(&lpdiaf->guidActionMap, &guid_str) != S_OK) return FALSE; - hkey = get_mapping_key(didev.tszInstanceName, username, guid_str); - - if (!hkey) + if (!(hkey = get_mapping_key( didev.tszInstanceName, username, guid_str, FALSE ))) { CoTaskMemFree(guid_str); return FALSE; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2861
From: Rémi Bernon <rbernon(a)codeweavers.com> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=54917 --- dlls/dinput/device.c | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/dlls/dinput/device.c b/dlls/dinput/device.c index 4f76168a1a3..8fc54d8ce32 100644 --- a/dlls/dinput/device.c +++ b/dlls/dinput/device.c @@ -1850,6 +1850,8 @@ static HRESULT WINAPI dinput_device_BuildActionMap( IDirectInputDevice8W *iface, !IsEqualCLSID( &action->guidInstance, &impl->guid )) continue; if (action->dwFlags & DIA_APPMAPPED) action->dwHow = DIAH_APPREQUESTED; else action->dwHow = 0; + if (action->dwHow == DIAH_APPREQUESTED || action->dwHow == DIAH_USERCONFIG) continue; + if (!(action->dwFlags & DIA_APPNOMAP)) action->guidInstance = GUID_NULL; } /* Unless asked the contrary by these flags, try to load a previous mapping */ @@ -1861,17 +1863,6 @@ static HRESULT WINAPI dinput_device_BuildActionMap( IDirectInputDevice8W *iface, load_mapping_settings( impl, format, username_buf ); } - action_end = format->rgoAction + format->dwNumActions; - for (action = format->rgoAction; action < action_end; action++) - { - if (action->dwHow == DIAH_APPREQUESTED || action->dwHow == DIAH_USERCONFIG) continue; - if (flags == DIDBAM_PRESERVE && !IsEqualCLSID( &action->guidInstance, &GUID_NULL ) && - !IsEqualCLSID( &action->guidInstance, &impl->guid )) continue; - if (action->dwFlags & DIA_APPNOMAP) continue; - action->guidInstance = GUID_NULL; - action->dwHow = 0; - } - if (!(mapped = calloc( impl->device_format.dwNumObjs, sizeof(*mapped) ))) return DIERR_OUTOFMEMORY; action_end = format->rgoAction + format->dwNumActions; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2861
From: Rémi Bernon <rbernon(a)codeweavers.com> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=54917 --- dlls/dinput/device.c | 1 + dlls/dinput/tests/device8.c | 4 ---- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/dlls/dinput/device.c b/dlls/dinput/device.c index 8fc54d8ce32..b761b3e9d6d 100644 --- a/dlls/dinput/device.c +++ b/dlls/dinput/device.c @@ -1851,6 +1851,7 @@ static HRESULT WINAPI dinput_device_BuildActionMap( IDirectInputDevice8W *iface, if (action->dwFlags & DIA_APPMAPPED) action->dwHow = DIAH_APPREQUESTED; else action->dwHow = 0; if (action->dwHow == DIAH_APPREQUESTED || action->dwHow == DIAH_USERCONFIG) continue; + if ((action->dwSemantic & 0xf0000000) == 0x80000000) action->dwFlags &= ~DIA_APPNOMAP; if (!(action->dwFlags & DIA_APPNOMAP)) action->guidInstance = GUID_NULL; } diff --git a/dlls/dinput/tests/device8.c b/dlls/dinput/tests/device8.c index dd606dce2a5..1d53d35a93b 100644 --- a/dlls/dinput/tests/device8.c +++ b/dlls/dinput/tests/device8.c @@ -1727,17 +1727,13 @@ static void check_diactionA_( int line, const DIACTIONA *actual, const DIACTIONA { check_member_( __FILE__, line, *actual, *expected, "%#Ix", uAppData ); check_member_( __FILE__, line, *actual, *expected, "%#lx", dwSemantic ); - todo_wine_if( expected->dwSemantic == 0x810004c8 ) check_member_( __FILE__, line, *actual, *expected, "%#lx", dwFlags ); if (actual->lptszActionName && expected->lptszActionName) check_member_str_( __FILE__, line, *actual, *expected, lptszActionName ); else check_member_( __FILE__, line, *actual, *expected, "%p", lptszActionName ); - todo_wine_if( expected->dwSemantic == 0x810004c8 ) check_member_guid_( __FILE__, line, *actual, *expected, guidInstance ); - todo_wine_if( expected->dwSemantic == 0x810004c8 ) check_member_( __FILE__, line, *actual, *expected, "%#lx", dwObjID ); - todo_wine_if( expected->dwSemantic == 0x810004c8 ) check_member_( __FILE__, line, *actual, *expected, "%#lx", dwHow ); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2861
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=132839 Your paranoid android. === debian11 (32 bit fr report) === dinput: device8.c:799: Test failed: Failed to get data hr=0x8007000c device8.c:800: Test failed: Expected 1 element, received 10 device8.c:803: Test failed: IDirectInputDevice8_GetDeviceState failed: 0x8007000c device8.c:804: Test failed: Expected DIK_SPACE key state down device8.c:811: Test failed: Failed to get data hr=0x8007000c device8.c:812: Test failed: Expected 1 element, received 10 device8.c:821: Test failed: Failed to get data hr=0x8007000c device8.c:822: Test failed: Expected 0 elements, received 10 device8.c:825: Test failed: IDirectInputDevice8_GetDeviceState failed: 0x8007000c device8.c:834: Test failed: Failed to get data hr=0x8007000c device8.c:835: Test failed: Expected 0 elements, received 10
On Fri May 19 16:28:32 2023 +0000, **** wrote:
Marvin replied on the mailing list: ``` Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=132839 Your paranoid android. === debian11 (32 bit fr report) === dinput: device8.c:799: Test failed: Failed to get data hr=0x8007000c device8.c:800: Test failed: Expected 1 element, received 10 device8.c:803: Test failed: IDirectInputDevice8_GetDeviceState failed: 0x8007000c device8.c:804: Test failed: Expected DIK_SPACE key state down device8.c:811: Test failed: Failed to get data hr=0x8007000c device8.c:812: Test failed: Expected 1 element, received 10 device8.c:821: Test failed: Failed to get data hr=0x8007000c device8.c:822: Test failed: Expected 0 elements, received 10 device8.c:825: Test failed: IDirectInputDevice8_GetDeviceState failed: 0x8007000c device8.c:834: Test failed: Failed to get data hr=0x8007000c device8.c:835: Test failed: Expected 0 elements, received 10 ``` I don't think that's related to the changes here, `0x8007000c` is DIERR_NOTACQUIRED so it looks like some foreground issue again.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2861#note_33258
participants (2)
-
Marvin -
Rémi Bernon