-- v5: winex11: Serialize window config requests with some other requests. winex11: Serialize managed window config change requests. winex11: Keep track of the last config above flag used. 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.
From: Rémi Bernon rbernon@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 {
From: Rémi Bernon rbernon@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 );
From: Rémi Bernon rbernon@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 ); }
From: Rémi Bernon rbernon@codeweavers.com
Some window manager don't send any ConfigureNotify if nothing changed, we need to be careful and avoid sending unnecessary requests or we will wait for them forever as we're going to serialize the requests. --- dlls/winex11.drv/window.c | 11 ++++++++--- dlls/winex11.drv/x11drv.h | 1 + 2 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/dlls/winex11.drv/window.c b/dlls/winex11.drv/window.c index ab7d02bb9fb..9495c808577 100644 --- a/dlls/winex11.drv/window.c +++ b/dlls/winex11.drv/window.c @@ -1289,12 +1289,14 @@ static void window_set_config( struct x11drv_win_data *data, RECT rect, BOOL abo 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; + BOOL old_above = data->pending_state.above; XWindowChanges changes; RECT *new_rect = ▭
data->desired_state.rect = *new_rect; + data->desired_state.above = above; 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 (EqualRect( old_rect, new_rect ) && old_above >= 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) && @@ -1348,6 +1350,7 @@ static void window_set_config( struct x11drv_win_data *data, RECT rect, BOOL abo }
data->pending_state.rect = *new_rect; + data->pending_state.above = above; 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, wine_dbgstr_rect(new_rect), mask, above, data->configure_serial ); @@ -1797,8 +1800,10 @@ 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 (!handle_state_change( serial, expect_serial, sizeof(*value), value, desired, pending, + current, expected, prefix, received, NULL )) + return; + data->pending_state.above = FALSE; /* allow requesting it again */ }
void net_active_window_notify( unsigned long serial, Window value, Time time ) diff --git a/dlls/winex11.drv/x11drv.h b/dlls/winex11.drv/x11drv.h index 1c7282d443d..41cac82c865 100644 --- a/dlls/winex11.drv/x11drv.h +++ b/dlls/winex11.drv/x11drv.h @@ -627,6 +627,7 @@ struct window_state MwmHints mwm_hints; long monitors[4]; RECT rect; + BOOL above; };
/* x11drv private window data */
From: Rémi Bernon rbernon@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 | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-)
diff --git a/dlls/winex11.drv/window.c b/dlls/winex11.drv/window.c index 9495c808577..2d9caf8b730 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; BOOL old_above = data->pending_state.above; @@ -1297,10 +1303,7 @@ static void window_set_config( struct x11drv_win_data *data, RECT rect, BOOL abo data->desired_state.above = above; if (!data->whole_window) return; /* no window, nothing to update */ if (EqualRect( old_rect, new_rect ) && old_above >= 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 @@ -1800,10 +1803,23 @@ 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 ) : "";
+ /* 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; data->pending_state.above = FALSE; /* allow requesting it again */ + + /* 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 )
From: Rémi Bernon rbernon@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 2d9caf8b730..07d43a62d05 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]; @@ -1305,11 +1344,7 @@ static void window_set_config( struct x11drv_win_data *data, RECT rect, BOOL abo if (EqualRect( old_rect, new_rect ) && old_above >= 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; }
@@ -1753,6 +1788,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 ); @@ -1776,6 +1812,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 ); }
@@ -1789,8 +1826,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 ) @@ -1819,6 +1863,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 ); }
A lot of tests are failing now, and I suspect that the !8128 change is to blame, either because of what I had described in that MR, with missing events from duplicate requests, or just because we start sending a lot more window reconfiguration requests and that confuses the window manager.
v4: Drop the change for now, track the above flag instead to avoid sending even more duplicate config requests.
Zhiyi Zhang (@zhiyi) commented about dlls/winex11.drv/window.c:
Using >= for BOOL types is weird. Let's not do that.
-
Rémi Bernon -
Rémi Bernon (＠rbernon)
-
Zhiyi Zhang (＠zhiyi)