[PATCH 0/1] MR8128: winex11.drv: Reconfigure window even if the new rectangle equals to the old one.
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. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8128
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 | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/dlls/winex11.drv/window.c b/dlls/winex11.drv/window.c index e154db1e23a..eee0db16d12 100644 --- a/dlls/winex11.drv/window.c +++ b/dlls/winex11.drv/window.c @@ -1287,12 +1287,14 @@ static void window_set_config( struct x11drv_win_data *data, const RECT *new_rec { 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; 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 */ + + /* 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 */ if (data->pending_state.wm_state == NormalState && data->net_wm_state_serial && !(data->pending_state.net_wm_state & fullscreen_mask) && @@ -1307,9 +1309,7 @@ static void window_set_config( struct x11drv_win_data *data, const RECT *new_rec } /* 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; @@ -1321,8 +1321,7 @@ static void window_set_config( struct x11drv_win_data *data, const RECT *new_rec } /* 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; @@ -1336,6 +1335,8 @@ static void window_set_config( struct x11drv_win_data *data, const RECT *new_rec 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/8128
This is actually trickier than it originally seemed, because we'll want to serialize the requests and wait for the ConfigureNotify events. Requesting configs which exactly matches the current config often causes the window manager to ignore it entirely, and no event is generated, which we would then wait forever. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8128#note_104327
Requesting configs which exactly matches the current config often causes the window manager to ignore it entirely, and no event is generated, which we would then wait forever.
Do you have an example for this? As I understand, sending a ConfigureRequest event to window managers will always generate a ConfigureNotify event, even if the requested config is the same as the current config in the window manager. This behavior is standardized in [ICCCM 4.1.5](https://tronche.com/gui/x/icccm/sec-4.html). ``` * Not changing the size, location, border width, or stacking order of the window at all. A client will receive a synthetic ConfigureNotify event that describes the (unchanged) geometry of the window. The (x,y) coordinates will be in the root coordinate system, adjusted for the border width the client requested, irrespective of any reparenting that has taken place. The border_width will be the border width the client requested. The client will not receive a real ConfigureNotify event because no change has actually taken place. ``` There is the Mutter code in meta_window_x11_move_resize_internal() ``` /* If this is a configure request and we change nothing, then we * must send configure notify. */ if (is_configure_request && !(need_move_client || need_move_frame || need_resize_client || need_resize_frame || priv->border_width != 0)) need_configure_notify = TRUE; ``` Also the KWin code in X11Window::configureRequestEvent(). ``` if (e->value_mask & (XCB_CONFIG_WINDOW_X | XCB_CONFIG_WINDOW_Y | XCB_CONFIG_WINDOW_HEIGHT | XCB_CONFIG_WINDOW_WIDTH)) { configureRequest(e->value_mask, Xcb::fromXNative(e->x), Xcb::fromXNative(e->y), Xcb::fromXNative(e->width), Xcb::fromXNative(e->height), 0, false); } if (e->value_mask & XCB_CONFIG_WINDOW_STACK_MODE) { restackWindow(e->sibling, e->stack_mode, NET::FromApplication, userTime(), false); } // Sending a synthetic configure notify always is fine, even in cases where // the ICCCM doesn't require this - it can be though of as 'the WM decided to move // the window later'. The window should not cause that many configure request, // so this should not have any significant impact. With user moving/resizing // the it should be optimized though (see also X11Window::setGeometry()/resize()/move()). sendSyntheticConfigureNotify(); ``` Of course, there might be window managers that might not be ICCCM compliant. If what you're saying can be true, then serializing configure requests and waiting for ConfigureNotify might be problematic as well. For example, let's say a window goes from 100x100 to fullscreen, then removing __NET_WM_STATE_FULLSCREEN and Wine requests it to 100x100. Removing __NET_WM_STATE_FULLSCREEN will cause the window manager to restore the window geometry back to the size before fullscreen, so 100x100. Then Wine sends a configure request to change to 100x100, without realizing the window geometry has already been changed to 100x100. Since they have the same geometry, no configure notify as you said. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8128#note_104337
On Fri May 23 14:42:36 2025 +0000, Zhiyi Zhang wrote:
Requesting configs which exactly matches the current config often causes the window manager to ignore it entirely, and no event is generated, which we would then wait forever. Do you have an example for this? As I understand, sending a ConfigureRequest event to window managers will always generate a ConfigureNotify event, even if the requested config is the same as the current config in the window manager. This behavior is standardized in [ICCCM 4.1.5](https://tronche.com/gui/x/icccm/sec-4.html).
* Not changing the size, location, border width, or stacking order of the window at all. A client will receive a synthetic ConfigureNotify event that describes the (unchanged) geometry of the window. The (x,y) coordinates will be in the root coordinate system, adjusted for the border width the client requested, irrespective of any reparenting that has taken place. The border_width will be the border width the client requested. The client will not receive a real ConfigureNotify event because no change has actually taken place.There is the Mutter code in meta_window_x11_move_resize_internal() ``` /* If this is a configure request and we change nothing, then we * must send configure notify. */ if (is_configure_request && !(need_move_client || need_move_frame || need_resize_client || need_resize_frame || priv->border_width != 0)) need_configure_notify = TRUE; ``` Also the KWin code in X11Window::configureRequestEvent(). ``` if (e->value_mask & (XCB_CONFIG_WINDOW_X | XCB_CONFIG_WINDOW_Y | XCB_CONFIG_WINDOW_HEIGHT | XCB_CONFIG_WINDOW_WIDTH)) { configureRequest(e->value_mask, Xcb::fromXNative(e->x), Xcb::fromXNative(e->y), Xcb::fromXNative(e->width), Xcb::fromXNative(e->height), 0, false); } if (e->value_mask & XCB_CONFIG_WINDOW_STACK_MODE) { restackWindow(e->sibling, e->stack_mode, NET::FromApplication, userTime(), false); } // Sending a synthetic configure notify always is fine, even in cases where // the ICCCM doesn't require this - it can be though of as 'the WM decided to move // the window later'. The window should not cause that many configure request, // so this should not have any significant impact. With user moving/resizing // the it should be optimized though (see also X11Window::setGeometry()/resize()/move()). sendSyntheticConfigureNotify(); ``` Of course, there might be window managers that might not be ICCCM compliant. If what you're saying can be true, then serializing configure requests and waiting for ConfigureNotify might be problematic as well. For example, let's say a window goes from 100x100 to fullscreen, then removing __NET_WM_STATE_FULLSCREEN and Wine requests it to 100x100. Removing __NET_WM_STATE_FULLSCREEN will cause the window manager to restore the window geometry back to the size before fullscreen, so 100x100. Then Wine sends a configure request to change to 100x100, without realizing the window geometry has already been changed to 100x100. Since they have the same geometry, no configure notify as you said. Hmm... I thought I had seen it locally during some tests but I can't reproduce it anymore and it's possible that the code I was testing was simply broken. It seems to be working indeed and I included this change into !8079 as it had some small conflicts (minor changes, mostly about tracing the actual rect we request, if size or height are being skipped because of the constraints).
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8128#note_104348
This merge request was closed by Zhiyi Zhang. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8128
participants (3)
-
Rémi Bernon -
Zhiyi Zhang
-
Zhiyi Zhang (＠zhiyi)