[PATCH 0/5] MR6944: winex11: Workaround KWin bug with maximized windows.
I know we've had a policy to avoid window manager specific workarounds, I think we could reconsider it. The fix could be unguarded and applied with every window manager, and could very well work, but I think it is ugly enough to justify checking for KWin specifically. The problem is that as soon as a window configure request is sent to a maximized window, KWin internal state gets bogus and it loses track of the window maximized state. I've described the KWin source details on https://bugs.kde.org/show_bug.cgi?id=496966 and opened https://invent.kde.org/plasma/kwin/-/merge_requests/6854 as a possible fix for it, but the time it will take to land could justify working around it in Wine. The workaround is to avoid sending configure requests to maximized windows, which sounds sensible and we already avoid resizing maximized windows, but, moving a maximized window to a different monitor *requires* sending a configure request. KWin bug makes no difference to requests with only position changes, and they trigger it all the same. So, the only solution is to temporarily remove the maximized state bits before sending the configure request, putting them back afterwards. This is quite straightforward to do with the new state tracker, but it could very well trigger other problems with other window managers. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=57465 -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6944
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/winex11.drv/window.c | 31 +++++++++++++++++-------------- dlls/winex11.drv/x11drv.h | 5 +++++ dlls/winex11.drv/x11drv_main.c | 1 + 3 files changed, 23 insertions(+), 14 deletions(-) diff --git a/dlls/winex11.drv/window.c b/dlls/winex11.drv/window.c index b99c96f644b..e35a3ff1873 100644 --- a/dlls/winex11.drv/window.c +++ b/dlls/winex11.drv/window.c @@ -104,11 +104,6 @@ static XContext host_window_context = 0; static Time last_user_time; static Window user_time_window; -/* list of _NET_SUPPORTED atoms */ -static Atom *net_supported; -static int net_supported_count; -static UINT net_wm_state_mask; - static const WCHAR whole_window_prop[] = {'_','_','w','i','n','e','_','x','1','1','_','w','h','o','l','e','_','w','i','n','d','o','w',0}; static const WCHAR clip_window_prop[] = @@ -1218,7 +1213,7 @@ static void window_set_net_wm_state( struct x11drv_win_data *data, UINT new_stat { UINT i, count, old_state = data->pending_state.net_wm_state; - new_state &= net_wm_state_mask; + 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->wm_state_serial) return; /* another WM_STATE update is pending, wait for it to complete */ @@ -3271,10 +3266,15 @@ LRESULT X11DRV_WindowMessage( HWND hwnd, UINT msg, WPARAM wp, LPARAM lp ) */ static BOOL is_netwm_supported( Atom atom ) { + struct x11drv_thread_data *data = x11drv_thread_data(); + BOOL supported; int i; - for (i = 0; i < net_supported_count; i++) - if (net_supported[i] == atom) return TRUE; - return FALSE; + + for (i = 0; i < data->net_supported_count; i++) + if (data->net_supported[i] == atom) break; + supported = i < data->net_supported_count; + + return supported; } @@ -3400,22 +3400,25 @@ void X11DRV_FlashWindowEx( FLASHWINFO *pfinfo ) release_win_data( data ); } -void init_win_context(void) +void net_supported_init( struct x11drv_thread_data *data ) { unsigned long count, remaining; int format, i; Atom type; - if (!XGetWindowProperty( gdi_display, DefaultRootWindow( gdi_display ), x11drv_atom(_NET_SUPPORTED), 0, 65536 / sizeof(CARD32), - False, XA_ATOM, &type, &format, &count, &remaining, (unsigned char **)&net_supported )) - net_supported_count = get_property_size( format, count ) / sizeof(Atom); + if (!XGetWindowProperty( data->display, DefaultRootWindow( data->display ), x11drv_atom(_NET_SUPPORTED), 0, 65536 / sizeof(CARD32), + False, XA_ATOM, &type, &format, &count, &remaining, (unsigned char **)&data->net_supported )) + data->net_supported_count = get_property_size( format, count ) / sizeof(Atom); for (i = 0; i < NB_NET_WM_STATES; i++) { Atom atom = X11DRV_Atoms[net_wm_state_atoms[i] - FIRST_XATOM]; - if (is_netwm_supported( atom )) net_wm_state_mask |= (1 << i); + if (is_netwm_supported( atom )) data->net_wm_state_mask |= (1 << i); } +} +void init_win_context(void) +{ init_recursive_mutex( &win_data_mutex ); winContext = XUniqueContext(); diff --git a/dlls/winex11.drv/x11drv.h b/dlls/winex11.drv/x11drv.h index 860ba22eae4..995df13e843 100644 --- a/dlls/winex11.drv/x11drv.h +++ b/dlls/winex11.drv/x11drv.h @@ -394,6 +394,9 @@ struct x11drv_thread_data unsigned long warp_serial; /* serial number of last pointer warp request */ Window clip_window; /* window used for cursor clipping */ BOOL clipping_cursor; /* whether thread is currently clipping the cursor */ + Atom *net_supported; /* list of _NET_SUPPORTED atoms */ + int net_supported_count; /* number of _NET_SUPPORTED atoms */ + UINT net_wm_state_mask; /* mask of supported _NET_WM_STATE *bits */ #ifdef HAVE_X11_EXTENSIONS_XINPUT2_H XIValuatorClassInfo x_valuator; XIValuatorClassInfo y_valuator; @@ -663,6 +666,8 @@ extern void window_net_wm_state_notify( struct x11drv_win_data *data, unsigned l extern void window_configure_notify( struct x11drv_win_data *data, unsigned long serial, const RECT *rect ); extern BOOL get_window_state_updates( HWND hwnd, UINT *state_cmd, UINT *config_cmd, RECT *rect ); +extern void net_supported_init( struct x11drv_thread_data *data ); + extern Window init_clip_window(void); extern void update_user_time( Time time ); extern UINT get_window_net_wm_state( Display *display, Window window ); diff --git a/dlls/winex11.drv/x11drv_main.c b/dlls/winex11.drv/x11drv_main.c index e441298c9aa..2e27c7f6bf1 100644 --- a/dlls/winex11.drv/x11drv_main.c +++ b/dlls/winex11.drv/x11drv_main.c @@ -755,6 +755,7 @@ struct x11drv_thread_data *x11drv_init_thread_data(void) if (use_xim) xim_thread_attach( data ); x11drv_xinput2_init( data ); + net_supported_init( data ); return data; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6944
From: Rémi Bernon <rbernon(a)codeweavers.com> This might happen if the window manager is restarted or changed while a Wine window is opened. We should support that use case properly. --- dlls/winex11.drv/event.c | 17 +++++++++++++++++ dlls/winex11.drv/window.c | 2 +- dlls/winex11.drv/x11drv_main.c | 1 + 3 files changed, 19 insertions(+), 1 deletion(-) diff --git a/dlls/winex11.drv/event.c b/dlls/winex11.drv/event.c index 65b31518691..cf9fd96a2fb 100644 --- a/dlls/winex11.drv/event.c +++ b/dlls/winex11.drv/event.c @@ -1245,6 +1245,21 @@ static void handle_net_wm_state_notify( HWND hwnd, XPropertyEvent *event ) NtUserPostMessage( hwnd, WM_WINE_WINDOW_STATE_CHANGED, 0, 0 ); } +static void handle_net_supported_notify( XPropertyEvent *event ) +{ + struct x11drv_thread_data *data = x11drv_thread_data(); + + if (data->net_supported) + { + data->net_supported_count = 0; + XFree( data->net_supported ); + data->net_supported = NULL; + data->net_wm_state_mask = 0; + } + + if (event->state == PropertyNewValue) net_supported_init( data ); +} + /*********************************************************************** * X11DRV_PropertyNotify */ @@ -1256,6 +1271,8 @@ static BOOL X11DRV_PropertyNotify( HWND hwnd, XEvent *xev ) if (event->atom == x11drv_atom(WM_STATE)) handle_wm_state_notify( hwnd, event ); if (event->atom == x11drv_atom(_XEMBED_INFO)) handle_xembed_info_notify( hwnd, event ); if (event->atom == x11drv_atom(_NET_WM_STATE)) handle_net_wm_state_notify( hwnd, event ); + if (event->atom == x11drv_atom(_NET_SUPPORTED)) handle_net_supported_notify( event ); + return TRUE; } diff --git a/dlls/winex11.drv/window.c b/dlls/winex11.drv/window.c index e35a3ff1873..179ca0b5ca3 100644 --- a/dlls/winex11.drv/window.c +++ b/dlls/winex11.drv/window.c @@ -2536,7 +2536,7 @@ void X11DRV_SystrayDockInit( HWND hwnd ) sprintf( systray_buffer, "_NET_SYSTEM_TRAY_S%u", DefaultScreen( display ) ); systray_atom = XInternAtom( display, systray_buffer, False ); } - XSelectInput( display, root_window, StructureNotifyMask ); + XSelectInput( display, root_window, StructureNotifyMask | PropertyChangeMask ); } diff --git a/dlls/winex11.drv/x11drv_main.c b/dlls/winex11.drv/x11drv_main.c index 2e27c7f6bf1..33cf2384822 100644 --- a/dlls/winex11.drv/x11drv_main.c +++ b/dlls/winex11.drv/x11drv_main.c @@ -753,6 +753,7 @@ struct x11drv_thread_data *x11drv_init_thread_data(void) set_queue_display_fd( data->display ); NtUserGetThreadInfo()->driver_data = (UINT_PTR)data; + XSelectInput( data->display, DefaultRootWindow( data->display ), PropertyChangeMask ); if (use_xim) xim_thread_attach( data ); x11drv_xinput2_init( data ); net_supported_init( data ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6944
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/winex11.drv/event.c | 8 ++++++++ dlls/winex11.drv/window.c | 28 ++++++++++++++++++++++++++++ dlls/winex11.drv/x11drv.h | 2 ++ dlls/winex11.drv/x11drv_main.c | 2 ++ 4 files changed, 40 insertions(+) diff --git a/dlls/winex11.drv/event.c b/dlls/winex11.drv/event.c index cf9fd96a2fb..e20d5cb70b5 100644 --- a/dlls/winex11.drv/event.c +++ b/dlls/winex11.drv/event.c @@ -1260,6 +1260,13 @@ static void handle_net_supported_notify( XPropertyEvent *event ) if (event->state == PropertyNewValue) net_supported_init( data ); } +static void handle_net_supporting_wm_check_notify( XPropertyEvent *event ) +{ + struct x11drv_thread_data *data = x11drv_thread_data(); + + if (event->state == PropertyNewValue) net_supporting_wm_check_init( data ); +} + /*********************************************************************** * X11DRV_PropertyNotify */ @@ -1272,6 +1279,7 @@ static BOOL X11DRV_PropertyNotify( HWND hwnd, XEvent *xev ) if (event->atom == x11drv_atom(_XEMBED_INFO)) handle_xembed_info_notify( hwnd, event ); if (event->atom == x11drv_atom(_NET_WM_STATE)) handle_net_wm_state_notify( hwnd, event ); if (event->atom == x11drv_atom(_NET_SUPPORTED)) handle_net_supported_notify( event ); + if (event->atom == x11drv_atom(_NET_SUPPORTING_WM_CHECK)) handle_net_supporting_wm_check_notify( event ); return TRUE; } diff --git a/dlls/winex11.drv/window.c b/dlls/winex11.drv/window.c index 179ca0b5ca3..d31f0d87030 100644 --- a/dlls/winex11.drv/window.c +++ b/dlls/winex11.drv/window.c @@ -3417,6 +3417,34 @@ void net_supported_init( struct x11drv_thread_data *data ) } } +static Window get_net_supporting_wm_check( Display *display, Window window ) +{ + unsigned long count, remaining; + Window *tmp, support = None; + int format; + Atom type; + + if (!XGetWindowProperty( display, window, x11drv_atom(_NET_SUPPORTING_WM_CHECK), 0, 65536 / sizeof(CARD32), + False, XA_WINDOW, &type, &format, &count, &remaining, (unsigned char **)&tmp )) + { + support = *tmp; + free( tmp ); + } + + return support; +} + +void net_supporting_wm_check_init( struct x11drv_thread_data *data ) +{ + Window window = None, other; + + window = get_net_supporting_wm_check( data->display, DefaultRootWindow( data->display ) ); + /* the window itself must have the property set too */ + X11DRV_expect_error( data->display, host_window_error, NULL ); + other = get_net_supporting_wm_check( data->display, window ); + if (X11DRV_check_error() || window != other) WARN( "Invalid _NET_SUPPORTING_WM_CHECK window\n" ); +} + void init_win_context(void) { init_recursive_mutex( &win_data_mutex ); diff --git a/dlls/winex11.drv/x11drv.h b/dlls/winex11.drv/x11drv.h index 995df13e843..324e9d0f3ae 100644 --- a/dlls/winex11.drv/x11drv.h +++ b/dlls/winex11.drv/x11drv.h @@ -487,6 +487,7 @@ enum x11drv_atoms XATOM__NET_STARTUP_INFO_BEGIN, XATOM__NET_STARTUP_INFO, XATOM__NET_SUPPORTED, + XATOM__NET_SUPPORTING_WM_CHECK, XATOM__NET_SYSTEM_TRAY_OPCODE, XATOM__NET_SYSTEM_TRAY_S0, XATOM__NET_SYSTEM_TRAY_VISUAL, @@ -667,6 +668,7 @@ extern void window_configure_notify( struct x11drv_win_data *data, unsigned long extern BOOL get_window_state_updates( HWND hwnd, UINT *state_cmd, UINT *config_cmd, RECT *rect ); extern void net_supported_init( struct x11drv_thread_data *data ); +extern void net_supporting_wm_check_init( struct x11drv_thread_data *data ); extern Window init_clip_window(void); extern void update_user_time( Time time ); diff --git a/dlls/winex11.drv/x11drv_main.c b/dlls/winex11.drv/x11drv_main.c index 33cf2384822..89afdbc8daa 100644 --- a/dlls/winex11.drv/x11drv_main.c +++ b/dlls/winex11.drv/x11drv_main.c @@ -132,6 +132,7 @@ static const char * const atom_names[NB_XATOMS - FIRST_XATOM] = "_NET_STARTUP_INFO_BEGIN", "_NET_STARTUP_INFO", "_NET_SUPPORTED", + "_NET_SUPPORTING_WM_CHECK", "_NET_SYSTEM_TRAY_OPCODE", "_NET_SYSTEM_TRAY_S0", "_NET_SYSTEM_TRAY_VISUAL", @@ -757,6 +758,7 @@ struct x11drv_thread_data *x11drv_init_thread_data(void) if (use_xim) xim_thread_attach( data ); x11drv_xinput2_init( data ); net_supported_init( data ); + net_supporting_wm_check_init( data ); return data; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6944
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/winex11.drv/event.c | 6 ++++++ dlls/winex11.drv/window.c | 15 +++++++++++++++ dlls/winex11.drv/x11drv.h | 2 ++ dlls/winex11.drv/x11drv_main.c | 1 + 4 files changed, 24 insertions(+) diff --git a/dlls/winex11.drv/event.c b/dlls/winex11.drv/event.c index e20d5cb70b5..29967539c20 100644 --- a/dlls/winex11.drv/event.c +++ b/dlls/winex11.drv/event.c @@ -1264,6 +1264,12 @@ static void handle_net_supporting_wm_check_notify( XPropertyEvent *event ) { struct x11drv_thread_data *data = x11drv_thread_data(); + if (data->window_manager) + { + XFree( data->window_manager ); + data->window_manager = NULL; + } + if (event->state == PropertyNewValue) net_supporting_wm_check_init( data ); } diff --git a/dlls/winex11.drv/window.c b/dlls/winex11.drv/window.c index d31f0d87030..b2684f7dfd2 100644 --- a/dlls/winex11.drv/window.c +++ b/dlls/winex11.drv/window.c @@ -3434,6 +3434,18 @@ static Window get_net_supporting_wm_check( Display *display, Window window ) return support; } +static BOOL get_window_property_str( Display *display, Window window, Atom atom, char **name ) +{ + unsigned long count, remaining; + int format, ret; + Atom type; + + X11DRV_expect_error( display, host_window_error, NULL ); + ret = XGetWindowProperty( display, window, atom, 0, 65536 / sizeof(CARD32), False, x11drv_atom(UTF8_STRING), + &type, &format, &count, &remaining, (unsigned char **)name ); + return !X11DRV_check_error() && !ret; +} + void net_supporting_wm_check_init( struct x11drv_thread_data *data ) { Window window = None, other; @@ -3443,6 +3455,9 @@ void net_supporting_wm_check_init( struct x11drv_thread_data *data ) X11DRV_expect_error( data->display, host_window_error, NULL ); other = get_net_supporting_wm_check( data->display, window ); if (X11DRV_check_error() || window != other) WARN( "Invalid _NET_SUPPORTING_WM_CHECK window\n" ); + else if (get_window_property_str( data->display, window, x11drv_atom(_NET_WM_NAME), &data->window_manager ) || + get_window_property_str( data->display, window, x11drv_atom(WM_NAME), &data->window_manager )) + TRACE( "Detected window manager: %s\n", debugstr_a(data->window_manager) ); } void init_win_context(void) diff --git a/dlls/winex11.drv/x11drv.h b/dlls/winex11.drv/x11drv.h index 324e9d0f3ae..ed72214bf58 100644 --- a/dlls/winex11.drv/x11drv.h +++ b/dlls/winex11.drv/x11drv.h @@ -397,6 +397,7 @@ struct x11drv_thread_data Atom *net_supported; /* list of _NET_SUPPORTED atoms */ int net_supported_count; /* number of _NET_SUPPORTED atoms */ UINT net_wm_state_mask; /* mask of supported _NET_WM_STATE *bits */ + char *window_manager; /* name of the supporting window manager */ #ifdef HAVE_X11_EXTENSIONS_XINPUT2_H XIValuatorClassInfo x_valuator; XIValuatorClassInfo y_valuator; @@ -477,6 +478,7 @@ enum x11drv_atoms XATOM_RAW_CAP_HEIGHT, XATOM_WM_PROTOCOLS, XATOM_WM_DELETE_WINDOW, + XATOM_WM_NAME, XATOM_WM_STATE, XATOM_WM_TAKE_FOCUS, XATOM_DndProtocol, diff --git a/dlls/winex11.drv/x11drv_main.c b/dlls/winex11.drv/x11drv_main.c index 89afdbc8daa..44712efde76 100644 --- a/dlls/winex11.drv/x11drv_main.c +++ b/dlls/winex11.drv/x11drv_main.c @@ -122,6 +122,7 @@ static const char * const atom_names[NB_XATOMS - FIRST_XATOM] = "RAW_CAP_HEIGHT", "WM_PROTOCOLS", "WM_DELETE_WINDOW", + "WM_NAME", "WM_STATE", "WM_TAKE_FOCUS", "DndProtocol", -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6944
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/winex11.drv/window.c | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/dlls/winex11.drv/window.c b/dlls/winex11.drv/window.c index b2684f7dfd2..50d1681c6c2 100644 --- a/dlls/winex11.drv/window.c +++ b/dlls/winex11.drv/window.c @@ -1278,7 +1278,8 @@ 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; + UINT style = NtUserGetWindowLongW( data->hwnd, GWL_STYLE ), mask = 0, net_wm_state = -1; + struct x11drv_thread_data *thread_data = x11drv_thread_data(); const RECT *old_rect = &data->pending_state.rect; XWindowChanges changes; @@ -1286,16 +1287,25 @@ 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 )) return; /* rects are the same, 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->pending_state.net_wm_state | data->current_state.net_wm_state) & fullscreen_mask) && + data->pending_state.wm_state == NormalState) { /* 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 ); - return; + if (data->net_wm_state_serial) + { + WARN( "window %p/%lx is updating _NET_WM_STATE, delaying request\n", data->hwnd, data->whole_window ); + return; + } + + if (thread_data->window_manager && !strcmp( thread_data->window_manager, "KWin" )) + { + /* Workaround for KWin bug 496966: clear maximized state before requesting new config */ + net_wm_state = data->pending_state.net_wm_state; + window_set_net_wm_state( data, net_wm_state & ~fullscreen_mask ); + } } /* resizing a managed maximized window is not allowed */ @@ -1330,6 +1340,9 @@ static void window_set_config( struct x11drv_win_data *data, const RECT *new_rec TRACE( "window %p/%lx, requesting config %s above %u, serial %lu\n", data->hwnd, data->whole_window, wine_dbgstr_rect(new_rect), above, data->configure_serial ); XReconfigureWMWindow( data->display, data->whole_window, data->vis.screen, mask, &changes ); + + /* Workaround for KWin bug 496966: restore maximized state after requesting new config */ + if (net_wm_state != -1) window_set_net_wm_state( data, net_wm_state ); } /*********************************************************************** -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6944
participants (1)
-
Rémi Bernon