[PATCH v2 0/7] MR4014: winewayland.drv: part 8: More window management
This MR improves window management related behavior and adds support for more scenarios. Some highlights (please see the full commit list and messages for more info): 1. Better support for tiled states. 2. Detect and handle fullscreen windows. 3. Increased robustness against state disagreements between Wine and the Wayland compositor. 4. Move top-level windows to (0,0) (using that fixed point for now, we will later generalize this mechanism for multiple monitors), in order to: a. maximize the accessible mouse input region (which may be clipped in Windows virtual screen space, but still accessible in Wayland space). b. introduce the basic mechanism to allow windows applications to (very roughly!) track which output(s) they are really on (to be continued in the full multi-monitor form). Although (4) is not a perfect solution for the lack of absolute positioning, it has worked well in practice in its full, multi-monitor implementation (i.e., in the experimental branch). I wonder if for (4a) in particular we can do better, e.g., by being able to emit input that circumvents the normal virtual screen clipping. Is there a way to do this already? If not, do you think that such an approach would be a feasible and acceptable way forward, or is virtual screen input clipping fundamentally baked in the current core design and assumptions? Thanks! -- v2: winewayland.drv: Avoid resizing fullscreen windows. winewayland.drv: Rename wayland_surface_configure_is_compatible for consistency. winewayland.drv: Use surface geometry to satisfy state size constraints. winewayland.drv: Handle application-initiated fullscreen state. https://gitlab.winehq.org/wine/wine/-/merge_requests/4014
From: Alexandros Frantzis <alexandros.frantzis(a)collabora.com> If we are processing a config request by the compositor, consider that config as authoritative for the window. Otherwise use the window state to determine the new Wayland state to apply. --- dlls/winewayland.drv/window.c | 34 ++++++++++++++++++++++------------ 1 file changed, 22 insertions(+), 12 deletions(-) diff --git a/dlls/winewayland.drv/window.c b/dlls/winewayland.drv/window.c index 5079d820929..09a4125ff22 100644 --- a/dlls/winewayland.drv/window.c +++ b/dlls/winewayland.drv/window.c @@ -196,7 +196,7 @@ static void wayland_win_data_update_wayland_state(struct wayland_win_data *data) { struct wayland_surface *surface = data->wayland_surface; uint32_t window_state; - struct wayland_surface_config *conf; + BOOL processing_config; pthread_mutex_lock(&surface->mutex); @@ -206,24 +206,34 @@ static void wayland_win_data_update_wayland_state(struct wayland_win_data *data) (NtUserGetWindowLongW(surface->hwnd, GWL_STYLE) & WS_MAXIMIZE) ? WAYLAND_SURFACE_CONFIG_STATE_MAXIMIZED : 0; - conf = surface->processing.serial ? &surface->processing : &surface->current; + processing_config = surface->processing.serial && + !surface->processing.processed; - TRACE("hwnd=%p window_state=%#x conf->state=%#x\n", - data->hwnd, window_state, conf->state); + TRACE("hwnd=%p window_state=%#x %s->state=%#x\n", + data->hwnd, window_state, + processing_config ? "processing" : "current", + processing_config ? surface->processing.state : surface->current.state); - if ((window_state & WAYLAND_SURFACE_CONFIG_STATE_MAXIMIZED) && - !(conf->state & WAYLAND_SURFACE_CONFIG_STATE_MAXIMIZED)) + /* If we are not processing a compositor requested config, use the + * window state to determine and update the Wayland state. */ + if (!processing_config) { - xdg_toplevel_set_maximized(surface->xdg_toplevel); + if ((window_state & WAYLAND_SURFACE_CONFIG_STATE_MAXIMIZED) && + !(surface->current.state & WAYLAND_SURFACE_CONFIG_STATE_MAXIMIZED)) + { + xdg_toplevel_set_maximized(surface->xdg_toplevel); + } + else if (!(window_state & WAYLAND_SURFACE_CONFIG_STATE_MAXIMIZED) && + (surface->current.state & WAYLAND_SURFACE_CONFIG_STATE_MAXIMIZED)) + { + xdg_toplevel_unset_maximized(surface->xdg_toplevel); + } } - else if (!(window_state & WAYLAND_SURFACE_CONFIG_STATE_MAXIMIZED) && - (conf->state & WAYLAND_SURFACE_CONFIG_STATE_MAXIMIZED)) + else { - xdg_toplevel_unset_maximized(surface->xdg_toplevel); + surface->processing.processed = TRUE; } - conf->processed = TRUE; - out: pthread_mutex_unlock(&surface->mutex); wl_display_flush(process_wayland.wl_display); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4014
From: Alexandros Frantzis <alexandros.frantzis(a)collabora.com> Tiled states don't place strict constraints on the surface size, but they indicate a strong size preference, so try to respect it. --- dlls/winewayland.drv/wayland_surface.c | 6 ++++++ dlls/winewayland.drv/waylanddrv.h | 3 ++- dlls/winewayland.drv/window.c | 9 +++++++-- 3 files changed, 15 insertions(+), 3 deletions(-) diff --git a/dlls/winewayland.drv/wayland_surface.c b/dlls/winewayland.drv/wayland_surface.c index 8551ad98eeb..ddb10be6e06 100644 --- a/dlls/winewayland.drv/wayland_surface.c +++ b/dlls/winewayland.drv/wayland_surface.c @@ -91,6 +91,12 @@ static void xdg_toplevel_handle_configure(void *data, case XDG_TOPLEVEL_STATE_RESIZING: config_state |= WAYLAND_SURFACE_CONFIG_STATE_RESIZING; break; + case XDG_TOPLEVEL_STATE_TILED_LEFT: + case XDG_TOPLEVEL_STATE_TILED_RIGHT: + case XDG_TOPLEVEL_STATE_TILED_TOP: + case XDG_TOPLEVEL_STATE_TILED_BOTTOM: + config_state |= WAYLAND_SURFACE_CONFIG_STATE_TILED; + break; default: break; } diff --git a/dlls/winewayland.drv/waylanddrv.h b/dlls/winewayland.drv/waylanddrv.h index faff529940d..fcfc187925d 100644 --- a/dlls/winewayland.drv/waylanddrv.h +++ b/dlls/winewayland.drv/waylanddrv.h @@ -60,7 +60,8 @@ enum wayland_window_message enum wayland_surface_config_state { WAYLAND_SURFACE_CONFIG_STATE_MAXIMIZED = (1 << 0), - WAYLAND_SURFACE_CONFIG_STATE_RESIZING = (1 << 1) + WAYLAND_SURFACE_CONFIG_STATE_RESIZING = (1 << 1), + WAYLAND_SURFACE_CONFIG_STATE_TILED = (1 << 2) }; struct wayland_cursor diff --git a/dlls/winewayland.drv/window.c b/dlls/winewayland.drv/window.c index 09a4125ff22..23f4c89ea0b 100644 --- a/dlls/winewayland.drv/window.c +++ b/dlls/winewayland.drv/window.c @@ -401,8 +401,13 @@ static void wayland_configure_window(HWND hwnd) } /* The Wayland maximized state is very strict about surface size, so don't - * let the application override it. */ - if (state & WAYLAND_SURFACE_CONFIG_STATE_MAXIMIZED) flags |= SWP_NOSENDCHANGING; + * let the application override it. The tiled state is not as strict, + * but it indicates a strong size preference, so try to respect it. */ + if (state & (WAYLAND_SURFACE_CONFIG_STATE_MAXIMIZED | + WAYLAND_SURFACE_CONFIG_STATE_TILED)) + { + flags |= SWP_NOSENDCHANGING; + } NtUserSetWindowPos(hwnd, 0, 0, 0, width, height, flags); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4014
From: Alexandros Frantzis <alexandros.frantzis(a)collabora.com> Instead of querying the window config for a wayland_surface on demand, query it and store it only when it is applied through SetWindowPos. In addition to being more efficient, this prevents deadlocks by avoiding calling NtUser functions that require the user lock, while holding the window_surface lock. --- dlls/winewayland.drv/wayland_surface.c | 20 ++++++----------- dlls/winewayland.drv/waylanddrv.h | 7 ++++++ dlls/winewayland.drv/window.c | 30 +++++++++++++++----------- 3 files changed, 32 insertions(+), 25 deletions(-) diff --git a/dlls/winewayland.drv/wayland_surface.c b/dlls/winewayland.drv/wayland_surface.c index ddb10be6e06..5bb8c959537 100644 --- a/dlls/winewayland.drv/wayland_surface.c +++ b/dlls/winewayland.drv/wayland_surface.c @@ -349,22 +349,16 @@ static BOOL wayland_surface_configure_is_compatible(struct wayland_surface_confi */ BOOL wayland_surface_reconfigure(struct wayland_surface *surface) { - RECT window_rect; + struct wayland_window_config *window = &surface->window; int width, height; - enum wayland_surface_config_state window_state; if (!surface->xdg_toplevel) return TRUE; - if (!NtUserGetWindowRect(surface->hwnd, &window_rect)) return FALSE; - width = window_rect.right - window_rect.left; - height = window_rect.bottom - window_rect.top; - - window_state = - (NtUserGetWindowLongW(surface->hwnd, GWL_STYLE) & WS_MAXIMIZE) ? - WAYLAND_SURFACE_CONFIG_STATE_MAXIMIZED : 0; + width = surface->window.rect.right - surface->window.rect.left; + height = surface->window.rect.bottom - surface->window.rect.top; TRACE("hwnd=%p window=%dx%d,%#x processing=%dx%d,%#x current=%dx%d,%#x\n", - surface->hwnd, width, height, window_state, + surface->hwnd, width, height, window->state, surface->processing.width, surface->processing.height, surface->processing.state, surface->current.width, surface->current.height, surface->current.state); @@ -373,7 +367,7 @@ BOOL wayland_surface_reconfigure(struct wayland_surface *surface) if (surface->processing.serial && surface->processing.processed && wayland_surface_configure_is_compatible(&surface->processing, width, height, - window_state)) + window->state)) { surface->current = surface->processing; memset(&surface->processing, 0, sizeof(surface->processing)); @@ -385,7 +379,7 @@ BOOL wayland_surface_reconfigure(struct wayland_surface *surface) else if (!surface->current.serial && surface->requested.serial && wayland_surface_configure_is_compatible(&surface->requested, width, height, - window_state)) + window->state)) { surface->current = surface->requested; memset(&surface->requested, 0, sizeof(surface->requested)); @@ -394,7 +388,7 @@ BOOL wayland_surface_reconfigure(struct wayland_surface *surface) else if (!surface->current.serial || !wayland_surface_configure_is_compatible(&surface->current, width, height, - window_state)) + window->state)) { return FALSE; } diff --git a/dlls/winewayland.drv/waylanddrv.h b/dlls/winewayland.drv/waylanddrv.h index fcfc187925d..4b74b674ea6 100644 --- a/dlls/winewayland.drv/waylanddrv.h +++ b/dlls/winewayland.drv/waylanddrv.h @@ -141,6 +141,12 @@ struct wayland_surface_config BOOL processed; }; +struct wayland_window_config +{ + RECT rect; + enum wayland_surface_config_state state; +}; + struct wayland_surface { HWND hwnd; @@ -151,6 +157,7 @@ struct wayland_surface struct wayland_surface_config pending, requested, processing, current; struct wayland_shm_buffer *latest_window_buffer; BOOL resizing; + struct wayland_window_config window; }; struct wayland_shm_buffer diff --git a/dlls/winewayland.drv/window.c b/dlls/winewayland.drv/window.c index 23f4c89ea0b..bf1a286b17d 100644 --- a/dlls/winewayland.drv/window.c +++ b/dlls/winewayland.drv/window.c @@ -146,6 +146,14 @@ static void wayland_win_data_release(struct wayland_win_data *data) pthread_mutex_unlock(&win_data_mutex); } +static BOOL wayland_window_get_config(HWND hwnd, struct wayland_window_config *conf) +{ + if (!NtUserGetWindowRect(hwnd, &conf->rect)) return FALSE; + conf->state = (NtUserGetWindowLongW(hwnd, GWL_STYLE) & WS_MAXIMIZE) ? + WAYLAND_SURFACE_CONFIG_STATE_MAXIMIZED : 0; + return TRUE; +} + static void wayland_win_data_update_wayland_surface(struct wayland_win_data *data) { struct wayland_surface *surface = data->wayland_surface; @@ -170,20 +178,23 @@ static void wayland_win_data_update_wayland_surface(struct wayland_win_data *dat visible = (NtUserGetWindowLongW(data->hwnd, GWL_STYLE) & WS_VISIBLE) == WS_VISIBLE; xdg_visible = surface->xdg_toplevel != NULL; + pthread_mutex_lock(&surface->mutex); + if (visible != xdg_visible) { - pthread_mutex_lock(&surface->mutex); - /* If we have a pre-existing surface ensure it has no role. */ if (data->wayland_surface) wayland_surface_clear_role(surface); /* If the window is a visible toplevel make it a wayland * xdg_toplevel. Otherwise keep it role-less to avoid polluting the * compositor with empty xdg_toplevels. */ if (visible) wayland_surface_make_toplevel(surface); - - pthread_mutex_unlock(&surface->mutex); } + if (!wayland_window_get_config(data->hwnd, &surface->window)) + WARN("failed to get window config for hwnd=%p\n", data->hwnd); + + pthread_mutex_unlock(&surface->mutex); + if (data->window_surface) wayland_window_surface_update_wayland_surface(data->window_surface, surface); @@ -195,22 +206,17 @@ out: static void wayland_win_data_update_wayland_state(struct wayland_win_data *data) { struct wayland_surface *surface = data->wayland_surface; - uint32_t window_state; BOOL processing_config; pthread_mutex_lock(&surface->mutex); if (!surface->xdg_toplevel) goto out; - window_state = - (NtUserGetWindowLongW(surface->hwnd, GWL_STYLE) & WS_MAXIMIZE) ? - WAYLAND_SURFACE_CONFIG_STATE_MAXIMIZED : 0; - processing_config = surface->processing.serial && !surface->processing.processed; TRACE("hwnd=%p window_state=%#x %s->state=%#x\n", - data->hwnd, window_state, + data->hwnd, surface->window.state, processing_config ? "processing" : "current", processing_config ? surface->processing.state : surface->current.state); @@ -218,12 +224,12 @@ static void wayland_win_data_update_wayland_state(struct wayland_win_data *data) * window state to determine and update the Wayland state. */ if (!processing_config) { - if ((window_state & WAYLAND_SURFACE_CONFIG_STATE_MAXIMIZED) && + if ((surface->window.state & WAYLAND_SURFACE_CONFIG_STATE_MAXIMIZED) && !(surface->current.state & WAYLAND_SURFACE_CONFIG_STATE_MAXIMIZED)) { xdg_toplevel_set_maximized(surface->xdg_toplevel); } - else if (!(window_state & WAYLAND_SURFACE_CONFIG_STATE_MAXIMIZED) && + else if (!(surface->window.state & WAYLAND_SURFACE_CONFIG_STATE_MAXIMIZED) && (surface->current.state & WAYLAND_SURFACE_CONFIG_STATE_MAXIMIZED)) { xdg_toplevel_unset_maximized(surface->xdg_toplevel); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4014
From: Alexandros Frantzis <alexandros.frantzis(a)collabora.com> Detect when an window wants to become fullscreen and request the Wayland fullscreen state from the compositor. The fullscreen state for windows is inferred by their size and position relative to the monitor that contains their largest region. Since there is no explicit fullscreen state for windows, compositor initiated fullscreen requests may lead to a size change, but will not directly affect other aspects of the window state. --- dlls/winewayland.drv/wayland_surface.c | 10 ++++ dlls/winewayland.drv/waylanddrv.h | 3 +- dlls/winewayland.drv/window.c | 66 +++++++++++++++++++++----- 3 files changed, 65 insertions(+), 14 deletions(-) diff --git a/dlls/winewayland.drv/wayland_surface.c b/dlls/winewayland.drv/wayland_surface.c index 5bb8c959537..124d15821bf 100644 --- a/dlls/winewayland.drv/wayland_surface.c +++ b/dlls/winewayland.drv/wayland_surface.c @@ -97,6 +97,9 @@ static void xdg_toplevel_handle_configure(void *data, case XDG_TOPLEVEL_STATE_TILED_BOTTOM: config_state |= WAYLAND_SURFACE_CONFIG_STATE_TILED; break; + case XDG_TOPLEVEL_STATE_FULLSCREEN: + config_state |= WAYLAND_SURFACE_CONFIG_STATE_FULLSCREEN; + break; default: break; } @@ -338,6 +341,13 @@ static BOOL wayland_surface_configure_is_compatible(struct wayland_surface_confi return FALSE; } + /* The fullscreen state requires at most the configured size. */ + if ((conf->state & WAYLAND_SURFACE_CONFIG_STATE_FULLSCREEN) && + (width > conf->width || height > conf->height)) + { + return FALSE; + } + return TRUE; } diff --git a/dlls/winewayland.drv/waylanddrv.h b/dlls/winewayland.drv/waylanddrv.h index 4b74b674ea6..e1dc58bdc80 100644 --- a/dlls/winewayland.drv/waylanddrv.h +++ b/dlls/winewayland.drv/waylanddrv.h @@ -61,7 +61,8 @@ enum wayland_surface_config_state { WAYLAND_SURFACE_CONFIG_STATE_MAXIMIZED = (1 << 0), WAYLAND_SURFACE_CONFIG_STATE_RESIZING = (1 << 1), - WAYLAND_SURFACE_CONFIG_STATE_TILED = (1 << 2) + WAYLAND_SURFACE_CONFIG_STATE_TILED = (1 << 2), + WAYLAND_SURFACE_CONFIG_STATE_FULLSCREEN = (1 << 3) }; struct wayland_cursor diff --git a/dlls/winewayland.drv/window.c b/dlls/winewayland.drv/window.c index bf1a286b17d..73daad31d47 100644 --- a/dlls/winewayland.drv/window.c +++ b/dlls/winewayland.drv/window.c @@ -148,9 +148,29 @@ static void wayland_win_data_release(struct wayland_win_data *data) static BOOL wayland_window_get_config(HWND hwnd, struct wayland_window_config *conf) { + enum wayland_surface_config_state window_state = 0; + DWORD style; + if (!NtUserGetWindowRect(hwnd, &conf->rect)) return FALSE; - conf->state = (NtUserGetWindowLongW(hwnd, GWL_STYLE) & WS_MAXIMIZE) ? - WAYLAND_SURFACE_CONFIG_STATE_MAXIMIZED : 0; + style = NtUserGetWindowLongW(hwnd, GWL_STYLE); + + TRACE("window=%s style=%#lx\n", wine_dbgstr_rect(&conf->rect), (long)style); + + /* The fullscreen state is implied by the window position and style. */ + if (NtUserIsWindowRectFullScreen(&conf->rect)) + { + if ((style & WS_MAXIMIZE) && (style & WS_CAPTION) == WS_CAPTION) + window_state |= WAYLAND_SURFACE_CONFIG_STATE_MAXIMIZED; + else if (!(style & WS_MINIMIZE)) + window_state |= WAYLAND_SURFACE_CONFIG_STATE_FULLSCREEN; + } + else if (style & WS_MAXIMIZE) + { + window_state |= WAYLAND_SURFACE_CONFIG_STATE_MAXIMIZED; + } + + conf->state = window_state; + return TRUE; } @@ -224,15 +244,28 @@ static void wayland_win_data_update_wayland_state(struct wayland_win_data *data) * window state to determine and update the Wayland state. */ if (!processing_config) { + /* First do all state unsettings, before setting new state. Some + * Wayland compositors misbehave if the order is reversed. */ + if (!(surface->window.state & WAYLAND_SURFACE_CONFIG_STATE_MAXIMIZED) && + (surface->current.state & WAYLAND_SURFACE_CONFIG_STATE_MAXIMIZED)) + { + xdg_toplevel_unset_maximized(surface->xdg_toplevel); + } + if (!(surface->window.state & WAYLAND_SURFACE_CONFIG_STATE_FULLSCREEN) && + (surface->current.state & WAYLAND_SURFACE_CONFIG_STATE_FULLSCREEN)) + { + xdg_toplevel_unset_fullscreen(surface->xdg_toplevel); + } + if ((surface->window.state & WAYLAND_SURFACE_CONFIG_STATE_MAXIMIZED) && !(surface->current.state & WAYLAND_SURFACE_CONFIG_STATE_MAXIMIZED)) { xdg_toplevel_set_maximized(surface->xdg_toplevel); } - else if (!(surface->window.state & WAYLAND_SURFACE_CONFIG_STATE_MAXIMIZED) && - (surface->current.state & WAYLAND_SURFACE_CONFIG_STATE_MAXIMIZED)) + if ((surface->window.state & WAYLAND_SURFACE_CONFIG_STATE_FULLSCREEN) && + !(surface->current.state & WAYLAND_SURFACE_CONFIG_STATE_FULLSCREEN)) { - xdg_toplevel_unset_maximized(surface->xdg_toplevel); + xdg_toplevel_set_fullscreen(surface->xdg_toplevel, NULL); } } else @@ -348,7 +381,7 @@ static void wayland_configure_window(HWND hwnd) { struct wayland_surface *surface; INT width, height; - UINT flags; + UINT flags = 0; uint32_t state; DWORD style; BOOL needs_enter_size_move = FALSE; @@ -389,6 +422,14 @@ static void wayland_configure_window(HWND hwnd) needs_exit_size_move = TRUE; } + /* Transitions between normal/max/fullscreen may entail a frame change. */ + if ((state ^ surface->current.state) & + (WAYLAND_SURFACE_CONFIG_STATE_MAXIMIZED | + WAYLAND_SURFACE_CONFIG_STATE_FULLSCREEN)) + { + flags |= SWP_FRAMECHANGED; + } + pthread_mutex_unlock(&surface->mutex); TRACE("processing=%dx%d,%#x\n", width, height, state); @@ -396,20 +437,19 @@ static void wayland_configure_window(HWND hwnd) if (needs_enter_size_move) send_message(hwnd, WM_ENTERSIZEMOVE, 0, 0); if (needs_exit_size_move) send_message(hwnd, WM_EXITSIZEMOVE, 0, 0); - flags = SWP_NOACTIVATE | SWP_NOZORDER | SWP_NOOWNERZORDER | SWP_NOMOVE; + flags |= SWP_NOACTIVATE | SWP_NOZORDER | SWP_NOOWNERZORDER | SWP_NOMOVE; if (width == 0 || height == 0) flags |= SWP_NOSIZE; style = NtUserGetWindowLongW(hwnd, GWL_STYLE); if (!(state & WAYLAND_SURFACE_CONFIG_STATE_MAXIMIZED) != !(style & WS_MAXIMIZE)) - { NtUserSetWindowLong(hwnd, GWL_STYLE, style ^ WS_MAXIMIZE, FALSE); - flags |= SWP_FRAMECHANGED; - } - /* The Wayland maximized state is very strict about surface size, so don't - * let the application override it. The tiled state is not as strict, - * but it indicates a strong size preference, so try to respect it. */ + /* The Wayland maximized and fullscreen states are very strict about + * surface size, so don't let the application override it. The tiled state + * is not as strict, but it indicates a strong size preference, so try to + * respect it. */ if (state & (WAYLAND_SURFACE_CONFIG_STATE_MAXIMIZED | + WAYLAND_SURFACE_CONFIG_STATE_FULLSCREEN | WAYLAND_SURFACE_CONFIG_STATE_TILED)) { flags |= SWP_NOSENDCHANGING; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4014
From: Alexandros Frantzis <alexandros.frantzis(a)collabora.com> The maximized and fullscreen states disallow surface contents that are larger than the configured size. However, if we have a larger surface, and only part of the surface contains useful client data, we can inform the compositor using xdg_surface_set_geometry, so that only the specified region is considered for compositor-side positioning and state constraints. We use the geometry to support the presentation of windows that insist on particular positions and sizes which may not match the compositor's requested size (e.g., fullscreen but may overshoot a few pixels outside the monitor boundary). --- dlls/winewayland.drv/wayland_surface.c | 92 ++++++++++++++++++++++++-- 1 file changed, 85 insertions(+), 7 deletions(-) diff --git a/dlls/winewayland.drv/wayland_surface.c b/dlls/winewayland.drv/wayland_surface.c index 124d15821bf..b581cec1b46 100644 --- a/dlls/winewayland.drv/wayland_surface.c +++ b/dlls/winewayland.drv/wayland_surface.c @@ -334,21 +334,97 @@ static BOOL wayland_surface_configure_is_compatible(struct wayland_surface_confi /* We require the same state. */ if ((state & mask) != (conf->state & mask)) return FALSE; - /* The maximized state requires the configured size. */ + /* The maximized state requires the configured size. During surface + * reconfiguration we can use surface geometry to provide smaller areas + * from larger sizes, so only smaller sizes are incompatible. */ if ((conf->state & WAYLAND_SURFACE_CONFIG_STATE_MAXIMIZED) && - (width != conf->width || height != conf->height)) + (width < conf->width || height < conf->height)) { return FALSE; } - /* The fullscreen state requires at most the configured size. */ - if ((conf->state & WAYLAND_SURFACE_CONFIG_STATE_FULLSCREEN) && - (width > conf->width || height > conf->height)) + /* The fullscreen state requires a size smaller or equal to the configured + * size. If we have a larger size, we can use surface geometry during + * surface reconfiguration to provide the smaller size, so we are always + * compatible with a fullscreen state. */ + + return TRUE; +} + +/********************************************************************** + * wayland_surface_get_rect_in_monitor + * + * Gets the largest rectangle within a surface's window (in window coordinates) + * that is visible in a monitor. + */ +static void wayland_surface_get_rect_in_monitor(struct wayland_surface *surface, + RECT *rect) +{ + HMONITOR hmonitor; + MONITORINFO mi; + + mi.cbSize = sizeof(mi); + if (!(hmonitor = NtUserMonitorFromRect(&surface->window.rect, 0)) || + !NtUserGetMonitorInfo(hmonitor, (MONITORINFO *)&mi)) { - return FALSE; + SetRectEmpty(rect); + return; } - return TRUE; + intersect_rect(rect, &mi.rcMonitor, &surface->window.rect); + OffsetRect(rect, -surface->window.rect.left, -surface->window.rect.top); +} + +/********************************************************************** + * wayland_surface_reconfigure_geometry + * + * Sets the xdg_surface geometry + */ +static void wayland_surface_reconfigure_geometry(struct wayland_surface *surface) +{ + int width, height; + RECT rect; + + width = surface->window.rect.right - surface->window.rect.left; + height = surface->window.rect.bottom - surface->window.rect.top; + + /* If the window size is bigger than the current state accepts, use the + * largest visible (from Windows' perspective) subregion of the window. */ + if ((surface->current.state & (WAYLAND_SURFACE_CONFIG_STATE_MAXIMIZED | + WAYLAND_SURFACE_CONFIG_STATE_FULLSCREEN)) && + (width > surface->current.width || height > surface->current.height)) + { + wayland_surface_get_rect_in_monitor(surface, &rect); + + /* If the window rect in the monitor is smaller than required, + * fall back to an appropriately sized rect at the top-left. */ + if ((surface->current.state & WAYLAND_SURFACE_CONFIG_STATE_MAXIMIZED) && + (rect.right - rect.left < surface->current.width || + rect.bottom - rect.top < surface->current.height)) + { + SetRect(&rect, 0, 0, surface->current.width, surface->current.height); + } + else + { + rect.right = min(rect.right, rect.left + surface->current.width); + rect.bottom = min(rect.bottom, rect.top + surface->current.height); + } + TRACE("Window is too large for Wayland state, using subregion\n"); + } + else + { + SetRect(&rect, 0, 0, width, height); + } + + TRACE("hwnd=%p geometry=%s\n", surface->hwnd, wine_dbgstr_rect(&rect)); + + if (!IsRectEmpty(&rect)) + { + xdg_surface_set_window_geometry(surface->xdg_surface, + rect.left, rect.top, + rect.right - rect.left, + rect.bottom - rect.top); + } } /********************************************************************** @@ -403,6 +479,8 @@ BOOL wayland_surface_reconfigure(struct wayland_surface *surface) return FALSE; } + wayland_surface_reconfigure_geometry(surface); + return TRUE; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4014
From: Alexandros Frantzis <alexandros.frantzis(a)collabora.com> Rename the function wayland_surface_configure_is_compatible to wayland_surface_config_is_compatible to match the associated struct name. --- dlls/winewayland.drv/wayland_surface.c | 28 +++++++++++++------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/dlls/winewayland.drv/wayland_surface.c b/dlls/winewayland.drv/wayland_surface.c index b581cec1b46..17d1fffc29b 100644 --- a/dlls/winewayland.drv/wayland_surface.c +++ b/dlls/winewayland.drv/wayland_surface.c @@ -319,14 +319,14 @@ void wayland_surface_attach_shm(struct wayland_surface *surface, } /********************************************************************** - * wayland_surface_configure_is_compatible + * wayland_surface_config_is_compatible * - * Checks whether a wayland_surface_configure object is compatible with the + * Checks whether a wayland_surface_config object is compatible with the * the provided arguments. */ -static BOOL wayland_surface_configure_is_compatible(struct wayland_surface_config *conf, - int width, int height, - enum wayland_surface_config_state state) +static BOOL wayland_surface_config_is_compatible(struct wayland_surface_config *conf, + int width, int height, + enum wayland_surface_config_state state) { static enum wayland_surface_config_state mask = WAYLAND_SURFACE_CONFIG_STATE_MAXIMIZED; @@ -451,9 +451,9 @@ BOOL wayland_surface_reconfigure(struct wayland_surface *surface) /* Acknowledge any compatible processed config. */ if (surface->processing.serial && surface->processing.processed && - wayland_surface_configure_is_compatible(&surface->processing, - width, height, - window->state)) + wayland_surface_config_is_compatible(&surface->processing, + width, height, + window->state)) { surface->current = surface->processing; memset(&surface->processing, 0, sizeof(surface->processing)); @@ -463,18 +463,18 @@ BOOL wayland_surface_reconfigure(struct wayland_surface *surface) * config, use that, in order to draw windows that don't go through the * message loop (e.g., some splash screens). */ else if (!surface->current.serial && surface->requested.serial && - wayland_surface_configure_is_compatible(&surface->requested, - width, height, - window->state)) + wayland_surface_config_is_compatible(&surface->requested, + width, height, + window->state)) { surface->current = surface->requested; memset(&surface->requested, 0, sizeof(surface->requested)); xdg_surface_ack_configure(surface->xdg_surface, surface->current.serial); } else if (!surface->current.serial || - !wayland_surface_configure_is_compatible(&surface->current, - width, height, - window->state)) + !wayland_surface_config_is_compatible(&surface->current, + width, height, + window->state)) { return FALSE; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4014
From: Alexandros Frantzis <alexandros.frantzis(a)collabora.com> If the window is already fullscreen and its size is compatible with what the compositor is requesting, don't force a resize, since some applications are very insistent on a particular fullscreen size (which may not match the monitor size). --- dlls/winewayland.drv/wayland_surface.c | 6 +++--- dlls/winewayland.drv/waylanddrv.h | 3 +++ dlls/winewayland.drv/window.c | 15 +++++++++++++++ 3 files changed, 21 insertions(+), 3 deletions(-) diff --git a/dlls/winewayland.drv/wayland_surface.c b/dlls/winewayland.drv/wayland_surface.c index 17d1fffc29b..ae4812ebb08 100644 --- a/dlls/winewayland.drv/wayland_surface.c +++ b/dlls/winewayland.drv/wayland_surface.c @@ -324,9 +324,9 @@ void wayland_surface_attach_shm(struct wayland_surface *surface, * Checks whether a wayland_surface_config object is compatible with the * the provided arguments. */ -static BOOL wayland_surface_config_is_compatible(struct wayland_surface_config *conf, - int width, int height, - enum wayland_surface_config_state state) +BOOL wayland_surface_config_is_compatible(struct wayland_surface_config *conf, + int width, int height, + enum wayland_surface_config_state state) { static enum wayland_surface_config_state mask = WAYLAND_SURFACE_CONFIG_STATE_MAXIMIZED; diff --git a/dlls/winewayland.drv/waylanddrv.h b/dlls/winewayland.drv/waylanddrv.h index e1dc58bdc80..4bcd9e6706e 100644 --- a/dlls/winewayland.drv/waylanddrv.h +++ b/dlls/winewayland.drv/waylanddrv.h @@ -201,6 +201,9 @@ void wayland_surface_attach_shm(struct wayland_surface *surface, HRGN surface_damage_region) DECLSPEC_HIDDEN; struct wayland_surface *wayland_surface_lock_hwnd(HWND hwnd) DECLSPEC_HIDDEN; BOOL wayland_surface_reconfigure(struct wayland_surface *surface) DECLSPEC_HIDDEN; +BOOL wayland_surface_config_is_compatible(struct wayland_surface_config *conf, + int width, int height, + enum wayland_surface_config_state state) DECLSPEC_HIDDEN; /********************************************************************** * Wayland SHM buffer diff --git a/dlls/winewayland.drv/window.c b/dlls/winewayland.drv/window.c index 73daad31d47..103294d0738 100644 --- a/dlls/winewayland.drv/window.c +++ b/dlls/winewayland.drv/window.c @@ -430,6 +430,21 @@ static void wayland_configure_window(HWND hwnd) flags |= SWP_FRAMECHANGED; } + /* If the window is already fullscreen and its size is compatible with what + * the compositor is requesting, don't force a resize, since some applications + * are very insistent on a particular fullscreen size (which may not match + * the monitor size). */ + if ((surface->window.state & WAYLAND_SURFACE_CONFIG_STATE_FULLSCREEN) && + wayland_surface_config_is_compatible(&surface->processing, + surface->window.rect.right - + surface->window.rect.left, + surface->window.rect.bottom - + surface->window.rect.top, + surface->window.state)) + { + flags |= SWP_NOSIZE; + } + pthread_mutex_unlock(&surface->mutex); TRACE("processing=%dx%d,%#x\n", width, height, state); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4014
No idea what else it would break but what about always using a maximal virtual desktop size on Wayland and ignoring the monitor coverage? That would be a change limited to wayland_resize_desktop, and in theory the rest should just work as if we had very very large monitors?
This idea briefly crossed my mind, but I disregarded it because I thought that `vscreen == desktop_win` was a core assumption. If that's not the case, I think this solution is worth exploring more. There are some technicalities to resolve, because we can't resize the desktop during the process init when we receive initial output info (we don't have the desktop window yet, you may recall the discussions about the issue from an earlier MR), and rely on `explorer.exe` to do this for us after creating the desktop window.
It's not a very common scenario, or is it?
Unfortunately, it's quite common. For windows larger than the vscreen, this is automatically an issue because parts of them are guaranteed to be inaccessible, but those parts may be visible from the user's (compositor) perspective. I have also seen some apps end up in completely arbitrary (inaccessible) places for unknown reasons. That's true under winex11, too, but there they seem to eventually get moved to more reasonable positions. Perhaps the X server plays a role here, or something else is going on, and I need to investigate further in case there is something we can replicate. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4014#note_48202
On Fri Oct 6 11:12:54 2023 +0000, Alexandros Frantzis wrote:
Thanks, I will investigate more how I can make use of visible_rect. I experimented a bit with `visible_rect` as a method to handle the overshooting situation, and I am not convinced that such a change is worth it at the moment.
My first observation is that, as you describe, using `visible_rect` actually crops the content, whereas setting the xdg geometry maintains the whole content but helps the compositor figure out how to position it properly. Thus the geometry method better matches the winex11 behavior in this case (but perhaps we don't care to replicate this?). FWIW, we could also tell the compositor to crop if we prefer (using `wp_viewporter`). The second observation is that to use `visible_rect` we will need to make a decision about the cropping based on the window config, whereas the geometry acts based on the latest current surface config, which is what we actually want to respect. This removal of the logic from the point it's actually needed makes it harder (for me, at least) to reason about the overall correctness. Perhaps when we will want to support server-side decorations we will have to go down this path, but there are also other alternatives (e.g., telling the compositor to crop instead).
to detect fullscreen windows is the visible_rect, which is actually different from the window_rect that this call returns.
Since, at the moment, I am not touching `visible_rect` (which implies `visible_rect == window_rect`) I am just using `window_rect` for everything. But I can switch to explicitly storing and using `visible_rect` (without changing it) if you think that's clearer. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4014#note_48201
On Fri Oct 6 11:12:54 2023 +0000, Alexandros Frantzis wrote:
I will look into this, thanks for the pointer! I updated the logic to be closer to winex11. The previous logic was taken from the experimental branch, so I might have had good reasons to diverge (which I don't recall now). But it could be the case that those reasons are not applicable anymore (e.g., compositor bugs), so let's start "clean" and iterate as needed.
I left out the `WS_MINIMIZE` logic for now, since minimization is a strange beast under Wayland, and the driver doesn't support it at this point. Reading the commit rationale for that line, it's likely that we will need something similar, but we may need something more or slightly different, so I'd rather not introduce it without being able to test it with compositors first. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4014#note_48200
On Fri Oct 6 18:24:01 2023 +0000, beh 591 wrote:
@afrantzis I don't know if you are aware of this problem but wine on wayland at least this branch https://gitlab.winehq.org/afrantzis/wine/-/tree/wayland doesn't support NVIDIA at all it shows a transparent or a black screen and i am pretty sure this is related to wayland not dxvk or anything related since disabling dxvk gives the same transparent or black screen I am using a gtx1650 laptop so I am not sure if this is a problem in all nvidia gpus but yeah that was my experience I builded your branch with https://github.com/lutris/buildbot I tried both opengl and vulkan backend and I am using bottles to test your wine version apps tested are vkcube and genshin impact @beh_591 Unfortunately all development (including bug fixing) on the experimental branch has ceased, to allow me to focus on the upstreaming process. FWIW, I used mostly AMD and Intel GPUs during development, but there were people that successfully tried NVIDIA, although I don't remember the details (e.g., free or proprietary drivers, GL or VK).
The rest of this MR doesn't really depend on the last commit, so it would be fine to move forward without it.
@rbernon I have removed forced repositioning from this branch. As the final commit in this version I kept a small fullscreen improvement that was previously part of the repositioning commit. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4014#note_48203
On Tue Oct 10 17:46:42 2023 +0000, Alexandros Frantzis wrote:
@beh_591 Unfortunately all development (including bug fixing) on the experimental branch has ceased, to allow me to focus on the upstreaming process. FWIW, I used mostly AMD and Intel GPUs during development, but there were people that successfully tried NVIDIA, although I don't remember the details (e.g., free or proprietary drivers, GL or VK).
The rest of this MR doesn't really depend on the last commit, so it would be fine to move forward without it. @rbernon I have removed forced repositioning from this branch. As the final commit in this version I kept a small fullscreen improvement that was previously part of the repositioning commit. That would have been me, maybe others but yea I was using the proprietary drivers. It wasn't working at first but I got it to work only for it to break and start working again, something something Nvidia bad 🤷♂️
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/4014#note_48233
This idea briefly crossed my mind, but I disregarded it because I thought that `vscreen == desktop_win` was a core assumption. If that's not the case, I think this solution is worth exploring more. There are some technicalities to resolve, because we can't resize the desktop during the process init when we receive initial output info (we don't have the desktop window yet, you may recall the discussions about the issue from an earlier MR), and rely on `explorer.exe` to do this for us after creating the desktop window.
No you're right and I was only focusing on the desktop window and completely forgot about the virtual screen, which would still match the monitors. Maybe we could have placeholder monitors left and right to fill the empty space, though it's a bit awkward. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4014#note_48419
On Fri Oct 6 11:12:52 2023 +0000, Alexandros Frantzis wrote:
(cc @erenoit) The linked proposal offers a mechanism to query Wayland surface positions (`get_placement`), which is information we could use to sync in one direction. For the other direction, only just today the scope has expanded to support surface movement hints at arbitrary points after creation, so more in line with what we could be useful for Wine. That being said, I don't think that the overall disposition of the Wayland community to absolute positioning has changed much. In the protocol discussion I am seeing significant pushback, so I wouldn't hold my breath for this (or something similar) landing any time soon. Perhaps a protocol that only implements the `get_placement` part would be more amenable to upstream, and would still provide some benefit to the Wayland driver. Of course, if (and that's a big if) we eventually get a relevant protocol, and which becomes sufficiently widely adopted, I would be happy to use it. I wasn't really expecting a different answer, but I think it raises the question whether using toplevel surfaces for toplevel Wine windows is the right approach. I don't know whether there are alternatives, but the need for workarounds and heuristics in the Wine driver doesn't sound very appealing.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/4014#note_48420
On Tue Oct 10 12:47:30 2023 +0000, Alexandros Frantzis wrote:
I experimented a bit with `visible_rect` as a method to handle the overshooting situation, and I am not convinced that such a change is worth it at the moment. My first observation is that, as you describe, using `visible_rect` actually crops the content, whereas setting the xdg geometry maintains the whole content but helps the compositor figure out how to position it properly. Thus the geometry method better matches the winex11 behavior in this case (but perhaps we don't care to replicate this?). FWIW, we could also tell the compositor to crop if we prefer (using `wp_viewporter`). The second observation is that to use `visible_rect` we will need to make a decision about the cropping based on the window config, whereas the geometry acts based on the latest current surface config, which is what we actually want to respect. This removal of the logic from the point it's actually needed makes it harder (for me, at least) to reason about the overall correctness. Perhaps when we will want to support server-side decorations we will have to go down this path, but there are also other alternatives (e.g., telling the compositor to crop instead).
to detect fullscreen windows is the visible_rect, which is actually different from the window_rect that this call returns. Since, at the moment, I am not touching `visible_rect` (which implies `visible_rect == window_rect`) I am just using `window_rect` for everything. But I can switch to explicitly storing and using `visible_rect` (without changing it) if you think that's clearer. I think passing the rectangle from `WindowPosChanged` would be better and avoid any confusion about the state of the window rect and is validity when we are querying it, as well as when this `wayland_window_config` can get updated (and also making it slightly more similar to `winex11`).
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/4014#note_48421
I have also seen some apps end up in completely arbitrary (inaccessible) places for unknown reasons. That's true under winex11, too, but there they seem to eventually get moved to more reasonable positions. Perhaps the X server plays a role here, or something else is going on, and I need to investigate further in case there is something we can replicate.
Yes, I'm not 100% sure and I don't remember the details but I believe there are similar mechanisms in both Windows and X window managers, automatically adjusting window positions when they are off. In both cases I think it's not a hard requirement, and some applications manage to override it. However this also means that some other applications may expect the mechanisms to be there and to adjust their windows sizes to the right position and size. With this in mind, the geometry adjustment and fullscreen window resize prevention look like workarounds for the cases where applications expect to succeed overriding the constraints, but it also looks to me that they may go against that mechanism and prevent a bit too much resizes from happening, and break the applications that may be expecting them. Anyway, I'm also not completely opposed to having it at this point, and the MR looks mostly good now, so I could probably approve it with the `window_rect` comment above addressed. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4014#note_48422
participants (4)
-
Alexandros Frantzis -
Alexandros Frantzis (＠afrantzis)
-
Rémi Bernon
-
Vixea (＠Vixea)