[PATCH 0/3] MR7182: winex11: Keep track and use expected window focus time on WM_STATE changes.
This is an attempt at improving X11 focus handling, and will most importantly be useful to later better handle dynamic window decoration changes on any mutter-based window manager. These window managers do a little dance in that case, unmapping/mapping the window every time decoration are added/removed, and we often get confused by the focus events it triggers. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7182
From: Rémi Bernon <rbernon(a)codeweavers.com> The Win32 state might not have been updated with the latest X11 state, and we want to take future updates into account when deciding to accept focus or not. At this point, and because we ignore WM_TAKE_FOCUS when a state change is pending, all the states are the same, this uses current_state as the logic is to accept focus, or not, according to our current knwledge of the X state. --- dlls/winex11.drv/event.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/dlls/winex11.drv/event.c b/dlls/winex11.drv/event.c index 428dcb7b8b7..cc11285d0c6 100644 --- a/dlls/winex11.drv/event.c +++ b/dlls/winex11.drv/event.c @@ -595,8 +595,18 @@ DWORD EVENT_x11_time_to_win32_time(Time time) static inline BOOL can_activate_window( HWND hwnd ) { LONG style = NtUserGetWindowLongW( hwnd, GWL_STYLE ); + struct x11drv_win_data *data; RECT rect; + if ((data = get_win_data( hwnd ))) + { + style = style & ~(WS_VISIBLE | WS_MINIMIZE | WS_MAXIMIZE); + if (data->current_state.wm_state != WithdrawnState) style |= WS_VISIBLE; + if (data->current_state.wm_state == IconicState) style |= WS_MINIMIZE; + if (data->current_state.net_wm_state & (1 << NET_WM_STATE_MAXIMIZED)) style |= WS_MAXIMIZE; + release_win_data( data ); + } + if (!(style & WS_VISIBLE)) return FALSE; if ((style & (WS_POPUP|WS_CHILD)) == WS_CHILD) return FALSE; if (style & WS_MINIMIZE) return FALSE; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7182
From: Rémi Bernon <rbernon(a)codeweavers.com> SetForegroundWindow calls aren't reflected to the X11 side, and we're sometimes becoming inconsistent between the X focused window and the Win32 foreground window. When a window gets unmapped, the window manager will send WM_TAKE_FOCUS to a different window, which may not be the foreground window from the Win32 point of view. Requesting input focus to our foreground before unmapping the focused window effectively resyncs the state. --- dlls/winex11.drv/event.c | 10 ++++++++-- dlls/winex11.drv/window.c | 8 ++++++++ dlls/winex11.drv/x11drv.h | 1 + 3 files changed, 17 insertions(+), 2 deletions(-) diff --git a/dlls/winex11.drv/event.c b/dlls/winex11.drv/event.c index cc11285d0c6..646ba4c140a 100644 --- a/dlls/winex11.drv/event.c +++ b/dlls/winex11.drv/event.c @@ -849,10 +849,13 @@ static BOOL X11DRV_FocusIn( HWND hwnd, XEvent *xev ) { HWND foreground = NtUserGetForegroundWindow(); XFocusChangeEvent *event = &xev->xfocus; + struct x11drv_win_data *data; BOOL was_grabbed; if (event->detail == NotifyPointer) return FALSE; - if (!hwnd) return FALSE; + if (!(data = get_win_data( hwnd ))) return FALSE; + data->has_focus = 1; + release_win_data( data ); if (window_has_pending_wm_state( hwnd, -1 )) { @@ -934,6 +937,7 @@ static BOOL X11DRV_FocusOut( HWND hwnd, XEvent *xev ) { HWND foreground = NtUserGetForegroundWindow(); XFocusChangeEvent *event = &xev->xfocus; + struct x11drv_win_data *data; if (event->detail == NotifyPointer) { @@ -946,7 +950,9 @@ static BOOL X11DRV_FocusOut( HWND hwnd, XEvent *xev ) } return TRUE; } - if (!hwnd) return FALSE; + if (!(data = get_win_data( hwnd ))) return FALSE; + data->has_focus = 0; + release_win_data( data ); if (window_has_pending_wm_state( hwnd, NormalState )) /* ignore FocusOut only if the window is being shown */ { diff --git a/dlls/winex11.drv/window.c b/dlls/winex11.drv/window.c index 32c3800abfe..8ce9c16d673 100644 --- a/dlls/winex11.drv/window.c +++ b/dlls/winex11.drv/window.c @@ -1441,6 +1441,7 @@ static void set_xembed_flags( struct x11drv_win_data *data, unsigned long flags static void window_set_wm_state( struct x11drv_win_data *data, UINT new_state ) { UINT old_state = data->pending_state.wm_state; + HWND foreground = NtUserGetForegroundWindow(); data->desired_state.wm_state = new_state; if (!data->whole_window) return; /* no window, nothing to update */ @@ -1463,6 +1464,13 @@ static void window_set_wm_state( struct x11drv_win_data *data, UINT new_state ) break; } + if (new_state != NormalState && data->has_focus && data->hwnd != foreground) + { + Window window = X11DRV_get_whole_window( foreground ); + WARN( "Inconsistent input focus, activating window %p/%lx\n", foreground, window ); + XSetInputFocus( data->display, window, RevertToParent, CurrentTime ); + } + data->pending_state.wm_state = new_state; data->wm_state_serial = NextRequest( data->display ); TRACE( "window %p/%lx, requesting WM_STATE %#x -> %#x serial %lu, foreground %p\n", data->hwnd, data->whole_window, diff --git a/dlls/winex11.drv/x11drv.h b/dlls/winex11.drv/x11drv.h index ee5bbe07dd4..55c5fc10f3c 100644 --- a/dlls/winex11.drv/x11drv.h +++ b/dlls/winex11.drv/x11drv.h @@ -636,6 +636,7 @@ struct x11drv_win_data UINT is_fullscreen : 1; /* is the window visible rect fullscreen */ UINT is_offscreen : 1; /* has been moved offscreen by the window manager */ UINT parent_invalid : 1; /* is the parent host window possibly invalid */ + UINT has_focus : 1; /* does window have X input focus */ Window embedder; /* window id of embedder */ Pixmap icon_pixmap; Pixmap icon_mask; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7182
From: Rémi Bernon <rbernon(a)codeweavers.com> Setting a __wine_x11_focus_time window property to indicate the time of the last NormalState for any window, 0 if the window is currently not in NormalState, and -1 if the state change is transient. --- dlls/winex11.drv/event.c | 6 +++--- dlls/winex11.drv/window.c | 19 ++++++++++++++++++- dlls/winex11.drv/x11drv.h | 3 ++- 3 files changed, 23 insertions(+), 5 deletions(-) diff --git a/dlls/winex11.drv/event.c b/dlls/winex11.drv/event.c index 646ba4c140a..8ccd81bbe80 100644 --- a/dlls/winex11.drv/event.c +++ b/dlls/winex11.drv/event.c @@ -757,7 +757,7 @@ static void handle_wm_protocols( HWND hwnd, XClientMessageEvent *event ) { HWND last_focus = x11drv_thread_data()->last_focus, foreground = NtUserGetForegroundWindow(); - if (window_has_pending_wm_state( hwnd, -1 )) + if (window_has_pending_wm_state( hwnd, -1 ) || !window_should_take_focus( hwnd, foreground, event_time )) { WARN( "Ignoring window %p/%lx WM_TAKE_FOCUS serial %lu, event_time %ld, foreground %p during WM_STATE change\n", hwnd, event->window, event->serial, event_time, foreground ); @@ -1233,7 +1233,7 @@ static void handle_wm_state_notify( HWND hwnd, XPropertyEvent *event ) if (!(data = get_win_data( hwnd ))) return; if (event->state == PropertyNewValue) value = get_window_wm_state( event->display, event->window ); - window_wm_state_notify( data, event->serial, value ); + window_wm_state_notify( data, event->serial, value, event->time ); release_win_data( data ); NtUserPostMessage( hwnd, WM_WINE_WINDOW_STATE_CHANGED, 0, 0 ); @@ -1246,7 +1246,7 @@ static void handle_xembed_info_notify( HWND hwnd, XPropertyEvent *event ) if (!(data = get_win_data( hwnd ))) return; if (event->state == PropertyNewValue) value = get_window_xembed_info( event->display, event->window ); - window_wm_state_notify( data, event->serial, value ? NormalState : WithdrawnState ); + window_wm_state_notify( data, event->serial, value ? NormalState : WithdrawnState, event->time ); release_win_data( data ); } diff --git a/dlls/winex11.drv/window.c b/dlls/winex11.drv/window.c index 8ce9c16d673..0007024475c 100644 --- a/dlls/winex11.drv/window.c +++ b/dlls/winex11.drv/window.c @@ -108,6 +108,8 @@ 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[] = {'_','_','w','i','n','e','_','x','1','1','_','c','l','i','p','_','w','i','n','d','o','w',0}; +static const WCHAR focus_time_prop[] = + {'_','_','w','i','n','e','_','x','1','1','_','f','o','c','u','s','_','t','i','m','e',0}; static pthread_mutex_t win_data_mutex = PTHREAD_MUTEX_INITIALIZER; @@ -1464,6 +1466,9 @@ static void window_set_wm_state( struct x11drv_win_data *data, UINT new_state ) break; } + if (new_state == NormalState) NtUserSetProp( data->hwnd, focus_time_prop, (HANDLE)-1 ); + else NtUserRemoveProp( data->hwnd, focus_time_prop ); + if (new_state != NormalState && data->has_focus && data->hwnd != foreground) { Window window = X11DRV_get_whole_window( foreground ); @@ -1642,7 +1647,7 @@ BOOL X11DRV_GetWindowStateUpdates( HWND hwnd, UINT *state_cmd, UINT *config_cmd, return *state_cmd || *config_cmd; } -void window_wm_state_notify( struct x11drv_win_data *data, unsigned long serial, UINT value ) +void window_wm_state_notify( struct x11drv_win_data *data, unsigned long serial, UINT value, Time time ) { UINT *desired = &data->desired_state.wm_state, *pending = &data->pending_state.wm_state, *current = &data->current_state.wm_state; unsigned long *expect_serial = &data->wm_state_serial; @@ -1679,6 +1684,9 @@ void window_wm_state_notify( struct x11drv_win_data *data, unsigned long serial, window_set_wm_state( data, data->desired_state.wm_state ); window_set_net_wm_state( data, data->desired_state.net_wm_state ); 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 ); } void window_net_wm_state_notify( struct x11drv_win_data *data, unsigned long serial, UINT value ) @@ -1763,6 +1771,15 @@ BOOL window_has_pending_wm_state( HWND hwnd, UINT state ) return pending; } +/* returns whether the window should accept focus instead of the current foreground window */ +BOOL window_should_take_focus( HWND hwnd, HWND foreground, Time time ) +{ + Time focus_time; + if (hwnd == foreground) return TRUE; + focus_time = (UINT_PTR)NtUserGetProp( foreground, focus_time_prop ); + return !focus_time || (int)(focus_time - time) < 0; +} + /*********************************************************************** * make_window_embedded */ diff --git a/dlls/winex11.drv/x11drv.h b/dlls/winex11.drv/x11drv.h index 55c5fc10f3c..d1cf5d46f6f 100644 --- a/dlls/winex11.drv/x11drv.h +++ b/dlls/winex11.drv/x11drv.h @@ -662,8 +662,9 @@ extern void set_gl_drawable_parent( HWND hwnd, HWND parent ); extern void destroy_gl_drawable( HWND hwnd ); extern void destroy_vk_surface( HWND hwnd ); +extern BOOL window_should_take_focus( HWND hwnd, HWND foreground, Time time ); extern BOOL window_has_pending_wm_state( HWND hwnd, UINT state ); -extern void window_wm_state_notify( struct x11drv_win_data *data, unsigned long serial, UINT value ); +extern void window_wm_state_notify( struct x11drv_win_data *data, unsigned long serial, UINT value, Time time ); extern void window_net_wm_state_notify( struct x11drv_win_data *data, unsigned long serial, UINT value ); 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 ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7182
I don't think it's correct to change the focus outside of processing a WM_TAKE_FOCUS. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7182#note_93084
This merge request was closed by Rémi Bernon. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7182
Superseded by https://gitlab.winehq.org/wine/wine/-/merge_requests/7367 and future changes. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7182#note_95135
participants (2)
-
Alexandre Julliard (＠julliard) -
Rémi Bernon