[PATCH v6 0/4] MR5422: win32u: Simplify virtual desktop mode by reporting all sources as detached.
-- v6: win32u: Introduce a new add_virtual_modes helper. winex11: Let win32u decide when to force update the display cache. win32u: Don't force refresh the display cache on thread desktop change. winex11: Report all sources as detached in virtual desktop mode.
From: Rémi Bernon rbernon@codeweavers.com
--- dlls/win32u/sysparams.c | 118 +++++++++++++++++------------------- dlls/winex11.drv/xinerama.c | 3 - dlls/winex11.drv/xrandr.c | 1 - 3 files changed, 54 insertions(+), 68 deletions(-)
diff --git a/dlls/win32u/sysparams.c b/dlls/win32u/sysparams.c index 26bddad7b59..077a3c35163 100644 --- a/dlls/win32u/sysparams.c +++ b/dlls/win32u/sysparams.c @@ -1313,6 +1313,10 @@ static void add_source( const char *name, UINT state_flags, void *param )
TRACE( "name %s, state_flags %#x\n", name, state_flags );
+ /* in virtual desktop mode, report all physical sources as detached */ + ctx->is_primary = !!(state_flags & DISPLAY_DEVICE_PRIMARY_DEVICE); + if (is_virtual_desktop()) state_flags &= ~(DISPLAY_DEVICE_ATTACHED_TO_DESKTOP | DISPLAY_DEVICE_PRIMARY_DEVICE); + memset( &ctx->source, 0, sizeof(ctx->source) ); ctx->source.gpu = &ctx->gpu; ctx->source.id = ctx->source_count; @@ -1448,11 +1452,17 @@ static void add_modes( const DEVMODEW *current, UINT modes_count, const DEVMODEW { struct device_manager_ctx *ctx = param; const DEVMODEW *mode; - DEVMODEW dummy; + DEVMODEW dummy, detached = *current;
TRACE( "current %s, modes_count %u, modes %p, param %p\n", debugstr_devmodew( current ), modes_count, modes, param );
- if (!read_source_mode( ctx->source_key, ENUM_REGISTRY_SETTINGS, &dummy )) + if (ctx->is_primary) ctx->primary = *current; + + detached.dmPelsWidth = 0; + detached.dmPelsHeight = 0; + if (!(ctx->source.state_flags & DISPLAY_DEVICE_ATTACHED_TO_DESKTOP)) current = &detached; + + if (current == &detached || !read_source_mode( ctx->source_key, ENUM_REGISTRY_SETTINGS, &dummy )) write_source_mode( ctx->source_key, ENUM_REGISTRY_SETTINGS, current ); write_source_mode( ctx->source_key, ENUM_CURRENT_SETTINGS, current );
@@ -1720,7 +1730,7 @@ static BOOL update_display_cache_from_registry(void) return ret; }
-static BOOL default_update_display_devices( const struct gdi_device_manager *manager, BOOL force, struct device_manager_ctx *ctx ) +static BOOL default_update_display_devices( BOOL force, struct device_manager_ctx *ctx ) { /* default implementation: expose an adapter and a monitor with a few standard modes, * and read / write current display settings from / to the registry. @@ -1747,8 +1757,8 @@ static BOOL default_update_display_devices( const struct gdi_device_manager *man
if (!force) return TRUE;
- manager->add_gpu( &gpu, ctx ); - manager->add_source( "Default", source_flags, ctx ); + add_gpu( &gpu, ctx ); + add_source( "Default", source_flags, ctx );
if (!read_source_mode( ctx->source_key, ENUM_CURRENT_SETTINGS, &mode )) { @@ -1760,18 +1770,12 @@ static BOOL default_update_display_devices( const struct gdi_device_manager *man monitor.rc_work.right = mode.dmPelsWidth; monitor.rc_work.bottom = mode.dmPelsHeight;
- manager->add_monitor( &monitor, ctx ); - manager->add_modes( &mode, ARRAY_SIZE(modes), modes, ctx ); + add_monitor( &monitor, ctx ); + add_modes( &mode, ARRAY_SIZE(modes), modes, ctx );
return TRUE; }
-static BOOL update_display_devices( const struct gdi_device_manager *manager, BOOL force, struct device_manager_ctx *ctx ) -{ - if (user_driver->pUpdateDisplayDevices( manager, force, ctx )) return TRUE; - return default_update_display_devices( manager, force, ctx ); -} - /* parse the desktop size specification */ static BOOL parse_size( const WCHAR *size, unsigned int *width, unsigned int *height ) { @@ -1804,38 +1808,8 @@ static BOOL get_default_desktop_size( unsigned int *width, unsigned int *height return TRUE; }
-static void desktop_add_gpu( const struct gdi_gpu *gpu, void *param ) -{ -} - -static void desktop_add_source( const char *name, UINT state_flags, void *param ) +static BOOL add_virtual_source( struct device_manager_ctx *ctx ) { - struct device_manager_ctx *ctx = param; - ctx->is_primary = !!(state_flags & DISPLAY_DEVICE_PRIMARY_DEVICE); -} - -static void desktop_add_monitor( const struct gdi_monitor *monitor, void *param ) -{ -} - -static void desktop_add_modes( const DEVMODEW *current, UINT modes_count, const DEVMODEW *modes, void *param ) -{ - struct device_manager_ctx *ctx = param; - if (ctx->is_primary) ctx->primary = *current; -} - -static const struct gdi_device_manager desktop_device_manager = -{ - desktop_add_gpu, - desktop_add_source, - desktop_add_monitor, - desktop_add_modes, -}; - -static BOOL desktop_update_display_devices( BOOL force, struct device_manager_ctx *ctx ) -{ - static const DWORD source_flags = DISPLAY_DEVICE_ATTACHED_TO_DESKTOP | DISPLAY_DEVICE_PRIMARY_DEVICE | DISPLAY_DEVICE_VGA_COMPATIBLE; - static const struct gdi_gpu gpu; struct gdi_monitor monitor = {0}; static struct screen_size { @@ -1875,19 +1849,34 @@ static BOOL desktop_update_display_devices( BOOL force, struct device_manager_ct {2560, 1600} };
- struct device_manager_ctx desktop_ctx = {0}; UINT screen_width, screen_height, max_width, max_height, modes_count; unsigned int depths[] = {8, 16, 0}; + struct source virtual_source = + { + .state_flags = DISPLAY_DEVICE_ATTACHED_TO_DESKTOP | DISPLAY_DEVICE_PRIMARY_DEVICE | DISPLAY_DEVICE_VGA_COMPATIBLE, + .id = ctx->source_count, + .gpu = &ctx->gpu, + }; DEVMODEW current, *modes; UINT i, j;
- if (!force) return TRUE; - /* in virtual desktop mode, read the device list from the user driver but expose virtual devices */ - if (!update_display_devices( &desktop_device_manager, TRUE, &desktop_ctx )) return FALSE; + /* Wine specific config key where source settings will be held, symlinked with the logically indexed config key */ + snprintf( virtual_source.path, sizeof(virtual_source.path), "%s\%s\Video\%s\Sources\%s", config_keyA, + control_keyA + strlen( "\Registry\Machine" ), virtual_source.gpu->guid, "Virtual" ); + + if (!write_source_to_registry( &virtual_source, &ctx->source_key )) + { + WARN( "Failed to write source to registry\n" ); + return FALSE; + } + + ctx->source = virtual_source; + ctx->gpu.source_count++; + ctx->source_count++;
- max_width = desktop_ctx.primary.dmPelsWidth; - max_height = desktop_ctx.primary.dmPelsHeight; - depths[ARRAY_SIZE(depths) - 1] = desktop_ctx.primary.dmBitsPerPel; + max_width = ctx->primary.dmPelsWidth; + max_height = ctx->primary.dmPelsHeight; + depths[ARRAY_SIZE(depths) - 1] = ctx->primary.dmBitsPerPel;
if (!get_default_desktop_size( &screen_width, &screen_height )) { @@ -1895,11 +1884,9 @@ static BOOL desktop_update_display_devices( BOOL force, struct device_manager_ct screen_height = max_height; }
- add_gpu( &gpu, ctx ); - add_source( "Default", source_flags, ctx ); if (!read_source_mode( ctx->source_key, ENUM_CURRENT_SETTINGS, ¤t )) { - current = desktop_ctx.primary; + current = ctx->primary; current.dmPelsWidth = screen_width; current.dmPelsHeight = screen_height; } @@ -1950,13 +1937,24 @@ static BOOL desktop_update_display_devices( BOOL force, struct device_manager_ct return TRUE; }
+static BOOL update_display_devices( BOOL force, struct device_manager_ctx *ctx ) +{ + if (user_driver->pUpdateDisplayDevices( &device_manager, force, ctx )) + { + if (ctx->source_count && is_virtual_desktop()) return add_virtual_source( ctx ); + return TRUE; + } + + return default_update_display_devices( force, ctx ); +} + BOOL update_display_cache( BOOL force ) { static const WCHAR wine_service_station_name[] = {'_','_','w','i','n','e','s','e','r','v','i','c','e','_','w','i','n','s','t','a','t','i','o','n',0}; HWINSTA winstation = NtUserGetProcessWindowStation(); struct device_manager_ctx ctx = {0}; - BOOL was_virtual_desktop, ret; + BOOL ret; WCHAR name[MAX_PATH];
/* services do not have any adapters, only a virtual monitor */ @@ -1970,15 +1968,7 @@ BOOL update_display_cache( BOOL force ) return TRUE; }
- if ((was_virtual_desktop = is_virtual_desktop())) ret = TRUE; - else ret = update_display_devices( &device_manager, force, &ctx ); - - /* as update_display_devices calls the user driver, it starts explorer and may change the virtual desktop state */ - if (ret && is_virtual_desktop()) - { - reset_display_manager_ctx( &ctx ); - ret = desktop_update_display_devices( force || !was_virtual_desktop, &ctx ); - } + ret = update_display_devices( force, &ctx );
release_display_manager_ctx( &ctx ); if (!ret) WARN( "Failed to update display devices\n" ); diff --git a/dlls/winex11.drv/xinerama.c b/dlls/winex11.drv/xinerama.c index 426ea6c11d4..fbf80819204 100644 --- a/dlls/winex11.drv/xinerama.c +++ b/dlls/winex11.drv/xinerama.c @@ -344,9 +344,6 @@ void xinerama_init( unsigned int width, unsigned int height ) int i; RECT rect;
- if (is_virtual_desktop()) - return; - pthread_mutex_lock( &xinerama_mutex );
SetRect( &rect, 0, 0, width, height ); diff --git a/dlls/winex11.drv/xrandr.c b/dlls/winex11.drv/xrandr.c index 0e7b9a07818..002ab77c086 100644 --- a/dlls/winex11.drv/xrandr.c +++ b/dlls/winex11.drv/xrandr.c @@ -1662,7 +1662,6 @@ void X11DRV_XRandR_Init(void)
if (major) return; /* already initialized? */ if (!usexrandr) return; /* disabled in config */ - if (is_virtual_desktop()) return; if (!(ret = load_xrandr())) return; /* can't load the Xrandr library */
/* see if Xrandr is available */
From: Rémi Bernon rbernon@codeweavers.com
--- dlls/win32u/winstation.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/dlls/win32u/winstation.c b/dlls/win32u/winstation.c index a3aed32f1da..3e71218200a 100644 --- a/dlls/win32u/winstation.c +++ b/dlls/win32u/winstation.c @@ -264,7 +264,7 @@ BOOL WINAPI NtUserSetThreadDesktop( HDESK handle ) thread_info->client_info.top_window = 0; thread_info->client_info.msg_window = 0; if (key_state_info) key_state_info->time = 0; - if (was_virtual_desktop != is_virtual_desktop()) update_display_cache( TRUE ); + if (was_virtual_desktop != is_virtual_desktop()) update_display_cache( FALSE ); } return ret; }
From: Rémi Bernon rbernon@codeweavers.com
--- dlls/winex11.drv/window.c | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-)
diff --git a/dlls/winex11.drv/window.c b/dlls/winex11.drv/window.c index 26cfee427ab..24b421d5fd7 100644 --- a/dlls/winex11.drv/window.c +++ b/dlls/winex11.drv/window.c @@ -2000,10 +2000,7 @@ void X11DRV_SetDesktopWindow( HWND hwnd )
if (!width && !height) /* not initialized yet */ { - RECT rect; - - X11DRV_DisplayDevices_Init( TRUE ); - rect = NtUserGetVirtualScreenRect(); + RECT rect = NtUserGetVirtualScreenRect();
SERVER_START_REQ( set_window_pos ) { @@ -2036,11 +2033,7 @@ void X11DRV_SetDesktopWindow( HWND hwnd ) else { Window win = (Window)NtUserGetProp( hwnd, whole_window_prop ); - if (win && win != root_window) - { - X11DRV_init_desktop( win, width, height ); - X11DRV_DisplayDevices_Init( TRUE ); - } + if (win && win != root_window) X11DRV_init_desktop( win, width, height ); } }
From: Rémi Bernon rbernon@codeweavers.com
--- dlls/win32u/sysparams.c | 108 ++++++++++++++++++++-------------------- 1 file changed, 54 insertions(+), 54 deletions(-)
diff --git a/dlls/win32u/sysparams.c b/dlls/win32u/sysparams.c index 077a3c35163..ec9f884164f 100644 --- a/dlls/win32u/sysparams.c +++ b/dlls/win32u/sysparams.c @@ -1777,7 +1777,7 @@ static BOOL default_update_display_devices( BOOL force, struct device_manager_ct }
/* parse the desktop size specification */ -static BOOL parse_size( const WCHAR *size, unsigned int *width, unsigned int *height ) +static BOOL parse_size( const WCHAR *size, DWORD *width, DWORD *height ) { WCHAR *end;
@@ -1790,7 +1790,7 @@ static BOOL parse_size( const WCHAR *size, unsigned int *width, unsigned int *he }
/* retrieve the default desktop size from the registry */ -static BOOL get_default_desktop_size( unsigned int *width, unsigned int *height ) +static BOOL get_default_desktop_size( DWORD *width, DWORD *height ) { WCHAR buffer[4096]; KEY_VALUE_PARTIAL_INFORMATION *value = (void *)buffer; @@ -1808,9 +1808,9 @@ static BOOL get_default_desktop_size( unsigned int *width, unsigned int *height return TRUE; }
-static BOOL add_virtual_source( struct device_manager_ctx *ctx ) +static void add_virtual_modes( struct device_manager_ctx *ctx, const DEVMODEW *current, + const DEVMODEW *initial, const DEVMODEW *maximum ) { - struct gdi_monitor monitor = {0}; static struct screen_size { unsigned int width; @@ -1848,17 +1848,57 @@ static BOOL add_virtual_source( struct device_manager_ctx *ctx ) {1920, 1200}, {2560, 1600} }; + UINT depths[] = {8, 16, initial->dmBitsPerPel}, i, j, modes_count; + DEVMODEW *modes; + + if (!(modes = malloc( ARRAY_SIZE(depths) * (ARRAY_SIZE(screen_sizes) + 2) * sizeof(*modes) ))) return; + + for (modes_count = i = 0; i < ARRAY_SIZE(depths); ++i) + { + DEVMODEW mode = + { + .dmFields = DM_DISPLAYORIENTATION | DM_BITSPERPEL | DM_PELSWIDTH | DM_PELSHEIGHT | DM_DISPLAYFLAGS | DM_DISPLAYFREQUENCY, + .dmDisplayFrequency = 60, + .dmBitsPerPel = depths[i], + }; + + for (j = 0; j < ARRAY_SIZE(screen_sizes); ++j) + { + mode.dmPelsWidth = screen_sizes[j].width; + mode.dmPelsHeight = screen_sizes[j].height; + + if (mode.dmPelsWidth > maximum->dmPelsWidth || mode.dmPelsHeight > maximum->dmPelsWidth) continue; + if (mode.dmPelsWidth == maximum->dmPelsWidth && mode.dmPelsHeight == maximum->dmPelsWidth) continue; + if (mode.dmPelsWidth == initial->dmPelsWidth && mode.dmPelsHeight == initial->dmPelsHeight) continue; + modes[modes_count++] = mode; + } + + mode.dmPelsWidth = initial->dmPelsWidth; + mode.dmPelsHeight = initial->dmPelsHeight; + modes[modes_count++] = mode; + + if (maximum->dmPelsWidth != initial->dmPelsWidth || maximum->dmPelsWidth != initial->dmPelsHeight) + { + mode.dmPelsWidth = maximum->dmPelsWidth; + mode.dmPelsHeight = maximum->dmPelsHeight; + modes[modes_count++] = mode; + } + } + + add_modes( current, modes_count, modes, ctx ); + free( modes ); +}
- UINT screen_width, screen_height, max_width, max_height, modes_count; - unsigned int depths[] = {8, 16, 0}; +static BOOL add_virtual_source( struct device_manager_ctx *ctx ) +{ + DEVMODEW current, initial = ctx->primary, maximum = ctx->primary; struct source virtual_source = { .state_flags = DISPLAY_DEVICE_ATTACHED_TO_DESKTOP | DISPLAY_DEVICE_PRIMARY_DEVICE | DISPLAY_DEVICE_VGA_COMPATIBLE, .id = ctx->source_count, .gpu = &ctx->gpu, }; - DEVMODEW current, *modes; - UINT i, j; + struct gdi_monitor monitor = {0};
/* Wine specific config key where source settings will be held, symlinked with the logically indexed config key */ snprintf( virtual_source.path, sizeof(virtual_source.path), "%s\%s\Video\%s\Sources\%s", config_keyA, @@ -1874,21 +1914,17 @@ static BOOL add_virtual_source( struct device_manager_ctx *ctx ) ctx->gpu.source_count++; ctx->source_count++;
- max_width = ctx->primary.dmPelsWidth; - max_height = ctx->primary.dmPelsHeight; - depths[ARRAY_SIZE(depths) - 1] = ctx->primary.dmBitsPerPel; - - if (!get_default_desktop_size( &screen_width, &screen_height )) + if (!get_default_desktop_size( &initial.dmPelsWidth, &initial.dmPelsHeight )) { - screen_width = max_width; - screen_height = max_height; + initial.dmPelsWidth = maximum.dmPelsWidth; + initial.dmPelsHeight = maximum.dmPelsHeight; }
if (!read_source_mode( ctx->source_key, ENUM_CURRENT_SETTINGS, ¤t )) { current = ctx->primary; - current.dmPelsWidth = screen_width; - current.dmPelsHeight = screen_height; + current.dmPelsWidth = initial.dmPelsWidth; + current.dmPelsHeight = initial.dmPelsHeight; }
monitor.rc_monitor.right = current.dmPelsWidth; @@ -1896,43 +1932,7 @@ static BOOL add_virtual_source( struct device_manager_ctx *ctx ) monitor.rc_work.right = current.dmPelsWidth; monitor.rc_work.bottom = current.dmPelsHeight; add_monitor( &monitor, ctx ); - - if (!(modes = malloc( ARRAY_SIZE(depths) * (ARRAY_SIZE(screen_sizes) + 2) * sizeof(*modes) ))) return FALSE; - - for (modes_count = i = 0; i < ARRAY_SIZE(depths); ++i) - { - DEVMODEW mode = - { - .dmFields = DM_DISPLAYORIENTATION | DM_BITSPERPEL | DM_PELSWIDTH | DM_PELSHEIGHT | DM_DISPLAYFLAGS | DM_DISPLAYFREQUENCY, - .dmDisplayFrequency = 60, - .dmBitsPerPel = depths[i], - }; - - for (j = 0; j < ARRAY_SIZE(screen_sizes); ++j) - { - mode.dmPelsWidth = screen_sizes[j].width; - mode.dmPelsHeight = screen_sizes[j].height; - - if (mode.dmPelsWidth > max_width || mode.dmPelsHeight > max_height) continue; - if (mode.dmPelsWidth == max_width && mode.dmPelsHeight == max_height) continue; - if (mode.dmPelsWidth == screen_width && mode.dmPelsHeight == screen_height) continue; - modes[modes_count++] = mode; - } - - mode.dmPelsWidth = screen_width; - mode.dmPelsHeight = screen_height; - modes[modes_count++] = mode; - - if (max_width != screen_width || max_height != screen_height) - { - mode.dmPelsWidth = max_width; - mode.dmPelsHeight = max_height; - modes[modes_count++] = mode; - } - } - - add_modes( ¤t, modes_count, modes, ctx ); - free( modes ); + add_virtual_modes( ctx, ¤t, &initial, &maximum );
return TRUE; }
On Mon Apr 22 12:49:33 2024 +0000, Rémi Bernon wrote:
changed this line in [version 5 of the diff](/wine/wine/-/merge_requests/5422/diffs?diff_id=110599&start_sha=85e47ab3c9bc90d4d99bf57e14d281ce06b16d22#85770a8b187bd82db4dbb9a2b8a5f34616049d0f_1489_1480)
I don't like much the idea of an additional parameter, I don't think drivers should be able to add virtual sources on their own.
On Wed May 1 09:18:14 2024 +0000, Zhiyi Zhang wrote:
Put this to where the old update_display_devices() was so the diff is clearer.
I can move it around before or keep it where it is but there are changes to the function regardless and the diff won't be much clearer. Keeping it where it is would require to forward declare `add_virtual_source`, or to move that one instead.
On Mon Apr 22 12:49:32 2024 +0000, Rémi Bernon wrote:
changed this line in [version 5 of the diff](/wine/wine/-/merge_requests/5422/diffs?diff_id=110599&start_sha=85e47ab3c9bc90d4d99bf57e14d281ce06b16d22#85770a8b187bd82db4dbb9a2b8a5f34616049d0f_1338_1338)
I don't think that an adapter source being enumerated necessarily means that you can enable it. ChangeDisplaySettings may fail already with the current code, for various reasons.
Ultimately, with full display device virtualization in mind, I think it will be interesting to keep both these physical / virtual sources. Instead of a winecfg switch, I can imagine display devices being virtualized dynamically from a physical output to a dedicated window, or de-virtualized from a window to a physical source.
On Mon Apr 22 12:49:34 2024 +0000, Rémi Bernon wrote:
changed this line in [version 5 of the diff](/wine/wine/-/merge_requests/5422/diffs?diff_id=110599&start_sha=85e47ab3c9bc90d4d99bf57e14d281ce06b16d22#85770a8b187bd82db4dbb9a2b8a5f34616049d0f_1836_1811)
Currently we use the primary display as a base for the virtual source / virtual desktop mode, but this is more a limitation than something intentional. With multiple monitors, virtual desktop mode won't work well on the non-primary display.
We should IMO implement support for non-primary virtual sources, which needs an initial, and a maximum display resolution.
On Wed May 1 09:18:15 2024 +0000, Zhiyi Zhang wrote:
What's the point of writing these modes for physical sources/monitors? We are not going to use them anyway.
Not much point but I don't see any reason to hide them either, especially if we keep dynamic virtualisation in mind.
On Mon Apr 22 12:49:33 2024 +0000, Rémi Bernon wrote:
changed this line in [version 5 of the diff](/wine/wine/-/merge_requests/5422/diffs?diff_id=110599&start_sha=85e47ab3c9bc90d4d99bf57e14d281ce06b16d22#85770a8b187bd82db4dbb9a2b8a5f34616049d0f_1198_1209)
This isn't necessary anymore since modes are now added all at once.
On Wed May 1 09:18:10 2024 +0000, Rémi Bernon wrote:
I don't think that an adapter source being enumerated necessarily means that you can enable it. ChangeDisplaySettings may fail already with the current code, for various reasons. Ultimately, with full display device virtualization in mind, I think it will be interesting to keep both these physical / virtual sources. Instead of a winecfg switch, I can imagine display devices being virtualized dynamically from a physical output to a dedicated window, or de-virtualized from a window to a physical source.
Exposing these detached sources produces the following errors when using virtual desktop. ``` 0090:err:system:NtUserChangeDisplaySettings Changing L"\\.\DISPLAY2" display settings returned -2. 0090:err:explorer:initialize_display_settings Failed to initialize registry display settings for L"\\.\DISPLAY2". ```
I think we can initialize the current modes in win32u as well instead of doing it in explorer.exe.
-
Rémi Bernon -
Rémi Bernon (＠rbernon)
-
Zhiyi Zhang (＠zhiyi)