[PATCH 0/1] MR11556: win32u: Lock host window state updates when applying new state.
Since https://gitlab.winehq.org/wine/wine/-/merge_requests/10699 and the revert of f82b115dfcf3eefbbb533d31976d178e21bf1237 we now call again into the window message proc before the restored state is applied to the win32 side. Any call to SetWindowPos, and more precisely any call that ends up in apply_window_pos and drivers WindowPosChanged, will request the win32 state again to the host, racing with the restored state being applied. In most cases this will resolve on its own when SC_RESTORE overrides the requests with the restored state again, but it emits a spurious minize / restore state requests that can confuse some picky window managers. If we're unlucky enough we might even process the transient state before SC_RESTORE. --- Fwiw this isn't actually specific to winex11, but it's the only driver using GetWindowStateUpdates at the moment. It's mostly a temporary fix to some immediate issue rather than a well thought out solution. It helps getting focus working with gamescope. I'm looking into ways to unify this a bit more with other drivers, but I'm not yet sure how to do that. Other drivers manage their host window state on a separate thread and perhaps it'd be better to move winex11 to the same model, though it is a lot of change. More generally I think it'd be nice to further separate win32 / host window states, with state tracking and transactional(-ish) updates, a bit like what we now have in winex11 but generalized for all drivers. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/11556
From: Rémi Bernon <rbernon@codeweavers.com> Since https://gitlab.winehq.org/wine/wine/-/merge_requests/10699 and the revert of f82b115dfcf3eefbbb533d31976d178e21bf1237 we now call again into the window message proc before the restored state is applied to the win32 side. Any call to SetWindowPos, and more precisely any call that ends up in apply_window_pos and drivers WindowPosChanged, will request the win32 state again to the host, racing with the restored state being applied. In most cases this will resolve on its own when SC_RESTORE overrides the requests with the restored state again, but it emits a spurious minize / restore state requests that can confuse some picky window managers. If we're unlucky enough we might even process the transient state before SC_RESTORE. --- dlls/win32u/message.c | 4 +++- dlls/winex11.drv/window.c | 35 ++++++++++++++++++++++++++--------- dlls/winex11.drv/x11drv.h | 1 + 3 files changed, 30 insertions(+), 10 deletions(-) diff --git a/dlls/win32u/message.c b/dlls/win32u/message.c index d432eb3c1af..8c9a2aaef7c 100644 --- a/dlls/win32u/message.c +++ b/dlls/win32u/message.c @@ -2235,7 +2235,7 @@ static LRESULT handle_internal_message( HWND hwnd, UINT msg, WPARAM wparam, LPAR RECT window_rect; HWND foreground; - if (!user_driver->pGetWindowStateUpdates( hwnd, &state_cmd, &swp_flags, &window_rect, &foreground )) return 0; + if (!user_driver->pGetWindowStateUpdates( hwnd, &state_cmd, &swp_flags, &window_rect, &foreground )) goto unlock; window_rect = map_rect_raw_to_virt( window_rect, get_thread_dpi() ); if (foreground) set_foreground_window( foreground, FALSE, TRUE ); @@ -2258,6 +2258,8 @@ static LRESULT handle_internal_message( HWND hwnd, UINT msg, WPARAM wparam, LPAR break; } + unlock: + user_driver->pGetWindowStateUpdates( hwnd, NULL, NULL, NULL, NULL ); /* unlock the host state */ return 0; } case WM_WINE_UPDATEWINDOWSTATE: diff --git a/dlls/winex11.drv/window.c b/dlls/winex11.drv/window.c index 297695749a1..cbc6f0c5b4d 100644 --- a/dlls/winex11.drv/window.c +++ b/dlls/winex11.drv/window.c @@ -991,6 +991,7 @@ static void window_set_mwm_hints( struct x11drv_win_data *data, const MwmHints * const MwmHints *old_hints = &data->pending_state.mwm_hints; data->desired_state.mwm_hints = *new_hints; + if (data->state_locks) return; /* win32 state is being updated, delay the change */ if (!data->whole_window || !data->managed || data->embedded) return; /* no window or not managed, nothing to update */ if (!memcmp( old_hints, new_hints, sizeof(*new_hints) )) return; /* hints are the same, nothing to update */ @@ -1277,6 +1278,7 @@ static void window_set_net_wm_fullscreen_monitors( struct x11drv_win_data *data, data->desired_state.monitors = *new_monitors; if (!(data->pending_state.net_wm_state & (1 << NET_WM_STATE_FULLSCREEN)) || is_virtual_desktop()) return; /* window isn't fullscreen, delay updating */ + if (data->state_locks) return; /* win32 state is being updated, delay the change */ if (!data->whole_window || !data->managed || data->embedded) return; /* no window or not managed, nothing to update */ if (!memcmp( old_monitors, new_monitors, sizeof(*new_monitors) )) return; /* states are the same, nothing to update */ @@ -1334,6 +1336,7 @@ static void window_set_net_wm_state( struct x11drv_win_data *data, UINT new_stat new_state &= x11drv_init_thread_data()->net_wm_state_mask; data->desired_state.net_wm_state = new_state; + if (data->state_locks) return; /* win32 state is being updated, delay the change */ if (!data->whole_window || !data->managed || data->embedded) return; /* no window or not managed, nothing to update */ if (data->wm_state_serial) return; /* another WM_STATE update is pending, wait for it to complete */ /* we ignore and override previous _NET_WM_STATE update requests */ @@ -1421,6 +1424,7 @@ static void window_set_config( struct x11drv_win_data *data, RECT rect, BOOL abo data->desired_state.rect = *new_rect; data->desired_state.above = above; + if (data->state_locks) return; /* win32 state is being updated, delay the change */ if (!data->whole_window) return; /* no window, nothing to update */ if (EqualRect( old_rect, new_rect ) && (old_above || !above || data->managed)) return; /* rects are the same, no need to be raised, nothing to update */ if (window_needs_config_change_delay( data )) @@ -1575,6 +1579,7 @@ static void window_set_wm_state( struct x11drv_win_data *data, UINT new_state, B data->desired_state.wm_state = new_state; data->desired_state.activate = activate; + if (data->state_locks) return; /* win32 state is being updated, delay the change */ if (!data->whole_window) return; /* no window, nothing to update */ if (data->wm_state_serial && !data->current_state.wm_state != !data->pending_state.wm_state) return; /* another map/unmap WM_STATE update is pending, wait for it to complete */ @@ -1787,6 +1792,15 @@ static UINT window_update_client_config( struct x11drv_win_data *data ) return flags; } +static void window_request_desired_state( struct x11drv_win_data *data ) +{ + window_set_wm_state( data, data->desired_state.wm_state, data->desired_state.activate ); + window_set_net_wm_state( data, data->desired_state.net_wm_state ); + window_set_net_wm_fullscreen_monitors( data, &data->desired_state.monitors ); + window_set_mwm_hints( data, &data->desired_state.mwm_hints ); + window_set_config( data, data->desired_state.rect, FALSE ); +} + /*********************************************************************** * GetWindowStateUpdates (X11DRV.@) */ @@ -1796,6 +1810,17 @@ BOOL X11DRV_GetWindowStateUpdates( HWND hwnd, UINT *state_cmd, UINT *swp_flags, struct x11drv_win_data *data; HWND old_foreground; + if (!state_cmd) + { + if ((data = get_win_data( hwnd ))) + { + if (!--data->state_locks) TRACE( "Unlocked window %p/%lx state\n", data->hwnd, data->whole_window ); + window_request_desired_state( data ); + release_win_data( data ); + } + return FALSE; + } + *state_cmd = *swp_flags = 0; *foreground = 0; @@ -1810,6 +1835,7 @@ BOOL X11DRV_GetWindowStateUpdates( HWND hwnd, UINT *state_cmd, UINT *swp_flags, if ((data = get_win_data( hwnd ))) { + if (!data->state_locks++) TRACE( "Locked window %p/%lx state\n", data->hwnd, data->whole_window ); *state_cmd = window_update_client_state( data ); *swp_flags = window_update_client_config( data ); *rect = window_rect_from_visible( &data->rects, data->current_state.rect ); @@ -1850,15 +1876,6 @@ static BOOL handle_state_change( unsigned long serial, unsigned long *expect_ser return TRUE; } -static void window_request_desired_state( struct x11drv_win_data *data ) -{ - window_set_wm_state( data, data->desired_state.wm_state, data->desired_state.activate ); - window_set_net_wm_state( data, data->desired_state.net_wm_state ); - window_set_net_wm_fullscreen_monitors( data, &data->desired_state.monitors ); - window_set_mwm_hints( data, &data->desired_state.mwm_hints ); - window_set_config( data, data->desired_state.rect, FALSE ); -} - void window_wm_state_notify( struct x11drv_win_data *data, unsigned long serial, UINT value, Time time ) { UINT *desired = &data->desired_state.wm_state, *pending = &data->pending_state.wm_state, *current = &data->current_state.wm_state; diff --git a/dlls/winex11.drv/x11drv.h b/dlls/winex11.drv/x11drv.h index 4325c99e5c5..9ed169996ce 100644 --- a/dlls/winex11.drv/x11drv.h +++ b/dlls/winex11.drv/x11drv.h @@ -709,6 +709,7 @@ struct x11drv_win_data unsigned long wm_normal_hints_serial;/* serial of last pending WM_NORMAL_HINTS request */ unsigned long configure_serial; /* serial of last pending configure request */ unsigned long net_wm_icon_serial; /* serial of last pending _NET_WM_ICON request */ + unsigned long state_locks; /* X11 state requests lock while updating win32 state */ }; extern struct x11drv_win_data *get_win_data( HWND hwnd ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/11556
Just FYI I intended to use `GetWindowStateUpdates` to implement focus changes in winewayland (primarily since I needed to implement window restore as well and that requires setting the active window before SC_RESTORE, among who knows what other future changes). That seemed to work pretty well and also removes the need for the winewayland specific driver message. This change is making me question if that would be an appropriate usage of `GetWindowStateUpdates` or if I should just keep the driver-specific message ? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/11556#note_147716
This merge request was approved by Zhiyi Zhang. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/11556
On Tue Aug 4 08:36:37 2026 +0000, Etaash Mathamsetty wrote:
Just FYI I intended to use `GetWindowStateUpdates` to implement focus changes in winewayland (primarily since I needed to implement window restore as well and that requires setting the active window before SC_RESTORE, among who knows what other future changes). That seemed to work pretty well and also removes the need for the winewayland specific driver message. This change is making me question if that would be an appropriate usage of `GetWindowStateUpdates` or if I should just keep the driver-specific message ? It's probably good to use it, as it brings us a bit closer to the goal of having something unified. The state locking in winewayland is more or less implemented through the `processing` config.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/11556#note_147810
participants (4)
-
Etaash Mathamsetty (＠etaash.mathamsetty) -
Rémi Bernon
-
Rémi Bernon (＠rbernon)
-
Zhiyi Zhang (＠zhiyi)