This helps when dealing with the quirky c_dfDIJoystick2 format, which defines all the sliders as overlapping, i.e. rglSlider[2], rglVSlider[2], rglASlider[2] and rglFSlider[2] all share the same offset.
With the Linux backends, if the device exposes enough axes (e.g. some HOTAS) we end up using rglSlider.
Let's make sure we don't unnecessarily override the values there.
Signed-off-by: Arkadiusz Hiler ahiler@codeweavers.com ---
create_DataFormat() is mixed tabs and spaces. All the newer changes seem to use spaces only. I've followed that which makes the funky looking diff here.
dlls/dinput/device.c | 14 ++--- dlls/dinput/tests/device.c | 122 +++++++++++++++++++++++++++++++++++++ 2 files changed, 129 insertions(+), 7 deletions(-)
diff --git a/dlls/dinput/device.c b/dlls/dinput/device.c index 29a569ca9f1..ac60a21326c 100644 --- a/dlls/dinput/device.c +++ b/dlls/dinput/device.c @@ -526,20 +526,20 @@ static HRESULT create_DataFormat(LPCDIDATAFORMAT asked_format, DataFormat *forma TRACE(" "); _dump_EnumObjects_flags(asked_format->rgodf[j].dwType); TRACE("\n"); TRACE(" * dwFlags: 0x%08x\n", asked_format->rgodf[j].dwFlags); TRACE(" "); _dump_ObjectDataFormat_flags(asked_format->rgodf[j].dwFlags); TRACE("\n"); - + + same = 0; + + if (!(asked_format->rgodf[j].dwType & DIDFT_POV)) + continue; /* fill_DataFormat memsets the buffer to 0 */ + if (asked_format->rgodf[j].dwType & DIDFT_BUTTON) dt[index].size = sizeof(BYTE); else dt[index].size = sizeof(DWORD); dt[index].offset_in = -1; dt[index].offset_out = asked_format->rgodf[j].dwOfs; - if (asked_format->rgodf[j].dwType & DIDFT_POV) - dt[index].value = -1; - else - dt[index].value = 0; + dt[index].value = -1; index++; - - same = 0; } }
diff --git a/dlls/dinput/tests/device.c b/dlls/dinput/tests/device.c index 485acca4f61..6307957b6e2 100644 --- a/dlls/dinput/tests/device.c +++ b/dlls/dinput/tests/device.c @@ -292,6 +292,126 @@ static BOOL CALLBACK enum_devices(const DIDEVICEINSTANCEA *lpddi, void *pvRef) return DIENUM_CONTINUE; }
+struct overlapped_state +{ + BYTE keys[4]; + DWORD extra_element; +}; + +static const DIOBJECTDATAFORMAT obj_overlapped_slider_format[] = { + { &GUID_Key, 0, DIDFT_OPTIONAL|DIDFT_BUTTON|DIDFT_MAKEINSTANCE(DIK_A),0}, + { &GUID_Key, 1, DIDFT_OPTIONAL|DIDFT_BUTTON|DIDFT_MAKEINSTANCE(DIK_S),0}, + { &GUID_Key, 2, DIDFT_OPTIONAL|DIDFT_BUTTON|DIDFT_MAKEINSTANCE(DIK_D),0}, + { &GUID_Key, 3, DIDFT_OPTIONAL|DIDFT_BUTTON|DIDFT_MAKEINSTANCE(DIK_F),0}, + { &GUID_Slider, 0, DIDFT_OPTIONAL|DIDFT_AXIS|DIDFT_ANYINSTANCE,DIDOI_ASPECTPOSITION}, +}; + +static const DIDATAFORMAT overlapped_slider_format = { + sizeof(DIDATAFORMAT), + sizeof(DIOBJECTDATAFORMAT), + DIDF_ABSAXIS, + sizeof(struct overlapped_state), + ARRAY_SIZE(obj_overlapped_slider_format), + (LPDIOBJECTDATAFORMAT)obj_overlapped_slider_format +}; + +static const DIOBJECTDATAFORMAT obj_overlapped_pov_format[] = { + { &GUID_Key, 0, DIDFT_OPTIONAL|DIDFT_BUTTON|DIDFT_MAKEINSTANCE(DIK_A),0}, + { &GUID_Key, 1, DIDFT_OPTIONAL|DIDFT_BUTTON|DIDFT_MAKEINSTANCE(DIK_S),0}, + { &GUID_Key, 2, DIDFT_OPTIONAL|DIDFT_BUTTON|DIDFT_MAKEINSTANCE(DIK_D),0}, + { &GUID_Key, 3, DIDFT_OPTIONAL|DIDFT_BUTTON|DIDFT_MAKEINSTANCE(DIK_F),0}, + { &GUID_POV, 0, DIDFT_OPTIONAL|DIDFT_POV|DIDFT_ANYINSTANCE,0}, +}; + +static const DIDATAFORMAT overlapped_pov_format = { + sizeof(DIDATAFORMAT), + sizeof(DIOBJECTDATAFORMAT), + DIDF_ABSAXIS, + sizeof(struct overlapped_state), + ARRAY_SIZE(obj_overlapped_pov_format), + (LPDIOBJECTDATAFORMAT)obj_overlapped_pov_format +}; + +static void pump_messages(void) +{ + MSG msg; + + while (PeekMessageA(&msg, 0, 0, 0, PM_REMOVE)) + { + TranslateMessage(&msg); + DispatchMessageA(&msg); + } +} + +void overlapped_format_tests(IDirectInputA *pDI, HWND hwnd) +{ + HRESULT hr; + struct overlapped_state state; + IDirectInputDeviceA *keyboard = NULL; + + hr = IDirectInput_CreateDevice(pDI, &GUID_SysKeyboard, &keyboard, NULL); + ok(SUCCEEDED(hr), "IDirectInput_CreateDevice() failed: %08x\n", hr); + + /* test overlapped slider - default value 0 */ + hr = IDirectInputDevice_SetDataFormat(keyboard, &overlapped_slider_format); + ok(SUCCEEDED(hr), "IDirectInputDevice_SetDataFormat() failed: %08x\n", hr); + hr = IDirectInputDevice_Acquire(keyboard); + ok(SUCCEEDED(hr), "IDirectInputDevice_Acquire() failed: %08x\n", hr); + + SetFocus(hwnd); + pump_messages(); + + /* press D */ + 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); + + 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"); + + /* release D */ + keybd_event(0, DIK_D, KEYEVENTF_SCANCODE|KEYEVENTF_KEYUP, 0); + pump_messages(); + + hr = IDirectInputDevice_Unacquire(keyboard); + ok(SUCCEEDED(hr), "IDirectInputDevice_Unacquire() failed: %08x\n", hr); + + /* test overlapped pov - default value - 0xFFFFFFFF */ + hr = IDirectInputDevice_SetDataFormat(keyboard, &overlapped_pov_format); + ok(SUCCEEDED(hr), "IDirectInputDevice_SetDataFormat() failed: %08x\n", hr); + hr = IDirectInputDevice_Acquire(keyboard); + ok(SUCCEEDED(hr), "IDirectInputDevice_Acquire() failed: %08x\n", hr); + + SetFocus(hwnd); + pump_messages(); + + /* press D */ + 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); + + 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); + pump_messages(); + + if (keyboard) IUnknown_Release(keyboard); +} + static void device_tests(void) { HRESULT hr; @@ -342,6 +462,8 @@ static void device_tests(void) if (device) IUnknown_Release(device); }
+ overlapped_format_tests(pDI, hwnd); + DestroyWindow(hwnd); } if (obj) IUnknown_Release(obj);
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=93874
Your paranoid android.
=== w8adm (32 bit report) ===
dinput: device.c:374: Test failed: keydown for D did not register
On Thu, Jul 08, 2021 at 05:10:43AM -0500, Marvin wrote:
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=93874
Your paranoid android.
=== w8adm (32 bit report) ===
dinput: device.c:374: Test failed: keydown for D did not register
Hurm, the tests may be a bit flaky... I haven't seen that failure locally or in my testbot runs. I'll look into making the keypresses registering more realible.
On Thu, 8 Jul 2021, Marvin wrote: [...]
Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=93874
[...]
=== w8adm (32 bit report) ===
dinput: device.c:374: Test failed: keydown for D did not register
This is a new random failure that happens on the w8 VM:
https://test.winehq.org/data/patterns.html#dinput:device
Could you look into it?
On Tue, Jul 20, 2021 at 01:09:25PM +0200, Francois Gouget wrote:
On Thu, 8 Jul 2021, Marvin wrote: [...]
Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=93874
[...]
=== w8adm (32 bit report) ===
dinput: device.c:374: Test failed: keydown for D did not register
This is a new random failure that happens on the w8 VM:
https://test.winehq.org/data/patterns.html#dinput:device
Could you look into it?
Hey,
Yes. I am already looking into this + other flaky dinput tests that fail in a similar way.
Turns out that on Windows dinput sometimes just drops key events injected via SendInput() / keybd_event().
I've tried many things like throttling the rate of the events or trying to repeat them multiple times if we didn't get anything but when running the tests in a loop eventually we will be unlucky and the test will fail.
The only reliable solution seems to be checking if the device has received any data via GetDeviceData(), and if we don't see any events there then we can win_skip(). AFAIU it will fail under Wine but just skip on Windows.
The tests would still have sporadic skips and I am not sure if this is any better than sporadic failures. Do we track those? This also creates room for breaking tests on Window without noticing...
On Tue, 20 Jul 2021, Arkadiusz Hiler wrote: [...]
Yes. I am already looking into this + other flaky dinput tests that fail in a similar way.
Thanks!
Turns out that on Windows dinput sometimes just drops key events injected via SendInput() / keybd_event().
Is it all Windows versions? This one failure seemed to only happen on one Windows 8.1 VM (but maybe the others just got lucky so far).
[...]
The only reliable solution seems to be checking if the device has received any data via GetDeviceData(), and if we don't see any events there then we can win_skip(). AFAIU it will fail under Wine but just skip on Windows.
Yes. win_skip() skips on Windows and fails in Wine.
The tests would still have sporadic skips and I am not sure if this is any better than sporadic failures. Do we track those? This also creates room for breaking tests on Window without noticing...
We don't really track skips. They are reported on the test result pages (they are the blue bars) but are not considered to be an issue as there are many legitimate reasons for skipping a test (e.g. if the API is not implemented, or if it tests a device which is not present).
https://test.winehq.org/data/d60c450c7be196c2072f74e34f7760d39e3bad32/index_...
On Tue, Jul 20, 2021 at 02:10:50PM +0200, Francois Gouget wrote:
On Tue, 20 Jul 2021, Arkadiusz Hiler wrote: [...]
Yes. I am already looking into this + other flaky dinput tests that fail in a similar way.
Thanks!
Turns out that on Windows dinput sometimes just drops key events injected via SendInput() / keybd_event().
Is it all Windows versions? This one failure seemed to only happen on one Windows 8.1 VM (but maybe the others just got lucky so far).
If run in a loop those tests fail (very rarely) for me on Win10 machine, and I am fairly sure that I've seen failures on Win7 too.
Here's a related failure in a test that emits a lot of events for a device with a smaller buffer to get DI_BUFFEROVERFLOW:
https://testbot.winehq.org/JobDetails.pl?Key=94396#k206
So some events have been missed.
[...]
The only reliable solution seems to be checking if the device has received any data via GetDeviceData(), and if we don't see any events there then we can win_skip(). AFAIU it will fail under Wine but just skip on Windows.
Yes. win_skip() skips on Windows and fails in Wine.
Turns out there are some test that already skip on Windows if injecting events fails. That's from my Win 10:
device.c:106: Tests skipped: We're not able to inject input into Windows dinput8 with events
The tests would still have sporadic skips and I am not sure if this is any better than sporadic failures. Do we track those? This also creates room for breaking tests on Window without noticing...
We don't really track skips. They are reported on the test result pages (they are the blue bars) but are not considered to be an issue as there are many legitimate reasons for skipping a test (e.g. if the API is not implemented, or if it tests a device which is not present).
https://test.winehq.org/data/d60c450c7be196c2072f74e34f7760d39e3bad32/index_...
Good to know. Thanks!