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....
-- v10: 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. win32u: Use an internal WINE_DM_PRIMARY_DEVICE flag on primary adapter modes. win32u: Remove the device name parameter from GetCurrentDisplaySettings. 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
Passing it in the DEVMODEW dmDeviceName field directly. --- dlls/win32u/driver.c | 6 +++--- dlls/win32u/sysparams.c | 28 +++++++++++++++++++++++----- dlls/wineandroid.drv/init.c | 2 +- dlls/winemac.drv/display.c | 6 +++--- dlls/winemac.drv/macdrv.h | 2 +- dlls/winex11.drv/desktop.c | 4 ++-- dlls/winex11.drv/display.c | 12 ++++++------ dlls/winex11.drv/x11drv.h | 7 ++++--- dlls/winex11.drv/xrandr.c | 8 ++++---- dlls/winex11.drv/xvidmode.c | 4 ++-- include/wine/gdi_driver.h | 2 +- 11 files changed, 50 insertions(+), 31 deletions(-)
diff --git a/dlls/win32u/driver.c b/dlls/win32u/driver.c index ee4a6534b50..7fb84fb289d 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( 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( LPDEVMODEW mode ) { - return load_driver()->pGetCurrentDisplaySettings( name, mode ); + return load_driver()->pGetCurrentDisplaySettings( mode ); }
static void loaderdrv_SetCursor( HCURSOR cursor ) diff --git a/dlls/win32u/sysparams.c b/dlls/win32u/sysparams.c index 6353467737e..536f1410e10 100644 --- a/dlls/win32u/sysparams.c +++ b/dlls/win32u/sysparams.c @@ -542,6 +542,21 @@ 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 ) +{ + DEVMODEW current_mode = {.dmSize = sizeof(DEVMODEW)}; + BOOL ret; + + lstrcpyW( current_mode.dmDeviceName, adapter->dev.device_name ); + if ((ret = user_driver->pGetCurrentDisplaySettings( ¤t_mode ))) + { + memcpy( &mode->dmFields, ¤t_mode.dmFields, mode->dmSize - offsetof(DEVMODEW, dmFields) ); + return TRUE; + } + + return ret; +} + static BOOL adapter_set_registry_settings( const struct adapter *adapter, const DEVMODEW *mode ) { HANDLE mutex; @@ -2214,7 +2229,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)) @@ -2250,12 +2266,13 @@ static DEVMODEW *get_display_settings( const WCHAR *devname, const DEVMODEW *dev LIST_FOR_EACH_ENTRY( adapter, &adapters, struct adapter, entry ) { mode->dmSize = sizeof(DEVMODEW); + if (devmode && !wcsicmp( devname, adapter->dev.device_name )) memcpy( &mode->dmFields, &devmode->dmFields, devmode->dmSize - offsetof(DEVMODEW, dmFields) ); 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 +2506,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 +2552,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 +2600,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..7fc4150c854 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( 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..b6543bbef28 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(LPDEVMODEW devmode) { struct macdrv_display *displays = NULL; int num_displays, display_idx; @@ -923,14 +923,14 @@ 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, %p + %hu\n", debugstr_w(devmode->dmDeviceName), devmode, devmode->dmSize);
init_original_display_mode();
if (macdrv_get_displays(&displays, &num_displays)) return FALSE;
- display_idx = wcstol(devname + 11, &end, 10) - 1; + display_idx = wcstol(devmode->dmDeviceName + 11, &end, 10) - 1; if (display_idx >= num_displays) { macdrv_free_displays(displays); diff --git a/dlls/winemac.drv/macdrv.h b/dlls/winemac.drv/macdrv.h index ea51cf0f702..743fadfe86f 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(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/desktop.c b/dlls/winex11.drv/desktop.c index c6a08291f6a..74eb432e9c0 100644 --- a/dlls/winex11.drv/desktop.c +++ b/dlls/winex11.drv/desktop.c @@ -123,11 +123,11 @@ 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 DEVMODEW *display, ULONG_PTR *id ) { WCHAR primary_adapter[CCHDEVICENAME];
- if (!get_primary_adapter( primary_adapter ) || wcsicmp( primary_adapter, device_name )) + if (!get_primary_adapter( primary_adapter ) || wcsicmp( primary_adapter, display->dmDeviceName )) return FALSE;
*id = 0; diff --git a/dlls/winex11.drv/display.c b/dlls/winex11.drv/display.c index 2a4a34e2c02..613153a8ded 100644 --- a/dlls/winex11.drv/display.c +++ b/dlls/winex11.drv/display.c @@ -66,14 +66,14 @@ 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 DEVMODEW *display, ULONG_PTR *id ) { WCHAR primary_adapter[CCHDEVICENAME];
if (!get_primary_adapter( primary_adapter )) return FALSE;
- *id = !wcsicmp( device_name, primary_adapter ) ? 1 : 0; + *id = !wcsicmp( display->dmDeviceName, primary_adapter ) ? 1 : 0; return TRUE; }
@@ -266,14 +266,14 @@ static DWORD get_display_depth(ULONG_PTR display_id) * GetCurrentDisplaySettings (X11DRV.@) * */ -BOOL X11DRV_GetCurrentDisplaySettings( LPCWSTR name, LPDEVMODEW devmode ) +BOOL X11DRV_GetCurrentDisplaySettings( LPDEVMODEW devmode ) { DEVMODEW mode; ULONG_PTR id;
- if (!settings_handler.get_id( name, &id ) || !settings_handler.get_current_mode( id, &mode )) + if (!settings_handler.get_id( devmode, &id ) || !settings_handler.get_current_mode( id, &mode )) { - ERR("Failed to get %s current display settings.\n", wine_dbgstr_w(name)); + ERR("Failed to get %s current display settings.\n", wine_dbgstr_w(devmode->dmDeviceName)); return FALSE; }
@@ -409,7 +409,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, 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 f8f8fe3d4d1..3e89f743a7d 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( 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; @@ -706,11 +706,12 @@ struct x11drv_settings_handler /* Higher priority can override handlers with a lower priority */ UINT priority;
- /* get_id() will be called to map a device name, e.g., \.\DISPLAY1 to a driver specific id. + /* get_id() will be called to map a display settings to a driver specific id, + * from its device name, e.g., \.\DISPLAY1. * 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 DEVMODEW *display, 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 diff --git a/dlls/winex11.drv/xrandr.c b/dlls/winex11.drv/xrandr.c index c7f922b1aae..a199d43ef1b 100644 --- a/dlls/winex11.drv/xrandr.c +++ b/dlls/winex11.drv/xrandr.c @@ -144,7 +144,7 @@ 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 DEVMODEW *display, ULONG_PTR *id ) { WCHAR primary_adapter[CCHDEVICENAME];
@@ -154,7 +154,7 @@ static BOOL xrandr10_get_id( const WCHAR *device_name, ULONG_PTR *id ) /* 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 = !wcsicmp( display->dmDeviceName, primary_adapter ) ? 1 : 0; return TRUE; }
@@ -1224,7 +1224,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 DEVMODEW *display, ULONG_PTR *id ) { struct current_mode *tmp_modes, *new_current_modes = NULL; INT gpu_count, adapter_count, new_current_mode_count = 0; @@ -1234,7 +1234,7 @@ static BOOL xrandr14_get_id( const WCHAR *device_name, ULONG_PTR *id ) WCHAR *end;
/* Parse \.\DISPLAY%d */ - display_idx = wcstol( device_name + 11, &end, 10 ) - 1; + display_idx = wcstol( display->dmDeviceName + 11, &end, 10 ) - 1; if (*end) return FALSE;
diff --git a/dlls/winex11.drv/xvidmode.c b/dlls/winex11.drv/xvidmode.c index 95342a84223..1ed2fbd7c15 100644 --- a/dlls/winex11.drv/xvidmode.c +++ b/dlls/winex11.drv/xvidmode.c @@ -85,7 +85,7 @@ 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 DEVMODEW *display, ULONG_PTR *id) { WCHAR primary_adapter[CCHDEVICENAME];
@@ -95,7 +95,7 @@ static BOOL xf86vm_get_id(const WCHAR *device_name, ULONG_PTR *id) /* 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 = !wcsicmp( display->dmDeviceName, primary_adapter ) ? 1 : 0; return TRUE; }
diff --git a/include/wine/gdi_driver.h b/include/wine/gdi_driver.h index 596d27156af..fe86b4d9049 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)(LPDEVMODEW); BOOL (*pUpdateDisplayDevices)(const struct gdi_device_manager *,BOOL,void*); /* windowing functions */ BOOL (*pCreateDesktopWindow)(HWND);
From: Rémi Bernon rbernon@codeweavers.com
Avoiding NtUserEnumDisplayDevices calls from GetCurrentDisplaySettings or ChangeDisplaySettings, as it requires entering the display device lock again. --- dlls/win32u/sysparams.c | 3 +++ dlls/winemac.drv/display.c | 24 +----------------------- dlls/winex11.drv/desktop.c | 6 +----- dlls/winex11.drv/display.c | 25 +------------------------ dlls/winex11.drv/x11drv.h | 3 +-- dlls/winex11.drv/xrandr.c | 7 +------ dlls/winex11.drv/xvidmode.c | 7 +------ include/wine/gdi_driver.h | 3 ++- 8 files changed, 11 insertions(+), 67 deletions(-)
diff --git a/dlls/win32u/sysparams.c b/dlls/win32u/sysparams.c index 536f1410e10..6258ebf2549 100644 --- a/dlls/win32u/sysparams.c +++ b/dlls/win32u/sysparams.c @@ -548,8 +548,10 @@ static BOOL adapter_get_current_settings( const struct adapter *adapter, DEVMODE BOOL ret;
lstrcpyW( current_mode.dmDeviceName, adapter->dev.device_name ); + if (adapter->dev.state_flags & DISPLAY_DEVICE_PRIMARY_DEVICE) current_mode.dmDisplayFlags |= WINE_DM_PRIMARY_DEVICE; if ((ret = user_driver->pGetCurrentDisplaySettings( ¤t_mode ))) { + current_mode.dmDisplayFlags &= ~WINE_DM_PRIMARY_DEVICE; memcpy( &mode->dmFields, ¤t_mode.dmFields, mode->dmSize - offsetof(DEVMODEW, dmFields) ); return TRUE; } @@ -2277,6 +2279,7 @@ static DEVMODEW *get_display_settings( const WCHAR *devname, const DEVMODEW *dev }
lstrcpyW( mode->dmDeviceName, adapter->dev.device_name ); + if (adapter->dev.state_flags & DISPLAY_DEVICE_PRIMARY_DEVICE) mode->dmDisplayFlags |= WINE_DM_PRIMARY_DEVICE; mode = NEXT_DEVMODEW(mode); }
diff --git a/dlls/winemac.drv/display.c b/dlls/winemac.drv/display.c index b6543bbef28..a3ff21e40e1 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 && @@ -771,7 +753,6 @@ static CGDisplayModeRef find_best_display_mode(DEVMODEW *devmode, CFArrayRef dis */ 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 +766,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 +782,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 (!(mode->dmDisplayFlags & WINE_DM_PRIMARY_DEVICE)) { FIXME("Changing non-primary adapter settings is currently unsupported.\n"); continue; diff --git a/dlls/winex11.drv/desktop.c b/dlls/winex11.drv/desktop.c index 74eb432e9c0..a9b20713ca1 100644 --- a/dlls/winex11.drv/desktop.c +++ b/dlls/winex11.drv/desktop.c @@ -125,11 +125,7 @@ BOOL is_virtual_desktop(void) /* Virtual desktop display settings handler */ static BOOL X11DRV_desktop_get_id( const DEVMODEW *display, ULONG_PTR *id ) { - WCHAR primary_adapter[CCHDEVICENAME]; - - if (!get_primary_adapter( primary_adapter ) || wcsicmp( primary_adapter, display->dmDeviceName )) - return FALSE; - + if (!(display->u2.dmDisplayFlags & WINE_DM_PRIMARY_DEVICE)) return FALSE; *id = 0; return TRUE; } diff --git a/dlls/winex11.drv/display.c b/dlls/winex11.drv/display.c index 613153a8ded..ead0efd3fcf 100644 --- a/dlls/winex11.drv/display.c +++ b/dlls/winex11.drv/display.c @@ -68,12 +68,7 @@ void X11DRV_Settings_SetHandler(const struct x11drv_settings_handler *new_handle */ static BOOL nores_get_id( const DEVMODEW *display, ULONG_PTR *id ) { - WCHAR primary_adapter[CCHDEVICENAME]; - - if (!get_primary_adapter( primary_adapter )) - return FALSE; - - *id = !wcsicmp( display->dmDeviceName, primary_adapter ) ? 1 : 0; + *id = (display->dmDisplayFlags & WINE_DM_PRIMARY_DEVICE) ? 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; diff --git a/dlls/winex11.drv/x11drv.h b/dlls/winex11.drv/x11drv.h index 3e89f743a7d..3549ca588dd 100644 --- a/dlls/winex11.drv/x11drv.h +++ b/dlls/winex11.drv/x11drv.h @@ -707,7 +707,7 @@ struct x11drv_settings_handler UINT priority;
/* get_id() will be called to map a display settings to a driver specific id, - * from its device name, e.g., \.\DISPLAY1. + * from its device name, e.g., \.\DISPLAY1 and WINE_DM_PRIMARY_DEVICE dmDisplayFlag. * Following functions use this id to identify the device. * * Return FALSE if the device cannot be found and TRUE on success */ @@ -752,7 +752,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 a199d43ef1b..a0f7329e4ad 100644 --- a/dlls/winex11.drv/xrandr.c +++ b/dlls/winex11.drv/xrandr.c @@ -146,15 +146,10 @@ static int XRandRErrorHandler(Display *dpy, XErrorEvent *event, void *arg) /* XRandR 1.0 display settings handler */ static BOOL xrandr10_get_id( const DEVMODEW *display, 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( display->dmDeviceName, primary_adapter ) ? 1 : 0; + *id = (display->u2.dmDisplayFlags & WINE_DM_PRIMARY_DEVICE) ? 1 : 0; return TRUE; }
diff --git a/dlls/winex11.drv/xvidmode.c b/dlls/winex11.drv/xvidmode.c index 1ed2fbd7c15..b804b49d0d6 100644 --- a/dlls/winex11.drv/xvidmode.c +++ b/dlls/winex11.drv/xvidmode.c @@ -87,15 +87,10 @@ static int XVidModeErrorHandler(Display *dpy, XErrorEvent *event, void *arg) /* XF86VidMode display settings handler */ static BOOL xf86vm_get_id(const DEVMODEW *display, 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( display->dmDeviceName, primary_adapter ) ? 1 : 0; + *id = (display->u2.dmDisplayFlags & WINE_DM_PRIMARY_DEVICE) ? 1 : 0; return TRUE; }
diff --git a/include/wine/gdi_driver.h b/include/wine/gdi_driver.h index fe86b4d9049..e8a3f47c752 100644 --- a/include/wine/gdi_driver.h +++ b/include/wine/gdi_driver.h @@ -269,7 +269,8 @@ struct gdi_device_manager void (*add_mode)( const DEVMODEW *mode, void *param ); };
-#define WINE_DM_UNSUPPORTED 0x80000000 +#define WINE_DM_UNSUPPORTED 0x80000000 /* maybe set internally on added / enumerated modes */ +#define WINE_DM_PRIMARY_DEVICE 0x40000000 /* maybe set internally for GetCurrentDisplaySettings / ChangeDisplaySettings */
struct tagUPDATELAYEREDWINDOWINFO;
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 6258ebf2549..6ad6f7551d9 100644 --- a/dlls/win32u/sysparams.c +++ b/dlls/win32u/sysparams.c @@ -2259,10 +2259,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 ) @@ -2275,7 +2273,11 @@ 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 ); @@ -2283,13 +2285,7 @@ static DEVMODEW *get_display_settings( const WCHAR *devname, const DEVMODEW *dev mode = NEXT_DEVMODEW(mode); }
- unlock_display_devices(); return displays; - -done: - unlock_display_devices(); - free( displays ); - return NULL; }
static INT offset_length( POINT offset ) @@ -2486,11 +2482,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; @@ -2499,6 +2500,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 | 49 +++++++++++++++++++++++++++++++++++++---- 2 files changed, 46 insertions(+), 5 deletions(-)
diff --git a/dlls/win32u/driver.c b/dlls/win32u/driver.c index 7fb84fb289d..30c11cfaf98 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; }
static BOOL nulldrv_GetCurrentDisplaySettings( LPDEVMODEW mode ) diff --git a/dlls/win32u/sysparams.c b/dlls/win32u/sysparams.c index 6ad6f7551d9..8422f6f832f 100644 --- a/dlls/win32u/sysparams.c +++ b/dlls/win32u/sysparams.c @@ -579,6 +579,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; @@ -1611,13 +1631,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 ); } @@ -2478,8 +2509,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; @@ -2499,7 +2530,17 @@ 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) + { + 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/sysparams.c | 13 +++++++++++++ 1 file changed, 13 insertions(+)
diff --git a/dlls/win32u/sysparams.c b/dlls/win32u/sysparams.c index 8422f6f832f..6e978da12b7 100644 --- a/dlls/win32u/sysparams.c +++ b/dlls/win32u/sysparams.c @@ -545,6 +545,8 @@ static BOOL adapter_get_registry_settings( const struct adapter *adapter, DEVMOD static BOOL adapter_get_current_settings( const struct adapter *adapter, DEVMODEW *mode ) { DEVMODEW current_mode = {.dmSize = sizeof(DEVMODEW)}; + HANDLE mutex; + HKEY hkey; BOOL ret;
lstrcpyW( current_mode.dmDeviceName, adapter->dev.device_name ); @@ -556,6 +558,17 @@ static BOOL adapter_get_current_settings( const struct adapter *adapter, DEVMODE return TRUE; }
+ 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=125041
Your paranoid android.
=== debian11 (build log) ===
03b8:err:winediag:d3d_device_create The application wants to create a Direct3D device, but the current DirectDrawRenderer does not support this. 03b8:err:winediag:d3d_device_create The application wants to create a Direct3D device, but the current DirectDrawRenderer does not support this. 03b8:err:winediag:d3d_device_create The application wants to create a Direct3D device, but the current DirectDrawRenderer does not support this.
=== debian11 (build log) ===
Use of uninitialized value $Flaky in addition (+) at /home/testbot/lib/WineTestBot/LogUtils.pm line 720, <$LogFile> line 24755. Use of uninitialized value $Flaky in addition (+) at /home/testbot/lib/WineTestBot/LogUtils.pm line 720, <$LogFile> line 24755. Use of uninitialized value $Flaky in addition (+) at /home/testbot/lib/WineTestBot/LogUtils.pm line 720, <$LogFile> line 24755.
Rebased on top of upstream, which should solve the lockup and timeout issues.
Zhiyi Zhang (@zhiyi) commented about dlls/win32u/driver.c:
return DISP_CHANGE_FAILED;}
While this patch is technically correct, I don't see the necessity for doing this. And I think having an explicit device name parameter is more intuitive than using a DEVMODE as both input and output parameter.
Zhiyi Zhang (@zhiyi) commented about dlls/win32u/sysparams.c:
BOOL ret; lstrcpyW( current_mode.dmDeviceName, adapter->dev.device_name );
I don't like how this is done. Since the display modes passed to drivers are full modes, you can use the position and resolution to determine if a mode is for the primary adapter. The primary adapter mode has an origin of (0, 0) and valid width and height. You can introduce an is_primary_mode() to do that.
Zhiyi Zhang (@zhiyi) commented about dlls/win32u/sysparams.c:
place_all_displays( displays );
- ret = user_driver->pChangeDisplaySettings( displays, hwnd, flags, lparam );
- if ((ret = user_driver->pChangeDisplaySettings( displays, hwnd, flags, lparam )) == E_NOTIMPL)
I think it would be better to have some comments noting the fallback to nulldrv. Same for later patches.
On Tue Oct 18 07:28:51 2022 +0000, Zhiyi Zhang wrote:
While this patch is technically correct, I don't see the necessity for doing this. And I think having an explicit device name parameter is more intuitive than using a DEVMODE as both input and output parameter.
It was done as a transition to the next patch where the device name isn't used anymore. Maybe I can squash both to avoid the awkward state.
On Tue Oct 18 07:29:09 2022 +0000, Zhiyi Zhang wrote:
I don't like how this is done. Since the display modes passed to drivers are full modes, you can use the position and resolution to determine if a mode is for the primary adapter. The primary adapter mode has an origin of (0, 0) and valid width and height. You can introduce an is_primary_mode() to do that.
Alright, there's still the matter of how the CurrentDisplaySettings would figure if the adapter is the primary one or not. I guess I could keep the win32u callbacks in that case, but I'd prefer if drivers would not have to call these win32u functions back.
On Tue Oct 18 08:04:28 2022 +0000, Rémi Bernon wrote:
Alright, there's still the matter of how the CurrentDisplaySettings would figure if the adapter is the primary one or not. I guess I could keep the win32u callbacks in that case, but I'd prefer if drivers would not have to call these win32u functions back.
For CurrentDisplaySettings(), maybe add an 'is_primary' parameter?
-
Marvin -
Rémi Bernon -
Rémi Bernon (＠rbernon)
-
Zhiyi Zhang (＠zhiyi)