This MR implements mouselook, which requires support for ClipCursor and relative motion events. The latter we can implement directly without using `SetCursorPos` for which no Wayland protocol exists at the moment.
1. Implement setting the foreground window, as this is required for clipping to work properly (i.e., for the ClipCursor driver callback to be called in the proper process/thread, see commit (2)). We can't unconditionally set the foreground window on keyboard focus, since compositors are eager to give the focus and some windows strongly dislike that (see https://gitlab.winehq.org/wine/wine/-/merge_requests/4102#note_51733). This MR borrows the "managed" window concept from WineX11 to detect windows that we shouldn't allow the compositor to manage, and avoids setting such windows as the foreground on keyboard focus. 2. Implement cursor clipping using the pointer-constraints protocol. 3. Switch to emitting relative motion events using the relative-pointer protocol when the cursor is not visible. --- 4. Finally, a potential fix for an issue I have been seeing where apps may transiently lose the active status (see commit message for more) as a result of the foreground state changes in this MR, which has undesirable side-effects. I have included this here for MR completeness/correctness, but it can be moved to a subsequent MR if preferred.
Note: the velocity of relative motion events when scaling is involved is somewhat inconsistent among Wayland compositors.
Thanks!
From: Alexandros Frantzis alexandros.frantzis@collabora.com
Borrow the concept of "managed" windows from WineX11 and use it to decide whether a window should become the foreground window when receiving the Wayland keyboard focus. --- dlls/winewayland.drv/wayland_keyboard.c | 7 ++ dlls/winewayland.drv/waylanddrv.h | 1 + dlls/winewayland.drv/window.c | 99 +++++++++++++++++++++++++ 3 files changed, 107 insertions(+)
diff --git a/dlls/winewayland.drv/wayland_keyboard.c b/dlls/winewayland.drv/wayland_keyboard.c index 26cf3e8a7af..ba6a66995a6 100644 --- a/dlls/winewayland.drv/wayland_keyboard.c +++ b/dlls/winewayland.drv/wayland_keyboard.c @@ -698,6 +698,7 @@ static void keyboard_handle_enter(void *data, struct wl_keyboard *wl_keyboard, struct wl_array *keys) { struct wayland_keyboard *keyboard = &process_wayland.keyboard; + struct wayland_surface *surface; HWND hwnd;
if (!wl_surface) return; @@ -713,6 +714,12 @@ static void keyboard_handle_enter(void *data, struct wl_keyboard *wl_keyboard,
NtUserPostMessage(keyboard->focused_hwnd, WM_INPUTLANGCHANGEREQUEST, 0 /*FIXME*/, (LPARAM)keyboard_hkl); + + if ((surface = wayland_surface_lock_hwnd(hwnd))) + { + if (surface->window.managed) NtUserSetForegroundWindow(hwnd); + pthread_mutex_unlock(&surface->mutex); + } }
static void keyboard_handle_leave(void *data, struct wl_keyboard *wl_keyboard, diff --git a/dlls/winewayland.drv/waylanddrv.h b/dlls/winewayland.drv/waylanddrv.h index 0e781352519..08abc247e16 100644 --- a/dlls/winewayland.drv/waylanddrv.h +++ b/dlls/winewayland.drv/waylanddrv.h @@ -167,6 +167,7 @@ struct wayland_window_config /* The scale (i.e., normalized dpi) the window is rendering at. */ double scale; BOOL visible; + BOOL managed; };
struct wayland_client_surface diff --git a/dlls/winewayland.drv/window.c b/dlls/winewayland.drv/window.c index 31ecb6542c6..93a730e8ada 100644 --- a/dlls/winewayland.drv/window.c +++ b/dlls/winewayland.drv/window.c @@ -27,6 +27,9 @@ #include <assert.h> #include <stdlib.h>
+#include "ntstatus.h" +#define WIN32_NO_STATUS + #include "waylanddrv.h"
#include "wine/debug.h" @@ -47,6 +50,7 @@ struct wayland_win_data RECT window_rect; /* USER client rectangle relative to win32 parent window client area */ RECT client_rect; + BOOL managed; };
static int wayland_win_data_cmp_rb(const void *key, @@ -182,6 +186,7 @@ static void wayland_win_data_get_config(struct wayland_win_data *data, conf->state = window_state; conf->scale = NtUserGetDpiForWindow(data->hwnd) / 96.0; conf->visible = (style & WS_VISIBLE) == WS_VISIBLE; + conf->managed = data->managed; }
static void wayland_win_data_update_wayland_surface(struct wayland_win_data *data) @@ -287,6 +292,99 @@ out: wl_display_flush(process_wayland.wl_display); }
+static BOOL is_managed(HWND hwnd) +{ + struct wayland_win_data *data = wayland_win_data_get(hwnd); + BOOL ret = data && data->managed; + if (data) wayland_win_data_release(data); + return ret; +} + +static HWND *build_hwnd_list(void) +{ + NTSTATUS status; + HWND *list; + ULONG count = 128; + + for (;;) + { + if (!(list = malloc(count * sizeof(*list)))) return NULL; + status = NtUserBuildHwndList(0, 0, 0, 0, 0, count, list, &count); + if (!status) return list; + free(list); + if (status != STATUS_BUFFER_TOO_SMALL) return NULL; + } +} + +static BOOL has_owned_popups(HWND hwnd) +{ + HWND *list; + UINT i; + BOOL ret = FALSE; + + if (!(list = build_hwnd_list())) return FALSE; + + for (i = 0; list[i] != HWND_BOTTOM; i++) + { + if (list[i] == hwnd) break; /* popups are always above owner */ + if (NtUserGetWindowRelative(list[i], GW_OWNER) != hwnd) continue; + if ((ret = is_managed(list[i]))) break; + } + + free(list); + return ret; +} + +static inline HWND get_active_window(void) +{ + GUITHREADINFO info; + info.cbSize = sizeof(info); + return NtUserGetGUIThreadInfo(GetCurrentThreadId(), &info) ? info.hwndActive : 0; +} + +/*********************************************************************** + * is_window_managed + * + * Check if a given window should be managed + */ +static BOOL is_window_managed(HWND hwnd, UINT swp_flags, const RECT *window_rect) +{ + DWORD style, ex_style; + + /* child windows are not managed */ + style = NtUserGetWindowLongW(hwnd, GWL_STYLE); + if ((style & (WS_CHILD|WS_POPUP)) == WS_CHILD) return FALSE; + /* activated windows are managed */ + if (!(swp_flags & (SWP_NOACTIVATE|SWP_HIDEWINDOW))) return TRUE; + if (hwnd == get_active_window()) return TRUE; + /* windows with caption are managed */ + if ((style & WS_CAPTION) == WS_CAPTION) return TRUE; + /* windows with thick frame are managed */ + if (style & WS_THICKFRAME) return TRUE; + if (style & WS_POPUP) + { + HMONITOR hmon; + MONITORINFO mi; + + /* popup with sysmenu == caption are managed */ + if (style & WS_SYSMENU) return TRUE; + /* full-screen popup windows are managed */ + hmon = NtUserMonitorFromWindow(hwnd, MONITOR_DEFAULTTOPRIMARY); + mi.cbSize = sizeof(mi); + NtUserGetMonitorInfo(hmon, &mi); + if (window_rect->left <= mi.rcWork.left && window_rect->right >= mi.rcWork.right && + window_rect->top <= mi.rcWork.top && window_rect->bottom >= mi.rcWork.bottom) + return TRUE; + } + /* application windows are managed */ + ex_style = NtUserGetWindowLongW(hwnd, GWL_EXSTYLE); + if (ex_style & WS_EX_APPWINDOW) return TRUE; + /* windows that own popups are managed */ + if (has_owned_popups(hwnd)) return TRUE; + /* default: not managed */ + return FALSE; +} + /*********************************************************************** * WAYLAND_DestroyWindow */ @@ -369,6 +467,7 @@ void WAYLAND_WindowPosChanged(HWND hwnd, HWND insert_after, UINT swp_flags,
data->window_rect = *window_rect; data->client_rect = *client_rect; + data->managed = is_window_managed(hwnd, swp_flags, window_rect);
if (surface) window_surface_add_ref(surface); if (data->window_surface) window_surface_release(data->window_surface);
From: Alexandros Frantzis alexandros.frantzis@collabora.com
--- dlls/winewayland.drv/wayland.c | 5 ----- 1 file changed, 5 deletions(-)
diff --git a/dlls/winewayland.drv/wayland.c b/dlls/winewayland.drv/wayland.c index f7119367543..31cd27f7a76 100644 --- a/dlls/winewayland.drv/wayland.c +++ b/dlls/winewayland.drv/wayland.c @@ -154,11 +154,6 @@ static void registry_handle_global(void *data, struct wl_registry *registry, process_wayland.wl_subcompositor = wl_registry_bind(registry, id, &wl_subcompositor_interface, 1); } - else if (strcmp(interface, "wp_viewporter") == 0) - { - process_wayland.wp_viewporter = - wl_registry_bind(registry, id, &wp_viewporter_interface, 1); - } }
static void registry_handle_global_remove(void *data, struct wl_registry *registry,
From: Alexandros Frantzis alexandros.frantzis@collabora.com
Use the zwp_pointer_constraints_v1 protocol to implement cursor clipping. Note that Wayland only allows us to constrain the cursor within the extents of the currently focused surface. --- dlls/winewayland.drv/Makefile.in | 1 + dlls/winewayland.drv/wayland.c | 10 +++ dlls/winewayland.drv/wayland_pointer.c | 108 +++++++++++++++++++++++++ dlls/winewayland.drv/waylanddrv.h | 4 + dlls/winewayland.drv/waylanddrv_main.c | 1 + 5 files changed, 124 insertions(+)
diff --git a/dlls/winewayland.drv/Makefile.in b/dlls/winewayland.drv/Makefile.in index 8be78bd2080..ec1eff8d97c 100644 --- a/dlls/winewayland.drv/Makefile.in +++ b/dlls/winewayland.drv/Makefile.in @@ -6,6 +6,7 @@ UNIX_LIBS = -lwin32u $(WAYLAND_CLIENT_LIBS) $(XKBCOMMON_LIBS) $(XKBREGISTRY_LIBS SOURCES = \ display.c \ dllmain.c \ + pointer-constraints-unstable-v1.xml \ version.rc \ viewporter.xml \ vulkan.c \ diff --git a/dlls/winewayland.drv/wayland.c b/dlls/winewayland.drv/wayland.c index 31cd27f7a76..066e9e7c963 100644 --- a/dlls/winewayland.drv/wayland.c +++ b/dlls/winewayland.drv/wayland.c @@ -154,6 +154,11 @@ static void registry_handle_global(void *data, struct wl_registry *registry, process_wayland.wl_subcompositor = wl_registry_bind(registry, id, &wl_subcompositor_interface, 1); } + else if (strcmp(interface, "zwp_pointer_constraints_v1") == 0) + { + process_wayland.zwp_pointer_constraints_v1 = + wl_registry_bind(registry, id, &zwp_pointer_constraints_v1_interface, 1); + } }
static void registry_handle_global_remove(void *data, struct wl_registry *registry, @@ -259,6 +264,11 @@ BOOL wayland_process_init(void) ERR("Wayland compositor doesn't support wl_subcompositor\n"); return FALSE; } + if (!process_wayland.zwp_pointer_constraints_v1) + { + ERR("Wayland compositor doesn't support zwp_pointer_constraints_v1\n"); + return FALSE; + }
wayland_init_display_devices(FALSE);
diff --git a/dlls/winewayland.drv/wayland_pointer.c b/dlls/winewayland.drv/wayland_pointer.c index 33fd14fa0c5..79a1c6a8e60 100644 --- a/dlls/winewayland.drv/wayland_pointer.c +++ b/dlls/winewayland.drv/wayland_pointer.c @@ -245,6 +245,11 @@ void wayland_pointer_deinit(void) struct wayland_pointer *pointer = &process_wayland.pointer;
pthread_mutex_lock(&pointer->mutex); + if (pointer->zwp_confined_pointer_v1) + { + zwp_confined_pointer_v1_destroy(pointer->zwp_confined_pointer_v1); + pointer->zwp_confined_pointer_v1 = NULL; + } wl_pointer_release(pointer->wl_pointer); pointer->wl_pointer = NULL; pointer->focused_hwnd = NULL; @@ -587,3 +592,106 @@ void WAYLAND_SetCursor(HWND hwnd, HCURSOR hcursor)
wayland_set_cursor(hwnd, hcursor, TRUE); } + +/*********************************************************************** + * WAYLAND_ClipCursor + */ +BOOL WAYLAND_ClipCursor(const RECT *clip, BOOL reset) +{ + struct wayland_pointer *pointer = &process_wayland.pointer; + HWND hwnd = wayland_pointer_get_focused_hwnd(); + struct wl_surface *wl_surface = NULL; + int top, left, bottom, right; + RECT window_clip; + + TRACE("clip=%s reset=%d\n", wine_dbgstr_rect(clip), reset); + + if (NtUserGetWindowThread(hwnd, NULL) != GetCurrentThreadId()) + { + pthread_mutex_lock(&pointer->mutex); + if (pointer->zwp_confined_pointer_v1) + { + zwp_confined_pointer_v1_destroy(pointer->zwp_confined_pointer_v1); + pointer->zwp_confined_pointer_v1 = NULL; + TRACE("Unconfining due to foreground thread mismatch\n"); + } + pthread_mutex_unlock(&pointer->mutex); + return TRUE; + } + + if (!reset && clip) + { + RECT vscreen_rect = NtUserGetVirtualScreenRect(); + struct wayland_surface *surface = NULL; + + /* FIXME: surface->window.rect is in window DPI, whereas + * clip and vscreen_rect are in thread DPI. */ + + if (!EqualRect(clip, &vscreen_rect) && + (surface = wayland_surface_lock_hwnd(pointer->focused_hwnd)) && + (intersect_rect(&window_clip, clip, &surface->window.rect) || + IsRectEmpty(clip))) + { + OffsetRect(&window_clip, + -surface->window.rect.left, + -surface->window.rect.top); + + wayland_surface_coords_from_window(surface, + window_clip.left, + window_clip.top, + &left, &top); + wayland_surface_coords_from_window(surface, + window_clip.right, + window_clip.bottom, + &right, &bottom); + wl_surface = surface->wl_surface; + } + if (surface) pthread_mutex_unlock(&surface->mutex); + } + + pthread_mutex_lock(&pointer->mutex); + + /* Since we have previously established that we are running in the context + * of the HWND thread, we know that the HWND, wayland_surface and wl_surface + * will not be invalidated, so we can access these without extra locking. */ + if (wl_surface) + { + struct wl_region *region; + + region = wl_compositor_create_region(process_wayland.wl_compositor); + wl_region_add(region, left, top, right - left, bottom - top); + + if (!pointer->zwp_confined_pointer_v1) + { + pointer->zwp_confined_pointer_v1 = + zwp_pointer_constraints_v1_confine_pointer( + process_wayland.zwp_pointer_constraints_v1, + wl_surface, + pointer->wl_pointer, + region, + ZWP_POINTER_CONSTRAINTS_V1_LIFETIME_PERSISTENT); + } + else + { + zwp_confined_pointer_v1_set_region(pointer->zwp_confined_pointer_v1, + region); + } + + TRACE("Confining to %s %d,%d+%d,%d\n", + wine_dbgstr_rect(&window_clip), + left, top, right - left, bottom - top); + + wl_region_destroy(region); + wl_display_flush(process_wayland.wl_display); + } + else if (pointer->zwp_confined_pointer_v1) + { + zwp_confined_pointer_v1_destroy(pointer->zwp_confined_pointer_v1); + pointer->zwp_confined_pointer_v1 = NULL; + TRACE("Unconfining\n"); + } + + pthread_mutex_unlock(&pointer->mutex); + + return TRUE; +} diff --git a/dlls/winewayland.drv/waylanddrv.h b/dlls/winewayland.drv/waylanddrv.h index 08abc247e16..f3c21f2871f 100644 --- a/dlls/winewayland.drv/waylanddrv.h +++ b/dlls/winewayland.drv/waylanddrv.h @@ -29,6 +29,7 @@ #include <wayland-client.h> #include <xkbcommon/xkbcommon.h> #include <xkbcommon/xkbregistry.h> +#include "pointer-constraints-unstable-v1-client-protocol.h" #include "viewporter-client-protocol.h" #include "xdg-output-unstable-v1-client-protocol.h" #include "xdg-shell-client-protocol.h" @@ -89,6 +90,7 @@ struct wayland_cursor struct wayland_pointer { struct wl_pointer *wl_pointer; + struct zwp_confined_pointer_v1 *zwp_confined_pointer_v1; HWND focused_hwnd; uint32_t enter_serial; uint32_t button_serial; @@ -115,6 +117,7 @@ struct wayland struct wl_shm *wl_shm; struct wp_viewporter *wp_viewporter; struct wl_subcompositor *wl_subcompositor; + struct zwp_pointer_constraints_v1 *zwp_pointer_constraints_v1; struct wayland_seat seat; struct wayland_keyboard keyboard; struct wayland_pointer pointer; @@ -305,6 +308,7 @@ RGNDATA *get_region_data(HRGN region); * USER driver functions */
+BOOL WAYLAND_ClipCursor(const RECT *clip, BOOL reset); LRESULT WAYLAND_DesktopWindowProc(HWND hwnd, UINT msg, WPARAM wp, LPARAM lp); void WAYLAND_DestroyWindow(HWND hwnd); void WAYLAND_SetCursor(HWND hwnd, HCURSOR hcursor); diff --git a/dlls/winewayland.drv/waylanddrv_main.c b/dlls/winewayland.drv/waylanddrv_main.c index 435a6d2b36c..b60d282aacb 100644 --- a/dlls/winewayland.drv/waylanddrv_main.c +++ b/dlls/winewayland.drv/waylanddrv_main.c @@ -31,6 +31,7 @@
static const struct user_driver_funcs waylanddrv_funcs = { + .pClipCursor = WAYLAND_ClipCursor, .pDesktopWindowProc = WAYLAND_DesktopWindowProc, .pDestroyWindow = WAYLAND_DestroyWindow, .pKbdLayerDescriptor = WAYLAND_KbdLayerDescriptor,
From: Alexandros Frantzis alexandros.frantzis@collabora.com
When the cursor is hidden, transition to relative mouse motion to enable mouselook in 3D games. --- dlls/winewayland.drv/Makefile.in | 1 + dlls/winewayland.drv/wayland.c | 5 ++ dlls/winewayland.drv/wayland_pointer.c | 118 ++++++++++++++++++++++++- dlls/winewayland.drv/waylanddrv.h | 3 + 4 files changed, 124 insertions(+), 3 deletions(-)
diff --git a/dlls/winewayland.drv/Makefile.in b/dlls/winewayland.drv/Makefile.in index ec1eff8d97c..b47bdb262c0 100644 --- a/dlls/winewayland.drv/Makefile.in +++ b/dlls/winewayland.drv/Makefile.in @@ -7,6 +7,7 @@ SOURCES = \ display.c \ dllmain.c \ pointer-constraints-unstable-v1.xml \ + relative-pointer-unstable-v1.xml \ version.rc \ viewporter.xml \ vulkan.c \ diff --git a/dlls/winewayland.drv/wayland.c b/dlls/winewayland.drv/wayland.c index 066e9e7c963..841b33ae560 100644 --- a/dlls/winewayland.drv/wayland.c +++ b/dlls/winewayland.drv/wayland.c @@ -159,6 +159,11 @@ static void registry_handle_global(void *data, struct wl_registry *registry, process_wayland.zwp_pointer_constraints_v1 = wl_registry_bind(registry, id, &zwp_pointer_constraints_v1_interface, 1); } + else if (strcmp(interface, "zwp_relative_pointer_manager_v1") == 0) + { + process_wayland.zwp_relative_pointer_manager_v1 = + wl_registry_bind(registry, id, &zwp_relative_pointer_manager_v1_interface, 1); + } }
static void registry_handle_global_remove(void *data, struct wl_registry *registry, diff --git a/dlls/winewayland.drv/wayland_pointer.c b/dlls/winewayland.drv/wayland_pointer.c index 79a1c6a8e60..79e72e41b9d 100644 --- a/dlls/winewayland.drv/wayland_pointer.c +++ b/dlls/winewayland.drv/wayland_pointer.c @@ -46,8 +46,7 @@ static HWND wayland_pointer_get_focused_hwnd(void) return hwnd; }
-static void pointer_handle_motion(void *data, struct wl_pointer *wl_pointer, - uint32_t time, wl_fixed_t sx, wl_fixed_t sy) +static void pointer_handle_motion_internal(wl_fixed_t sx, wl_fixed_t sy) { INPUT input = {0}; RECT *window_rect; @@ -90,6 +89,17 @@ static void pointer_handle_motion(void *data, struct wl_pointer *wl_pointer, __wine_send_input(hwnd, &input, NULL); }
+static void pointer_handle_motion(void *data, struct wl_pointer *wl_pointer, + uint32_t time, wl_fixed_t sx, wl_fixed_t sy) +{ + struct wayland_pointer *pointer = &process_wayland.pointer; + + /* Ignore absolute motion events if in relative mode. */ + if (pointer->zwp_relative_pointer_v1) return; + + pointer_handle_motion_internal(sx, sy); +} + static void wayland_set_cursor(HWND hwnd, HCURSOR hcursor, BOOL use_hcursor);
static void pointer_handle_enter(void *data, struct wl_pointer *wl_pointer, @@ -118,7 +128,7 @@ static void pointer_handle_enter(void *data, struct wl_pointer *wl_pointer, /* Handle the enter as a motion, to account for cases where the * window first appears beneath the pointer and won't get a separate * motion event. */ - pointer_handle_motion(data, wl_pointer, 0, sx, sy); + pointer_handle_motion_internal(sx, sy); }
static void pointer_handle_leave(void *data, struct wl_pointer *wl_pointer, @@ -228,6 +238,80 @@ static const struct wl_pointer_listener pointer_listener = pointer_handle_axis_discrete };
+static void relative_pointer_v1_relative_motion(void *data, + struct zwp_relative_pointer_v1 *zwp_relative_pointer_v1, + uint32_t utime_hi, uint32_t utime_lo, + wl_fixed_t dx, wl_fixed_t dy, + wl_fixed_t dx_unaccel, wl_fixed_t dy_unaccel) +{ + INPUT input = {0}; + HWND hwnd; + POINT screen, origin; + struct wayland_surface *surface; + RECT window_rect; + + if (!(hwnd = wayland_pointer_get_focused_hwnd())) return; + if (!(surface = wayland_surface_lock_hwnd(hwnd))) return; + + window_rect = surface->window.rect; + + wayland_surface_coords_to_window(surface, + wl_fixed_to_double(dx), + wl_fixed_to_double(dy), + (int *)&screen.x, (int *)&screen.y); + + pthread_mutex_unlock(&surface->mutex); + + if (screen.x >= 0) + { + origin.x = window_rect.left; + screen.x += origin.x; + if (screen.x >= window_rect.right) screen.x = window_rect.right - 1; + } + else + { + origin.x = window_rect.right; + screen.x += origin.x; + if (screen.x < window_rect.left) screen.x = window_rect.left; + } + + if (screen.y >= 0) + { + origin.y = window_rect.top; + screen.y += origin.y; + if (screen.y >= window_rect.bottom) screen.y = window_rect.bottom - 1; + } + else + { + origin.y = window_rect.bottom; + screen.y += origin.y; + if (screen.y < window_rect.top) screen.y = window_rect.top; + } + + /* Transform the relative motion from window coordinates to physical + * coordinates required for the input event. */ + if (!NtUserLogicalToPerMonitorDPIPhysicalPoint(hwnd, &screen)) return; + if (!NtUserLogicalToPerMonitorDPIPhysicalPoint(hwnd, &origin)) return; + screen.x -= origin.x; + screen.y -= origin.y; + + input.type = INPUT_MOUSE; + input.mi.dx = screen.x; + input.mi.dy = screen.y; + input.mi.dwFlags = MOUSEEVENTF_MOVE; + + TRACE("hwnd=%p wayland_dxdy=%.2f,%.2f screen_dxdy=%d,%d\n", + hwnd, wl_fixed_to_double(dx), wl_fixed_to_double(dy), + (int)screen.x, (int)screen.y); + + __wine_send_input(hwnd, &input, NULL); +} + +static const struct zwp_relative_pointer_v1_listener relative_pointer_v1_listener = +{ + relative_pointer_v1_relative_motion +}; + void wayland_pointer_init(struct wl_pointer *wl_pointer) { struct wayland_pointer *pointer = &process_wayland.pointer; @@ -250,6 +334,11 @@ void wayland_pointer_deinit(void) zwp_confined_pointer_v1_destroy(pointer->zwp_confined_pointer_v1); pointer->zwp_confined_pointer_v1 = NULL; } + if (pointer->zwp_relative_pointer_v1) + { + zwp_relative_pointer_v1_destroy(pointer->zwp_relative_pointer_v1); + pointer->zwp_relative_pointer_v1 = NULL; + } wl_pointer_release(pointer->wl_pointer); pointer->wl_pointer = NULL; pointer->focused_hwnd = NULL; @@ -552,6 +641,28 @@ clear_cursor: } }
+static void wayland_pointer_update_relative(void) +{ + struct wayland_pointer *pointer = &process_wayland.pointer; + + if (!pointer->cursor.wl_surface && !pointer->zwp_relative_pointer_v1) + { + pointer->zwp_relative_pointer_v1 = + zwp_relative_pointer_manager_v1_get_relative_pointer( + process_wayland.zwp_relative_pointer_manager_v1, + pointer->wl_pointer); + zwp_relative_pointer_v1_add_listener(pointer->zwp_relative_pointer_v1, + &relative_pointer_v1_listener, NULL); + TRACE("Enabling relative motion\n"); + } + else if (pointer->cursor.wl_surface && pointer->zwp_relative_pointer_v1) + { + zwp_relative_pointer_v1_destroy(pointer->zwp_relative_pointer_v1); + pointer->zwp_relative_pointer_v1 = NULL; + TRACE("Disabling relative motion\n"); + } +} + static void wayland_set_cursor(HWND hwnd, HCURSOR hcursor, BOOL use_hcursor) { struct wayland_pointer *pointer = &process_wayland.pointer; @@ -578,6 +689,7 @@ static void wayland_set_cursor(HWND hwnd, HCURSOR hcursor, BOOL use_hcursor) pointer->cursor.wl_surface, pointer->cursor.hotspot_x, pointer->cursor.hotspot_y); + wayland_pointer_update_relative(); wl_display_flush(process_wayland.wl_display); } pthread_mutex_unlock(&pointer->mutex); diff --git a/dlls/winewayland.drv/waylanddrv.h b/dlls/winewayland.drv/waylanddrv.h index f3c21f2871f..69dd2a3659c 100644 --- a/dlls/winewayland.drv/waylanddrv.h +++ b/dlls/winewayland.drv/waylanddrv.h @@ -30,6 +30,7 @@ #include <xkbcommon/xkbcommon.h> #include <xkbcommon/xkbregistry.h> #include "pointer-constraints-unstable-v1-client-protocol.h" +#include "relative-pointer-unstable-v1-client-protocol.h" #include "viewporter-client-protocol.h" #include "xdg-output-unstable-v1-client-protocol.h" #include "xdg-shell-client-protocol.h" @@ -91,6 +92,7 @@ struct wayland_pointer { struct wl_pointer *wl_pointer; struct zwp_confined_pointer_v1 *zwp_confined_pointer_v1; + struct zwp_relative_pointer_v1 *zwp_relative_pointer_v1; HWND focused_hwnd; uint32_t enter_serial; uint32_t button_serial; @@ -118,6 +120,7 @@ struct wayland struct wp_viewporter *wp_viewporter; struct wl_subcompositor *wl_subcompositor; struct zwp_pointer_constraints_v1 *zwp_pointer_constraints_v1; + struct zwp_relative_pointer_manager_v1 *zwp_relative_pointer_manager_v1; struct wayland_seat seat; struct wayland_keyboard keyboard; struct wayland_pointer pointer;
From: Alexandros Frantzis alexandros.frantzis@collabora.com
If when updating the foreground window, both the old and new active window belong to the same thread, even if it's not the current thread, activate the new window without explicitly deactivating the previous one. This matches the current behavior of the code when both windows belong to the current thread.
This avoids the transient deactivation of the target thread which can lead to unwanted side effects (e.g., some apps may minimize when they become inactive). --- dlls/win32u/input.c | 16 ++++++++++------ include/wine/server_protocol.h | 4 ++-- server/protocol.def | 9 +++++---- server/queue.c | 2 ++ server/request.h | 1 + server/trace.c | 1 + 6 files changed, 21 insertions(+), 12 deletions(-)
diff --git a/dlls/win32u/input.c b/dlls/win32u/input.c index 3ee46f0bfcf..a04c8980b92 100644 --- a/dlls/win32u/input.c +++ b/dlls/win32u/input.c @@ -2056,7 +2056,7 @@ HWND WINAPI NtUserSetFocus( HWND hwnd ) */ BOOL set_foreground_window( HWND hwnd, BOOL mouse ) { - BOOL ret, send_msg_old = FALSE, send_msg_new = FALSE; + BOOL ret, send_msg_old = FALSE, send_msg_new = FALSE, deactivate_old = FALSE; HWND previous = 0;
if (mouse) hwnd = get_full_window_handle( hwnd ); @@ -2069,17 +2069,21 @@ BOOL set_foreground_window( HWND hwnd, BOOL mouse ) previous = wine_server_ptr_handle( reply->previous ); send_msg_old = reply->send_msg_old; send_msg_new = reply->send_msg_new; + deactivate_old = reply->deactivate_old; } } SERVER_END_REQ;
if (ret && previous != hwnd) { - if (send_msg_old) /* old window belongs to other thread */ - NtUserMessageCall( previous, WM_WINE_SETACTIVEWINDOW, 0, 0, - 0, NtUserSendNotifyMessage, FALSE ); - else if (send_msg_new) /* old window belongs to us but new one to other thread */ - ret = set_active_window( 0, NULL, mouse, TRUE ); + if (deactivate_old) + { + if (send_msg_old) /* old window belongs to other thread */ + NtUserMessageCall( previous, WM_WINE_SETACTIVEWINDOW, 0, 0, + 0, NtUserSendNotifyMessage, FALSE ); + else if (send_msg_new) /* old window belongs to us but new one to other thread */ + ret = set_active_window( 0, NULL, mouse, TRUE ); + }
if (send_msg_new) /* new window belongs to other thread */ NtUserMessageCall( hwnd, WM_WINE_SETACTIVEWINDOW, (WPARAM)hwnd, 0, diff --git a/include/wine/server_protocol.h b/include/wine/server_protocol.h index 139a7bca69e..0870a631fbe 100644 --- a/include/wine/server_protocol.h +++ b/include/wine/server_protocol.h @@ -4050,7 +4050,7 @@ struct set_foreground_window_reply user_handle_t previous; int send_msg_old; int send_msg_new; - char __pad_20[4]; + int deactivate_old; };
@@ -6507,7 +6507,7 @@ union generic_reply
/* ### protocol_version begin ### */
-#define SERVER_PROTOCOL_VERSION 786 +#define SERVER_PROTOCOL_VERSION 788
/* ### protocol_version end ### */
diff --git a/server/protocol.def b/server/protocol.def index 5d60e7fcda3..2ea410afea7 100644 --- a/server/protocol.def +++ b/server/protocol.def @@ -2900,11 +2900,12 @@ enum coords_relative
/* Set the system foreground window */ @REQ(set_foreground_window) - user_handle_t handle; /* handle to the foreground window */ + user_handle_t handle; /* handle to the foreground window */ @REPLY - user_handle_t previous; /* handle to the previous foreground window */ - int send_msg_old; /* whether we have to send a msg to the old window */ - int send_msg_new; /* whether we have to send a msg to the new window */ + user_handle_t previous; /* handle to the previous foreground window */ + int send_msg_old; /* whether we have to send a msg to the old window */ + int send_msg_new; /* whether we have to send a msg to the new window */ + int deactivate_old; /* whether we have to explicitly deactivate the old window */ @END
/* Set the current thread focus window */ diff --git a/server/queue.c b/server/queue.c index 0558bd111f9..0d19039e05f 100644 --- a/server/queue.c +++ b/server/queue.c @@ -3216,11 +3216,13 @@ DECL_HANDLER(set_foreground_window) reply->previous = desktop->foreground_input ? desktop->foreground_input->active : 0; reply->send_msg_old = (reply->previous && desktop->foreground_input != queue->input); reply->send_msg_new = FALSE; + reply->deactivate_old = FALSE;
if (is_valid_foreground_window( req->handle ) && (thread = get_window_thread( req->handle )) && thread->queue->input->desktop == desktop) { + reply->deactivate_old = (desktop->foreground_input != thread->queue->input); set_foreground_input( desktop, thread->queue->input ); reply->send_msg_new = (desktop->foreground_input != queue->input); } diff --git a/server/request.h b/server/request.h index 89d5a621b16..5185229b483 100644 --- a/server/request.h +++ b/server/request.h @@ -1808,6 +1808,7 @@ C_ASSERT( sizeof(struct set_foreground_window_request) == 16 ); C_ASSERT( FIELD_OFFSET(struct set_foreground_window_reply, previous) == 8 ); C_ASSERT( FIELD_OFFSET(struct set_foreground_window_reply, send_msg_old) == 12 ); C_ASSERT( FIELD_OFFSET(struct set_foreground_window_reply, send_msg_new) == 16 ); +C_ASSERT( FIELD_OFFSET(struct set_foreground_window_reply, deactivate_old) == 20 ); C_ASSERT( sizeof(struct set_foreground_window_reply) == 24 ); C_ASSERT( FIELD_OFFSET(struct set_focus_window_request, handle) == 12 ); C_ASSERT( sizeof(struct set_focus_window_request) == 16 ); diff --git a/server/trace.c b/server/trace.c index 1b65d2b977e..141acebb85b 100644 --- a/server/trace.c +++ b/server/trace.c @@ -3534,6 +3534,7 @@ static void dump_set_foreground_window_reply( const struct set_foreground_window fprintf( stderr, " previous=%08x", req->previous ); fprintf( stderr, ", send_msg_old=%d", req->send_msg_old ); fprintf( stderr, ", send_msg_new=%d", req->send_msg_new ); + fprintf( stderr, ", deactivate_old=%d", req->deactivate_old ); }
static void dump_set_focus_window_request( const struct set_focus_window_request *req )
Hi,
It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated.
The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=140712
Your paranoid android.
=== debian11 (build log) ===
makedep: error: open pointer-constraints-unstable-v1.xml : No such file or directory config.status: error: could not create Makefile Task: The win32 Wine build failed
=== debian11b (build log) ===
makedep: error: open pointer-constraints-unstable-v1.xml : No such file or directory config.status: error: could not create Makefile Task: The wow64 Wine build failed
Rémi Bernon (@rbernon) commented about dlls/winewayland.drv/wayland_pointer.c:
- RECT window_clip;
- TRACE("clip=%s reset=%d\n", wine_dbgstr_rect(clip), reset);
- if (NtUserGetWindowThread(hwnd, NULL) != GetCurrentThreadId())
- {
pthread_mutex_lock(&pointer->mutex);
if (pointer->zwp_confined_pointer_v1)
{
zwp_confined_pointer_v1_destroy(pointer->zwp_confined_pointer_v1);
pointer->zwp_confined_pointer_v1 = NULL;
TRACE("Unconfining due to foreground thread mismatch\n");
}
pthread_mutex_unlock(&pointer->mutex);
return TRUE;
- }
How can it happen exactly that we have an active pointer grab but still end up with the pointer entering a different window (or even a different thread's)? Does it even matter? And if it does, do we actually need to unconfine the cursor then?
Rémi Bernon (@rbernon) commented about dlls/winewayland.drv/wayland_pointer.c:
- TRACE("clip=%s reset=%d\n", wine_dbgstr_rect(clip), reset);
- if (NtUserGetWindowThread(hwnd, NULL) != GetCurrentThreadId())
- {
pthread_mutex_lock(&pointer->mutex);
if (pointer->zwp_confined_pointer_v1)
{
zwp_confined_pointer_v1_destroy(pointer->zwp_confined_pointer_v1);
pointer->zwp_confined_pointer_v1 = NULL;
TRACE("Unconfining due to foreground thread mismatch\n");
}
pthread_mutex_unlock(&pointer->mutex);
return TRUE;
- }
- if (!reset && clip)
Fwiw clip is always NULL when reset is set. It is meant to notify the clipping thread (as well as the desktop in case the clipping thread is unresponsive) that they should release any active grab.
Rémi Bernon (@rbernon) commented about dlls/winewayland.drv/Makefile.in:
SOURCES = \ display.c \ dllmain.c \
- pointer-constraints-unstable-v1.xml \
How "unstable" is this?
Rémi Bernon (@rbernon) commented about dlls/winewayland.drv/Makefile.in:
display.c \ dllmain.c \ pointer-constraints-unstable-v1.xml \
- relative-pointer-unstable-v1.xml \
Same question, why is this "unstable" and how much of an issue is it?
Rémi Bernon (@rbernon) commented about dlls/winewayland.drv/wayland_pointer.c:
__wine_send_input(hwnd, &input, NULL);
}
+static void pointer_handle_motion(void *data, struct wl_pointer *wl_pointer,
uint32_t time, wl_fixed_t sx, wl_fixed_t sy)
+{
- struct wayland_pointer *pointer = &process_wayland.pointer;
- /* Ignore absolute motion events if in relative mode. */
- if (pointer->zwp_relative_pointer_v1) return;
We may want to keep the absolute motion too to avoid any skew with the relative motion? I believe winex11 does that, and wineserver would skip any unnecessary moves.
Rémi Bernon (@rbernon) commented about dlls/winewayland.drv/wayland_pointer.c:
pointer->cursor.wl_surface, pointer->cursor.hotspot_x, pointer->cursor.hotspot_y);
wayland_pointer_update_relative();
Do we want to do that all the time? Are we sure that we can track relative pointer motion accurately wrt all the window motions and scaling involved?
I think it might only be useful when the cursor is confined, maybe worth doing it like winex11?
Rémi Bernon (@rbernon) commented about dlls/winewayland.drv/wayland_pointer.c:
- if (!(surface = wayland_surface_lock_hwnd(hwnd))) return;
- window_rect = surface->window.rect;
- wayland_surface_coords_to_window(surface,
wl_fixed_to_double(dx),
wl_fixed_to_double(dy),
(int *)&screen.x, (int *)&screen.y);
- pthread_mutex_unlock(&surface->mutex);
- if (screen.x >= 0)
- {
origin.x = window_rect.left;
screen.x += origin.x;
if (screen.x >= window_rect.right) screen.x = window_rect.right - 1;
Why forbidding relative motion larger than the window?
On Mon Dec 4 18:25:42 2023 +0000, Rémi Bernon wrote:
How "unstable" is this?
I would say the name is a little misleading, I think every application that does pointer constraints uses it.
When trying to compile this patch I get the following error: `makedep: error: open pointer-constraints-unstable-v1.xml : No such file or directory`
And then when I checked this MR again, I noticed that the XML files were not present. Could you please add them?
On Mon Dec 4 20:09:29 2023 +0000, Etaash Mathamsetty wrote:
I would say the name is a little misleading, I think every application that does pointer constraints uses it.
Currently this is part of wayland unstable protocols. According to READMEmd from wayland-protocols: An "unstable" protocol refers to a protocol categorization policy previously used by wayland-protocols, where protocols initially placed in the `unstable/` directory had certain naming conventions were applied, requiring a backward incompatible change to be declared "stable".
I tested mouse input with this patch in Cyberpunk 2077. It works with manually adding `pointer-constraints-unstable-v1.xml` and `relative-pointer-unstable-v1.xml` for the build. But there is another issue - the desktop cursor doesn't disappear (KDE Plasma 5.27.9, Wayland session).
On Mon Dec 4 20:28:41 2023 +0000, Vijay Kiran Kamuju wrote:
Currently this is part of wayland unstable protocols. According to READMEmd from wayland-protocols: An "unstable" protocol refers to a protocol categorization policy previously used by wayland-protocols, where protocols initially placed in the `unstable/` directory had certain naming conventions were applied, requiring a backward incompatible change to be declared "stable".
The question is whether this is ever going to be renamed to something else (like "stable", and prefixes from "zwp_" to "wp_") when moving them out of unstable (as they want to do it in https://gitlab.freedesktop.org/wayland/wayland-protocols/-/merge_requests/25...), and how certain we can be that nobody will drop the "unstable" flavor in the future and break old Wine versions that were using it.
From that MR discussion it looks like they are in an awkward position with these unstable protocols as many people have started implementing and using it in production, which is quite unfortunate for something that was supposed to be a draft.
I understand from https://gitlab.freedesktop.org/wayland/wayland-protocols/-/merge_requests/25... that these protocols are meant to be moved to "staging" instead, so that probably means they are then supposed to be supported forever, but it is still not yet the case.
Realistically we can probably assume that, but I can only say how ugly if feels.
On Tue Dec 5 10:28:36 2023 +0000, Rémi Bernon wrote:
The question is whether this is ever going to be renamed to something else (like "stable", and prefixes from "zwp_" to "wp_") when moving them out of unstable (as they want to do it in https://gitlab.freedesktop.org/wayland/wayland-protocols/-/merge_requests/25...), and how certain we can be that nobody will drop the "unstable" flavor in the future and break old Wine versions that were using it. From that MR discussion it looks like they are in an awkward position with these unstable protocols as many people have started implementing and using it in production, which is quite unfortunate for something that was supposed to be a draft. I understand from https://gitlab.freedesktop.org/wayland/wayland-protocols/-/merge_requests/25... that these protocols are meant to be moved to "staging" instead, so that probably means they are then supposed to be supported forever, but it is still not yet the case. Realistically we can probably assume that, but I can only say how ugly if feels.
The situation with "unstable" is indeed unfortunate, and hence why the community has moved to the "staging" approach (i.e., no name changes when becoming stable). Practically I don't expect this to be an issue, since these protocols are used very widely and breaking compatibility (e.g., renaming zwp -> wp without also keeping support for the old name) will be a total nightmare.
Besides the naming aspect, the stability of the pointer-constraints protocol is attested by:
1. It's in the list of protocols considered in the move-to-staging wayland-protocols MR above. 2. It's implemented by all major compositors: https://wayland.app/protocols/pointer-constraints-unstable-v1#compositor-sup... 3. Hasn't had any functional change since its introduction https://gitlab.freedesktop.org/wayland/wayland-protocols/-/commits/main/unst...
On Mon Dec 4 18:25:44 2023 +0000, Rémi Bernon wrote:
Same question, why is this "unstable" and how much of an issue is it?
Same answer as for pointer-constraints.
On Mon Dec 4 18:25:45 2023 +0000, Rémi Bernon wrote:
We may want to keep the absolute motion too to avoid any skew with the relative motion? I believe winex11 does that, and wineserver would skip any unnecessary moves.
At the moment we can't enable both if we want to support mouselook, since we don't support `SetCursorPos` and the absolute positions between Wine and Wayland will diverge. This will confuse apps that calculate relative motion by using the diff of the current cursor position from an origin point to which they return the cursor with `SetCursorPos`.
On Mon Dec 4 18:25:47 2023 +0000, Rémi Bernon wrote:
Do we want to do that all the time? Are we sure that we can track relative pointer motion accurately wrt all the window motions and scaling involved? I think it might only be useful when the cursor is confined, maybe worth doing it like winex11?
At the moment we enable relative motion when the cursor is hidden, since it's the simplest condition that makes sense for mouselook scenarios.
An active confinement also makes sense as an additional check for mouselook (in fact the exp. branch takes it into account) to reduce the cases where we switch to relative motion when absolute motion would be preferable.
On Mon Dec 4 18:25:49 2023 +0000, Rémi Bernon wrote:
Why forbidding relative motion larger than the window?
This is to keep the `NtUserLogicalToPerMonitorDPIPhysicalPoint` call happy, since it will fail if the point is outside the window rect.
I have tried to stick with the current win32u exports, to avoid leaking some of the current internal assumptions (e.g., `get_win_monitor_dpi(hwnd) == system_dpi`). Perhaps at some point it may be worth exposing some general facilities from win32u for drivers to perform such dpi mappings with more versatility.
That's also the reason I have left a 'FIXME' for thread->win dpi mapping in the WAYLAND_ClipCursor code, since getting the current thread dpi for calculations will require a duplication of the related internal win32u code.
On Tue Dec 5 11:01:39 2023 +0000, Etaash Mathamsetty wrote:
When trying to compile this patch I get the following error: `makedep: error: open pointer-constraints-unstable-v1.xml : No such file or directory` And then when I checked this MR again, I noticed that the XML files were not present. Could you please add them?
Ack, I somehow missed them in the commits.
On Tue Dec 5 11:01:39 2023 +0000, Shmerl wrote:
I tested mouse input with this patch in Cyberpunk 2077. It works with manually adding `pointer-constraints-unstable-v1.xml` and `relative-pointer-unstable-v1.xml` for the build. But there is another issue - the desktop cursor doesn't disappear (KDE Plasma 5.27.9, Wayland session). Alt+Tabb'ing from the game and back in hides it though.
I have also seen this with KDE, and my current inclination is to say that this is a compositor problem. Looking at the logs it seems the driver is issuing the proper requests, but the cursor remains visible.
On Mon Dec 4 18:25:40 2023 +0000, Rémi Bernon wrote:
Fwiw clip is always NULL when reset is set. It is meant to notify the clipping thread (as well as the desktop in case the clipping thread is unresponsive) that they should release any active grab.
Ack, will check for clip only.
On Mon Dec 4 18:25:38 2023 +0000, Rémi Bernon wrote:
How can it happen exactly that we have an active pointer grab but still end up with the pointer entering a different window (or even a different thread's)? Does it even matter? And if it does, do we actually need to unconfine the cursor then?
Pointer constraints are applied per-surface and can be removed and reapplied at any time by the compositor (we can actually track this through events, but I haven't found much benefit in doing so).
Also, from what I understand, it's the foreground thread that receives the ClipCursor callback, but that can become out of sync with the currently pointer focused_hwnd which I am using here.
But now you got me thinking that acting on the currently Wayland focused hwnd is not the right approach, and I should be using the foreground hwnd instead, regardless of its Wayland focus state. This guarantees that this callback will always be called in the matching (i.e., foreground) thread (except possibly the reset case), so no check is required, correct? Of course the constraint will likely be applied by the compositor only when the surface it gets the Wayland focus. Does this make more sense?
On Tue Dec 5 11:01:38 2023 +0000, Alexandros Frantzis wrote:
This is to keep the `NtUserLogicalToPerMonitorDPIPhysicalPoint` call happy, since it will fail if the point is outside the window rect. I have tried to stick with the current win32u exports, to avoid leaking some of the current internal assumptions (e.g., `get_win_monitor_dpi(hwnd) == system_dpi`). Perhaps at some point it may be worth exposing some general facilities from win32u for drivers to perform such dpi mappings with more versatility. That's also the reason I have left a 'FIXME' for thread->win dpi mapping in the WAYLAND_ClipCursor code, since getting the current thread dpi for calculations will require a duplication of the related internal win32u code.
Okay, I don't have a good knowledge of this DPI stuff, I don't have a good idea what to do yet. Probably fine to keep it like that but maybe with a comment there too.
Also, from what I understand, it's the foreground thread that receives the ClipCursor callback, but that can become out of sync with the currently pointer focused_hwnd which I am using here.
Yes. And focus_hwnd might very well a window belonging to the same process but a different thread, if the cursor is outside of the active window, or somehow the other window is covering it.
This guarantees that this callback will always be called in the matching (i.e., foreground) thread (except possibly the reset case), so no check is required, correct? Of course the constraint will likely be applied by the compositor only when the surface it gets the Wayland focus.
I think as well.
On Tue Dec 5 11:01:39 2023 +0000, Alexandros Frantzis wrote:
I have also seen this with KDE, and my current inclination is to say that this is a compositor problem. Looking at the logs it seems the driver is issuing the proper requests, but the cursor remains visible.
Can you share the wayland debug logs somewhere please?
On Tue Dec 5 14:10:07 2023 +0000, Alexandros Frantzis wrote:
At the moment we enable relative motion when the cursor is hidden, since it's the simplest condition that makes sense for mouselook scenarios. An active confinement also makes sense as an additional check for mouselook (in fact the exp. branch takes it into account) to reduce the cases where we switch to relative motion when absolute motion would be preferable.
Alright.
On Tue Dec 5 11:01:38 2023 +0000, Alexandros Frantzis wrote:
At the moment we can't enable both if we want to support mouselook, since we don't support `SetCursorPos` and the absolute positions between Wine and Wayland will diverge. This will confuse apps that calculate relative motion by using the diff of the current cursor position from an origin point to which they return the cursor with `SetCursorPos`.
Okay. Fwiw some of these calls might very well be coming from dinput, and possibly some could be unnecessary. I think in some cases we were using SetCursorPos even though ClipCursor is working fine enough. Anyway, applications might be doing that themselves too.
On Tue Dec 5 13:38:42 2023 +0000, Vlad Zahorodnii wrote:
Can you share the wayland debug logs somewhere please?
See [wine_wayland.log.xz](/uploads/890b174b1edca3c296e5610d94cb5da4/wine_wayland.log.xz)