[PATCH 0/3] MR1876: d3d9/tests: Fix a few more test failures on Gitlab CI.
From: Alexandre Julliard <julliard(a)winehq.org> It fails on the Gitlab CI. --- dlls/d3d9/tests/visual.c | 1 + 1 file changed, 1 insertion(+) diff --git a/dlls/d3d9/tests/visual.c b/dlls/d3d9/tests/visual.c index ccfa8ce5e3a..d85be0ee2b2 100644 --- a/dlls/d3d9/tests/visual.c +++ b/dlls/d3d9/tests/visual.c @@ -26608,6 +26608,7 @@ static void test_mismatched_sample_types(void) colour = getPixelColor(device, 320, 240); + todo_wine_if(!color_match(colour, tests[i].expected_colour, 1)) ok(color_match(colour, tests[i].expected_colour, 1) || broken(tests[i].expected_broken && color_match(colour, tests[i].expected_broken, 1)) || broken(tests[i].expected_broken2 && color_match(colour, tests[i].expected_broken2, 1)), -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1876
From: Alexandre Julliard <julliard(a)winehq.org> Split the check_rt_color() function to allow conditional todos. --- dlls/d3d9/tests/visual.c | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/dlls/d3d9/tests/visual.c b/dlls/d3d9/tests/visual.c index d85be0ee2b2..fed70a675e1 100644 --- a/dlls/d3d9/tests/visual.c +++ b/dlls/d3d9/tests/visual.c @@ -258,9 +258,7 @@ static DWORD getPixelColor(IDirect3DDevice9 *device, UINT x, UINT y) return ret; } -#define check_rt_color(a, b) check_rt_color_(__LINE__, a, b, false) -#define check_rt_color_todo(a, b) check_rt_color_(__LINE__, a, b, true) -static void check_rt_color_(unsigned int line, IDirect3DSurface9 *rt, D3DCOLOR expected_color, bool todo) +static D3DCOLOR check_expected_rt_color(unsigned int line, IDirect3DSurface9 *rt, D3DCOLOR expected_color) { unsigned int color = 0xdeadbeef; struct surface_readback rb; @@ -284,6 +282,15 @@ static void check_rt_color_(unsigned int line, IDirect3DSurface9 *rt, D3DCOLOR e break; } release_surface_readback(&rb); + return color; +} + +#define check_rt_color(a, b) check_rt_color_(__LINE__, a, b, false) +#define check_rt_color_todo(a, b) check_rt_color_(__LINE__, a, b, true) +static void check_rt_color_(unsigned int line, IDirect3DSurface9 *rt, D3DCOLOR expected_color, bool todo) +{ + unsigned int color = check_expected_rt_color(line, rt, expected_color); + todo_wine_if (todo) ok_(__FILE__, line)(color == expected_color, "Got unexpected color 0x%08x.\n", color); } @@ -26913,7 +26920,12 @@ static void test_sample_attached_rendertarget(void) if (is_warp || color == 0x00010101) skip("Sampling attached render targets is not supported.\n"); else - check_rt_color(rt, 0x00c1c1c1); + { + unsigned int expected_color = 0x00c1c1c1; + unsigned int color = check_expected_rt_color(__LINE__, rt, expected_color); + todo_wine_if(color != expected_color) + ok(color == expected_color, "Got unexpected color 0x%08x.\n", color); + } IDirect3DQuery9_Release(event_query); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1876
From: Alexandre Julliard <julliard(a)winehq.org> By backporting the corresponding window management tweaks from the d3d8:device test. --- dlls/d3d9/tests/device.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/dlls/d3d9/tests/device.c b/dlls/d3d9/tests/device.c index 5c4417a2568..858e766c13a 100644 --- a/dlls/d3d9/tests/device.c +++ b/dlls/d3d9/tests/device.c @@ -4294,7 +4294,6 @@ static void test_wndproc(void) SetForegroundWindow(GetDesktopWindow()); ShowWindow(device_window, SW_MINIMIZE); - ShowWindow(device_window, SW_RESTORE); ShowWindow(focus_window, SW_MINIMIZE); ShowWindow(focus_window, SW_RESTORE); SetForegroundWindow(focus_window); @@ -4339,7 +4338,6 @@ static void test_wndproc(void) } filter_messages = NULL; SetForegroundWindow(focus_window); /* For KDE. */ - flush_events(); expect_messages = focus_loss_messages_filtered; windowposchanged_received = 0; @@ -4416,6 +4414,10 @@ static void test_wndproc(void) ref = IDirect3DDevice9_Release(device); ok(!ref, "Unexpected refcount %lu, i=%u.\n", ref, i); + ShowWindow(device_window, SW_RESTORE); + SetForegroundWindow(focus_window); + flush_events(); + filter_messages = focus_window; device_desc.device_window = device_window; if (!(device = create_device(d3d9, focus_window, &device_desc))) @@ -4776,6 +4778,7 @@ static void test_reset_fullscreen(void) skip("Unable to create device. Skipping test.\n"); goto cleanup; } + ShowWindow( device_window, SW_SHOWNORMAL ); /* make sure WM_SIZE has been processed */ /* * Switch to fullscreen mode. -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1876
Stefan Dösinger (@stefan) commented about dlls/d3d9/tests/device.c:
} filter_messages = NULL; SetForegroundWindow(focus_window); /* For KDE. */ - flush_events();
I find it suspicious that removing a flush fixes anything, but I don't have a better solution and my suspicions shouldn't hinder Gitlab CI... -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1876#note_20025
Patch 2/3 isn't my favourite, but I don't immediately see a better option, so I'll sign off on it for now. [Of course, for that matter, I don't like the todo_wine_ifs anyway, since they make the tests useless, but the same deal applies there. Better to sign off now so that we can at least run the tests on llvmpipe, I guess...] Anyway, patch 3/3:
@@ -4294,7 +4294,6 @@ static void test_wndproc(void)
SetForegroundWindow(GetDesktopWindow()); ShowWindow(device_window, SW_MINIMIZE); - ShowWindow(device_window, SW_RESTORE); ShowWindow(focus_window, SW_MINIMIZE); ShowWindow(focus_window, SW_RESTORE); SetForegroundWindow(focus_window);
This was introduced with a mismatch between d3d8 and d3d9 in f4d520d6e2e. Matching the two of them seems fine regardless, but I also don't understand why the minimize/restore is there in the first place. Stefan, I don't suppose you can elucidate?
@@ -4339,7 +4338,6 @@ static void test_wndproc(void) } filter_messages = NULL; SetForegroundWindow(focus_window); /* For KDE. */ - flush_events();
expect_messages = focus_loss_messages_filtered; windowposchanged_received = 0;
The KDE workaround including flush_events() was introduced with the test. As discussed in merge request 1861 [1] it was missing from the d3d8 tests when it was introduced. It is indeed disturbing that removing a flush_events() call fixes the tests, but I tested with KDE and it at least doesn't make anything worse, so I'll sign off on this part.
@@ -4416,6 +4414,10 @@ static void test_wndproc(void) ref = IDirect3DDevice9_Release(device); ok(!ref, "Unexpected refcount %lu, i=%u.\n", ref, i);
+ ShowWindow(device_window, SW_RESTORE); + SetForegroundWindow(focus_window); + flush_events(); + filter_messages = focus_window; device_desc.device_window = device_window; if (!(device = create_device(d3d9, focus_window, &device_desc)))
This one is (thankfully) explained in [2], committed as 90c7e551903, but the corresponding change was never applied to d3d9 in 04d4584dea. All three of the above changes should probably be applied to d3d9ex as well, although that could be relegated to a separate patch series anyway.
@@ -4776,6 +4778,7 @@ static void test_reset_fullscreen(void) skip("Unable to create device. Skipping test.\n"); goto cleanup; } + ShowWindow( device_window, SW_SHOWNORMAL ); /* make sure WM_SIZE has been processed */
/* * Switch to fullscreen mode.
This test isn't part of d3d8, contrary to the commit message. The test also explicitly states that switching to fullscreen mode should cause the window to be shown, so this would seem to be making the test less interesting. I also don't see how ShowWindow() is supposed to make sure WM_SIZE is processed? Why not flush_events() instead? [1] https://gitlab.winehq.org/wine/wine/-/merge_requests/1861 [2] https://www.winehq.org/pipermail/wine-devel/2017-November/119627.html -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1876#note_20074
This was introduced with a mismatch between d3d8 and d3d9 in f4d520d6 <https://gitlab.winehq.org/wine/wine/-/commit/f4d520d6e2e3d486fa691eefee07fecc0ba6fe6c>. Matching the two of them seems fine regardless, but I also don't understand why the minimize/restore is there in the first place. Stefan, I don't suppose you can elucidate?
It's a long time since I wrote those tests, so my memory might be off... I vaguely remember that minimizing and restoring a window allowed foreground stealing on Windows. Afaiu on an older testbot version, those tests ran from a shell that wasn't the foreground window. A pile of minimize and restore sequences got it working. Also d3d8 and d3d9 behaved (behaved?) differently when your process wasn't foreground. Native d3d9 magically allowed us to call SetForegroundWindow even though our parent wasn't the foreground process. My guess is it called AllowSetForegroundWindow. I once experimented with this and tried to get the tests to pass with builtin d3d9 (+wined3d) on windows, but wasn't entirely successful. So d3d8 vs d3d9 differences aren't entirely unexplainable. Finally I had to pacify Alexandre's focus-follows-mouse fvwm2. It is a bit hard to write focus tests when a mouse pointer, left behind at a random location, will override the tests's choice of foreground window. fvwm2 in click-to-focus was about the sanest WM I tested this on. focus-follows-mouse is hell. Afair I fixed this with a mix of testing if my SetForegroundWindow actually did what I wanted it to do and some more redundant minimize-restore in the hope of fighting fvwm2 to a draw. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1876#note_20084
participants (4)
-
Alexandre Julliard -
Alexandre Julliard (＠julliard)
-
Stefan Dösinger
-
Zebediah Figura (＠zfigura)