[PATCH 0/4] MR5137: win32u: Use strings for adapter names and use them in the adapter config keys.
I'm not completely sure what the expectations are regarding these registry key paths, but @afrantzis said Wayland reports adapters in an arbitrary order and this would let us find the right adapter config and their current settings based on their unique name instead. Other drivers simply format the numeric ID themselves, though might not be 0-based index anymore. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5137
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/win32u/sysparams.c | 60 +++++++++++++++-------------------------- 1 file changed, 22 insertions(+), 38 deletions(-) diff --git a/dlls/win32u/sysparams.c b/dlls/win32u/sysparams.c index 5dcf42ff7f5..d264120909f 100644 --- a/dlls/win32u/sysparams.c +++ b/dlls/win32u/sysparams.c @@ -200,14 +200,10 @@ static const WCHAR guid_devclass_monitorW[] = {'{','4','D','3','6','E','9','6','E','-','E','3','2','5','-','1','1','C','E','-', 'B','F','C','1','-','0','8','0','0','2','B','E','1','0','3','1','8','}',0}; -static const WCHAR guid_devinterface_display_adapterW[] = - {'{','5','B','4','5','2','0','1','D','-','F','2','F','2','-','4','F','3','B','-', - '8','5','B','B','-','3','0','F','F','1','F','9','5','3','5','9','9','}',0}; - -static const WCHAR guid_display_device_arrivalW[] = - {'{','1','C','A','0','5','1','8','0','-','A','6','9','9','-','4','5','0','A','-', - '9','A','0','C','-','D','E','4','F','B','E','3','D','D','D','8','9','}',0}; +static const char guid_devinterface_display_adapterA[] = "{5B45201D-F2F2-4F3B-85BB-30FF1F953599}"; +static const char guid_display_device_arrivalA[] = "{1CA05180-A699-450A-9A0C-DE4FBE3DDD89}"; +static const char guid_devinterface_monitorA[] = "{E6F07B5F-EE97-4A90-B076-33F57BF4EAA7}"; static const WCHAR guid_devinterface_monitorW[] = {'{','E','6','F','0','7','B','5','F','-','E','E','9','7','-','4','A','9','0','-', 'B','0','7','6','-','3','3','F','5','7','B','F','4','E','A','A','7','}',0}; @@ -1131,7 +1127,7 @@ struct device_manager_ctx unsigned int output_count; unsigned int mode_count; HANDLE mutex; - WCHAR gpuid[128]; + char gpuid[128]; WCHAR gpu_guid[64]; LUID gpu_luid; HKEY adapter_key; @@ -1142,41 +1138,30 @@ struct device_manager_ctx UINT primary_height; }; -static void link_device( const WCHAR *instance, const WCHAR *class ) +static void link_device( const char *instance, const char *class ) { - unsigned int instance_len = lstrlenW( instance ), len; - unsigned int class_len = lstrlenW( class ); - WCHAR buffer[MAX_PATH], *ptr; + char buffer[MAX_PATH], *ptr; + WCHAR bufferW[MAX_PATH]; HKEY hkey, subkey; + unsigned int pos; static const WCHAR symbolic_linkW[] = {'S','y','m','b','o','l','i','c','L','i','n','k',0}; static const WCHAR hashW[] = {'#'}; - len = asciiz_to_unicode( buffer, "DeviceClasses\\" ) / sizeof(WCHAR) - 1; - memcpy( buffer + len, class, class_len * sizeof(WCHAR) ); - len += class_len; - len += asciiz_to_unicode( buffer + len, "\\##?#" ) / sizeof(WCHAR) - 1; - memcpy( buffer + len, instance, instance_len * sizeof(WCHAR) ); - for (ptr = buffer + len; *ptr; ptr++) if (*ptr == '\\') *ptr = '#'; - len += instance_len; - buffer[len++] = '#'; - memcpy( buffer + len, class, class_len * sizeof(WCHAR) ); - len += class_len; - hkey = reg_create_key( control_key, buffer, len * sizeof(WCHAR), 0, NULL ); + pos = snprintf( buffer, ARRAY_SIZE(buffer), "DeviceClasses\\%s\\", class ); + snprintf( buffer + pos, ARRAY_SIZE(buffer) - pos, "##?#%s#%s", instance, class ); + for (ptr = buffer + pos; *ptr; ptr++) if (*ptr == '\\') *ptr = '#'; - set_reg_value( hkey, device_instanceW, REG_SZ, instance, (instance_len + 1) * sizeof(WCHAR) ); + hkey = reg_create_key( control_key, bufferW, asciiz_to_unicode( bufferW, buffer ) - sizeof(WCHAR), 0, NULL ); + set_reg_value( hkey, device_instanceW, REG_SZ, bufferW, asciiz_to_unicode( bufferW, instance ) ); subkey = reg_create_key( hkey, hashW, sizeof(hashW), REG_OPTION_VOLATILE, NULL ); NtClose( hkey ); hkey = subkey; - len = asciiz_to_unicode( buffer, "\\\\?\\" ) / sizeof(WCHAR) - 1; - memcpy( buffer + len, instance, (instance_len + 1) * sizeof(WCHAR) ); - len += instance_len; - memcpy( buffer + len, class, (class_len + 1) * sizeof(WCHAR) ); - len += class_len + 1; + snprintf( buffer, ARRAY_SIZE(buffer), "\\\\?\\%s#%s", instance, class ); for (ptr = buffer + 4; *ptr; ptr++) if (*ptr == '\\') *ptr = '#'; - set_reg_value( hkey, symbolic_linkW, REG_SZ, buffer, len * sizeof(WCHAR) ); + set_reg_value( hkey, symbolic_linkW, REG_SZ, bufferW, asciiz_to_unicode( bufferW, buffer ) ); if ((subkey = reg_create_key( hkey, controlW, sizeof(controlW), REG_OPTION_VOLATILE, NULL ))) { @@ -1246,10 +1231,10 @@ static void add_gpu( const struct gdi_gpu *gpu, void *param ) prepare_devices(); } - sprintf( buffer, "PCI\\VEN_%04X&DEV_%04X&SUBSYS_%08X&REV_%02X\\%08X", + sprintf( ctx->gpuid, "PCI\\VEN_%04X&DEV_%04X&SUBSYS_%08X&REV_%02X\\%08X", gpu->vendor_id, gpu->device_id, gpu->subsys_id, gpu->revision_id, gpu_index ); - size = asciiz_to_unicode( ctx->gpuid, buffer ); - if (!(hkey = reg_create_key( enum_key, ctx->gpuid, size - sizeof(WCHAR), 0, NULL ))) return; + size = asciiz_to_unicode( bufferW, ctx->gpuid ); + if (!(hkey = reg_create_key( enum_key, bufferW, size - sizeof(WCHAR), 0, NULL ))) return; set_reg_value( hkey, classW, REG_SZ, displayW, sizeof(displayW) ); set_reg_value( hkey, class_guidW, REG_SZ, guid_devclass_displayW, @@ -1405,8 +1390,8 @@ static void add_gpu( const struct gdi_gpu *gpu, void *param ) NtClose( hkey ); - link_device( ctx->gpuid, guid_devinterface_display_adapterW ); - link_device( ctx->gpuid, guid_display_device_arrivalW ); + link_device( ctx->gpuid, guid_devinterface_display_adapterA ); + link_device( ctx->gpuid, guid_display_device_arrivalA ); } static void add_adapter( const struct gdi_adapter *adapter, void *param ) @@ -1465,8 +1450,7 @@ static void add_adapter( const struct gdi_adapter *adapter, void *param ) ctx->adapter_key = reg_create_key( config_key, bufferW, len * sizeof(WCHAR), REG_OPTION_VOLATILE, NULL ); - set_reg_value( ctx->adapter_key, gpu_idW, REG_SZ, ctx->gpuid, - (lstrlenW( ctx->gpuid ) + 1) * sizeof(WCHAR) ); + set_reg_value( ctx->adapter_key, gpu_idW, REG_SZ, bufferW, asciiz_to_unicode( bufferW, ctx->gpuid ) ); set_reg_value( ctx->adapter_key, state_flagsW, REG_DWORD, &adapter->state_flags, sizeof(adapter->state_flags) ); } @@ -1501,7 +1485,7 @@ static void add_monitor( const struct gdi_monitor *monitor, void *param ) 0, NULL ); if (!hkey) return; - link_device( bufferW, guid_devinterface_monitorW ); + link_device( instance, guid_devinterface_monitorA ); asciiz_to_unicode( bufferW, "Generic Non-PnP Monitor" ); set_reg_value( hkey, device_descW, REG_SZ, bufferW, (lstrlenW( bufferW ) + 1) * sizeof(WCHAR) ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5137
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/win32u/sysparams.c | 40 +++++++++++++++------------------------- 1 file changed, 15 insertions(+), 25 deletions(-) diff --git a/dlls/win32u/sysparams.c b/dlls/win32u/sysparams.c index d264120909f..e57407b6e79 100644 --- a/dlls/win32u/sysparams.c +++ b/dlls/win32u/sysparams.c @@ -1128,7 +1128,7 @@ struct device_manager_ctx unsigned int mode_count; HANDLE mutex; char gpuid[128]; - WCHAR gpu_guid[64]; + char gpu_guid[39]; LUID gpu_luid; HKEY adapter_key; /* for the virtual desktop settings */ @@ -1178,7 +1178,7 @@ static void add_gpu( const struct gdi_gpu *gpu, void *param ) char buffer[4096]; WCHAR bufferW[512]; KEY_VALUE_PARTIAL_INFORMATION *value = (void *)buffer; - unsigned int gpu_index, size; + unsigned int gpu_index, size, i; HKEY hkey, subkey; LARGE_INTEGER ft; ULONG memory_size; @@ -1290,17 +1290,16 @@ static void add_gpu( const struct gdi_gpu *gpu, void *param ) { GUID guid; uuid_create( &guid ); - sprintf( buffer, "{%08x-%04x-%04x-%02x%02x-%02x%02x%02x%02x%02x%02x}", + sprintf( ctx->gpu_guid, "{%08x-%04x-%04x-%02x%02x-%02x%02x%02x%02x%02x%02x}", (unsigned int)guid.Data1, guid.Data2, guid.Data3, guid.Data4[0], guid.Data4[1], guid.Data4[2], guid.Data4[3], guid.Data4[4], guid.Data4[5], guid.Data4[6], guid.Data4[7] ); - size = asciiz_to_unicode( ctx->gpu_guid, buffer ); - TRACE( "created guid %s\n", debugstr_w(ctx->gpu_guid) ); - set_reg_value( subkey, video_idW, REG_SZ, ctx->gpu_guid, size ); + TRACE( "created guid %s\n", debugstr_a(ctx->gpu_guid) ); + set_reg_value( subkey, video_idW, REG_SZ, bufferW, asciiz_to_unicode( bufferW, ctx->gpu_guid ) ); } else { - memcpy( ctx->gpu_guid, value->Data, value->DataLength ); - TRACE( "got guid %s\n", debugstr_w(ctx->gpu_guid) ); + for (i = 0; i < sizeof(ctx->gpu_guid); i++) ctx->gpu_guid[i] = value->Data[i]; + TRACE( "got guid %s\n", debugstr_a(ctx->gpu_guid) ); } NtClose( subkey ); } @@ -1415,16 +1414,12 @@ static void add_adapter( const struct gdi_adapter *adapter, void *param ) ctx->monitor_count = 0; ctx->mode_count = 0; - len = asciiz_to_unicode( bufferW, "\\Registry\\Machine\\System\\CurrentControlSet\\" - "Control\\Video\\" ) / sizeof(WCHAR) - 1; - lstrcpyW( bufferW + len, ctx->gpu_guid ); - len += lstrlenW( bufferW + len ); - sprintf( buffer, "\\%04x", adapter_index ); - len += asciiz_to_unicode( bufferW + len, buffer ) / sizeof(WCHAR) - 1; - hkey = reg_create_key( NULL, bufferW, len * sizeof(WCHAR), - REG_OPTION_VOLATILE | REG_OPTION_CREATE_LINK, NULL ); - if (!hkey) hkey = reg_create_key( NULL, bufferW, len * sizeof(WCHAR), - REG_OPTION_VOLATILE | REG_OPTION_OPEN_LINK, NULL ); + snprintf( buffer, ARRAY_SIZE(buffer), "\\Registry\\Machine\\System\\CurrentControlSet\\Control\\Video\\%s\\%04x", + ctx->gpu_guid, adapter_index ); + len = asciiz_to_unicode( bufferW, buffer ) - sizeof(WCHAR); + + hkey = reg_create_key( NULL, bufferW, len, REG_OPTION_VOLATILE | REG_OPTION_CREATE_LINK, NULL ); + if (!hkey) hkey = reg_create_key( NULL, bufferW, len, REG_OPTION_VOLATILE | REG_OPTION_OPEN_LINK, NULL ); sprintf( name, "\\Device\\Video%u", video_index ); asciiz_to_unicode( nameW, name ); @@ -1441,13 +1436,8 @@ static void add_adapter( const struct gdi_adapter *adapter, void *param ) else ERR( "failed to create link key\n" ); /* Following information is Wine specific, it doesn't really exist on Windows. */ - len = asciiz_to_unicode( bufferW, "System\\CurrentControlSet\\Control\\Video\\" ) - / sizeof(WCHAR) - 1; - lstrcpyW( bufferW + len, ctx->gpu_guid ); - len += lstrlenW( bufferW + len ); - sprintf( buffer, "\\%04x", adapter_index ); - len += asciiz_to_unicode( bufferW + len, buffer ) / sizeof(WCHAR) - 1; - ctx->adapter_key = reg_create_key( config_key, bufferW, len * sizeof(WCHAR), + snprintf( buffer, ARRAY_SIZE(buffer), "System\\CurrentControlSet\\Control\\Video\\%s\\%04x", ctx->gpu_guid, adapter_index ); + ctx->adapter_key = reg_create_key( config_key, bufferW, asciiz_to_unicode( bufferW, buffer ) - sizeof(WCHAR), REG_OPTION_VOLATILE, NULL ); set_reg_value( ctx->adapter_key, gpu_idW, REG_SZ, bufferW, asciiz_to_unicode( bufferW, ctx->gpuid ) ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5137
From: Rémi Bernon <rbernon(a)codeweavers.com> Adapter id/index is later used to describe the \\.\DISPLAY#, as well as \Device\Video#. It matches the video_index in the device_manager_ctx, so lets rename it to be consistent. --- dlls/win32u/sysparams.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/dlls/win32u/sysparams.c b/dlls/win32u/sysparams.c index e57407b6e79..b969cab90ee 100644 --- a/dlls/win32u/sysparams.c +++ b/dlls/win32u/sysparams.c @@ -1121,8 +1121,8 @@ static unsigned int format_date( WCHAR *bufferW, LONGLONG time ) struct device_manager_ctx { unsigned int gpu_count; + unsigned int gpu_adapter_count; unsigned int adapter_count; - unsigned int video_count; unsigned int monitor_count; unsigned int output_count; unsigned int mode_count; @@ -1217,7 +1217,7 @@ static void add_gpu( const struct gdi_gpu *gpu, void *param ) gpu->vendor_id, gpu->device_id, gpu->subsys_id, gpu->revision_id ); gpu_index = ctx->gpu_count++; - ctx->adapter_count = 0; + ctx->gpu_adapter_count = 0; ctx->monitor_count = 0; ctx->mode_count = 0; @@ -1396,7 +1396,7 @@ static void add_gpu( const struct gdi_gpu *gpu, void *param ) static void add_adapter( const struct gdi_adapter *adapter, void *param ) { struct device_manager_ctx *ctx = param; - unsigned int adapter_index, video_index, len; + unsigned int adapter_name, adapter_index, len; char name[64], buffer[MAX_PATH]; WCHAR nameW[64], bufferW[MAX_PATH]; HKEY hkey; @@ -1409,19 +1409,19 @@ static void add_adapter( const struct gdi_adapter *adapter, void *param ) ctx->adapter_key = NULL; } + adapter_name = ctx->gpu_adapter_count++; adapter_index = ctx->adapter_count++; - video_index = ctx->video_count++; ctx->monitor_count = 0; ctx->mode_count = 0; snprintf( buffer, ARRAY_SIZE(buffer), "\\Registry\\Machine\\System\\CurrentControlSet\\Control\\Video\\%s\\%04x", - ctx->gpu_guid, adapter_index ); + ctx->gpu_guid, adapter_name ); len = asciiz_to_unicode( bufferW, buffer ) - sizeof(WCHAR); hkey = reg_create_key( NULL, bufferW, len, REG_OPTION_VOLATILE | REG_OPTION_CREATE_LINK, NULL ); if (!hkey) hkey = reg_create_key( NULL, bufferW, len, REG_OPTION_VOLATILE | REG_OPTION_OPEN_LINK, NULL ); - sprintf( name, "\\Device\\Video%u", video_index ); + sprintf( name, "\\Device\\Video%u", adapter_index ); asciiz_to_unicode( nameW, name ); set_reg_value( video_key, nameW, REG_SZ, bufferW, (lstrlenW( bufferW ) + 1) * sizeof(WCHAR) ); @@ -1436,7 +1436,7 @@ static void add_adapter( const struct gdi_adapter *adapter, void *param ) else ERR( "failed to create link key\n" ); /* Following information is Wine specific, it doesn't really exist on Windows. */ - snprintf( buffer, ARRAY_SIZE(buffer), "System\\CurrentControlSet\\Control\\Video\\%s\\%04x", ctx->gpu_guid, adapter_index ); + snprintf( buffer, ARRAY_SIZE(buffer), "System\\CurrentControlSet\\Control\\Video\\%s\\%04x", ctx->gpu_guid, adapter_name ); ctx->adapter_key = reg_create_key( config_key, bufferW, asciiz_to_unicode( bufferW, buffer ) - sizeof(WCHAR), REG_OPTION_VOLATILE, NULL ); @@ -1468,7 +1468,7 @@ static void add_monitor( const struct gdi_monitor *monitor, void *param ) strcpy( monitor_id_string, "Default_Monitor" ); sprintf( buffer, "MonitorID%u", monitor_index ); - sprintf( instance, "DISPLAY\\%s\\%04X&%04X", monitor_id_string, ctx->video_count - 1, monitor_index ); + sprintf( instance, "DISPLAY\\%s\\%04X&%04X", monitor_id_string, ctx->adapter_count - 1, monitor_index ); set_reg_ascii_value( ctx->adapter_key, buffer, instance ); hkey = reg_create_key( enum_key, bufferW, asciiz_to_unicode( bufferW, instance ) - sizeof(WCHAR), @@ -1524,7 +1524,7 @@ static void add_monitor( const struct gdi_monitor *monitor, void *param ) if ((subkey = reg_create_key( hkey, wine_devpropkey_monitor_adapternameW, sizeof(wine_devpropkey_monitor_adapternameW), 0, NULL ))) { - sprintf( buffer, "\\\\.\\DISPLAY%u", ctx->video_count ); + sprintf( buffer, "\\\\.\\DISPLAY%u", ctx->adapter_count ); set_reg_value( subkey, NULL, 0xffff0000 | DEVPROP_TYPE_STRING, bufferW, asciiz_to_unicode( bufferW, buffer )); NtClose( subkey ); @@ -1561,7 +1561,7 @@ static void add_mode( const DEVMODEW *mode, BOOL current, void *param ) struct device_manager_ctx *ctx = param; DEVMODEW nopos_mode; - if (!ctx->adapter_count) + if (!ctx->gpu_adapter_count) { static const struct gdi_adapter default_adapter = { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5137
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/win32u/sysparams.c | 13 ++++++++----- dlls/wineandroid.drv/init.c | 1 + dlls/winemac.drv/display.c | 7 ++----- dlls/winewayland.drv/display.c | 5 ++--- dlls/winex11.drv/display.c | 6 +++--- dlls/winex11.drv/x11drv.h | 2 +- dlls/winex11.drv/xinerama.c | 11 +++++------ dlls/winex11.drv/xrandr.c | 13 ++++++++----- include/wine/gdi_driver.h | 2 +- 9 files changed, 31 insertions(+), 29 deletions(-) diff --git a/dlls/win32u/sysparams.c b/dlls/win32u/sysparams.c index b969cab90ee..3f4d2821d33 100644 --- a/dlls/win32u/sysparams.c +++ b/dlls/win32u/sysparams.c @@ -1396,7 +1396,7 @@ static void add_gpu( const struct gdi_gpu *gpu, void *param ) static void add_adapter( const struct gdi_adapter *adapter, void *param ) { struct device_manager_ctx *ctx = param; - unsigned int adapter_name, adapter_index, len; + unsigned int adapter_index, len; char name[64], buffer[MAX_PATH]; WCHAR nameW[64], bufferW[MAX_PATH]; HKEY hkey; @@ -1409,13 +1409,13 @@ static void add_adapter( const struct gdi_adapter *adapter, void *param ) ctx->adapter_key = NULL; } - adapter_name = ctx->gpu_adapter_count++; + ctx->gpu_adapter_count++; adapter_index = ctx->adapter_count++; ctx->monitor_count = 0; ctx->mode_count = 0; - snprintf( buffer, ARRAY_SIZE(buffer), "\\Registry\\Machine\\System\\CurrentControlSet\\Control\\Video\\%s\\%04x", - ctx->gpu_guid, adapter_name ); + snprintf( buffer, ARRAY_SIZE(buffer), "\\Registry\\Machine\\System\\CurrentControlSet\\Control\\Video\\%s\\%s", + ctx->gpu_guid, adapter->name ); len = asciiz_to_unicode( bufferW, buffer ) - sizeof(WCHAR); hkey = reg_create_key( NULL, bufferW, len, REG_OPTION_VOLATILE | REG_OPTION_CREATE_LINK, NULL ); @@ -1436,7 +1436,7 @@ static void add_adapter( const struct gdi_adapter *adapter, void *param ) else ERR( "failed to create link key\n" ); /* Following information is Wine specific, it doesn't really exist on Windows. */ - snprintf( buffer, ARRAY_SIZE(buffer), "System\\CurrentControlSet\\Control\\Video\\%s\\%04x", ctx->gpu_guid, adapter_name ); + snprintf( buffer, ARRAY_SIZE(buffer), "System\\CurrentControlSet\\Control\\Video\\%s\\%s", ctx->gpu_guid, adapter->name ); ctx->adapter_key = reg_create_key( config_key, bufferW, asciiz_to_unicode( bufferW, buffer ) - sizeof(WCHAR), REG_OPTION_VOLATILE, NULL ); @@ -1566,6 +1566,7 @@ static void add_mode( const DEVMODEW *mode, BOOL current, void *param ) static const struct gdi_adapter default_adapter = { .state_flags = DISPLAY_DEVICE_ATTACHED_TO_DESKTOP | DISPLAY_DEVICE_PRIMARY_DEVICE | DISPLAY_DEVICE_VGA_COMPATIBLE, + .name = "Default Adapter", }; TRACE( "adding default fake adapter\n" ); add_adapter( &default_adapter, ctx ); @@ -1831,6 +1832,7 @@ static BOOL default_update_display_devices( const struct gdi_device_manager *man static const struct gdi_adapter adapter = { .state_flags = DISPLAY_DEVICE_ATTACHED_TO_DESKTOP | DISPLAY_DEVICE_PRIMARY_DEVICE | DISPLAY_DEVICE_VGA_COMPATIBLE, + .name = "Default Adapter", }; struct gdi_monitor monitor = {0}; DEVMODEW mode = {{0}}; @@ -1940,6 +1942,7 @@ static BOOL desktop_update_display_devices( BOOL force, struct device_manager_ct static const struct gdi_adapter adapter = { .state_flags = DISPLAY_DEVICE_ATTACHED_TO_DESKTOP | DISPLAY_DEVICE_PRIMARY_DEVICE | DISPLAY_DEVICE_VGA_COMPATIBLE, + .name = "Virtual Adapter", }; struct gdi_monitor monitor = {0}; static struct screen_size diff --git a/dlls/wineandroid.drv/init.c b/dlls/wineandroid.drv/init.c index c39bf690a06..f65e28662d7 100644 --- a/dlls/wineandroid.drv/init.c +++ b/dlls/wineandroid.drv/init.c @@ -277,6 +277,7 @@ BOOL ANDROID_UpdateDisplayDevices( const struct gdi_device_manager *device_manag static const struct gdi_adapter adapter = { .state_flags = DISPLAY_DEVICE_ATTACHED_TO_DESKTOP | DISPLAY_DEVICE_PRIMARY_DEVICE | DISPLAY_DEVICE_VGA_COMPATIBLE, + .name = "Default Adapter", }; struct gdi_monitor gdi_monitor = { diff --git a/dlls/winemac.drv/display.c b/dlls/winemac.drv/display.c index 6d6e76da1ee..8c0442174c0 100644 --- a/dlls/winemac.drv/display.c +++ b/dlls/winemac.drv/display.c @@ -1178,11 +1178,8 @@ BOOL macdrv_UpdateDisplayDevices( const struct gdi_device_manager *device_manage for (adapter = adapters; adapter < adapters + adapter_count; adapter++) { DEVMODEW current_mode = { .dmSize = sizeof(current_mode) }; - struct gdi_adapter gdi_adapter = - { - .id = adapter->id, - .state_flags = adapter->state_flags, - }; + struct gdi_adapter gdi_adapter = { .state_flags = adapter->state_flags }; + sprintf( gdi_adapter.name, "%04x", adapter->id ); device_manager->add_adapter( &gdi_adapter, param ); if (macdrv_get_monitors(adapter->id, &monitors, &monitor_count)) break; diff --git a/dlls/winewayland.drv/display.c b/dlls/winewayland.drv/display.c index 6c399cbf321..e95fade0016 100644 --- a/dlls/winewayland.drv/display.c +++ b/dlls/winewayland.drv/display.c @@ -214,13 +214,12 @@ static void wayland_add_device_adapter(const struct gdi_device_manager *device_m void *param, INT output_id) { struct gdi_adapter adapter; - adapter.id = output_id; + sprintf( adapter.name, "%04x", output_id ); adapter.state_flags = DISPLAY_DEVICE_ATTACHED_TO_DESKTOP; if (output_id == 0) adapter.state_flags |= DISPLAY_DEVICE_PRIMARY_DEVICE; - TRACE("id=0x%s state_flags=0x%x\n", - wine_dbgstr_longlong(adapter.id), (UINT)adapter.state_flags); + TRACE("id=0x%s state_flags=0x%x\n", adapter.name, (UINT)adapter.state_flags); device_manager->add_adapter(&adapter, param); } diff --git a/dlls/winex11.drv/display.c b/dlls/winex11.drv/display.c index 49475571f71..142f581c6ce 100644 --- a/dlls/winex11.drv/display.c +++ b/dlls/winex11.drv/display.c @@ -404,7 +404,7 @@ RECT get_host_primary_monitor_rect(void) /* The first monitor is always primary */ if (host_handler.get_gpus(&gpus, &gpu_count, FALSE) && gpu_count && host_handler.get_adapters(gpus[0].id, &adapters, &adapter_count) && adapter_count && - host_handler.get_monitors(adapters[0].id, &monitors, &monitor_count) && monitor_count) + host_handler.get_monitors(adapters[0].name, &monitors, &monitor_count) && monitor_count) rect = monitors[0].rc_monitor; if (gpus) host_handler.free_gpus(gpus); @@ -550,8 +550,8 @@ BOOL X11DRV_UpdateDisplayDevices( const struct gdi_device_manager *device_manage device_manager->add_adapter( &adapters[adapter], param ); - if (!host_handler.get_monitors( adapters[adapter].id, &monitors, &monitor_count )) break; - TRACE("adapter: %#lx, monitor count: %d\n", adapters[adapter].id, monitor_count); + if (!host_handler.get_monitors( adapters[adapter].name, &monitors, &monitor_count )) break; + TRACE("adapter: %s, monitor count: %d\n", adapters[adapter].name, monitor_count); /* Initialize monitors */ for (monitor = 0; monitor < monitor_count; monitor++) diff --git a/dlls/winex11.drv/x11drv.h b/dlls/winex11.drv/x11drv.h index 9c1b8012466..0a6b45d76e3 100644 --- a/dlls/winex11.drv/x11drv.h +++ b/dlls/winex11.drv/x11drv.h @@ -788,7 +788,7 @@ struct x11drv_display_device_handler * The first monitor has to be primary if adapter is primary. * * Return FALSE on failure with parameters unchanged */ - BOOL (*get_monitors)(ULONG_PTR adapter_id, struct gdi_monitor **monitors, int *count); + BOOL (*get_monitors)(const char *adapter_id, struct gdi_monitor **monitors, int *count); /* free_gpus will be called to free a GPU list from get_gpus */ void (*free_gpus)(struct gdi_gpu *gpus); diff --git a/dlls/winex11.drv/xinerama.c b/dlls/winex11.drv/xinerama.c index 43b25c02b86..17414e6d06e 100644 --- a/dlls/winex11.drv/xinerama.c +++ b/dlls/winex11.drv/xinerama.c @@ -254,7 +254,7 @@ static BOOL xinerama_get_adapters( ULONG_PTR gpu_id, struct gdi_adapter **new_ad continue; /* Use monitor index as id */ - adapters[index].id = (ULONG_PTR)i; + sprintf( adapters[index].name, "%04x", i ); if (i == primary_index) adapters[index].state_flags |= DISPLAY_DEVICE_PRIMARY_DEVICE; @@ -285,13 +285,12 @@ static void xinerama_free_adapters( struct gdi_adapter *adapters ) free( adapters ); } -static BOOL xinerama_get_monitors( ULONG_PTR adapter_id, struct gdi_monitor **new_monitors, int *count ) +static BOOL xinerama_get_monitors( const char *adapter_id, struct gdi_monitor **new_monitors, int *count ) { + int i, first, index = 0, monitor_count = 0; struct gdi_monitor *monitor; - INT first = (INT)adapter_id; - INT monitor_count = 0; - INT index = 0; - INT i; + + if (!sscanf( adapter_id, "%04x", &first )) return FALSE; pthread_mutex_lock( &xinerama_mutex ); diff --git a/dlls/winex11.drv/xrandr.c b/dlls/winex11.drv/xrandr.c index 5c0269f3550..3df34ad7004 100644 --- a/dlls/winex11.drv/xrandr.c +++ b/dlls/winex11.drv/xrandr.c @@ -977,7 +977,7 @@ static BOOL xrandr14_get_adapters( ULONG_PTR gpu_id, struct gdi_adapter **new_ad { /* Use RROutput as adapter id. The reason of not using RRCrtc is that we need to detect inactive but * attached monitors */ - adapters[adapter_count].id = outputs[i]; + sprintf( adapters[adapter_count].name, "%04lx", outputs[i] ); if (!detached) adapters[adapter_count].state_flags |= DISPLAY_DEVICE_ATTACHED_TO_DESKTOP; if (is_crtc_primary( primary_rect, crtc_info )) @@ -1031,7 +1031,7 @@ static void xrandr14_free_adapters( struct gdi_adapter *adapters ) free( adapters ); } -static BOOL xrandr14_get_monitors( ULONG_PTR adapter_id, struct gdi_monitor **new_monitors, int *count ) +static BOOL xrandr14_get_monitors( const char *adapter_id, struct gdi_monitor **new_monitors, int *count ) { struct gdi_monitor *realloc_monitors, *monitors = NULL; XRRScreenResources *screen_resources = NULL; @@ -1040,8 +1040,11 @@ static BOOL xrandr14_get_monitors( ULONG_PTR adapter_id, struct gdi_monitor **ne INT primary_index = 0, monitor_count = 0, capacity; RECT primary_rect; BOOL ret = FALSE; + RROutput output; INT i; + if (!sscanf( adapter_id, "%04lx", &output )) return FALSE; + screen_resources = xrandr_get_screen_resources(); if (!screen_resources) goto done; @@ -1052,7 +1055,7 @@ static BOOL xrandr14_get_monitors( ULONG_PTR adapter_id, struct gdi_monitor **ne if (!monitors) goto done; - output_info = pXRRGetOutputInfo( gdi_display, screen_resources, adapter_id ); + output_info = pXRRGetOutputInfo( gdi_display, screen_resources, output ); if (!output_info) goto done; @@ -1066,7 +1069,7 @@ static BOOL xrandr14_get_monitors( ULONG_PTR adapter_id, struct gdi_monitor **ne /* Inactive but attached monitor, no need to check for mirrored/replica monitors */ if (!output_info->crtc || !crtc_info->mode) { - monitors[monitor_count].edid_len = get_edid( adapter_id, &monitors[monitor_count].edid ); + monitors[monitor_count].edid_len = get_edid( output, &monitors[monitor_count].edid ); monitor_count = 1; } /* Active monitors, need to find other monitors with the same coordinates as mirrored */ @@ -1255,7 +1258,7 @@ static BOOL xrandr14_get_id( const WCHAR *device_name, BOOL is_primary, x11drv_s for (adapter_idx = 0; adapter_idx < adapter_count; ++adapter_idx) { - new_current_modes[new_current_mode_count + adapter_idx].id = adapters[adapter_idx].id; + sscanf( adapters[adapter_idx].name, "%04lx", &new_current_modes[new_current_mode_count + adapter_idx].id ); new_current_modes[new_current_mode_count + adapter_idx].loaded = FALSE; } new_current_mode_count += adapter_count; diff --git a/include/wine/gdi_driver.h b/include/wine/gdi_driver.h index 4acf23df1d4..becfc533d66 100644 --- a/include/wine/gdi_driver.h +++ b/include/wine/gdi_driver.h @@ -256,7 +256,7 @@ struct gdi_gpu struct gdi_adapter { - ULONG_PTR id; + char name[128]; DWORD state_flags; }; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5137
Zhiyi Zhang (@zhiyi) commented about dlls/win32u/sysparams.c:
ctx->adapter_key = NULL; }
- adapter_name = ctx->gpu_adapter_count++; + ctx->gpu_adapter_count++; adapter_index = ctx->adapter_count++; ctx->monitor_count = 0; ctx->mode_count = 0;
- snprintf( buffer, ARRAY_SIZE(buffer), "\\Registry\\Machine\\System\\CurrentControlSet\\Control\\Video\\%s\\%04x", - ctx->gpu_guid, adapter_name ); + snprintf( buffer, ARRAY_SIZE(buffer), "\\Registry\\Machine\\System\\CurrentControlSet\\Control\\Video\\%s\\%s", + ctx->gpu_guid, adapter->name );
This is wrong. The name should be a 4-digit string as it's on Windows. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5137#note_62634
I don't think this MR is going in the right direction. (struct gdi_adapter).id is internal to each driver. So it doesn't have any use in win32u. It got exposed to win32u because of the win32u/user32 refactoring. What you need to worry about is the map from GDI adapter name \\.\DISPLAY* to wl_output. The map must be stable so that each \\.\DISPLAY* should refer to the same wl_output or wl_output collection (concerning mirroring) when no output changes are detected. Better, keep this map consistent even after output changes so that each \\.\DISPLAY* still points to the same wl_output. The number index in \\.\DISPLAY* is from (struct adapter).id, generated sequentially in update_display_cache_from_registry(). So what you need to do is to make sure that winewayland.drv knows how to map \\.\DISPLAY* to each wl_output. What winex11.drv RandR 1.4 backend does is that it always adds the GDI adapter in the same order, so xrandr14_get_id() knows which RandR output a DISPLAY index points to. So I guess you can always add GDI adapters with them sorted alphabetically by the wl_output name and keep this GDI display index to the wl_output map in winewayland.drv. For example, entry [0] "\\.\DISPLAY1" -> "DP-1" and entry [1] "\\.\DISPLAY2" -> "HDMI-1". And then you call (struct gdi_device_manager).add_adapter to add "DP-1" wl_output first so when "\\.\DISPLAY1" is passed to winewayland.drv, you can know "\\.\DISPLAY1" points to the first entry, which is "DP-1". I don't think \\.\DISPLAY* indices are guaranteed to be the same across display changes so you can ignore output changes affecting GDI display indices for now because you can remap them due to a new display device initialization, which is what Wine does now. Then, of course, you'll need to update display devices when the reported wl_output array changes by responding to their events. You'll also need to make sure the map is the same when it's called from another process. Finally, back to the original question of (struct gdi_adapter).id, you can pass the wl_output name internally in winewayland.drv the way you like. But such internal details shouldn't concern other graphics drivers and win32u. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5137#note_62635
This doesn't sound right though, and although I understand that this registry key may need to use the integer indices matching the \.\DISPLAY indices, I think we'll then need to introduce some mapping in win32u to match these indices with the physical adapters reported by the drivers. \.\DISPLAY indices only correspond to the index the physical adapter was enumerated at, they do not match a specific physical adapter. Lets consider a scenario where a new adapter appears, such as when hot-plugging. Whether we sort the physical adapters or not on the driver side, and unless it ends up last, the mapping between \.\DISPLAY indices and physical adapters will be different. In win32u we use that registry key to save the display settings. The current and enumerated modes are refreshed on every enumeration, but the registry settings is kept from the last time it was written. When the \.\DISPLAY to physical index mapping changes, we will incorrectly use the registry settings that was written for the old physical adapter, instead of creating one from scratch for that new adapter. So in my opinion we need to have a physical adapter id -> logical index mapping in win32u, and keep the display settings linked to the physical adapters. This will also save a lot of complexity on the driver side as they won't need to sort anything anymore. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5137#note_62639
So in my opinion we need to have a physical adapter id -> logical index mapping in win32u, and keep the display settings linked to the physical adapters. This will also save a lot of complexity on the driver side as they won't need to sort anything anymore.
I agree that we should have a physical adapter id (or output name as in wl_output) to \\.\DISPLAY* map. The map currently exists in graphics drivers. If we want to deduplicate code and put it in win32u, I think that's ok. Ideally, we should let graphics drivers expose their physical adapters and outputs to win32u. Then we can build GDI adapters(logical adapters, that correspond to win32u outputs) and GDI monitors from the win32u physical display devices. Then graphics driver API can use physical device information only to get and set device state, eliminating the need for something like X11DRV_ChangeDisplaySettings(), which uses GDI adapter names instead of physical device names, thus removing the need for GDI->physical device map. The D3DKMT* functions are worth looking into for defining such a graphics driver interface to win32u. Anyway, this MR needs more thought. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5137#note_62659
Take winex11.drv for example, there are four functions that use GDI display names. They are X11DRV_ChangeDisplaySettings, X11DRV_GetCurrentDisplaySettings, X11DRV_GetDisplayDepth, and X11DRV_UpdateDisplayDevices. I think we would want to work towards the goal of using physical display names here so we can move the GDI display names to physical display names mapping to win32u. Since winex11.drv already have such a mapping but winewayland.drv doesn't. We could also support such a mapping in win32u first for winewayland.drv. And then gradually phase out the winex11.drv internal mapping. So the wl_output name should be in "struct monitor" instead. I would prefer "struct gpu" or "struct adapter" for physical GPU and "struct monitor" for physical display outputs. Then "struct gdi_adapter" and "struct gdi_monitor" can be constructed from the physical display information. This way, we have a clear separation between physical and logical/GDI displays. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5137#note_62767
participants (2)
-
Rémi Bernon -
Zhiyi Zhang (＠zhiyi)