[PATCH 1/3] dinput/tests: Make overlapped format tests more robust.

[Rémi Bernon]

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 at 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 at codeweavers.com>