[PATCH v5 0/3] MR5060: win32u, explorer, winex11.drv: Restore display settings on process exit.
-- v5: win32u, explorer: Restore display settings on process exit. winex11.drv: Process RRNotify events in xrandr14_get_id. user32/tests: Test that display settings are restored on process exit.
From: Anton Baskanov baskanov@gmail.com
--- dlls/user32/tests/monitor.c | 267 +++++++++++++++++++++++++++++++++--- 1 file changed, 249 insertions(+), 18 deletions(-)
diff --git a/dlls/user32/tests/monitor.c b/dlls/user32/tests/monitor.c index e93a84242c4..425c32c85e2 100644 --- a/dlls/user32/tests/monitor.c +++ b/dlls/user32/tests/monitor.c @@ -291,8 +291,8 @@ struct device_info DEVMODEA original_mode; };
-#define expect_dm(a, b, c) _expect_dm(__LINE__, a, b, c) -static void _expect_dm(INT line, const DEVMODEA *expected, const CHAR *device, DWORD test) +#define expect_dm(a, b, c, d) _expect_dm(__LINE__, a, b, c, d) +static void _expect_dm(INT line, const DEVMODEA *expected, const CHAR *device, DWORD test, BOOL todo) { DEVMODEA dm; BOOL ret; @@ -307,9 +307,9 @@ static void _expect_dm(INT line, const DEVMODEA *expected, const CHAR *device, D "Device %s test %ld expect dmFields to contain %#lx, got %#lx\n", device, test, expected->dmFields, dm.dmFields); ok_(__FILE__, line)(!(expected->dmFields & DM_BITSPERPEL) || dm.dmBitsPerPel == expected->dmBitsPerPel, "Device %s test %ld expect dmBitsPerPel %lu, got %lu\n", device, test, expected->dmBitsPerPel, dm.dmBitsPerPel); - ok_(__FILE__, line)(!(expected->dmFields & DM_PELSWIDTH) || dm.dmPelsWidth == expected->dmPelsWidth, + todo_wine_if(todo) ok_(__FILE__, line)(!(expected->dmFields & DM_PELSWIDTH) || dm.dmPelsWidth == expected->dmPelsWidth, "Device %s test %ld expect dmPelsWidth %lu, got %lu\n", device, test, expected->dmPelsWidth, dm.dmPelsWidth); - ok_(__FILE__, line)(!(expected->dmFields & DM_PELSHEIGHT) || dm.dmPelsHeight == expected->dmPelsHeight, + todo_wine_if(todo) ok_(__FILE__, line)(!(expected->dmFields & DM_PELSHEIGHT) || dm.dmPelsHeight == expected->dmPelsHeight, "Device %s test %ld expect dmPelsHeight %lu, got %lu\n", device, test, expected->dmPelsHeight, dm.dmPelsHeight); ok_(__FILE__, line)(!(expected->dmFields & DM_POSITION) || dm.dmPosition.x == expected->dmPosition.x, "Device %s test %ld expect dmPosition.x %ld, got %ld\n", device, test, expected->dmPosition.x, dm.dmPosition.x); @@ -325,18 +325,81 @@ static void _expect_dm(INT line, const DEVMODEA *expected, const CHAR *device, D dm.dmDisplayOrientation); }
-static void test_ChangeDisplaySettingsEx(void) +#define wait_for_dm(a, b, c, d) wait_for_dm_(__LINE__, a, b, c, d) +static void wait_for_dm_(int line, const char *device, DWORD expectedWidth, DWORD expectedHeight, BOOL todo) +{ + DEVMODEA dm; + BOOL ret; + int i; + + for (i = 0; i < 100; ++i) + { + memset(&dm, 0, sizeof(dm)); + dm.dmSize = sizeof(dm); + SetLastError(0xdeadbeef); + ret = EnumDisplaySettingsA(device, ENUM_CURRENT_SETTINGS, &dm); + ok_(__FILE__, line)(ret, "Device %s EnumDisplaySettingsA failed, error %#lx\n", device, GetLastError()); + ok_(__FILE__, line)((dm.dmFields & (DM_PELSWIDTH | DM_PELSHEIGHT)) == (DM_PELSWIDTH | DM_PELSHEIGHT), + "Device %s expect dmFields to contain %#lx, got %#lx\n", device, DM_PELSWIDTH | DM_PELSHEIGHT, dm.dmFields); + + if (dm.dmPelsWidth == expectedWidth && dm.dmPelsHeight == expectedHeight) + break; + + Sleep(100); + } + + todo_wine_if(todo) ok_(__FILE__, line)(dm.dmPelsWidth == expectedWidth, + "Device %s expect dmPelsWidth %lu, got %lu\n", device, expectedWidth, dm.dmPelsWidth); + todo_wine_if(todo) ok_(__FILE__, line)(dm.dmPelsHeight == expectedHeight, + "Device %s expect dmPelsHeight %lu, got %lu\n", device, expectedHeight, dm.dmPelsHeight); +} + +static HANDLE test_child_process_ChangeDisplaySettingsEx(const char *argv0, const char *device, const char *exit_event_name) +{ + static const char *cds_event_name = "test_child_process_ChangeDisplaySettingsEx_cds_event"; + PROCESS_INFORMATION info; + char buffer[MAX_PATH]; + STARTUPINFOA startup; + DWORD wait_result; + HANDLE cds_event; + LONG res; + + cds_event = CreateEventA(NULL, FALSE, FALSE, cds_event_name); + ok(!!cds_event, "CreateEventA failed, error %#lx\n", GetLastError()); + + memset(&startup, 0, sizeof(startup)); + startup.cb = sizeof(startup); + + snprintf(buffer, sizeof(buffer), "%s monitor fullscreen %s %s %s", argv0, device, + cds_event_name, exit_event_name); + res = CreateProcessA(NULL, buffer, NULL, NULL, FALSE, 0, NULL, NULL, &startup, &info); + ok(res, "CreateProcessA returned unexpected %ld\n", res); + wait_result = WaitForSingleObject(cds_event, 10000); + ok(wait_result == WAIT_OBJECT_0, "WaitForSingleObject returned %lx.\n", wait_result); + + CloseHandle(cds_event); + CloseHandle(info.hThread); + + return info.hProcess; +} + +static void test_ChangeDisplaySettingsEx(int myARGC, char **myARGV) { static const DWORD registry_fields = DM_DISPLAYORIENTATION | DM_BITSPERPEL | DM_PELSWIDTH | DM_PELSHEIGHT | DM_DISPLAYFLAGS | DM_DISPLAYFREQUENCY | DM_POSITION; + static const char *exit_event0_name = "test_ChangeDisplaySettingsEx_exit_event0"; + static const char *exit_event1_name = "test_ChangeDisplaySettingsEx_exit_event1"; static const DWORD depths[] = {8, 16, 32}; DPI_AWARENESS_CONTEXT context = NULL; UINT primary, device, test, mode; + HANDLE exit_event0, exit_event1; UINT device_size, device_count; struct device_info *devices; + HANDLE process0, process1; DEVMODEA dm, dm2, dm3; INT count, old_count; DISPLAY_DEVICEA dd; + DWORD wait_result; POINTL position; DEVMODEW dmW; BOOL found; @@ -668,7 +731,7 @@ static void test_ChangeDisplaySettingsEx(void) continue; } flush_events(); - expect_dm(&dm3, devices[device].name, test); + expect_dm(&dm3, devices[device].name, test, FALSE);
/* Change the registry mode to the second mode */ res = ChangeDisplaySettingsExA(devices[device].name, &dm2, NULL, CDS_UPDATEREGISTRY | CDS_NORESET, NULL); @@ -802,7 +865,7 @@ static void test_ChangeDisplaySettingsEx(void) }
flush_events(); - expect_dm(&dm, devices[device].name, mode); + expect_dm(&dm, devices[device].name, mode, FALSE); }
/* Restore settings */ @@ -875,7 +938,7 @@ static void test_ChangeDisplaySettingsEx(void) }
flush_events(); - expect_dm(&dm, devices[device].name, 0); + expect_dm(&dm, devices[device].name, 0, FALSE);
/* Test specifying only position, width and height */ memset(&dm, 0, sizeof(dm)); @@ -920,7 +983,7 @@ static void test_ChangeDisplaySettingsEx(void) ok(dm.dmBitsPerPel, "Expected dmBitsPerPel not zero.\n"); ok(dm.dmDisplayFrequency, "Expected dmDisplayFrequency not zero.\n");
- expect_dm(&dm, devices[device].name, 0); + expect_dm(&dm, devices[device].name, 0, FALSE); }
/* Test dmPosition */ @@ -992,7 +1055,7 @@ static void test_ChangeDisplaySettingsEx(void) ok(res == DISP_CHANGE_SUCCESSFUL, "ChangeDisplaySettingsExA %s returned unexpected %ld\n", devices[1].name, res);
dm2.dmPosition.x = dm.dmPosition.x + dm.dmPelsWidth; - expect_dm(&dm2, devices[1].name, 0); + expect_dm(&dm2, devices[1].name, 0, FALSE);
/* Test placing the secondary adapter to all sides of the primary adapter */ for (test = 0; test < 8; ++test) @@ -1051,7 +1114,7 @@ static void test_ChangeDisplaySettingsEx(void) }
flush_events(); - expect_dm(&dm2, devices[1].name, test); + expect_dm(&dm2, devices[1].name, test, FALSE); }
/* Test automatic position update when other adapters change resolution */ @@ -1116,7 +1179,7 @@ static void test_ChangeDisplaySettingsEx(void) ok(res == DISP_CHANGE_SUCCESSFUL, "ChangeDisplaySettingsExA %s mode %d returned unexpected %ld.\n", devices[device].name, mode, res); flush_events(); - expect_dm(&dm2, devices[device].name, mode); + expect_dm(&dm2, devices[device].name, mode, FALSE);
/* EnumDisplaySettingsEx without EDS_ROTATEDMODE reports modes with current orientation */ memset(&dm3, 0, sizeof(dm3)); @@ -1162,7 +1225,136 @@ static void test_ChangeDisplaySettingsEx(void) broken(res == DISP_CHANGE_FAILED), /* win8 TestBot */ "ChangeDisplaySettingsExA returned unexpected %ld\n", res); for (device = 0; device < device_count; ++device) - expect_dm(&devices[device].original_mode, devices[device].name, 0); + expect_dm(&devices[device].original_mode, devices[device].name, 0, FALSE); + + exit_event0 = CreateEventA(NULL, FALSE, FALSE, exit_event0_name); + ok(!!exit_event0, "CreateEventA failed, error %#lx\n", GetLastError()); + exit_event1 = CreateEventA(NULL, FALSE, FALSE, exit_event1_name); + ok(!!exit_event1, "CreateEventA failed, error %#lx\n", GetLastError()); + + /* Test that if the most recent ChangeDisplaySettingsEx call had + * CDS_FULLSCREEN set, the the settings are restored when the caller + * process exits */ + + process0 = test_child_process_ChangeDisplaySettingsEx(myARGV[0], devices[0].name, exit_event0_name); + process1 = test_child_process_ChangeDisplaySettingsEx(myARGV[0], devices[0].name, exit_event1_name); + + /* Verify that the settings are restored to the current registry settings */ + for (device = 0; device < device_count; ++device) + { + memset(&dm, 0, sizeof(dm)); + dm.dmSize = sizeof(dm); + dm.dmFields = DM_PELSWIDTH | DM_PELSHEIGHT; + dm.dmPelsWidth = 800; + dm.dmPelsHeight = 600; + res = ChangeDisplaySettingsExA(devices[device].name, &dm, NULL, CDS_UPDATEREGISTRY | CDS_NORESET, NULL); + ok(res == DISP_CHANGE_SUCCESSFUL, "ChangeDisplaySettingsExA %s returned %ld.\n", devices[device].name, res); + } + + SetEvent(exit_event0); + wait_result = WaitForSingleObject(process0, 10000); + ok(wait_result == WAIT_OBJECT_0, "WaitForSingleObject returned %lx.\n", wait_result); + + Sleep(100); + + memset(&dm, 0, sizeof(dm)); + dm.dmSize = sizeof(dm); + dm.dmPelsWidth = 640; + dm.dmPelsHeight = 480; + dm.dmFields = DM_PELSWIDTH | DM_PELSHEIGHT; + expect_dm(&dm, devices[0].name, 0, TRUE); + + SetEvent(exit_event1); + wait_result = WaitForSingleObject(process1, 10000); + ok(wait_result == WAIT_OBJECT_0, "WaitForSingleObject returned %lx.\n", wait_result); + + wait_for_dm(devices[0].name, 800, 600, TRUE); + + CloseHandle(process1); + CloseHandle(process0); + + /* Test processes exiting in reverse order */ + + process0 = test_child_process_ChangeDisplaySettingsEx(myARGV[0], devices[0].name, exit_event0_name); + process1 = test_child_process_ChangeDisplaySettingsEx(myARGV[0], devices[0].name, exit_event1_name); + + SetEvent(exit_event1); + wait_result = WaitForSingleObject(process1, 10000); + ok(wait_result == WAIT_OBJECT_0, "WaitForSingleObject returned %lx.\n", wait_result); + + wait_for_dm(devices[0].name, 800, 600, TRUE); + + SetEvent(exit_event0); + wait_result = WaitForSingleObject(process0, 10000); + ok(wait_result == WAIT_OBJECT_0, "WaitForSingleObject returned %lx.\n", wait_result); + + Sleep(100); + + memset(&dm, 0, sizeof(dm)); + dm.dmSize = sizeof(dm); + dm.dmPelsWidth = 800; + dm.dmPelsHeight = 600; + dm.dmFields = DM_PELSWIDTH | DM_PELSHEIGHT; + expect_dm(&dm, devices[0].name, 0, TRUE); + + CloseHandle(process1); + CloseHandle(process0); + + if (device_count < 2) + { + skip("Only one device found.\n"); + } + else + { + /* Test that the settings are restored for all devices, regardless of + * the process that changed them */ + + process0 = test_child_process_ChangeDisplaySettingsEx(myARGV[0], devices[0].name, exit_event0_name); + process1 = test_child_process_ChangeDisplaySettingsEx(myARGV[0], devices[1].name, exit_event1_name); + + SetEvent(exit_event1); + wait_result = WaitForSingleObject(process1, 10000); + ok(wait_result == WAIT_OBJECT_0, "WaitForSingleObject returned %lx.\n", wait_result); + + wait_for_dm(devices[0].name, 800, 600, TRUE); + wait_for_dm(devices[1].name, 800, 600, TRUE); + + SetEvent(exit_event0); + wait_result = WaitForSingleObject(process0, 10000); + ok(wait_result == WAIT_OBJECT_0, "WaitForSingleObject returned %lx.\n", wait_result); + + Sleep(100); + + memset(&dm, 0, sizeof(dm)); + dm.dmSize = sizeof(dm); + dm.dmPelsWidth = 800; + dm.dmPelsHeight = 600; + dm.dmFields = DM_PELSWIDTH | DM_PELSHEIGHT; + expect_dm(&dm, devices[0].name, 0, TRUE); + expect_dm(&dm, devices[1].name, 0, TRUE); + + CloseHandle(process1); + CloseHandle(process0); + } + + CloseHandle(exit_event1); + CloseHandle(exit_event0); + + /* Restore all adapters to their original settings */ + for (device = 0; device < device_count; ++device) + { + res = ChangeDisplaySettingsExA(devices[device].name, &devices[device].original_mode, NULL, + CDS_UPDATEREGISTRY | CDS_NORESET, NULL); + ok(res == DISP_CHANGE_SUCCESSFUL || + broken(res == DISP_CHANGE_FAILED), /* win8 TestBot */ + "ChangeDisplaySettingsExA %s returned unexpected %ld\n", devices[device].name, res); + } + res = ChangeDisplaySettingsExA(NULL, NULL, NULL, 0, NULL); + ok(res == DISP_CHANGE_SUCCESSFUL || + broken(res == DISP_CHANGE_FAILED), /* win8 TestBot */ + "ChangeDisplaySettingsExA returned unexpected %ld\n", res); + for (device = 0; device < device_count; ++device) + expect_dm(&devices[device].original_mode, devices[device].name, 0, FALSE);
free(devices); } @@ -2670,15 +2862,54 @@ START_TEST(monitor)
init_function_pointers();
- if (myARGC >= 3 && strcmp(myARGV[2], "info") == 0) + if (myARGC >= 3) { - printf("Monitor information:\n"); - EnumDisplayMonitors(NULL, NULL, MonitorEnumProc, 0); - return; + if (strcmp(myARGV[2], "info") == 0) + { + printf("Monitor information:\n"); + EnumDisplayMonitors(NULL, NULL, MonitorEnumProc, 0); + return; + } + else if (strcmp(myARGV[2], "fullscreen") == 0) + { + HANDLE event0, event1; + DWORD wait_result; + DEVMODEA dm; + LONG res; + + if (myARGC < 6) + { + ok(0, "too few arguments.\n"); + return; + } + + event0 = OpenEventA(EVENT_MODIFY_STATE, FALSE, myARGV[4]); + ok(!!event0, "OpenEventA failed, error %#lx\n", GetLastError()); + event1 = OpenEventA(SYNCHRONIZE, FALSE, myARGV[5]); + ok(!!event1, "OpenEventA failed, error %#lx\n", GetLastError()); + + memset(&dm, 0, sizeof(dm)); + dm.dmSize = sizeof(dm); + dm.dmFields = DM_PELSWIDTH | DM_PELSHEIGHT; + dm.dmPelsWidth = 640; + dm.dmPelsHeight = 480; + res = ChangeDisplaySettingsExA(myARGV[3], &dm, NULL, CDS_FULLSCREEN, NULL); + ok(res == DISP_CHANGE_SUCCESSFUL, + "ChangeDisplaySettingsExA %s returned unexpected %ld.\n", myARGV[3], res); + + SetEvent(event0); + CloseHandle(event0); + + wait_result = WaitForSingleObject(event1, 20000); + ok(wait_result == WAIT_OBJECT_0, "WaitForSingleObject returned %lx.\n", wait_result); + CloseHandle(event1); + + return; + } }
test_enumdisplaydevices(); - test_ChangeDisplaySettingsEx(); + test_ChangeDisplaySettingsEx(myARGC, myARGV); test_DisplayConfigSetDeviceInfo(); test_EnumDisplayMonitors(); test_monitors();
From: Anton Baskanov baskanov@gmail.com
We have to invalidate the current mode cache if there are pending RRNotify events. The performance hit on EnumDisplaySettingsExW is around 7%.
Also call X11DRV_DisplayDevices_RegisterEventHandlers in x11drv_init. Otherwise, RRNotify events will only be handled in the explorer process. --- dlls/user32/tests/monitor.c | 2 +- dlls/winex11.drv/event.c | 2 +- dlls/winex11.drv/window.c | 1 - dlls/winex11.drv/x11drv.h | 1 + dlls/winex11.drv/x11drv_main.c | 1 + dlls/winex11.drv/xrandr.c | 28 ++++++++++++++++++++++++++++ 6 files changed, 32 insertions(+), 3 deletions(-)
diff --git a/dlls/user32/tests/monitor.c b/dlls/user32/tests/monitor.c index 425c32c85e2..4badd97ab04 100644 --- a/dlls/user32/tests/monitor.c +++ b/dlls/user32/tests/monitor.c @@ -1262,7 +1262,7 @@ static void test_ChangeDisplaySettingsEx(int myARGC, char **myARGV) dm.dmPelsWidth = 640; dm.dmPelsHeight = 480; dm.dmFields = DM_PELSWIDTH | DM_PELSHEIGHT; - expect_dm(&dm, devices[0].name, 0, TRUE); + expect_dm(&dm, devices[0].name, 0, FALSE);
SetEvent(exit_event1); wait_result = WaitForSingleObject(process1, 10000); diff --git a/dlls/winex11.drv/event.c b/dlls/winex11.drv/event.c index 8b02361aaff..b1c8dd778d9 100644 --- a/dlls/winex11.drv/event.c +++ b/dlls/winex11.drv/event.c @@ -405,7 +405,7 @@ static inline BOOL call_event_handler( Display *display, XEvent *event ) /*********************************************************************** * process_events */ -static BOOL process_events( Display *display, Bool (*filter)(Display*, XEvent*,XPointer), ULONG_PTR arg ) +BOOL process_events( Display *display, Bool (*filter)(Display*, XEvent*,XPointer), ULONG_PTR arg ) { XEvent event, prev_event; int count = 0; diff --git a/dlls/winex11.drv/window.c b/dlls/winex11.drv/window.c index 84878e95e37..e90a36a254e 100644 --- a/dlls/winex11.drv/window.c +++ b/dlls/winex11.drv/window.c @@ -2040,7 +2040,6 @@ BOOL X11DRV_CreateWindow( HWND hwnd ) CWOverrideRedirect | CWEventMask, &attr ); XFlush( data->display ); NtUserSetProp( hwnd, clip_window_prop, (HANDLE)data->clip_window ); - X11DRV_DisplayDevices_RegisterEventHandlers(); } return TRUE; } diff --git a/dlls/winex11.drv/x11drv.h b/dlls/winex11.drv/x11drv.h index 9c1b8012466..3abdd14e600 100644 --- a/dlls/winex11.drv/x11drv.h +++ b/dlls/winex11.drv/x11drv.h @@ -682,6 +682,7 @@ extern void retry_grab_clipping_window(void); extern void ungrab_clipping_window(void); extern void move_resize_window( HWND hwnd, int dir ); extern void X11DRV_InitKeyboard( Display *display ); +extern BOOL process_events( Display *display, Bool (*filter)(Display*, XEvent*, XPointer), ULONG_PTR arg ); extern BOOL X11DRV_ProcessEvents( DWORD mask ); extern HWND *build_hwnd_list(void);
diff --git a/dlls/winex11.drv/x11drv_main.c b/dlls/winex11.drv/x11drv_main.c index 0925fe54b9c..c4d56c2d542 100644 --- a/dlls/winex11.drv/x11drv_main.c +++ b/dlls/winex11.drv/x11drv_main.c @@ -703,6 +703,7 @@ static NTSTATUS x11drv_init( void *arg )
init_user_driver(); X11DRV_DisplayDevices_Init(FALSE); + X11DRV_DisplayDevices_RegisterEventHandlers(); return STATUS_SUCCESS; }
diff --git a/dlls/winex11.drv/xrandr.c b/dlls/winex11.drv/xrandr.c index d77eb1a0163..bc4ae0209ac 100644 --- a/dlls/winex11.drv/xrandr.c +++ b/dlls/winex11.drv/xrandr.c @@ -1213,6 +1213,32 @@ static void xrandr14_register_event_handlers(void) "XRandR ProviderChange" ); }
+static Bool filter_rrnotify_event( Display *display, XEvent *event, char *arg ) +{ + ULONG_PTR event_base = (ULONG_PTR)arg; + + if (event->type == event_base + RRNotify_CrtcChange + || event->type == event_base + RRNotify_OutputChange + || event->type == event_base + RRNotify_ProviderChange) + return 1; + + return 0; +} + +static void process_rrnotify_events(void) +{ + struct x11drv_thread_data *data = x11drv_thread_data(); + int event_base, error_base; + + if (!data) return; + if (data->current_event) return; /* don't process nested events */ + + if (!pXRRQueryExtension( data->display, &event_base, &error_base )) + return; + + process_events( data->display, filter_rrnotify_event, event_base ); +} + /* XRandR 1.4 display settings handler */ static BOOL xrandr14_get_id( const WCHAR *device_name, BOOL is_primary, x11drv_settings_id *id ) { @@ -1228,6 +1254,8 @@ static BOOL xrandr14_get_id( const WCHAR *device_name, BOOL is_primary, x11drv_s if (*end) return FALSE;
+ process_rrnotify_events(); + /* Update cache */ pthread_mutex_lock( &xrandr_mutex ); if (!current_modes)
From: Anton Baskanov baskanov@gmail.com
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=49674 --- dlls/user32/tests/monitor.c | 50 ++++++++--------- dlls/win32u/sysparams.c | 15 ++++++ programs/explorer/desktop.c | 104 +++++++++++++++++++++++++++++++++--- 3 files changed, 136 insertions(+), 33 deletions(-)
diff --git a/dlls/user32/tests/monitor.c b/dlls/user32/tests/monitor.c index 4badd97ab04..4c70fc189cf 100644 --- a/dlls/user32/tests/monitor.c +++ b/dlls/user32/tests/monitor.c @@ -291,8 +291,8 @@ struct device_info DEVMODEA original_mode; };
-#define expect_dm(a, b, c, d) _expect_dm(__LINE__, a, b, c, d) -static void _expect_dm(INT line, const DEVMODEA *expected, const CHAR *device, DWORD test, BOOL todo) +#define expect_dm(a, b, c) _expect_dm(__LINE__, a, b, c) +static void _expect_dm(INT line, const DEVMODEA *expected, const CHAR *device, DWORD test) { DEVMODEA dm; BOOL ret; @@ -307,9 +307,9 @@ static void _expect_dm(INT line, const DEVMODEA *expected, const CHAR *device, D "Device %s test %ld expect dmFields to contain %#lx, got %#lx\n", device, test, expected->dmFields, dm.dmFields); ok_(__FILE__, line)(!(expected->dmFields & DM_BITSPERPEL) || dm.dmBitsPerPel == expected->dmBitsPerPel, "Device %s test %ld expect dmBitsPerPel %lu, got %lu\n", device, test, expected->dmBitsPerPel, dm.dmBitsPerPel); - todo_wine_if(todo) ok_(__FILE__, line)(!(expected->dmFields & DM_PELSWIDTH) || dm.dmPelsWidth == expected->dmPelsWidth, + ok_(__FILE__, line)(!(expected->dmFields & DM_PELSWIDTH) || dm.dmPelsWidth == expected->dmPelsWidth, "Device %s test %ld expect dmPelsWidth %lu, got %lu\n", device, test, expected->dmPelsWidth, dm.dmPelsWidth); - todo_wine_if(todo) ok_(__FILE__, line)(!(expected->dmFields & DM_PELSHEIGHT) || dm.dmPelsHeight == expected->dmPelsHeight, + ok_(__FILE__, line)(!(expected->dmFields & DM_PELSHEIGHT) || dm.dmPelsHeight == expected->dmPelsHeight, "Device %s test %ld expect dmPelsHeight %lu, got %lu\n", device, test, expected->dmPelsHeight, dm.dmPelsHeight); ok_(__FILE__, line)(!(expected->dmFields & DM_POSITION) || dm.dmPosition.x == expected->dmPosition.x, "Device %s test %ld expect dmPosition.x %ld, got %ld\n", device, test, expected->dmPosition.x, dm.dmPosition.x); @@ -325,8 +325,8 @@ static void _expect_dm(INT line, const DEVMODEA *expected, const CHAR *device, D dm.dmDisplayOrientation); }
-#define wait_for_dm(a, b, c, d) wait_for_dm_(__LINE__, a, b, c, d) -static void wait_for_dm_(int line, const char *device, DWORD expectedWidth, DWORD expectedHeight, BOOL todo) +#define wait_for_dm(a, b, c) wait_for_dm_(__LINE__, a, b, c) +static void wait_for_dm_(int line, const char *device, DWORD expectedWidth, DWORD expectedHeight) { DEVMODEA dm; BOOL ret; @@ -348,9 +348,9 @@ static void wait_for_dm_(int line, const char *device, DWORD expectedWidth, DWOR Sleep(100); }
- todo_wine_if(todo) ok_(__FILE__, line)(dm.dmPelsWidth == expectedWidth, + ok_(__FILE__, line)(dm.dmPelsWidth == expectedWidth, "Device %s expect dmPelsWidth %lu, got %lu\n", device, expectedWidth, dm.dmPelsWidth); - todo_wine_if(todo) ok_(__FILE__, line)(dm.dmPelsHeight == expectedHeight, + ok_(__FILE__, line)(dm.dmPelsHeight == expectedHeight, "Device %s expect dmPelsHeight %lu, got %lu\n", device, expectedHeight, dm.dmPelsHeight); }
@@ -731,7 +731,7 @@ static void test_ChangeDisplaySettingsEx(int myARGC, char **myARGV) continue; } flush_events(); - expect_dm(&dm3, devices[device].name, test, FALSE); + expect_dm(&dm3, devices[device].name, test);
/* Change the registry mode to the second mode */ res = ChangeDisplaySettingsExA(devices[device].name, &dm2, NULL, CDS_UPDATEREGISTRY | CDS_NORESET, NULL); @@ -865,7 +865,7 @@ static void test_ChangeDisplaySettingsEx(int myARGC, char **myARGV) }
flush_events(); - expect_dm(&dm, devices[device].name, mode, FALSE); + expect_dm(&dm, devices[device].name, mode); }
/* Restore settings */ @@ -938,7 +938,7 @@ static void test_ChangeDisplaySettingsEx(int myARGC, char **myARGV) }
flush_events(); - expect_dm(&dm, devices[device].name, 0, FALSE); + expect_dm(&dm, devices[device].name, 0);
/* Test specifying only position, width and height */ memset(&dm, 0, sizeof(dm)); @@ -983,7 +983,7 @@ static void test_ChangeDisplaySettingsEx(int myARGC, char **myARGV) ok(dm.dmBitsPerPel, "Expected dmBitsPerPel not zero.\n"); ok(dm.dmDisplayFrequency, "Expected dmDisplayFrequency not zero.\n");
- expect_dm(&dm, devices[device].name, 0, FALSE); + expect_dm(&dm, devices[device].name, 0); }
/* Test dmPosition */ @@ -1055,7 +1055,7 @@ static void test_ChangeDisplaySettingsEx(int myARGC, char **myARGV) ok(res == DISP_CHANGE_SUCCESSFUL, "ChangeDisplaySettingsExA %s returned unexpected %ld\n", devices[1].name, res);
dm2.dmPosition.x = dm.dmPosition.x + dm.dmPelsWidth; - expect_dm(&dm2, devices[1].name, 0, FALSE); + expect_dm(&dm2, devices[1].name, 0);
/* Test placing the secondary adapter to all sides of the primary adapter */ for (test = 0; test < 8; ++test) @@ -1114,7 +1114,7 @@ static void test_ChangeDisplaySettingsEx(int myARGC, char **myARGV) }
flush_events(); - expect_dm(&dm2, devices[1].name, test, FALSE); + expect_dm(&dm2, devices[1].name, test); }
/* Test automatic position update when other adapters change resolution */ @@ -1179,7 +1179,7 @@ static void test_ChangeDisplaySettingsEx(int myARGC, char **myARGV) ok(res == DISP_CHANGE_SUCCESSFUL, "ChangeDisplaySettingsExA %s mode %d returned unexpected %ld.\n", devices[device].name, mode, res); flush_events(); - expect_dm(&dm2, devices[device].name, mode, FALSE); + expect_dm(&dm2, devices[device].name, mode);
/* EnumDisplaySettingsEx without EDS_ROTATEDMODE reports modes with current orientation */ memset(&dm3, 0, sizeof(dm3)); @@ -1225,7 +1225,7 @@ static void test_ChangeDisplaySettingsEx(int myARGC, char **myARGV) broken(res == DISP_CHANGE_FAILED), /* win8 TestBot */ "ChangeDisplaySettingsExA returned unexpected %ld\n", res); for (device = 0; device < device_count; ++device) - expect_dm(&devices[device].original_mode, devices[device].name, 0, FALSE); + expect_dm(&devices[device].original_mode, devices[device].name, 0);
exit_event0 = CreateEventA(NULL, FALSE, FALSE, exit_event0_name); ok(!!exit_event0, "CreateEventA failed, error %#lx\n", GetLastError()); @@ -1262,13 +1262,13 @@ static void test_ChangeDisplaySettingsEx(int myARGC, char **myARGV) dm.dmPelsWidth = 640; dm.dmPelsHeight = 480; dm.dmFields = DM_PELSWIDTH | DM_PELSHEIGHT; - expect_dm(&dm, devices[0].name, 0, FALSE); + expect_dm(&dm, devices[0].name, 0);
SetEvent(exit_event1); wait_result = WaitForSingleObject(process1, 10000); ok(wait_result == WAIT_OBJECT_0, "WaitForSingleObject returned %lx.\n", wait_result);
- wait_for_dm(devices[0].name, 800, 600, TRUE); + wait_for_dm(devices[0].name, 800, 600);
CloseHandle(process1); CloseHandle(process0); @@ -1282,7 +1282,7 @@ static void test_ChangeDisplaySettingsEx(int myARGC, char **myARGV) wait_result = WaitForSingleObject(process1, 10000); ok(wait_result == WAIT_OBJECT_0, "WaitForSingleObject returned %lx.\n", wait_result);
- wait_for_dm(devices[0].name, 800, 600, TRUE); + wait_for_dm(devices[0].name, 800, 600);
SetEvent(exit_event0); wait_result = WaitForSingleObject(process0, 10000); @@ -1295,7 +1295,7 @@ static void test_ChangeDisplaySettingsEx(int myARGC, char **myARGV) dm.dmPelsWidth = 800; dm.dmPelsHeight = 600; dm.dmFields = DM_PELSWIDTH | DM_PELSHEIGHT; - expect_dm(&dm, devices[0].name, 0, TRUE); + expect_dm(&dm, devices[0].name, 0);
CloseHandle(process1); CloseHandle(process0); @@ -1316,8 +1316,8 @@ static void test_ChangeDisplaySettingsEx(int myARGC, char **myARGV) wait_result = WaitForSingleObject(process1, 10000); ok(wait_result == WAIT_OBJECT_0, "WaitForSingleObject returned %lx.\n", wait_result);
- wait_for_dm(devices[0].name, 800, 600, TRUE); - wait_for_dm(devices[1].name, 800, 600, TRUE); + wait_for_dm(devices[0].name, 800, 600); + wait_for_dm(devices[1].name, 800, 600);
SetEvent(exit_event0); wait_result = WaitForSingleObject(process0, 10000); @@ -1330,8 +1330,8 @@ static void test_ChangeDisplaySettingsEx(int myARGC, char **myARGV) dm.dmPelsWidth = 800; dm.dmPelsHeight = 600; dm.dmFields = DM_PELSWIDTH | DM_PELSHEIGHT; - expect_dm(&dm, devices[0].name, 0, TRUE); - expect_dm(&dm, devices[1].name, 0, TRUE); + expect_dm(&dm, devices[0].name, 0); + expect_dm(&dm, devices[1].name, 0);
CloseHandle(process1); CloseHandle(process0); @@ -1354,7 +1354,7 @@ static void test_ChangeDisplaySettingsEx(int myARGC, char **myARGV) broken(res == DISP_CHANGE_FAILED), /* win8 TestBot */ "ChangeDisplaySettingsExA returned unexpected %ld\n", res); for (device = 0; device < device_count; ++device) - expect_dm(&devices[device].original_mode, devices[device].name, 0, FALSE); + expect_dm(&devices[device].original_mode, devices[device].name, 0);
free(devices); } diff --git a/dlls/win32u/sysparams.c b/dlls/win32u/sysparams.c index a31d586a5b6..c7b09d2e879 100644 --- a/dlls/win32u/sysparams.c +++ b/dlls/win32u/sysparams.c @@ -3209,12 +3209,21 @@ static BOOL all_detached_settings( const DEVMODEW *displays ) static LONG apply_display_settings( const WCHAR *devname, const DEVMODEW *devmode, HWND hwnd, DWORD flags, void *lparam ) { + static const WCHAR restorerW[] = {'_','_','w','i','n','e','_','d','i','s','p','l','a','y','_', + 's','e','t','t','i','n','g','s','_','r','e','s','t','o','r','e','r',0}; + UNICODE_STRING restoter_str = RTL_CONSTANT_STRING( restorerW ); WCHAR primary_name[CCHDEVICENAME]; struct display_device *primary; + DWORD fullscreen_process_id; DEVMODEW *mode, *displays; + DWORD restorer_thread_id; struct adapter *adapter; + HWND restorer_window; LONG ret;
+ restorer_window = NtUserFindWindowEx( NULL, NULL, &restoter_str, NULL, 0 ); + restorer_thread_id = NtUserGetWindowThread( restorer_window, NULL ); + if (!lock_display_devices()) return DISP_CHANGE_FAILED; if (!(displays = get_display_settings( devname, devmode ))) { @@ -3256,6 +3265,12 @@ static LONG apply_display_settings( const WCHAR *devname, const DEVMODEW *devmod free( displays ); if (ret) return ret;
+ if ( restorer_thread_id != GetCurrentThreadId() ) + { + fullscreen_process_id = (flags & CDS_FULLSCREEN) ? GetCurrentProcessId() : 0; + send_message( restorer_window, WM_USER + 0, 0, fullscreen_process_id ); + } + if (!update_display_cache( TRUE )) WARN( "Failed to update display cache after mode change.\n" );
diff --git a/programs/explorer/desktop.c b/programs/explorer/desktop.c index 4e8f9a84c07..3c2564964be 100644 --- a/programs/explorer/desktop.c +++ b/programs/explorer/desktop.c @@ -611,6 +611,19 @@ static void initialize_launchers( HWND hwnd ) } }
+static BOOL wait_named_mutex( const WCHAR *name ) +{ + HANDLE mutex; + + mutex = CreateMutexW( NULL, TRUE, name ); + if (GetLastError() == ERROR_ALREADY_EXISTS) + { + TRACE( "waiting for mutex %s\n", debugstr_w( name )); + WaitForSingleObject( mutex, INFINITE ); + } + return TRUE; +} + /************************************************************************** * wait_clipboard_mutex * @@ -620,7 +633,6 @@ static BOOL wait_clipboard_mutex(void) { static const WCHAR prefix[] = L"__wine_clipboard_"; WCHAR buffer[MAX_PATH + ARRAY_SIZE( prefix )]; - HANDLE mutex;
memcpy( buffer, prefix, sizeof(prefix) ); if (!GetUserObjectInformationW( GetProcessWindowStation(), UOI_NAME, @@ -630,13 +642,7 @@ static BOOL wait_clipboard_mutex(void) ERR( "failed to get winstation name\n" ); return FALSE; } - mutex = CreateMutexW( NULL, TRUE, buffer ); - if (GetLastError() == ERROR_ALREADY_EXISTS) - { - TRACE( "waiting for mutex %s\n", debugstr_w( buffer )); - WaitForSingleObject( mutex, INFINITE ); - } - return TRUE; + return wait_named_mutex( buffer ); }
@@ -696,6 +702,86 @@ static DWORD WINAPI clipboard_thread( void *arg ) return 0; }
+static HANDLE fullscreen_process; + +static LRESULT WINAPI display_settings_restorer_wndproc( HWND hwnd, UINT message, WPARAM wp, LPARAM lp ) +{ + TRACE( "got msg %04x wp %Ix lp %Ix\n", message, wp, lp ); + + switch(message) + { + case WM_USER + 0: + TRACE( "fullscreen process id %Iu.\n", lp ); + + if (fullscreen_process) + { + CloseHandle( fullscreen_process ); + fullscreen_process = NULL; + } + + if (lp) + fullscreen_process = OpenProcess( SYNCHRONIZE, FALSE, lp ); + + return 0; + } + + return DefWindowProcW( hwnd, message, wp, lp ); +} + +static DWORD WINAPI display_settings_restorer_thread( void *param ) +{ + static const WCHAR display_settings_restorer_classname[] = L"__wine_display_settings_restorer"; + DWORD wait_result; + WNDCLASSW class; + ATOM atom; + MSG msg; + + if (!wait_named_mutex( L"__wine_display_settings_restorer_mutex" )) return 0; + + memset( &class, 0, sizeof(class) ); + class.lpfnWndProc = display_settings_restorer_wndproc; + class.lpszClassName = display_settings_restorer_classname; + + if (!(atom = RegisterClassW( &class )) && GetLastError() != ERROR_CLASS_ALREADY_EXISTS) + { + ERR( "could not register display settings restorer window class err %lu\n", GetLastError() ); + return 0; + } + if (!CreateWindowW( display_settings_restorer_classname, NULL, 0, 0, 0, 0, 0, HWND_MESSAGE, 0, 0, NULL )) + { + TRACE( "failed to create display settings restorer window err %lu\n", GetLastError() ); + UnregisterClassW( MAKEINTRESOURCEW(atom), NULL ); + return 0; + } + + for (;;) + { + if (PeekMessageW( &msg, NULL, 0, 0, PM_REMOVE )) + { + if (msg.message == WM_QUIT) + break; + DispatchMessageW( &msg ); + continue; + } + + wait_result = MsgWaitForMultipleObjects( fullscreen_process ? 1 : 0, &fullscreen_process, + FALSE, INFINITE, QS_ALLINPUT ); + if (wait_result == WAIT_FAILED) + break; + if (!fullscreen_process || wait_result != WAIT_OBJECT_0) + continue; + + WARN( "restoring display settings on process exit\n" ); + + ChangeDisplaySettingsExW( NULL, NULL, NULL, 0, NULL ); + + CloseHandle( fullscreen_process ); + fullscreen_process = NULL; + } + + return 0; +} + static WNDPROC desktop_orig_wndproc;
/* window procedure for the desktop window */ @@ -1123,6 +1209,8 @@ void manage_desktop( WCHAR *arg ) SWP_SHOWWINDOW ); thread = CreateThread( NULL, 0, clipboard_thread, NULL, 0, &id ); if (thread) CloseHandle( thread ); + thread = CreateThread( NULL, 0, display_settings_restorer_thread, NULL, 0, &id ); + if (thread) CloseHandle( thread ); SystemParametersInfoW( SPI_SETDESKWALLPAPER, 0, NULL, FALSE ); ClipCursor( NULL ); initialize_display_settings( width, height );
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=143574
Your paranoid android.
=== w8 (32 bit report) ===
user32: monitor.c:2897: Test failed: ChangeDisplaySettingsExA \.\DISPLAY1 returned unexpected -1. monitor.c:2897: Test failed: ChangeDisplaySettingsExA \.\DISPLAY1 returned unexpected -1. monitor.c:1251: Test failed: ChangeDisplaySettingsExA \.\DISPLAY1 returned -1. monitor.c:1265: Test failed: Device \.\DISPLAY1 test 0 expect dmPelsWidth 640, got 1024 monitor.c:1265: Test failed: Device \.\DISPLAY1 test 0 expect dmPelsHeight 480, got 768 monitor.c:1271: Test failed: Device \.\DISPLAY1 expect dmPelsWidth 800, got 1024 monitor.c:1271: Test failed: Device \.\DISPLAY1 expect dmPelsHeight 600, got 768 monitor.c:2897: Test failed: ChangeDisplaySettingsExA \.\DISPLAY1 returned unexpected -1. monitor.c:2897: Test failed: ChangeDisplaySettingsExA \.\DISPLAY1 returned unexpected -1. monitor.c:1285: Test failed: Device \.\DISPLAY1 expect dmPelsWidth 800, got 1024 monitor.c:1285: Test failed: Device \.\DISPLAY1 expect dmPelsHeight 600, got 768 monitor.c:1298: Test failed: Device \.\DISPLAY1 test 0 expect dmPelsWidth 800, got 1024 monitor.c:1298: Test failed: Device \.\DISPLAY1 test 0 expect dmPelsHeight 600, got 768
=== w8adm (32 bit report) ===
user32: monitor.c:2897: Test failed: ChangeDisplaySettingsExA \.\DISPLAY1 returned unexpected -1. monitor.c:2897: Test failed: ChangeDisplaySettingsExA \.\DISPLAY1 returned unexpected -1. monitor.c:1251: Test failed: ChangeDisplaySettingsExA \.\DISPLAY1 returned -1. monitor.c:1265: Test failed: Device \.\DISPLAY1 test 0 expect dmPelsWidth 640, got 1024 monitor.c:1265: Test failed: Device \.\DISPLAY1 test 0 expect dmPelsHeight 480, got 768 monitor.c:1271: Test failed: Device \.\DISPLAY1 expect dmPelsWidth 800, got 1024 monitor.c:1271: Test failed: Device \.\DISPLAY1 expect dmPelsHeight 600, got 768 monitor.c:2897: Test failed: ChangeDisplaySettingsExA \.\DISPLAY1 returned unexpected -1. monitor.c:2897: Test failed: ChangeDisplaySettingsExA \.\DISPLAY1 returned unexpected -1. monitor.c:1285: Test failed: Device \.\DISPLAY1 expect dmPelsWidth 800, got 1024 monitor.c:1285: Test failed: Device \.\DISPLAY1 expect dmPelsHeight 600, got 768 monitor.c:1298: Test failed: Device \.\DISPLAY1 test 0 expect dmPelsWidth 800, got 1024 monitor.c:1298: Test failed: Device \.\DISPLAY1 test 0 expect dmPelsHeight 600, got 768
=== w864 (32 bit report) ===
user32: monitor.c:2897: Test failed: ChangeDisplaySettingsExA \.\DISPLAY1 returned unexpected -1. monitor.c:2897: Test failed: ChangeDisplaySettingsExA \.\DISPLAY1 returned unexpected -1. monitor.c:1251: Test failed: ChangeDisplaySettingsExA \.\DISPLAY1 returned -1. monitor.c:1265: Test failed: Device \.\DISPLAY1 test 0 expect dmPelsWidth 640, got 1024 monitor.c:1265: Test failed: Device \.\DISPLAY1 test 0 expect dmPelsHeight 480, got 768 monitor.c:1271: Test failed: Device \.\DISPLAY1 expect dmPelsWidth 800, got 1024 monitor.c:1271: Test failed: Device \.\DISPLAY1 expect dmPelsHeight 600, got 768 monitor.c:2897: Test failed: ChangeDisplaySettingsExA \.\DISPLAY1 returned unexpected -1. monitor.c:2897: Test failed: ChangeDisplaySettingsExA \.\DISPLAY1 returned unexpected -1. monitor.c:1285: Test failed: Device \.\DISPLAY1 expect dmPelsWidth 800, got 1024 monitor.c:1285: Test failed: Device \.\DISPLAY1 expect dmPelsHeight 600, got 768 monitor.c:1298: Test failed: Device \.\DISPLAY1 test 0 expect dmPelsWidth 800, got 1024 monitor.c:1298: Test failed: Device \.\DISPLAY1 test 0 expect dmPelsHeight 600, got 768
=== w864 (64 bit report) ===
user32: monitor.c:2897: Test failed: ChangeDisplaySettingsExA \.\DISPLAY1 returned unexpected -1. monitor.c:2897: Test failed: ChangeDisplaySettingsExA \.\DISPLAY1 returned unexpected -1. monitor.c:1251: Test failed: ChangeDisplaySettingsExA \.\DISPLAY1 returned -1. monitor.c:1265: Test failed: Device \.\DISPLAY1 test 0 expect dmPelsWidth 640, got 1024 monitor.c:1265: Test failed: Device \.\DISPLAY1 test 0 expect dmPelsHeight 480, got 768 monitor.c:1271: Test failed: Device \.\DISPLAY1 expect dmPelsWidth 800, got 1024 monitor.c:1271: Test failed: Device \.\DISPLAY1 expect dmPelsHeight 600, got 768 monitor.c:2897: Test failed: ChangeDisplaySettingsExA \.\DISPLAY1 returned unexpected -1. monitor.c:2897: Test failed: ChangeDisplaySettingsExA \.\DISPLAY1 returned unexpected -1. monitor.c:1285: Test failed: Device \.\DISPLAY1 expect dmPelsWidth 800, got 1024 monitor.c:1285: Test failed: Device \.\DISPLAY1 expect dmPelsHeight 600, got 768 monitor.c:1298: Test failed: Device \.\DISPLAY1 test 0 expect dmPelsWidth 800, got 1024 monitor.c:1298: Test failed: Device \.\DISPLAY1 test 0 expect dmPelsHeight 600, got 768
On Thu Feb 29 15:27:10 2024 +0000, Anton Baskanov wrote:
changed this line in [version 5 of the diff](/wine/wine/-/merge_requests/5060/diffs?diff_id=102366&start_sha=1b2a85444b2f65745313d4aee79903c6a8fe4eb3#1a2fc38375b48458ba2fb34a2716bf76579d9527_54_53)
Done.
On Thu Feb 29 15:27:10 2024 +0000, Anton Baskanov wrote:
changed this line in [version 5 of the diff](/wine/wine/-/merge_requests/5060/diffs?diff_id=102366&start_sha=1b2a85444b2f65745313d4aee79903c6a8fe4eb3#1a2fc38375b48458ba2fb34a2716bf76579d9527_1213_1230)
Done.
On Thu Feb 29 15:27:12 2024 +0000, Anton Baskanov wrote:
changed this line in [version 5 of the diff](/wine/wine/-/merge_requests/5060/diffs?diff_id=102366&start_sha=1b2a85444b2f65745313d4aee79903c6a8fe4eb3#1a2fc38375b48458ba2fb34a2716bf76579d9527_1222_1239)
Done.
On Tue Feb 27 03:05:29 2024 +0000, Zhiyi Zhang wrote:
I think you can add the tests last so these back-and-forth changes can be avoided.
If you don't mind, I'd leave it like this, as it allows me to verify that the tests reproduce the issue.
On Tue Feb 27 03:05:30 2024 +0000, Zhiyi Zhang wrote:
I don't think this is how Windows implements CDS_FULLSCREEN. On Windows, the display resolution can be restored even if explorer.exe is not running.
Do we need to replicate this behavior? Also, do you have any suggestions for a better way to implement this?
On Tue Feb 27 03:05:28 2024 +0000, Zhiyi Zhang wrote:
This patch should be merged with patch 3.
Done.
On Thu Feb 29 15:27:13 2024 +0000, Anton Baskanov wrote:
changed this line in [version 5 of the diff](/wine/wine/-/merge_requests/5060/diffs?diff_id=102366&start_sha=1b2a85444b2f65745313d4aee79903c6a8fe4eb3#281cd1a9b246cf9b867f5ef76600365651a8aacf_225_225)
Done.
On Thu Feb 29 15:27:11 2024 +0000, Anton Baskanov wrote:
changed this line in [version 5 of the diff](/wine/wine/-/merge_requests/5060/diffs?diff_id=102366&start_sha=1b2a85444b2f65745313d4aee79903c6a8fe4eb3#1a2fc38375b48458ba2fb34a2716bf76579d9527_1209_1230)
I need to control both the order of ChangeDisplaySettingsEx() calls and the order in which the child processes exit.
v4: - Remove global `myARGV` and `myARGC` - Remove `dwFlags` and `wShowWindow` assigmnents. - Add a helper function to start child process. - Merge patches 2 and 3 into one. - Move event processing from `X11DRV_GetCurrentDisplaySettings` to `xrandr14_get_id`. - Only process RRNotify events. - Update commit message with the correct performance info for `EnumDisplaySettingsExW` (the previous one was messed up by CPU frequency scaling).
On Thu Feb 29 15:27:53 2024 +0000, Anton Baskanov wrote:
Do we need to replicate this behavior? Also, do you have any suggestions for a better way to implement this?
It's better to do it in the same way.
I don't know how it's implemented. You'll need to dig deeper if possible. There are some possibilities you can try, for example, PsSetCreateProcessNotifyRoutine() as shown at [Detecting Windows NT/2K process execution](https://www.codeproject.com/Articles/2018/Detecting-Windows-NT-2K-process-ex...), or a thread in win32u? maybe window hooks? You can also try calling ChangeDisplaySettingsEx(CDS_FULLSCREEN) in a console application and see how it reacts.
On Mon Mar 4 02:45:29 2024 +0000, Zhiyi Zhang wrote:
It's better to do it in the same way. I don't know how it's implemented. You'll need to dig deeper if possible. There are some possibilities you can try, for example, PsSetCreateProcessNotifyRoutine() as shown at [Detecting Windows NT/2K process execution](https://www.codeproject.com/Articles/2018/Detecting-Windows-NT-2K-process-ex...), or a thread in win32u? maybe window hooks? You can also try calling ChangeDisplaySettingsEx(CDS_FULLSCREEN) in a console application and see how it reacts.
I don't think that trying to handle process exit from within the process itself is a good idea. The process state might be inconsistent due to a crash, or the process might be terminated by a SIGKILL, which can't be intercepted.
I tried wiping a process address space by calling `VirtualFreeEx`/`UnmapViewOfFile2` on all of the regions (a few them failed to free, though), and it did not break the mode restoration, so it looks like it is handled externally. I guess it is in csrss.exe, but I don't know how to verify it (without breaking the clean-room guidelines, that is).
Note also that Wine's explorer.exe does other things that are done by csrss.exe on Windows, e.g. creating the desktop window.
On Mon Mar 11 03:56:44 2024 +0000, Anton Baskanov wrote:
I don't think that trying to handle process exit from within the process itself is a good idea. The process state might be inconsistent due to a crash, or the process might be terminated by a SIGKILL, which can't be intercepted. I tried wiping a process address space by calling `VirtualFreeEx`/`UnmapViewOfFile2` on all of the regions (a few them failed to free, though), and it did not break the mode restoration, so it looks like it is handled externally. I guess it is in csrss.exe, but I don't know how to verify it (without breaking the clean-room guidelines, that is). Note also that Wine's explorer.exe does other things that are done by csrss.exe on Windows, e.g. creating the desktop window.
I see. explorer.exe seems like a reasonable choice.
Zhiyi Zhang (@zhiyi) commented about dlls/user32/tests/monitor.c:
dm.dmDisplayOrientation);}
-static void test_ChangeDisplaySettingsEx(void) +#define wait_for_dm(a, b, c, d) wait_for_dm_(__LINE__, a, b, c, d) +static void wait_for_dm_(int line, const char *device, DWORD expectedWidth, DWORD expectedHeight, BOOL todo)
Let's use expected_width instead expectedWidth. Same for expectedHeight.
Zhiyi Zhang (@zhiyi) commented about dlls/user32/tests/monitor.c:
dm.dmDisplayOrientation);}
-static void test_ChangeDisplaySettingsEx(void) +#define wait_for_dm(a, b, c, d) wait_for_dm_(__LINE__, a, b, c, d) +static void wait_for_dm_(int line, const char *device, DWORD expectedWidth, DWORD expectedHeight, BOOL todo) +{
- DEVMODEA dm;
- BOOL ret;
- int i;
- for (i = 0; i < 100; ++i)
5s should be enough.
Zhiyi Zhang (@zhiyi) commented about dlls/user32/tests/monitor.c:
-static void test_ChangeDisplaySettingsEx(void) +#define wait_for_dm(a, b, c, d) wait_for_dm_(__LINE__, a, b, c, d) +static void wait_for_dm_(int line, const char *device, DWORD expectedWidth, DWORD expectedHeight, BOOL todo) +{
- DEVMODEA dm;
- BOOL ret;
- int i;
- for (i = 0; i < 100; ++i)
- {
memset(&dm, 0, sizeof(dm));dm.dmSize = sizeof(dm);SetLastError(0xdeadbeef);ret = EnumDisplaySettingsA(device, ENUM_CURRENT_SETTINGS, &dm);ok_(__FILE__, line)(ret, "Device %s EnumDisplaySettingsA failed, error %#lx\n", device, GetLastError());ok_(__FILE__, line)((dm.dmFields & (DM_PELSWIDTH | DM_PELSHEIGHT)) == (DM_PELSWIDTH | DM_PELSHEIGHT),
This is already tested elsewhere. You can remove it.
Zhiyi Zhang (@zhiyi) commented about dlls/user32/tests/monitor.c:
- STARTUPINFOA startup;
- DWORD wait_result;
- HANDLE cds_event;
- LONG res;
- cds_event = CreateEventA(NULL, FALSE, FALSE, cds_event_name);
- ok(!!cds_event, "CreateEventA failed, error %#lx\n", GetLastError());
- memset(&startup, 0, sizeof(startup));
- startup.cb = sizeof(startup);
- snprintf(buffer, sizeof(buffer), "%s monitor fullscreen %s %s %s", argv0, device,
cds_event_name, exit_event_name);- res = CreateProcessA(NULL, buffer, NULL, NULL, FALSE, 0, NULL, NULL, &startup, &info);
- ok(res, "CreateProcessA returned unexpected %ld\n", res);
- wait_result = WaitForSingleObject(cds_event, 10000);
1~2s should be enough.
Zhiyi Zhang (@zhiyi) commented about dlls/user32/tests/monitor.c:
if (dm.dmPelsWidth == expectedWidth && dm.dmPelsHeight == expectedHeight)break;Sleep(100);- }
- todo_wine_if(todo) ok_(__FILE__, line)(dm.dmPelsWidth == expectedWidth,
"Device %s expect dmPelsWidth %lu, got %lu\n", device, expectedWidth, dm.dmPelsWidth);- todo_wine_if(todo) ok_(__FILE__, line)(dm.dmPelsHeight == expectedHeight,
"Device %s expect dmPelsHeight %lu, got %lu\n", device, expectedHeight, dm.dmPelsHeight);+}
+static HANDLE test_child_process_ChangeDisplaySettingsEx(const char *argv0, const char *device, const char *exit_event_name) +{
- static const char *cds_event_name = "test_child_process_ChangeDisplaySettingsEx_cds_event";
cds already stands for ChangeDisplaySettingsEx(). Let's use "test_child_process_cds_event"
Zhiyi Zhang (@zhiyi) commented about dlls/user32/tests/monitor.c:
- res = CreateProcessA(NULL, buffer, NULL, NULL, FALSE, 0, NULL, NULL, &startup, &info);
- ok(res, "CreateProcessA returned unexpected %ld\n", res);
- wait_result = WaitForSingleObject(cds_event, 10000);
- ok(wait_result == WAIT_OBJECT_0, "WaitForSingleObject returned %lx.\n", wait_result);
- CloseHandle(cds_event);
- CloseHandle(info.hThread);
- return info.hProcess;
+}
+static void test_ChangeDisplaySettingsEx(int myARGC, char **myARGV) { static const DWORD registry_fields = DM_DISPLAYORIENTATION | DM_BITSPERPEL | DM_PELSWIDTH | DM_PELSHEIGHT | DM_DISPLAYFLAGS | DM_DISPLAYFREQUENCY | DM_POSITION;
- static const char *exit_event0_name = "test_ChangeDisplaySettingsEx_exit_event0";
use test_cds_exit_event0/1
Zhiyi Zhang (@zhiyi) commented about dlls/user32/tests/monitor.c:
- SetEvent(exit_event0);
- wait_result = WaitForSingleObject(process0, 10000);
- ok(wait_result == WAIT_OBJECT_0, "WaitForSingleObject returned %lx.\n", wait_result);
- Sleep(100);
- memset(&dm, 0, sizeof(dm));
- dm.dmSize = sizeof(dm);
- dm.dmPelsWidth = 640;
- dm.dmPelsHeight = 480;
- dm.dmFields = DM_PELSWIDTH | DM_PELSHEIGHT;
- expect_dm(&dm, devices[0].name, 0, TRUE);
- SetEvent(exit_event1);
- wait_result = WaitForSingleObject(process1, 10000);
Same here.
Zhiyi Zhang (@zhiyi) commented about dlls/user32/tests/monitor.c:
- process1 = test_child_process_ChangeDisplaySettingsEx(myARGV[0], devices[0].name, exit_event1_name);
- /* Verify that the settings are restored to the current registry settings */
- for (device = 0; device < device_count; ++device)
- {
memset(&dm, 0, sizeof(dm));dm.dmSize = sizeof(dm);dm.dmFields = DM_PELSWIDTH | DM_PELSHEIGHT;dm.dmPelsWidth = 800;dm.dmPelsHeight = 600;res = ChangeDisplaySettingsExA(devices[device].name, &dm, NULL, CDS_UPDATEREGISTRY | CDS_NORESET, NULL);ok(res == DISP_CHANGE_SUCCESSFUL, "ChangeDisplaySettingsExA %s returned %ld.\n", devices[device].name, res);- }
- SetEvent(exit_event0);
- wait_result = WaitForSingleObject(process0, 10000);
5s should be enough.
Zhiyi Zhang (@zhiyi) commented about dlls/user32/tests/monitor.c:
- for (device = 0; device < device_count; ++device)
- {
memset(&dm, 0, sizeof(dm));dm.dmSize = sizeof(dm);dm.dmFields = DM_PELSWIDTH | DM_PELSHEIGHT;dm.dmPelsWidth = 800;dm.dmPelsHeight = 600;res = ChangeDisplaySettingsExA(devices[device].name, &dm, NULL, CDS_UPDATEREGISTRY | CDS_NORESET, NULL);ok(res == DISP_CHANGE_SUCCESSFUL, "ChangeDisplaySettingsExA %s returned %ld.\n", devices[device].name, res);- }
- SetEvent(exit_event0);
- wait_result = WaitForSingleObject(process0, 10000);
- ok(wait_result == WAIT_OBJECT_0, "WaitForSingleObject returned %lx.\n", wait_result);
- Sleep(100);
Why do you need this here?
Zhiyi Zhang (@zhiyi) commented about dlls/user32/tests/monitor.c:
- startup.cb = sizeof(startup);
- snprintf(buffer, sizeof(buffer), "%s monitor fullscreen %s %s %s", argv0, device,
cds_event_name, exit_event_name);- res = CreateProcessA(NULL, buffer, NULL, NULL, FALSE, 0, NULL, NULL, &startup, &info);
- ok(res, "CreateProcessA returned unexpected %ld\n", res);
- wait_result = WaitForSingleObject(cds_event, 10000);
- ok(wait_result == WAIT_OBJECT_0, "WaitForSingleObject returned %lx.\n", wait_result);
- CloseHandle(cds_event);
- CloseHandle(info.hThread);
- return info.hProcess;
+}
+static void test_ChangeDisplaySettingsEx(int myARGC, char **myARGV)
Use argc and argv.
Zhiyi Zhang (@zhiyi) commented about dlls/user32/tests/monitor.c:
- dm.dmPelsHeight = 600;
- dm.dmFields = DM_PELSWIDTH | DM_PELSHEIGHT;
- expect_dm(&dm, devices[0].name, 0, TRUE);
- CloseHandle(process1);
- CloseHandle(process0);
- if (device_count < 2)
- {
skip("Only one device found.\n");- }
- else
- {
/* Test that the settings are restored for all devices, regardless of* the process that changed them */
This test doesn't need two processes.
Zhiyi Zhang (@zhiyi) commented about dlls/user32/tests/monitor.c:
wait_for_dm(devices[0].name, 800, 600, TRUE);wait_for_dm(devices[1].name, 800, 600, TRUE);SetEvent(exit_event0);wait_result = WaitForSingleObject(process0, 10000);ok(wait_result == WAIT_OBJECT_0, "WaitForSingleObject returned %lx.\n", wait_result);Sleep(100);memset(&dm, 0, sizeof(dm));dm.dmSize = sizeof(dm);dm.dmPelsWidth = 800;dm.dmPelsHeight = 600;dm.dmFields = DM_PELSWIDTH | DM_PELSHEIGHT;expect_dm(&dm, devices[0].name, 0, TRUE);expect_dm(&dm, devices[1].name, 0, TRUE);
You've already called wait_for_dm(). Calling expect_dm seems redundant.
Zhiyi Zhang (@zhiyi) commented about dlls/user32/tests/monitor.c:
memset(&dm, 0, sizeof(dm));dm.dmSize = sizeof(dm);dm.dmPelsWidth = 800;dm.dmPelsHeight = 600;dm.dmFields = DM_PELSWIDTH | DM_PELSHEIGHT;expect_dm(&dm, devices[0].name, 0, TRUE);expect_dm(&dm, devices[1].name, 0, TRUE);CloseHandle(process1);CloseHandle(process0);- }
- CloseHandle(exit_event1);
- CloseHandle(exit_event0);
- /* Restore all adapters to their original settings */
This part is duplicated. Could you move the tests up so that you don't have to restore them again?
Zhiyi Zhang (@zhiyi) commented about dlls/user32/tests/monitor.c:
EnumDisplayMonitors(NULL, NULL, MonitorEnumProc, 0);return;
if (strcmp(myARGV[2], "info") == 0){printf("Monitor information:\n");EnumDisplayMonitors(NULL, NULL, MonitorEnumProc, 0);return;}else if (strcmp(myARGV[2], "fullscreen") == 0){HANDLE event0, event1;DWORD wait_result;DEVMODEA dm;LONG res;if (myARGC < 6)
Place the following code into a helper so it remains clean in the main function.
Zhiyi Zhang (@zhiyi) commented about dlls/user32/tests/monitor.c:
event1 = OpenEventA(SYNCHRONIZE, FALSE, myARGV[5]);ok(!!event1, "OpenEventA failed, error %#lx\n", GetLastError());memset(&dm, 0, sizeof(dm));dm.dmSize = sizeof(dm);dm.dmFields = DM_PELSWIDTH | DM_PELSHEIGHT;dm.dmPelsWidth = 640;dm.dmPelsHeight = 480;res = ChangeDisplaySettingsExA(myARGV[3], &dm, NULL, CDS_FULLSCREEN, NULL);ok(res == DISP_CHANGE_SUCCESSFUL,"ChangeDisplaySettingsExA %s returned unexpected %ld.\n", myARGV[3], res);SetEvent(event0);CloseHandle(event0);wait_result = WaitForSingleObject(event1, 20000);
20s is really lone. Try to use the smallest timeout possible.
Zhiyi Zhang (@zhiyi) commented about dlls/user32/tests/monitor.c:
DEVMODEA original_mode;
Change the commit subject to "explorer: Restore display settings on process exit." and explain it restores the display settings to the ones in the registry when CDS_FULLSCREEN is used in ChangeDisplaySettings().
Zhiyi Zhang (@zhiyi) commented about dlls/user32/tests/monitor.c:
{ok(0, "too few arguments.\n");return;}event0 = OpenEventA(EVENT_MODIFY_STATE, FALSE, myARGV[4]);ok(!!event0, "OpenEventA failed, error %#lx\n", GetLastError());event1 = OpenEventA(SYNCHRONIZE, FALSE, myARGV[5]);ok(!!event1, "OpenEventA failed, error %#lx\n", GetLastError());memset(&dm, 0, sizeof(dm));dm.dmSize = sizeof(dm);dm.dmFields = DM_PELSWIDTH | DM_PELSHEIGHT;dm.dmPelsWidth = 640;dm.dmPelsHeight = 480;res = ChangeDisplaySettingsExA(myARGV[3], &dm, NULL, CDS_FULLSCREEN, NULL);
Please also test that scenario that one process calls ChangeDisplaySettingsExA() with CDS_FULLSCREEN and another process without CDS_FULLSCREEN and their results when either of them exits first.
Zhiyi Zhang (@zhiyi) commented about programs/explorer/desktop.c:
CloseHandle( fullscreen_process );fullscreen_process = NULL;}if (lp)fullscreen_process = OpenProcess( SYNCHRONIZE, FALSE, lp );return 0;- }
- return DefWindowProcW( hwnd, message, wp, lp );
+}
+static DWORD WINAPI display_settings_restorer_thread( void *param ) +{
- static const WCHAR display_settings_restorer_classname[] = L"__wine_display_settings_restorer";
You can use pointer instead.
Zhiyi Zhang (@zhiyi) commented about programs/explorer/desktop.c:
+static DWORD WINAPI display_settings_restorer_thread( void *param ) +{
- static const WCHAR display_settings_restorer_classname[] = L"__wine_display_settings_restorer";
- DWORD wait_result;
- WNDCLASSW class;
- ATOM atom;
- MSG msg;
- if (!wait_named_mutex( L"__wine_display_settings_restorer_mutex" )) return 0;
- memset( &class, 0, sizeof(class) );
- class.lpfnWndProc = display_settings_restorer_wndproc;
- class.lpszClassName = display_settings_restorer_classname;
- if (!(atom = RegisterClassW( &class )) && GetLastError() != ERROR_CLASS_ALREADY_EXISTS)
You don't need to save the result atom. You can use window class name instead for calling UnregisterClassW().
Zhiyi Zhang (@zhiyi) commented about programs/explorer/desktop.c:
SWP_SHOWWINDOW ); thread = CreateThread( NULL, 0, clipboard_thread, NULL, 0, &id ); if (thread) CloseHandle( thread );
thread = CreateThread( NULL, 0, display_settings_restorer_thread, NULL, 0, &id );
Let's put this in initialize_display_settings().
Zhiyi Zhang (@zhiyi) commented about programs/explorer/desktop.c:
}}
+static BOOL wait_named_mutex( const WCHAR *name ) +{
- HANDLE mutex;
- mutex = CreateMutexW( NULL, TRUE, name );
- if (GetLastError() == ERROR_ALREADY_EXISTS)
- {
TRACE( "waiting for mutex %s\n", debugstr_w( name ));WaitForSingleObject( mutex, INFINITE );- }
- return TRUE;
This function only returns TRUE so there is no point returning a value.
Zhiyi Zhang (@zhiyi) commented about dlls/user32/tests/monitor.c:
dm.dmDisplayOrientation);}
-static void test_ChangeDisplaySettingsEx(void) +#define wait_for_dm(a, b, c, d) wait_for_dm_(__LINE__, a, b, c, d) +static void wait_for_dm_(int line, const char *device, DWORD expectedWidth, DWORD expectedHeight, BOOL todo)
Let's use expected_width instead expectedWidth. Same for expectedHeight.
Zhiyi Zhang (@zhiyi) commented about dlls/user32/tests/monitor.c:
dm.dmDisplayOrientation);}
-static void test_ChangeDisplaySettingsEx(void) +#define wait_for_dm(a, b, c, d) wait_for_dm_(__LINE__, a, b, c, d) +static void wait_for_dm_(int line, const char *device, DWORD expectedWidth, DWORD expectedHeight, BOOL todo) +{
- DEVMODEA dm;
- BOOL ret;
- int i;
- for (i = 0; i < 100; ++i)
5s should be enough.
Zhiyi Zhang (@zhiyi) commented about dlls/user32/tests/monitor.c:
-static void test_ChangeDisplaySettingsEx(void) +#define wait_for_dm(a, b, c, d) wait_for_dm_(__LINE__, a, b, c, d) +static void wait_for_dm_(int line, const char *device, DWORD expectedWidth, DWORD expectedHeight, BOOL todo) +{
- DEVMODEA dm;
- BOOL ret;
- int i;
- for (i = 0; i < 100; ++i)
- {
memset(&dm, 0, sizeof(dm));dm.dmSize = sizeof(dm);SetLastError(0xdeadbeef);ret = EnumDisplaySettingsA(device, ENUM_CURRENT_SETTINGS, &dm);ok_(__FILE__, line)(ret, "Device %s EnumDisplaySettingsA failed, error %#lx\n", device, GetLastError());ok_(__FILE__, line)((dm.dmFields & (DM_PELSWIDTH | DM_PELSHEIGHT)) == (DM_PELSWIDTH | DM_PELSHEIGHT),
This is already tested elsewhere. You can remove it.
Zhiyi Zhang (@zhiyi) commented about dlls/user32/tests/monitor.c:
- STARTUPINFOA startup;
- DWORD wait_result;
- HANDLE cds_event;
- LONG res;
- cds_event = CreateEventA(NULL, FALSE, FALSE, cds_event_name);
- ok(!!cds_event, "CreateEventA failed, error %#lx\n", GetLastError());
- memset(&startup, 0, sizeof(startup));
- startup.cb = sizeof(startup);
- snprintf(buffer, sizeof(buffer), "%s monitor fullscreen %s %s %s", argv0, device,
cds_event_name, exit_event_name);- res = CreateProcessA(NULL, buffer, NULL, NULL, FALSE, 0, NULL, NULL, &startup, &info);
- ok(res, "CreateProcessA returned unexpected %ld\n", res);
- wait_result = WaitForSingleObject(cds_event, 10000);
1~2s should be enough.
Zhiyi Zhang (@zhiyi) commented about dlls/user32/tests/monitor.c:
if (dm.dmPelsWidth == expectedWidth && dm.dmPelsHeight == expectedHeight)break;Sleep(100);- }
- todo_wine_if(todo) ok_(__FILE__, line)(dm.dmPelsWidth == expectedWidth,
"Device %s expect dmPelsWidth %lu, got %lu\n", device, expectedWidth, dm.dmPelsWidth);- todo_wine_if(todo) ok_(__FILE__, line)(dm.dmPelsHeight == expectedHeight,
"Device %s expect dmPelsHeight %lu, got %lu\n", device, expectedHeight, dm.dmPelsHeight);+}
+static HANDLE test_child_process_ChangeDisplaySettingsEx(const char *argv0, const char *device, const char *exit_event_name) +{
- static const char *cds_event_name = "test_child_process_ChangeDisplaySettingsEx_cds_event";
cds already stands for ChangeDisplaySettingsEx(). Let's use "test_child_process_cds_event"
Zhiyi Zhang (@zhiyi) commented about dlls/user32/tests/monitor.c:
- res = CreateProcessA(NULL, buffer, NULL, NULL, FALSE, 0, NULL, NULL, &startup, &info);
- ok(res, "CreateProcessA returned unexpected %ld\n", res);
- wait_result = WaitForSingleObject(cds_event, 10000);
- ok(wait_result == WAIT_OBJECT_0, "WaitForSingleObject returned %lx.\n", wait_result);
- CloseHandle(cds_event);
- CloseHandle(info.hThread);
- return info.hProcess;
+}
+static void test_ChangeDisplaySettingsEx(int myARGC, char **myARGV) { static const DWORD registry_fields = DM_DISPLAYORIENTATION | DM_BITSPERPEL | DM_PELSWIDTH | DM_PELSHEIGHT | DM_DISPLAYFLAGS | DM_DISPLAYFREQUENCY | DM_POSITION;
- static const char *exit_event0_name = "test_ChangeDisplaySettingsEx_exit_event0";
use test_cds_exit_event0/1
Zhiyi Zhang (@zhiyi) commented about dlls/user32/tests/monitor.c:
- process1 = test_child_process_ChangeDisplaySettingsEx(myARGV[0], devices[0].name, exit_event1_name);
- /* Verify that the settings are restored to the current registry settings */
- for (device = 0; device < device_count; ++device)
- {
memset(&dm, 0, sizeof(dm));dm.dmSize = sizeof(dm);dm.dmFields = DM_PELSWIDTH | DM_PELSHEIGHT;dm.dmPelsWidth = 800;dm.dmPelsHeight = 600;res = ChangeDisplaySettingsExA(devices[device].name, &dm, NULL, CDS_UPDATEREGISTRY | CDS_NORESET, NULL);ok(res == DISP_CHANGE_SUCCESSFUL, "ChangeDisplaySettingsExA %s returned %ld.\n", devices[device].name, res);- }
- SetEvent(exit_event0);
- wait_result = WaitForSingleObject(process0, 10000);
5s should be enough.
Zhiyi Zhang (@zhiyi) commented about dlls/user32/tests/monitor.c:
- for (device = 0; device < device_count; ++device)
- {
memset(&dm, 0, sizeof(dm));dm.dmSize = sizeof(dm);dm.dmFields = DM_PELSWIDTH | DM_PELSHEIGHT;dm.dmPelsWidth = 800;dm.dmPelsHeight = 600;res = ChangeDisplaySettingsExA(devices[device].name, &dm, NULL, CDS_UPDATEREGISTRY | CDS_NORESET, NULL);ok(res == DISP_CHANGE_SUCCESSFUL, "ChangeDisplaySettingsExA %s returned %ld.\n", devices[device].name, res);- }
- SetEvent(exit_event0);
- wait_result = WaitForSingleObject(process0, 10000);
- ok(wait_result == WAIT_OBJECT_0, "WaitForSingleObject returned %lx.\n", wait_result);
- Sleep(100);
Why do you need this here?
Zhiyi Zhang (@zhiyi) commented about dlls/user32/tests/monitor.c:
- SetEvent(exit_event0);
- wait_result = WaitForSingleObject(process0, 10000);
- ok(wait_result == WAIT_OBJECT_0, "WaitForSingleObject returned %lx.\n", wait_result);
- Sleep(100);
- memset(&dm, 0, sizeof(dm));
- dm.dmSize = sizeof(dm);
- dm.dmPelsWidth = 640;
- dm.dmPelsHeight = 480;
- dm.dmFields = DM_PELSWIDTH | DM_PELSHEIGHT;
- expect_dm(&dm, devices[0].name, 0, TRUE);
- SetEvent(exit_event1);
- wait_result = WaitForSingleObject(process1, 10000);
Same here.
Zhiyi Zhang (@zhiyi) commented about dlls/user32/tests/monitor.c:
- startup.cb = sizeof(startup);
- snprintf(buffer, sizeof(buffer), "%s monitor fullscreen %s %s %s", argv0, device,
cds_event_name, exit_event_name);- res = CreateProcessA(NULL, buffer, NULL, NULL, FALSE, 0, NULL, NULL, &startup, &info);
- ok(res, "CreateProcessA returned unexpected %ld\n", res);
- wait_result = WaitForSingleObject(cds_event, 10000);
- ok(wait_result == WAIT_OBJECT_0, "WaitForSingleObject returned %lx.\n", wait_result);
- CloseHandle(cds_event);
- CloseHandle(info.hThread);
- return info.hProcess;
+}
+static void test_ChangeDisplaySettingsEx(int myARGC, char **myARGV)
Use argc and argv.
Zhiyi Zhang (@zhiyi) commented about dlls/user32/tests/monitor.c:
- dm.dmPelsHeight = 600;
- dm.dmFields = DM_PELSWIDTH | DM_PELSHEIGHT;
- expect_dm(&dm, devices[0].name, 0, TRUE);
- CloseHandle(process1);
- CloseHandle(process0);
- if (device_count < 2)
- {
skip("Only one device found.\n");- }
- else
- {
/* Test that the settings are restored for all devices, regardless of* the process that changed them */
This test doesn't need two processes.
Zhiyi Zhang (@zhiyi) commented about dlls/user32/tests/monitor.c:
wait_for_dm(devices[0].name, 800, 600, TRUE);wait_for_dm(devices[1].name, 800, 600, TRUE);SetEvent(exit_event0);wait_result = WaitForSingleObject(process0, 10000);ok(wait_result == WAIT_OBJECT_0, "WaitForSingleObject returned %lx.\n", wait_result);Sleep(100);memset(&dm, 0, sizeof(dm));dm.dmSize = sizeof(dm);dm.dmPelsWidth = 800;dm.dmPelsHeight = 600;dm.dmFields = DM_PELSWIDTH | DM_PELSHEIGHT;expect_dm(&dm, devices[0].name, 0, TRUE);expect_dm(&dm, devices[1].name, 0, TRUE);
You've already called wait_for_dm(). Calling expect_dm seems redundant.
Zhiyi Zhang (@zhiyi) commented about dlls/user32/tests/monitor.c:
memset(&dm, 0, sizeof(dm));dm.dmSize = sizeof(dm);dm.dmPelsWidth = 800;dm.dmPelsHeight = 600;dm.dmFields = DM_PELSWIDTH | DM_PELSHEIGHT;expect_dm(&dm, devices[0].name, 0, TRUE);expect_dm(&dm, devices[1].name, 0, TRUE);CloseHandle(process1);CloseHandle(process0);- }
- CloseHandle(exit_event1);
- CloseHandle(exit_event0);
- /* Restore all adapters to their original settings */
This part is duplicated. Could you move the tests up so that you don't have to restore them again?
Zhiyi Zhang (@zhiyi) commented about dlls/user32/tests/monitor.c:
EnumDisplayMonitors(NULL, NULL, MonitorEnumProc, 0);return;
if (strcmp(myARGV[2], "info") == 0){printf("Monitor information:\n");EnumDisplayMonitors(NULL, NULL, MonitorEnumProc, 0);return;}else if (strcmp(myARGV[2], "fullscreen") == 0){HANDLE event0, event1;DWORD wait_result;DEVMODEA dm;LONG res;if (myARGC < 6)
Place the following code into a helper so it remains clean in the main function.
Zhiyi Zhang (@zhiyi) commented about dlls/user32/tests/monitor.c:
event1 = OpenEventA(SYNCHRONIZE, FALSE, myARGV[5]);ok(!!event1, "OpenEventA failed, error %#lx\n", GetLastError());memset(&dm, 0, sizeof(dm));dm.dmSize = sizeof(dm);dm.dmFields = DM_PELSWIDTH | DM_PELSHEIGHT;dm.dmPelsWidth = 640;dm.dmPelsHeight = 480;res = ChangeDisplaySettingsExA(myARGV[3], &dm, NULL, CDS_FULLSCREEN, NULL);ok(res == DISP_CHANGE_SUCCESSFUL,"ChangeDisplaySettingsExA %s returned unexpected %ld.\n", myARGV[3], res);SetEvent(event0);CloseHandle(event0);wait_result = WaitForSingleObject(event1, 20000);
20s is really lone. Try to use the smallest timeout possible.
Zhiyi Zhang (@zhiyi) commented about dlls/user32/tests/monitor.c:
DEVMODEA original_mode;
Change the commit subject to "explorer: Restore display settings on process exit." and explain it restores the display settings to the ones in the registry when CDS_FULLSCREEN is used in ChangeDisplaySettings().
Zhiyi Zhang (@zhiyi) commented about dlls/user32/tests/monitor.c:
{ok(0, "too few arguments.\n");return;}event0 = OpenEventA(EVENT_MODIFY_STATE, FALSE, myARGV[4]);ok(!!event0, "OpenEventA failed, error %#lx\n", GetLastError());event1 = OpenEventA(SYNCHRONIZE, FALSE, myARGV[5]);ok(!!event1, "OpenEventA failed, error %#lx\n", GetLastError());memset(&dm, 0, sizeof(dm));dm.dmSize = sizeof(dm);dm.dmFields = DM_PELSWIDTH | DM_PELSHEIGHT;dm.dmPelsWidth = 640;dm.dmPelsHeight = 480;res = ChangeDisplaySettingsExA(myARGV[3], &dm, NULL, CDS_FULLSCREEN, NULL);
Please also test that scenario that one process calls ChangeDisplaySettingsExA() with CDS_FULLSCREEN and another process without CDS_FULLSCREEN and their results when either of them exits first.
Zhiyi Zhang (@zhiyi) commented about programs/explorer/desktop.c:
CloseHandle( fullscreen_process );fullscreen_process = NULL;}if (lp)fullscreen_process = OpenProcess( SYNCHRONIZE, FALSE, lp );return 0;- }
- return DefWindowProcW( hwnd, message, wp, lp );
+}
+static DWORD WINAPI display_settings_restorer_thread( void *param ) +{
- static const WCHAR display_settings_restorer_classname[] = L"__wine_display_settings_restorer";
You can use pointer instead.
Zhiyi Zhang (@zhiyi) commented about programs/explorer/desktop.c:
+static DWORD WINAPI display_settings_restorer_thread( void *param ) +{
- static const WCHAR display_settings_restorer_classname[] = L"__wine_display_settings_restorer";
- DWORD wait_result;
- WNDCLASSW class;
- ATOM atom;
- MSG msg;
- if (!wait_named_mutex( L"__wine_display_settings_restorer_mutex" )) return 0;
- memset( &class, 0, sizeof(class) );
- class.lpfnWndProc = display_settings_restorer_wndproc;
- class.lpszClassName = display_settings_restorer_classname;
- if (!(atom = RegisterClassW( &class )) && GetLastError() != ERROR_CLASS_ALREADY_EXISTS)
You don't need to save the result atom. You can use window class name instead for calling UnregisterClassW().
Zhiyi Zhang (@zhiyi) commented about programs/explorer/desktop.c:
SWP_SHOWWINDOW ); thread = CreateThread( NULL, 0, clipboard_thread, NULL, 0, &id ); if (thread) CloseHandle( thread );
thread = CreateThread( NULL, 0, display_settings_restorer_thread, NULL, 0, &id );
Let's put this in initialize_display_settings().
Zhiyi Zhang (@zhiyi) commented about programs/explorer/desktop.c:
}}
+static BOOL wait_named_mutex( const WCHAR *name ) +{
- HANDLE mutex;
- mutex = CreateMutexW( NULL, TRUE, name );
- if (GetLastError() == ERROR_ALREADY_EXISTS)
- {
TRACE( "waiting for mutex %s\n", debugstr_w( name ));WaitForSingleObject( mutex, INFINITE );- }
- return TRUE;
This function only returns TRUE so there is no point returning a value.
-
Anton Baskanov -
Anton Baskanov (＠baskanov)
-
Marvin -
Zhiyi Zhang (＠zhiyi)