[PATCH v4 0/4] MR5060: win32u, explorer, winex11.drv: Restore display settings on process exit.
-- v4: win32u, explorer: Restore display settings on process exit. winex11.drv: Process events in X11DRV_GetCurrentDisplaySettings. winex11.drv: Call X11DRV_DisplayDevices_RegisterEventHandlers in x11drv_init. user32/tests: Test that display settings are restored on process exit. https://gitlab.winehq.org/wine/wine/-/merge_requests/5060
From: Anton Baskanov <baskanov(a)gmail.com> --- dlls/user32/tests/monitor.c | 257 +++++++++++++++++++++++++++++++++--- 1 file changed, 239 insertions(+), 18 deletions(-) diff --git a/dlls/user32/tests/monitor.c b/dlls/user32/tests/monitor.c index e93a84242c4..fd78d9ff9ba 100644 --- a/dlls/user32/tests/monitor.c +++ b/dlls/user32/tests/monitor.c @@ -50,6 +50,9 @@ static DPI_AWARENESS_CONTEXT (WINAPI *pSetThreadDpiAwarenessContext)(DPI_AWARENE static NTSTATUS (WINAPI *pD3DKMTCloseAdapter)(const D3DKMT_CLOSEADAPTER*); static NTSTATUS (WINAPI *pD3DKMTOpenAdapterFromGdiDisplayName)(D3DKMT_OPENADAPTERFROMGDIDISPLAYNAME*); +char** myARGV; +int myARGC; + static void init_function_pointers(void) { HMODULE user32 = GetModuleHandleA("user32.dll"); @@ -291,8 +294,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 +310,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,6 +328,35 @@ 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) +{ + 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 void test_ChangeDisplaySettingsEx(void) { static const DWORD registry_fields = DM_DISPLAYORIENTATION | DM_BITSPERPEL | DM_PELSWIDTH | DM_PELSHEIGHT | @@ -332,11 +364,16 @@ static void test_ChangeDisplaySettingsEx(void) static const DWORD depths[] = {8, 16, 32}; DPI_AWARENESS_CONTEXT context = NULL; UINT primary, device, test, mode; + PROCESS_INFORMATION info0, info1; UINT device_size, device_count; + HANDLE event0, event1, event2; struct device_info *devices; DEVMODEA dm, dm2, dm3; + char buffer[MAX_PATH]; + STARTUPINFOA startup; INT count, old_count; DISPLAY_DEVICEA dd; + DWORD wait_result; POINTL position; DEVMODEW dmW; BOOL found; @@ -668,7 +705,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 +839,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 +912,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 +957,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 +1029,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 +1088,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 +1153,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 +1199,153 @@ 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); + + event0 = CreateEventW(NULL, FALSE, FALSE, L"test_ChangeDisplaySettingsEx_event0"); + ok(!!event0, "OpenEventA failed, error %#lx\n", GetLastError()); + event1 = CreateEventW(NULL, FALSE, FALSE, L"test_ChangeDisplaySettingsEx_event1"); + ok(!!event1, "OpenEventA failed, error %#lx\n", GetLastError()); + event2 = CreateEventW(NULL, FALSE, FALSE, L"test_ChangeDisplaySettingsEx_event2"); + ok(!!event2, "OpenEventA failed, error %#lx\n", GetLastError()); + + memset(&startup, 0, sizeof(startup)); + startup.cb = sizeof(startup); + startup.dwFlags = STARTF_USESHOWWINDOW; + startup.wShowWindow = SW_SHOWNORMAL; + + /* Test that if the most recent ChangeDisplaySettingsEx call had + * CDS_FULLSCREEN set, the the settings are restored when the caller + * process exits */ + + snprintf(buffer, sizeof(buffer), "%s monitor fullscreen %s %s %s", myARGV[0], devices[0].name, + "test_ChangeDisplaySettingsEx_event2", "test_ChangeDisplaySettingsEx_event0"); + res = CreateProcessA(NULL, buffer, NULL, NULL, FALSE, 0, NULL, NULL, &startup, &info0); + ok(res, "CreateProcessA returned unexpected %ld\n", res); + wait_result = WaitForSingleObject(event2, 10000); + ok(wait_result == WAIT_OBJECT_0, "WaitForSingleObject returned %lx.\n", wait_result); + + snprintf(buffer, sizeof(buffer), "%s monitor fullscreen %s %s %s", myARGV[0], devices[0].name, + "test_ChangeDisplaySettingsEx_event2", "test_ChangeDisplaySettingsEx_event1"); + res = CreateProcessA(NULL, buffer, NULL, NULL, FALSE, 0, NULL, NULL, &startup, &info1); + ok(res, "CreateProcessA returned unexpected %ld\n", res); + wait_result = WaitForSingleObject(event2, 10000); + ok(wait_result == WAIT_OBJECT_0, "WaitForSingleObject returned %lx.\n", wait_result); + + SetEvent(event0); + wait_result = WaitForSingleObject(info0.hProcess, 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(event1); + wait_result = WaitForSingleObject(info1.hProcess, 10000); + ok(wait_result == WAIT_OBJECT_0, "WaitForSingleObject returned %lx.\n", wait_result); + + wait_for_dm(devices[0].name, devices[0].original_mode.dmPelsWidth, devices[0].original_mode.dmPelsHeight, FALSE); + + CloseHandle(info1.hProcess); + CloseHandle(info1.hThread); + CloseHandle(info0.hProcess); + CloseHandle(info0.hThread); + + /* Test processes exiting in reverse order */ + + snprintf(buffer, sizeof(buffer), "%s monitor fullscreen %s %s %s", myARGV[0], devices[0].name, + "test_ChangeDisplaySettingsEx_event2", "test_ChangeDisplaySettingsEx_event0"); + res = CreateProcessA(NULL, buffer, NULL, NULL, FALSE, 0, NULL, NULL, &startup, &info0); + ok(res, "CreateProcessA returned unexpected %ld\n", res); + wait_result = WaitForSingleObject(event2, 10000); + ok(wait_result == WAIT_OBJECT_0, "WaitForSingleObject returned %lx.\n", wait_result); + + snprintf(buffer, sizeof(buffer), "%s monitor fullscreen %s %s %s", myARGV[0], devices[0].name, + "test_ChangeDisplaySettingsEx_event2", "test_ChangeDisplaySettingsEx_event1"); + res = CreateProcessA(NULL, buffer, NULL, NULL, FALSE, 0, NULL, NULL, &startup, &info1); + ok(res, "CreateProcessA returned unexpected %ld\n", res); + wait_result = WaitForSingleObject(event2, 10000); + ok(wait_result == WAIT_OBJECT_0, "WaitForSingleObject returned %lx.\n", wait_result); + + SetEvent(event1); + wait_result = WaitForSingleObject(info1.hProcess, 10000); + ok(wait_result == WAIT_OBJECT_0, "WaitForSingleObject returned %lx.\n", wait_result); + + wait_for_dm(devices[0].name, devices[0].original_mode.dmPelsWidth, devices[0].original_mode.dmPelsHeight, FALSE); + + SetEvent(event0); + wait_result = WaitForSingleObject(info0.hProcess, 10000); + ok(wait_result == WAIT_OBJECT_0, "WaitForSingleObject returned %lx.\n", wait_result); + + Sleep(100); + + expect_dm(&devices[0].original_mode, devices[0].name, 0, FALSE); + + CloseHandle(info1.hProcess); + CloseHandle(info1.hThread); + CloseHandle(info0.hProcess); + CloseHandle(info0.hThread); + + 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 */ + + snprintf(buffer, sizeof(buffer), "%s monitor fullscreen %s %s %s", myARGV[0], devices[0].name, + "test_ChangeDisplaySettingsEx_event2", "test_ChangeDisplaySettingsEx_event0"); + res = CreateProcessA(NULL, buffer, NULL, NULL, FALSE, 0, NULL, NULL, &startup, &info0); + ok(res, "CreateProcessA returned unexpected %ld\n", res); + wait_result = WaitForSingleObject(event2, 10000); + ok(wait_result == WAIT_OBJECT_0, "WaitForSingleObject returned %lx.\n", wait_result); + + snprintf(buffer, sizeof(buffer), "%s monitor fullscreen %s %s %s", myARGV[0], devices[1].name, + "test_ChangeDisplaySettingsEx_event2", "test_ChangeDisplaySettingsEx_event1"); + res = CreateProcessA(NULL, buffer, NULL, NULL, FALSE, 0, NULL, NULL, &startup, &info1); + ok(res, "CreateProcessA returned unexpected %ld\n", res); + wait_result = WaitForSingleObject(event2, 10000); + ok(wait_result == WAIT_OBJECT_0, "WaitForSingleObject returned %lx.\n", wait_result); + + SetEvent(event1); + wait_result = WaitForSingleObject(info1.hProcess, 10000); + ok(wait_result == WAIT_OBJECT_0, "WaitForSingleObject returned %lx.\n", wait_result); + + wait_for_dm(devices[0].name, devices[0].original_mode.dmPelsWidth, devices[0].original_mode.dmPelsHeight, FALSE); + wait_for_dm(devices[1].name, devices[1].original_mode.dmPelsWidth, devices[1].original_mode.dmPelsHeight, FALSE); + + SetEvent(event0); + wait_result = WaitForSingleObject(info0.hProcess, 10000); + ok(wait_result == WAIT_OBJECT_0, "WaitForSingleObject returned %lx.\n", wait_result); + + Sleep(100); + + dm = devices[0].original_mode; + dm.dmFields = DM_PELSWIDTH | DM_PELSHEIGHT; + expect_dm(&dm, devices[0].name, 0, FALSE); + dm = devices[1].original_mode; + dm.dmFields = DM_PELSWIDTH | DM_PELSHEIGHT; + expect_dm(&dm, devices[1].name, 0, FALSE); + + CloseHandle(info1.hProcess); + CloseHandle(info1.hThread); + CloseHandle(info0.hProcess); + CloseHandle(info0.hThread); + } + + CloseHandle(event2); + CloseHandle(event1); + CloseHandle(event0); + + /* Restore all adapters to their original settings */ + res = ChangeDisplaySettingsExA(NULL, NULL, NULL, 0, NULL); + ok(res == DISP_CHANGE_SUCCESSFUL, "ChangeDisplaySettingsExA returned unexpected %ld\n", res); free(devices); } @@ -2665,16 +2848,54 @@ BOOL CALLBACK MonitorEnumProc(HMONITOR hMonitor, HDC hdcMonitor, START_TEST(monitor) { - char** myARGV; - int myARGC = winetest_get_mainargs(&myARGV); + myARGC = winetest_get_mainargs(&myARGV); 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(); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5060
From: Anton Baskanov <baskanov(a)gmail.com> Otherwise, RRNotify events will only be handled in the explorer process. --- dlls/winex11.drv/window.c | 1 - dlls/winex11.drv/x11drv_main.c | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) 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_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; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5060
From: Anton Baskanov <baskanov(a)gmail.com> We have to invalidate the current mode cache if there are pending RRNotify events. The performance hit on EnumDisplaySettingsExW is around 15-20%. --- dlls/user32/tests/monitor.c | 16 ++++++++-------- dlls/winex11.drv/display.c | 2 ++ 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/dlls/user32/tests/monitor.c b/dlls/user32/tests/monitor.c index fd78d9ff9ba..7c9bee0cee6 100644 --- a/dlls/user32/tests/monitor.c +++ b/dlls/user32/tests/monitor.c @@ -1242,13 +1242,13 @@ static void test_ChangeDisplaySettingsEx(void) 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(event1); wait_result = WaitForSingleObject(info1.hProcess, 10000); ok(wait_result == WAIT_OBJECT_0, "WaitForSingleObject returned %lx.\n", wait_result); - wait_for_dm(devices[0].name, devices[0].original_mode.dmPelsWidth, devices[0].original_mode.dmPelsHeight, FALSE); + wait_for_dm(devices[0].name, devices[0].original_mode.dmPelsWidth, devices[0].original_mode.dmPelsHeight, TRUE); CloseHandle(info1.hProcess); CloseHandle(info1.hThread); @@ -1275,7 +1275,7 @@ static void test_ChangeDisplaySettingsEx(void) wait_result = WaitForSingleObject(info1.hProcess, 10000); ok(wait_result == WAIT_OBJECT_0, "WaitForSingleObject returned %lx.\n", wait_result); - wait_for_dm(devices[0].name, devices[0].original_mode.dmPelsWidth, devices[0].original_mode.dmPelsHeight, FALSE); + wait_for_dm(devices[0].name, devices[0].original_mode.dmPelsWidth, devices[0].original_mode.dmPelsHeight, TRUE); SetEvent(event0); wait_result = WaitForSingleObject(info0.hProcess, 10000); @@ -1283,7 +1283,7 @@ static void test_ChangeDisplaySettingsEx(void) Sleep(100); - expect_dm(&devices[0].original_mode, devices[0].name, 0, FALSE); + expect_dm(&devices[0].original_mode, devices[0].name, 0, TRUE); CloseHandle(info1.hProcess); CloseHandle(info1.hThread); @@ -1317,8 +1317,8 @@ static void test_ChangeDisplaySettingsEx(void) wait_result = WaitForSingleObject(info1.hProcess, 10000); ok(wait_result == WAIT_OBJECT_0, "WaitForSingleObject returned %lx.\n", wait_result); - wait_for_dm(devices[0].name, devices[0].original_mode.dmPelsWidth, devices[0].original_mode.dmPelsHeight, FALSE); - wait_for_dm(devices[1].name, devices[1].original_mode.dmPelsWidth, devices[1].original_mode.dmPelsHeight, FALSE); + wait_for_dm(devices[0].name, devices[0].original_mode.dmPelsWidth, devices[0].original_mode.dmPelsHeight, TRUE); + wait_for_dm(devices[1].name, devices[1].original_mode.dmPelsWidth, devices[1].original_mode.dmPelsHeight, TRUE); SetEvent(event0); wait_result = WaitForSingleObject(info0.hProcess, 10000); @@ -1328,10 +1328,10 @@ static void test_ChangeDisplaySettingsEx(void) dm = devices[0].original_mode; dm.dmFields = DM_PELSWIDTH | DM_PELSHEIGHT; - expect_dm(&dm, devices[0].name, 0, FALSE); + expect_dm(&dm, devices[0].name, 0, TRUE); dm = devices[1].original_mode; dm.dmFields = DM_PELSWIDTH | DM_PELSHEIGHT; - expect_dm(&dm, devices[1].name, 0, FALSE); + expect_dm(&dm, devices[1].name, 0, TRUE); CloseHandle(info1.hProcess); CloseHandle(info1.hThread); diff --git a/dlls/winex11.drv/display.c b/dlls/winex11.drv/display.c index 49475571f71..664ee5bed03 100644 --- a/dlls/winex11.drv/display.c +++ b/dlls/winex11.drv/display.c @@ -222,6 +222,8 @@ BOOL X11DRV_GetCurrentDisplaySettings( LPCWSTR name, BOOL is_primary, LPDEVMODEW DEVMODEW mode; x11drv_settings_id id; + X11DRV_ProcessEvents( QS_SENDMESSAGE ); + if (!settings_handler.get_id( name, is_primary, &id ) || !settings_handler.get_current_mode( id, &mode )) { ERR("Failed to get %s current display settings.\n", wine_dbgstr_w(name)); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5060
From: Anton Baskanov <baskanov(a)gmail.com> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=49674 --- dlls/user32/tests/monitor.c | 48 ++++++++--------- dlls/win32u/sysparams.c | 15 ++++++ programs/explorer/desktop.c | 104 +++++++++++++++++++++++++++++++++--- 3 files changed, 135 insertions(+), 32 deletions(-) diff --git a/dlls/user32/tests/monitor.c b/dlls/user32/tests/monitor.c index 7c9bee0cee6..fa7d0d9c1ec 100644 --- a/dlls/user32/tests/monitor.c +++ b/dlls/user32/tests/monitor.c @@ -294,8 +294,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; @@ -310,9 +310,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); @@ -328,8 +328,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; @@ -351,9 +351,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); } @@ -705,7 +705,7 @@ static void test_ChangeDisplaySettingsEx(void) 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); @@ -839,7 +839,7 @@ static void test_ChangeDisplaySettingsEx(void) } flush_events(); - expect_dm(&dm, devices[device].name, mode, FALSE); + expect_dm(&dm, devices[device].name, mode); } /* Restore settings */ @@ -912,7 +912,7 @@ static void test_ChangeDisplaySettingsEx(void) } 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)); @@ -957,7 +957,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, FALSE); + expect_dm(&dm, devices[device].name, 0); } /* Test dmPosition */ @@ -1029,7 +1029,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, 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) @@ -1088,7 +1088,7 @@ static void test_ChangeDisplaySettingsEx(void) } 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 */ @@ -1153,7 +1153,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, FALSE); + expect_dm(&dm2, devices[device].name, mode); /* EnumDisplaySettingsEx without EDS_ROTATEDMODE reports modes with current orientation */ memset(&dm3, 0, sizeof(dm3)); @@ -1199,7 +1199,7 @@ 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, FALSE); + expect_dm(&devices[device].original_mode, devices[device].name, 0); event0 = CreateEventW(NULL, FALSE, FALSE, L"test_ChangeDisplaySettingsEx_event0"); ok(!!event0, "OpenEventA failed, error %#lx\n", GetLastError()); @@ -1242,13 +1242,13 @@ static void test_ChangeDisplaySettingsEx(void) 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(event1); wait_result = WaitForSingleObject(info1.hProcess, 10000); ok(wait_result == WAIT_OBJECT_0, "WaitForSingleObject returned %lx.\n", wait_result); - wait_for_dm(devices[0].name, devices[0].original_mode.dmPelsWidth, devices[0].original_mode.dmPelsHeight, TRUE); + wait_for_dm(devices[0].name, devices[0].original_mode.dmPelsWidth, devices[0].original_mode.dmPelsHeight); CloseHandle(info1.hProcess); CloseHandle(info1.hThread); @@ -1275,7 +1275,7 @@ static void test_ChangeDisplaySettingsEx(void) wait_result = WaitForSingleObject(info1.hProcess, 10000); ok(wait_result == WAIT_OBJECT_0, "WaitForSingleObject returned %lx.\n", wait_result); - wait_for_dm(devices[0].name, devices[0].original_mode.dmPelsWidth, devices[0].original_mode.dmPelsHeight, TRUE); + wait_for_dm(devices[0].name, devices[0].original_mode.dmPelsWidth, devices[0].original_mode.dmPelsHeight); SetEvent(event0); wait_result = WaitForSingleObject(info0.hProcess, 10000); @@ -1283,7 +1283,7 @@ static void test_ChangeDisplaySettingsEx(void) Sleep(100); - expect_dm(&devices[0].original_mode, devices[0].name, 0, TRUE); + expect_dm(&devices[0].original_mode, devices[0].name, 0); CloseHandle(info1.hProcess); CloseHandle(info1.hThread); @@ -1317,8 +1317,8 @@ static void test_ChangeDisplaySettingsEx(void) wait_result = WaitForSingleObject(info1.hProcess, 10000); ok(wait_result == WAIT_OBJECT_0, "WaitForSingleObject returned %lx.\n", wait_result); - wait_for_dm(devices[0].name, devices[0].original_mode.dmPelsWidth, devices[0].original_mode.dmPelsHeight, TRUE); - wait_for_dm(devices[1].name, devices[1].original_mode.dmPelsWidth, devices[1].original_mode.dmPelsHeight, TRUE); + wait_for_dm(devices[0].name, devices[0].original_mode.dmPelsWidth, devices[0].original_mode.dmPelsHeight); + wait_for_dm(devices[1].name, devices[1].original_mode.dmPelsWidth, devices[1].original_mode.dmPelsHeight); SetEvent(event0); wait_result = WaitForSingleObject(info0.hProcess, 10000); @@ -1328,10 +1328,10 @@ static void test_ChangeDisplaySettingsEx(void) dm = devices[0].original_mode; dm.dmFields = DM_PELSWIDTH | DM_PELSHEIGHT; - expect_dm(&dm, devices[0].name, 0, TRUE); + expect_dm(&dm, devices[0].name, 0); dm = devices[1].original_mode; dm.dmFields = DM_PELSWIDTH | DM_PELSHEIGHT; - expect_dm(&dm, devices[1].name, 0, TRUE); + expect_dm(&dm, devices[1].name, 0); CloseHandle(info1.hProcess); CloseHandle(info1.hThread); diff --git a/dlls/win32u/sysparams.c b/dlls/win32u/sysparams.c index 5dcf42ff7f5..bdca389be5b 100644 --- a/dlls/win32u/sysparams.c +++ b/dlls/win32u/sysparams.c @@ -3216,12 +3216,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 ))) { @@ -3263,6 +3272,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 ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5060
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=143331 Your paranoid android. === w8 (32 bit report) === user32: monitor.c:2887: Test failed: ChangeDisplaySettingsExA \\.\DISPLAY1 returned unexpected -1. monitor.c:2887: Test failed: ChangeDisplaySettingsExA \\.\DISPLAY1 returned unexpected -1. monitor.c:1245: Test failed: Device \\.\DISPLAY1 test 0 expect dmPelsWidth 640, got 1024 monitor.c:1245: Test failed: Device \\.\DISPLAY1 test 0 expect dmPelsHeight 480, got 768 monitor.c:2887: Test failed: ChangeDisplaySettingsExA \\.\DISPLAY1 returned unexpected -1. monitor.c:2887: Test failed: ChangeDisplaySettingsExA \\.\DISPLAY1 returned unexpected -1. monitor.c:1348: Test failed: ChangeDisplaySettingsExA returned unexpected -1 === w8adm (32 bit report) === user32: monitor.c:2887: Test failed: ChangeDisplaySettingsExA \\.\DISPLAY1 returned unexpected -1. monitor.c:2887: Test failed: ChangeDisplaySettingsExA \\.\DISPLAY1 returned unexpected -1. monitor.c:1245: Test failed: Device \\.\DISPLAY1 test 0 expect dmPelsWidth 640, got 1024 monitor.c:1245: Test failed: Device \\.\DISPLAY1 test 0 expect dmPelsHeight 480, got 768 monitor.c:2887: Test failed: ChangeDisplaySettingsExA \\.\DISPLAY1 returned unexpected -1. monitor.c:2887: Test failed: ChangeDisplaySettingsExA \\.\DISPLAY1 returned unexpected -1. monitor.c:1348: Test failed: ChangeDisplaySettingsExA returned unexpected -1 === w864 (32 bit report) === user32: monitor.c:2887: Test failed: ChangeDisplaySettingsExA \\.\DISPLAY1 returned unexpected -1. monitor.c:2887: Test failed: ChangeDisplaySettingsExA \\.\DISPLAY1 returned unexpected -1. monitor.c:1245: Test failed: Device \\.\DISPLAY1 test 0 expect dmPelsWidth 640, got 1024 monitor.c:1245: Test failed: Device \\.\DISPLAY1 test 0 expect dmPelsHeight 480, got 768 monitor.c:2887: Test failed: ChangeDisplaySettingsExA \\.\DISPLAY1 returned unexpected -1. monitor.c:2887: Test failed: ChangeDisplaySettingsExA \\.\DISPLAY1 returned unexpected -1. monitor.c:1348: Test failed: ChangeDisplaySettingsExA returned unexpected -1 === w864 (64 bit report) === user32: monitor.c:2887: Test failed: ChangeDisplaySettingsExA \\.\DISPLAY1 returned unexpected -1. monitor.c:2887: Test failed: ChangeDisplaySettingsExA \\.\DISPLAY1 returned unexpected -1. monitor.c:1245: Test failed: Device \\.\DISPLAY1 test 0 expect dmPelsWidth 640, got 1024 monitor.c:1245: Test failed: Device \\.\DISPLAY1 test 0 expect dmPelsHeight 480, got 768 monitor.c:2887: Test failed: ChangeDisplaySettingsExA \\.\DISPLAY1 returned unexpected -1. monitor.c:2887: Test failed: ChangeDisplaySettingsExA \\.\DISPLAY1 returned unexpected -1. monitor.c:1348: Test failed: ChangeDisplaySettingsExA returned unexpected -1
v3: Use ChangeDisplaySettingsExW( NULL, NULL, NULL, 0, NULL ) instead of iterating over devices. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5060#note_62303
I think `ChangeDisplaySettingsExW( NULL, NULL, NULL, 0, NULL );` only applies whatever was stored in the registry? So if a process changes the registry settings, this will *not* restore the original display mode, is that intended? Generally speaking, in my opinion we should simply stop messing with the host display settings entirely and instead move toward a fully virtualized display modes. There's work in progress towards that for the wayland driver. It's still a long way to go for x11 though. This looks interesting nonetheless, even though it also raises a couple of questions like what should happen if two processes change display mode one after another? Ideally for best user experience we should probably try to always restore the initial display settings. Which maybe could be queried by explorer on startup. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5060#note_62308
So if a process changes the registry settings, this will _not_ restore the original display mode, is that intended?
Yes, native also restores to the current registry settings.
Generally speaking, in my opinion we should simply stop messing with the host display settings entirely and instead move toward a fully virtualized display modes.
Are we going to make the virtual display mode process-specific? If not, a restoration mechanism like this would still be useful.
This looks interesting nonetheless, even though it also raises a couple of questions like what should happen if two processes change display mode one after another?
The tests show that the last process to call ChangeDisplaySettings* "wins". -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5060#note_62322
Zhiyi Zhang (@zhiyi) commented about dlls/user32/tests/monitor.c:
static NTSTATUS (WINAPI *pD3DKMTCloseAdapter)(const D3DKMT_CLOSEADAPTER*); static NTSTATUS (WINAPI *pD3DKMTOpenAdapterFromGdiDisplayName)(D3DKMT_OPENADAPTERFROMGDIDISPLAYNAME*);
+char** myARGV; +int myARGC; I don't think this is necessary. You can keep them in the local scope of START_TEST(monitor) and pass their values to corresponding functions via parameters.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5060#note_62772
Zhiyi Zhang (@zhiyi) commented about dlls/user32/tests/monitor.c:
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); + + event0 = CreateEventW(NULL, FALSE, FALSE, L"test_ChangeDisplaySettingsEx_event0"); + ok(!!event0, "OpenEventA failed, error %#lx\n", GetLastError()); + event1 = CreateEventW(NULL, FALSE, FALSE, L"test_ChangeDisplaySettingsEx_event1"); + ok(!!event1, "OpenEventA failed, error %#lx\n", GetLastError()); + event2 = CreateEventW(NULL, FALSE, FALSE, L"test_ChangeDisplaySettingsEx_event2"); + ok(!!event2, "OpenEventA failed, error %#lx\n", GetLastError()); + + memset(&startup, 0, sizeof(startup)); + startup.cb = sizeof(startup); + startup.dwFlags = STARTF_USESHOWWINDOW;
I don't think you need to show any window in the child process and you can keep dwFlags and wShowWindow zero. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5060#note_62773
Zhiyi Zhang (@zhiyi) commented about dlls/user32/tests/monitor.c:
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); + + event0 = CreateEventW(NULL, FALSE, FALSE, L"test_ChangeDisplaySettingsEx_event0"); + ok(!!event0, "OpenEventA failed, error %#lx\n", GetLastError()); + event1 = CreateEventW(NULL, FALSE, FALSE, L"test_ChangeDisplaySettingsEx_event1"); + ok(!!event1, "OpenEventA failed, error %#lx\n", GetLastError()); + event2 = CreateEventW(NULL, FALSE, FALSE, L"test_ChangeDisplaySettingsEx_event2"); + ok(!!event2, "OpenEventA failed, error %#lx\n", GetLastError());
I think only one child_process_exit_event should be enough. You start a child process. The child process changes display mode according to argv parameters, set process exit events, and then exits. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5060#note_62774
Zhiyi Zhang (@zhiyi) commented about dlls/user32/tests/monitor.c:
+ ok(!!event1, "OpenEventA failed, error %#lx\n", GetLastError()); + event2 = CreateEventW(NULL, FALSE, FALSE, L"test_ChangeDisplaySettingsEx_event2"); + ok(!!event2, "OpenEventA failed, error %#lx\n", GetLastError()); + + memset(&startup, 0, sizeof(startup)); + startup.cb = sizeof(startup); + startup.dwFlags = STARTF_USESHOWWINDOW; + startup.wShowWindow = SW_SHOWNORMAL; + + /* Test that if the most recent ChangeDisplaySettingsEx call had + * CDS_FULLSCREEN set, the the settings are restored when the caller + * process exits */ + + snprintf(buffer, sizeof(buffer), "%s monitor fullscreen %s %s %s", myARGV[0], devices[0].name, + "test_ChangeDisplaySettingsEx_event2", "test_ChangeDisplaySettingsEx_event0"); + res = CreateProcessA(NULL, buffer, NULL, NULL, FALSE, 0, NULL, NULL, &startup, &info0); Please introduce a helper that launches the child process. For example, test_child_process_ChangeDisplaySettingsEx(CHAR *adapter_name, DWORD flags). Then you can use the helper to launch processes and call ChangeDisplaySettingsEx() with or without CDS_FULLSCREEN for the specified GDI adapter name. You can also call the helper in a different order and test that the last call to ChangeDisplaySettingsEx() CDS_FULLSCREEN wins and the registry display settings should be restored in such order.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5060#note_62775
Zhiyi Zhang (@zhiyi) commented about dlls/winex11.drv/x11drv_main.c:
init_user_driver(); X11DRV_DisplayDevices_Init(FALSE); + X11DRV_DisplayDevices_RegisterEventHandlers();
This patch should be merged with patch 3. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5060#note_62776
Zhiyi Zhang (@zhiyi) commented about dlls/winex11.drv/display.c:
DEVMODEW mode; x11drv_settings_id id;
+ X11DRV_ProcessEvents( QS_SENDMESSAGE );
We should only check RRNotify events here instead of all sent messages. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5060#note_62777
Zhiyi Zhang (@zhiyi) commented about dlls/user32/tests/monitor.c:
}
flush_events(); - expect_dm(&dm, devices[device].name, 0, FALSE); + expect_dm(&dm, devices[device].name, 0);
I think you can add the tests last so these back-and-forth changes can be avoided. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5060#note_62778
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 );
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. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5060#note_62779
participants (5)
-
Anton Baskanov -
Anton Baskanov (＠baskanov)
-
Marvin -
Rémi Bernon
-
Zhiyi Zhang (＠zhiyi)