June 2022 - Wine-Devel - list.winehq.org

Re: [PATCH 0/4] MR233: win32u: Partially move raw input APIs.

by Rémi Bernon

Rémi Bernon (@rbernon) commented about dlls/win32u/rawinput.c: > + wine_server_set_reply( req, msg_data, RAWINPUT_BUFFER_SIZE - rawinput->header.dwSize ); > + if (wine_server_call( req )) return ~0u; > + next_size = reply->next_size; > + count = reply->count; > + } > + SERVER_END_REQ; > + > + remaining = *data_size; > + for (i = 0; i < count; ++i) > + { > + data->header.dwSize = remaining; > + if (!rawinput_from_hardware_message( data, msg_data )) break; > + if (overhead) > + memmove( (char *)&data->data + overhead, &data->data, > + data->header.dwSize - sizeof(RAWINPUTHEADER) ); > + data->header.dwSize += overhead; I'd prefer either the original single hanging style, or ```C if (overhead) { memmove( (char *)&data->data + overhead, &data->data, data->header.dwSize - sizeof(RAWINPUTHEADER) ); data->header.dwSize += overhead; } ``` rather than a double hanging line like this. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/233#note_1939

June 13, 2022

Re: [PATCH 0/4] MR233: win32u: Partially move raw input APIs.

by Rémi Bernon

Rémi Bernon (@rbernon) commented about dlls/user32/tests/input.c: > ret = GetRawInputData((HRAWINPUT)lparam, RID_INPUT, &raw, &raw_size, sizeof(RAWINPUTHEADER)); > ok(ret > 0 && ret != (UINT)-1, "GetRawInputData failed\n"); > ok(raw.header.dwType == RIM_TYPEMOUSE, "Unexpected rawinput type: %lu\n", raw.header.dwType); > + ok(raw.header.dwSize == raw_size, "Expected size %u, got %lu\n", raw_size, raw.header.dwSize); > + todo_wine_if (wparam) > + ok(raw.header.wParam == wparam, "Expected wparam %Iu, got %Iu\n", wparam, raw.header.wParam); We don't usually indent `todo_wine_(if)`, do we? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/233#note_1938

June 13, 2022

Re: [PATCH 0/4] MR233: win32u: Partially move raw input APIs.

by Rémi Bernon

Rémi Bernon (@rbernon) commented about dlls/user32/tests/input.c: > ok(GetLastError() == ERROR_INVALID_PARAMETER, "GetRawInputBuffer returned %08lx\n", GetLastError()); > > + SetLastError(0xdeadbeef); > + size = sizeof(buffer); > + count = GetRawInputBuffer((RAWINPUT*)buffer, &size, sizeof(RAWINPUTHEADER) + 1); > + ok(count == ~0U, "GetRawInputBuffer succeeded\n"); > + ok(GetLastError() == ERROR_INVALID_PARAMETER, "GetRawInputBuffer returned %08lx\n", GetLastError()); > + > + /* the function returns 64-bit RAWINPUT structures on WoW64, but still > + * forbids sizeof(RAWINPUTHEADER) from the wrong architecture */ > + SetLastError(0xdeadbeef); > + size = sizeof(buffer); > + if (sizeof(void *) == 8) > + count = GetRawInputBuffer((RAWINPUT*)buffer, &size, sizeof(RAWINPUTHEADER32)); > + else > + count = GetRawInputBuffer((RAWINPUT*)buffer, &size, sizeof(RAWINPUTHEADER64)); Same here, I think adding a conditionally initialized `header_size` variable would make it simpler to read. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/233#note_1937

June 13, 2022

Re: [PATCH 0/4] MR233: win32u: Partially move raw input APIs.

by Rémi Bernon

Rémi Bernon (@rbernon) commented about dlls/user32/tests/input.c: > + ok(count == ~0u, "GetRawInputData returned %d\n", count); > + ok(GetLastError() == ERROR_INVALID_PARAMETER, "GetRawInputData returned %08lx\n", GetLastError()); > + } > + else if (is_wow64) > + { > + count = GetRawInputData((HRAWINPUT)lparam, RID_INPUT, &ri, &size, sizeof(RAWINPUTHEADER64)); > + todo_wine ok(count == sizeof(ri), "GetRawInputData returned %d\n", count); > + ok(ri.data.mouse.lLastX == 6, "Unexpected rawinput data: %ld\n", ri.data.mouse.lLastX); > + todo_wine ok(GetLastError() == 0xdeadbeef, "GetRawInputData returned %08lx\n", GetLastError()); > + } > + else > + { > + count = GetRawInputData((HRAWINPUT)lparam, RID_INPUT, &ri, &size, sizeof(RAWINPUTHEADER64)); > + ok(count == ~0u, "GetRawInputData returned %d\n", count); > + ok(GetLastError() == ERROR_INVALID_PARAMETER, "GetRawInputData returned %08lx\n", GetLastError()); > + } It feels a bit unbalanced to test invalid cases at the same time as a valid case depending on the arch. Could we have instead a test for the invalid sizes, and the another one with valid size? IMHO using a ternary operator and a variable for the header size instead of two different `GetRawInputData` calls would make it simpler too. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/233#note_1936

