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