[PATCH 0/3] MR3746: test fixes for fvwm3
These tests make the ddraw-d3d9 tests pass inside Xephyr with fvwm3 for me. fvwm3 still randomly fails in a lot of places if it is run on a real display. It also fails in Xephyr if I wiggle the mouse too much. Those two issues might even be related. A virtual X server should be enough for CI though. fvwm2 has a lot of random failures even inside Xephyr or xf86-video-dummy. I will not attempt to fix them. I tested the SW_SHOWMINNOACTIVE in patches 2 and 3 on Windows XP, Windows 11, the testbot, kwin, openbox. It seems to work without breaking any of those systems. With those things being flaky changes though I can never fully gurantee this though. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3746
From: Stefan Dösinger <stefan(a)codeweavers.com> fvwm does that for two independent reasons. The important thing is that we don't receive a WM_SIZE during the reset call itself. --- dlls/d3d9/tests/device.c | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/dlls/d3d9/tests/device.c b/dlls/d3d9/tests/device.c index 8284e3365c8..d36b76156f5 100644 --- a/dlls/d3d9/tests/device.c +++ b/dlls/d3d9/tests/device.c @@ -4757,6 +4757,7 @@ static void test_reset_fullscreen(void) IDirect3DDevice9 *device; WNDCLASSEXA wc = {0}; IDirect3D9 *d3d; + RECT r1, r2; HRESULT hr; ATOM atom; static const struct message messages[] = @@ -4835,7 +4836,35 @@ static void test_reset_fullscreen(void) device_desc.flags = CREATE_DEVICE_FULLSCREEN; ok(SUCCEEDED(reset_device(device, &device_desc)), "Failed to reset device.\n"); + /* We shouldn't receive a WM_SIZE message during the reset because d3d filters + * messages sent to the focus window. This is important because some games + * respond to WM_SIZE messages by calling Reset(), resulting in an endless + * recursion. */ + ok(!wm_size_received, "Received unexpected WM_SIZE message.\n"); + GetWindowRect(device_window, &r1); + flush_events(); + GetWindowRect(device_window, &r2); + + /* fvwm2 and 3 resize the window though. We learn about this when processing + * events, after Reset() has finished and d3d is no longer filtering window + * messages. This still shouldn't happen but won't lead to an endless recursion. + * + * As far as I understand the fvwm3 source code, the WM doesn't expect a window + * to change from size A maximized to size B maximized. It will un-maximize the + * window, picking the stored normal size - which appens to be the size of the + * first fullscreen mode above, but not necessarily the screen size at startup. + * + * fvwm2 on the other hand doesn't understand mode switches at all. It will see + * the window is fullscreen (it has separate flags for fullscreen and maximized), + * and try to resize it to what it thinks is the display mode - which is the mode + * that fvwm2 was started with. The above fvwm3 bug also exists in fvwm2 but is + * hidden by the mode bug. + * + * This comment is based on a very superficial understanding of fvwm's event + * and window dimension handling code, which is about 10k lines of code. So it + * may be wrong. */ + todo_wine_if(!EqualRect(&r1, &r2)) ok(!wm_size_received, "Received unexpected WM_SIZE message.\n"); cleanup: -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3746
From: Stefan Dösinger <stefan(a)codeweavers.com> --- d3d9ex doesn't need a similar change because it doesn't check those messages in the first place. --- dlls/d3d9/tests/device.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/dlls/d3d9/tests/device.c b/dlls/d3d9/tests/device.c index d36b76156f5..c972e97f572 100644 --- a/dlls/d3d9/tests/device.c +++ b/dlls/d3d9/tests/device.c @@ -4230,9 +4230,15 @@ static void test_wndproc(void) /* I have to minimize and restore the focus window, otherwise native d3d9 fails * device::reset with D3DERR_DEVICELOST. This does not happen when the window - * restore is triggered by the user. */ + * restore is triggered by the user. + * + * fvwm randomly sends a focus loss notification when we minimize, so do it + * before checking the incoming messages. It might match WM_ACTIVATEAPP but has + * a wrong WPARAM. Use SW_SHOWMINNOACTIVE to make sure we don't accidentally + * activate the window at this point and miss our WM_ACTIVATEAPP(wparam=1). */ + ShowWindow(focus_window, SW_SHOWMINNOACTIVE); + flush_events(); expect_messages = tests[i].reactivate_messages; - ShowWindow(focus_window, SW_MINIMIZE); ShowWindow(focus_window, SW_RESTORE); /* Set focus twice to make KDE and fvwm in focus-follows-mouse mode happy. */ SetForegroundWindow(focus_window); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3746
From: Stefan Dösinger <stefan(a)codeweavers.com> --- dlls/d3d8/tests/device.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/dlls/d3d8/tests/device.c b/dlls/d3d8/tests/device.c index ec6f2ce51b3..b61ad207cc9 100644 --- a/dlls/d3d8/tests/device.c +++ b/dlls/d3d8/tests/device.c @@ -3203,9 +3203,15 @@ static void test_wndproc(void) /* I have to minimize and restore the focus window, otherwise native d3d8 fails * device::reset with D3DERR_DEVICELOST. This does not happen when the window - * restore is triggered by the user. */ + * restore is triggered by the user. + * + * fvwm randomly sends a focus loss notification when we minimize, so do it + * before checking the incoming messages. It might match WM_ACTIVATEAPP but has + * a wrong WPARAM. Use SW_SHOWMINNOACTIVE to make sure we don't accidentally + * activate the window at this point and miss our WM_ACTIVATEAPP(wparam=1). */ + ShowWindow(focus_window, SW_SHOWMINNOACTIVE); + flush_events(); expect_messages = reactivate_messages; - ShowWindow(focus_window, SW_MINIMIZE); ShowWindow(focus_window, SW_RESTORE); /* Set focus twice to make KDE and fvwm in focus-follows-mouse mode happy. */ SetForegroundWindow(focus_window); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3746
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=137072 Your paranoid android. === w10pro64 (64 bit report) === d3d9: device.c:4159: Test failed: Expected message 0x46 for window 0x1, but didn't receive it, i=0. device.c:4203: Test failed: Expected message 0x7e for window 0, but didn't receive it, i=0. device.c:4207: Test failed: The device window is active, i=0. device.c:4212: Test failed: Got unexpected hr 0x88760869. device.c:4216: Test failed: Got unexpected screen size 800x600. device.c:4248: Test failed: Expected message 0x46 for window 0, but didn't receive it, i=0. device.c:4257: Test failed: Got unexpected screen size 800x600. device.c:4274: Test failed: Expected message 0x7e for window 0, but didn't receive it, i=0. device.c:4282: Test failed: Got unexpected width 640. device.c:4283: Test failed: Got unexpected height 480. device.c:4364: Test failed: Expected message 0x7e for window 0x1, but didn't receive it, i=0. device.c:4371: Test failed: Expected IsIconic 1, got 0, i=0. device.c:4375: Test failed: Got unexpected hr 0. device.c:4383: Test failed: Expected message 0x46 for window 0x1, but didn't receive it, i=0. device.c:4421: Test failed: Expected message 0x1c for window 0x1, but didn't receive it, i=0. device.c:4431: Test failed: Got unexpected hr 0. device.c:4489: Test failed: Expected message 0x18 for window 0, but didn't receive it, i=0. device.c:4495: Test failed: Got unexpected WINDOWPOS hwnd=0000000000000000, insertAfter=0000000000000000, x=0, y=0, cx=0, cy=0, flags=0 device.c:4512: Test failed: Expected the device window to be visible, i=0. device.c:4159: Test failed: Expected message 0x46 for window 0x1, but didn't receive it, i=1. device.c:4203: Test failed: Expected message 0x7e for window 0, but didn't receive it, i=1. device.c:4207: Test failed: The device window is active, i=1. device.c:4212: Test failed: Got unexpected hr 0x88760869. device.c:4216: Test failed: Got unexpected screen size 800x600. device.c:4257: Test failed: Got unexpected screen size 800x600. device.c:4274: Test failed: Expected message 0x7e for window 0, but didn't receive it, i=1. device.c:4282: Test failed: Got unexpected width 640. device.c:4283: Test failed: Got unexpected height 480. === debian11 (32 bit report) === d3d8: device.c:3221: Test failed: Expected message 0x46 for window 0, but didn't receive it, i=1. === debian11 (32 bit ar:MA report) === d3d8: device.c:3221: Test failed: Expected message 0x46 for window 0, but didn't receive it, i=1. === debian11 (32 bit de report) === d3d8: device.c:3221: Test failed: Expected message 0x46 for window 0, but didn't receive it, i=1. === debian11 (32 bit fr report) === d3d8: device.c:3221: Test failed: Expected message 0x46 for window 0, but didn't receive it, i=1. === debian11 (32 bit he:IL report) === d3d8: device.c:3221: Test failed: Expected message 0x46 for window 0, but didn't receive it, i=1. === debian11 (32 bit hi:IN report) === d3d8: device.c:3221: Test failed: Expected message 0x46 for window 0, but didn't receive it, i=1. === debian11 (32 bit ja:JP report) === d3d8: device.c:3221: Test failed: Expected message 0x46 for window 0, but didn't receive it, i=1. === debian11 (32 bit zh:CN report) === d3d8: device.c:3221: Test failed: Expected message 0x46 for window 0, but didn't receive it, i=1. === debian11b (32 bit WoW report) === d3d8: device.c:3221: Test failed: Expected message 0x46 for window 0, but didn't receive it, i=1. === debian11b (64 bit WoW report) === d3d8: device.c:3221: Test failed: Expected message 0x46 for window 0, but didn't receive it, i=1.
Another thing that is noteworthy about fvwm3 is that it is still affected by the accidental device window restore that randomly broke one test in fvwm2 and that I attempted to address with https://gitlab.winehq.org/wine/wine/-/merge_requests/3565/diffs?commit_id=bd... . However, that window restore did not break the following ok() lines for me, so this commit is not needed. In fact, it is harmful on fvwm3 - the re-minimization causes follow-up issues. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3746#note_44500
fwiw the testbot reported a failure on one of the Windows 10 machines but somehow it didn't end up here: https://testbot.winehq.org/JobDetails.pl?Key=137072&f508=exe64.report#k508 This is the "device.c:4142: Tests skipped: Not running in foreground, skip foreground window test" fluke again. I don't know why it happens on the testbot, but I can reproduce it locally if I run the tests from an msys git bash shell instead of cmd.exe. We had this before, which is why some tests check for this but not all do. I guess those checks need to be updated and added to more tests to avoid this. This is a separate issue though, I don't believe it has anything to do with the changes in this MR. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3746#note_44534
This merge request was approved by Zebediah Figura. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3746
This merge request was approved by Jan Sikorski. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3746
participants (4)
-
Jan Sikorski (＠jsikorski) -
Marvin -
Stefan Dösinger
-
Zebediah Figura (＠zfigura)