First change is to avoid creating and mapping windows concurrently, this is completely racy and can cause random results later on depending on which window ends up being focused and depending on the X11 events. In addition, FVWM focus tracking suffers from various race conditions already, which only makes it worse.
Second change is to avoid failing the tests because of spurious window resize messages on the dummy window. I'm not sure what the window is for exactly but I believe it's not meaningful in the message sequence as it never appears there. As it is maximized, the window is sometimes resized by some WMs when the display mode changes, which cause the spurious WM_SIZE messages.
From: Rémi Bernon rbernon@codeweavers.com
This causes a foreground window race condition and will cause spurious failures depending on how pending X11 events are processed concurrenly.
Some tests now fail consistently and more todo_wine are needed. --- dlls/d3d9/tests/d3d9ex.c | 81 +++++++++++++++------------------------- 1 file changed, 30 insertions(+), 51 deletions(-)
diff --git a/dlls/d3d9/tests/d3d9ex.c b/dlls/d3d9/tests/d3d9ex.c index e946b960055..424f18073e1 100644 --- a/dlls/d3d9/tests/d3d9ex.c +++ b/dlls/d3d9/tests/d3d9ex.c @@ -2508,7 +2508,6 @@ struct wndproc_thread_param HWND dummy_window; HANDLE window_created; HANDLE test_finished; - BOOL running_in_foreground; };
static LRESULT CALLBACK test_proc(HWND hwnd, UINT message, WPARAM wparam, LPARAM lparam) @@ -2584,7 +2583,7 @@ static DWORD WINAPI wndproc_thread(void *param) p->dummy_window = CreateWindowA("d3d9_test_wndproc_wc", "d3d9_test", WS_MAXIMIZE | WS_VISIBLE | WS_CAPTION, 0, 0, registry_mode.dmPelsWidth, registry_mode.dmPelsHeight, 0, 0, 0, 0); - p->running_in_foreground = SetForegroundWindow(p->dummy_window); + flush_events();
ret = SetEvent(p->window_created); ok(ret, "SetEvent failed, last error %#lx.\n", GetLastError()); @@ -2868,11 +2867,14 @@ static void test_wndproc(void) WS_MAXIMIZE | WS_VISIBLE | WS_CAPTION, 0, 0, user32_width, user32_height, 0, 0, 0, 0); device_window = CreateWindowA("d3d9_test_wndproc_wc", "d3d9_test", WS_MAXIMIZE | WS_VISIBLE | WS_CAPTION, 0, 0, user32_width, user32_height, 0, 0, 0, 0); + flush_events(); + thread = CreateThread(NULL, 0, wndproc_thread, &thread_params, 0, &tid); ok(!!thread, "Failed to create thread, last error %#lx.\n", GetLastError());
res = WaitForSingleObject(thread_params.window_created, INFINITE); ok(res == WAIT_OBJECT_0, "Wait failed (%#lx), last error %#lx.\n", res, GetLastError()); + flush_events();
proc = GetWindowLongPtrA(device_window, GWLP_WNDPROC); ok(proc == (LONG_PTR)test_proc, "Expected wndproc %#Ix, got %#Ix.\n", @@ -2885,15 +2887,10 @@ static void test_wndproc(void) device_window, focus_window, thread_params.dummy_window);
tmp = GetFocus(); - ok(tmp == device_window, "Expected focus %p, got %p.\n", device_window, tmp); - if (thread_params.running_in_foreground) - { - tmp = GetForegroundWindow(); - ok(tmp == thread_params.dummy_window, "Expected foreground window %p, got %p.\n", - thread_params.dummy_window, tmp); - } - else - skip("Not running in foreground, skip foreground window test\n"); + ok(!tmp, "Expected focus %p, got %p.\n", device_window, tmp); + tmp = GetForegroundWindow(); + ok(tmp == thread_params.dummy_window, "Expected foreground window %p, got %p.\n", + thread_params.dummy_window, tmp);
flush_events();
@@ -2913,13 +2910,10 @@ static void test_wndproc(void) expect_messages->message, expect_messages->window, i); expect_messages = NULL;
- if (0) /* Disabled until we can make this work in a reliable way on Wine. */ - { - tmp = GetFocus(); - ok(tmp == focus_window, "Expected focus %p, got %p.\n", focus_window, tmp); - tmp = GetForegroundWindow(); - ok(tmp == focus_window, "Expected foreground window %p, got %p.\n", focus_window, tmp); - } + tmp = GetFocus(); + ok(tmp == focus_window, "Expected focus %p, got %p.\n", focus_window, tmp); + tmp = GetForegroundWindow(); + ok(tmp == focus_window, "Expected foreground window %p, got %p.\n", focus_window, tmp); SetForegroundWindow(focus_window); flush_events();
@@ -3027,28 +3021,12 @@ static void test_wndproc(void)
flush_events();
- /* Openbox accidentally sets focus to the device window, causing WM_ACTIVATEAPP to be sent to the focus - * window. d3d9ex then restores the screen mode. This only happens in the D3DCREATE_NOWINDOWCHANGES case. - * - * This appears to be a race condition - it goes away if openbox is started with --sync. d3d9:device and - * d3d8:device are affected too, but because in their case d3d does not automatically restore the screen - * mode (it needs a call to device::Reset), the EnumDisplaySettings check succeeds regardless. - * - * Note that this is not a case of focus follows mouse. This happens when Openbox is configured to use - * click to focus too. */ - if (GetForegroundWindow() == device_window) - { - skip("WM set focus to the device window, not checking screen mode.\n"); - } - else - { - ret = EnumDisplaySettingsW(NULL, ENUM_CURRENT_SETTINGS, &devmode); - ok(ret, "Failed to get display mode.\n"); - ok(devmode.dmPelsWidth == registry_mode.dmPelsWidth, - "Got unexpected width %lu.\n", devmode.dmPelsWidth); - ok(devmode.dmPelsHeight == registry_mode.dmPelsHeight, - "Got unexpected height %lu.\n", devmode.dmPelsHeight); - } + ret = EnumDisplaySettingsW(NULL, ENUM_CURRENT_SETTINGS, &devmode); + ok(ret, "Failed to get display mode.\n"); + ok(devmode.dmPelsWidth == registry_mode.dmPelsWidth, + "Got unexpected width %lu.\n", devmode.dmPelsWidth); + ok(devmode.dmPelsHeight == registry_mode.dmPelsHeight, + "Got unexpected height %lu.\n", devmode.dmPelsHeight);
/* SW_SHOWMINNOACTIVE is needed to make FVWM happy. SW_SHOWNOACTIVATE is needed to make windows * send SIZE_RESTORED after ShowWindow(SW_SHOWMINNOACTIVE). */ @@ -3165,6 +3143,7 @@ static void test_wndproc(void)
expect_messages = sc_maximize_messages; SendMessageA(focus_window, WM_SYSCOMMAND, SC_MAXIMIZE, 0); + todo_wine_if(tests[i].create_flags & CREATE_DEVICE_NOWINDOWCHANGES) ok(!expect_messages->message, "Expected message %#x for window %#x, but didn't receive it, i=%u.\n", expect_messages->message, expect_messages->window, i); expect_messages = NULL; @@ -3207,6 +3186,7 @@ static void test_wndproc(void) * it is clear that the window has not been resized. In previous Windows version the window is resized. */
flush_events(); + todo_wine_if(!(tests[i].create_flags & CREATE_DEVICE_NOWINDOWCHANGES)) ok(!expect_messages->message, "Expected message %#x for window %#x, but didn't receive it, i=%u.\n", expect_messages->message, expect_messages->window, i); expect_messages = NULL; @@ -3227,12 +3207,14 @@ static void test_wndproc(void) filter_messages = NULL;
flush_events(); + todo_wine_if(!(tests[i].create_flags & CREATE_DEVICE_NOWINDOWCHANGES)) ok(!expect_messages->message, "Expected message %#x for window %#x, but didn't receive it, i=%u.\n", expect_messages->message, expect_messages->window, i); expect_messages = NULL;
if (!(tests[i].create_flags & CREATE_DEVICE_NOWINDOWCHANGES)) { + todo_wine_if(!(tests[i].create_flags & CREATE_DEVICE_NOWINDOWCHANGES)) ok(windowpos.hwnd == device_window && !windowpos.x && !windowpos.y && !windowpos.cx && !windowpos.cy && windowpos.flags == (SWP_SHOWWINDOW | SWP_NOMOVE | SWP_NOSIZE), @@ -3311,11 +3293,14 @@ static void test_wndproc_windowed(void) device_window = CreateWindowA("d3d9_test_wndproc_wc", "d3d9_test", WS_MAXIMIZE | WS_VISIBLE | WS_CAPTION, 0, 0, registry_mode.dmPelsWidth, registry_mode.dmPelsHeight, 0, 0, 0, 0); + flush_events(); + thread = CreateThread(NULL, 0, wndproc_thread, &thread_params, 0, &tid); ok(!!thread, "Failed to create thread, last error %#lx.\n", GetLastError());
res = WaitForSingleObject(thread_params.window_created, INFINITE); ok(res == WAIT_OBJECT_0, "Wait failed (%#lx), last error %#lx.\n", res, GetLastError()); + flush_events();
proc = GetWindowLongPtrA(device_window, GWLP_WNDPROC); ok(proc == (LONG_PTR)test_proc, "Expected wndproc %#Ix, got %#Ix.\n", @@ -3328,16 +3313,10 @@ static void test_wndproc_windowed(void) device_window, focus_window, thread_params.dummy_window);
tmp = GetFocus(); - ok(tmp == device_window, "Expected focus %p, got %p.\n", device_window, tmp); - if (thread_params.running_in_foreground) - { - tmp = GetForegroundWindow(); - flaky - ok(tmp == thread_params.dummy_window, "Expected foreground window %p, got %p.\n", - thread_params.dummy_window, tmp); - } - else - skip("Not running in foreground, skip foreground window test\n"); + ok(!tmp, "Expected focus %p, got %p.\n", device_window, tmp); + tmp = GetForegroundWindow(); + ok(tmp == thread_params.dummy_window, "Expected foreground window %p, got %p.\n", + thread_params.dummy_window, tmp);
filter_messages = focus_window;
@@ -3352,7 +3331,7 @@ static void test_wndproc_windowed(void) }
tmp = GetFocus(); - ok(tmp == device_window, "Expected focus %p, got %p.\n", device_window, tmp); + ok(!tmp, "Expected focus %p, got %p.\n", device_window, tmp); tmp = GetForegroundWindow(); ok(tmp == thread_params.dummy_window, "Expected foreground window %p, got %p.\n", thread_params.dummy_window, tmp);
From: Rémi Bernon rbernon@codeweavers.com
--- dlls/d3d9/tests/d3d9ex.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/dlls/d3d9/tests/d3d9ex.c b/dlls/d3d9/tests/d3d9ex.c index 424f18073e1..28a32301768 100644 --- a/dlls/d3d9/tests/d3d9ex.c +++ b/dlls/d3d9/tests/d3d9ex.c @@ -2580,7 +2580,7 @@ static DWORD WINAPI wndproc_thread(void *param) DWORD res; BOOL ret;
- p->dummy_window = CreateWindowA("d3d9_test_wndproc_wc", "d3d9_test", + p->dummy_window = CreateWindowA("static", "d3d9_test", WS_MAXIMIZE | WS_VISIBLE | WS_CAPTION, 0, 0, registry_mode.dmPelsWidth, registry_mode.dmPelsHeight, 0, 0, 0, 0); flush_events(); @@ -3007,9 +3007,6 @@ static void test_wndproc(void) SetForegroundWindow(GetDesktopWindow()); ok(!expect_messages->message, "Expected message %#x for window %#x, but didn't receive it, i=%u.\n", expect_messages->message, expect_messages->window, i); - - /* kwin sometimes resizes hidden windows. */ - flaky ok(!windowposchanged_received, "Received WM_WINDOWPOSCHANGED but did not expect it, i=%u.\n", i);
expect_messages = NULL; @@ -3103,9 +3100,6 @@ static void test_wndproc(void) flaky_wine ok(!expect_messages->message, "Expected message %#x for window %#x, but didn't receive it, i=%u.\n", expect_messages->message, expect_messages->window, i); - - /* kwin and Win8+ sometimes resize hidden windows. */ - flaky ok(!windowposchanged_received, "Received WM_WINDOWPOSCHANGED but did not expect it, i=%u.\n", i);
expect_messages = NULL;
Henri, you added the dummy window tests, is there a chance you can explain what their purpose was? Do we care about the messages it receives?
In any case, 2/2 needs the explanation in the patch itself. Also, shouldn't these changes be ported to d3d8:device and d3d9:device as well?
Henri, you added the dummy window tests, is there a chance you can explain what their purpose was? Do we care about the messages it receives?
IIRC the main point was to show/verify that creating a fullscreen Direct3D device makes the focus window the foreground window, even if the current foreground window belongs to a different thread. I.e., essentially commit 0838d0ab6491bdc0fb32d0b8715fbb57088b926a.
I don't think we care all that much about what messages the dummy window does or doesn't receive, although the "windowposchanged_received" bit introduced in 36553d862b539a95b71ec6fcea19f328e1562604 should probably take "hwnd" into account in any case. I think that's true for "syscommand_received" (and "wm_size_received" in the d3d9 version) as well.
Note that the window wasn't intended to be maximized either; that seems to have been introduced by 766e732fb37cf0c6dabd895f56506e5e4feb10cd. IIRC the idea there was to avoid resizing the device/focus window if it already matched the backbuffer size, but that doesn't really apply to the dummy window here. It should be fine to use e.g. a 100x100 dummy window as well.
-
Elizabeth Figura (＠zfigura) -
Henri Verbeet (＠hverbeet)
-
Rémi Bernon
-
Rémi Bernon (＠rbernon)