[PATCH v3 0/6] MR8079: winex11: Serialize fullscreen/maximized and config changes.
-- v3: winex11: Serialize managed window config change requests. winex11: Track pending_state.rect position / size independently. winex11: Serialize fullscreen/maximized and config changes. winex11: Serialize individual _NET_WM_STATE bit changes. winex11: Move managed window check to window_set_net_wm_state. winex11: Trace more window change request serials. https://gitlab.winehq.org/wine/wine/-/merge_requests/8079
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/winex11.drv/window.c | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/dlls/winex11.drv/window.c b/dlls/winex11.drv/window.c index 7fd1151bdff..c6ef4ab7d9d 100644 --- a/dlls/winex11.drv/window.c +++ b/dlls/winex11.drv/window.c @@ -506,6 +506,8 @@ static void sync_window_style( struct x11drv_win_data *data ) XSetWindowAttributes attr; int mask = get_window_attributes( data, &attr ); + TRACE( "window %p/%lx changing attributes mask %#x, serial %lu\n", data->hwnd, + data->whole_window, mask, NextRequest( data->display ) ); XChangeWindowAttributes( data->display, data->whole_window, mask, &attr ); x11drv_xinput2_enable( data->display, data->whole_window ); } @@ -893,6 +895,9 @@ static void set_size_hints( struct x11drv_win_data *data, DWORD style ) size_hints->flags |= PMinSize | PMaxSize; } } + + TRACE( "window %p/%lx requesting WM_NORMAL_HINTS flags %#lx, serial %lu\n", data->hwnd, + data->whole_window, size_hints->flags, NextRequest( data->display ) ); XSetWMNormalHints( data->display, data->whole_window, size_hints ); XFree( size_hints ); } @@ -935,8 +940,8 @@ static void set_mwm_hints( struct x11drv_win_data *data, UINT style, UINT ex_sty } } - TRACE( "%p setting mwm hints to %lx,%lx (style %x exstyle %x)\n", - data->hwnd, mwm_hints.decorations, mwm_hints.functions, style, ex_style ); + TRACE( "window %p/%lx requesting _MOTIF_WM_HINTS %#lx,%#lx (style %#x ex_style %#x) serial %lu\n", data->hwnd, + data->whole_window, mwm_hints.decorations, mwm_hints.functions, style, ex_style, NextRequest( data->display ) ); mwm_hints.flags = MWM_HINTS_FUNCTIONS | MWM_HINTS_DECORATIONS; mwm_hints.input_mode = 0; @@ -979,6 +984,8 @@ static void set_style_hints( struct x11drv_win_data *data, DWORD style, DWORD ex else window_type = x11drv_atom(_NET_WM_WINDOW_TYPE_NORMAL); + TRACE( "window %p/%lx requesting _NET_WM_WINDOW_TYPE %#lx, serial %lu\n", data->hwnd, + data->whole_window, window_type, NextRequest( data->display ) ); XChangeProperty(data->display, data->whole_window, x11drv_atom(_NET_WM_WINDOW_TYPE), XA_ATOM, 32, PropModeReplace, (unsigned char*)&window_type, 1); @@ -994,16 +1001,27 @@ static void set_style_hints( struct x11drv_win_data *data, DWORD style, DWORD ex wm_hints->icon_mask = data->icon_mask; wm_hints->flags |= IconPixmapHint | IconMaskHint; } + + TRACE( "window %p/%lx requesting WM_HINTS flags %#lx, serial %lu\n", data->hwnd, + data->whole_window, wm_hints->flags, NextRequest( data->display ) ); XSetWMHints( data->display, data->whole_window, wm_hints ); XFree( wm_hints ); } if (data->icon_bits) + { + TRACE( "window %p/%lx requesting _NET_WM_ICON, serial %lu\n", data->hwnd, + data->whole_window, NextRequest( data->display ) ); XChangeProperty( data->display, data->whole_window, x11drv_atom(_NET_WM_ICON), XA_CARDINAL, 32, PropModeReplace, (unsigned char *)data->icon_bits, data->icon_size ); + } else + { + TRACE( "window %p/%lx deleting _NET_WM_ICON, serial %lu\n", data->hwnd, + data->whole_window, NextRequest( data->display ) ); XDeleteProperty( data->display, data->whole_window, x11drv_atom(_NET_WM_ICON) ); + } } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8079
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/winex11.drv/window.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dlls/winex11.drv/window.c b/dlls/winex11.drv/window.c index c6ef4ab7d9d..c8d593bb08b 100644 --- a/dlls/winex11.drv/window.c +++ b/dlls/winex11.drv/window.c @@ -1200,7 +1200,7 @@ static void window_set_net_wm_state( struct x11drv_win_data *data, UINT new_stat new_state &= x11drv_thread_data()->net_wm_state_mask; data->desired_state.net_wm_state = new_state; - if (!data->whole_window) return; /* no window, nothing to update */ + if (!data->whole_window || !data->managed) 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 */ if (old_state == new_state) return; /* states are the same, nothing to update */ @@ -1328,7 +1328,7 @@ static void update_net_wm_states( struct x11drv_win_data *data ) static const UINT fullscreen_mask = (1 << NET_WM_STATE_MAXIMIZED) | (1 << NET_WM_STATE_FULLSCREEN); UINT style, ex_style, new_state = 0; - if (!data->managed || data->embedded) return; + if (data->embedded) return; if (data->whole_window == root_window) { if (is_virtual_desktop()) window_set_net_wm_state( data, is_desktop_fullscreen() ? (1 << NET_WM_STATE_FULLSCREEN) : 0 ); -- 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 c8d593bb08b..7db2acd2d3a 100644 --- a/dlls/winex11.drv/window.c +++ b/dlls/winex11.drv/window.c @@ -1241,6 +1241,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; @@ -1248,7 +1249,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 | 36 +++++++++++++++++++++++++++++------- 1 file changed, 29 insertions(+), 7 deletions(-) diff --git a/dlls/winex11.drv/window.c b/dlls/winex11.drv/window.c index 7db2acd2d3a..f957d9b84f3 100644 --- a/dlls/winex11.drv/window.c +++ b/dlls/winex11.drv/window.c @@ -1194,6 +1194,24 @@ static void update_net_wm_fullscreen_monitors( struct x11drv_win_data *data ) } } +/* 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_net_wm_state_change_delay( struct x11drv_win_data *data ) +{ + if (data->pending_state.wm_state != NormalState || !data->configure_serial) return FALSE; + /* delay any new _NET_WM_STATE request which might trigger a ConfigureNotify when a config 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 ) +{ + if (data->pending_state.wm_state != NormalState || !data->net_wm_state_serial) return FALSE; + /* delay any config request when a _NET_WM_STATE change that can trigger a ConfigureNotify is in flight */ + return (data->pending_state.net_wm_state ^ data->current_state.net_wm_state) & config_notify_mask; +} + static void window_set_net_wm_state( struct x11drv_win_data *data, UINT new_state ) { UINT i, count, old_state = data->pending_state.net_wm_state; @@ -1205,6 +1223,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 (window_needs_net_wm_state_change_delay( data )) + { + /* another maximized/fullscreen NET_WM_STATE update is pending, wait for it to complete as we might have delayed our config request */ + WARN( "window %p/%lx is updating config, delaying request\n", data->hwnd, data->whole_window ); + return; + } + if (data->pending_state.wm_state == IconicState) return; /* window is iconic, don't update its state now */ if (data->pending_state.wm_state == WithdrawnState) /* set the _NET_WM_STATE atom directly */ { @@ -1263,7 +1288,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, const RECT *new_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; @@ -1272,15 +1296,13 @@ static void window_set_config( struct x11drv_win_data *data, const RECT *new_rec 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 (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. + * entering/exiting a maximized/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 ); + WARN( "window %p/%lx is updating unsafe _NET_WM_STATE, delaying request\n", data->hwnd, data->whole_window ); return; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8079
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/winex11.drv/window.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/dlls/winex11.drv/window.c b/dlls/winex11.drv/window.c index f957d9b84f3..42d6a468dac 100644 --- a/dlls/winex11.drv/window.c +++ b/dlls/winex11.drv/window.c @@ -1336,10 +1336,14 @@ static void window_set_config( struct x11drv_win_data *data, const RECT *new_rec mask |= CWStackMode; } - data->pending_state.rect = *new_rect; + if (mask & CWX) OffsetRect( &data->pending_state.rect, new_rect->left - old_rect->left, 0 ); + if (mask & CWY) OffsetRect( &data->pending_state.rect, 0, new_rect->top - old_rect->top ); + if (mask & CWWidth) data->pending_state.rect.right = data->pending_state.rect.left + new_rect->right - new_rect->left; + if (mask & CWHeight) data->pending_state.rect.bottom = data->pending_state.rect.top + new_rect->bottom - new_rect->top; + 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 ); + wine_dbgstr_rect(&data->pending_state.rect), mask, above, data->configure_serial ); XReconfigureWMWindow( data->display, data->whole_window, data->vis.screen, mask, &changes ); } -- 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. --- dlls/winex11.drv/window.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/dlls/winex11.drv/window.c b/dlls/winex11.drv/window.c index 42d6a468dac..2c1ef70ee2a 100644 --- a/dlls/winex11.drv/window.c +++ b/dlls/winex11.drv/window.c @@ -1295,6 +1295,7 @@ static void window_set_config( struct x11drv_win_data *data, const RECT *new_rec 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 && data->configure_serial) return; /* another config update is pending, wait for it to complete */ if (window_needs_config_change_delay( data )) { @@ -1776,8 +1777,14 @@ 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; + + /* 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
v2: Better request serialization. * Serialize _NET_WM_STATE requests with each other, serials aren't reliable to tell old requests replies from latest ones. * Same thing for config requests with each other when they go through the window manager. * Serialize more _NET_WM_STATE bits that would likely generate a ConfigureNotify, checking `desired_state ^ pending_state` to decide whether the bit is going to be changed. * Request pending state in `window_configure_notify` too, as config requests are now serialized too. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8079#note_103859
participants (1)
-
Rémi Bernon