[PATCH v12 0/3] MR551: win32u: Complete support for display modes in nulldrv.
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.... -- v12: 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. https://gitlab.winehq.org/wine/wine/-/merge_requests/551
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/win32u/sysparams.c | 35 +++++++++++++++++------------------ 1 file changed, 17 insertions(+), 18 deletions(-) diff --git a/dlls/win32u/sysparams.c b/dlls/win32u/sysparams.c index cd8e72b251f..d1f9dc2c8ba 100644 --- a/dlls/win32u/sysparams.c +++ b/dlls/win32u/sysparams.c @@ -2208,10 +2208,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 ) @@ -2223,20 +2221,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 ) @@ -2430,15 +2426,21 @@ static LONG apply_display_settings( const WCHAR *devname, const DEVMODEW *devmod HWND hwnd, DWORD flags, void *lparam ) { WCHAR primary_name[CCHDEVICENAME]; + struct display_device *primary; struct adapter *adapter; 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; @@ -2446,14 +2448,11 @@ static LONG apply_display_settings( const WCHAR *devname, const DEVMODEW *devmod place_all_displays( displays ); - if (!(adapter = find_adapter( NULL ))) primary_name[0] = 0; - else - { - wcscpy( primary_name, adapter->dev.device_name ); - adapter_release( adapter ); - } + if (!(primary = find_adapter_device_by_id( 0 ))) primary_name[0] = 0; + else wcscpy( primary_name, primary->device_name ); ret = user_driver->pChangeDisplaySettings( displays, primary_name, hwnd, flags, lparam ); + unlock_display_devices(); free( displays ); if (ret) return ret; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/551
From: Rémi Bernon <rbernon(a)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 f8aa15c46ce..e7dda4a5cf4 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, LPCWSTR primary_name, 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 d1f9dc2c8ba..c6b71181608 100644 --- a/dlls/win32u/sysparams.c +++ b/dlls/win32u/sysparams.c @@ -509,6 +509,26 @@ static BOOL adapter_get_current_settings( const struct adapter *adapter, DEVMODE return user_driver->pGetCurrentDisplaySettings( adapter->dev.device_name, is_primary, mode ); } +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; @@ -1537,6 +1557,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, @@ -1560,13 +1583,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 ); } @@ -2427,8 +2461,8 @@ static LONG apply_display_settings( const WCHAR *devname, const DEVMODEW *devmod { WCHAR primary_name[CCHDEVICENAME]; struct display_device *primary; + DEVMODEW *mode, *displays; struct adapter *adapter; - DEVMODEW *displays; LONG ret; if (!lock_display_devices()) return DISP_CHANGE_FAILED; @@ -2451,7 +2485,18 @@ static LONG apply_display_settings( const WCHAR *devname, const DEVMODEW *devmod if (!(primary = find_adapter_device_by_id( 0 ))) primary_name[0] = 0; else wcscpy( primary_name, primary->device_name ); - ret = user_driver->pChangeDisplaySettings( displays, primary_name, hwnd, flags, lparam ); + if ((ret = user_driver->pChangeDisplaySettings( displays, primary_name, hwnd, flags, lparam ))) + { + /* 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 ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/551
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/win32u/driver.c | 2 +- dlls/win32u/sysparams.c | 24 ++++++++++++++++++++++-- 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/dlls/win32u/driver.c b/dlls/win32u/driver.c index e7dda4a5cf4..557d555d673 100644 --- a/dlls/win32u/driver.c +++ b/dlls/win32u/driver.c @@ -761,7 +761,7 @@ static LONG nulldrv_ChangeDisplaySettings( LPDEVMODEW displays, LPCWSTR primary_ 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 c6b71181608..7381495bce0 100644 --- a/dlls/win32u/sysparams.c +++ b/dlls/win32u/sysparams.c @@ -506,7 +506,26 @@ static BOOL adapter_set_registry_settings( const struct adapter *adapter, const static BOOL adapter_get_current_settings( const struct adapter *adapter, DEVMODEW *mode ) { BOOL is_primary = !!(adapter->dev.state_flags & DISPLAY_DEVICE_PRIMARY_DEVICE); - return user_driver->pGetCurrentDisplaySettings( adapter->dev.device_name, is_primary, mode ); + HANDLE mutex; + HKEY hkey; + BOOL ret; + + 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; } static BOOL adapter_set_current_settings( const struct adapter *adapter, const DEVMODEW *mode ) @@ -2508,7 +2527,8 @@ static LONG apply_display_settings( const WCHAR *devname, const DEVMODEW *devmod if ((adapter = find_adapter( NULL ))) { DEVMODEW current_mode = {.dmSize = sizeof(DEVMODEW)}; - adapter_get_current_settings( adapter, ¤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, -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/551
Zhiyi Zhang (@zhiyi) commented about dlls/win32u/sysparams.c:
if (!(primary = find_adapter_device_by_id( 0 ))) primary_name[0] = 0; else wcscpy( primary_name, primary->device_name );
- ret = user_driver->pChangeDisplaySettings( displays, primary_name, hwnd, flags, lparam ); + if ((ret = user_driver->pChangeDisplaySettings( displays, primary_name, hwnd, flags, lparam )))
In your earlier revisions, you checked ret against E_NOTIMPL. Now you're going for the fallback anytime the user driver fails to change display modes? why did you change it? I think the earlier versions are correct. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/551#note_12776
Zhiyi Zhang (@zhiyi) commented about dlls/win32u/sysparams.c:
if (!user_driver->pUpdateDisplayDevices( &device_manager, force, &ctx ) && force) { + /* default implementation: expose an adapter and a monitor with few standard modes,
few -> a few? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/551#note_12777
On Wed Nov 2 12:51:39 2022 +0000, Zhiyi Zhang wrote:
In your earlier revisions, you checked ret against E_NOTIMPL. Now you're going for the fallback anytime the user driver fails to change display modes? why did you change it? I think the earlier versions are correct. Well... I have no idea. You're right, I'll change it back.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/551#note_12785
participants (2)
-
Rémi Bernon -
Zhiyi Zhang (＠zhiyi)