[PATCH v2 0/3] MR10620: winex11: Serialize _NET_WM_FULLSCREEN_MONITORS changes by expecting a ConfigureNotify.
When window has the _NET_WM_STATE_FULLSCREEN bit set. -- v2: winex11: Serialize _NET_WM_FULLSCREEN_MONITORS changes by expecting a ConfigureNotify. winex11: Update _NET_WM_FULLSCREEN_MONITORS before requesting _NET_WM_STATE_FULLSCREEN. winex11: Track actual _NET_WM_FULLSCREEN_MONITORS property updates. https://gitlab.winehq.org/wine/wine/-/merge_requests/10620
From: Rémi Bernon <rbernon@codeweavers.com> --- dlls/winex11.drv/event.c | 12 ++++++++ dlls/winex11.drv/window.c | 58 ++++++++++++++++++++++++++++++--------- dlls/winex11.drv/x11drv.h | 3 ++ 3 files changed, 60 insertions(+), 13 deletions(-) diff --git a/dlls/winex11.drv/event.c b/dlls/winex11.drv/event.c index 2983d7bfe8a..2656ebeebaa 100644 --- a/dlls/winex11.drv/event.c +++ b/dlls/winex11.drv/event.c @@ -1276,6 +1276,17 @@ static void handle_net_wm_state_notify( HWND hwnd, XPropertyEvent *event ) NtUserPostMessage( hwnd, WM_WINE_WINDOW_STATE_CHANGED, 0, 0 ); } +static void handle_monitor_notify( HWND hwnd, XPropertyEvent *event ) +{ + struct x11drv_win_data *data; + long indices[4] = {0}; + + if (!(data = get_win_data( hwnd ))) return; + if (event->state == PropertyNewValue) get_window_monitors( event->display, event->window, indices ); + window_monitors_notify( data, event->serial, indices ); + release_win_data( data ); +} + static void handle_wm_hints_notify( HWND hwnd, XPropertyEvent *event ) { struct x11drv_win_data *data; @@ -1350,6 +1361,7 @@ 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_WM_FULLSCREEN_MONITORS)) handle_monitor_notify( hwnd, event ); if (event->atom == x11drv_atom(WM_HINTS)) handle_wm_hints_notify( hwnd, event ); if (event->atom == x11drv_atom(_MOTIF_WM_HINTS)) handle_mwm_hints_notify( hwnd, event ); if (event->atom == x11drv_atom(WM_NORMAL_HINTS)) handle_wm_normal_hints_notify( hwnd, event ); diff --git a/dlls/winex11.drv/window.c b/dlls/winex11.drv/window.c index a83359b2d74..4090c2a9eb9 100644 --- a/dlls/winex11.drv/window.c +++ b/dlls/winex11.drv/window.c @@ -1271,7 +1271,7 @@ void window_set_user_time( struct x11drv_win_data *data, Time time, BOOL init ) 32, PropModeReplace, (unsigned char *)&time, 1 ); } -static void window_set_net_wm_fullscreen_monitors( struct x11drv_win_data *data, const struct monitor_indices *new_monitors ) +static void window_set_monitors( struct x11drv_win_data *data, const struct monitor_indices *new_monitors ) { const struct monitor_indices *old_monitors = &data->pending_state.monitors; data->desired_state.monitors = *new_monitors; @@ -1284,8 +1284,9 @@ static void window_set_net_wm_fullscreen_monitors( struct x11drv_win_data *data, if (data->pending_state.wm_state == WithdrawnState) { memcpy( &data->pending_state.monitors, new_monitors, sizeof(*new_monitors) ); - TRACE( "window %p/%lx, requesting _NET_WM_FULLSCREEN_MONITORS %s serial %lu\n", data->hwnd, data->whole_window, - debugstr_monitor_indices( new_monitors ), NextRequest( data->display ) ); + data->monitors_serial = NextRequest( data->display ); + TRACE( "window %p/%lx, requesting _NET_WM_FULLSCREEN_MONITORS %s serial %lu\n", data->hwnd, + data->whole_window, debugstr_monitor_indices( new_monitors ), data->monitors_serial ); XChangeProperty( data->display, data->whole_window, x11drv_atom(_NET_WM_FULLSCREEN_MONITORS), XA_CARDINAL, 32, PropModeReplace, (unsigned char *)new_monitors->indices, 4 ); } @@ -1304,19 +1305,17 @@ static void window_set_net_wm_fullscreen_monitors( struct x11drv_win_data *data, memcpy( xev.xclient.data.l, new_monitors->indices, sizeof(new_monitors->indices) ); memcpy( &data->pending_state.monitors, new_monitors, sizeof(*new_monitors) ); - TRACE( "window %p/%lx, requesting _NET_WM_FULLSCREEN_MONITORS %s serial %lu\n", data->hwnd, data->whole_window, - debugstr_monitor_indices( new_monitors ), NextRequest( data->display ) ); + data->monitors_serial = NextRequest( data->display ); + TRACE( "window %p/%lx, requesting _NET_WM_FULLSCREEN_MONITORS %s serial %lu\n", data->hwnd, + data->whole_window, debugstr_monitor_indices( new_monitors ), data->monitors_serial ); XSendEvent( data->display, DefaultRootWindow( data->display ), False, SubstructureRedirectMask | SubstructureNotifyMask, &xev ); } - - /* assume it changes immediately, we don't track the property for now */ - memcpy( &data->current_state.monitors, new_monitors, sizeof(*new_monitors) ); } /* Update _NET_WM_FULLSCREEN_MONITORS when _NET_WM_STATE_FULLSCREEN is set to support fullscreen * windows spanning multiple monitors */ -static void update_net_wm_fullscreen_monitors( struct x11drv_win_data *data ) +static void update_fullscreen_monitors( struct x11drv_win_data *data ) { struct monitor_indices monitors = {0}; @@ -1325,7 +1324,7 @@ static void update_net_wm_fullscreen_monitors( struct x11drv_win_data *data ) * indices because of stale xinerama monitor information */ if (!X11DRV_DisplayDevices_SupportEventHandlers()) return; if (!xinerama_get_fullscreen_monitors( &data->rects.visible, &monitors.generation, monitors.indices )) return; - window_set_net_wm_fullscreen_monitors( data, &monitors ); + window_set_monitors( data, &monitors ); } static void window_set_net_wm_state( struct x11drv_win_data *data, UINT new_state ) @@ -1512,7 +1511,7 @@ static void update_net_wm_states( struct x11drv_win_data *data ) } window_set_net_wm_state( data, new_state ); - update_net_wm_fullscreen_monitors( data ); + update_fullscreen_monitors( data ); } /*********************************************************************** @@ -1554,6 +1553,20 @@ UINT get_window_net_wm_state( Display *display, Window window ) return new_state; } +void get_window_monitors( Display *display, Window window, long *indices ) +{ + unsigned long count, remaining; + long *value; + int format; + Atom type; + + if (!XGetWindowProperty( display, window, x11drv_atom( _NET_WM_FULLSCREEN_MONITORS ), 0, 65536, False, + XA_CARDINAL, &type, &format, &count, &remaining, (unsigned char **)&value )) + { + if (type == XA_CARDINAL && format == 32) memcpy( indices, value, 4 * sizeof(indices) ); + XFree( value ); + } +} /*********************************************************************** * set_xembed_flags @@ -1602,7 +1615,7 @@ static void window_set_wm_state( struct x11drv_win_data *data, UINT new_state, B set_wm_hints( data ); update_net_wm_states( data ); sync_window_style( data ); - update_net_wm_fullscreen_monitors( data ); + update_fullscreen_monitors( data ); break; case MAKELONG(IconicState, NormalState): case MAKELONG(NormalState, IconicState): @@ -1849,7 +1862,7 @@ static void window_request_desired_state( struct x11drv_win_data *data ) { 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_net_wm_fullscreen_monitors( data, &data->desired_state.monitors ); + window_set_monitors( data, &data->desired_state.monitors ); window_set_mwm_hints( data, &data->desired_state.mwm_hints ); window_set_config( data, data->desired_state.rect, FALSE ); } @@ -1897,6 +1910,25 @@ void window_net_wm_state_notify( struct x11drv_win_data *data, unsigned long ser window_request_desired_state( data ); } +void window_monitors_notify( struct x11drv_win_data *data, unsigned long serial, const long *indices ) +{ + struct monitor_indices *desired = &data->desired_state.monitors, *pending = &data->pending_state.monitors, *current = &data->current_state.monitors; + const struct monitor_indices value = {pending->generation, {indices[0], indices[1], indices[2], indices[3]}}; + unsigned long *expect_serial = &data->monitors_serial; + const char *expected, *received, *prefix; + + prefix = wine_dbg_sprintf( "window %p/%lx ", data->hwnd, data->whole_window ); + received = wine_dbg_sprintf( "_NET_WM_FULLSCREEN_MONITORS %s/%lu", debugstr_monitor_indices( &value ), serial ); + expected = *expect_serial ? wine_dbg_sprintf( ", expected %s/%lu", debugstr_monitor_indices( pending ), *expect_serial ) : ""; + + 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_request_desired_state( data ); +} + void window_wm_hints_notify( struct x11drv_win_data *data, unsigned long serial, const XWMHints *value ) { XWMHints *desired = &data->desired_state.wm_hints, *pending = &data->pending_state.wm_hints, *current = &data->current_state.wm_hints; diff --git a/dlls/winex11.drv/x11drv.h b/dlls/winex11.drv/x11drv.h index 8b0c0a62f91..24d2d276eb1 100644 --- a/dlls/winex11.drv/x11drv.h +++ b/dlls/winex11.drv/x11drv.h @@ -690,6 +690,7 @@ struct x11drv_win_data struct window_state current_state; /* window state tracking the current X11 state */ unsigned long wm_state_serial; /* serial of last pending WM_STATE request */ unsigned long net_wm_state_serial; /* serial of last pending _NET_WM_STATE request */ + unsigned long monitors_serial; /* serial of last pending _NET_WM_FULLSCREEN_MONITORS request */ unsigned long wm_hints_serial; /* serial of last pending WM_HINTS request */ unsigned long mwm_hints_serial; /* serial of last pending _MOTIF_WM_HINTS request */ unsigned long wm_normal_hints_serial;/* serial of last pending WM_NORMAL_HINTS request */ @@ -708,6 +709,7 @@ extern BOOL window_should_take_focus( HWND hwnd, 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, Time time ); extern void window_net_wm_state_notify( struct x11drv_win_data *data, unsigned long serial, UINT value ); +extern void window_monitors_notify( struct x11drv_win_data *data, unsigned long serial, const long *value ); extern void window_wm_hints_notify( struct x11drv_win_data *data, unsigned long serial, const XWMHints *hints ); extern void window_mwm_hints_notify( struct x11drv_win_data *data, unsigned long serial, const MwmHints *hints ); extern void window_wm_normal_hints_notify( struct x11drv_win_data *data, unsigned long serial, const XSizeHints *hints ); @@ -723,6 +725,7 @@ extern BOOL is_net_supported( Atom atom ); extern Window init_clip_window(void); extern void window_set_user_time( struct x11drv_win_data *data, Time time, BOOL init ); extern UINT get_window_net_wm_state( Display *display, Window window ); +extern void get_window_monitors( Display *display, Window window, long *indices ); extern void make_window_embedded( struct x11drv_win_data *data ); extern Window create_client_window( HWND hwnd, RECT client_rect, const XVisualInfo *visual, Colormap colormap ); extern void detach_client_window( struct x11drv_win_data *data, Window client_window ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10620
From: Rémi Bernon <rbernon@codeweavers.com> --- dlls/winex11.drv/window.c | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/dlls/winex11.drv/window.c b/dlls/winex11.drv/window.c index 4090c2a9eb9..0ca6066b3d7 100644 --- a/dlls/winex11.drv/window.c +++ b/dlls/winex11.drv/window.c @@ -1271,12 +1271,13 @@ void window_set_user_time( struct x11drv_win_data *data, Time time, BOOL init ) 32, PropModeReplace, (unsigned char *)&time, 1 ); } -static void window_set_monitors( struct x11drv_win_data *data, const struct monitor_indices *new_monitors ) +static void window_set_monitors( struct x11drv_win_data *data, const struct monitor_indices *new_monitors, BOOL force ) { + BOOL is_fullscreen = data->pending_state.net_wm_state & (1 << NET_WM_STATE_FULLSCREEN); const struct monitor_indices *old_monitors = &data->pending_state.monitors; data->desired_state.monitors = *new_monitors; - if (!(data->pending_state.net_wm_state & (1 << NET_WM_STATE_FULLSCREEN)) || is_virtual_desktop()) return; /* window isn't fullscreen, delay updating */ + if (!(force || is_fullscreen) || is_virtual_desktop()) return; /* window isn't fullscreen, delay updating */ if (!data->whole_window || !data->managed || data->embedded) return; /* no window or not managed, nothing to update */ if (!memcmp( old_monitors, new_monitors, sizeof(*new_monitors) )) return; /* states are the same, nothing to update */ @@ -1315,7 +1316,7 @@ static void window_set_monitors( struct x11drv_win_data *data, const struct moni /* Update _NET_WM_FULLSCREEN_MONITORS when _NET_WM_STATE_FULLSCREEN is set to support fullscreen * windows spanning multiple monitors */ -static void update_fullscreen_monitors( struct x11drv_win_data *data ) +static void update_fullscreen_monitors( struct x11drv_win_data *data, BOOL force ) { struct monitor_indices monitors = {0}; @@ -1324,7 +1325,7 @@ static void update_fullscreen_monitors( struct x11drv_win_data *data ) * indices because of stale xinerama monitor information */ if (!X11DRV_DisplayDevices_SupportEventHandlers()) return; if (!xinerama_get_fullscreen_monitors( &data->rects.visible, &monitors.generation, monitors.indices )) return; - window_set_monitors( data, &monitors ); + window_set_monitors( data, &monitors, force ); } static void window_set_net_wm_state( struct x11drv_win_data *data, UINT new_state ) @@ -1388,6 +1389,13 @@ 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); + /* update _NET_WM_FULLSCREEN_MONITORS before requesting _NET_WM_STATE when becoming fullscreen */ + if (i == NET_WM_STATE_FULLSCREEN && xev.xclient.data.l[0] == _NET_WM_STATE_ADD) + { + WARN( "window %p/%lx becomes fullscreen, updating _NET_WM_STATE_FULLSCREEN_MONITORS\n", data->hwnd, data->whole_window ); + update_fullscreen_monitors( data, TRUE ); + } + 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, @@ -1511,7 +1519,7 @@ static void update_net_wm_states( struct x11drv_win_data *data ) } window_set_net_wm_state( data, new_state ); - update_fullscreen_monitors( data ); + update_fullscreen_monitors( data, FALSE ); } /*********************************************************************** @@ -1615,7 +1623,7 @@ static void window_set_wm_state( struct x11drv_win_data *data, UINT new_state, B set_wm_hints( data ); update_net_wm_states( data ); sync_window_style( data ); - update_fullscreen_monitors( data ); + update_fullscreen_monitors( data, FALSE ); break; case MAKELONG(IconicState, NormalState): case MAKELONG(NormalState, IconicState): @@ -1862,7 +1870,7 @@ static void window_request_desired_state( struct x11drv_win_data *data ) { 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_monitors( data, &data->desired_state.monitors ); + window_set_monitors( data, &data->desired_state.monitors, FALSE ); window_set_mwm_hints( data, &data->desired_state.mwm_hints ); window_set_config( data, data->desired_state.rect, FALSE ); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10620
From: Rémi Bernon <rbernon@codeweavers.com> When window has the _NET_WM_STATE_FULLSCREEN bit set. --- dlls/winex11.drv/window.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/dlls/winex11.drv/window.c b/dlls/winex11.drv/window.c index 0ca6066b3d7..67959868d33 100644 --- a/dlls/winex11.drv/window.c +++ b/dlls/winex11.drv/window.c @@ -1295,6 +1295,12 @@ static void window_set_monitors( struct x11drv_win_data *data, const struct moni { XEvent xev; + if (is_fullscreen && (data->configure_serial || window_needs_config_change_delay( data ))) + { + TRACE( "window %p/%lx is updating or delaying config requests, delaying monitors update\n", data->hwnd, data->whole_window ); + return; /* another window config change is pending or delayed, wait for it to complete */ + } + xev.xclient.type = ClientMessage; xev.xclient.window = data->whole_window; xev.xclient.message_type = x11drv_atom(_NET_WM_FULLSCREEN_MONITORS); @@ -1311,6 +1317,14 @@ static void window_set_monitors( struct x11drv_win_data *data, const struct moni data->whole_window, debugstr_monitor_indices( new_monitors ), data->monitors_serial ); XSendEvent( data->display, DefaultRootWindow( data->display ), False, SubstructureRedirectMask | SubstructureNotifyMask, &xev ); + + if (is_fullscreen) + { + data->configure_serial = data->monitors_serial; + data->pending_state.rect = data->rects.visible; + TRACE( "window %p/%lx changes fullscreen monitors, expecting config %s, serial %lu\n", data->hwnd, + data->whole_window, wine_dbgstr_rect( &data->pending_state.rect ), data->configure_serial ); + } } } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10620
Zhiyi Zhang (@zhiyi) commented about dlls/winex11.drv/window.c:
return new_state; }
+void get_window_monitors( Display *display, Window window, long *indices ) +{ + unsigned long count, remaining; + long *value; + int format; + Atom type; + + if (!XGetWindowProperty( display, window, x11drv_atom( _NET_WM_FULLSCREEN_MONITORS ), 0, 65536, False, + XA_CARDINAL, &type, &format, &count, &remaining, (unsigned char **)&value )) + { + if (type == XA_CARDINAL && format == 32) memcpy( indices, value, 4 * sizeof(indices) );
Should be `4 * sizeof(*indices)` -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10620#note_135897
Zhiyi Zhang (@zhiyi) commented about dlls/winex11.drv/window.c:
{ XEvent xev;
+ if (is_fullscreen && (data->configure_serial || window_needs_config_change_delay( data ))) + { + TRACE( "window %p/%lx is updating or delaying config requests, delaying monitors update\n", data->hwnd, data->whole_window ); + return; /* another window config change is pending or delayed, wait for it to complete */ + }
I think there is a slight chance that we can get here with the same monitor indices. A ConfigureNotify might not be sent in that case. For example, xrandr14_device_change_handler() changes the monitor generation without changing the monitor count. Then we might be waiting for a ConfigureNotify that never comes. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10620#note_135898
On Sun Apr 12 09:37:14 2026 +0000, Zhiyi Zhang wrote:
I think there is a slight chance that we can get here with the same monitor indices. A ConfigureNotify might not be sent in that case. For example, xrandr14_device_change_handler() changes the monitor generation without changing the monitor count. Then we might be waiting for a ConfigureNotify that never comes. Hmm... in theory this might happen even if monitor indices have changed, for instance if somehow monitor location are swapped and window end up at the exact same location, but maybe we can be optimistic? We could decide to stop serializing when the _NET_WM_FULLSCREEN_MONITORS property update is received [^1], but there might still already be some ConfigureNotify event in flight that we will receive shortly after and that might confuse us, though hopefully it'll match our expected window position and should then be ignored.
We could also check whether the monitor indices have changed or not, and only expect a reconfigure when they did, but then it also raises the question to why we need to update the monitor indices in that case. It seems to me that this is something the WM should implement: if the monitor indices are the same there should be no need for applications to update the property again. The XRandR monitors mapped to these indices might change when displays are rearranged, but the WM should keep the window mapped to the monitor matching the indices set by the window at all time, and eventually move it around on its own if necessary. In general I feel that this can only work when XRandR monitors don't actually change (ie: with modesetting emulation), as handling actual monitor rearrangement reliably would require many more race conditions that aren't mitigated right now. For instance we would need to make sure we wait for actual monitor placement update before moving our windows, as we might otherwise not have the Win32 monitor correctly positioned and might expect incorrect window configs. [^1]: I decided to reuse the window config serialization in order to avoid introducing a fourth dimension to the already complicated ConfigureNotify serialization logic (in addition to window configure requests, _NET_WM_STATE induced configs and _MOTIF_WM_HINTS induced configs). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10620#note_135932
participants (3)
-
Rémi Bernon -
Rémi Bernon (＠rbernon)
-
Zhiyi Zhang (＠zhiyi)