[PATCH 0/1] MR7629: winex11: Use -1 as fullscreen monitor indices to clear the property.
This property is the only way to move fullscreen windows across monitors with KWin, it needs to be set to avoid having to mess with _NET_WM_STATE workarounds. Both KWin and Mutter support cases with invalid indices and treat them mostly the same as clearing the property. For Mutter, see: * meta_x11_display_xinerama_index_to_logical_monitor[1] * meta_window_update_fullscreen_monitors[2] * meta_window_clear_fullscreen_monitors[3] For KWin, see: * NETFullscreenMonitors structure from KWindowSystem[4], * xineramaIndexToOutput[5] * fullscreenMonitorArea[6] Mostly reverts 70c9239cb2eef696eea109f3f8e7a58f80cd3823. [1] https://gitlab.gnome.org/GNOME/mutter/-/blob/9a1fa7e13b5feefb76902287ae7ca25... [2] https://gitlab.gnome.org/GNOME/mutter/-/blob/9a1fa7e13b5feefb76902287ae7ca25... [3] https://gitlab.gnome.org/GNOME/mutter/-/blob/9a1fa7e13b5feefb76902287ae7ca25... [4] https://github.com/KDE/kwindowsystem/blob/1399ec9c2f05107c36bc8661593df35bad... [5] https://invent.kde.org/plasma/kwin/-/blob/b488f3b6c6200aca9fa1f61cbf0253eeda... [6] https://invent.kde.org/plasma/kwin/-/blob/b488f3b6c6200aca9fa1f61cbf0253eeda... -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7629
From: Rémi Bernon <rbernon(a)codeweavers.com> This property is the only way to move fullscreen windows across monitors with KWin, it needs to be set to avoid having to mess with _NET_WM_STATE workarounds. Both KWin and Mutter support cases with invalid indices and treat them mostly the same as clearing the property. For Mutter, see: * meta_x11_display_xinerama_index_to_logical_monitor[1] * meta_window_update_fullscreen_monitors[2] * meta_window_clear_fullscreen_monitors[3] For KWin, see: * NETFullscreenMonitors structure from KWindowSystem[4], * xineramaIndexToOutput[5] * fullscreenMonitorArea[6] Mostly reverts 70c9239cb2eef696eea109f3f8e7a58f80cd3823. [1] https://gitlab.gnome.org/GNOME/mutter/-/blob/9a1fa7e13b5feefb76902287ae7ca25... [2] https://gitlab.gnome.org/GNOME/mutter/-/blob/9a1fa7e13b5feefb76902287ae7ca25... [3] https://gitlab.gnome.org/GNOME/mutter/-/blob/9a1fa7e13b5feefb76902287ae7ca25... [4] https://github.com/KDE/kwindowsystem/blob/1399ec9c2f05107c36bc8661593df35bad... [5] https://invent.kde.org/plasma/kwin/-/blob/b488f3b6c6200aca9fa1f61cbf0253eeda... [6] https://invent.kde.org/plasma/kwin/-/blob/b488f3b6c6200aca9fa1f61cbf0253eeda... --- dlls/winex11.drv/window.c | 30 ++++++++++-------------------- dlls/winex11.drv/x11drv.h | 3 +-- dlls/winex11.drv/xinerama.c | 30 ++++++++---------------------- 3 files changed, 19 insertions(+), 44 deletions(-) diff --git a/dlls/winex11.drv/window.c b/dlls/winex11.drv/window.c index f70e74296cd..0e617d41316 100644 --- a/dlls/winex11.drv/window.c +++ b/dlls/winex11.drv/window.c @@ -1168,27 +1168,15 @@ static void update_net_wm_fullscreen_monitors( struct x11drv_win_data *data ) if (!X11DRV_DisplayDevices_SupportEventHandlers()) return; - if (!xinerama_get_fullscreen_monitors( &data->rects.visible, monitors )) - return; - - /* If _NET_WM_FULLSCREEN_MONITORS is not set and the fullscreen monitors are spanning only one - * monitor then do not set _NET_WM_FULLSCREEN_MONITORS. - * - * If _NET_WM_FULLSCREEN_MONITORS is set then the property needs to be updated because it can't - * be deleted by sending a _NET_WM_FULLSCREEN_MONITORS client message to the root window - * according to the wm-spec version 1.5. Having the window spanning more than two monitors also - * needs the property set. In other cases, _NET_WM_FULLSCREEN_MONITORS doesn't need to be set. - * What's more, setting _NET_WM_FULLSCREEN_MONITORS adds a constraint on Mutter so that such a - * window can't be moved to another monitor by using the Shift+Super+Up/Down/Left/Right - * shortcut. So the property should be added only when necessary. */ - if (monitors[0] == monitors[1] && monitors[1] == monitors[2] && monitors[2] == monitors[3] - && !data->net_wm_fullscreen_monitors_set) - return; + xinerama_get_fullscreen_monitors( &data->rects.visible, monitors ); if (data->pending_state.wm_state == WithdrawnState) { - XChangeProperty( data->display, data->whole_window, x11drv_atom(_NET_WM_FULLSCREEN_MONITORS), - XA_CARDINAL, 32, PropModeReplace, (unsigned char *)monitors, 4 ); + TRACE( "window %p/%lx, requesting _NET_WM_FULLSCREEN_MONITORS %ld,%ld,%ld,%ld serial %lu\n", data->hwnd, data->whole_window, + monitors[0], monitors[1], monitors[2], monitors[3], NextRequest( data->display ) ); + if (monitors[0] == -1) XDeleteProperty( data->display, data->whole_window, x11drv_atom(_NET_WM_FULLSCREEN_MONITORS) ); + else XChangeProperty( data->display, data->whole_window, x11drv_atom(_NET_WM_FULLSCREEN_MONITORS), + XA_CARDINAL, 32, PropModeReplace, (unsigned char *)monitors, 4 ); } else { @@ -1201,10 +1189,12 @@ static void update_net_wm_fullscreen_monitors( struct x11drv_win_data *data ) xev.xclient.format = 32; xev.xclient.data.l[4] = 1; memcpy( xev.xclient.data.l, monitors, sizeof(monitors) ); - XSendEvent( data->display, root_window, False, + + TRACE( "window %p/%lx, requesting _NET_WM_FULLSCREEN_MONITORS %ld,%ld,%ld,%ld serial %lu\n", data->hwnd, data->whole_window, + monitors[0], monitors[1], monitors[2], monitors[3], NextRequest( data->display ) ); + XSendEvent( data->display, DefaultRootWindow( data->display ), False, SubstructureRedirectMask | SubstructureNotifyMask, &xev ); } - data->net_wm_fullscreen_monitors_set = TRUE; } static void window_set_net_wm_state( struct x11drv_win_data *data, UINT new_state ) diff --git a/dlls/winex11.drv/x11drv.h b/dlls/winex11.drv/x11drv.h index a5bab864ad9..b6255c2a880 100644 --- a/dlls/winex11.drv/x11drv.h +++ b/dlls/winex11.drv/x11drv.h @@ -633,7 +633,6 @@ struct x11drv_win_data UINT use_alpha : 1; /* does window use an alpha channel? */ UINT skip_taskbar : 1; /* does window should be deleted from taskbar */ UINT add_taskbar : 1; /* does window should be added to taskbar regardless of style */ - UINT net_wm_fullscreen_monitors_set : 1; /* is _NET_WM_FULLSCREEN_MONITORS set */ 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 */ @@ -716,7 +715,7 @@ extern POINT virtual_screen_to_root( INT x, INT y ); extern POINT root_to_virtual_screen( INT x, INT y ); extern RECT get_host_primary_monitor_rect(void); extern RECT get_work_area( const RECT *monitor_rect ); -extern BOOL xinerama_get_fullscreen_monitors( const RECT *rect, long *indices ); +extern void xinerama_get_fullscreen_monitors( const RECT *rect, long *indices ); extern void xinerama_init( unsigned int width, unsigned int height ); extern void init_recursive_mutex( pthread_mutex_t *mutex ); diff --git a/dlls/winex11.drv/xinerama.c b/dlls/winex11.drv/xinerama.c index 1fd170b0e07..096bd95f04e 100644 --- a/dlls/winex11.drv/xinerama.c +++ b/dlls/winex11.drv/xinerama.c @@ -124,10 +124,9 @@ static inline int query_screens(void) #endif /* SONAME_LIBXINERAMA */ /* Get xinerama monitor indices required for _NET_WM_FULLSCREEN_MONITORS */ -BOOL xinerama_get_fullscreen_monitors( const RECT *rect, long *indices ) +void xinerama_get_fullscreen_monitors( const RECT *rect, long *indices ) { RECT window_rect, intersected_rect, monitor_rect; - BOOL ret = FALSE; POINT offset; INT i; @@ -135,7 +134,6 @@ BOOL xinerama_get_fullscreen_monitors( const RECT *rect, long *indices ) if (nb_monitors == 1) { memset( indices, 0, sizeof(*indices) * 4 ); - ret = TRUE; goto done; } @@ -155,10 +153,7 @@ BOOL xinerama_get_fullscreen_monitors( const RECT *rect, long *indices ) offset.y = min( offset.y, monitors[i].rcMonitor.top ); } - indices[0] = -1; - indices[1] = -1; - indices[2] = -1; - indices[3] = -1; + indices[0] = indices[1] = indices[2] = indices[3] = -1; for (i = 0; i < nb_monitors; ++i) { SetRect( &monitor_rect, monitors[i].rcMonitor.left - offset.x, @@ -167,27 +162,18 @@ BOOL xinerama_get_fullscreen_monitors( const RECT *rect, long *indices ) intersect_rect( &intersected_rect, &window_rect, &monitor_rect ); if (EqualRect( &intersected_rect, &monitor_rect )) { - if (indices[0] == -1 || monitors[i].rcMonitor.top < monitors[indices[0]].rcMonitor.top) - indices[0] = i; - if (indices[1] == -1 || monitors[i].rcMonitor.bottom > monitors[indices[1]].rcMonitor.bottom) - indices[1] = i; - if (indices[2] == -1 || monitors[i].rcMonitor.left < monitors[indices[2]].rcMonitor.left) - indices[2] = i; - if (indices[3] == -1 || monitors[i].rcMonitor.right > monitors[indices[3]].rcMonitor.right) - indices[3] = i; + if (indices[0] == -1) indices[0] = indices[1] = indices[2] = indices[3] = i; + if (monitors[i].rcMonitor.top < monitors[indices[0]].rcMonitor.top) indices[0] = i; + if (monitors[i].rcMonitor.bottom > monitors[indices[1]].rcMonitor.bottom) indices[1] = i; + if (monitors[i].rcMonitor.left < monitors[indices[2]].rcMonitor.left) indices[2] = i; + if (monitors[i].rcMonitor.right > monitors[indices[3]].rcMonitor.right) indices[3] = i; } } - if (indices[0] == -1 || indices[1] == -1 || indices[2] == -1 || indices[3] == -1) - ERR("Failed to get xinerama fullscreen monitor indices.\n"); - else - ret = TRUE; + if (indices[0] == -1) WARN("Failed to get xinerama fullscreen monitor indices.\n"); done: pthread_mutex_unlock( &xinerama_mutex ); - if (ret) - TRACE( "fullscreen monitors: %ld,%ld,%ld,%ld.\n", indices[0], indices[1], indices[2], indices[3] ); - return ret; } static BOOL xinerama_get_gpus( struct x11drv_gpu **new_gpus, int *count, BOOL get_properties ) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7629
Actually not sure what happened with the tests here, maybe just a fluke... I can't reproduce failures locally. I don't see how this could change anything with fvwm as it simply doesn't support that property at all. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7629#note_98533
Zhiyi Zhang (@zhiyi) commented about dlls/winex11.drv/window.c:
if (!X11DRV_DisplayDevices_SupportEventHandlers()) return;
- if (!xinerama_get_fullscreen_monitors( &data->rects.visible, monitors )) - return; - - /* If _NET_WM_FULLSCREEN_MONITORS is not set and the fullscreen monitors are spanning only one - * monitor then do not set _NET_WM_FULLSCREEN_MONITORS. If _NET_WM_FULLSCREEN_MONITORS is not set and the fullscreen monitors are spanning only one monitor then do not set _NET_WM_FULLSCREEN_MONITORS.
And from 70c9239c
What's more, setting _NET_WM_FULLSCREEN_MONITORS adds a constraint on Mutter so that such a window can't be moved to another monitor by using the Shift+Super+Up/Down/Left/Right shortcut. So the property should be added only when necessary.
So this brings back the regression? Because now _NET_WM_FULLSCREEN_MONITORS is set even when the window only covers one monitor. I don't immediately know what the correct resolution is. Perhaps setting _NET_WM_FULLSCREEN_MONITORS and then deleting/resetting it? Would that work? Also according to wm-spec version 1.5, there is no way to delete this. This -1 special value thing might be only available to Mutter and KWin. So we should probably avoid setting _NET_WM_FULLSCREEN_MONITORS unless necessary. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7629#note_98585
On Fri Mar 21 08:22:28 2025 +0000, Zhiyi Zhang wrote:
If _NET_WM_FULLSCREEN_MONITORS is not set and the fullscreen monitors are spanning only one monitor then do not set _NET_WM_FULLSCREEN_MONITORS. And from 70c9239c What's more, setting _NET_WM_FULLSCREEN_MONITORS adds a constraint on Mutter so that such a window can't be moved to another monitor by using the Shift+Super+Up/Down/Left/Right shortcut. So the property should be added only when necessary. So this brings back the regression? Because now _NET_WM_FULLSCREEN_MONITORS is set even when the window only covers one monitor. I don't immediately know what the correct resolution is. Perhaps setting _NET_WM_FULLSCREEN_MONITORS and then deleting/resetting it? Would that work? Also according to wm-spec version 1.5, there is no way to delete this. This -1 special value thing might be only available to Mutter and KWin. So we should probably avoid setting _NET_WM_FULLSCREEN_MONITORS unless necessary. I also created https://gitlab.gnome.org/GNOME/mutter/-/issues/2967 years ago to allow Shift+Super+Up/Down/Left/Right when _NET_WM_FULLSCREEN_MONITORS is set but haven't heard anything.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7629#note_98586
On Fri Mar 21 08:27:07 2025 +0000, Zhiyi Zhang wrote:
I also created https://gitlab.gnome.org/GNOME/mutter/-/issues/2967 years ago to allow Shift+Super+Up/Down/Left/Right when _NET_WM_FULLSCREEN_MONITORS is set but haven't heard anything. This looks like a Mutter bug that they need to fix. Moreover, I'm not even sure that improving this use case is even worth it, it would require applications to be ready to receive fullscreen window position changes when it doesn't seem to be something that Windows can even do.
The other use case, which we actually need to support, is for applications moving their own windows to a different monitor and I think we have more issues with this than issues with fullscreen windows not being movable by the user: if the keyboard shortcut doesn't work, there's still other ways to move the window. In any case, clearing the fullscreen monitors doesn't seem to be really needed and probably won't even happen: this is called only for fullscreen windows and they cover one of the monitors. I checked that it appears to be supported by Mutter/KWin because it was described as a reason not to set the property, but I don't think it's ever going to happen anyway. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7629#note_98587
This merge request was approved by Zhiyi Zhang. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7629
participants (2)
-
Rémi Bernon -
Zhiyi Zhang (＠zhiyi)