[PATCH 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! -- 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 | 63 ++++++++++++++++++++------ 3 files changed, 62 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..2a45642f889 100644 --- a/dlls/winewayland.drv/window.c +++ b/dlls/winewayland.drv/window.c @@ -148,9 +148,26 @@ 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) && + !(style & (WS_MINIMIZE | WS_CAPTION))) + { + window_state |= WAYLAND_SURFACE_CONFIG_STATE_FULLSCREEN; + } + + if (style & WS_MAXIMIZE) + window_state |= WAYLAND_SURFACE_CONFIG_STATE_MAXIMIZED; + + conf->state = window_state; + return TRUE; } @@ -224,15 +241,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 +378,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 +419,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 +434,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> Since Wayland doesn't allow querying or setting the absolute position of surfaces, the position of a window in the Windows virtual screen space and its actual position in the compositor space are not in sync. This can lead to situations where parts of a window become apparently inaccessible to mouse input, because although those parts are visible from a Wayland perspective they are outside the Windows virtual screen. As a basic workaround, handling the common case of a single monitor, move windows to the origin of the virtual screen space (0,0), to maximize the window area that is available for user input. We make an exception for windows that are fullscreen, since in such a state some applications insist on a very particular position and size, and trying to force our preference could lead to infinite repositioning loops. In such cases our xdg_surface geometry logic may kick in to ensure that we don't violate size constraints. --- dlls/winewayland.drv/wayland_surface.c | 6 ++--- dlls/winewayland.drv/waylanddrv.h | 6 ++++- dlls/winewayland.drv/window.c | 37 ++++++++++++++++++++++++++ 3 files changed, 45 insertions(+), 4 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..d4cde8bd29c 100644 --- a/dlls/winewayland.drv/waylanddrv.h +++ b/dlls/winewayland.drv/waylanddrv.h @@ -54,7 +54,8 @@ extern struct wayland process_wayland DECLSPEC_HIDDEN; enum wayland_window_message { WM_WAYLAND_INIT_DISPLAY_DEVICES = 0x80001000, - WM_WAYLAND_CONFIGURE = 0x80001001 + WM_WAYLAND_CONFIGURE = 0x80001001, + WM_WAYLAND_SYNC_WINDOW_POSITION = 0x80001002 }; enum wayland_surface_config_state @@ -201,6 +202,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 2a45642f889..95373efadca 100644 --- a/dlls/winewayland.drv/window.c +++ b/dlls/winewayland.drv/window.c @@ -270,6 +270,17 @@ static void wayland_win_data_update_wayland_state(struct wayland_win_data *data) surface->processing.processed = TRUE; } + /* Move the window to the monitor origin to maximize the available window + * area that is available for user input. Don't perform the move if the + * window is fullscreen, since some applications are very insistent on + * a particular fullscreen position (which may not match the monitor + * origin). */ + if (!(surface->window.state & WAYLAND_SURFACE_CONFIG_STATE_FULLSCREEN) && + (surface->window.rect.top != 0 || surface->window.rect.left != 0)) + { + NtUserPostMessage(data->hwnd, WM_WAYLAND_SYNC_WINDOW_POSITION, 0, 0); + } + out: pthread_mutex_unlock(&surface->mutex); wl_display_flush(process_wayland.wl_display); @@ -427,6 +438,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); @@ -455,6 +481,14 @@ static void wayland_configure_window(HWND hwnd) NtUserSetWindowPos(hwnd, 0, 0, 0, width, height, flags); } +static void wayland_sync_window_position(HWND hwnd) +{ + DWORD flags = SWP_NOACTIVATE | SWP_NOZORDER | SWP_NOOWNERZORDER | + SWP_NOSENDCHANGING | SWP_NOSIZE; + + NtUserSetWindowPos(hwnd, 0, 0, 0, 0, 0, flags); +} + /********************************************************************** * WAYLAND_WindowMessage */ @@ -469,6 +503,9 @@ LRESULT WAYLAND_WindowMessage(HWND hwnd, UINT msg, WPARAM wp, LPARAM lp) case WM_WAYLAND_CONFIGURE: wayland_configure_window(hwnd); return 0; + case WM_WAYLAND_SYNC_WINDOW_POSITION: + wayland_sync_window_position(hwnd); + return 0; default: FIXME("got window msg %x hwnd %p wp %lx lp %lx\n", msg, hwnd, (long)wp, lp); return 0; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4014
I haven't looked at the code yet but I'm afraid that force moving the windows sounds like a bad idea at first glance. It may work in many cases, for all the applications which don't care, but it simply won't in many other cases. We already have quite a number of games not very happy about X11 moving the windows slightly differently from native, even though we already try to eat spurious configure messages. You can't really rely on the fullscreen state for that, because some games will first try to move the window to a specific size / position, wait for some expected resize / move sequence *before* even trying to make it fullscreen. I'm also not sure to understand what you mean by Windows virtual space being clipped compared to Wayland input space, do you mean when ClipCursor is used? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4014#note_47581
I am not sure if I understand (4) correctly but there is a [proposal for absolute positioning protocol](https://gitlab.freedesktop.org/wayland/wayland-protocols/-/merge_requests/24...). The merge request is only a week old and still in discussion. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4014#note_47686
This sounds like a required feature for Wine to work properly, and it could perhaps give even more weight to the proposal. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4014#note_47688
@afrantzis I think you should either try to find way to translate Wayland coordinate space to Wine coordinate space, maybe not worrying about where the windows actually are in Wayland space. Also, unless this MR is a hard requirement for the rest of the driver, I think you can perhaps try opening merge requests for other more independent parts of the driver, in order for it not to be blocked until we have a better solution here. I believe we would be very happy to have even a roughly usable driver for 9.0 (in two month time, if that's reasonable for you ofc). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4014#note_47716
On Thu Oct 5 22:49:11 2023 +0000, Rémi Bernon wrote:
@afrantzis I think you should either try to find way to translate Wayland coordinate space to Wine coordinate space, or maybe not worry about where the windows actually are in Wayland space. Also, unless this MR is a hard requirement for the rest of the driver, I think you can perhaps try opening merge requests for other more independent parts of the driver, in order for it not to be blocked until we have a better solution here. I believe we would be very happy to have even a roughly usable driver for 9.0 (in two month time, if that's reasonable for you ofc). Oh that would be a blast indeed!
You guys are the best, I really appreciate what you're doing, in a very wrong world, you are like the sun in winter (that actually makes you less cold and saves your life, heh). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4014#note_47788
Rémi Bernon (@rbernon) commented about dlls/winewayland.drv/window.c:
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;
Instead of querying the rectangle you should probably pass the rects from `WAYLAND_WindowPosChanged`, note that there are different rects there for different purposes and I think the one you want to detect fullscreen windows is the `visible_rect`, which is actually different from the `window_rect` that this call returns. The driver is actually responsible for setting `visible_rect` in `WindowPosChanging`, to the subset of `window_rect` that is going to be displayed on screen. It can be used for instance to crop window decorations, but could also perhaps be used to avoid overshooting monitors, like you are trying to workaround later? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4014#note_47831
Rémi Bernon (@rbernon) commented about dlls/winewayland.drv/window.c:
- 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) && + !(style & (WS_MINIMIZE | WS_CAPTION))) + { + window_state |= WAYLAND_SURFACE_CONFIG_STATE_FULLSCREEN; + } + + if (style & WS_MAXIMIZE) + window_state |= WAYLAND_SURFACE_CONFIG_STATE_MAXIMIZED; + + conf->state = window_state; I think you should probably try to reproduce the winex11 `update_net_wm_states` logic here as, although not perfect, it has been much tested already:
```c style = NtUserGetWindowLongW( data->hwnd, GWL_STYLE ); if (style & WS_MINIMIZE) new_state |= data->net_wm_state & ((1 << NET_WM_STATE_FULLSCREEN)|(1 << NET_WM_STATE_MAXIMIZED)); if (NtUserIsWindowRectFullScreen( &data->whole_rect )) { if ((style & WS_MAXIMIZE) && (style & WS_CAPTION) == WS_CAPTION) new_state |= (1 << NET_WM_STATE_MAXIMIZED); else if (!(style & WS_MINIMIZE)) new_state |= (1 << NET_WM_STATE_FULLSCREEN); } else if (style & WS_MAXIMIZE) new_state |= (1 << NET_WM_STATE_MAXIMIZED); ``` Where I believe `whole_rect` is the same as `visible_rect`. For instance, setting `WS_MINIMIZE` is not supposed to clear the maximized / fullscreen state (and I highly suspect some applications expect this). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4014#note_47832
I'm also not sure to understand what you mean by Windows virtual space being clipped compared to Wayland input space, do you mean when ClipCursor is used?
In this case, I am referring to the cursor clip which is effective when we don't have a set ClipCursor. That effective clip is set to the extents of the desktop window, which matches the virtual screen rectangle. For a more concrete example of the issue, consider: 1. A single monitor 100x100 at (0,0), so desktop window and vscreen is (0,0)-(100,100) (right-open interval). 2. A 50x50 window at (-25,0) in Windows virtual screen, so the left half part is out of the screen from Windows' perspective. 3. The corresponding Wayland surface is fully visible to user, let's say at (0,0)-(50,50) in the Wayland compositor space. 4. The user moves the mouse to Wayland (0,0), i.e., the top left of the surface, and the driver sends a input mouse event with screen coordinates (-25, 0). 5. During hardware event processing in the server, the (-25, 0) coordinate is [clipped](https://gitlab.winehq.org/wine/wine/-/blob/41ab207c71cfaa82ef95c83b291654aba...) to the effective (desktop window) clip, so it becomes (0,0), and the cursor position is updated accordingly. 6. The result is a confused user, which can't meaningfully interact with the left part of the surface, despite it being fully visible to them. Given the lack of Wayland absolute positioning (see other comment about why I don't think this is coming any time soon), here is a more detailed summary of alternatives to try to mitigate the problem: 1. Move the window to (0,0) (i.e., the current proposal), to maximize the region of the window that is accessible to input events. As mentioned, I have had good practical results with this approach, but it is imperfect because: a. It doesn't help if the window is larger than the vscreen, in which case the right and bottom parts are still effectively inaccessible b. As you mention, some apps many not be happy with our forceful moves. 2. As proposed in the MR description, introduce a mechanism to circumvent the clipping for mouse input messages sent by the driver, e.g., `__wine_send_input(..., WINE_INPUT_IGNORE_CLIP)`. As a quick experiment I commented out the server [clipping](https://gitlab.winehq.org/wine/wine/-/blob/41ab207c71cfaa82ef95c83b291654aba...), and the results were positive. However, I don't know if ignoring the clip in this way for Wayland input events would invalidate some very core assumptions, and if it would confuse applications. I imagine some apps will also perform their own internal clipping. Of course, I would be more than happy to hear any other alternatives! As things stand in the Wayland world, I don't think we can be perfect here. We are solidly in best-effort territory, but, even so, I do think we can cater to the majority of use cases. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4014#note_47855
On Fri Oct 6 11:12:53 2023 +0000, Rémi Bernon wrote:
This sounds like a required feature for Wine to work properly, and it could perhaps give even more weight to the proposal. (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. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4014#note_47856
On Fri Oct 6 08:31:12 2023 +0000, Rémi Bernon wrote:
Instead of querying the rectangle you should probably pass the rects from `WAYLAND_WindowPosChanged`, note that there are different rects there for different purposes and I think the one you want to detect fullscreen windows is the `visible_rect`, which is actually different from the `window_rect` that this call returns. The driver is actually responsible for setting `visible_rect` in `WindowPosChanging`, to the subset of `window_rect` that is going to be displayed on screen. It can be used for instance to crop window decorations, but could also perhaps be used to avoid overshooting monitors, like you are trying to workaround later? Thanks, I will investigate more how I can make use of visible_rect.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/4014#note_47857
On Fri Oct 6 08:31:13 2023 +0000, Rémi Bernon wrote:
I think you should probably try to reproduce the winex11 `update_net_wm_states` logic here as, although not perfect, it has been much tested already: ```c style = NtUserGetWindowLongW( data->hwnd, GWL_STYLE ); if (style & WS_MINIMIZE) new_state |= data->net_wm_state & ((1 << NET_WM_STATE_FULLSCREEN)|(1 << NET_WM_STATE_MAXIMIZED)); if (NtUserIsWindowRectFullScreen( &data->whole_rect )) { if ((style & WS_MAXIMIZE) && (style & WS_CAPTION) == WS_CAPTION) new_state |= (1 << NET_WM_STATE_MAXIMIZED); else if (!(style & WS_MINIMIZE)) new_state |= (1 << NET_WM_STATE_FULLSCREEN); } else if (style & WS_MAXIMIZE) new_state |= (1 << NET_WM_STATE_MAXIMIZED); ``` Where I believe `whole_rect` is the same as `visible_rect`. For instance, setting `WS_MINIMIZE` is not supposed to clear the maximized / fullscreen state (and I highly suspect some applications expect this). I will look into this, thanks for the pointer!
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/4014#note_47858
I think you should either try to find way to translate Wayland coordinate space to Wine coordinate space, or maybe not worry about where the windows actually are in Wayland space. Also, unless this MR is a hard requirement for the rest of the driver, I think you can perhaps try opening merge requests for other more independent parts of the driver, in order for it not to be blocked until we have a better solution here
I think not syncing the windows would be fine for now, but ideally I would like to have some solution for the input clipping issue (see other comment) since it's very annoying and confusing for users (me included!). Performing some rough tracking and moving of windows to the right monitor, based on the Wayland output they are on (point (4b) in the MR description) will help with proper multi-monitor integration (e.g., the compositor deciding to make a surface fullscreen on a particular output, and we want to reflect this on the Wine side, too). I think that this would be less problematic since we could at least try to make it be a one-off request, rather than a persistent forceful move. In any case, let's leave that for later. Practically, this all means that I can just drop the last commit from this MR, and perhaps move it to a parallel MR where we can continue to discuss solutions, while we move forward with other things. The rest of this MR doesn't really depend on the last commit, so it would be fine to move forward without it.
I believe we would be very happy to have even a roughly usable driver for 9.0 (in two month time, if that's reasonable for you ofc).
That would be ideal and is my (best-effort) goal, but I don't expect we will be able to reach full feature parity with the experimental branch in this time frame. I will need to prioritize the features which maximize "usability" for some particular target. For example, if the target is primarily games, then I would need to ensure we get in mode change emulation, GL/Vulkan, basic keyboard, and relative motion. I would deprioritize features like copy/paste, full keymap support, cross-process rendering (although that would help with some games/launchers/store apps, but is unfortunately quite complex). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4014#note_47859
On Fri Oct 6 11:12:52 2023 +0000, Alexandros Frantzis wrote: > > I'm also not sure to understand what you mean by Windows virtual space > being clipped compared to Wayland input space, do you mean when > ClipCursor is used? > In this case, I am referring to the cursor clip which is effective when > we don't have a set ClipCursor. That effective clip is set to the > extents of the desktop window, which matches the virtual screen > rectangle. For a more concrete example of the issue, consider: > 1. A single monitor 100x100 at (0,0), so desktop window and vscreen is > (0,0)-(100,100) (right-open interval). > 2. A 50x50 window at (-25,0) in Windows virtual screen, so the left half > part is out of the screen from Windows' perspective. > 3. The corresponding Wayland surface is fully visible to user, let's say > at (0,0)-(50,50) in the Wayland compositor space. > 4. The user moves the mouse to Wayland (0,0), i.e., the top left of the > surface, and the driver sends a input mouse event with screen > coordinates (-25, 0). > 5. During hardware event processing in the server, the (-25, 0) > coordinate is > [clipped](https://gitlab.winehq.org/wine/wine/-/blob/41ab207c71cfaa82ef95c83b291654aba023a45c/server/queue.c#L451) > to the effective (desktop window) clip, so it becomes (0,0), and the > cursor position is updated accordingly. > 6. The result is a confused user, which can't meaningfully interact with > the left part of the surface, despite it being fully visible to them. > Given the lack of Wayland absolute positioning (see other comment about > why I don't think this is coming any time soon), here is a more detailed > summary of alternatives to try to mitigate the problem: > 1. Move the window to (0,0) (i.e., the current proposal), to maximize > the region of the window that is accessible to input events. As > mentioned, I have had good practical results with this approach, but it > is imperfect because: > a. It doesn't help if the window is larger than the vscreen, in which > case the right and bottom parts are still effectively inaccessible > b. As you mention, some apps many not be happy with our forceful moves. > 2. As proposed in the MR description, introduce a mechanism to > circumvent the clipping for mouse input messages sent by the driver, > e.g., `__wine_send_input(..., WINE_INPUT_IGNORE_CLIP)`. As a quick > experiment I commented out the server > [clipping](https://gitlab.winehq.org/wine/wine/-/blob/41ab207c71cfaa82ef95c83b291654aba023a45c/server/queue.c#L451), > and the results were positive. However, I don't know if ignoring the > clip in this way for Wayland input events would invalidate some very > core assumptions, and if it would confuse applications. I imagine some > apps will also perform their own internal clipping. > Of course, I would be more than happy to hear any other alternatives! > As things stand in the Wayland world, I don't think we can be perfect > here. We are solidly in best-effort territory, but, even so, I do think > we can cater to the majority of use cases. 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? Otherwise I think this is some consideration that could be left for later, I don't think it's a huge issue if windows that are moved out of the virtual screen cannot be interacted with, at least not at this point. It's not a very common scenario, or is it? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4014#note_47869
Practically, this all means that I can just drop the last commit from this MR, and perhaps move it to a parallel MR where we can continue to discuss solutions, while we move forward with other things. The rest of this MR doesn't really depend on the last commit, so it would be fine to move forward without it.
Yep.
I would deprioritize features like copy/paste, full keymap support, cross-process rendering (although that would help with some games/launchers/store apps, but is unfortunately quite complex).
Yes, I don't think these are critical things to have. Regarding keyboard mapping specifically, I have seen in some branch that you had copied winex11 keyboard layout detection, and I would really like to avoid duplicating that code. I'm trying to get rid of it on the winex11 side, and it's already quite difficult and controversial there, so I was hoping maybe we could do something better in the wayland driver from the start :). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4014#note_47870
On Fri Oct 6 13:01:05 2023 +0000, Rémi Bernon wrote:
Practically, this all means that I can just drop the last commit from this MR, and perhaps move it to a parallel MR where we can continue to discuss solutions, while we move forward with other things. The rest of this MR doesn't really depend on the last commit, so it would be fine to move forward without it. Yep. I would deprioritize features like copy/paste, full keymap support, cross-process rendering (although that would help with some games/launchers/store apps, but is unfortunately quite complex). Yes, I don't think these are critical things to have. Regarding keyboard mapping specifically, I have seen in some branch that you had copied winex11 keyboard layout detection, and I would really like to avoid duplicating that code. I'm trying to get rid of it on the winex11 side, and it's already quite difficult and controversial there, so I was hoping maybe we could do something better in the wayland driver from the start :). Having only US layout support at first would probably be good enough. @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 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 -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4014#note_47883
On Fri Oct 6 15:13: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 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 @afrantzis since just talking will not help these are some of the errors its throwing at me
`0104:warn:virtual:free_ranges_insert_view range 0x1fe0000 - 0x20e0000 is already mapped 010c:warn:threadname:NtSetInformationThread Thread renamed to L"wine_rpcrt4_server" 00ec:warn:rpc:RPCRT4_default_receive_fragment Short read of header, -1 bytes 00ec:warn:rpc:RPCRT4_io_thread receive failed with error 6be 0108:warn:virtual:free_ranges_insert_view range 0x22e0000 - 0x23e0000 is already mapped 0110:warn:threadname:NtSetInformationThread Thread renamed to L"wine_rpcrt4_io" 0110:warn:virtual:free_ranges_insert_view range 0x25e0000 - 0x26e0000 is already mapped 0114:warn:threadname:NtSetInformationThread Thread renamed to L"wine_threadpool_worker" 00d4:warn:file:NtCreateFile L"\??\C:\VulkanSDK\1.3.261.1\Bin\winewayland.drv" not found (c0000034) 00d4:warn:system:find_adapter_device_by_id Failed to find adapter with id 1. 00d4:warn:system:find_adapter_device_by_id Failed to find adapter with id 1. 00d4:warn:virtual:free_ranges_insert_view range 0x16a0000 - 0x17a0000 is already mapped 00 d4:warn:file:NtCreateFile L"\??\C:\VulkanSDK\1.3.261.1\Bin\uxtheme.dll" not found (c0000034) 00dc:warn:imm:IMM_GetImmHkl Can't find function ImeInquire in ime 00dc:fixme:imm:ImmReleaseContext (0000000000010024, 0000000000000000): stub 00d4:warn:imm:IMM_GetImmHkl Can't find function ImeInquire in ime 00d4:warn:file:NtCreateFile L"\??\C:\VulkanSDK\1.3.261.1\Bin\ole32.dll" not found (c0000034) 00d4:warn:file:NtCreateFile L"\??\C:\VulkanSDK\1.3.261.1\Bin\combase.dll" not found (c0000034) 00d4:fixme:imm:ImmReleaseContext (000000000001004E, 0000000000000000): stub` \----------------------------------------------------------- `007c:fixme:hid:handle_IRP_MN_QUERY_ID Unhandled type 00000005 007c:fixme:hid:handle_IRP_MN_QUERY_ID Unhandled type 00000005 007c:fixme:hid:handle_IRP_MN_QUERY_ID Unhandled type 00000005 007c:fixme:hid:handle_IRP_MN_QUERY_ID Unhandled type 00000005 Selected GPU 0: NVIDIA GeForce GTX 1650 Ti, type: DiscreteGpu` `008c:fixme:imm:ImmReleaseContext (0000000000010024, 0000000000000000): stub 0024:fixme:imm:ImmReleaseContext (000000000001004E, 0000000000000000): stub` `Assertion failed: !status, file dlls/winevulkan/loader_thunks.c, line 5102` `these two lines are always repeating so maybe that can help` `008c:fixme:imm:ImmReleaseContext (0000000000010024, 0000000000000000): stub` `0024:fixme:imm:ImmReleaseContext (000000000001004E, 0000000000000000): stub` -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4014#note_47888
participants (6)
-
Alexandros Frantzis -
Alexandros Frantzis (＠afrantzis)
-
beh 591 (＠beh_591)
-
Eren Önen
-
Gabriele (＠gabriele2000)
-
Rémi Bernon