June 13, 2022

Re: [PATCH 0/4] MR233: win32u: Partially move raw input APIs.

by Rémi Bernon

Rémi Bernon (@rbernon) commented about dlls/user32/tests/input.c: > GetCursorPos(&pt); > ok(pt.x == 300 && pt.y == 300, "Unexpected cursor position pos %ldx%ld\n", pt.x, pt.y); > > - hwnd = CreateWindowA("static", "static", WS_VISIBLE | WS_POPUP, > + hwnd = CreateWindowA("static", "static", WS_POPUP, > 100, 100, 100, 100, 0, NULL, NULL, NULL); > SetWindowLongPtrA(hwnd, GWLP_WNDPROC, (LONG_PTR)rawinputbuffer_wndproc); > ok(hwnd != 0, "CreateWindow failed\n"); Wine currently doesn't listen to X11 mouse events in the background, it needs a visible window to receive them and only when the cursor moves over it. If you want to test `RIDEV_INPUTSINK` I think the best way is to use a second window (possibly invisible), and use it as the `hwndTarget`, while still moving the cursor over the first, visible, window. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/233#note_1935

June 13, 2022

Re: [PATCH 0/4] MR233: win32u: Partially move raw input APIs.

by Rémi Bernon

Rémi Bernon (@rbernon) commented about dlls/user32/tests/input.c: > + { > + if (is_wow64) > + { > + const RAWINPUT64 *data = (RAWINPUT64 *)buffer; > + ok(data->header.dwType == RIM_TYPEMOUSE, "Unexpected rawinput type: %lu\n", data->header.dwType); > + ok(data->header.dwSize == sizeof(*data), "Unexpected rawinput size: %lu\n", data->header.dwSize); > + todo_wine ok(data->header.wParam == wparam, "Unexpected wparam: %#I64x\n", data->header.wParam); > + } > + else > + { > + const RAWINPUT *data = (RAWINPUT *)buffer; > + ok(data->header.dwType == RIM_TYPEMOUSE, "Unexpected rawinput type: %lu\n", data->header.dwType); > + ok(data->header.dwSize == sizeof(*data), "Unexpected rawinput size: %lu\n", data->header.dwSize); > + todo_wine ok(data->header.wParam == wparam, "Unexpected wparam: %#Ix\n", data->header.wParam); > + } > + } The i variable isn't used here, I guess you wanted to iterate the buffer? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/233#note_1934

June 13, 2022

Re: [PATCH 4/4] win32u: Move NtUserGetRawInputBuffer from user32.

by Marvin

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=116822 Your paranoid android. === debian11 (32 bit report) === user32: input.c:2349: Test failed: GetRawInputBuffer succeeded input.c:2350: Test failed: GetRawInputBuffer returned unexpected size: 0 input.c:2351: Test failed: GetRawInputBuffer returned deadbeef input.c:2356: Test failed: GetRawInputBuffer returned unexpected size: 0 input.c:2384: Test failed: GetRawInputBuffer returned 0 input.c:2385: Test failed: GetRawInputBuffer returned unexpected size: 0 input.c:2386: Test failed: Unexpected rawinput data: 0 input.c:2397: Test failed: GetRawInputBuffer succeeded input.c:2398: Test failed: GetRawInputBuffer returned deadbeef input.c:2399: Test failed: Unexpected rawinput data: 0 input.c:2412: Test failed: Spurious WM_INPUT messages === debian11 (32 bit Chinese:China report) === user32: input.c:2349: Test failed: GetRawInputBuffer succeeded input.c:2350: Test failed: GetRawInputBuffer returned unexpected size: 0 input.c:2351: Test failed: GetRawInputBuffer returned deadbeef input.c:2356: Test failed: GetRawInputBuffer returned unexpected size: 0 input.c:2384: Test failed: GetRawInputBuffer returned 0 input.c:2385: Test failed: GetRawInputBuffer returned unexpected size: 0 input.c:2386: Test failed: Unexpected rawinput data: 0 input.c:2397: Test failed: GetRawInputBuffer succeeded input.c:2398: Test failed: GetRawInputBuffer returned deadbeef input.c:2399: Test failed: Unexpected rawinput data: 0 input.c:2412: Test failed: Spurious WM_INPUT messages === debian11 (32 bit WoW report) === user32: input.c:2349: Test failed: GetRawInputBuffer succeeded input.c:2350: Test failed: GetRawInputBuffer returned unexpected size: 0 input.c:2351: Test failed: GetRawInputBuffer returned deadbeef input.c:2356: Test failed: GetRawInputBuffer returned unexpected size: 0 input.c:2384: Test failed: GetRawInputBuffer returned 0 input.c:2385: Test failed: GetRawInputBuffer returned unexpected size: 0 input.c:2386: Test failed: Unexpected rawinput data: 0 input.c:2397: Test failed: GetRawInputBuffer succeeded input.c:2398: Test failed: GetRawInputBuffer returned deadbeef input.c:2399: Test failed: Unexpected rawinput data: 0 input.c:2412: Test failed: Spurious WM_INPUT messages === debian11 (64 bit WoW report) === user32: input.c:2349: Test failed: GetRawInputBuffer succeeded input.c:2350: Test failed: GetRawInputBuffer returned unexpected size: 0 input.c:2351: Test failed: GetRawInputBuffer returned deadbeef input.c:2356: Test failed: GetRawInputBuffer returned unexpected size: 0 input.c:2384: Test failed: GetRawInputBuffer returned 0 input.c:2385: Test failed: GetRawInputBuffer returned unexpected size: 0 input.c:2386: Test failed: Unexpected rawinput data: 0 input.c:2397: Test failed: GetRawInputBuffer succeeded input.c:2398: Test failed: GetRawInputBuffer returned deadbeef input.c:2399: Test failed: Unexpected rawinput data: 0 input.c:2412: Test failed: Spurious WM_INPUT messages

June 13, 2022

Re: [PATCH 3/4] win32u: Move NtUserGetRawInputData from user32.

by Marvin

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=116821 Your paranoid android. === debian11 (32 bit report) === user32: input.c:2349: Test failed: GetRawInputBuffer succeeded input.c:2350: Test failed: GetRawInputBuffer returned unexpected size: 0 input.c:2351: Test failed: GetRawInputBuffer returned deadbeef input.c:2356: Test failed: GetRawInputBuffer returned unexpected size: 0 input.c:2384: Test failed: GetRawInputBuffer returned 0 input.c:2385: Test failed: GetRawInputBuffer returned unexpected size: 0 input.c:2386: Test failed: Unexpected rawinput data: 0 input.c:2397: Test failed: GetRawInputBuffer succeeded input.c:2398: Test failed: GetRawInputBuffer returned deadbeef input.c:2399: Test failed: Unexpected rawinput data: 0 input.c:2412: Test failed: Spurious WM_INPUT messages === debian11 (32 bit Chinese:China report) === user32: input.c:2349: Test failed: GetRawInputBuffer succeeded input.c:2350: Test failed: GetRawInputBuffer returned unexpected size: 0 input.c:2351: Test failed: GetRawInputBuffer returned deadbeef input.c:2356: Test failed: GetRawInputBuffer returned unexpected size: 0 input.c:2384: Test failed: GetRawInputBuffer returned 0 input.c:2385: Test failed: GetRawInputBuffer returned unexpected size: 0 input.c:2386: Test failed: Unexpected rawinput data: 0 input.c:2397: Test failed: GetRawInputBuffer succeeded input.c:2398: Test failed: GetRawInputBuffer returned deadbeef input.c:2399: Test failed: Unexpected rawinput data: 0 input.c:2412: Test failed: Spurious WM_INPUT messages input.c:2870: Test failed: 15: expected WM_INPUT message input.c:2873: Test failed: 15: expected RIM_INPUT message === debian11 (32 bit WoW report) === user32: input.c:2349: Test failed: GetRawInputBuffer succeeded input.c:2350: Test failed: GetRawInputBuffer returned unexpected size: 0 input.c:2351: Test failed: GetRawInputBuffer returned deadbeef input.c:2356: Test failed: GetRawInputBuffer returned unexpected size: 0 input.c:2384: Test failed: GetRawInputBuffer returned 0 input.c:2385: Test failed: GetRawInputBuffer returned unexpected size: 0 input.c:2386: Test failed: Unexpected rawinput data: 0 input.c:2397: Test failed: GetRawInputBuffer succeeded input.c:2398: Test failed: GetRawInputBuffer returned deadbeef input.c:2399: Test failed: Unexpected rawinput data: 0 input.c:2412: Test failed: Spurious WM_INPUT messages win.c:11770: Test succeeded inside todo block: child should be topmost win.c:11773: Test failed: grandchild should be topmost win.c:11781: Test failed: child should NOT be topmost win.c:11784: Test succeeded inside todo block: grandchild should NOT be topmost win.c:11787: Test failed: 011D00CE: expected NOT topmost win.c:11606: Test succeeded inside todo block: 5: hwnd 014E0054 is still topmost === debian11 (64 bit WoW report) === user32: input.c:2349: Test failed: GetRawInputBuffer succeeded input.c:2350: Test failed: GetRawInputBuffer returned unexpected size: 0 input.c:2351: Test failed: GetRawInputBuffer returned deadbeef input.c:2356: Test failed: GetRawInputBuffer returned unexpected size: 0 input.c:2384: Test failed: GetRawInputBuffer returned 0 input.c:2385: Test failed: GetRawInputBuffer returned unexpected size: 0 input.c:2386: Test failed: Unexpected rawinput data: 0 input.c:2397: Test failed: GetRawInputBuffer succeeded input.c:2398: Test failed: GetRawInputBuffer returned deadbeef input.c:2399: Test failed: Unexpected rawinput data: 0 input.c:2412: Test failed: Spurious WM_INPUT messages

June 13, 2022

Re: [PATCH v2 4/5] win32u: Move GetDialogBaseUnits implementation from user32.

by Marvin

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=116817 Your paranoid android. === debian11 (32 bit Chinese:China report) === user32: msg.c:6881: Test failed: SetFocus(hwnd) on a button: 4: the msg 0x0007 was expected, but got msg 0x0005 instead msg.c:6881: Test failed: SetFocus(hwnd) on a button: 5: the msg 0x0138 was expected, but got msg 0x030f instead msg.c:6881: Test failed: SetFocus(hwnd) on a button: 6: the msg 0x0111 was expected, but got msg 0x001c instead msg.c:6881: Test failed: SetFocus(hwnd) on a button: 8: the msg 0x8000 was expected, but got msg 0x0086 instead msg.c:6881: Test failed: SetFocus(hwnd) on a button: 9: the msg sequence is not complete: expected 0000 - actual 0006

June 13, 2022

Re: [PATCH v2 3/5] win32u: Move WM_NCACTIVATE implementation from user32.

by Marvin

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=116816 Your paranoid android. === debian11 (32 bit report) === user32: win.c:11770: Test succeeded inside todo block: child should be topmost win.c:11773: Test failed: grandchild should be topmost win.c:11781: Test failed: child should NOT be topmost win.c:11784: Test succeeded inside todo block: grandchild should NOT be topmost win.c:11787: Test failed: 007200BE: expected NOT topmost win.c:11606: Test succeeded inside todo block: 5: hwnd 013F0054 is still topmost

June 13, 2022