[PATCH v4 0/6] MR8079: winex11: Serialize fullscreen/maximized and config changes.
-- v4: winex11: Reconfigure window even if the new rectangle equals to the old one. winex11: Serialize window config requests with some other requests. winex11: Serialize managed window config change requests. winex11: Track window pending config position / size independently. winex11: Serialize individual _NET_WM_STATE bit changes. winex11: Continue requesting desired window state on no-op event. https://gitlab.winehq.org/wine/wine/-/merge_requests/8079
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/winex11.drv/window.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/dlls/winex11.drv/window.c b/dlls/winex11.drv/window.c index e154db1e23a..2a7eaee8f00 100644 --- a/dlls/winex11.drv/window.c +++ b/dlls/winex11.drv/window.c @@ -1693,8 +1693,6 @@ static BOOL handle_state_change( unsigned long serial, unsigned long *expect_ser const char *prefix, const char *received, const char *reason ) { if (serial < *expect_serial) reason = "old "; - else if (!*expect_serial && !memcmp( current, value, size )) reason = "no-op "; - if (reason) { WARN( "Ignoring %s%s%s%s\n", prefix, reason, received, expected ); @@ -1702,9 +1700,8 @@ static BOOL handle_state_change( unsigned long serial, unsigned long *expect_ser return FALSE; } - if (!*expect_serial) reason = "unexpected "; - else if (memcmp( pending, value, size )) reason = "mismatch "; - + if (!*expect_serial && memcmp( current, value, size )) reason = "unexpected "; + if (*expect_serial && memcmp( pending, value, size )) reason = "mismatch "; if (!reason) TRACE( "%s%s%s\n", prefix, received, expected ); else { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8079
From: Rémi Bernon <rbernon(a)codeweavers.com> _NET_WM_STATE change requests are going through the window manager and the PropertyNotify events might be received with a serial that doesn't match its request. --- dlls/winex11.drv/window.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/dlls/winex11.drv/window.c b/dlls/winex11.drv/window.c index 2a7eaee8f00..0ee599e4d07 100644 --- a/dlls/winex11.drv/window.c +++ b/dlls/winex11.drv/window.c @@ -1264,6 +1264,7 @@ static void window_set_net_wm_state( struct x11drv_win_data *data, UINT new_stat for (i = 0; i < NB_NET_WM_STATES; i++) { + if (data->net_wm_state_serial) break; /* another _NET_WM_STATE update is pending, wait for it to complete */ if (!((old_state ^ new_state) & (1 << i))) continue; xev.xclient.data.l[0] = (new_state & (1 << i)) ? _NET_WM_STATE_ADD : _NET_WM_STATE_REMOVE; @@ -1271,7 +1272,7 @@ static void window_set_net_wm_state( struct x11drv_win_data *data, UINT new_stat xev.xclient.data.l[2] = ((net_wm_state_atoms[i] == XATOM__NET_WM_STATE_MAXIMIZED_VERT) ? x11drv_atom(_NET_WM_STATE_MAXIMIZED_HORZ) : 0); - data->pending_state.net_wm_state = new_state; + data->pending_state.net_wm_state ^= (1 << i); data->net_wm_state_serial = NextRequest( data->display ); TRACE( "window %p/%lx, requesting _NET_WM_STATE %#x serial %lu\n", data->hwnd, data->whole_window, data->pending_state.net_wm_state, data->net_wm_state_serial ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8079
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/winex11.drv/window.c | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/dlls/winex11.drv/window.c b/dlls/winex11.drv/window.c index 0ee599e4d07..ab7d02bb9fb 100644 --- a/dlls/winex11.drv/window.c +++ b/dlls/winex11.drv/window.c @@ -1284,12 +1284,13 @@ static void window_set_net_wm_state( struct x11drv_win_data *data, UINT new_stat XFlush( data->display ); } -static void window_set_config( struct x11drv_win_data *data, const RECT *new_rect, BOOL above ) +static void window_set_config( struct x11drv_win_data *data, RECT rect, BOOL above ) { static const UINT fullscreen_mask = (1 << NET_WM_STATE_MAXIMIZED) | (1 << NET_WM_STATE_FULLSCREEN); UINT style = NtUserGetWindowLongW( data->hwnd, GWL_STYLE ), mask = 0; const RECT *old_rect = &data->pending_state.rect; XWindowChanges changes; + RECT *new_rect = ▭ data->desired_state.rect = *new_rect; if (!data->whole_window) return; /* no window, nothing to update */ @@ -1320,6 +1321,11 @@ static void window_set_config( struct x11drv_win_data *data, const RECT *new_rec if (changes.height > 65535) changes.height = 65535; mask |= CWWidth | CWHeight; } + else + { + new_rect->right = new_rect->left + old_rect->right - old_rect->left; + new_rect->bottom = new_rect->top + old_rect->bottom - old_rect->top; + } /* only the size is allowed to change for the desktop window or systray docked windows */ if ((old_rect->left != new_rect->left || old_rect->top != new_rect->top) && @@ -1330,6 +1336,10 @@ static void window_set_config( struct x11drv_win_data *data, const RECT *new_rec changes.y = pt.y; mask |= CWX | CWY; } + else + { + OffsetRect( new_rect, old_rect->left - new_rect->left, old_rect->top - new_rect->top ); + } if (above) { @@ -1737,7 +1747,7 @@ void window_wm_state_notify( struct x11drv_win_data *data, unsigned long serial, /* send any pending changes from the desired state */ 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_config( data, &data->desired_state.rect, FALSE ); + window_set_config( data, data->desired_state.rect, FALSE ); if (data->current_state.wm_state == NormalState) NtUserSetProp( data->hwnd, focus_time_prop, (HANDLE)time ); else if (!data->wm_state_serial) NtUserRemoveProp( data->hwnd, focus_time_prop ); @@ -1760,7 +1770,7 @@ void window_net_wm_state_notify( struct x11drv_win_data *data, unsigned long ser /* send any pending changes from the desired state */ 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_config( data, &data->desired_state.rect, FALSE ); + window_set_config( data, data->desired_state.rect, FALSE ); } void window_mwm_hints_notify( struct x11drv_win_data *data, unsigned long serial, const MwmHints *value ) @@ -1951,7 +1961,7 @@ static void sync_window_position( struct x11drv_win_data *data, UINT swp_flags, if (data->is_offscreen) OffsetRect( &new_rect, window_rect.left - old_rects->window.left, window_rect.top - old_rects->window.top ); - window_set_config( data, &new_rect, above ); + window_set_config( data, new_rect, above ); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8079
From: Rémi Bernon <rbernon(a)codeweavers.com> When using a window manager, config requests are going through the WM and their ConfigureNotify might be received with a different serial from the request that triggered them. We still want to override past and transient configs with our desired state as much as possible, and we ignore the received values if some changes were delayed, and request them instead. --- dlls/winex11.drv/window.c | 31 ++++++++++++++++++++++++------- 1 file changed, 24 insertions(+), 7 deletions(-) diff --git a/dlls/winex11.drv/window.c b/dlls/winex11.drv/window.c index ab7d02bb9fb..f2bb01ea1b4 100644 --- a/dlls/winex11.drv/window.c +++ b/dlls/winex11.drv/window.c @@ -906,6 +906,13 @@ static void set_size_hints( struct x11drv_win_data *data, DWORD style ) XFree( size_hints ); } +static BOOL window_needs_config_change_delay( struct x11drv_win_data *data ) +{ + static const UINT fullscreen_mask = (1 << NET_WM_STATE_MAXIMIZED) | (1 << NET_WM_STATE_FULLSCREEN); + if (data->pending_state.wm_state != NormalState) return FALSE; + if (data->configure_serial) return TRUE; /* another config update is pending, wait for it to complete */ + return data->net_wm_state_serial && !(data->pending_state.net_wm_state & fullscreen_mask) && (data->current_state.net_wm_state & fullscreen_mask); +} static void window_set_mwm_hints( struct x11drv_win_data *data, const MwmHints *new_hints ) { @@ -1286,7 +1293,6 @@ static void window_set_net_wm_state( struct x11drv_win_data *data, UINT new_stat static void window_set_config( struct x11drv_win_data *data, RECT rect, BOOL above ) { - static const UINT fullscreen_mask = (1 << NET_WM_STATE_MAXIMIZED) | (1 << NET_WM_STATE_FULLSCREEN); UINT style = NtUserGetWindowLongW( data->hwnd, GWL_STYLE ), mask = 0; const RECT *old_rect = &data->pending_state.rect; XWindowChanges changes; @@ -1295,10 +1301,7 @@ static void window_set_config( struct x11drv_win_data *data, RECT rect, BOOL abo data->desired_state.rect = *new_rect; if (!data->whole_window) return; /* no window, nothing to update */ if (EqualRect( old_rect, new_rect ) && !above) return; /* rects are the same, no need to be raised, nothing to update */ - - if (data->pending_state.wm_state == NormalState && data->net_wm_state_serial && - !(data->pending_state.net_wm_state & fullscreen_mask) && - (data->current_state.net_wm_state & fullscreen_mask)) + if (data->managed && window_needs_config_change_delay( data )) { /* Some window managers are sending a ConfigureNotify event with the fullscreen size when * exiting a fullscreen window, with a serial that we cannot predict. Handling that event @@ -1797,8 +1800,22 @@ void window_configure_notify( struct x11drv_win_data *data, unsigned long serial received = wine_dbg_sprintf( "config %s/%lu", wine_dbgstr_rect(value), serial ); expected = *expect_serial ? wine_dbg_sprintf( ", expected %s/%lu", wine_dbgstr_rect(pending), *expect_serial ) : ""; - handle_state_change( serial, expect_serial, sizeof(*value), value, desired, pending, - current, expected, prefix, received, NULL ); + /* if we've delayed some config we want to continue with it, make sure handle_state_change doesn't overwrite it */ + if ((*expect_serial || window_needs_config_change_delay( data )) && + serial >= *expect_serial && !EqualRect( desired, pending )) + { + WARN( "%spreserving delayed config %s\n", prefix, wine_dbgstr_rect(desired) ); + desired = pending; + } + + if (!handle_state_change( serial, expect_serial, sizeof(*value), value, desired, pending, + current, expected, prefix, received, NULL )) + return; + + /* send any pending changes from the desired state */ + 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_config( data, data->desired_state.rect, FALSE ); } void net_active_window_notify( unsigned long serial, Window value, Time time ) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8079
From: Rémi Bernon <rbernon(a)codeweavers.com> When they will likely induce some ConfigureNotify events, we need to wait for these events to arrive before requesting our changes, as we are otherwise unable to tell which ConfigureNotify event is an old one and which one corresponds to our requests. --- dlls/winex11.drv/window.c | 67 ++++++++++++++++++++++++++++++++------- 1 file changed, 56 insertions(+), 11 deletions(-) diff --git a/dlls/winex11.drv/window.c b/dlls/winex11.drv/window.c index f2bb01ea1b4..690a98a594d 100644 --- a/dlls/winex11.drv/window.c +++ b/dlls/winex11.drv/window.c @@ -906,12 +906,37 @@ static void set_size_hints( struct x11drv_win_data *data, DWORD style ) XFree( size_hints ); } +/* bits that can trigger spurious ConfigureNotify events */ +static const UINT config_notify_mask = (1 << NET_WM_STATE_MAXIMIZED) | (1 << NET_WM_STATE_FULLSCREEN) | + (1 << NET_WM_STATE_ABOVE); + +static BOOL window_needs_mwm_hints_change_delay( struct x11drv_win_data *data ) +{ + if (data->pending_state.wm_state == WithdrawnState) return FALSE; /* window is unmapped, should be safe to make any change */ + if (!data->configure_serial && !data->net_wm_state_serial) return FALSE; /* no other requests are pending, should be safe */ + /* check whether we have a pending configure, either directly or because of a _NET_WM_STATE change which might trigger one */ + if (!data->configure_serial && !((data->pending_state.net_wm_state ^ data->current_state.net_wm_state) & config_notify_mask)) return FALSE; + /* delay any new _MOTIF_WM_HINTS change which might trigger a ConfigureNotify when a config/_NET_WM_STATE change is pending */ + return (!data->desired_state.mwm_hints.decorations != !data->pending_state.mwm_hints.decorations); +} + +static BOOL window_needs_net_wm_state_change_delay( struct x11drv_win_data *data ) +{ + if (data->pending_state.wm_state == WithdrawnState) return FALSE; /* window is unmapped, should be safe to make any change */ + if (!data->configure_serial && !data->mwm_hints_serial) return FALSE; /* no other requests are pending, should be safe */ + /* check whether we have a pending configure, either directly or because _MOTIF_WM_HINTS decoration changed */ + if (!data->configure_serial && !(!data->pending_state.mwm_hints.decorations != !data->current_state.mwm_hints.decorations)) return FALSE; + /* delay any new _NET_WM_STATE change which might trigger a ConfigureNotify when a config/_MOTIF_WM_HINTS change is pending */ + return (data->desired_state.net_wm_state ^ data->pending_state.net_wm_state) & config_notify_mask; +} + static BOOL window_needs_config_change_delay( struct x11drv_win_data *data ) { - static const UINT fullscreen_mask = (1 << NET_WM_STATE_MAXIMIZED) | (1 << NET_WM_STATE_FULLSCREEN); - if (data->pending_state.wm_state != NormalState) return FALSE; + if (data->pending_state.wm_state == WithdrawnState) return FALSE; /* window is unmapped, should be safe to make any change */ if (data->configure_serial) return TRUE; /* another config update is pending, wait for it to complete */ - return data->net_wm_state_serial && !(data->pending_state.net_wm_state & fullscreen_mask) && (data->current_state.net_wm_state & fullscreen_mask); + /* delay any config request when a _NET_WM_STATE or _MOTIF_WM_HINTS change which might trigger a ConfigureNotify is in flight */ + return (data->net_wm_state_serial && (data->pending_state.net_wm_state ^ data->current_state.net_wm_state) & config_notify_mask) || + (data->mwm_hints_serial && (!data->pending_state.mwm_hints.decorations != !data->current_state.mwm_hints.decorations)); } static void window_set_mwm_hints( struct x11drv_win_data *data, const MwmHints *new_hints ) @@ -922,6 +947,14 @@ static void window_set_mwm_hints( struct x11drv_win_data *data, const MwmHints * if (!data->whole_window || !data->managed) 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 */ + if (window_needs_mwm_hints_change_delay( data )) + { + TRACE( "window %p/%lx is updating _NET_WM_STATE/config, delaying request\n", data->hwnd, data->whole_window ); + return; + } + + if (data->pending_state.wm_state == IconicState) return; /* window is iconic and may be mapped or not, don't update its state now */ + data->pending_state.mwm_hints = *new_hints; data->mwm_hints_serial = NextRequest( data->display ); TRACE( "window %p/%lx, requesting _MOTIF_WM_HINTS %s serial %lu\n", data->hwnd, data->whole_window, @@ -1235,7 +1268,13 @@ static void window_set_net_wm_state( struct x11drv_win_data *data, UINT new_stat /* we ignore and override previous _NET_WM_STATE update requests */ if (old_state == new_state) return; /* states are the same, nothing to update */ - if (data->pending_state.wm_state == IconicState) return; /* window is iconic, don't update its state now */ + if (window_needs_net_wm_state_change_delay( data )) + { + TRACE( "window %p/%lx is updating config/_MOTIF_WM_HINTS, delaying request\n", data->hwnd, data->whole_window ); + return; + } + + if (data->pending_state.wm_state == IconicState) return; /* window is iconic and may be mapped or not, don't update its state now */ if (data->pending_state.wm_state == WithdrawnState) /* set the _NET_WM_STATE atom directly */ { Atom atoms[NB_NET_WM_STATES + 1]; @@ -1303,11 +1342,7 @@ static void window_set_config( struct x11drv_win_data *data, RECT rect, BOOL abo if (EqualRect( old_rect, new_rect ) && !above) return; /* rects are the same, no need to be raised, nothing to update */ if (data->managed && window_needs_config_change_delay( data )) { - /* Some window managers are sending a ConfigureNotify event with the fullscreen size when - * exiting a fullscreen window, with a serial that we cannot predict. Handling that event - * will override the Win32 window size and make the window fullscreen again. - */ - WARN( "window %p/%lx is exiting maximize/fullscreen, delaying request\n", data->hwnd, data->whole_window ); + TRACE( "window %p/%lx is updating _NET_WM_STATE/_MOTIF_WM_HINTS, delaying request\n", data->hwnd, data->whole_window ); return; } @@ -1750,6 +1785,7 @@ void window_wm_state_notify( struct x11drv_win_data *data, unsigned long serial, /* send any pending changes from the desired state */ 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_mwm_hints( data, &data->desired_state.mwm_hints ); window_set_config( data, data->desired_state.rect, FALSE ); if (data->current_state.wm_state == NormalState) NtUserSetProp( data->hwnd, focus_time_prop, (HANDLE)time ); @@ -1773,6 +1809,7 @@ void window_net_wm_state_notify( struct x11drv_win_data *data, unsigned long ser /* send any pending changes from the desired state */ 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_mwm_hints( data, &data->desired_state.mwm_hints ); window_set_config( data, data->desired_state.rect, FALSE ); } @@ -1786,8 +1823,15 @@ void window_mwm_hints_notify( struct x11drv_win_data *data, unsigned long serial received = wine_dbg_sprintf( "_MOTIF_WM_HINTS %s/%lu", debugstr_mwm_hints(value), serial ); expected = *expect_serial ? wine_dbg_sprintf( ", expected %s/%lu", debugstr_mwm_hints(pending), *expect_serial ) : ""; - handle_state_change( serial, expect_serial, sizeof(*value), value, desired, pending, - current, expected, prefix, received, NULL ); + if (!handle_state_change( serial, expect_serial, sizeof(*value), value, desired, pending, + current, expected, prefix, received, NULL )) + return; + + /* send any pending changes from the desired state */ + 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_mwm_hints( data, &data->desired_state.mwm_hints ); + window_set_config( data, data->desired_state.rect, FALSE ); } void window_configure_notify( struct x11drv_win_data *data, unsigned long serial, const RECT *value ) @@ -1815,6 +1859,7 @@ void window_configure_notify( struct x11drv_win_data *data, unsigned long serial /* send any pending changes from the desired state */ 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_mwm_hints( data, &data->desired_state.mwm_hints ); window_set_config( data, data->desired_state.rect, FALSE ); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8079
From: Zhiyi Zhang <zzhang(a)codeweavers.com> CW* masks in window_set_config() are from comparing against the old rect from data->pending_state.rect, not the actual X11 window rectangle. So it's still possible that after removing __NET_WM_STATE_FULLSCREEN, WM moves the window behind Wine's back. Then window_set_config() calculates CW* masks and calls XReconfigureWMWindow(), assuming the old rect is still the actual X11 window rect. As the result, some rectangle updates might be missed. --- dlls/winex11.drv/window.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/dlls/winex11.drv/window.c b/dlls/winex11.drv/window.c index 690a98a594d..a556c76eae5 100644 --- a/dlls/winex11.drv/window.c +++ b/dlls/winex11.drv/window.c @@ -1339,17 +1339,18 @@ static void window_set_config( struct x11drv_win_data *data, RECT rect, BOOL abo data->desired_state.rect = *new_rect; if (!data->whole_window) return; /* no window, nothing to update */ - if (EqualRect( old_rect, new_rect ) && !above) return; /* rects are the same, no need to be raised, nothing to update */ if (data->managed && window_needs_config_change_delay( data )) { TRACE( "window %p/%lx is updating _NET_WM_STATE/_MOTIF_WM_HINTS, delaying request\n", data->hwnd, data->whole_window ); return; } + /* Request an update even if the new_rect equals to the last requested rect because the X11 + * window geometry may have been changed by the WM. For example, removing _NET_WM_STATE_FULLSCREEN + * causes WM to restore geometry for the window */ + /* resizing a managed maximized window is not allowed */ - if ((old_rect->right - old_rect->left != new_rect->right - new_rect->left || - old_rect->bottom - old_rect->top != new_rect->bottom - new_rect->top) && - (!(style & WS_MAXIMIZE) || !data->managed)) + if (!(style & WS_MAXIMIZE) || !data->managed) { changes.width = new_rect->right - new_rect->left; changes.height = new_rect->bottom - new_rect->top; @@ -1366,8 +1367,7 @@ static void window_set_config( struct x11drv_win_data *data, RECT rect, BOOL abo } /* only the size is allowed to change for the desktop window or systray docked windows */ - if ((old_rect->left != new_rect->left || old_rect->top != new_rect->top) && - (data->whole_window != root_window && !data->embedded)) + if (data->whole_window != root_window && !data->embedded) { POINT pt = virtual_screen_to_root( new_rect->left, new_rect->top ); changes.x = pt.x; @@ -1385,6 +1385,8 @@ static void window_set_config( struct x11drv_win_data *data, RECT rect, BOOL abo mask |= CWStackMode; } + if (mask == 0) return; /* nothing to update */ + data->pending_state.rect = *new_rect; data->configure_serial = NextRequest( data->display ); TRACE( "window %p/%lx, requesting config %s mask %#x above %u, serial %lu\n", data->hwnd, data->whole_window, -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8079
v3: Continue requesting delayed configs instead of discarding the desired state as soon as an event mismatches, serialize requests with _MOTIF_WM_HINTS when decorations are switched on/off as this often triggers ConfigureNotify events too. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8079#note_104351
participants (2)
-
Rémi Bernon -
Zhiyi Zhang