[PATCH 1/3] dinput/tests: Make overlapped format tests more robust.
On Windows dinput sometimes ignores injected input events, so let's make sure that the event was registered by checking buffered data for the device. Signed-off-by: Arkadiusz Hiler <ahiler(a)codeweavers.com> --- This series supersedes 210326, 210148, 210147 and 210146. In this revision I use the _DX3 variant of structure for old dinput tests. Thanks Rémi! dlls/dinput/tests/device.c | 72 ++++++++++++++++++++++++++++---------- 1 file changed, 53 insertions(+), 19 deletions(-) diff --git a/dlls/dinput/tests/device.c b/dlls/dinput/tests/device.c index 6307957b6e2..b662bc43b27 100644 --- a/dlls/dinput/tests/device.c +++ b/dlls/dinput/tests/device.c @@ -348,6 +348,9 @@ void overlapped_format_tests(IDirectInputA *pDI, HWND hwnd) HRESULT hr; struct overlapped_state state; IDirectInputDeviceA *keyboard = NULL; + DIDEVICEOBJECTDATA_DX3 buffer[10]; + DIPROPDWORD dp; + DWORD cnt; hr = IDirectInput_CreateDevice(pDI, &GUID_SysKeyboard, &keyboard, NULL); ok(SUCCEEDED(hr), "IDirectInput_CreateDevice() failed: %08x\n", hr); @@ -355,6 +358,15 @@ void overlapped_format_tests(IDirectInputA *pDI, HWND hwnd) /* test overlapped slider - default value 0 */ hr = IDirectInputDevice_SetDataFormat(keyboard, &overlapped_slider_format); ok(SUCCEEDED(hr), "IDirectInputDevice_SetDataFormat() failed: %08x\n", hr); + + dp.diph.dwSize = sizeof(DIPROPDWORD); + dp.diph.dwHeaderSize = sizeof(DIPROPHEADER); + dp.diph.dwHow = DIPH_DEVICE; + dp.diph.dwObj = 0; + dp.dwData = ARRAY_SIZE(buffer); + hr = IDirectInputDevice_SetProperty(keyboard, DIPROP_BUFFERSIZE, &dp.diph); + ok(SUCCEEDED(hr), "IDirectInputDevice_SetProperty() failed: %08x\n", hr); + hr = IDirectInputDevice_Acquire(keyboard); ok(SUCCEEDED(hr), "IDirectInputDevice_Acquire() failed: %08x\n", hr); @@ -365,23 +377,34 @@ void overlapped_format_tests(IDirectInputA *pDI, HWND hwnd) keybd_event(0, DIK_D, KEYEVENTF_SCANCODE, 0); pump_messages(); - memset(&state, 0xFF, sizeof(state)); - hr = IDirectInputDevice_GetDeviceState(keyboard, sizeof(state), &state); - ok(SUCCEEDED(hr), "IDirectInputDevice_GetDeviceState() failed: %08x\n", hr); + cnt = ARRAY_SIZE(buffer); + hr = IDirectInputDevice_GetDeviceData(keyboard, sizeof(buffer[0]), (DIDEVICEOBJECTDATA*)buffer, &cnt, 0); + ok(SUCCEEDED(hr), "IDirectInputDevice_GetDeviceData() failed: %08x\n", hr); - ok(state.keys[0] == 0x00, "key A should be still up\n"); - ok(state.keys[1] == 0x00, "key S should be still up\n"); - ok(state.keys[2] == 0x80, "keydown for D did not register\n"); - ok(state.keys[3] == 0x00, "key F should be still up\n"); - ok(state.extra_element == 0, "State struct was not memset to zero\n"); + if (cnt != 1) + { + win_skip("We're not able to inject input into Windows dinput with events\n"); + } + else + { + memset(&state, 0xFF, sizeof(state)); + hr = IDirectInputDevice_GetDeviceState(keyboard, sizeof(state), &state); + ok(SUCCEEDED(hr), "IDirectInputDevice_GetDeviceState() failed: %08x\n", hr); - /* release D */ - keybd_event(0, DIK_D, KEYEVENTF_SCANCODE|KEYEVENTF_KEYUP, 0); - pump_messages(); + ok(state.keys[0] == 0x00, "key A should be still up\n"); + ok(state.keys[1] == 0x00, "key S should be still up\n"); + ok(state.keys[2] == 0x80, "keydown for D did not register\n"); + ok(state.keys[3] == 0x00, "key F should be still up\n"); + ok(state.extra_element == 0, "State struct was not memset to zero\n"); + } hr = IDirectInputDevice_Unacquire(keyboard); ok(SUCCEEDED(hr), "IDirectInputDevice_Unacquire() failed: %08x\n", hr); + /* release D */ + keybd_event(0, DIK_D, KEYEVENTF_SCANCODE|KEYEVENTF_KEYUP, 0); + pump_messages(); + /* test overlapped pov - default value - 0xFFFFFFFF */ hr = IDirectInputDevice_SetDataFormat(keyboard, &overlapped_pov_format); ok(SUCCEEDED(hr), "IDirectInputDevice_SetDataFormat() failed: %08x\n", hr); @@ -395,15 +418,26 @@ void overlapped_format_tests(IDirectInputA *pDI, HWND hwnd) keybd_event(0, DIK_D, KEYEVENTF_SCANCODE, 0); pump_messages(); - memset(&state, 0xFF, sizeof(state)); - hr = IDirectInputDevice_GetDeviceState(keyboard, sizeof(state), &state); - ok(SUCCEEDED(hr), "IDirectInputDevice_GetDeviceState() failed: %08x\n", hr); + cnt = ARRAY_SIZE(buffer); + hr = IDirectInputDevice_GetDeviceData(keyboard, sizeof(buffer[0]), (DIDEVICEOBJECTDATA*)buffer, &cnt, 0); + ok(SUCCEEDED(hr), "IDirectInputDevice_GetDeviceData() failed: %08x\n", hr); - ok(state.keys[0] == 0xFF, "key state should have been overwritten by the overlapped POV\n"); - ok(state.keys[1] == 0xFF, "key state should have been overwritten by the overlapped POV\n"); - ok(state.keys[2] == 0xFF, "key state should have been overwritten by the overlapped POV\n"); - ok(state.keys[3] == 0xFF, "key state should have been overwritten by the overlapped POV\n"); - ok(state.extra_element == 0, "State struct was not memset to zero\n"); + if (cnt != 1) + { + win_skip("We're not able to inject input into Windows dinput with events\n"); + } + else + { + memset(&state, 0xFF, sizeof(state)); + hr = IDirectInputDevice_GetDeviceState(keyboard, sizeof(state), &state); + ok(SUCCEEDED(hr), "IDirectInputDevice_GetDeviceState() failed: %08x\n", hr); + + ok(state.keys[0] == 0xFF, "key state should have been overwritten by the overlapped POV\n"); + ok(state.keys[1] == 0xFF, "key state should have been overwritten by the overlapped POV\n"); + ok(state.keys[2] == 0xFF, "key state should have been overwritten by the overlapped POV\n"); + ok(state.keys[3] == 0xFF, "key state should have been overwritten by the overlapped POV\n"); + ok(state.extra_element == 0, "State struct was not memset to zero\n"); + } /* release D */ keybd_event(0, DIK_D, KEYEVENTF_SCANCODE|KEYEVENTF_KEYUP, 0); -- 2.32.0
On Windows dinput sometimes ignores injected input events. Signed-off-by: Arkadiusz Hiler <ahiler(a)codeweavers.com> --- dlls/dinput8/tests/device.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/dlls/dinput8/tests/device.c b/dlls/dinput8/tests/device.c index 17deed193dd..7d6f8442085 100644 --- a/dlls/dinput8/tests/device.c +++ b/dlls/dinput8/tests/device.c @@ -127,10 +127,16 @@ static void test_device_input(IDirectInputDevice8A *lpdid, DWORD event_type, DWO data_size = 1; hr = IDirectInputDevice8_GetDeviceData(lpdid, sizeof(obj_data), &obj_data, &data_size, 0); - ok(hr == DI_BUFFEROVERFLOW, "GetDeviceData() failed: %08x\n", hr); - data_size = 1; - hr = IDirectInputDevice8_GetDeviceData(lpdid, sizeof(obj_data), &obj_data, &data_size, 0); - ok(hr == DI_OK && data_size == 1, "GetDeviceData() failed: %08x cnt:%d\n", hr, data_size); + if (hr != DI_BUFFEROVERFLOW) + { + win_skip("We're not able to inject input into Windows dinput8 with events\n"); + } + else + { + data_size = 1; + hr = IDirectInputDevice8_GetDeviceData(lpdid, sizeof(obj_data), &obj_data, &data_size, 0); + ok(hr == DI_OK && data_size == 1, "GetDeviceData() failed: %08x cnt:%d\n", hr, data_size); + } /* drain device's queue */ while (data_size == 1) -- 2.32.0
On Windows dinput sometimes ignores injected input events. Signed-off-by: Arkadiusz Hiler <ahiler(a)codeweavers.com> --- dlls/dinput8/tests/device.c | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/dlls/dinput8/tests/device.c b/dlls/dinput8/tests/device.c index 7d6f8442085..56aa7b353aa 100644 --- a/dlls/dinput8/tests/device.c +++ b/dlls/dinput8/tests/device.c @@ -869,11 +869,16 @@ static void test_keyboard_events(void) data_size = ARRAY_SIZE(obj_data); hr = IDirectInputDevice8_GetDeviceData(di_keyboard, sizeof(DIDEVICEOBJECTDATA), obj_data, &data_size, 0); ok(SUCCEEDED(hr), "Failed to get data hr=%08x\n", hr); - ok(data_size == 1, "Expected 1 element, received %d\n", data_size); - - hr = IDirectInputDevice8_GetDeviceState(di_keyboard, sizeof(kbdata), kbdata); - ok(SUCCEEDED(hr), "IDirectInputDevice8_GetDeviceState failed: %08x\n", hr); - ok(kbdata[DIK_SPACE], "Expected DIK_SPACE key state down\n"); + if (data_size != 1) + { + win_skip("We're not able to inject input into Windows dinput8 with events\n"); + } + else + { + hr = IDirectInputDevice8_GetDeviceState(di_keyboard, sizeof(kbdata), kbdata); + ok(SUCCEEDED(hr), "IDirectInputDevice8_GetDeviceState failed: %08x\n", hr); + ok(kbdata[DIK_SPACE], "Expected DIK_SPACE key state down\n"); + } keybd_event(VK_SPACE, DIK_SPACE, KEYEVENTF_KEYUP, 0); flush_events(); @@ -881,7 +886,8 @@ static void test_keyboard_events(void) data_size = ARRAY_SIZE(obj_data); hr = IDirectInputDevice8_GetDeviceData(di_keyboard, sizeof(DIDEVICEOBJECTDATA), obj_data, &data_size, 0); ok(SUCCEEDED(hr), "Failed to get data hr=%08x\n", hr); - ok(data_size == 1, "Expected 1 element, received %d\n", data_size); + if (data_size != 1) + win_skip("We're not able to inject input into Windows dinput8 with events\n"); /* Test injecting keyboard events with scancode=0. * Windows DInput ignores the VK, sets scancode 0 to be pressed, and GetDeviceData returns no elements. */ @@ -895,8 +901,10 @@ static void test_keyboard_events(void) hr = IDirectInputDevice8_GetDeviceState(di_keyboard, sizeof(kbdata), kbdata); ok(SUCCEEDED(hr), "IDirectInputDevice8_GetDeviceState failed: %08x\n", hr); - todo_wine - ok(kbdata[0], "Expected key 0 state down\n"); + if (!strcmp(winetest_platform, "windows") && !kbdata[0]) + skip("We're not able to inject input into Windows dinput8 with events\n"); + else + todo_wine ok(kbdata[0], "Expected key 0 state down\n"); keybd_event(VK_SPACE, 0, KEYEVENTF_KEYUP, 0); flush_events(); -- 2.32.0
On 8/3/21 12:59 PM, Arkadiusz Hiler wrote:
On Windows dinput sometimes ignores injected input events, so let's make sure that the event was registered by checking buffered data for the device.
Signed-off-by: Arkadiusz Hiler <ahiler(a)codeweavers.com> ---
This series supersedes 210326, 210148, 210147 and 210146.
In this revision I use the _DX3 variant of structure for old dinput tests. Thanks Rémi!
Are the injected events really ignored? This seems more like a timing problem, where Acquire operates asynchronously and misses some events when they are injected quickly after? As far as I can tell, Wine's implementation is synchronous when acquiring a device, and we wait for the background thread to complete the hooking (or rawinput device registration), but maybe native isn't. Wouldn't it be better to wait for injected events to be received after acquiring the device instead of skipping the tests completely? -- Rémi Bernon <rbernon(a)codeweavers.com>
On Tue, Aug 03, 2021 at 01:25:16PM +0200, Rémi Bernon wrote:
On 8/3/21 12:59 PM, Arkadiusz Hiler wrote:
On Windows dinput sometimes ignores injected input events, so let's make sure that the event was registered by checking buffered data for the device.
Signed-off-by: Arkadiusz Hiler <ahiler(a)codeweavers.com> ---
This series supersedes 210326, 210148, 210147 and 210146.
In this revision I use the _DX3 variant of structure for old dinput tests. Thanks Rémi!
Are the injected events really ignored? This seems more like a timing problem, where Acquire operates asynchronously and misses some events when they are injected quickly after?
As far as I can tell, Wine's implementation is synchronous when acquiring a device, and we wait for the background thread to complete the hooking (or rawinput device registration), but maybe native isn't.
According to my testing we may receive a few events after acquire but then miss some of them a bit later on. src: https://hiler.eu/p/34ddbfbdb0ba.txt res: https://testbot.winehq.org/JobDetails.pl?Key=94912&f101=exe64.report#k101
Wouldn't it be better to wait for injected events to be received after acquiring the device instead of skipping the tests completely?
I don't think it's related to Acquire() being async. Yes, we can continue firing events until we get one or a timeout happens, hoping that at least one will reach us, but we can still be extremely unlucky. -- Cheers, Arek
On 8/3/21 2:33 PM, Arkadiusz Hiler wrote:
On Tue, Aug 03, 2021 at 01:25:16PM +0200, Rémi Bernon wrote:
On 8/3/21 12:59 PM, Arkadiusz Hiler wrote:
On Windows dinput sometimes ignores injected input events, so let's make sure that the event was registered by checking buffered data for the device.
Signed-off-by: Arkadiusz Hiler <ahiler(a)codeweavers.com> ---
This series supersedes 210326, 210148, 210147 and 210146.
In this revision I use the _DX3 variant of structure for old dinput tests. Thanks Rémi!
Are the injected events really ignored? This seems more like a timing problem, where Acquire operates asynchronously and misses some events when they are injected quickly after?
As far as I can tell, Wine's implementation is synchronous when acquiring a device, and we wait for the background thread to complete the hooking (or rawinput device registration), but maybe native isn't.
According to my testing we may receive a few events after acquire but then miss some of them a bit later on.
src: https://hiler.eu/p/34ddbfbdb0ba.txt res: https://testbot.winehq.org/JobDetails.pl?Key=94912&f101=exe64.report#k101
Wouldn't it be better to wait for injected events to be received after acquiring the device instead of skipping the tests completely?
I don't think it's related to Acquire() being async. Yes, we can continue firing events until we get one or a timeout happens, hoping that at least one will reach us, but we can still be extremely unlucky.
Sounds fishy though. I modified your program in the following way, to instead keep a list of expected events according to what's been injected so far, and only compare them to the stream of data being returned, and I don't see any missing event (well more precisely I see some very rare misses right after Acquire):
DIDEVICEOBJECTDATA expect[1024]; DWORD read = 0, write = 0;
start = GetTickCount(); while (GetTickCount() - start < 20000) { DWORD inner_start = GetTickCount(); DIDEVICEOBJECTDATA data[10]; DWORD k = 0, keys = 4; BOOL got_one = FALSE;
loop++; assert(SUCCEEDED(IDirectInputDevice_Acquire(keyboard)));
do { DWORD i, cnt = ARRAYSIZE(data);
keybd_event(0, DIK_D + (k % keys), KEYEVENTF_SCANCODE, 0); pump_messages(); expect[write % ARRAYSIZE(expect)].dwData = 0x80; expect[write % ARRAYSIZE(expect)].dwOfs = DIK_D + (k % keys); write++;
assert(SUCCEEDED(IDirectInputDevice_GetDeviceData(keyboard, sizeof(data[0]), data, &cnt, 0)));
for (i = 0; i < cnt; ++i) { /* may miss one event right after Acquire */ if (expect[read % ARRAYSIZE(expect)].dwData != data[i].dwData && !got_one) read++; got_one = TRUE; assert(expect[read % ARRAYSIZE(expect)].dwData == data[i].dwData); assert(expect[read % ARRAYSIZE(expect)].dwOfs == data[i].dwOfs); assert(read < write); read++; }
keybd_event(0, DIK_D + (k % keys), KEYEVENTF_SCANCODE | KEYEVENTF_KEYUP, 0); pump_messages(); expect[write % ARRAYSIZE(expect)].dwData = 0x0; expect[write % ARRAYSIZE(expect)].dwOfs = DIK_D + (k % keys); write++;
got_one = TRUE; k++; } while (GetTickCount() - inner_start < 500);
assert(SUCCEEDED(IDirectInputDevice_Unacquire(keyboard))); }
assert(read == write - 1);
FWIW I think your original program should always inject the release event if dinput didn't report the press, otherwise the next press may not be considered as an event at all from the user32 perspective. It doesn't explain everything though. Otherwise I think the missing events you are seeing are more likely to be a consequence of the asynchronous processing of the dinput events. Possibly even more likely to happen with dinput >= 8, as dinput <= 7 uses low-level hooks which I believe should block the main thread (either the injection, or its reception of window messages). Skipping the tests, even if it's only on Windows, feels a bit misleading, like if we were acknowledging the fact that sometimes injected events may be genuinely missed. Cheers, -- Rémi Bernon <rbernon(a)codeweavers.com>
participants (2)
-
Arkadiusz Hiler -
Rémi Bernon