With this series it's now possible to run and pass `user32:monitor` and `user32:sysparams` tests with nulldrv, and so most `user32` tests (except for a few desktop cursor position tests). This still requires some prefix configuration to enable the nulldrv driver, or a change like https://gitlab.winehq.org/rbernon/wine/-/commit/753368ad0ec52f03f8d6e78ca794... to enable it when `DISPLAY` environment variable is unset.
This then shows that some of the user32 tests are failing with winex11 but passing with nulldrv, as in https://gitlab.winehq.org/rbernon/wine/-/commit/6d5f4109a514a0dc266899fcacfa....
-- v11: win32u: Read mode from the registry if GetCurrentDisplaySettings fails. win32u: Write display settings to the registry in apply_display_settings. win32u: Lock display devices while applying display settings. winemac.drv: Avoid calling back to win32u to find the primary adapter. winex11.drv: Avoid calling back to win32u to find the primary adapter. win32u: Add an is_primary parameter to ChangeDisplaySettings. win32u: Force update display cache after NtUserChangeDisplaySettingsEx. win32u: Add a BOOL force parameter to update_display_cache.
From: Rémi Bernon rbernon@codeweavers.com
And call it recursively with force = TRUE instead of calling graphics driver pUpdateDisplayDevices separately. --- dlls/win32u/sysparams.c | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-)
diff --git a/dlls/win32u/sysparams.c b/dlls/win32u/sysparams.c index 6b5a6f65aef..e55ffa98cdb 100644 --- a/dlls/win32u/sysparams.c +++ b/dlls/win32u/sysparams.c @@ -1552,7 +1552,7 @@ static BOOL update_display_cache_from_registry(void) return ret; }
-static BOOL update_display_cache(void) +static BOOL update_display_cache( BOOL force ) { HWINSTA winstation = NtUserGetProcessWindowStation(); struct device_manager_ctx ctx = {0}; @@ -1569,17 +1569,7 @@ static BOOL update_display_cache(void) return TRUE; }
- user_driver->pUpdateDisplayDevices( &device_manager, FALSE, &ctx ); - release_display_manager_ctx( &ctx ); - - if (update_display_cache_from_registry()) return TRUE; - if (ctx.gpu_count) - { - ERR( "driver reported devices, but we failed to read them\n" ); - return FALSE; - } - - if (!user_driver->pUpdateDisplayDevices( &device_manager, TRUE, &ctx )) + if (!user_driver->pUpdateDisplayDevices( &device_manager, force, &ctx ) && force) { static const DEVMODEW modes[] = { @@ -1618,15 +1608,27 @@ static BOOL update_display_cache(void)
if (!update_display_cache_from_registry()) { - ERR( "failed to read display config\n" ); - return FALSE; + if (force) + { + ERR( "Failed to read display config.\n" ); + return FALSE; + } + + if (ctx.gpu_count) + { + ERR( "Driver reported devices, but we failed to read them.\n" ); + return FALSE; + } + + return update_display_cache( TRUE ); } + return TRUE; }
static BOOL lock_display_devices(void) { - if (!update_display_cache()) return FALSE; + if (!update_display_cache( FALSE )) return FALSE; pthread_mutex_lock( &display_lock ); return TRUE; }
From: Rémi Bernon rbernon@codeweavers.com
This makes sure the driver does not need to call back to win32u when changing display settings, and allows us to lock display devices in apply_display_settings in next patch. --- dlls/win32u/sysparams.c | 3 +++ dlls/winemac.drv/display.c | 4 +--- dlls/winex11.drv/display.c | 2 -- 3 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/dlls/win32u/sysparams.c b/dlls/win32u/sysparams.c index e55ffa98cdb..6353467737e 100644 --- a/dlls/win32u/sysparams.c +++ b/dlls/win32u/sysparams.c @@ -2483,6 +2483,9 @@ static LONG apply_display_settings( const WCHAR *devname, const DEVMODEW *devmod free( displays ); if (ret) return ret;
+ if (!update_display_cache( TRUE )) + WARN( "Failed to update display cache after mode change.\n" ); + if ((adapter = find_adapter( NULL ))) { DEVMODEW current_mode = {.dmSize = sizeof(DEVMODEW)}; diff --git a/dlls/winemac.drv/display.c b/dlls/winemac.drv/display.c index e8d26dfd241..9012f54ad3d 100644 --- a/dlls/winemac.drv/display.c +++ b/dlls/winemac.drv/display.c @@ -830,9 +830,7 @@ LONG macdrv_ChangeDisplaySettings(LPDEVMODEW displays, HWND hwnd, DWORD flags, L bpp, mode->dmDisplayFrequency); ret = DISP_CHANGE_BADMODE; } - else if (macdrv_set_display_mode(&macdrv_displays[0], best_display_mode)) - macdrv_init_display_devices(TRUE); - else + else if (!macdrv_set_display_mode(&macdrv_displays[0], best_display_mode)) { WARN("Failed to set display mode\n"); ret = DISP_CHANGE_FAILED; diff --git a/dlls/winex11.drv/display.c b/dlls/winex11.drv/display.c index 572b81aa491..2a4a34e2c02 100644 --- a/dlls/winex11.drv/display.c +++ b/dlls/winex11.drv/display.c @@ -418,8 +418,6 @@ LONG X11DRV_ChangeDisplaySettings( LPDEVMODEW displays, HWND hwnd, DWORD flags, ret = apply_display_settings( displays, ids, FALSE ); if (ret == DISP_CHANGE_SUCCESSFUL) ret = apply_display_settings( displays, ids, TRUE ); - if (ret == DISP_CHANGE_SUCCESSFUL) - X11DRV_DisplayDevices_Init(TRUE);
done: free( ids );
From: Rémi Bernon rbernon@codeweavers.com
--- dlls/win32u/driver.c | 6 +++--- dlls/win32u/sysparams.c | 21 ++++++++++++++++----- dlls/wineandroid.drv/init.c | 2 +- dlls/winemac.drv/display.c | 4 ++-- dlls/winemac.drv/macdrv.h | 2 +- dlls/winex11.drv/display.c | 2 +- dlls/winex11.drv/x11drv.h | 2 +- include/wine/gdi_driver.h | 2 +- 8 files changed, 26 insertions(+), 15 deletions(-)
diff --git a/dlls/win32u/driver.c b/dlls/win32u/driver.c index ee4a6534b50..fc777362d93 100644 --- a/dlls/win32u/driver.c +++ b/dlls/win32u/driver.c @@ -759,7 +759,7 @@ static LONG nulldrv_ChangeDisplaySettings( LPDEVMODEW displays, HWND hwnd, return DISP_CHANGE_FAILED; }
-static BOOL nulldrv_GetCurrentDisplaySettings( LPCWSTR name, LPDEVMODEW mode ) +static BOOL nulldrv_GetCurrentDisplaySettings( LPCWSTR name, BOOL is_primary, LPDEVMODEW mode ) { return FALSE; } @@ -1072,9 +1072,9 @@ static LONG loaderdrv_ChangeDisplaySettings( LPDEVMODEW displays, HWND hwnd, return load_driver()->pChangeDisplaySettings( displays, hwnd, flags, lparam ); }
-static BOOL loaderdrv_GetCurrentDisplaySettings( LPCWSTR name, LPDEVMODEW mode ) +static BOOL loaderdrv_GetCurrentDisplaySettings( LPCWSTR name, BOOL is_primary, LPDEVMODEW mode ) { - return load_driver()->pGetCurrentDisplaySettings( name, mode ); + return load_driver()->pGetCurrentDisplaySettings( name, is_primary, mode ); }
static void loaderdrv_SetCursor( HCURSOR cursor ) diff --git a/dlls/win32u/sysparams.c b/dlls/win32u/sysparams.c index 6353467737e..abd576b0d1d 100644 --- a/dlls/win32u/sysparams.c +++ b/dlls/win32u/sysparams.c @@ -542,6 +542,15 @@ static BOOL adapter_get_registry_settings( const struct adapter *adapter, DEVMOD return ret; }
+static BOOL adapter_get_current_settings( const struct adapter *adapter, DEVMODEW *mode ) +{ + BOOL ret, is_primary = !!(adapter->dev.state_flags & DISPLAY_DEVICE_PRIMARY_DEVICE); + + if ((ret = user_driver->pGetCurrentDisplaySettings( adapter->dev.device_name, is_primary, mode ))) return TRUE; + + return ret; +} + static BOOL adapter_set_registry_settings( const struct adapter *adapter, const DEVMODEW *mode ) { HANDLE mutex; @@ -2214,7 +2223,8 @@ static BOOL adapter_get_full_mode( const struct adapter *adapter, const DEVMODEW if (!is_detached_mode( full_mode ) && (!full_mode->dmPelsWidth || !full_mode->dmPelsHeight || !(full_mode->dmFields & DM_POSITION))) { DEVMODEW current_mode = {.dmSize = sizeof(DEVMODEW)}; - if (!user_driver->pGetCurrentDisplaySettings( adapter->dev.device_name, ¤t_mode )) return FALSE; + + if (!adapter_get_current_settings( adapter, ¤t_mode )) return FALSE; if (!full_mode->dmPelsWidth) full_mode->dmPelsWidth = current_mode.dmPelsWidth; if (!full_mode->dmPelsHeight) full_mode->dmPelsHeight = current_mode.dmPelsHeight; if (!(full_mode->dmFields & DM_POSITION)) @@ -2255,7 +2265,7 @@ static DEVMODEW *get_display_settings( const WCHAR *devname, const DEVMODEW *dev else { if (!devname) ret = adapter_get_registry_settings( adapter, mode ); - else ret = user_driver->pGetCurrentDisplaySettings( adapter->dev.device_name, mode ); + else ret = adapter_get_current_settings( adapter, mode ); if (!ret) goto done; }
@@ -2489,7 +2499,8 @@ static LONG apply_display_settings( const WCHAR *devname, const DEVMODEW *devmod if ((adapter = find_adapter( NULL ))) { DEVMODEW current_mode = {.dmSize = sizeof(DEVMODEW)}; - user_driver->pGetCurrentDisplaySettings( adapter->dev.device_name, ¤t_mode ); + + if (!adapter_get_current_settings( adapter, ¤t_mode )) WARN( "Failed to get primary adapter current display settings.\n" ); adapter_release( adapter );
send_notify_message( NtUserGetDesktopWindow(), WM_DISPLAYCHANGE, current_mode.dmBitsPerPel, @@ -2534,7 +2545,7 @@ static BOOL adapter_enum_display_settings( const struct adapter *adapter, DWORD DEVMODEW current_mode = {.dmSize = sizeof(DEVMODEW)}; const DEVMODEW *adapter_mode;
- if (!(flags & EDS_ROTATEDMODE) && !user_driver->pGetCurrentDisplaySettings( adapter->dev.device_name, ¤t_mode )) + if (!(flags & EDS_ROTATEDMODE) && !adapter_get_current_settings( adapter, ¤t_mode )) { WARN( "Failed to query current display mode for EDS_ROTATEDMODE flag.\n" ); return FALSE; @@ -2582,7 +2593,7 @@ BOOL WINAPI NtUserEnumDisplaySettings( UNICODE_STRING *device, DWORD index, DEVM devmode->dmDriverExtra = 0;
if (index == ENUM_REGISTRY_SETTINGS) ret = adapter_get_registry_settings( adapter, devmode ); - else if (index == ENUM_CURRENT_SETTINGS) ret = user_driver->pGetCurrentDisplaySettings( adapter->dev.device_name, devmode ); + else if (index == ENUM_CURRENT_SETTINGS) ret = adapter_get_current_settings( adapter, devmode ); else ret = adapter_enum_display_settings( adapter, index, devmode, flags ); adapter_release( adapter );
diff --git a/dlls/wineandroid.drv/init.c b/dlls/wineandroid.drv/init.c index 6011e20c8ce..dedb6ca0ea5 100644 --- a/dlls/wineandroid.drv/init.c +++ b/dlls/wineandroid.drv/init.c @@ -306,7 +306,7 @@ BOOL ANDROID_UpdateDisplayDevices( const struct gdi_device_manager *device_manag /*********************************************************************** * ANDROID_GetCurrentDisplaySettings */ -BOOL ANDROID_GetCurrentDisplaySettings( LPCWSTR name, LPDEVMODEW devmode ) +BOOL ANDROID_GetCurrentDisplaySettings( LPCWSTR name, BOOL is_primary, LPDEVMODEW devmode ) { devmode->u2.dmDisplayFlags = 0; devmode->u1.s2.dmPosition.x = 0; diff --git a/dlls/winemac.drv/display.c b/dlls/winemac.drv/display.c index 9012f54ad3d..ad746320ba9 100644 --- a/dlls/winemac.drv/display.c +++ b/dlls/winemac.drv/display.c @@ -915,7 +915,7 @@ static DEVMODEW *display_get_modes(CGDirectDisplayID display_id, int *modes_coun * GetCurrentDisplaySettings (MACDRV.@) * */ -BOOL macdrv_GetCurrentDisplaySettings(LPCWSTR devname, LPDEVMODEW devmode) +BOOL macdrv_GetCurrentDisplaySettings(LPCWSTR devname, BOOL is_primary, LPDEVMODEW devmode) { struct macdrv_display *displays = NULL; int num_displays, display_idx; @@ -923,7 +923,7 @@ BOOL macdrv_GetCurrentDisplaySettings(LPCWSTR devname, LPDEVMODEW devmode) CGDirectDisplayID display_id; WCHAR *end;
- TRACE("%s, %p + %hu\n", debugstr_w(devname), devmode, devmode->dmSize); + TRACE("%s, %u, %p + %hu\n", debugstr_w(devname), is_primary, devmode, devmode->dmSize);
init_original_display_mode();
diff --git a/dlls/winemac.drv/macdrv.h b/dlls/winemac.drv/macdrv.h index ea51cf0f702..b00ceca7c3f 100644 --- a/dlls/winemac.drv/macdrv.h +++ b/dlls/winemac.drv/macdrv.h @@ -123,7 +123,7 @@ static inline struct macdrv_thread_data *macdrv_thread_data(void) extern BOOL macdrv_ActivateKeyboardLayout(HKL hkl, UINT flags) DECLSPEC_HIDDEN; extern void macdrv_Beep(void) DECLSPEC_HIDDEN; extern LONG macdrv_ChangeDisplaySettings(LPDEVMODEW displays, HWND hwnd, DWORD flags, LPVOID lpvoid) DECLSPEC_HIDDEN; -extern BOOL macdrv_GetCurrentDisplaySettings(LPCWSTR name, LPDEVMODEW devmode) DECLSPEC_HIDDEN; +extern BOOL macdrv_GetCurrentDisplaySettings(LPCWSTR name, BOOL is_primary, LPDEVMODEW devmode) DECLSPEC_HIDDEN; extern LRESULT macdrv_ClipboardWindowProc(HWND hwnd, UINT msg, WPARAM wp, LPARAM lp) DECLSPEC_HIDDEN; extern BOOL macdrv_UpdateDisplayDevices( const struct gdi_device_manager *device_manager, BOOL force, void *param ) DECLSPEC_HIDDEN; diff --git a/dlls/winex11.drv/display.c b/dlls/winex11.drv/display.c index 2a4a34e2c02..51bca969450 100644 --- a/dlls/winex11.drv/display.c +++ b/dlls/winex11.drv/display.c @@ -266,7 +266,7 @@ static DWORD get_display_depth(ULONG_PTR display_id) * GetCurrentDisplaySettings (X11DRV.@) * */ -BOOL X11DRV_GetCurrentDisplaySettings( LPCWSTR name, LPDEVMODEW devmode ) +BOOL X11DRV_GetCurrentDisplaySettings( LPCWSTR name, BOOL is_primary, LPDEVMODEW devmode ) { DEVMODEW mode; ULONG_PTR id; diff --git a/dlls/winex11.drv/x11drv.h b/dlls/winex11.drv/x11drv.h index f8f8fe3d4d1..caf669c6a5d 100644 --- a/dlls/winex11.drv/x11drv.h +++ b/dlls/winex11.drv/x11drv.h @@ -214,7 +214,7 @@ extern BOOL X11DRV_SetCursorPos( INT x, INT y ) DECLSPEC_HIDDEN; extern BOOL X11DRV_GetCursorPos( LPPOINT pos ) DECLSPEC_HIDDEN; extern BOOL X11DRV_ClipCursor( LPCRECT clip ) DECLSPEC_HIDDEN; extern LONG X11DRV_ChangeDisplaySettings( LPDEVMODEW displays, HWND hwnd, DWORD flags, LPVOID lpvoid ) DECLSPEC_HIDDEN; -extern BOOL X11DRV_GetCurrentDisplaySettings( LPCWSTR name, LPDEVMODEW devmode ) DECLSPEC_HIDDEN; +extern BOOL X11DRV_GetCurrentDisplaySettings( LPCWSTR name, BOOL is_primary, LPDEVMODEW devmode ) DECLSPEC_HIDDEN; extern BOOL X11DRV_UpdateDisplayDevices( const struct gdi_device_manager *device_manager, BOOL force, void *param ) DECLSPEC_HIDDEN; extern BOOL X11DRV_CreateDesktopWindow( HWND hwnd ) DECLSPEC_HIDDEN; diff --git a/include/wine/gdi_driver.h b/include/wine/gdi_driver.h index 596d27156af..1e9584f2be2 100644 --- a/include/wine/gdi_driver.h +++ b/include/wine/gdi_driver.h @@ -298,7 +298,7 @@ struct user_driver_funcs void (*pUpdateClipboard)(void); /* display modes */ LONG (*pChangeDisplaySettings)(LPDEVMODEW,HWND,DWORD,LPVOID); - BOOL (*pGetCurrentDisplaySettings)(LPCWSTR,LPDEVMODEW); + BOOL (*pGetCurrentDisplaySettings)(LPCWSTR,BOOL,LPDEVMODEW); BOOL (*pUpdateDisplayDevices)(const struct gdi_device_manager *,BOOL,void*); /* windowing functions */ BOOL (*pCreateDesktopWindow)(HWND);
From: Rémi Bernon rbernon@codeweavers.com
--- dlls/winex11.drv/desktop.c | 11 +++------- dlls/winex11.drv/display.c | 41 +++++++++++++------------------------ dlls/winex11.drv/x11drv.h | 3 +-- dlls/winex11.drv/xrandr.c | 11 +++------- dlls/winex11.drv/xvidmode.c | 9 ++------ 5 files changed, 23 insertions(+), 52 deletions(-)
diff --git a/dlls/winex11.drv/desktop.c b/dlls/winex11.drv/desktop.c index c6a08291f6a..1b1d1ff13bf 100644 --- a/dlls/winex11.drv/desktop.c +++ b/dlls/winex11.drv/desktop.c @@ -123,15 +123,10 @@ BOOL is_virtual_desktop(void) }
/* Virtual desktop display settings handler */ -static BOOL X11DRV_desktop_get_id( const WCHAR *device_name, ULONG_PTR *id ) +static BOOL X11DRV_desktop_get_id( const WCHAR *device_name, BOOL is_primary, ULONG_PTR *id ) { - WCHAR primary_adapter[CCHDEVICENAME]; - - if (!get_primary_adapter( primary_adapter ) || wcsicmp( primary_adapter, device_name )) - return FALSE; - - *id = 0; - return TRUE; + if (is_primary) *id = 0; + return is_primary; }
static void add_desktop_mode( DEVMODEW *mode, DWORD depth, DWORD width, DWORD height ) diff --git a/dlls/winex11.drv/display.c b/dlls/winex11.drv/display.c index 51bca969450..e6407cda851 100644 --- a/dlls/winex11.drv/display.c +++ b/dlls/winex11.drv/display.c @@ -66,14 +66,9 @@ void X11DRV_Settings_SetHandler(const struct x11drv_settings_handler *new_handle * Default handlers if resolution switching is not enabled * */ -static BOOL nores_get_id(const WCHAR *device_name, ULONG_PTR *id) +static BOOL nores_get_id(const WCHAR *device_name, BOOL is_primary, ULONG_PTR *id) { - WCHAR primary_adapter[CCHDEVICENAME]; - - if (!get_primary_adapter( primary_adapter )) - return FALSE; - - *id = !wcsicmp( device_name, primary_adapter ) ? 1 : 0; + *id = is_primary ? 1 : 0; return TRUE; }
@@ -196,24 +191,6 @@ void init_registry_display_settings(void) } }
-BOOL get_primary_adapter(WCHAR *name) -{ - DISPLAY_DEVICEW dd; - DWORD i; - - dd.cb = sizeof(dd); - for (i = 0; !NtUserEnumDisplayDevices( NULL, i, &dd, 0 ); ++i) - { - if (dd.StateFlags & DISPLAY_DEVICE_PRIMARY_DEVICE) - { - lstrcpyW(name, dd.DeviceName); - return TRUE; - } - } - - return FALSE; -} - static void set_display_depth(ULONG_PTR display_id, DWORD depth) { struct x11drv_display_depth *display_depth; @@ -271,7 +248,7 @@ BOOL X11DRV_GetCurrentDisplaySettings( LPCWSTR name, BOOL is_primary, LPDEVMODEW DEVMODEW mode; ULONG_PTR id;
- if (!settings_handler.get_id( name, &id ) || !settings_handler.get_current_mode( id, &mode )) + 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)); return FALSE; @@ -386,6 +363,16 @@ static LONG apply_display_settings( DEVMODEW *displays, ULONG_PTR *ids, BOOL do_ return DISP_CHANGE_SUCCESSFUL; }
+static BOOL is_primary_display( DEVMODEW *mode ) +{ + if (is_detached_mode( mode )) return FALSE; + return mode->dmFields & DM_POSITION && + mode->dmFields & DM_PELSWIDTH && + mode->dmFields & DM_PELSHEIGHT && + mode->dmPosition.x == 0 && + mode->dmPosition.y == 0; +} + /*********************************************************************** * ChangeDisplaySettings (X11DRV.@) * @@ -409,7 +396,7 @@ LONG X11DRV_ChangeDisplaySettings( LPDEVMODEW displays, HWND hwnd, DWORD flags, if (!(ids = calloc( count, sizeof(*ids) ))) return DISP_CHANGE_FAILED; for (count = 0, mode = displays; mode->dmSize; mode = NEXT_DEVMODEW(mode), count++) { - if (!settings_handler.get_id( mode->dmDeviceName, ids + count )) goto done; + if (!settings_handler.get_id( mode->dmDeviceName, is_primary_display( mode ), ids + count )) goto done; mode->dmPosition.x -= left_most; mode->dmPosition.y -= top_most; } diff --git a/dlls/winex11.drv/x11drv.h b/dlls/winex11.drv/x11drv.h index caf669c6a5d..d93f9b2098e 100644 --- a/dlls/winex11.drv/x11drv.h +++ b/dlls/winex11.drv/x11drv.h @@ -710,7 +710,7 @@ struct x11drv_settings_handler * Following functions use this id to identify the device. * * Return FALSE if the device cannot be found and TRUE on success */ - BOOL (*get_id)(const WCHAR *device_name, ULONG_PTR *id); + BOOL (*get_id)(const WCHAR *device_name, BOOL is_primary, ULONG_PTR *id);
/* get_modes() will be called to get a list of supported modes of the device of id in modes * with respect to flags, which could be 0, EDS_RAWMODE or EDS_ROTATEDMODE. If the implementation @@ -751,7 +751,6 @@ extern BOOL is_virtual_desktop(void) DECLSPEC_HIDDEN; extern BOOL is_desktop_fullscreen(void) DECLSPEC_HIDDEN; extern BOOL is_detached_mode(const DEVMODEW *) DECLSPEC_HIDDEN; extern BOOL create_desktop_win_data( Window win ) DECLSPEC_HIDDEN; -extern BOOL get_primary_adapter(WCHAR *) DECLSPEC_HIDDEN; void X11DRV_Settings_Init(void) DECLSPEC_HIDDEN;
void X11DRV_XF86VM_Init(void) DECLSPEC_HIDDEN; diff --git a/dlls/winex11.drv/xrandr.c b/dlls/winex11.drv/xrandr.c index c7f922b1aae..2b75e3b28eb 100644 --- a/dlls/winex11.drv/xrandr.c +++ b/dlls/winex11.drv/xrandr.c @@ -144,17 +144,12 @@ static int XRandRErrorHandler(Display *dpy, XErrorEvent *event, void *arg) }
/* XRandR 1.0 display settings handler */ -static BOOL xrandr10_get_id( const WCHAR *device_name, ULONG_PTR *id ) +static BOOL xrandr10_get_id( const WCHAR *device_name, BOOL is_primary, ULONG_PTR *id ) { - WCHAR primary_adapter[CCHDEVICENAME]; - - if (!get_primary_adapter( primary_adapter )) - return FALSE; - /* RandR 1.0 only supports changing the primary adapter settings. * For non-primary adapters, an id is still provided but getting * and changing non-primary adapters' settings will be ignored. */ - *id = !wcsicmp( device_name, primary_adapter ) ? 1 : 0; + *id = is_primary ? 1 : 0; return TRUE; }
@@ -1224,7 +1219,7 @@ static void xrandr14_register_event_handlers(void) }
/* XRandR 1.4 display settings handler */ -static BOOL xrandr14_get_id( const WCHAR *device_name, ULONG_PTR *id ) +static BOOL xrandr14_get_id( const WCHAR *device_name, BOOL is_primary, ULONG_PTR *id ) { struct current_mode *tmp_modes, *new_current_modes = NULL; INT gpu_count, adapter_count, new_current_mode_count = 0; diff --git a/dlls/winex11.drv/xvidmode.c b/dlls/winex11.drv/xvidmode.c index 95342a84223..3a3932b611b 100644 --- a/dlls/winex11.drv/xvidmode.c +++ b/dlls/winex11.drv/xvidmode.c @@ -85,17 +85,12 @@ static int XVidModeErrorHandler(Display *dpy, XErrorEvent *event, void *arg) }
/* XF86VidMode display settings handler */ -static BOOL xf86vm_get_id(const WCHAR *device_name, ULONG_PTR *id) +static BOOL xf86vm_get_id(const WCHAR *device_name, BOOL is_primary, ULONG_PTR *id) { - WCHAR primary_adapter[CCHDEVICENAME]; - - if (!get_primary_adapter( primary_adapter )) - return FALSE; - /* XVidMode only supports changing the primary adapter settings. * For non-primary adapters, an id is still provided but getting * and changing non-primary adapters' settings will be ignored. */ - *id = !wcsicmp( device_name, primary_adapter ) ? 1 : 0; + *id = is_primary ? 1 : 0; return TRUE; }
From: Rémi Bernon rbernon@codeweavers.com
--- dlls/winemac.drv/display.c | 34 +++++++++++----------------------- 1 file changed, 11 insertions(+), 23 deletions(-)
diff --git a/dlls/winemac.drv/display.c b/dlls/winemac.drv/display.c index ad746320ba9..1d675a49ef2 100644 --- a/dlls/winemac.drv/display.c +++ b/dlls/winemac.drv/display.c @@ -685,24 +685,6 @@ void check_retina_status(void) } }
-static BOOL get_primary_adapter(WCHAR *name) -{ - DISPLAY_DEVICEW dd; - DWORD i; - - dd.cb = sizeof(dd); - for (i = 0; !NtUserEnumDisplayDevices(NULL, i, &dd, 0); ++i) - { - if (dd.StateFlags & DISPLAY_DEVICE_PRIMARY_DEVICE) - { - lstrcpyW(name, dd.DeviceName); - return TRUE; - } - } - - return FALSE; -} - static BOOL is_detached_mode(const DEVMODEW *mode) { return mode->dmFields & DM_POSITION && @@ -765,13 +747,22 @@ static CGDisplayModeRef find_best_display_mode(DEVMODEW *devmode, CFArrayRef dis return best_display_mode; }
+static BOOL is_primary_display( DEVMODEW *mode ) +{ + if (is_detached_mode( mode )) return FALSE; + return mode->dmFields & DM_POSITION && + mode->dmFields & DM_PELSWIDTH && + mode->dmFields & DM_PELSHEIGHT && + mode->dmPosition.x == 0 && + mode->dmPosition.y == 0; +} + /*********************************************************************** * ChangeDisplaySettings (MACDRV.@) * */ LONG macdrv_ChangeDisplaySettings(LPDEVMODEW displays, HWND hwnd, DWORD flags, LPVOID lpvoid) { - WCHAR primary_adapter[CCHDEVICENAME]; LONG ret = DISP_CHANGE_SUCCESSFUL; DEVMODEW *mode; int bpp; @@ -785,9 +776,6 @@ LONG macdrv_ChangeDisplaySettings(LPDEVMODEW displays, HWND hwnd, DWORD flags, L
init_original_display_mode();
- if (!get_primary_adapter(primary_adapter)) - return DISP_CHANGE_FAILED; - if (macdrv_get_displays(&macdrv_displays, &num_displays)) return DISP_CHANGE_FAILED;
@@ -804,7 +792,7 @@ LONG macdrv_ChangeDisplaySettings(LPDEVMODEW displays, HWND hwnd, DWORD flags, L
for (mode = displays; mode->dmSize && !ret; mode = NEXT_DEVMODEW(mode)) { - if (wcsicmp(primary_adapter, mode->dmDeviceName)) + if (!is_primary_display(mode)) { FIXME("Changing non-primary adapter settings is currently unsupported.\n"); continue;
From: Rémi Bernon rbernon@codeweavers.com
--- dlls/win32u/sysparams.c | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-)
diff --git a/dlls/win32u/sysparams.c b/dlls/win32u/sysparams.c index abd576b0d1d..7e6bebbcf72 100644 --- a/dlls/win32u/sysparams.c +++ b/dlls/win32u/sysparams.c @@ -2251,10 +2251,8 @@ static DEVMODEW *get_display_settings( const WCHAR *devname, const DEVMODEW *dev struct adapter *adapter; BOOL ret;
- if (!lock_display_devices()) return NULL; - /* allocate an extra mode for easier iteration */ - if (!(displays = calloc( list_count( &adapters ) + 1, sizeof(DEVMODEW) ))) goto done; + if (!(displays = calloc( list_count( &adapters ) + 1, sizeof(DEVMODEW) ))) return NULL; mode = displays;
LIST_FOR_EACH_ENTRY( adapter, &adapters, struct adapter, entry ) @@ -2266,20 +2264,18 @@ static DEVMODEW *get_display_settings( const WCHAR *devname, const DEVMODEW *dev { if (!devname) ret = adapter_get_registry_settings( adapter, mode ); else ret = adapter_get_current_settings( adapter, mode ); - if (!ret) goto done; + if (!ret) + { + free( displays ); + return NULL; + } }
lstrcpyW( mode->dmDeviceName, adapter->dev.device_name ); mode = NEXT_DEVMODEW(mode); }
- unlock_display_devices(); return displays; - -done: - unlock_display_devices(); - free( displays ); - return NULL; }
static INT offset_length( POINT offset ) @@ -2476,11 +2472,16 @@ static LONG apply_display_settings( const WCHAR *devname, const DEVMODEW *devmod DEVMODEW *displays; LONG ret;
- displays = get_display_settings( devname, devmode ); - if (!displays) return DISP_CHANGE_FAILED; + if (!lock_display_devices()) return DISP_CHANGE_FAILED; + if (!(displays = get_display_settings( devname, devmode ))) + { + unlock_display_devices(); + return DISP_CHANGE_FAILED; + }
if (all_detached_settings( displays )) { + unlock_display_devices(); WARN( "Detaching all modes is not permitted.\n" ); free( displays ); return DISP_CHANGE_SUCCESSFUL; @@ -2489,6 +2490,7 @@ static LONG apply_display_settings( const WCHAR *devname, const DEVMODEW *devmod place_all_displays( displays );
ret = user_driver->pChangeDisplaySettings( displays, hwnd, flags, lparam ); + unlock_display_devices();
free( displays ); if (ret) return ret;
From: Rémi Bernon rbernon@codeweavers.com
As a fallback if ChangeDisplaySettings returns E_NOTIMPL. --- dlls/win32u/driver.c | 2 +- dlls/win32u/sysparams.c | 53 +++++++++++++++++++++++++++++++++++++---- 2 files changed, 50 insertions(+), 5 deletions(-)
diff --git a/dlls/win32u/driver.c b/dlls/win32u/driver.c index fc777362d93..cd71ea3c20e 100644 --- a/dlls/win32u/driver.c +++ b/dlls/win32u/driver.c @@ -756,7 +756,7 @@ static void nulldrv_UpdateClipboard(void) static LONG nulldrv_ChangeDisplaySettings( LPDEVMODEW displays, HWND hwnd, DWORD flags, LPVOID lparam ) { - return DISP_CHANGE_FAILED; + return E_NOTIMPL; /* use default implementation */ }
static BOOL nulldrv_GetCurrentDisplaySettings( LPCWSTR name, BOOL is_primary, LPDEVMODEW mode ) diff --git a/dlls/win32u/sysparams.c b/dlls/win32u/sysparams.c index 7e6bebbcf72..f25280bd243 100644 --- a/dlls/win32u/sysparams.c +++ b/dlls/win32u/sysparams.c @@ -571,6 +571,26 @@ static BOOL adapter_set_registry_settings( const struct adapter *adapter, const return ret; }
+static BOOL adapter_set_current_settings( const struct adapter *adapter, const DEVMODEW *mode ) +{ + HANDLE mutex; + HKEY hkey; + BOOL ret; + + mutex = get_display_device_init_mutex(); + + if (!config_key && !(config_key = reg_open_key( NULL, config_keyW, sizeof(config_keyW) ))) ret = FALSE; + if (!(hkey = reg_open_key( config_key, adapter->config_key, lstrlenW( adapter->config_key ) * sizeof(WCHAR) ))) ret = FALSE; + else + { + ret = write_adapter_mode( hkey, ENUM_CURRENT_SETTINGS, mode ); + NtClose( hkey ); + } + + release_display_device_init_mutex( mutex ); + return ret; +} + static int mode_compare(const void *p1, const void *p2) { BOOL a_interlaced, b_interlaced, a_stretched, b_stretched; @@ -1580,6 +1600,9 @@ static BOOL update_display_cache( BOOL force )
if (!user_driver->pUpdateDisplayDevices( &device_manager, force, &ctx ) && force) { + /* default implementation: expose an adapter and a monitor with few standard modes, + * and read / write current display settings from / to the registry. + */ static const DEVMODEW modes[] = { { .dmFields = DM_DISPLAYORIENTATION | DM_BITSPERPEL | DM_PELSWIDTH | DM_PELSHEIGHT | DM_DISPLAYFLAGS | DM_DISPLAYFREQUENCY, @@ -1603,13 +1626,24 @@ static BOOL update_display_cache( BOOL force ) struct gdi_monitor monitor = { .state_flags = DISPLAY_DEVICE_ACTIVE | DISPLAY_DEVICE_ATTACHED, - .rc_monitor = {.right = modes[2].dmPelsWidth, .bottom = modes[2].dmPelsHeight}, - .rc_work = {.right = modes[2].dmPelsWidth, .bottom = modes[2].dmPelsHeight}, }; + DEVMODEW mode = {{0}}; UINT i;
add_gpu( &gpu, &ctx ); add_adapter( &adapter, &ctx ); + + if (!read_adapter_mode( ctx.adapter_key, ENUM_CURRENT_SETTINGS, &mode )) + { + mode = modes[2]; + mode.dmFields |= DM_POSITION; + write_adapter_mode( ctx.adapter_key, ENUM_CURRENT_SETTINGS, &mode ); + } + monitor.rc_monitor.right = mode.dmPelsWidth; + monitor.rc_monitor.bottom = mode.dmPelsHeight; + monitor.rc_work.right = mode.dmPelsWidth; + monitor.rc_work.bottom = mode.dmPelsHeight; + add_monitor( &monitor, &ctx ); for (i = 0; i < ARRAY_SIZE(modes); ++i) add_mode( modes + i, &ctx ); } @@ -2468,8 +2502,8 @@ 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 ) { + DEVMODEW *mode, *displays; struct adapter *adapter; - DEVMODEW *displays; LONG ret;
if (!lock_display_devices()) return DISP_CHANGE_FAILED; @@ -2489,7 +2523,18 @@ static LONG apply_display_settings( const WCHAR *devname, const DEVMODEW *devmod
place_all_displays( displays );
- ret = user_driver->pChangeDisplaySettings( displays, hwnd, flags, lparam ); + if ((ret = user_driver->pChangeDisplaySettings( displays, hwnd, flags, lparam )) == E_NOTIMPL) + { + /* default implementation: write current display settings to the registry. */ + mode = displays; + LIST_FOR_EACH_ENTRY( adapter, &adapters, struct adapter, entry ) + { + if (!adapter_set_current_settings( adapter, mode )) + WARN( "Failed to write adapter %s current mode.\n", debugstr_w(adapter->dev.device_name) ); + mode = NEXT_DEVMODEW(mode); + } + ret = DISP_CHANGE_SUCCESSFUL; + } unlock_display_devices();
free( displays );
From: Rémi Bernon rbernon@codeweavers.com
--- dlls/win32u/driver.c | 2 +- dlls/win32u/sysparams.c | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-)
diff --git a/dlls/win32u/driver.c b/dlls/win32u/driver.c index cd71ea3c20e..72c9e2d9d66 100644 --- a/dlls/win32u/driver.c +++ b/dlls/win32u/driver.c @@ -761,7 +761,7 @@ static LONG nulldrv_ChangeDisplaySettings( LPDEVMODEW displays, HWND hwnd,
static BOOL nulldrv_GetCurrentDisplaySettings( LPCWSTR name, BOOL is_primary, LPDEVMODEW mode ) { - return FALSE; + return FALSE; /* use default implementation */ }
static BOOL nulldrv_UpdateDisplayDevices( const struct gdi_device_manager *manager, BOOL force, void *param ) diff --git a/dlls/win32u/sysparams.c b/dlls/win32u/sysparams.c index f25280bd243..859fa859d2b 100644 --- a/dlls/win32u/sysparams.c +++ b/dlls/win32u/sysparams.c @@ -545,9 +545,24 @@ static BOOL adapter_get_registry_settings( const struct adapter *adapter, DEVMOD static BOOL adapter_get_current_settings( const struct adapter *adapter, DEVMODEW *mode ) { BOOL ret, is_primary = !!(adapter->dev.state_flags & DISPLAY_DEVICE_PRIMARY_DEVICE); + HANDLE mutex; + HKEY hkey;
if ((ret = user_driver->pGetCurrentDisplaySettings( adapter->dev.device_name, is_primary, mode ))) return TRUE;
+ /* default implementation: read current display settings from the registry. */ + + mutex = get_display_device_init_mutex(); + + if (!config_key && !(config_key = reg_open_key( NULL, config_keyW, sizeof(config_keyW) ))) ret = FALSE; + else if (!(hkey = reg_open_key( config_key, adapter->config_key, lstrlenW( adapter->config_key ) * sizeof(WCHAR) ))) ret = FALSE; + else + { + ret = read_adapter_mode( hkey, ENUM_CURRENT_SETTINGS, mode ); + NtClose( hkey ); + } + + release_display_device_init_mutex( mutex ); return ret; }
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=125084
Your paranoid android.
=== debian11 (32 bit report) ===
ddraw: ddraw7.c:15663: Test failed: Expected unsynchronised map for flags 0x1000. ddraw7.c:15663: Test failed: Expected unsynchronised map for flags 0x3000.
=== debian11 (build log) ===
Use of uninitialized value $Flaky in addition (+) at /home/testbot/lib/WineTestBot/LogUtils.pm line 720, <$LogFile> line 24739. Use of uninitialized value $Flaky in addition (+) at /home/testbot/lib/WineTestBot/LogUtils.pm line 720, <$LogFile> line 24739. Use of uninitialized value $Flaky in addition (+) at /home/testbot/lib/WineTestBot/LogUtils.pm line 720, <$LogFile> line 24739.
Zhiyi Zhang (@zhiyi) commented about dlls/winex11.drv/display.c:
return DISP_CHANGE_SUCCESSFUL;}
+static BOOL is_primary_display( DEVMODEW *mode )
You can avoid using is_detached_mode().
` return mode->dmFields & DM_POSITION && mode->dmFields & DM_PELSWIDTH && mode->dmFields & DM_PELSHEIGHT && mode->dmPosition.x == 0 && mode->dmPosition.y == 0 && mode->dmPelsWidth && mode->dmPelsHeight; `
And add const to the mode parameter.
Zhiyi Zhang (@zhiyi) commented about dlls/win32u/sysparams.c:
return ret;}
+static BOOL adapter_get_current_settings( const struct adapter *adapter, DEVMODEW *mode ) +{
- BOOL ret, is_primary = !!(adapter->dev.state_flags & DISPLAY_DEVICE_PRIMARY_DEVICE);
- if ((ret = user_driver->pGetCurrentDisplaySettings( adapter->dev.device_name, is_primary, mode ))) return TRUE;
The following is simpler.
return user_driver->pGetCurrentDisplaySettings( adapter->dev.device_name, adapter->dev.state_flags & DISPLAY_DEVICE_PRIMARY_DEVICE, mode );
Zhiyi Zhang (@zhiyi) commented about dlls/winex11.drv/display.c:
if (!(ids = calloc( count, sizeof(*ids) ))) return DISP_CHANGE_FAILED; for (count = 0, mode = displays; mode->dmSize; mode = NEXT_DEVMODEW(mode), count++) {
if (!settings_handler.get_id( mode->dmDeviceName, ids + count )) goto done;
if (!settings_handler.get_id( mode->dmDeviceName, is_primary_display( mode ), ids + count )) goto done;
Eh, I didn't think of this. But notice that 'mode' is the display mode the application wants to change to, not the original mode. So is_primary_display() actually returns the future primary display. Say if we need to detach the current primary display, is_primary_display() will return FALSE here and cause confusion. This is a behavior change compared to the old implementation. Please find another way to do this. Maybe find the primary display name in win32u and pass it to ChangeDisplaySettings()? or keep the old ways or something else.
On Thu Oct 20 06:30:13 2022 +0000, Zhiyi Zhang wrote:
Eh, I didn't think of this. But notice that 'mode' is the display mode the application wants to change to, not the original mode. So is_primary_display() actually returns the future primary display. Say if we need to detach the current primary display, is_primary_display() will return FALSE here and cause confusion. This is a behavior change compared to the old implementation. Please find another way to do this. Maybe find the primary display name in win32u and pass it to ChangeDisplaySettings()? or keep the old ways or something else.
You're right, I think the best way forward is to improve the non-primary adapter support on all backends. I've opened https://gitlab.winehq.org/wine/wine/-/merge_requests/1116 and https://gitlab.winehq.org/wine/wine/-/merge_requests/1117 for this purpose.
I'm not completely sure about the winemac.drv multi-monitor support, so if it doesn't work I'll probably drop this for now and keep the display mode change non-atomic.
On Thu Oct 20 10:00:11 2022 +0000, Rémi Bernon wrote:
You're right, I think the best way forward is to improve the non-primary adapter support on all backends. I've opened https://gitlab.winehq.org/wine/wine/-/merge_requests/1116 and https://gitlab.winehq.org/wine/wine/-/merge_requests/1117 for this purpose. I'm not completely sure about the winemac.drv multi-monitor support, so if it doesn't work I'll probably drop this for now and keep the display mode change non-atomic.
That didn't go very well, I'm going back to the approach passing is_primary / primary adapter name to display driver callbacks. I've split these changes into https://gitlab.winehq.org/wine/wine/-/merge_requests/1136.
-
Marvin -
Rémi Bernon -
Rémi Bernon (＠rbernon)
-
Zhiyi Zhang (＠zhiyi)