[PATCH v4 0/3] MR2593: winex11.drv: UpdateDisplayDevice regression fixes
This MR contains two somewhat related commits/fixes for regressions that were made apparent in https://bugs.winehq.org/show_bug.cgi?id=54781. 1. The adapter ids used by the host and settings handlers are not guaranteed to be compatible, so ensure we are using the proper id with each handler. Note that although the possibly incompatible id was already used in `settings_handler.get_modes()` before, the settings handler was ignoring it anyway for `get_modes()` in the case it was really incompatible (e.g., using host handler xrandr 1.4, settings handler xrandr 1.0), . The problem manifests now since we are additionally calling `settings_handler.get_current_mode()` which does check the id value (e.g., in xrandr 1.0 to differentiate primary vs non-primary). 2. The virtual desktop get_current_mode() implementation recurses into win32u (through NtUserGetPrimaryScreenRect) causing a deadlock if we call it from within UpdateDisplayDevices. There is a way to avoid the deadlock if we get the current_modes before calling add_gpu(), but we don't need to get and update the win32u current/registry in the virtual desktop case anyway, so skip the call completely instead. -- v4: winex11.drv: Use a distinct type for the settings id. winex11.drv: Do not call desktop get_current_mode() from UpdateDisplayDevices. https://gitlab.winehq.org/wine/wine/-/merge_requests/2593
From: Alexandros Frantzis <alexandros.frantzis(a)collabora.com> The adapter ids used by the host and settings handlers are not guaranteed to be compatible, so ensure we are using the proper id with each handler. Signed-off-by: Alexandros Frantzis <alexandros.frantzis(a)collabora.com> --- dlls/winex11.drv/display.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/dlls/winex11.drv/display.c b/dlls/winex11.drv/display.c index cd36c3c559c..75b585ff201 100644 --- a/dlls/winex11.drv/display.c +++ b/dlls/winex11.drv/display.c @@ -564,6 +564,10 @@ BOOL X11DRV_UpdateDisplayDevices( const struct gdi_device_manager *device_manage for (adapter = 0; adapter < adapter_count; adapter++) { DEVMODEW current_mode = {.dmSize = sizeof(current_mode)}; + WCHAR devname[32]; + char buffer[32]; + ULONG_PTR settings_id; + BOOL is_primary = adapters[adapter].state_flags & DISPLAY_DEVICE_PRIMARY_DEVICE; device_manager->add_adapter( &adapters[adapter], param ); @@ -576,8 +580,13 @@ BOOL X11DRV_UpdateDisplayDevices( const struct gdi_device_manager *device_manage handler->free_monitors(monitors, monitor_count); - settings_handler.get_current_mode( adapters[adapter].id, ¤t_mode ); - if (!settings_handler.get_modes( adapters[adapter].id, EDS_ROTATEDMODE, &modes, &mode_count )) + /* Get the settings handler id for the adapter */ + snprintf( buffer, sizeof(buffer), "\\\\.\\DISPLAY%d", adapter + 1 ); + asciiz_to_unicode( devname, buffer ); + if (!settings_handler.get_id( devname, is_primary, &settings_id )) break; + + settings_handler.get_current_mode( settings_id, ¤t_mode ); + if (!settings_handler.get_modes( settings_id, EDS_ROTATEDMODE, &modes, &mode_count )) continue; for (mode = modes; mode_count; mode_count--) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2593
From: Alexandros Frantzis <alexandros.frantzis(a)collabora.com> We don't need to set the win32u current mode when we are in virtual desktop mode, and, additionally, skipping this avoids a deadlock, since the virtual desktop get_current_mode() implementation recurses into win32u. Signed-off-by: Alexandros Frantzis <alexandros.frantzis(a)collabora.com> Fixes: 4232312dffe1cd115aa6bfd755f5b7c6a403e3fc Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=54781 --- dlls/winex11.drv/display.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/dlls/winex11.drv/display.c b/dlls/winex11.drv/display.c index 75b585ff201..c75977002ef 100644 --- a/dlls/winex11.drv/display.c +++ b/dlls/winex11.drv/display.c @@ -542,10 +542,12 @@ BOOL X11DRV_UpdateDisplayDevices( const struct gdi_device_manager *device_manage INT gpu, adapter, monitor; DEVMODEW *modes, *mode; UINT mode_count; + BOOL virtual_desktop; if (!force && !force_display_devices_refresh) return TRUE; force_display_devices_refresh = FALSE; - handler = is_virtual_desktop() ? &desktop_handler : &host_handler; + virtual_desktop = is_virtual_desktop(); + handler = virtual_desktop ? &desktop_handler : &host_handler; TRACE("via %s\n", wine_dbgstr_a(handler->name)); @@ -585,7 +587,11 @@ BOOL X11DRV_UpdateDisplayDevices( const struct gdi_device_manager *device_manage asciiz_to_unicode( devname, buffer ); if (!settings_handler.get_id( devname, is_primary, &settings_id )) break; - settings_handler.get_current_mode( settings_id, ¤t_mode ); + /* We don't need to set the win32u current mode when we are in + * virtual desktop mode, and, additionally, skipping this avoids a + * deadlock, since the desktop get_current_mode() implementation + * recurses into win32u. */ + if (!virtual_desktop) settings_handler.get_current_mode( settings_id, ¤t_mode ); if (!settings_handler.get_modes( settings_id, EDS_ROTATEDMODE, &modes, &mode_count )) continue; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2593
From: Alexandros Frantzis <alexandros.frantzis(a)collabora.com> This helps us avoid using an id from a different namespace (e.g., from the host handler id namespace) as a settings id. Signed-off-by: Alexandros Frantzis <alexandros.frantzis(a)collabora.com> --- dlls/winex11.drv/desktop.c | 10 +++++----- dlls/winex11.drv/display.c | 36 ++++++++++++++++++------------------ dlls/winex11.drv/x11drv.h | 11 +++++++---- dlls/winex11.drv/xrandr.c | 32 ++++++++++++++++---------------- dlls/winex11.drv/xvidmode.c | 14 +++++++------- 5 files changed, 53 insertions(+), 50 deletions(-) diff --git a/dlls/winex11.drv/desktop.c b/dlls/winex11.drv/desktop.c index 8a9f476ede2..26350fbe558 100644 --- a/dlls/winex11.drv/desktop.c +++ b/dlls/winex11.drv/desktop.c @@ -123,10 +123,10 @@ BOOL is_virtual_desktop(void) } /* Virtual desktop display settings handler */ -static BOOL X11DRV_desktop_get_id( const WCHAR *device_name, BOOL is_primary, ULONG_PTR *id ) +static BOOL X11DRV_desktop_get_id( const WCHAR *device_name, BOOL is_primary, x11drv_settings_id *id ) { if (!is_primary) return FALSE; - *id = 0; + id->id = 0; return TRUE; } @@ -143,7 +143,7 @@ static void add_desktop_mode( DEVMODEW *mode, DWORD depth, DWORD width, DWORD he mode->dmDisplayFrequency = 60; } -static BOOL X11DRV_desktop_get_modes( ULONG_PTR id, DWORD flags, DEVMODEW **new_modes, UINT *mode_count ) +static BOOL X11DRV_desktop_get_modes( x11drv_settings_id id, DWORD flags, DEVMODEW **new_modes, UINT *mode_count ) { UINT depth_idx, size_idx, mode_idx = 0; UINT screen_width, screen_height; @@ -197,7 +197,7 @@ static void X11DRV_desktop_free_modes( DEVMODEW *modes ) free( modes ); } -static BOOL X11DRV_desktop_get_current_mode( ULONG_PTR id, DEVMODEW *mode ) +static BOOL X11DRV_desktop_get_current_mode( x11drv_settings_id id, DEVMODEW *mode ) { RECT primary_rect = NtUserGetPrimaryMonitorRect(); @@ -214,7 +214,7 @@ static BOOL X11DRV_desktop_get_current_mode( ULONG_PTR id, DEVMODEW *mode ) return TRUE; } -static LONG X11DRV_desktop_set_current_mode( ULONG_PTR id, const DEVMODEW *mode ) +static LONG X11DRV_desktop_set_current_mode( x11drv_settings_id id, const DEVMODEW *mode ) { if (mode->dmFields & DM_BITSPERPEL && mode->dmBitsPerPel != screen_bpp) WARN("Cannot change screen color depth from %dbits to %dbits!\n", diff --git a/dlls/winex11.drv/display.c b/dlls/winex11.drv/display.c index c75977002ef..ab3159dc923 100644 --- a/dlls/winex11.drv/display.c +++ b/dlls/winex11.drv/display.c @@ -37,7 +37,7 @@ static struct x11drv_settings_handler settings_handler; struct x11drv_display_depth { struct list entry; - ULONG_PTR display_id; + x11drv_settings_id display_id; DWORD depth; }; @@ -66,13 +66,13 @@ 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, BOOL is_primary, ULONG_PTR *id) +static BOOL nores_get_id(const WCHAR *device_name, BOOL is_primary, x11drv_settings_id *id) { - *id = is_primary ? 1 : 0; + id->id = is_primary ? 1 : 0; return TRUE; } -static BOOL nores_get_modes(ULONG_PTR id, DWORD flags, DEVMODEW **new_modes, UINT *mode_count) +static BOOL nores_get_modes(x11drv_settings_id id, DWORD flags, DEVMODEW **new_modes, UINT *mode_count) { RECT primary = get_host_primary_monitor_rect(); DEVMODEW *modes; @@ -105,7 +105,7 @@ static void nores_free_modes(DEVMODEW *modes) free(modes); } -static BOOL nores_get_current_mode(ULONG_PTR id, DEVMODEW *mode) +static BOOL nores_get_current_mode(x11drv_settings_id id, DEVMODEW *mode) { RECT primary = get_host_primary_monitor_rect(); @@ -116,7 +116,7 @@ static BOOL nores_get_current_mode(ULONG_PTR id, DEVMODEW *mode) mode->dmPosition.x = 0; mode->dmPosition.y = 0; - if (id != 1) + if (id.id != 1) { FIXME("Non-primary adapters are unsupported.\n"); mode->dmBitsPerPel = 0; @@ -133,7 +133,7 @@ static BOOL nores_get_current_mode(ULONG_PTR id, DEVMODEW *mode) return TRUE; } -static LONG nores_set_current_mode(ULONG_PTR id, const DEVMODEW *mode) +static LONG nores_set_current_mode(x11drv_settings_id id, const DEVMODEW *mode) { WARN("NoRes settings handler, ignoring mode change request.\n"); return DISP_CHANGE_SUCCESSFUL; @@ -156,14 +156,14 @@ void X11DRV_Settings_Init(void) X11DRV_Settings_SetHandler(&nores_handler); } -static void set_display_depth(ULONG_PTR display_id, DWORD depth) +static void set_display_depth(x11drv_settings_id display_id, DWORD depth) { struct x11drv_display_depth *display_depth; pthread_mutex_lock( &settings_mutex ); LIST_FOR_EACH_ENTRY(display_depth, &x11drv_display_depth_list, struct x11drv_display_depth, entry) { - if (display_depth->display_id == display_id) + if (display_depth->display_id.id == display_id.id) { display_depth->depth = depth; pthread_mutex_unlock( &settings_mutex ); @@ -185,7 +185,7 @@ static void set_display_depth(ULONG_PTR display_id, DWORD depth) pthread_mutex_unlock( &settings_mutex ); } -static DWORD get_display_depth(ULONG_PTR display_id) +static DWORD get_display_depth(x11drv_settings_id display_id) { struct x11drv_display_depth *display_depth; DWORD depth; @@ -193,7 +193,7 @@ static DWORD get_display_depth(ULONG_PTR display_id) pthread_mutex_lock( &settings_mutex ); LIST_FOR_EACH_ENTRY(display_depth, &x11drv_display_depth_list, struct x11drv_display_depth, entry) { - if (display_depth->display_id == display_id) + if (display_depth->display_id.id == display_id.id) { depth = display_depth->depth; pthread_mutex_unlock( &settings_mutex ); @@ -206,7 +206,7 @@ static DWORD get_display_depth(ULONG_PTR display_id) INT X11DRV_GetDisplayDepth(LPCWSTR name, BOOL is_primary) { - ULONG_PTR id; + x11drv_settings_id id; if (settings_handler.get_id( name, is_primary, &id )) return get_display_depth( id ); @@ -221,7 +221,7 @@ INT X11DRV_GetDisplayDepth(LPCWSTR name, BOOL is_primary) BOOL X11DRV_GetCurrentDisplaySettings( LPCWSTR name, BOOL is_primary, LPDEVMODEW devmode ) { DEVMODEW mode; - ULONG_PTR id; + x11drv_settings_id id; if (!settings_handler.get_id( name, is_primary, &id ) || !settings_handler.get_current_mode( id, &mode )) { @@ -254,7 +254,7 @@ static BOOL is_same_devmode( const DEVMODEW *a, const DEVMODEW *b ) /* Get the full display mode with all the necessary fields set. * Return NULL on failure. Caller should call free_full_mode() to free the returned mode. */ -static DEVMODEW *get_full_mode(ULONG_PTR id, DEVMODEW *dev_mode) +static DEVMODEW *get_full_mode(x11drv_settings_id id, DEVMODEW *dev_mode) { DEVMODEW *modes, *full_mode, *found_mode = NULL; UINT mode_count, mode_idx; @@ -297,7 +297,7 @@ static void free_full_mode(DEVMODEW *mode) free(mode); } -static LONG apply_display_settings( DEVMODEW *displays, ULONG_PTR *ids, BOOL do_attach ) +static LONG apply_display_settings( DEVMODEW *displays, x11drv_settings_id *ids, BOOL do_attach ) { DEVMODEW *full_mode; BOOL attached_mode; @@ -306,7 +306,7 @@ static LONG apply_display_settings( DEVMODEW *displays, ULONG_PTR *ids, BOOL do_ for (count = 0, mode = displays; mode->dmSize; mode = NEXT_DEVMODEW(mode), count++) { - ULONG_PTR *id = ids + count; + x11drv_settings_id *id = ids + count; attached_mode = !is_detached_mode(mode); if ((attached_mode && !do_attach) || (!attached_mode && do_attach)) @@ -343,7 +343,7 @@ LONG X11DRV_ChangeDisplaySettings( LPDEVMODEW displays, LPCWSTR primary_name, HW { INT left_most = INT_MAX, top_most = INT_MAX; LONG count, ret = DISP_CHANGE_BADPARAM; - ULONG_PTR *ids; + x11drv_settings_id *ids; DEVMODEW *mode; /* Convert virtual screen coordinates to root coordinates, and find display ids. @@ -568,7 +568,7 @@ BOOL X11DRV_UpdateDisplayDevices( const struct gdi_device_manager *device_manage DEVMODEW current_mode = {.dmSize = sizeof(current_mode)}; WCHAR devname[32]; char buffer[32]; - ULONG_PTR settings_id; + x11drv_settings_id settings_id; BOOL is_primary = adapters[adapter].state_flags & DISPLAY_DEVICE_PRIMARY_DEVICE; device_manager->add_adapter( &adapters[adapter], param ); diff --git a/dlls/winex11.drv/x11drv.h b/dlls/winex11.drv/x11drv.h index 893099489e3..8da26775a4c 100644 --- a/dlls/winex11.drv/x11drv.h +++ b/dlls/winex11.drv/x11drv.h @@ -709,6 +709,9 @@ extern void init_recursive_mutex( pthread_mutex_t *mutex ) DECLSPEC_HIDDEN; #define DEPTH_COUNT 3 extern const unsigned int *depths DECLSPEC_HIDDEN; +/* Use a distinct type for the settings id, to avoid mixups other types of ids */ +typedef struct { ULONG_PTR id; } x11drv_settings_id; + /* Required functions for changing and enumerating display settings */ struct x11drv_settings_handler { @@ -722,7 +725,7 @@ struct x11drv_settings_handler * Following functions use this id to identify the device. * * Return FALSE if the device cannot be found and TRUE on success */ - BOOL (*get_id)(const WCHAR *device_name, BOOL is_primary, ULONG_PTR *id); + BOOL (*get_id)(const WCHAR *device_name, BOOL is_primary, x11drv_settings_id *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 @@ -733,7 +736,7 @@ struct x11drv_settings_handler * dmDisplayFlags and dmDisplayFrequency * * Return FALSE on failure with parameters unchanged and error code set. Return TRUE on success */ - BOOL (*get_modes)(ULONG_PTR id, DWORD flags, DEVMODEW **modes, UINT *mode_count); + BOOL (*get_modes)(x11drv_settings_id id, DWORD flags, DEVMODEW **modes, UINT *mode_count); /* free_modes() will be called to free the mode list returned from get_modes() */ void (*free_modes)(DEVMODEW *modes); @@ -745,13 +748,13 @@ struct x11drv_settings_handler * dmDisplayFrequency and dmPosition * * Return FALSE on failure with parameters unchanged and error code set. Return TRUE on success */ - BOOL (*get_current_mode)(ULONG_PTR id, DEVMODEW *mode); + BOOL (*get_current_mode)(x11drv_settings_id id, DEVMODEW *mode); /* set_current_mode() will be called to change the display mode of the display device of id. * mode must be a valid mode from get_modes() with optional fields, such as dmPosition set. * * Return DISP_CHANGE_*, same as ChangeDisplaySettingsExW() return values */ - LONG (*set_current_mode)(ULONG_PTR id, const DEVMODEW *mode); + LONG (*set_current_mode)(x11drv_settings_id id, const DEVMODEW *mode); }; extern void X11DRV_Settings_SetHandler(const struct x11drv_settings_handler *handler) DECLSPEC_HIDDEN; diff --git a/dlls/winex11.drv/xrandr.c b/dlls/winex11.drv/xrandr.c index 3731ca6b4d0..f54960f1c9b 100644 --- a/dlls/winex11.drv/xrandr.c +++ b/dlls/winex11.drv/xrandr.c @@ -142,12 +142,12 @@ static int XRandRErrorHandler(Display *dpy, XErrorEvent *event, void *arg) } /* XRandR 1.0 display settings handler */ -static BOOL xrandr10_get_id( const WCHAR *device_name, BOOL is_primary, ULONG_PTR *id ) +static BOOL xrandr10_get_id( const WCHAR *device_name, BOOL is_primary, x11drv_settings_id *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 = is_primary ? 1 : 0; + id->id = is_primary ? 1 : 0; return TRUE; } @@ -171,7 +171,7 @@ static void add_xrandr10_mode( DEVMODEW *mode, DWORD depth, DWORD width, DWORD h memcpy( (BYTE *)mode + sizeof(*mode), &size_id, sizeof(size_id) ); } -static BOOL xrandr10_get_modes( ULONG_PTR id, DWORD flags, DEVMODEW **new_modes, UINT *new_mode_count ) +static BOOL xrandr10_get_modes( x11drv_settings_id id, DWORD flags, DEVMODEW **new_modes, UINT *new_mode_count ) { INT size_idx, depth_idx, rate_idx, mode_idx = 0; INT size_count, rate_count, mode_count = 0; @@ -233,7 +233,7 @@ static void xrandr10_free_modes( DEVMODEW *modes ) free( modes ); } -static BOOL xrandr10_get_current_mode( ULONG_PTR id, DEVMODEW *mode ) +static BOOL xrandr10_get_current_mode( x11drv_settings_id id, DEVMODEW *mode ) { XRRScreenConfiguration *screen_config; XRRScreenSize *sizes; @@ -249,7 +249,7 @@ static BOOL xrandr10_get_current_mode( ULONG_PTR id, DEVMODEW *mode ) mode->u1.s2.dmPosition.x = 0; mode->u1.s2.dmPosition.y = 0; - if (id != 1) + if (id.id != 1) { FIXME("Non-primary adapters are unsupported.\n"); mode->dmBitsPerPel = 0; @@ -275,7 +275,7 @@ static BOOL xrandr10_get_current_mode( ULONG_PTR id, DEVMODEW *mode ) return TRUE; } -static LONG xrandr10_set_current_mode( ULONG_PTR id, const DEVMODEW *mode ) +static LONG xrandr10_set_current_mode( x11drv_settings_id id, const DEVMODEW *mode ) { XRRScreenConfiguration *screen_config; Rotation rotation; @@ -283,7 +283,7 @@ static LONG xrandr10_set_current_mode( ULONG_PTR id, const DEVMODEW *mode ) Window root; Status stat; - if (id != 1) + if (id.id != 1) { FIXME("Non-primary adapters are unsupported.\n"); return DISP_CHANGE_SUCCESSFUL; @@ -1215,7 +1215,7 @@ static void xrandr14_register_event_handlers(void) } /* XRandR 1.4 display settings handler */ -static BOOL xrandr14_get_id( const WCHAR *device_name, BOOL is_primary, ULONG_PTR *id ) +static BOOL xrandr14_get_id( const WCHAR *device_name, BOOL is_primary, x11drv_settings_id *id ) { struct current_mode *tmp_modes, *new_current_modes = NULL; INT gpu_count, adapter_count, new_current_mode_count = 0; @@ -1276,7 +1276,7 @@ static BOOL xrandr14_get_id( const WCHAR *device_name, BOOL is_primary, ULONG_PT return FALSE; } - *id = current_modes[display_idx].id; + id->id = current_modes[display_idx].id; pthread_mutex_unlock( &xrandr_mutex ); return TRUE; } @@ -1309,12 +1309,12 @@ static void add_xrandr14_mode( DEVMODEW *mode, XRRModeInfo *info, DWORD depth, D memcpy( (BYTE *)mode + sizeof(*mode), &info->id, sizeof(info->id) ); } -static BOOL xrandr14_get_modes( ULONG_PTR id, DWORD flags, DEVMODEW **new_modes, UINT *mode_count ) +static BOOL xrandr14_get_modes( x11drv_settings_id id, DWORD flags, DEVMODEW **new_modes, UINT *mode_count ) { DWORD frequency, orientation, orientation_count; XRRScreenResources *screen_resources; XRROutputInfo *output_info = NULL; - RROutput output = (RROutput)id; + RROutput output = (RROutput)id.id; XRRCrtcInfo *crtc_info = NULL; UINT depth_idx, mode_idx = 0; XRRModeInfo *mode_info; @@ -1426,12 +1426,12 @@ static void xrandr14_free_modes( DEVMODEW *modes ) free( modes ); } -static BOOL xrandr14_get_current_mode( ULONG_PTR id, DEVMODEW *mode ) +static BOOL xrandr14_get_current_mode( x11drv_settings_id id, DEVMODEW *mode ) { struct current_mode *mode_ptr = NULL; XRRScreenResources *screen_resources; XRROutputInfo *output_info = NULL; - RROutput output = (RROutput)id; + RROutput output = (RROutput)id.id; XRRModeInfo *mode_info = NULL; XRRCrtcInfo *crtc_info = NULL; BOOL ret = FALSE; @@ -1441,7 +1441,7 @@ static BOOL xrandr14_get_current_mode( ULONG_PTR id, DEVMODEW *mode ) pthread_mutex_lock( &xrandr_mutex ); for (mode_idx = 0; mode_idx < current_mode_count; ++mode_idx) { - if (current_modes[mode_idx].id != id) + if (current_modes[mode_idx].id != id.id) continue; if (!current_modes[mode_idx].loaded) @@ -1532,10 +1532,10 @@ done: return ret; } -static LONG xrandr14_set_current_mode( ULONG_PTR id, const DEVMODEW *mode ) +static LONG xrandr14_set_current_mode( x11drv_settings_id id, const DEVMODEW *mode ) { unsigned int screen_width, screen_height; - RROutput output = (RROutput)id, *outputs; + RROutput output = (RROutput)id.id, *outputs; XRRScreenResources *screen_resources; XRROutputInfo *output_info = NULL; XRRCrtcInfo *crtc_info = NULL; diff --git a/dlls/winex11.drv/xvidmode.c b/dlls/winex11.drv/xvidmode.c index ae8116da7b8..4d1aa3ab8e8 100644 --- a/dlls/winex11.drv/xvidmode.c +++ b/dlls/winex11.drv/xvidmode.c @@ -85,12 +85,12 @@ static int XVidModeErrorHandler(Display *dpy, XErrorEvent *event, void *arg) } /* XF86VidMode display settings handler */ -static BOOL xf86vm_get_id(const WCHAR *device_name, BOOL is_primary, ULONG_PTR *id) +static BOOL xf86vm_get_id(const WCHAR *device_name, BOOL is_primary, x11drv_settings_id *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 = is_primary ? 1 : 0; + id->id = is_primary ? 1 : 0; return TRUE; } @@ -112,7 +112,7 @@ static void add_xf86vm_mode(DEVMODEW *mode, DWORD depth, const XF86VidModeModeIn memcpy((BYTE *)mode + sizeof(*mode), &mode_info, sizeof(mode_info)); } -static BOOL xf86vm_get_modes(ULONG_PTR id, DWORD flags, DEVMODEW **new_modes, UINT *mode_count) +static BOOL xf86vm_get_modes(x11drv_settings_id id, DWORD flags, DEVMODEW **new_modes, UINT *mode_count) { INT xf86vm_mode_idx, xf86vm_mode_count; XF86VidModeModeInfo **xf86vm_modes; @@ -169,7 +169,7 @@ static void xf86vm_free_modes(DEVMODEW *modes) free(modes); } -static BOOL xf86vm_get_current_mode(ULONG_PTR id, DEVMODEW *mode) +static BOOL xf86vm_get_current_mode(x11drv_settings_id id, DEVMODEW *mode) { XF86VidModeModeLine xf86vm_mode; INT dotclock; @@ -182,7 +182,7 @@ static BOOL xf86vm_get_current_mode(ULONG_PTR id, DEVMODEW *mode) mode->u1.s2.dmPosition.x = 0; mode->u1.s2.dmPosition.y = 0; - if (id != 1) + if (id.id != 1) { FIXME("Non-primary adapters are unsupported.\n"); mode->dmBitsPerPel = 0; @@ -210,12 +210,12 @@ static BOOL xf86vm_get_current_mode(ULONG_PTR id, DEVMODEW *mode) return TRUE; } -static LONG xf86vm_set_current_mode(ULONG_PTR id, const DEVMODEW *mode) +static LONG xf86vm_set_current_mode(x11drv_settings_id id, const DEVMODEW *mode) { XF86VidModeModeInfo *xf86vm_mode; Bool ret; - if (id != 1) + if (id.id != 1) { FIXME("Non-primary adapters are unsupported.\n"); return DISP_CHANGE_SUCCESSFUL; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2593
On Thu Apr 6 10:41:28 2023 +0000, Alexandros Frantzis wrote:
Keeping as 'Draft' for now, until we get some more feedback from the bug reporter. The bug reported verified the fix.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2593#note_29140
On Thu Apr 6 03:14:22 2023 +0000, Zhiyi Zhang wrote:
Yes, this is overlooked. When using both the XRandR 1.4 display device handler and settings handler, their IDs are compatible but not in other cases. To avoid future confusion, maybe it's better to introduce types for adapter device ID and adapter setting ID. Or maybe adding comments in struct x11drv_settings_handler and struct x11drv_display_device_handler about not to mix IDs is enough. I like the idea with the separate type(s), so I have added an new commit that implements a distinct type for the settings id, for your consideration. It introduces a bit of churn but it is otherwise straightforward.
I didn't add a new type for the the device id, since that id type is designed to match the `struct gdi_adapter` id field type, so it's not directly under winex11 control. Also having just the distinct settings id type provides the guarantees we want. (This change is not necessary for this MR, so we can include, skip or refine in another MR as preferred.) -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2593#note_29144
On Thu Apr 6 10:40:19 2023 +0000, Alexandros Frantzis wrote:
changed this line in [version 4 of the diff](/wine/wine/-/merge_requests/2593/diffs?diff_id=41028&start_sha=93391931f14a6fd4ffdb8b75337a261bcfaaf56e#281cd1a9b246cf9b867f5ef76600365651a8aacf_592_594) Done.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2593#note_29145
On Thu Apr 6 10:40:18 2023 +0000, Alexandros Frantzis wrote:
changed this line in [version 4 of the diff](/wine/wine/-/merge_requests/2593/diffs?diff_id=41028&start_sha=93391931f14a6fd4ffdb8b75337a261bcfaaf56e#281cd1a9b246cf9b867f5ef76600365651a8aacf_592_594) Saved `is_virtual_desktop()` to a variable and used as @zhiyi suggested.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2593#note_29146
participants (2)
-
Alexandros Frantzis -
Alexandros Frantzis (＠afrantzis)