[PATCH v3 0/5] MR2944: winewayland.drv: First window contents on screen
The goal of this MR is to set up the minimum necessary infrastructure to display the contents of some simple, software rendered windows. This involves two major steps: 1. Associate a window with a Wayland surface and give it the `xdg_toplevel` role, so that the compositor can display it. We also have to implement the required initial `xdg_surface` configuration sequence to be able to safely (i.e., without the compositor disconnecting us with an error) attach buffers to the surface in step (2). 2. Implement the `window_surface` interface for the Wayland driver. For now we provide a simple (and suboptimal) `window_surface_flush` implementation: for each flush we create a new `wl_shm` buffer, copy the whole window contents (ignoring damaged bounds for now) into it and attach it to the target Wayland surface. In the next MR I will optimize this implementation in multiple ways: a. implement a buffer queue to avoid constantly allocating new buffers b. respect the damaged bounds of the `window_surface` to minimize copying of data from the `window_surface` to the SHM buffer (and also for correctness) c. communicate damaged surface regions to the compositor to (potentially) allow more efficient texture uploads. With this MR many (software-rendered) applications can now display dynamic content on screen. We can't interact with the apps yet, but we do get to enjoy `notepad` in all its blinking-cursor glory. Thanks! -- v3: winewayland.drv: Implement a simple window_surface flush. winewayland.drv: Introduce window_surface for Wayland. winewayland.drv: Ensure Wayland surface handlers don't access invalid data. winewayland.drv: Basic Wayland toplevel surface support. winewayland.drv: Introduce per-window driver data. https://gitlab.winehq.org/wine/wine/-/merge_requests/2944
From: Alexandros Frantzis <alexandros.frantzis(a)collabora.com> Create and manage an internal driver data structure for each non-desktop, non-message window. --- dlls/winewayland.drv/waylanddrv.h | 4 + dlls/winewayland.drv/waylanddrv_main.c | 4 +- dlls/winewayland.drv/window.c | 139 +++++++++++++++++++++++++ 3 files changed, 146 insertions(+), 1 deletion(-) diff --git a/dlls/winewayland.drv/waylanddrv.h b/dlls/winewayland.drv/waylanddrv.h index 57125e011cb..ed1d3ea0be2 100644 --- a/dlls/winewayland.drv/waylanddrv.h +++ b/dlls/winewayland.drv/waylanddrv.h @@ -111,8 +111,12 @@ void wayland_output_use_xdg_extension(struct wayland_output *output) DECLSPEC_HI */ LRESULT WAYLAND_DesktopWindowProc(HWND hwnd, UINT msg, WPARAM wp, LPARAM lp) DECLSPEC_HIDDEN; +void WAYLAND_DestroyWindow(HWND hwnd) DECLSPEC_HIDDEN; BOOL WAYLAND_UpdateDisplayDevices(const struct gdi_device_manager *device_manager, BOOL force, void *param) DECLSPEC_HIDDEN; LRESULT WAYLAND_WindowMessage(HWND hwnd, UINT msg, WPARAM wp, LPARAM lp) DECLSPEC_HIDDEN; +BOOL WAYLAND_WindowPosChanging(HWND hwnd, HWND insert_after, UINT swp_flags, + const RECT *window_rect, const RECT *client_rect, + RECT *visible_rect, struct window_surface **surface) DECLSPEC_HIDDEN; #endif /* __WINE_WAYLANDDRV_H */ diff --git a/dlls/winewayland.drv/waylanddrv_main.c b/dlls/winewayland.drv/waylanddrv_main.c index 2a7bed9d1d8..3ef03e16d0b 100644 --- a/dlls/winewayland.drv/waylanddrv_main.c +++ b/dlls/winewayland.drv/waylanddrv_main.c @@ -32,8 +32,10 @@ static const struct user_driver_funcs waylanddrv_funcs = { .pDesktopWindowProc = WAYLAND_DesktopWindowProc, + .pDestroyWindow = WAYLAND_DestroyWindow, .pUpdateDisplayDevices = WAYLAND_UpdateDisplayDevices, - .pWindowMessage = WAYLAND_WindowMessage + .pWindowMessage = WAYLAND_WindowMessage, + .pWindowPosChanging = WAYLAND_WindowPosChanging }; static NTSTATUS waylanddrv_unix_init(void *arg) diff --git a/dlls/winewayland.drv/window.c b/dlls/winewayland.drv/window.c index ea5ac22a8a2..9eeaaea357d 100644 --- a/dlls/winewayland.drv/window.c +++ b/dlls/winewayland.drv/window.c @@ -28,8 +28,147 @@ #include "wine/debug.h" +#include <assert.h> +#include <stdlib.h> + WINE_DEFAULT_DEBUG_CHANNEL(waylanddrv); +/* private window data */ +struct wayland_win_data +{ + struct rb_entry entry; + /* hwnd that this private data belongs to */ + HWND hwnd; +}; + +static int wayland_win_data_cmp_rb(const void *key, + const struct rb_entry *entry) +{ + HWND key_hwnd = (HWND)key; /* cast to work around const */ + const struct wayland_win_data *entry_win_data = + RB_ENTRY_VALUE(entry, const struct wayland_win_data, entry); + + if (key_hwnd < entry_win_data->hwnd) return -1; + if (key_hwnd > entry_win_data->hwnd) return 1; + return 0; +} + +static pthread_mutex_t win_data_mutex = PTHREAD_MUTEX_INITIALIZER; +static struct rb_tree win_data_rb = { wayland_win_data_cmp_rb }; + +/*********************************************************************** + * wayland_win_data_create + * + * Create a data window structure for an existing window. + */ +static struct wayland_win_data *wayland_win_data_create(HWND hwnd) +{ + struct wayland_win_data *data; + struct rb_entry *rb_entry; + HWND parent; + + /* Don't create win data for desktop or HWND_MESSAGE windows. */ + if (!(parent = NtUserGetAncestor(hwnd, GA_PARENT))) return NULL; + if (parent != NtUserGetDesktopWindow() && !NtUserGetAncestor(parent, GA_PARENT)) + return NULL; + + if (!(data = calloc(1, sizeof(*data)))) return NULL; + + data->hwnd = hwnd; + + pthread_mutex_lock(&win_data_mutex); + + /* Check that another thread hasn't already created the wayland_win_data. */ + if ((rb_entry = rb_get(&win_data_rb, hwnd))) + { + free(data); + return RB_ENTRY_VALUE(rb_entry, struct wayland_win_data, entry); + } + + rb_put(&win_data_rb, hwnd, &data->entry); + + TRACE("hwnd=%p\n", data->hwnd); + + return data; +} + +/*********************************************************************** + * wayland_win_data_destroy + */ +static void wayland_win_data_destroy(struct wayland_win_data *data) +{ + TRACE("hwnd=%p\n", data->hwnd); + + rb_remove(&win_data_rb, &data->entry); + + pthread_mutex_unlock(&win_data_mutex); + + free(data); +} + +/*********************************************************************** + * wayland_win_data_get + * + * Lock and return the data structure associated with a window. + */ +static struct wayland_win_data *wayland_win_data_get(HWND hwnd) +{ + struct rb_entry *rb_entry; + + pthread_mutex_lock(&win_data_mutex); + + if ((rb_entry = rb_get(&win_data_rb, hwnd))) + return RB_ENTRY_VALUE(rb_entry, struct wayland_win_data, entry); + + pthread_mutex_unlock(&win_data_mutex); + + return NULL; +} + +/*********************************************************************** + * wayland_win_data_release + * + * Release the data returned by wayland_win_data_get. + */ +static void wayland_win_data_release(struct wayland_win_data *data) +{ + assert(data); + pthread_mutex_unlock(&win_data_mutex); +} + +/*********************************************************************** + * WAYLAND_DestroyWindow + */ +void WAYLAND_DestroyWindow(HWND hwnd) +{ + struct wayland_win_data *data; + + TRACE("%p\n", hwnd); + + if (!(data = wayland_win_data_get(hwnd))) return; + wayland_win_data_destroy(data); +} + +/*********************************************************************** + * WAYLAND_WindowPosChanging + */ +BOOL WAYLAND_WindowPosChanging(HWND hwnd, HWND insert_after, UINT swp_flags, + const RECT *window_rect, const RECT *client_rect, + RECT *visible_rect, struct window_surface **surface) +{ + struct wayland_win_data *data = wayland_win_data_get(hwnd); + + TRACE("hwnd %p window %s client %s visible %s after %p flags %08x\n", + hwnd, wine_dbgstr_rect(window_rect), wine_dbgstr_rect(client_rect), + wine_dbgstr_rect(visible_rect), insert_after, swp_flags); + + if (!data && !(data = wayland_win_data_create(hwnd))) return TRUE; + + wayland_win_data_release(data); + + return TRUE; +} + static void wayland_resize_desktop(void) { RECT virtual_rect = NtUserGetVirtualScreenRect(); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2944
From: Alexandros Frantzis <alexandros.frantzis(a)collabora.com> Use the xdg-shell Wayland protocol to create Wayland xdg_surface xdg_toplevel surfaces for non-child windows. Perform some minimal handling of the required initial xdg_surface configuration sequence to ensure we are able to safely attach buffers to the surface. --- dlls/winewayland.drv/Makefile.in | 4 +- dlls/winewayland.drv/wayland.c | 41 + dlls/winewayland.drv/wayland_surface.c | 197 ++++ dlls/winewayland.drv/waylanddrv.h | 27 + dlls/winewayland.drv/waylanddrv_main.c | 1 + dlls/winewayland.drv/window.c | 118 +++ dlls/winewayland.drv/xdg-shell.xml | 1351 ++++++++++++++++++++++++ 7 files changed, 1738 insertions(+), 1 deletion(-) create mode 100644 dlls/winewayland.drv/wayland_surface.c create mode 100644 dlls/winewayland.drv/xdg-shell.xml diff --git a/dlls/winewayland.drv/Makefile.in b/dlls/winewayland.drv/Makefile.in index 454a7e5a4b4..9a682e7e007 100644 --- a/dlls/winewayland.drv/Makefile.in +++ b/dlls/winewayland.drv/Makefile.in @@ -9,6 +9,8 @@ SOURCES = \ version.rc \ wayland.c \ wayland_output.c \ + wayland_surface.c \ waylanddrv_main.c \ window.c \ - xdg-output-unstable-v1.xml + xdg-output-unstable-v1.xml \ + xdg-shell.xml diff --git a/dlls/winewayland.drv/wayland.c b/dlls/winewayland.drv/wayland.c index 34aa8bec04e..357dc4208d3 100644 --- a/dlls/winewayland.drv/wayland.c +++ b/dlls/winewayland.drv/wayland.c @@ -38,6 +38,21 @@ struct wayland process_wayland = .output_mutex = PTHREAD_MUTEX_INITIALIZER }; +/********************************************************************** + * xdg_wm_base handling + */ + +static void xdg_wm_base_handle_ping(void *data, struct xdg_wm_base *shell, + uint32_t serial) +{ + xdg_wm_base_pong(shell, serial); +} + +static const struct xdg_wm_base_listener xdg_wm_base_listener = +{ + xdg_wm_base_handle_ping +}; + /********************************************************************** * Registry handling */ @@ -65,6 +80,20 @@ static void registry_handle_global(void *data, struct wl_registry *registry, wl_list_for_each(output, &process_wayland.output_list, link) wayland_output_use_xdg_extension(output); } + else if (strcmp(interface, "wl_compositor") == 0) + { + process_wayland.wl_compositor = + wl_registry_bind(registry, id, &wl_compositor_interface, 4); + } + else if (strcmp(interface, "xdg_wm_base") == 0) + { + /* Bind version 2 so that compositors (e.g., sway) can properly send tiled + * states, instead of falling back to (ab)using the maximized state. */ + process_wayland.xdg_wm_base = + wl_registry_bind(registry, id, &xdg_wm_base_interface, + version < 2 ? version : 2); + xdg_wm_base_add_listener(process_wayland.xdg_wm_base, &xdg_wm_base_listener, NULL); + } } static void registry_handle_global_remove(void *data, struct wl_registry *registry, @@ -136,6 +165,18 @@ BOOL wayland_process_init(void) wl_display_roundtrip_queue(process_wayland.wl_display, process_wayland.wl_event_queue); wl_display_roundtrip_queue(process_wayland.wl_display, process_wayland.wl_event_queue); + /* Check for required protocol globals. */ + if (!process_wayland.wl_compositor) + { + ERR("Wayland compositor doesn't support wl_compositor\n"); + return FALSE; + } + if (!process_wayland.xdg_wm_base) + { + ERR("Wayland compositor doesn't support xdg_wm_base\n"); + return FALSE; + } + wayland_init_display_devices(FALSE); process_wayland.initialized = TRUE; diff --git a/dlls/winewayland.drv/wayland_surface.c b/dlls/winewayland.drv/wayland_surface.c new file mode 100644 index 00000000000..84181ae45f4 --- /dev/null +++ b/dlls/winewayland.drv/wayland_surface.c @@ -0,0 +1,197 @@ +/* + * Wayland surfaces + * + * Copyright 2020 Alexandros Frantzis for Collabora Ltd + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA + */ + +#if 0 +#pragma makedep unix +#endif + +#include "config.h" + +#include "waylanddrv.h" + +#include "wine/debug.h" + +#include <stdlib.h> + +WINE_DEFAULT_DEBUG_CHANNEL(waylanddrv); + +static void xdg_surface_handle_configure(void *data, struct xdg_surface *xdg_surface, + uint32_t serial) +{ + struct wayland_surface *surface = data; + BOOL initial_configure; + + TRACE("serial=%u\n", serial); + + pthread_mutex_lock(&surface->mutex); + /* Handle this event only if wayland_surface is still associated with + * the target xdg_surface. */ + if (surface->xdg_surface && surface->xdg_surface == xdg_surface) + { + initial_configure = surface->pending_serial == 0 && + surface->current_serial == 0; + surface->pending_serial = serial; + if (initial_configure) pthread_cond_signal(&surface->configured_cond); + } + pthread_mutex_unlock(&surface->mutex); +} + +static const struct xdg_surface_listener xdg_surface_listener = +{ + xdg_surface_handle_configure +}; + +/********************************************************************** + * wayland_surface_create + * + * Creates a role-less wayland surface. + */ +struct wayland_surface *wayland_surface_create(void) +{ + struct wayland_surface *surface; + + surface = calloc(1, sizeof(*surface)); + if (!surface) + { + ERR("Failed to allocate space for Wayland surface\n"); + goto err; + } + + TRACE("surface=%p\n", surface); + + pthread_mutex_init(&surface->mutex, NULL); + pthread_cond_init(&surface->configured_cond, NULL); + + surface->wl_surface = wl_compositor_create_surface(process_wayland.wl_compositor); + if (!surface->wl_surface) + { + ERR("Failed to create wl_surface Wayland surface\n"); + goto err; + } + + return surface; + +err: + if (surface) wayland_surface_destroy(surface); + return NULL; +} + +/********************************************************************** + * wayland_surface_destroy + * + * Destroys a wayland surface. + */ +void wayland_surface_destroy(struct wayland_surface *surface) +{ + pthread_mutex_lock(&surface->mutex); + + if (surface->xdg_toplevel) + { + xdg_toplevel_destroy(surface->xdg_toplevel); + surface->xdg_toplevel = NULL; + } + + if (surface->xdg_surface) + { + xdg_surface_destroy(surface->xdg_surface); + surface->xdg_surface = NULL; + } + + if (surface->wl_surface) + { + wl_surface_destroy(surface->wl_surface); + surface->wl_surface = NULL; + } + + pthread_mutex_unlock(&surface->mutex); + + wl_display_flush(process_wayland.wl_display); + + pthread_cond_destroy(&surface->configured_cond); + pthread_mutex_destroy(&surface->mutex); + + free(surface); +} + +/********************************************************************** + * wayland_surface_make_toplevel + * + * Gives the toplevel role to a plain wayland surface. + * + * Must be called under the surface lock. + */ +void wayland_surface_make_toplevel(struct wayland_surface *surface) +{ + TRACE("surface=%p\n", surface); + + surface->xdg_surface = + xdg_wm_base_get_xdg_surface(process_wayland.xdg_wm_base, surface->wl_surface); + if (!surface->xdg_surface) goto err; + xdg_surface_add_listener(surface->xdg_surface, &xdg_surface_listener, surface); + + surface->xdg_toplevel = xdg_surface_get_toplevel(surface->xdg_surface); + if (!surface->xdg_toplevel) goto err; + + wl_surface_commit(surface->wl_surface); + wl_display_flush(process_wayland.wl_display); + + /* Wait for the initial configure event */ + while (!surface->pending_serial) + pthread_cond_wait(&surface->configured_cond, &surface->mutex); + + return; + +err: + wayland_surface_clear_role(surface); + ERR("Failed to assign toplevel role to wayland surface\n"); +} + +/********************************************************************** + * wayland_surface_clear_role + * + * Clears the role related Wayland objects of a Wayland surface, making it a + * plain surface again. We can later assign the same role (but not a + * different one!) to the surface. + */ +void wayland_surface_clear_role(struct wayland_surface *surface) +{ + TRACE("surface=%p\n", surface); + + if (surface->xdg_toplevel) + { + xdg_toplevel_destroy(surface->xdg_toplevel); + surface->xdg_toplevel = NULL; + } + + if (surface->xdg_surface) + { + xdg_surface_destroy(surface->xdg_surface); + surface->xdg_surface = NULL; + } + + surface->pending_serial = 0; + surface->current_serial = 0; + + /* Ensure no buffer is attached, otherwise future role assignments may fail. */ + wl_surface_attach(surface->wl_surface, NULL, 0, 0); + wl_surface_commit(surface->wl_surface); + + wl_display_flush(process_wayland.wl_display); +} diff --git a/dlls/winewayland.drv/waylanddrv.h b/dlls/winewayland.drv/waylanddrv.h index ed1d3ea0be2..4c42dcd6268 100644 --- a/dlls/winewayland.drv/waylanddrv.h +++ b/dlls/winewayland.drv/waylanddrv.h @@ -28,6 +28,7 @@ #include <pthread.h> #include <wayland-client.h> #include "xdg-output-unstable-v1-client-protocol.h" +#include "xdg-shell-client-protocol.h" #include "windef.h" #include "winbase.h" @@ -58,6 +59,8 @@ struct wayland struct wl_event_queue *wl_event_queue; struct wl_registry *wl_registry; struct zxdg_output_manager_v1 *zxdg_output_manager_v1; + struct wl_compositor *wl_compositor; + struct xdg_wm_base *xdg_wm_base; struct wl_list output_list; /* Protects the output_list and the wayland_output.current states. */ pthread_mutex_t output_mutex; @@ -91,6 +94,17 @@ struct wayland_output struct wayland_output_state current; }; +struct wayland_surface +{ + struct wl_surface *wl_surface; + struct xdg_surface *xdg_surface; + struct xdg_toplevel *xdg_toplevel; + pthread_mutex_t mutex; + pthread_cond_t configured_cond; + uint32_t pending_serial; + uint32_t current_serial; +}; + /********************************************************************** * Wayland initialization */ @@ -106,6 +120,15 @@ BOOL wayland_output_create(uint32_t id, uint32_t version) DECLSPEC_HIDDEN; void wayland_output_destroy(struct wayland_output *output) DECLSPEC_HIDDEN; void wayland_output_use_xdg_extension(struct wayland_output *output) DECLSPEC_HIDDEN; +/********************************************************************** + * Wayland surface + */ + +struct wayland_surface *wayland_surface_create(void) DECLSPEC_HIDDEN; +void wayland_surface_destroy(struct wayland_surface *surface) DECLSPEC_HIDDEN; +void wayland_surface_make_toplevel(struct wayland_surface *surface) DECLSPEC_HIDDEN; +void wayland_surface_clear_role(struct wayland_surface *surface) DECLSPEC_HIDDEN; + /********************************************************************** * USER driver functions */ @@ -115,6 +138,10 @@ void WAYLAND_DestroyWindow(HWND hwnd) DECLSPEC_HIDDEN; BOOL WAYLAND_UpdateDisplayDevices(const struct gdi_device_manager *device_manager, BOOL force, void *param) DECLSPEC_HIDDEN; LRESULT WAYLAND_WindowMessage(HWND hwnd, UINT msg, WPARAM wp, LPARAM lp) DECLSPEC_HIDDEN; +void WAYLAND_WindowPosChanged(HWND hwnd, HWND insert_after, UINT swp_flags, + const RECT *window_rect, const RECT *client_rect, + const RECT *visible_rect, const RECT *valid_rects, + struct window_surface *surface) DECLSPEC_HIDDEN; BOOL WAYLAND_WindowPosChanging(HWND hwnd, HWND insert_after, UINT swp_flags, const RECT *window_rect, const RECT *client_rect, RECT *visible_rect, struct window_surface **surface) DECLSPEC_HIDDEN; diff --git a/dlls/winewayland.drv/waylanddrv_main.c b/dlls/winewayland.drv/waylanddrv_main.c index 3ef03e16d0b..530cdd83d90 100644 --- a/dlls/winewayland.drv/waylanddrv_main.c +++ b/dlls/winewayland.drv/waylanddrv_main.c @@ -35,6 +35,7 @@ static const struct user_driver_funcs waylanddrv_funcs = .pDestroyWindow = WAYLAND_DestroyWindow, .pUpdateDisplayDevices = WAYLAND_UpdateDisplayDevices, .pWindowMessage = WAYLAND_WindowMessage, + .pWindowPosChanged = WAYLAND_WindowPosChanged, .pWindowPosChanging = WAYLAND_WindowPosChanging }; diff --git a/dlls/winewayland.drv/window.c b/dlls/winewayland.drv/window.c index 9eeaaea357d..fcdc6f0f69b 100644 --- a/dlls/winewayland.drv/window.c +++ b/dlls/winewayland.drv/window.c @@ -39,6 +39,14 @@ struct wayland_win_data struct rb_entry entry; /* hwnd that this private data belongs to */ HWND hwnd; + /* parent hwnd for child windows */ + HWND parent; + /* old parent hwnd for child windows */ + HWND old_parent; + /* wayland surface (if any) for this window */ + struct wayland_surface *wayland_surface; + /* whether this window is visible */ + BOOL visible; }; static int wayland_win_data_cmp_rb(const void *key, @@ -75,6 +83,7 @@ static struct wayland_win_data *wayland_win_data_create(HWND hwnd) if (!(data = calloc(1, sizeof(*data)))) return NULL; data->hwnd = hwnd; + data->parent = (parent == NtUserGetDesktopWindow()) ? 0 : parent; pthread_mutex_lock(&win_data_mutex); @@ -103,6 +112,7 @@ static void wayland_win_data_destroy(struct wayland_win_data *data) pthread_mutex_unlock(&win_data_mutex); + if (data->wayland_surface) wayland_surface_destroy(data->wayland_surface); free(data); } @@ -136,6 +146,81 @@ static void wayland_win_data_release(struct wayland_win_data *data) pthread_mutex_unlock(&win_data_mutex); } +static BOOL wayland_win_data_wayland_surface_needs_update(struct wayland_win_data *data) +{ + BOOL xdg_visible; + + /* If window child status changed, update the wayland surface. */ + if (!data->parent != !data->old_parent) return TRUE; + + /* Don't create wayland surfaces for child windows. */ + if (data->parent) return FALSE; + + /* Otherwise we want a surface if we don't have one. */ + if (!data->wayland_surface) return TRUE; + + /* If this is currently or potentially a toplevel surface, and its + * visibility state has changed, update it so that we have + * an xdg_toplevel iff the window is visible. */ + xdg_visible = data->wayland_surface->xdg_toplevel != NULL; + if (data->visible != xdg_visible) return TRUE; + + return FALSE; +} + +static void wayland_win_data_update_wayland_surface(struct wayland_win_data *data) +{ + struct wayland_surface *surface = data->wayland_surface; + + TRACE("hwnd=%p\n", data->hwnd); + + /* We don't want wayland surfaces for child windows. */ + if (data->parent) + { + if (surface) wayland_surface_destroy(surface); + data->wayland_surface = NULL; + return; + } + + if (!surface) + { + surface = wayland_surface_create(); + if (!surface) return; + pthread_mutex_lock(&surface->mutex); + } + else + { + pthread_mutex_lock(&surface->mutex); + 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 (data->visible) wayland_surface_make_toplevel(surface); + + pthread_mutex_unlock(&surface->mutex); + + data->wayland_surface = surface; +} + +static void wayland_win_data_update_wayland_surface_state(struct wayland_win_data *data) +{ + struct wayland_surface *surface = data->wayland_surface; + + if (surface->xdg_toplevel) + { + pthread_mutex_lock(&surface->mutex); + if (surface->pending_serial) + { + xdg_surface_ack_configure(surface->xdg_surface, surface->pending_serial); + surface->current_serial = surface->pending_serial; + surface->pending_serial = 0; + } + pthread_mutex_unlock(&surface->mutex); + } +} + /*********************************************************************** * WAYLAND_DestroyWindow */ @@ -157,6 +242,8 @@ BOOL WAYLAND_WindowPosChanging(HWND hwnd, HWND insert_after, UINT swp_flags, RECT *visible_rect, struct window_surface **surface) { struct wayland_win_data *data = wayland_win_data_get(hwnd); + DWORD style = NtUserGetWindowLongW(hwnd, GWL_STYLE); + HWND parent = NtUserGetAncestor(hwnd, GA_PARENT); TRACE("hwnd %p window %s client %s visible %s after %p flags %08x\n", hwnd, wine_dbgstr_rect(window_rect), wine_dbgstr_rect(client_rect), @@ -164,11 +251,42 @@ BOOL WAYLAND_WindowPosChanging(HWND hwnd, HWND insert_after, UINT swp_flags, if (!data && !(data = wayland_win_data_create(hwnd))) return TRUE; + data->old_parent = data->parent; + data->parent = (parent == NtUserGetDesktopWindow()) ? 0 : parent; + data->visible = ((style & WS_VISIBLE) == WS_VISIBLE || + (swp_flags & SWP_SHOWWINDOW)) && + !(swp_flags & SWP_HIDEWINDOW); + wayland_win_data_release(data); return TRUE; } +/*********************************************************************** + * WAYLAND_WindowPosChanged + */ +void WAYLAND_WindowPosChanged(HWND hwnd, HWND insert_after, UINT swp_flags, + const RECT *window_rect, const RECT *client_rect, + const RECT *visible_rect, const RECT *valid_rects, + struct window_surface *surface) +{ + struct wayland_win_data *data = wayland_win_data_get(hwnd); + + TRACE("hwnd %p window %s client %s visible %s after %p flags %08x\n", + hwnd, wine_dbgstr_rect(window_rect), wine_dbgstr_rect(client_rect), + wine_dbgstr_rect(visible_rect), insert_after, swp_flags); + + if (!data) return; + + if (wayland_win_data_wayland_surface_needs_update(data)) + wayland_win_data_update_wayland_surface(data); + + if (data->wayland_surface) + wayland_win_data_update_wayland_surface_state(data); + + wayland_win_data_release(data); +} + static void wayland_resize_desktop(void) { RECT virtual_rect = NtUserGetVirtualScreenRect(); diff --git a/dlls/winewayland.drv/xdg-shell.xml b/dlls/winewayland.drv/xdg-shell.xml new file mode 100644 index 00000000000..6eb0a67c5d8 --- /dev/null +++ b/dlls/winewayland.drv/xdg-shell.xml @@ -0,0 +1,1351 @@ +<?xml version="1.0" encoding="UTF-8"?> +<protocol name="xdg_shell"> + + <copyright> + Copyright © 2008-2013 Kristian Høgsberg + Copyright © 2013 Rafael Antognolli + Copyright © 2013 Jasper St. Pierre + Copyright © 2010-2013 Intel Corporation + Copyright © 2015-2017 Samsung Electronics Co., Ltd + Copyright © 2015-2017 Red Hat Inc. + + Permission is hereby granted, free of charge, to any person obtaining a + copy of this software and associated documentation files (the "Software"), + to deal in the Software without restriction, including without limitation + the rights to use, copy, modify, merge, publish, distribute, sublicense, + and/or sell copies of the Software, and to permit persons to whom the + Software is furnished to do so, subject to the following conditions: + + The above copyright notice and this permission notice (including the next + paragraph) shall be included in all copies or substantial portions of the + Software. + + THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER + DEALINGS IN THE SOFTWARE. + </copyright> + + <interface name="xdg_wm_base" version="5"> + <description summary="create desktop-style surfaces"> + The xdg_wm_base interface is exposed as a global object enabling clients + to turn their wl_surfaces into windows in a desktop environment. It + defines the basic functionality needed for clients and the compositor to + create windows that can be dragged, resized, maximized, etc, as well as + creating transient windows such as popup menus. + </description> + + <enum name="error"> + <entry name="role" value="0" summary="given wl_surface has another role"/> + <entry name="defunct_surfaces" value="1" + summary="xdg_wm_base was destroyed before children"/> + <entry name="not_the_topmost_popup" value="2" + summary="the client tried to map or destroy a non-topmost popup"/> + <entry name="invalid_popup_parent" value="3" + summary="the client specified an invalid popup parent surface"/> + <entry name="invalid_surface_state" value="4" + summary="the client provided an invalid surface state"/> + <entry name="invalid_positioner" value="5" + summary="the client provided an invalid positioner"/> + <entry name="unresponsive" value="6" + summary="the client didn’t respond to a ping event in time"/> + </enum> + + <request name="destroy" type="destructor"> + <description summary="destroy xdg_wm_base"> + Destroy this xdg_wm_base object. + + Destroying a bound xdg_wm_base object while there are surfaces + still alive created by this xdg_wm_base object instance is illegal + and will result in a defunct_surfaces error. + </description> + </request> + + <request name="create_positioner"> + <description summary="create a positioner object"> + Create a positioner object. A positioner object is used to position + surfaces relative to some parent surface. See the interface description + and xdg_surface.get_popup for details. + </description> + <arg name="id" type="new_id" interface="xdg_positioner"/> + </request> + + <request name="get_xdg_surface"> + <description summary="create a shell surface from a surface"> + This creates an xdg_surface for the given surface. While xdg_surface + itself is not a role, the corresponding surface may only be assigned + a role extending xdg_surface, such as xdg_toplevel or xdg_popup. It is + illegal to create an xdg_surface for a wl_surface which already has an + assigned role and this will result in a role error. + + This creates an xdg_surface for the given surface. An xdg_surface is + used as basis to define a role to a given surface, such as xdg_toplevel + or xdg_popup. It also manages functionality shared between xdg_surface + based surface roles. + + See the documentation of xdg_surface for more details about what an + xdg_surface is and how it is used. + </description> + <arg name="id" type="new_id" interface="xdg_surface"/> + <arg name="surface" type="object" interface="wl_surface"/> + </request> + + <request name="pong"> + <description summary="respond to a ping event"> + A client must respond to a ping event with a pong request or + the client may be deemed unresponsive. See xdg_wm_base.ping + and xdg_wm_base.error.unresponsive. + </description> + <arg name="serial" type="uint" summary="serial of the ping event"/> + </request> + + <event name="ping"> + <description summary="check if the client is alive"> + The ping event asks the client if it's still alive. Pass the + serial specified in the event back to the compositor by sending + a "pong" request back with the specified serial. See xdg_wm_base.pong. + + Compositors can use this to determine if the client is still + alive. It's unspecified what will happen if the client doesn't + respond to the ping request, or in what timeframe. Clients should + try to respond in a reasonable amount of time. The “unresponsive” + error is provided for compositors that wish to disconnect unresponsive + clients. + + A compositor is free to ping in any way it wants, but a client must + always respond to any xdg_wm_base object it created. + </description> + <arg name="serial" type="uint" summary="pass this to the pong request"/> + </event> + </interface> + + <interface name="xdg_positioner" version="5"> + <description summary="child surface positioner"> + The xdg_positioner provides a collection of rules for the placement of a + child surface relative to a parent surface. Rules can be defined to ensure + the child surface remains within the visible area's borders, and to + specify how the child surface changes its position, such as sliding along + an axis, or flipping around a rectangle. These positioner-created rules are + constrained by the requirement that a child surface must intersect with or + be at least partially adjacent to its parent surface. + + See the various requests for details about possible rules. + + At the time of the request, the compositor makes a copy of the rules + specified by the xdg_positioner. Thus, after the request is complete the + xdg_positioner object can be destroyed or reused; further changes to the + object will have no effect on previous usages. + + For an xdg_positioner object to be considered complete, it must have a + non-zero size set by set_size, and a non-zero anchor rectangle set by + set_anchor_rect. Passing an incomplete xdg_positioner object when + positioning a surface raises an invalid_positioner error. + </description> + + <enum name="error"> + <entry name="invalid_input" value="0" summary="invalid input provided"/> + </enum> + + <request name="destroy" type="destructor"> + <description summary="destroy the xdg_positioner object"> + Notify the compositor that the xdg_positioner will no longer be used. + </description> + </request> + + <request name="set_size"> + <description summary="set the size of the to-be positioned rectangle"> + Set the size of the surface that is to be positioned with the positioner + object. The size is in surface-local coordinates and corresponds to the + window geometry. See xdg_surface.set_window_geometry. + + If a zero or negative size is set the invalid_input error is raised. + </description> + <arg name="width" type="int" summary="width of positioned rectangle"/> + <arg name="height" type="int" summary="height of positioned rectangle"/> + </request> + + <request name="set_anchor_rect"> + <description summary="set the anchor rectangle within the parent surface"> + Specify the anchor rectangle within the parent surface that the child + surface will be placed relative to. The rectangle is relative to the + window geometry as defined by xdg_surface.set_window_geometry of the + parent surface. + + When the xdg_positioner object is used to position a child surface, the + anchor rectangle may not extend outside the window geometry of the + positioned child's parent surface. + + If a negative size is set the invalid_input error is raised. + </description> + <arg name="x" type="int" summary="x position of anchor rectangle"/> + <arg name="y" type="int" summary="y position of anchor rectangle"/> + <arg name="width" type="int" summary="width of anchor rectangle"/> + <arg name="height" type="int" summary="height of anchor rectangle"/> + </request> + + <enum name="anchor"> + <entry name="none" value="0"/> + <entry name="top" value="1"/> + <entry name="bottom" value="2"/> + <entry name="left" value="3"/> + <entry name="right" value="4"/> + <entry name="top_left" value="5"/> + <entry name="bottom_left" value="6"/> + <entry name="top_right" value="7"/> + <entry name="bottom_right" value="8"/> + </enum> + + <request name="set_anchor"> + <description summary="set anchor rectangle anchor"> + Defines the anchor point for the anchor rectangle. The specified anchor + is used derive an anchor point that the child surface will be + positioned relative to. If a corner anchor is set (e.g. 'top_left' or + 'bottom_right'), the anchor point will be at the specified corner; + otherwise, the derived anchor point will be centered on the specified + edge, or in the center of the anchor rectangle if no edge is specified. + </description> + <arg name="anchor" type="uint" enum="anchor" + summary="anchor"/> + </request> + + <enum name="gravity"> + <entry name="none" value="0"/> + <entry name="top" value="1"/> + <entry name="bottom" value="2"/> + <entry name="left" value="3"/> + <entry name="right" value="4"/> + <entry name="top_left" value="5"/> + <entry name="bottom_left" value="6"/> + <entry name="top_right" value="7"/> + <entry name="bottom_right" value="8"/> + </enum> + + <request name="set_gravity"> + <description summary="set child surface gravity"> + Defines in what direction a surface should be positioned, relative to + the anchor point of the parent surface. If a corner gravity is + specified (e.g. 'bottom_right' or 'top_left'), then the child surface + will be placed towards the specified gravity; otherwise, the child + surface will be centered over the anchor point on any axis that had no + gravity specified. If the gravity is not in the ‘gravity’ enum, an + invalid_input error is raised. + </description> + <arg name="gravity" type="uint" enum="gravity" + summary="gravity direction"/> + </request> + + <enum name="constraint_adjustment" bitfield="true"> + <description summary="constraint adjustments"> + The constraint adjustment value define ways the compositor will adjust + the position of the surface, if the unadjusted position would result + in the surface being partly constrained. + + Whether a surface is considered 'constrained' is left to the compositor + to determine. For example, the surface may be partly outside the + compositor's defined 'work area', thus necessitating the child surface's + position be adjusted until it is entirely inside the work area. + + The adjustments can be combined, according to a defined precedence: 1) + Flip, 2) Slide, 3) Resize. + </description> + <entry name="none" value="0"> + <description summary="don't move the child surface when constrained"> + Don't alter the surface position even if it is constrained on some + axis, for example partially outside the edge of an output. + </description> + </entry> + <entry name="slide_x" value="1"> + <description summary="move along the x axis until unconstrained"> + Slide the surface along the x axis until it is no longer constrained. + + First try to slide towards the direction of the gravity on the x axis + until either the edge in the opposite direction of the gravity is + unconstrained or the edge in the direction of the gravity is + constrained. + + Then try to slide towards the opposite direction of the gravity on the + x axis until either the edge in the direction of the gravity is + unconstrained or the edge in the opposite direction of the gravity is + constrained. + </description> + </entry> + <entry name="slide_y" value="2"> + <description summary="move along the y axis until unconstrained"> + Slide the surface along the y axis until it is no longer constrained. + + First try to slide towards the direction of the gravity on the y axis + until either the edge in the opposite direction of the gravity is + unconstrained or the edge in the direction of the gravity is + constrained. + + Then try to slide towards the opposite direction of the gravity on the + y axis until either the edge in the direction of the gravity is + unconstrained or the edge in the opposite direction of the gravity is + constrained. + </description> + </entry> + <entry name="flip_x" value="4"> + <description summary="invert the anchor and gravity on the x axis"> + Invert the anchor and gravity on the x axis if the surface is + constrained on the x axis. For example, if the left edge of the + surface is constrained, the gravity is 'left' and the anchor is + 'left', change the gravity to 'right' and the anchor to 'right'. + + If the adjusted position also ends up being constrained, the resulting + position of the flip_x adjustment will be the one before the + adjustment. + </description> + </entry> + <entry name="flip_y" value="8"> + <description summary="invert the anchor and gravity on the y axis"> + Invert the anchor and gravity on the y axis if the surface is + constrained on the y axis. For example, if the bottom edge of the + surface is constrained, the gravity is 'bottom' and the anchor is + 'bottom', change the gravity to 'top' and the anchor to 'top'. + + The adjusted position is calculated given the original anchor + rectangle and offset, but with the new flipped anchor and gravity + values. + + If the adjusted position also ends up being constrained, the resulting + position of the flip_y adjustment will be the one before the + adjustment. + </description> + </entry> + <entry name="resize_x" value="16"> + <description summary="horizontally resize the surface"> + Resize the surface horizontally so that it is completely + unconstrained. + </description> + </entry> + <entry name="resize_y" value="32"> + <description summary="vertically resize the surface"> + Resize the surface vertically so that it is completely unconstrained. + </description> + </entry> + </enum> + + <request name="set_constraint_adjustment"> + <description summary="set the adjustment to be done when constrained"> + Specify how the window should be positioned if the originally intended + position caused the surface to be constrained, meaning at least + partially outside positioning boundaries set by the compositor. The + adjustment is set by constructing a bitmask describing the adjustment to + be made when the surface is constrained on that axis. + + If no bit for one axis is set, the compositor will assume that the child + surface should not change its position on that axis when constrained. + + If more than one bit for one axis is set, the order of how adjustments + are applied is specified in the corresponding adjustment descriptions. + + The default adjustment is none. + </description> + <arg name="constraint_adjustment" type="uint" + summary="bit mask of constraint adjustments"/> + </request> + + <request name="set_offset"> + <description summary="set surface position offset"> + Specify the surface position offset relative to the position of the + anchor on the anchor rectangle and the anchor on the surface. For + example if the anchor of the anchor rectangle is at (x, y), the surface + has the gravity bottom|right, and the offset is (ox, oy), the calculated + surface position will be (x + ox, y + oy). The offset position of the + surface is the one used for constraint testing. See + set_constraint_adjustment. + + An example use case is placing a popup menu on top of a user interface + element, while aligning the user interface element of the parent surface + with some user interface element placed somewhere in the popup surface. + </description> + <arg name="x" type="int" summary="surface position x offset"/> + <arg name="y" type="int" summary="surface position y offset"/> + </request> + + <!-- Version 3 additions --> + + <request name="set_reactive" since="3"> + <description summary="continuously reconstrain the surface"> + When set reactive, the surface is reconstrained if the conditions used + for constraining changed, e.g. the parent window moved. + + If the conditions changed and the popup was reconstrained, an + xdg_popup.configure event is sent with updated geometry, followed by an + xdg_surface.configure event. + </description> + </request> + + <request name="set_parent_size" since="3"> + <description summary=""> + Set the parent window geometry the compositor should use when + positioning the popup. The compositor may use this information to + determine the future state the popup should be constrained using. If + this doesn't match the dimension of the parent the popup is eventually + positioned against, the behavior is undefined. + + The arguments are given in the surface-local coordinate space. + </description> + <arg name="parent_width" type="int" + summary="future window geometry width of parent"/> + <arg name="parent_height" type="int" + summary="future window geometry height of parent"/> + </request> + + <request name="set_parent_configure" since="3"> + <description summary="set parent configure this is a response to"> + Set the serial of an xdg_surface.configure event this positioner will be + used in response to. The compositor may use this information together + with set_parent_size to determine what future state the popup should be + constrained using. + </description> + <arg name="serial" type="uint" + summary="serial of parent configure event"/> + </request> + </interface> + + <interface name="xdg_surface" version="5"> + <description summary="desktop user interface surface base interface"> + An interface that may be implemented by a wl_surface, for + implementations that provide a desktop-style user interface. + + It provides a base set of functionality required to construct user + interface elements requiring management by the compositor, such as + toplevel windows, menus, etc. The types of functionality are split into + xdg_surface roles. + + Creating an xdg_surface does not set the role for a wl_surface. In order + to map an xdg_surface, the client must create a role-specific object + using, e.g., get_toplevel, get_popup. The wl_surface for any given + xdg_surface can have at most one role, and may not be assigned any role + not based on xdg_surface. + + A role must be assigned before any other requests are made to the + xdg_surface object. + + The client must call wl_surface.commit on the corresponding wl_surface + for the xdg_surface state to take effect. + + Creating an xdg_surface from a wl_surface which has a buffer attached or + committed is a client error, and any attempts by a client to attach or + manipulate a buffer prior to the first xdg_surface.configure call must + also be treated as errors. + + After creating a role-specific object and setting it up, the client must + perform an initial commit without any buffer attached. The compositor + will reply with an xdg_surface.configure event. The client must + acknowledge it and is then allowed to attach a buffer to map the surface. + + Mapping an xdg_surface-based role surface is defined as making it + possible for the surface to be shown by the compositor. Note that + a mapped surface is not guaranteed to be visible once it is mapped. + + For an xdg_surface to be mapped by the compositor, the following + conditions must be met: + (1) the client has assigned an xdg_surface-based role to the surface + (2) the client has set and committed the xdg_surface state and the + role-dependent state to the surface + (3) the client has committed a buffer to the surface + + A newly-unmapped surface is considered to have met condition (1) out + of the 3 required conditions for mapping a surface if its role surface + has not been destroyed, i.e. the client must perform the initial commit + again before attaching a buffer. + </description> + + <enum name="error"> + <entry name="not_constructed" value="1" + summary="Surface was not fully constructed"/> + <entry name="already_constructed" value="2" + summary="Surface was already constructed"/> + <entry name="unconfigured_buffer" value="3" + summary="Attaching a buffer to an unconfigured surface"/> + <entry name="invalid_serial" value="4" + summary="Invalid serial number when acking a configure event"/> + <entry name="invalid_size" value="5" + summary="Width or height was zero or negative"/> + <entry name="defunct_role_object" value="6" + summary="Surface was destroyed before its role object"/> + </enum> + + <request name="destroy" type="destructor"> + <description summary="destroy the xdg_surface"> + Destroy the xdg_surface object. An xdg_surface must only be destroyed + after its role object has been destroyed, otherwise + a defunct_role_object error is raised. + </description> + </request> + + <request name="get_toplevel"> + <description summary="assign the xdg_toplevel surface role"> + This creates an xdg_toplevel object for the given xdg_surface and gives + the associated wl_surface the xdg_toplevel role. + + See the documentation of xdg_toplevel for more details about what an + xdg_toplevel is and how it is used. + </description> + <arg name="id" type="new_id" interface="xdg_toplevel"/> + </request> + + <request name="get_popup"> + <description summary="assign the xdg_popup surface role"> + This creates an xdg_popup object for the given xdg_surface and gives + the associated wl_surface the xdg_popup role. + + If null is passed as a parent, a parent surface must be specified using + some other protocol, before committing the initial state. + + See the documentation of xdg_popup for more details about what an + xdg_popup is and how it is used. + </description> + <arg name="id" type="new_id" interface="xdg_popup"/> + <arg name="parent" type="object" interface="xdg_surface" allow-null="true"/> + <arg name="positioner" type="object" interface="xdg_positioner"/> + </request> + + <request name="set_window_geometry"> + <description summary="set the new window geometry"> + The window geometry of a surface is its "visible bounds" from the + user's perspective. Client-side decorations often have invisible + portions like drop-shadows which should be ignored for the + purposes of aligning, placing and constraining windows. + + The window geometry is double buffered, and will be applied at the + time wl_surface.commit of the corresponding wl_surface is called. + + When maintaining a position, the compositor should treat the (x, y) + coordinate of the window geometry as the top left corner of the window. + A client changing the (x, y) window geometry coordinate should in + general not alter the position of the window. + + Once the window geometry of the surface is set, it is not possible to + unset it, and it will remain the same until set_window_geometry is + called again, even if a new subsurface or buffer is attached. + + If never set, the value is the full bounds of the surface, + including any subsurfaces. This updates dynamically on every + commit. This unset is meant for extremely simple clients. + + The arguments are given in the surface-local coordinate space of + the wl_surface associated with this xdg_surface. + + The width and height must be greater than zero. Setting an invalid size + will raise an invalid_size error. When applied, the effective window + geometry will be the set window geometry clamped to the bounding + rectangle of the combined geometry of the surface of the xdg_surface and + the associated subsurfaces. + </description> + <arg name="x" type="int"/> + <arg name="y" type="int"/> + <arg name="width" type="int"/> + <arg name="height" type="int"/> + </request> + + <request name="ack_configure"> + <description summary="ack a configure event"> + When a configure event is received, if a client commits the + surface in response to the configure event, then the client + must make an ack_configure request sometime before the commit + request, passing along the serial of the configure event. + + For instance, for toplevel surfaces the compositor might use this + information to move a surface to the top left only when the client has + drawn itself for the maximized or fullscreen state. + + If the client receives multiple configure events before it + can respond to one, it only has to ack the last configure event. + Acking a configure event that was never sent raises an invalid_serial + error. + + A client is not required to commit immediately after sending + an ack_configure request - it may even ack_configure several times + before its next surface commit. + + A client may send multiple ack_configure requests before committing, but + only the last request sent before a commit indicates which configure + event the client really is responding to. + + Sending an ack_configure request consumes the serial number sent with + the request, as well as serial numbers sent by all configure events + sent on this xdg_surface prior to the configure event referenced by + the committed serial. + + It is an error to issue multiple ack_configure requests referencing a + serial from the same configure event, or to issue an ack_configure + request referencing a serial from a configure event issued before the + event identified by the last ack_configure request for the same + xdg_surface. Doing so will raise an invalid_serial error. + </description> + <arg name="serial" type="uint" summary="the serial from the configure event"/> + </request> + + <event name="configure"> + <description summary="suggest a surface change"> + The configure event marks the end of a configure sequence. A configure + sequence is a set of one or more events configuring the state of the + xdg_surface, including the final xdg_surface.configure event. + + Where applicable, xdg_surface surface roles will during a configure + sequence extend this event as a latched state sent as events before the + xdg_surface.configure event. Such events should be considered to make up + a set of atomically applied configuration states, where the + xdg_surface.configure commits the accumulated state. + + Clients should arrange their surface for the new states, and then send + an ack_configure request with the serial sent in this configure event at + some point before committing the new surface. + + If the client receives multiple configure events before it can respond + to one, it is free to discard all but the last event it received. + </description> + <arg name="serial" type="uint" summary="serial of the configure event"/> + </event> + + </interface> + + <interface name="xdg_toplevel" version="5"> + <description summary="toplevel surface"> + This interface defines an xdg_surface role which allows a surface to, + among other things, set window-like properties such as maximize, + fullscreen, and minimize, set application-specific metadata like title and + id, and well as trigger user interactive operations such as interactive + resize and move. + + Unmapping an xdg_toplevel means that the surface cannot be shown + by the compositor until it is explicitly mapped again. + All active operations (e.g., move, resize) are canceled and all + attributes (e.g. title, state, stacking, ...) are discarded for + an xdg_toplevel surface when it is unmapped. The xdg_toplevel returns to + the state it had right after xdg_surface.get_toplevel. The client + can re-map the toplevel by perfoming a commit without any buffer + attached, waiting for a configure event and handling it as usual (see + xdg_surface description). + + Attaching a null buffer to a toplevel unmaps the surface. + </description> + + <request name="destroy" type="destructor"> + <description summary="destroy the xdg_toplevel"> + This request destroys the role surface and unmaps the surface; + see "Unmapping" behavior in interface section for details. + </description> + </request> + + <enum name="error"> + <entry name="invalid_resize_edge" value="0" summary="provided value is + not a valid variant of the resize_edge enum"/> + <entry name="invalid_parent" value="1" + summary="invalid parent toplevel"/> + <entry name="invalid_size" value="2" + summary="client provided an invalid min or max size"/> + </enum> + + <request name="set_parent"> + <description summary="set the parent of this surface"> + Set the "parent" of this surface. This surface should be stacked + above the parent surface and all other ancestor surfaces. + + Parent surfaces should be set on dialogs, toolboxes, or other + "auxiliary" surfaces, so that the parent is raised when the dialog + is raised. + + Setting a null parent for a child surface unsets its parent. Setting + a null parent for a surface which currently has no parent is a no-op. + + Only mapped surfaces can have child surfaces. Setting a parent which + is not mapped is equivalent to setting a null parent. If a surface + becomes unmapped, its children's parent is set to the parent of + the now-unmapped surface. If the now-unmapped surface has no parent, + its children's parent is unset. If the now-unmapped surface becomes + mapped again, its parent-child relationship is not restored. + + The parent toplevel must not be one of the child toplevel's + descendants, and the parent must be different from the child toplevel, + otherwise the invalid_parent protocol error is raised. + </description> + <arg name="parent" type="object" interface="xdg_toplevel" allow-null="true"/> + </request> + + <request name="set_title"> + <description summary="set surface title"> + Set a short title for the surface. + + This string may be used to identify the surface in a task bar, + window list, or other user interface elements provided by the + compositor. + + The string must be encoded in UTF-8. + </description> + <arg name="title" type="string"/> + </request> + + <request name="set_app_id"> + <description summary="set application ID"> + Set an application identifier for the surface. + + The app ID identifies the general class of applications to which + the surface belongs. The compositor can use this to group multiple + surfaces together, or to determine how to launch a new application. + + For D-Bus activatable applications, the app ID is used as the D-Bus + service name. + + The compositor shell will try to group application surfaces together + by their app ID. As a best practice, it is suggested to select app + ID's that match the basename of the application's .desktop file. + For example, "org.freedesktop.FooViewer" where the .desktop file is + "org.freedesktop.FooViewer.desktop". + + Like other properties, a set_app_id request can be sent after the + xdg_toplevel has been mapped to update the property. + + See the desktop-entry specification [0] for more details on + application identifiers and how they relate to well-known D-Bus + names and .desktop files. + + [0] https://standards.freedesktop.org/desktop-entry-spec/ + </description> + <arg name="app_id" type="string"/> + </request> + + <request name="show_window_menu"> + <description summary="show the window menu"> + Clients implementing client-side decorations might want to show + a context menu when right-clicking on the decorations, giving the + user a menu that they can use to maximize or minimize the window. + + This request asks the compositor to pop up such a window menu at + the given position, relative to the local surface coordinates of + the parent surface. There are no guarantees as to what menu items + the window menu contains, or even if a window menu will be drawn + at all. + + This request must be used in response to some sort of user action + like a button press, key press, or touch down event. + </description> + <arg name="seat" type="object" interface="wl_seat" summary="the wl_seat of the user event"/> + <arg name="serial" type="uint" summary="the serial of the user event"/> + <arg name="x" type="int" summary="the x position to pop up the window menu at"/> + <arg name="y" type="int" summary="the y position to pop up the window menu at"/> + </request> + + <request name="move"> + <description summary="start an interactive move"> + Start an interactive, user-driven move of the surface. + + This request must be used in response to some sort of user action + like a button press, key press, or touch down event. The passed + serial is used to determine the type of interactive move (touch, + pointer, etc). + + The server may ignore move requests depending on the state of + the surface (e.g. fullscreen or maximized), or if the passed serial + is no longer valid. + + If triggered, the surface will lose the focus of the device + (wl_pointer, wl_touch, etc) used for the move. It is up to the + compositor to visually indicate that the move is taking place, such as + updating a pointer cursor, during the move. There is no guarantee + that the device focus will return when the move is completed. + </description> + <arg name="seat" type="object" interface="wl_seat" summary="the wl_seat of the user event"/> + <arg name="serial" type="uint" summary="the serial of the user event"/> + </request> + + <enum name="resize_edge"> + <description summary="edge values for resizing"> + These values are used to indicate which edge of a surface + is being dragged in a resize operation. + </description> + <entry name="none" value="0"/> + <entry name="top" value="1"/> + <entry name="bottom" value="2"/> + <entry name="left" value="4"/> + <entry name="top_left" value="5"/> + <entry name="bottom_left" value="6"/> + <entry name="right" value="8"/> + <entry name="top_right" value="9"/> + <entry name="bottom_right" value="10"/> + </enum> + + <request name="resize"> + <description summary="start an interactive resize"> + Start a user-driven, interactive resize of the surface. + + This request must be used in response to some sort of user action + like a button press, key press, or touch down event. The passed + serial is used to determine the type of interactive resize (touch, + pointer, etc). + + The server may ignore resize requests depending on the state of + the surface (e.g. fullscreen or maximized). + + If triggered, the client will receive configure events with the + "resize" state enum value and the expected sizes. See the "resize" + enum value for more details about what is required. The client + must also acknowledge configure events using "ack_configure". After + the resize is completed, the client will receive another "configure" + event without the resize state. + + If triggered, the surface also will lose the focus of the device + (wl_pointer, wl_touch, etc) used for the resize. It is up to the + compositor to visually indicate that the resize is taking place, + such as updating a pointer cursor, during the resize. There is no + guarantee that the device focus will return when the resize is + completed. + + The edges parameter specifies how the surface should be resized, and + is one of the values of the resize_edge enum. Values not matching + a variant of the enum will cause a protocol error. The compositor + may use this information to update the surface position for example + when dragging the top left corner. The compositor may also use + this information to adapt its behavior, e.g. choose an appropriate + cursor image. + </description> + <arg name="seat" type="object" interface="wl_seat" summary="the wl_seat of the user event"/> + <arg name="serial" type="uint" summary="the serial of the user event"/> + <arg name="edges" type="uint" enum="resize_edge" summary="which edge or corner is being dragged"/> + </request> + + <enum name="state"> + <description summary="types of state on the surface"> + The different state values used on the surface. This is designed for + state values like maximized, fullscreen. It is paired with the + configure event to ensure that both the client and the compositor + setting the state can be synchronized. + + States set in this way are double-buffered. They will get applied on + the next commit. + </description> + <entry name="maximized" value="1" summary="the surface is maximized"> + <description summary="the surface is maximized"> + The surface is maximized. The window geometry specified in the configure + event must be obeyed by the client. + + The client should draw without shadow or other + decoration outside of the window geometry. + </description> + </entry> + <entry name="fullscreen" value="2" summary="the surface is fullscreen"> + <description summary="the surface is fullscreen"> + The surface is fullscreen. The window geometry specified in the + configure event is a maximum; the client cannot resize beyond it. For + a surface to cover the whole fullscreened area, the geometry + dimensions must be obeyed by the client. For more details, see + xdg_toplevel.set_fullscreen. + </description> + </entry> + <entry name="resizing" value="3" summary="the surface is being resized"> + <description summary="the surface is being resized"> + The surface is being resized. The window geometry specified in the + configure event is a maximum; the client cannot resize beyond it. + Clients that have aspect ratio or cell sizing configuration can use + a smaller size, however. + </description> + </entry> + <entry name="activated" value="4" summary="the surface is now activated"> + <description summary="the surface is now activated"> + Client window decorations should be painted as if the window is + active. Do not assume this means that the window actually has + keyboard or pointer focus. + </description> + </entry> + <entry name="tiled_left" value="5" since="2"> + <description summary="the surface’s left edge is tiled"> + The window is currently in a tiled layout and the left edge is + considered to be adjacent to another part of the tiling grid. + </description> + </entry> + <entry name="tiled_right" value="6" since="2"> + <description summary="the surface’s right edge is tiled"> + The window is currently in a tiled layout and the right edge is + considered to be adjacent to another part of the tiling grid. + </description> + </entry> + <entry name="tiled_top" value="7" since="2"> + <description summary="the surface’s top edge is tiled"> + The window is currently in a tiled layout and the top edge is + considered to be adjacent to another part of the tiling grid. + </description> + </entry> + <entry name="tiled_bottom" value="8" since="2"> + <description summary="the surface’s bottom edge is tiled"> + The window is currently in a tiled layout and the bottom edge is + considered to be adjacent to another part of the tiling grid. + </description> + </entry> + </enum> + + <request name="set_max_size"> + <description summary="set the maximum size"> + Set a maximum size for the window. + + The client can specify a maximum size so that the compositor does + not try to configure the window beyond this size. + + The width and height arguments are in window geometry coordinates. + See xdg_surface.set_window_geometry. + + Values set in this way are double-buffered. They will get applied + on the next commit. + + The compositor can use this information to allow or disallow + different states like maximize or fullscreen and draw accurate + animations. + + Similarly, a tiling window manager may use this information to + place and resize client windows in a more effective way. + + The client should not rely on the compositor to obey the maximum + size. The compositor may decide to ignore the values set by the + client and request a larger size. + + If never set, or a value of zero in the request, means that the + client has no expected maximum size in the given dimension. + As a result, a client wishing to reset the maximum size + to an unspecified state can use zero for width and height in the + request. + + Requesting a maximum size to be smaller than the minimum size of + a surface is illegal and will result in an invalid_size error. + + The width and height must be greater than or equal to zero. Using + strictly negative values for width or height will result in a + invalid_size error. + </description> + <arg name="width" type="int"/> + <arg name="height" type="int"/> + </request> + + <request name="set_min_size"> + <description summary="set the minimum size"> + Set a minimum size for the window. + + The client can specify a minimum size so that the compositor does + not try to configure the window below this size. + + The width and height arguments are in window geometry coordinates. + See xdg_surface.set_window_geometry. + + Values set in this way are double-buffered. They will get applied + on the next commit. + + The compositor can use this information to allow or disallow + different states like maximize or fullscreen and draw accurate + animations. + + Similarly, a tiling window manager may use this information to + place and resize client windows in a more effective way. + + The client should not rely on the compositor to obey the minimum + size. The compositor may decide to ignore the values set by the + client and request a smaller size. + + If never set, or a value of zero in the request, means that the + client has no expected minimum size in the given dimension. + As a result, a client wishing to reset the minimum size + to an unspecified state can use zero for width and height in the + request. + + Requesting a minimum size to be larger than the maximum size of + a surface is illegal and will result in an invalid_size error. + + The width and height must be greater than or equal to zero. Using + strictly negative values for width and height will result in a + invalid_size error. + </description> + <arg name="width" type="int"/> + <arg name="height" type="int"/> + </request> + + <request name="set_maximized"> + <description summary="maximize the window"> + Maximize the surface. + + After requesting that the surface should be maximized, the compositor + will respond by emitting a configure event. Whether this configure + actually sets the window maximized is subject to compositor policies. + The client must then update its content, drawing in the configured + state. The client must also acknowledge the configure when committing + the new content (see ack_configure). + + It is up to the compositor to decide how and where to maximize the + surface, for example which output and what region of the screen should + be used. + + If the surface was already maximized, the compositor will still emit + a configure event with the "maximized" state. + + If the surface is in a fullscreen state, this request has no direct + effect. It may alter the state the surface is returned to when + unmaximized unless overridden by the compositor. + </description> + </request> + + <request name="unset_maximized"> + <description summary="unmaximize the window"> + Unmaximize the surface. + + After requesting that the surface should be unmaximized, the compositor + will respond by emitting a configure event. Whether this actually + un-maximizes the window is subject to compositor policies. + If available and applicable, the compositor will include the window + geometry dimensions the window had prior to being maximized in the + configure event. The client must then update its content, drawing it in + the configured state. The client must also acknowledge the configure + when committing the new content (see ack_configure). + + It is up to the compositor to position the surface after it was + unmaximized; usually the position the surface had before maximizing, if + applicable. + + If the surface was already not maximized, the compositor will still + emit a configure event without the "maximized" state. + + If the surface is in a fullscreen state, this request has no direct + effect. It may alter the state the surface is returned to when + unmaximized unless overridden by the compositor. + </description> + </request> + + <request name="set_fullscreen"> + <description summary="set the window as fullscreen on an output"> + Make the surface fullscreen. + + After requesting that the surface should be fullscreened, the + compositor will respond by emitting a configure event. Whether the + client is actually put into a fullscreen state is subject to compositor + policies. The client must also acknowledge the configure when + committing the new content (see ack_configure). + + The output passed by the request indicates the client's preference as + to which display it should be set fullscreen on. If this value is NULL, + it's up to the compositor to choose which display will be used to map + this surface. + + If the surface doesn't cover the whole output, the compositor will + position the surface in the center of the output and compensate with + with border fill covering the rest of the output. The content of the + border fill is undefined, but should be assumed to be in some way that + attempts to blend into the surrounding area (e.g. solid black). + + If the fullscreened surface is not opaque, the compositor must make + sure that other screen content not part of the same surface tree (made + up of subsurfaces, popups or similarly coupled surfaces) are not + visible below the fullscreened surface. + </description> + <arg name="output" type="object" interface="wl_output" allow-null="true"/> + </request> + + <request name="unset_fullscreen"> + <description summary="unset the window as fullscreen"> + Make the surface no longer fullscreen. + + After requesting that the surface should be unfullscreened, the + compositor will respond by emitting a configure event. + Whether this actually removes the fullscreen state of the client is + subject to compositor policies. + + Making a surface unfullscreen sets states for the surface based on the following: + * the state(s) it may have had before becoming fullscreen + * any state(s) decided by the compositor + * any state(s) requested by the client while the surface was fullscreen + + The compositor may include the previous window geometry dimensions in + the configure event, if applicable. + + The client must also acknowledge the configure when committing the new + content (see ack_configure). + </description> + </request> + + <request name="set_minimized"> + <description summary="set the window as minimized"> + Request that the compositor minimize your surface. There is no + way to know if the surface is currently minimized, nor is there + any way to unset minimization on this surface. + + If you are looking to throttle redrawing when minimized, please + instead use the wl_surface.frame event for this, as this will + also work with live previews on windows in Alt-Tab, Expose or + similar compositor features. + </description> + </request> + + <event name="configure"> + <description summary="suggest a surface change"> + This configure event asks the client to resize its toplevel surface or + to change its state. The configured state should not be applied + immediately. See xdg_surface.configure for details. + + The width and height arguments specify a hint to the window + about how its surface should be resized in window geometry + coordinates. See set_window_geometry. + + If the width or height arguments are zero, it means the client + should decide its own window dimension. This may happen when the + compositor needs to configure the state of the surface but doesn't + have any information about any previous or expected dimension. + + The states listed in the event specify how the width/height + arguments should be interpreted, and possibly how it should be + drawn. + + Clients must send an ack_configure in response to this event. See + xdg_surface.configure and xdg_surface.ack_configure for details. + </description> + <arg name="width" type="int"/> + <arg name="height" type="int"/> + <arg name="states" type="array"/> + </event> + + <event name="close"> + <description summary="surface wants to be closed"> + The close event is sent by the compositor when the user + wants the surface to be closed. This should be equivalent to + the user clicking the close button in client-side decorations, + if your application has any. + + This is only a request that the user intends to close the + window. The client may choose to ignore this request, or show + a dialog to ask the user to save their data, etc. + </description> + </event> + + <!-- Version 4 additions --> + + <event name="configure_bounds" since="4"> + <description summary="recommended window geometry bounds"> + The configure_bounds event may be sent prior to a xdg_toplevel.configure + event to communicate the bounds a window geometry size is recommended + to constrain to. + + The passed width and height are in surface coordinate space. If width + and height are 0, it means bounds is unknown and equivalent to as if no + configure_bounds event was ever sent for this surface. + + The bounds can for example correspond to the size of a monitor excluding + any panels or other shell components, so that a surface isn't created in + a way that it cannot fit. + + The bounds may change at any point, and in such a case, a new + xdg_toplevel.configure_bounds will be sent, followed by + xdg_toplevel.configure and xdg_surface.configure. + </description> + <arg name="width" type="int"/> + <arg name="height" type="int"/> + </event> + + <!-- Version 5 additions --> + + <enum name="wm_capabilities" since="5"> + <entry name="window_menu" value="1" summary="show_window_menu is available"/> + <entry name="maximize" value="2" summary="set_maximized and unset_maximized are available"/> + <entry name="fullscreen" value="3" summary="set_fullscreen and unset_fullscreen are available"/> + <entry name="minimize" value="4" summary="set_minimized is available"/> + </enum> + + <event name="wm_capabilities" since="5"> + <description summary="compositor capabilities"> + This event advertises the capabilities supported by the compositor. If + a capability isn't supported, clients should hide or disable the UI + elements that expose this functionality. For instance, if the + compositor doesn't advertise support for minimized toplevels, a button + triggering the set_minimized request should not be displayed. + + The compositor will ignore requests it doesn't support. For instance, + a compositor which doesn't advertise support for minimized will ignore + set_minimized requests. + + Compositors must send this event once before the first + xdg_surface.configure event. When the capabilities change, compositors + must send this event again and then send an xdg_surface.configure + event. + + The configured state should not be applied immediately. See + xdg_surface.configure for details. + + The capabilities are sent as an array of 32-bit unsigned integers in + native endianness. + </description> + <arg name="capabilities" type="array" summary="array of 32-bit capabilities"/> + </event> + </interface> + + <interface name="xdg_popup" version="5"> + <description summary="short-lived, popup surfaces for menus"> + A popup surface is a short-lived, temporary surface. It can be used to + implement for example menus, popovers, tooltips and other similar user + interface concepts. + + A popup can be made to take an explicit grab. See xdg_popup.grab for + details. + + When the popup is dismissed, a popup_done event will be sent out, and at + the same time the surface will be unmapped. See the xdg_popup.popup_done + event for details. + + Explicitly destroying the xdg_popup object will also dismiss the popup and + unmap the surface. Clients that want to dismiss the popup when another + surface of their own is clicked should dismiss the popup using the destroy + request. + + A newly created xdg_popup will be stacked on top of all previously created + xdg_popup surfaces associated with the same xdg_toplevel. + + The parent of an xdg_popup must be mapped (see the xdg_surface + description) before the xdg_popup itself. + + The client must call wl_surface.commit on the corresponding wl_surface + for the xdg_popup state to take effect. + </description> + + <enum name="error"> + <entry name="invalid_grab" value="0" + summary="tried to grab after being mapped"/> + </enum> + + <request name="destroy" type="destructor"> + <description summary="remove xdg_popup interface"> + This destroys the popup. Explicitly destroying the xdg_popup + object will also dismiss the popup, and unmap the surface. + + If this xdg_popup is not the "topmost" popup, a protocol error + will be sent. + </description> + </request> + + <request name="grab"> + <description summary="make the popup take an explicit grab"> + This request makes the created popup take an explicit grab. An explicit + grab will be dismissed when the user dismisses the popup, or when the + client destroys the xdg_popup. This can be done by the user clicking + outside the surface, using the keyboard, or even locking the screen + through closing the lid or a timeout. + + If the compositor denies the grab, the popup will be immediately + dismissed. + + This request must be used in response to some sort of user action like a + button press, key press, or touch down event. The serial number of the + event should be passed as 'serial'. + + The parent of a grabbing popup must either be an xdg_toplevel surface or + another xdg_popup with an explicit grab. If the parent is another + xdg_popup it means that the popups are nested, with this popup now being + the topmost popup. + + Nested popups must be destroyed in the reverse order they were created + in, e.g. the only popup you are allowed to destroy at all times is the + topmost one. + + When compositors choose to dismiss a popup, they may dismiss every + nested grabbing popup as well. When a compositor dismisses popups, it + will follow the same dismissing order as required from the client. + + If the topmost grabbing popup is destroyed, the grab will be returned to + the parent of the popup, if that parent previously had an explicit grab. + + If the parent is a grabbing popup which has already been dismissed, this + popup will be immediately dismissed. If the parent is a popup that did + not take an explicit grab, an error will be raised. + + During a popup grab, the client owning the grab will receive pointer + and touch events for all their surfaces as normal (similar to an + "owner-events" grab in X11 parlance), while the top most grabbing popup + will always have keyboard focus. + </description> + <arg name="seat" type="object" interface="wl_seat" + summary="the wl_seat of the user event"/> + <arg name="serial" type="uint" summary="the serial of the user event"/> + </request> + + <event name="configure"> + <description summary="configure the popup surface"> + This event asks the popup surface to configure itself given the + configuration. The configured state should not be applied immediately. + See xdg_surface.configure for details. + + The x and y arguments represent the position the popup was placed at + given the xdg_positioner rule, relative to the upper left corner of the + window geometry of the parent surface. + + For version 2 or older, the configure event for an xdg_popup is only + ever sent once for the initial configuration. Starting with version 3, + it may be sent again if the popup is setup with an xdg_positioner with + set_reactive requested, or in response to xdg_popup.reposition requests. + </description> + <arg name="x" type="int" + summary="x position relative to parent surface window geometry"/> + <arg name="y" type="int" + summary="y position relative to parent surface window geometry"/> + <arg name="width" type="int" summary="window geometry width"/> + <arg name="height" type="int" summary="window geometry height"/> + </event> + + <event name="popup_done"> + <description summary="popup interaction is done"> + The popup_done event is sent out when a popup is dismissed by the + compositor. The client should destroy the xdg_popup object at this + point. + </description> + </event> + + <!-- Version 3 additions --> + + <request name="reposition" since="3"> + <description summary="recalculate the popup's location"> + Reposition an already-mapped popup. The popup will be placed given the + details in the passed xdg_positioner object, and a + xdg_popup.repositioned followed by xdg_popup.configure and + xdg_surface.configure will be emitted in response. Any parameters set + by the previous positioner will be discarded. + + The passed token will be sent in the corresponding + xdg_popup.repositioned event. The new popup position will not take + effect until the corresponding configure event is acknowledged by the + client. See xdg_popup.repositioned for details. The token itself is + opaque, and has no other special meaning. + + If multiple reposition requests are sent, the compositor may skip all + but the last one. + + If the popup is repositioned in response to a configure event for its + parent, the client should send an xdg_positioner.set_parent_configure + and possibly an xdg_positioner.set_parent_size request to allow the + compositor to properly constrain the popup. + + If the popup is repositioned together with a parent that is being + resized, but not in response to a configure event, the client should + send an xdg_positioner.set_parent_size request. + </description> + <arg name="positioner" type="object" interface="xdg_positioner"/> + <arg name="token" type="uint" summary="reposition request token"/> + </request> + + <event name="repositioned" since="3"> + <description summary="signal the completion of a repositioned request"> + The repositioned event is sent as part of a popup configuration + sequence, together with xdg_popup.configure and lastly + xdg_surface.configure to notify the completion of a reposition request. + + The repositioned event is to notify about the completion of a + xdg_popup.reposition request. The token argument is the token passed + in the xdg_popup.reposition request. + + Immediately after this event is emitted, xdg_popup.configure and + xdg_surface.configure will be sent with the updated size and position, + as well as a new configure serial. + + The client should optionally update the content of the popup, but must + acknowledge the new popup configuration for the new position to take + effect. See xdg_surface.ack_configure for details. + </description> + <arg name="token" type="uint" summary="reposition request token"/> + </event> + + </interface> +</protocol> -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2944
From: Alexandros Frantzis <alexandros.frantzis(a)collabora.com> In our setup with a dedicated event dispatch thread, libwayland ensures that object proxies associated with an event handler remain valid (or NULL) while the handler is executing. However, no such guarantees are given for the proxy user data. It is thus possible for the user data to become invalid (e.g., its memory freed from a different thread) right after the event handler is entered. This is an issue for wayland_surface associated proxies since they may receive unsolicited events from the compositor at any time (e.g., xdg_surface.configure), even while we are destroying the wayland_surface. To avoid the problem, we add a Wayland event loop synchronization operation after the destruction of wayland_surface related proxies, but before we destroy the wayland_surface itself. The sync operation guarantees that all libwayland event dispatches after it will take into account the destroyed proxies, and thus will not provide access to these proxies or their user data. Event dispatches before the sync will have valid user data since the wayland_surface is still not destroyed at that point. --- dlls/winewayland.drv/wayland.c | 152 +++++++++++++++++++++++++ dlls/winewayland.drv/wayland_surface.c | 4 + dlls/winewayland.drv/waylanddrv.h | 7 ++ dlls/winewayland.drv/waylanddrv_main.c | 4 +- 4 files changed, 164 insertions(+), 3 deletions(-) diff --git a/dlls/winewayland.drv/wayland.c b/dlls/winewayland.drv/wayland.c index 357dc4208d3..5d2fe122368 100644 --- a/dlls/winewayland.drv/wayland.c +++ b/dlls/winewayland.drv/wayland.c @@ -28,7 +28,11 @@ #include "wine/debug.h" +#include <errno.h> +#include <fcntl.h> +#include <poll.h> #include <stdlib.h> +#include <unistd.h> WINE_DEFAULT_DEBUG_CHANNEL(waylanddrv); @@ -38,6 +42,8 @@ struct wayland process_wayland = .output_mutex = PTHREAD_MUTEX_INITIALIZER }; +static int wayland_callback_pipe[2]; + /********************************************************************** * xdg_wm_base handling */ @@ -135,6 +141,16 @@ BOOL wayland_process_init(void) TRACE("wl_display=%p\n", process_wayland.wl_display); + /* Make the read end of the callback pipe non-blocking. */ + if (pipe(wayland_callback_pipe) == -1 || + fcntl(wayland_callback_pipe[0], F_SETFD, FD_CLOEXEC) == -1 || + fcntl(wayland_callback_pipe[0], F_SETFL, O_NONBLOCK) == -1 || + fcntl(wayland_callback_pipe[1], F_SETFD, FD_CLOEXEC) == -1) + { + ERR("Failed to create callback pipe fds\n"); + return FALSE; + } + if (!(process_wayland.wl_event_queue = wl_display_create_queue(process_wayland.wl_display))) { ERR("Failed to create event queue\n"); @@ -183,3 +199,139 @@ BOOL wayland_process_init(void) return TRUE; } + +struct wayland_callback +{ + pthread_mutex_t mutex; + pthread_cond_t done_cond; + BOOL done; +}; + +static int wayland_dispatch_callbacks(void) +{ + struct wayland_callback *cb; + int nread; + + while (TRUE) + { + do + { + nread = read(wayland_callback_pipe[0], &cb, sizeof(cb)); + } while (nread == -1 && errno == EINTR); + + if (nread == -1 && errno == EAGAIN) break; + + if (nread < sizeof(cb)) + { + TRACE("Failed to read callback nread=%d errno=%d\n", nread, errno); + return -1; + } + + pthread_mutex_lock(&cb->mutex); + cb->done = TRUE; + pthread_cond_signal(&cb->done_cond); + pthread_mutex_unlock(&cb->mutex); + } + + return 0; +} + +/********************************************************************** + * wayland_dispatch_events + * + * Dispatch the events received from the compositor and any scheduled + * internal callbacks. + */ +int wayland_dispatch_events(void) +{ + struct pollfd pfd[2] = {0}; + int ret; + struct wl_display *display = process_wayland.wl_display; + struct wl_event_queue *queue = process_wayland.wl_event_queue; + + pfd[0].fd = wl_display_get_fd(display); + + if (wl_display_prepare_read_queue(display, queue) == -1) + return wl_display_dispatch_queue_pending(display, queue); + + while (TRUE) + { + ret = wl_display_flush(display); + + if (ret != -1 || errno != EAGAIN) break; + + pfd[0].events = POLLOUT; + while ((ret = poll(pfd, 1, -1)) == -1 && errno == EINTR) continue; + + if (ret == -1) + { + wl_display_cancel_read(display); + return -1; + } + } + + if (ret == -1 && errno != EPIPE) + { + wl_display_cancel_read(display); + return -1; + } + + /* Events from the compositor */ + pfd[0].events = POLLIN; + pfd[0].revents = 0; + /* Internal callbacks */ + pfd[1].events = POLLIN; + pfd[1].fd = wayland_callback_pipe[0]; + + while ((ret = poll(pfd, 2, -1)) == -1 && errno == EINTR) continue; + + if (!(pfd[0].revents & POLLIN)) wl_display_cancel_read(display); + + if (ret == -1) return -1; + + /* Handle wl_display fd input. */ + if (pfd[0].revents & POLLIN) + { + if (wl_display_read_events(display) == -1) return -1; + if (wl_display_dispatch_queue_pending(display, queue) == -1) return -1; + } + + /* Handle callbacks. */ + if (pfd[1].revents & POLLIN) + if (wayland_dispatch_callbacks() == -1) return -1; + + return 0; +} + +/********************************************************************** + * wayland_sync + * + * Wait for all currently queued Wayland events to be handled. + */ +void wayland_sync(void) +{ + struct wayland_callback cb = + { + .mutex = PTHREAD_MUTEX_INITIALIZER, + .done_cond = PTHREAD_COND_INITIALIZER, + .done = FALSE + }; + struct wayland_callback *cb_ptr = &cb; + int nwritten; + + do + { + nwritten = write(wayland_callback_pipe[1], &cb_ptr, sizeof(cb_ptr)); + } while (nwritten == -1 && errno == EINTR); + + if (nwritten < sizeof(cb_ptr)) + { + ERR("Failed to write callback data to pipe nwritten=%d errno=%d\n", + nwritten, errno); + return; + } + + pthread_mutex_lock(&cb.mutex); + while (!cb.done) pthread_cond_wait(&cb.done_cond, &cb.mutex); + pthread_mutex_unlock(&cb.mutex); +} diff --git a/dlls/winewayland.drv/wayland_surface.c b/dlls/winewayland.drv/wayland_surface.c index 84181ae45f4..d9ee42863fa 100644 --- a/dlls/winewayland.drv/wayland_surface.c +++ b/dlls/winewayland.drv/wayland_surface.c @@ -124,6 +124,10 @@ void wayland_surface_destroy(struct wayland_surface *surface) wl_display_flush(process_wayland.wl_display); + /* Sync to ensure no event handlers are running, or are going to run, for + * the just destroyed objects. */ + wayland_sync(); + pthread_cond_destroy(&surface->configured_cond); pthread_mutex_destroy(&surface->mutex); diff --git a/dlls/winewayland.drv/waylanddrv.h b/dlls/winewayland.drv/waylanddrv.h index 4c42dcd6268..ba084a1f3c4 100644 --- a/dlls/winewayland.drv/waylanddrv.h +++ b/dlls/winewayland.drv/waylanddrv.h @@ -112,6 +112,13 @@ struct wayland_surface BOOL wayland_process_init(void) DECLSPEC_HIDDEN; void wayland_init_display_devices(BOOL force) DECLSPEC_HIDDEN; +/********************************************************************** + * Wayland event loop + */ + +int wayland_dispatch_events(void) DECLSPEC_HIDDEN; +void wayland_sync(void) DECLSPEC_HIDDEN; + /********************************************************************** * Wayland output */ diff --git a/dlls/winewayland.drv/waylanddrv_main.c b/dlls/winewayland.drv/waylanddrv_main.c index 530cdd83d90..3154e2d5752 100644 --- a/dlls/winewayland.drv/waylanddrv_main.c +++ b/dlls/winewayland.drv/waylanddrv_main.c @@ -56,9 +56,7 @@ err: static NTSTATUS waylanddrv_unix_read_events(void *arg) { - while (wl_display_dispatch_queue(process_wayland.wl_display, - process_wayland.wl_event_queue) != -1) - continue; + while (wayland_dispatch_events() != -1) continue; /* This function only returns on a fatal error, e.g., if our connection * to the Wayland server is lost. */ return STATUS_UNSUCCESSFUL; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2944
From: Alexandros Frantzis <alexandros.frantzis(a)collabora.com> --- dlls/winewayland.drv/Makefile.in | 1 + dlls/winewayland.drv/waylanddrv.h | 6 + dlls/winewayland.drv/window.c | 31 +++++ dlls/winewayland.drv/window_surface.c | 185 ++++++++++++++++++++++++++ 4 files changed, 223 insertions(+) create mode 100644 dlls/winewayland.drv/window_surface.c diff --git a/dlls/winewayland.drv/Makefile.in b/dlls/winewayland.drv/Makefile.in index 9a682e7e007..ef75c289b76 100644 --- a/dlls/winewayland.drv/Makefile.in +++ b/dlls/winewayland.drv/Makefile.in @@ -12,5 +12,6 @@ SOURCES = \ wayland_surface.c \ waylanddrv_main.c \ window.c \ + window_surface.c \ xdg-output-unstable-v1.xml \ xdg-shell.xml diff --git a/dlls/winewayland.drv/waylanddrv.h b/dlls/winewayland.drv/waylanddrv.h index ba084a1f3c4..ba938179bc6 100644 --- a/dlls/winewayland.drv/waylanddrv.h +++ b/dlls/winewayland.drv/waylanddrv.h @@ -136,6 +136,12 @@ void wayland_surface_destroy(struct wayland_surface *surface) DECLSPEC_HIDDEN; void wayland_surface_make_toplevel(struct wayland_surface *surface) DECLSPEC_HIDDEN; void wayland_surface_clear_role(struct wayland_surface *surface) DECLSPEC_HIDDEN; +/********************************************************************** + * Wayland window surface + */ + +struct window_surface *wayland_window_surface_create(HWND hwnd, const RECT *rect) DECLSPEC_HIDDEN; + /********************************************************************** * USER driver functions */ diff --git a/dlls/winewayland.drv/window.c b/dlls/winewayland.drv/window.c index fcdc6f0f69b..b7fdc0f85fe 100644 --- a/dlls/winewayland.drv/window.c +++ b/dlls/winewayland.drv/window.c @@ -47,6 +47,8 @@ struct wayland_win_data struct wayland_surface *wayland_surface; /* whether this window is visible */ BOOL visible; + /* wine window_surface backing this window */ + struct window_surface *window_surface; }; static int wayland_win_data_cmp_rb(const void *key, @@ -112,6 +114,7 @@ static void wayland_win_data_destroy(struct wayland_win_data *data) pthread_mutex_unlock(&win_data_mutex); + if (data->window_surface) window_surface_release(data->window_surface); if (data->wayland_surface) wayland_surface_destroy(data->wayland_surface); free(data); } @@ -244,6 +247,7 @@ BOOL WAYLAND_WindowPosChanging(HWND hwnd, HWND insert_after, UINT swp_flags, struct wayland_win_data *data = wayland_win_data_get(hwnd); DWORD style = NtUserGetWindowLongW(hwnd, GWL_STYLE); HWND parent = NtUserGetAncestor(hwnd, GA_PARENT); + RECT surface_rect; TRACE("hwnd %p window %s client %s visible %s after %p flags %08x\n", hwnd, wine_dbgstr_rect(window_rect), wine_dbgstr_rect(client_rect), @@ -257,6 +261,29 @@ BOOL WAYLAND_WindowPosChanging(HWND hwnd, HWND insert_after, UINT swp_flags, (swp_flags & SWP_SHOWWINDOW)) && !(swp_flags & SWP_HIDEWINDOW); + /* Release the dummy surface wine provides for toplevels. */ + if (*surface) window_surface_release(*surface); + *surface = NULL; + + /* Check if we don't want a dedicated window surface. */ + if ((parent && parent != NtUserGetDesktopWindow()) || !data->visible) goto done; + + surface_rect = *window_rect; + OffsetRect(&surface_rect, -surface_rect.left, -surface_rect.top); + + /* Check if we can reuse our current window surface. */ + if (data->window_surface && + EqualRect(&data->window_surface->rect, &surface_rect)) + { + window_surface_add_ref(data->window_surface); + *surface = data->window_surface; + TRACE("reusing surface %p\n", *surface); + goto done; + } + + *surface = wayland_window_surface_create(data->hwnd, &surface_rect); + +done: wayland_win_data_release(data); return TRUE; @@ -278,6 +305,10 @@ void WAYLAND_WindowPosChanged(HWND hwnd, HWND insert_after, UINT swp_flags, if (!data) return; + if (surface) window_surface_add_ref(surface); + if (data->window_surface) window_surface_release(data->window_surface); + data->window_surface = surface; + if (wayland_win_data_wayland_surface_needs_update(data)) wayland_win_data_update_wayland_surface(data); diff --git a/dlls/winewayland.drv/window_surface.c b/dlls/winewayland.drv/window_surface.c new file mode 100644 index 00000000000..bee46259d49 --- /dev/null +++ b/dlls/winewayland.drv/window_surface.c @@ -0,0 +1,185 @@ +/* + * Wayland window surface implementation + * + * Copyright 2020 Alexandros Frantzis for Collabora Ltd + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA + */ + +#if 0 +#pragma makedep unix +#endif + +#include "config.h" + +#include "waylanddrv.h" + +#include "wine/debug.h" + +#include <limits.h> +#include <stdlib.h> + +WINE_DEFAULT_DEBUG_CHANNEL(waylanddrv); + +struct wayland_window_surface +{ + struct window_surface header; + HWND hwnd; + RECT bounds; + void *bits; + pthread_mutex_t mutex; + BITMAPINFO info; +}; + +static struct wayland_window_surface *wayland_window_surface_cast( + struct window_surface *window_surface) +{ + return (struct wayland_window_surface *)window_surface; +} + +static inline void reset_bounds(RECT *bounds) +{ + bounds->left = bounds->top = INT_MAX; + bounds->right = bounds->bottom = INT_MIN; +} + +/*********************************************************************** + * wayland_window_surface_lock + */ +static void wayland_window_surface_lock(struct window_surface *window_surface) +{ + struct wayland_window_surface *wws = wayland_window_surface_cast(window_surface); + pthread_mutex_lock(&wws->mutex); +} + +/*********************************************************************** + * wayland_window_surface_unlock + */ +static void wayland_window_surface_unlock(struct window_surface *window_surface) +{ + struct wayland_window_surface *wws = wayland_window_surface_cast(window_surface); + pthread_mutex_unlock(&wws->mutex); +} + +/*********************************************************************** + * wayland_window_surface_get_bitmap_info + */ +static void *wayland_window_surface_get_bitmap_info(struct window_surface *window_surface, + BITMAPINFO *info) +{ + struct wayland_window_surface *surface = wayland_window_surface_cast(window_surface); + /* We don't store any additional information at the end of our BITMAPINFO, so + * just copy the structure itself. */ + memcpy(info, &surface->info, sizeof(*info)); + return surface->bits; +} + +/*********************************************************************** + * wayland_window_surface_get_bounds + */ +static RECT *wayland_window_surface_get_bounds(struct window_surface *window_surface) +{ + struct wayland_window_surface *wws = wayland_window_surface_cast(window_surface); + return &wws->bounds; +} + +/*********************************************************************** + * wayland_window_surface_set_region + */ +static void wayland_window_surface_set_region(struct window_surface *window_surface, + HRGN region) +{ + /* TODO */ +} + +/*********************************************************************** + * wayland_window_surface_flush + */ +static void wayland_window_surface_flush(struct window_surface *window_surface) +{ + /* TODO */ +} + +/*********************************************************************** + * wayland_window_surface_destroy + */ +static void wayland_window_surface_destroy(struct window_surface *window_surface) +{ + struct wayland_window_surface *wws = wayland_window_surface_cast(window_surface); + + TRACE("surface=%p\n", wws); + + pthread_mutex_destroy(&wws->mutex); + free(wws->bits); + free(wws); +} + +static const struct window_surface_funcs wayland_window_surface_funcs = +{ + wayland_window_surface_lock, + wayland_window_surface_unlock, + wayland_window_surface_get_bitmap_info, + wayland_window_surface_get_bounds, + wayland_window_surface_set_region, + wayland_window_surface_flush, + wayland_window_surface_destroy +}; + +/*********************************************************************** + * wayland_window_surface_create + */ +struct window_surface *wayland_window_surface_create(HWND hwnd, const RECT *rect) +{ + struct wayland_window_surface *wws; + int width = rect->right - rect->left; + int height = rect->bottom - rect->top; + pthread_mutexattr_t mutexattr; + + TRACE("hwnd %p rect %s\n", hwnd, wine_dbgstr_rect(rect)); + + wws = calloc(1, sizeof(*wws)); + if (!wws) return NULL; + wws->info.bmiHeader.biSize = sizeof(wws->info.bmiHeader); + wws->info.bmiHeader.biClrUsed = 0; + wws->info.bmiHeader.biBitCount = 32; + wws->info.bmiHeader.biCompression = BI_RGB; + wws->info.bmiHeader.biWidth = width; + wws->info.bmiHeader.biHeight = -height; /* top-down */ + wws->info.bmiHeader.biPlanes = 1; + wws->info.bmiHeader.biSizeImage = width * height * 4; + + pthread_mutexattr_init(&mutexattr); + pthread_mutexattr_settype(&mutexattr, PTHREAD_MUTEX_RECURSIVE); + pthread_mutex_init(&wws->mutex, &mutexattr); + pthread_mutexattr_destroy(&mutexattr); + + wws->header.funcs = &wayland_window_surface_funcs; + wws->header.rect = *rect; + wws->header.ref = 1; + wws->hwnd = hwnd; + reset_bounds(&wws->bounds); + + if (!(wws->bits = malloc(wws->info.bmiHeader.biSizeImage))) + goto failed; + + TRACE("created %p hwnd %p %s bits [%p,%p)\n", wws, hwnd, wine_dbgstr_rect(rect), + wws->bits, (char *)wws->bits + wws->info.bmiHeader.biSizeImage); + + return &wws->header; + +failed: + wayland_window_surface_destroy(&wws->header); + return NULL; +} -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2944
From: Alexandros Frantzis <alexandros.frantzis(a)collabora.com> Flush a window_surface to a Wayland surface by creating a wl_shm buffer matching the window size, copying the whole window contents to that buffer and attaching it to the corresponding Wayland surface. --- configure | 64 +++++++++++ configure.ac | 5 +- dlls/winewayland.drv/Makefile.in | 2 +- dlls/winewayland.drv/wayland.c | 9 ++ dlls/winewayland.drv/wayland_surface.c | 153 +++++++++++++++++++++++++ dlls/winewayland.drv/waylanddrv.h | 22 ++++ dlls/winewayland.drv/window.c | 12 +- dlls/winewayland.drv/window_surface.c | 66 ++++++++++- 8 files changed, 329 insertions(+), 4 deletions(-) diff --git a/configure b/configure index 4cab5e67e43..9336990a5ec 100755 --- a/configure +++ b/configure @@ -702,6 +702,7 @@ INOTIFY_LIBS INOTIFY_CFLAGS PCSCLITE_LIBS PCAP_LIBS +WAYLAND_EXTRA_LIBS WAYLAND_SCANNER WAYLAND_CLIENT_LIBS WAYLAND_CLIENT_CFLAGS @@ -15706,6 +15707,68 @@ printf "%s\n" "no" >&6; } fi + ac_save_LIBS=$LIBS + { printf "%s\n" "$as_me:${as_lineno-$LINENO}: checking for library containing shm_open" >&5 +printf %s "checking for library containing shm_open... " >&6; } +if test ${ac_cv_search_shm_open+y} +then : + printf %s "(cached) " >&6 +else $as_nop + ac_func_search_save_LIBS=$LIBS +cat confdefs.h - <<_ACEOF >conftest.$ac_ext +/* end confdefs.h. */ + +/* Override any GCC internal prototype to avoid an error. + Use char because int might match the return type of a GCC + builtin and then its argument prototype would still apply. */ +char shm_open (); +int +main (void) +{ +return shm_open (); + ; + return 0; +} +_ACEOF +for ac_lib in '' rt +do + if test -z "$ac_lib"; then + ac_res="none required" + else + ac_res=-l$ac_lib + LIBS="-l$ac_lib $ac_func_search_save_LIBS" + fi + if ac_fn_c_try_link "$LINENO" +then : + ac_cv_search_shm_open=$ac_res +fi +rm -f core conftest.err conftest.$ac_objext conftest.beam \ + conftest$ac_exeext + if test ${ac_cv_search_shm_open+y} +then : + break +fi +done +if test ${ac_cv_search_shm_open+y} +then : + +else $as_nop + ac_cv_search_shm_open=no +fi +rm conftest.$ac_ext +LIBS=$ac_func_search_save_LIBS +fi +{ printf "%s\n" "$as_me:${as_lineno-$LINENO}: result: $ac_cv_search_shm_open" >&5 +printf "%s\n" "$ac_cv_search_shm_open" >&6; } +ac_res=$ac_cv_search_shm_open +if test "$ac_res" != no +then : + test "$ac_res" = "none required" || LIBS="$ac_res $LIBS" + test "$ac_res" = "none required" || WAYLAND_EXTRA_LIBS="$ac_res" + +fi + + LIBS=$ac_save_LIBS else $as_nop WAYLAND_CLIENT_LIBS="" fi @@ -23233,6 +23296,7 @@ X_EXTRA_LIBS = $X_EXTRA_LIBS WAYLAND_CLIENT_CFLAGS = $WAYLAND_CLIENT_CFLAGS WAYLAND_CLIENT_LIBS = $WAYLAND_CLIENT_LIBS WAYLAND_SCANNER = $WAYLAND_SCANNER +WAYLAND_EXTRA_LIBS = $WAYLAND_EXTRA_LIBS PCAP_LIBS = $PCAP_LIBS PCSCLITE_LIBS = $PCSCLITE_LIBS INOTIFY_CFLAGS = $INOTIFY_CFLAGS diff --git a/configure.ac b/configure.ac index 3af0aa5fa98..57116268d51 100644 --- a/configure.ac +++ b/configure.ac @@ -1343,7 +1343,10 @@ then [AC_CHECK_HEADERS([wayland-client.h], [AC_CHECK_LIB(wayland-client,wl_display_connect, [AC_PATH_PROG(WAYLAND_SCANNER,wayland-scanner, - [`test -n "$PKG_CONFIG" && $PKG_CONFIG --variable=wayland_scanner wayland-scanner 2>/dev/null`])], + [`test -n "$PKG_CONFIG" && $PKG_CONFIG --variable=wayland_scanner wayland-scanner 2>/dev/null`]) + ac_save_LIBS=$LIBS + AC_SEARCH_LIBS(shm_open,rt,[test "$ac_res" = "none required" || AC_SUBST(WAYLAND_EXTRA_LIBS,"$ac_res")]) + LIBS=$ac_save_LIBS], [WAYLAND_CLIENT_LIBS=""],[$WAYLAND_CLIENT_LIBS])])]) fi WINE_NOTICE_WITH(wayland, [test -z "$WAYLAND_CLIENT_LIBS" -o -z "$WAYLAND_SCANNER"], diff --git a/dlls/winewayland.drv/Makefile.in b/dlls/winewayland.drv/Makefile.in index ef75c289b76..ae2ffaf3b10 100644 --- a/dlls/winewayland.drv/Makefile.in +++ b/dlls/winewayland.drv/Makefile.in @@ -1,7 +1,7 @@ MODULE = winewayland.drv UNIXLIB = winewayland.so UNIX_CFLAGS = $(WAYLAND_CLIENT_CFLAGS) -UNIX_LIBS = -lwin32u $(WAYLAND_CLIENT_LIBS) $(PTHREAD_LIBS) +UNIX_LIBS = -lwin32u $(WAYLAND_CLIENT_LIBS) $(PTHREAD_LIBS) $(WAYLAND_EXTRA_LIBS) SOURCES = \ display.c \ diff --git a/dlls/winewayland.drv/wayland.c b/dlls/winewayland.drv/wayland.c index 5d2fe122368..c5942e342d4 100644 --- a/dlls/winewayland.drv/wayland.c +++ b/dlls/winewayland.drv/wayland.c @@ -100,6 +100,10 @@ static void registry_handle_global(void *data, struct wl_registry *registry, version < 2 ? version : 2); xdg_wm_base_add_listener(process_wayland.xdg_wm_base, &xdg_wm_base_listener, NULL); } + else if (strcmp(interface, "wl_shm") == 0) + { + process_wayland.wl_shm = wl_registry_bind(registry, id, &wl_shm_interface, 1); + } } static void registry_handle_global_remove(void *data, struct wl_registry *registry, @@ -192,6 +196,11 @@ BOOL wayland_process_init(void) ERR("Wayland compositor doesn't support xdg_wm_base\n"); return FALSE; } + if (!process_wayland.wl_shm) + { + ERR("Wayland compositor doesn't support wl_shm\n"); + return FALSE; + } wayland_init_display_devices(FALSE); diff --git a/dlls/winewayland.drv/wayland_surface.c b/dlls/winewayland.drv/wayland_surface.c index d9ee42863fa..fa1b8e27cbb 100644 --- a/dlls/winewayland.drv/wayland_surface.c +++ b/dlls/winewayland.drv/wayland_surface.c @@ -28,10 +28,18 @@ #include "wine/debug.h" +#include <errno.h> +#include <fcntl.h> +#include <sys/mman.h> #include <stdlib.h> +#include <stdio.h> +#include <unistd.h> WINE_DEFAULT_DEBUG_CHANNEL(waylanddrv); +/* We only use 4 byte formats. */ +#define WINEWAYLAND_BYTES_PER_PIXEL 4 + static void xdg_surface_handle_configure(void *data, struct xdg_surface *xdg_surface, uint32_t serial) { @@ -199,3 +207,148 @@ void wayland_surface_clear_role(struct wayland_surface *surface) wl_display_flush(process_wayland.wl_display); } + +/********************************************************************** + * wayland_surface_attach_shm + * + * Attaches a SHM buffer on a wayland surface. + */ +void wayland_surface_attach_shm(struct wayland_surface *surface, + struct wayland_shm_buffer *shm_buffer) +{ + TRACE("surface=%p shm_buffer=%p (%dx%d)\n", + surface, shm_buffer, shm_buffer->width, shm_buffer->height); + + wl_surface_attach(surface->wl_surface, shm_buffer->wl_buffer, 0, 0); + wl_surface_damage_buffer(surface->wl_surface, 0, 0, + shm_buffer->width, shm_buffer->height); +} + +/********************************************************************** + * wayland_shmfd_create + * + * Creates a file descriptor representing an anonymous SHM region. + */ +static int wayland_shmfd_create(int size) +{ + int ret; + int fd; + char name[64]; + + do + { + snprintf(name, sizeof(name), "/winewayland-shm-%d-%d", getpid(), rand()); + fd = shm_open(name, O_CREAT | O_EXCL | O_RDWR, 0600); + } while (fd < 0 && (errno == EINTR || errno == EEXIST)); + + if (fd < 0) return -1; + shm_unlink(name); + + fcntl(fd, F_ADD_SEALS, F_SEAL_SHRINK | F_SEAL_SEAL); + + do + { + ret = ftruncate(fd, size); + } while (ret < 0 && errno == EINTR); + + if (ret < 0) + { + close(fd); + return -1; + } + + return fd; +} + +/********************************************************************** + * wayland_shm_buffer_destroy + * + * Destroys a SHM buffer. + */ +void wayland_shm_buffer_destroy(struct wayland_shm_buffer *shm_buffer) +{ + TRACE("%p map=%p\n", shm_buffer, shm_buffer->map_data); + + if (shm_buffer->wl_buffer) + wl_buffer_destroy(shm_buffer->wl_buffer); + if (shm_buffer->map_data != MAP_FAILED) + munmap(shm_buffer->map_data, shm_buffer->map_size); + + free(shm_buffer); +} + +/********************************************************************** + * wayland_shm_buffer_create + * + * Creates a SHM buffer with the specified width, height and format. + */ +struct wayland_shm_buffer *wayland_shm_buffer_create(int width, int height, + enum wl_shm_format format) +{ + struct wayland_shm_buffer *shm_buffer = NULL; + int fd = -1; + struct wl_shm_pool *pool; + int stride, size; + + stride = width * WINEWAYLAND_BYTES_PER_PIXEL; + size = stride * height; + if (size == 0) + { + ERR("Invalid shm_buffer size %dx%d\n", width, height); + goto err; + } + + shm_buffer = calloc(1, sizeof(*shm_buffer)); + if (!shm_buffer) + { + ERR("Failed to allocate space for SHM buffer\n"); + goto err; + } + + TRACE("%p %dx%d format=%d size=%d\n", shm_buffer, width, height, format, size); + + shm_buffer->width = width; + shm_buffer->height = height; + shm_buffer->map_size = size; + shm_buffer->map_data = MAP_FAILED; + + fd = wayland_shmfd_create(size); + if (fd < 0) + { + ERR("Failed to create SHM fd\n"); + goto err; + } + + shm_buffer->map_data = mmap(NULL, size, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0); + if (shm_buffer->map_data == MAP_FAILED) + { + ERR("Failed to mmap SHM fd: %s size=%d\n", strerror(errno), size); + goto err; + } + + pool = wl_shm_create_pool(process_wayland.wl_shm, fd, size); + if (!pool) + { + ERR("Failed to create SHM pool fd=%d size=%d\n", fd, size); + goto err; + } + shm_buffer->wl_buffer = wl_shm_pool_create_buffer(pool, 0, width, height, + stride, format); + wl_shm_pool_destroy(pool); + if (!shm_buffer->wl_buffer) + { + ERR("Failed to create SHM buffer %dx%d\n", width, height); + goto err; + } + + close(fd); + + TRACE("=> map=%p\n", shm_buffer->map_data); + + return shm_buffer; + +err: + if (fd >= 0) close(fd); + if (shm_buffer) wayland_shm_buffer_destroy(shm_buffer); + return NULL; +} diff --git a/dlls/winewayland.drv/waylanddrv.h b/dlls/winewayland.drv/waylanddrv.h index ba938179bc6..2e39367a4c8 100644 --- a/dlls/winewayland.drv/waylanddrv.h +++ b/dlls/winewayland.drv/waylanddrv.h @@ -61,6 +61,7 @@ struct wayland struct zxdg_output_manager_v1 *zxdg_output_manager_v1; struct wl_compositor *wl_compositor; struct xdg_wm_base *xdg_wm_base; + struct wl_shm *wl_shm; struct wl_list output_list; /* Protects the output_list and the wayland_output.current states. */ pthread_mutex_t output_mutex; @@ -105,6 +106,14 @@ struct wayland_surface uint32_t current_serial; }; +struct wayland_shm_buffer +{ + struct wl_buffer *wl_buffer; + int width, height; + void *map_data; + size_t map_size; +}; + /********************************************************************** * Wayland initialization */ @@ -135,12 +144,25 @@ struct wayland_surface *wayland_surface_create(void) DECLSPEC_HIDDEN; void wayland_surface_destroy(struct wayland_surface *surface) DECLSPEC_HIDDEN; void wayland_surface_make_toplevel(struct wayland_surface *surface) DECLSPEC_HIDDEN; void wayland_surface_clear_role(struct wayland_surface *surface) DECLSPEC_HIDDEN; +void wayland_surface_attach_shm(struct wayland_surface *surface, + struct wayland_shm_buffer *shm_buffer) DECLSPEC_HIDDEN; + +/********************************************************************** + * Wayland SHM buffer + */ + +struct wayland_shm_buffer *wayland_shm_buffer_create(int width, int height, + enum wl_shm_format format) DECLSPEC_HIDDEN; +void wayland_shm_buffer_destroy(struct wayland_shm_buffer *shm_buffer) DECLSPEC_HIDDEN; /********************************************************************** * Wayland window surface */ struct window_surface *wayland_window_surface_create(HWND hwnd, const RECT *rect) DECLSPEC_HIDDEN; +BOOL wayland_window_surface_needs_flush(struct window_surface *surface) DECLSPEC_HIDDEN; +void wayland_window_surface_update_wayland_surface(struct window_surface *surface, + struct wayland_surface *wayland_surface) DECLSPEC_HIDDEN; /********************************************************************** * USER driver functions diff --git a/dlls/winewayland.drv/window.c b/dlls/winewayland.drv/window.c index b7fdc0f85fe..dffdd013b96 100644 --- a/dlls/winewayland.drv/window.c +++ b/dlls/winewayland.drv/window.c @@ -114,7 +114,11 @@ static void wayland_win_data_destroy(struct wayland_win_data *data) pthread_mutex_unlock(&win_data_mutex); - if (data->window_surface) window_surface_release(data->window_surface); + if (data->window_surface) + { + wayland_window_surface_update_wayland_surface(data->window_surface, NULL); + window_surface_release(data->window_surface); + } if (data->wayland_surface) wayland_surface_destroy(data->wayland_surface); free(data); } @@ -315,6 +319,12 @@ void WAYLAND_WindowPosChanged(HWND hwnd, HWND insert_after, UINT swp_flags, if (data->wayland_surface) wayland_win_data_update_wayland_surface_state(data); + if (data->window_surface) + { + wayland_window_surface_update_wayland_surface(data->window_surface, + data->wayland_surface); + } + wayland_win_data_release(data); } diff --git a/dlls/winewayland.drv/window_surface.c b/dlls/winewayland.drv/window_surface.c index bee46259d49..2d57401ad8f 100644 --- a/dlls/winewayland.drv/window_surface.c +++ b/dlls/winewayland.drv/window_surface.c @@ -37,6 +37,7 @@ struct wayland_window_surface { struct window_surface header; HWND hwnd; + struct wayland_surface *wayland_surface; RECT bounds; void *bits; pthread_mutex_t mutex; @@ -55,6 +56,15 @@ static inline void reset_bounds(RECT *bounds) bounds->right = bounds->bottom = INT_MIN; } +static void buffer_release(void *data, struct wl_buffer *buffer) +{ + struct wayland_shm_buffer *shm_buffer = data; + TRACE("shm_buffer=%p\n", shm_buffer); + wayland_shm_buffer_destroy(shm_buffer); +} + +static const struct wl_buffer_listener buffer_listener = { buffer_release }; + /*********************************************************************** * wayland_window_surface_lock */ @@ -109,7 +119,44 @@ static void wayland_window_surface_set_region(struct window_surface *window_surf */ static void wayland_window_surface_flush(struct window_surface *window_surface) { - /* TODO */ + struct wayland_window_surface *wws = wayland_window_surface_cast(window_surface); + struct wayland_shm_buffer *shm_buffer; + + wayland_window_surface_lock(window_surface); + + if (IsRectEmpty(&wws->bounds)) goto done; + + if (!wws->wayland_surface) + { + ERR("missing wayland surface, returning\n"); + goto done; + } + + TRACE("surface=%p hwnd=%p surface_rect=%s bounds=%s\n", wws, wws->hwnd, + wine_dbgstr_rect(&wws->header.rect), wine_dbgstr_rect(&wws->bounds)); + + shm_buffer = wayland_shm_buffer_create(wws->info.bmiHeader.biWidth, + abs(wws->info.bmiHeader.biHeight), + WL_SHM_FORMAT_XRGB8888); + if (!shm_buffer) + { + ERR("failed to create Wayland SHM buffer, returning\n"); + goto done; + } + + wl_buffer_add_listener(shm_buffer->wl_buffer, &buffer_listener, shm_buffer); + + memcpy(shm_buffer->map_data, wws->bits, shm_buffer->map_size); + + pthread_mutex_lock(&wws->wayland_surface->mutex); + wayland_surface_attach_shm(wws->wayland_surface, shm_buffer); + wl_surface_commit(wws->wayland_surface->wl_surface); + pthread_mutex_unlock(&wws->wayland_surface->mutex); + wl_display_flush(process_wayland.wl_display); + +done: + reset_bounds(&wws->bounds); + wayland_window_surface_unlock(window_surface); } /*********************************************************************** @@ -183,3 +230,20 @@ failed: wayland_window_surface_destroy(&wws->header); return NULL; } + +/*********************************************************************** + * wayland_window_surface_update_wayland_surface + */ +void wayland_window_surface_update_wayland_surface(struct window_surface *window_surface, + struct wayland_surface *wayland_surface) +{ + struct wayland_window_surface *wws = wayland_window_surface_cast(window_surface); + + wayland_window_surface_lock(window_surface); + + TRACE("surface=%p hwnd=%p wayland_surface=%p\n", wws, wws->hwnd, wayland_surface); + + wws->wayland_surface = wayland_surface; + + wayland_window_surface_unlock(window_surface); +} -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2944
Yeah I think it's best to wait for accelerated surface before introducing them, and then only do it on a as-needed basis.
Done, including some extra logic to be able to properly handle reparenting, as we move between child/non-child states.
I don't know how this will work in Wayland, but on X11 we ended up (not upstream but in Proton) with multiple client windows for a single HWND, because of how Windows surfaces can work.
In the Wayland driver the plan is to create a separate `wl_subsurface` (making it a child of the main `wl_surface` for the HWND) to hold any accelerated rendering buffers. This will apply to both child window accelerated rendering and main window accelerated rendering. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2944#note_34922
On Wed Jun 7 10:20:27 2023 +0000, Alexandros Frantzis wrote:
changed this line in [version 3 of the diff](/wine/wine/-/merge_requests/2944/diffs?diff_id=50589&start_sha=cc2ba7ee7ea998f1b311d32b030bc46f3331c5e5#be6b1a6381414c8dae6f91e46942652a04a69f08_131_152) I have kept the internal locking approach (more convenient as you say), and added a check in create to verify we do not already have `win_data`. To minimize the operations performed under lock, I went with a more optimistic concurrency approach, and the check is done after we have already created the new `win_data`.
I think NULL should not be accepted here.
Changed to `assert()`; -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2944#note_34923
On Fri Jun 2 15:38:27 2023 +0000, Alexandros Frantzis wrote:
Will do. Done.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2944#note_34924
On Fri Jun 2 16:21:54 2023 +0000, Rémi Bernon wrote:
I think a `rb_tree` is a good solution as a starting point. If it proves to be ineffective for some reason it can always be changed later. I can only suppose that a hash table have probably been discussed, but I think it is hard to write one that is at the same time simple, intrusive ptr based (and doesn't require external data), and header-only, like the rb tree and lists are. Changed to use rb_tree.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2944#note_34925
On Wed Jun 7 10:45:32 2023 +0000, Alexandros Frantzis wrote:
Yeah I think it's best to wait for accelerated surface before introducing them, and then only do it on a as-needed basis. Done, including some extra logic to be able to properly handle reparenting, as we move between child/non-child states. I don't know how this will work in Wayland, but on X11 we ended up (not upstream but in Proton) with multiple client windows for a single HWND, because of how Windows surfaces can work. In the Wayland driver the plan is to create a separate `wl_subsurface` (making it a child of the main `wl_surface` for the HWND) to hold any accelerated rendering buffers. This will apply to both child window accelerated rendering and main window accelerated rendering. Looking at it again, I'm not sure it's a good idea to duplicate the parent / old_parent information, because then it's not obvious that it is in sync with `NtUserGetAncestor(hwnd, GA_PARENT)`. I also don't see any reason there to keep parent except to compare with old_parent.
Instead, what do you think about comparing something like `has_surface != needs_surface`, checking the current window parent if needed, and decide whether to create or destroy the surface according to that. If you want to be notified of parent changes, I think you can implement the SetParent driver callback, but I guess you know that already and that it would come later. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2944#note_35020
Rémi Bernon (@rbernon) commented about dlls/winewayland.drv/wayland_surface.c:
+ + if (surface->xdg_toplevel) + { + xdg_toplevel_destroy(surface->xdg_toplevel); + surface->xdg_toplevel = NULL; + } + + if (surface->xdg_surface) + { + xdg_surface_destroy(surface->xdg_surface); + surface->xdg_surface = NULL; + } + + surface->pending_serial = 0; + surface->current_serial = 0; + You probably want to lock the surface around there?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2944#note_35021
Rémi Bernon (@rbernon) commented about dlls/winewayland.drv/wayland_surface.c:
+ TRACE("surface=%p\n", surface); + + surface->xdg_surface = + xdg_wm_base_get_xdg_surface(process_wayland.xdg_wm_base, surface->wl_surface); + if (!surface->xdg_surface) goto err; + xdg_surface_add_listener(surface->xdg_surface, &xdg_surface_listener, surface); + + surface->xdg_toplevel = xdg_surface_get_toplevel(surface->xdg_surface); + if (!surface->xdg_toplevel) goto err; + + wl_surface_commit(surface->wl_surface); + wl_display_flush(process_wayland.wl_display); + + /* Wait for the initial configure event */ + while (!surface->pending_serial) + pthread_cond_wait(&surface->configured_cond, &surface->mutex); I don't know how critical this is, but FWIW waiting on condition variables is currently unstable in Wine, and you might end up with crashes that are difficult to debug. If possible, I think you should avoid it.
Also, as a general rule from winex11, I believe we try as much as possible to avoid blocking the threads. So it would be great if this could be asynchronous and decoupled from the configure event handler. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2944#note_35022
On Fri Jun 2 16:29:58 2023 +0000, Rémi Bernon wrote:
Okay, I will need some time to look and think about that, I don't really have a strong opinion anyway but at the same time, like for the previous MR, I'm afraid that my eventual approval won't be enough. What about something like that?
```C static pthread_mutex_t xdg_data_mutex = PTHREAD_MUTEX_INITIALIZER; static struct wayland_surface *wayland_surface_lock_xdg(struct xdg_surface *xdg_surface) { struct wayland_surface *surface; pthread_mutex_lock(&xdg_data_mutex); surface = xdg_surface_get_user_data(xdg_surface); if (surface) pthread_mutex_lock(&surface->mutex); pthread_mutex_unlock(&xdg_data_mutex); return surface; } static void xdg_surface_handle_configure(void *data, struct xdg_surface *xdg_surface, uint32_t serial) { struct wayland_surface *surface; BOOL initial_configure; TRACE("serial=%u\n", serial); if (!(surface = wayland_surface_lock_xdg(xdg_surface))) return; initial_configure = surface->pending_serial == 0 && surface->current_serial == 0; surface->pending_serial = serial; if (initial_configure) pthread_cond_signal(&surface->configured_cond); pthread_mutex_unlock(&surface->mutex); } /* [...] */ void wayland_surface_destroy(struct wayland_surface *surface) { pthread_mutex_lock(&xdg_data_mutex); pthread_mutex_lock(&surface->mutex); if (surface->xdg_toplevel) { xdg_toplevel_destroy(surface->xdg_toplevel); surface->xdg_toplevel = NULL; } if (surface->xdg_surface) { xdg_surface_set_user_data(surface->xdg_surface, NULL); xdg_surface_destroy(surface->xdg_surface); surface->xdg_surface = NULL; } if (surface->wl_surface) { wl_surface_destroy(surface->wl_surface); surface->wl_surface = NULL; } pthread_mutex_unlock(&surface->mutex); pthread_mutex_unlock(&xdg_data_mutex); wl_display_flush(process_wayland.wl_display); pthread_cond_destroy(&surface->configured_cond); pthread_mutex_destroy(&surface->mutex); free(surface); } /* And something similar in wayland_surface_clear_role */ ``` I can also imagine something more generic, with refcounted objects for anything that is used as wl_proxy_user_data, and references held as long as they are in user_data, or used in a callback, instead of this double locking which is slightly ugly. This approach would also avoid unnecessary synchronization between the dispatcher and threads destroying objects. And like I mentioned above, I believe thread synchronization is best avoided in the drivers. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2944#note_35023
Rémi Bernon (@rbernon) commented about dlls/winewayland.drv/wayland_surface.c:
+/********************************************************************** + * wayland_shmfd_create + * + * Creates a file descriptor representing an anonymous SHM region. + */ +static int wayland_shmfd_create(int size) +{ + int ret; + int fd; + char name[64]; + + do + { + snprintf(name, sizeof(name), "/winewayland-shm-%d-%d", getpid(), rand()); + fd = shm_open(name, O_CREAT | O_EXCL | O_RDWR, 0600); + } while (fd < 0 && (errno == EINTR || errno == EEXIST)); We're not using shm_open anywhere else and I have no idea how portable it is and if this is acceptable, or if there's a better way.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2944#note_35024
Rémi Bernon (@rbernon) commented about dlls/winewayland.drv/wayland_surface.c:
+static int wayland_shmfd_create(int size) +{ + int ret; + int fd; + char name[64]; + + do + { + snprintf(name, sizeof(name), "/winewayland-shm-%d-%d", getpid(), rand()); + fd = shm_open(name, O_CREAT | O_EXCL | O_RDWR, 0600); + } while (fd < 0 && (errno == EINTR || errno == EEXIST)); + + if (fd < 0) return -1; + shm_unlink(name); + + fcntl(fd, F_ADD_SEALS, F_SEAL_SHRINK | F_SEAL_SEAL); Same comment with F_ADD_SEALS.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2944#note_35025
On Wed Jun 7 20:54:06 2023 +0000, Rémi Bernon wrote:
You probably want to lock the surface around there? Nvm, it's externally locked. Well then it would need to be adjusted somehow with the locking pattern I suggested above.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2944#note_35029
On Wed Jun 7 20:54:07 2023 +0000, Rémi Bernon wrote:
What about something like that? ```C static pthread_mutex_t xdg_data_mutex = PTHREAD_MUTEX_INITIALIZER; static struct wayland_surface *wayland_surface_lock_xdg(struct xdg_surface *xdg_surface) { struct wayland_surface *surface; pthread_mutex_lock(&xdg_data_mutex); surface = xdg_surface_get_user_data(xdg_surface); if (surface) pthread_mutex_lock(&surface->mutex); pthread_mutex_unlock(&xdg_data_mutex); return surface; } static void xdg_surface_handle_configure(void *data, struct xdg_surface *xdg_surface, uint32_t serial) { struct wayland_surface *surface; BOOL initial_configure; TRACE("serial=%u\n", serial); if (!(surface = wayland_surface_lock_xdg(xdg_surface))) return; initial_configure = surface->pending_serial == 0 && surface->current_serial == 0; surface->pending_serial = serial; if (initial_configure) pthread_cond_signal(&surface->configured_cond); pthread_mutex_unlock(&surface->mutex); } /* [...] */ void wayland_surface_destroy(struct wayland_surface *surface) { pthread_mutex_lock(&xdg_data_mutex); pthread_mutex_lock(&surface->mutex); if (surface->xdg_toplevel) { xdg_toplevel_destroy(surface->xdg_toplevel); surface->xdg_toplevel = NULL; } if (surface->xdg_surface) { xdg_surface_set_user_data(surface->xdg_surface, NULL); xdg_surface_destroy(surface->xdg_surface); surface->xdg_surface = NULL; } if (surface->wl_surface) { wl_surface_destroy(surface->wl_surface); surface->wl_surface = NULL; } pthread_mutex_unlock(&surface->mutex); pthread_mutex_unlock(&xdg_data_mutex); wl_display_flush(process_wayland.wl_display); pthread_cond_destroy(&surface->configured_cond); pthread_mutex_destroy(&surface->mutex); free(surface); } /* And something similar in wayland_surface_clear_role */ ``` I can also imagine something more generic, with refcounted objects for anything that is used as wl_proxy_user_data, and references held as long as they are in user_data, or used in a callback, instead of this double locking which is slightly ugly. This approach would also avoid unnecessary synchronization between the dispatcher and threads destroying objects. And like I mentioned above, I believe thread synchronization is best avoided in the drivers. Thanks @rbernon, I experimented with what you propose and I think it is a good alternative. I am not sure I would consider it simpler than the current proposal (I guess it's complicated in a different way, the implementation details and locking requirements leak into the affected code a bit more), but since it makes trade-offs that seem to be more appropriate for the driver (no thread sync etc), I am happy to go with it in the next iteration of this MR.
I also think (but need to ponder on a bit more) we can avoid changing `wayland_surface_clear_role` at all if we keep the `if (surface->xdg_surface == xdg_surface)` in `xdg_surface_handle_configure` (like we are doing in the current approach). I prefer this compared to introducing the extra xdg user data locking in `wayland_surface_clear_role`, if possible. I haven't experimented with the refcount approach, but I find that the double locking is contained enough to not be a problem, for now at least. If we get more cases that can benefit from a more general refcount based pattern, we can revisit (unless you feel strongly about exploring this more now). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2944#note_35482
On Wed Jun 7 20:54:07 2023 +0000, Rémi Bernon wrote:
I don't know how critical this is, but FWIW waiting on condition variables is currently unstable in Wine, and you might end up with crashes that are difficult to debug. If possible, I think you should avoid it. Also, as a general rule from winex11, I believe we try as much as possible to avoid blocking the threads. So it would be great if this could be asynchronous and decoupled from the configure event handler. Background: We are strictly not allowed to commit a buffer to a `wl_surface` with the `xdg_surface` role before we ack the first configure event (see [protocol text](https://gitlab.freedesktop.org/wayland/wayland-protocols/-/blob/bbe9298e8522...)). Failing to adhere to this requirement will get our client disconnected by the compositor with a [protocol error](https://gitlab.freedesktop.org/wayland/wayland-protocols/-/blob/bbe9298e8522...).
I have an alternative solution that doesn't block waiting for this initial event to arrive. The trade-off is that until that event arrives we must effectively ignore any flushes to `window_surface` (or any other operations that may commit buffers). Of course, when the initial event arrives we can trigger a flush, to ensure the on-screen output is synchronized with window contents, but we may have skipped some frames in the meantime. The initial configure event typically arrives quickly, so in most cases such asynchronous handling is not a concern at all. Please me know if you think this trade-off is acceptable, and I can update the code accordingly. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2944#note_35483
On Wed Jun 7 20:54:08 2023 +0000, Rémi Bernon wrote:
We're not using shm_open anywhere else and I have no idea how portable it is and if this is acceptable, or if there's a better way. We need to create a shared memory fd in some way, and [`shm_open`](https://pubs.opengroup.org/onlinepubs/9699919799/functions/shm_open.html) is the standard POSIX way to do this, so, AFAIK, we are safe from a portability perspective.
There are better platform-specific mechanism to get anonymous shared memory compared to plain `shm_open` + `shm_unlink` (e.g., `memfd_create` on linux, `shm_open(SHM_ANON)` on FreeBSD), which we can introduce in the future if we want. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2944#note_35484
On Wed Jun 7 20:54:08 2023 +0000, Rémi Bernon wrote:
Same comment with F_ADD_SEALS. This is indeed linux specific functionality, so I will remove it for now. If we want, we can introduce it along with `memfd_create` in the future.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2944#note_35485
On Wed Jun 7 20:54:05 2023 +0000, Rémi Bernon wrote:
Looking at it again, I'm not sure it's a good idea to duplicate the parent / old_parent information, because then it's not obvious that it is in sync with `NtUserGetAncestor(hwnd, GA_PARENT)`. I also don't see any reason there to keep parent except to compare with old_parent. Instead, what do you think about comparing something like `has_surface != needs_surface`, checking the current window parent if needed, and decide whether to create or destroy the surface according to that. If you want to be notified of parent changes, I think you can implement the SetParent driver callback, but I guess you know that already and that it would come later. Thanks, I will replace the `old_parent/parent` approach with more robust handling.
Actually, in my latest (non-upstream) code, I ended up not using the `SetParent` callback at all, since it has been more convenient to centralize parent handling (regardless of whether it comes from `NtUserSetParent` or `NtUserCreateWindowEx`) in the `WindowPosChanged` callback (which also is transitively called from `NtUserSetParent`). But please let me know if you think that depending on `WindowPosChanged` could prove to be fragile for this, so that I can adapt accordingly. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2944#note_35486
On Mon Jun 12 16:37:53 2023 +0000, Alexandros Frantzis wrote:
Background: We are strictly not allowed to commit a buffer to a `wl_surface` with the `xdg_surface` role before we ack the first configure event (see [protocol text](https://gitlab.freedesktop.org/wayland/wayland-protocols/-/blob/bbe9298e8522...)). Failing to adhere to this requirement will get our client disconnected by the compositor with a [protocol error](https://gitlab.freedesktop.org/wayland/wayland-protocols/-/blob/bbe9298e8522...). I have an alternative solution that doesn't block waiting for this initial event to arrive. The trade-off is that until that event arrives we must effectively ignore any flushes to `window_surface` (or any other operations that may commit buffers). Of course, when the initial event arrives we can trigger a flush, to ensure the on-screen output is synchronized with window contents, but we may have skipped some frames in the meantime. The initial configure event typically arrives quickly, so in most cases such asynchronous handling is not a concern at all. Please me know if you think this trade-off is acceptable, and I can update the code accordingly. In my opinion it is fine to skip frames.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2944#note_35488
On Mon Jun 12 16:37:53 2023 +0000, Alexandros Frantzis wrote:
Thanks @rbernon, I experimented with what you propose and I think it is a good alternative. I am not sure I would consider it simpler than the current proposal (I guess it's complicated in a different way, the implementation details and locking requirements leak into the affected code a bit more), but since it makes trade-offs that seem to be more appropriate for the driver (no thread sync etc), I am happy to go with it in the next iteration of this MR. I also think (but need to ponder on a bit more) we can avoid changing `wayland_surface_clear_role` at all if we keep the `if (surface->xdg_surface == xdg_surface)` in `xdg_surface_handle_configure` (like we are doing in the current approach). I prefer this compared to introducing the extra xdg user data locking in `wayland_surface_clear_role`, if possible. I haven't experimented with the refcount approach, but I find that the double locking is contained enough to not be a problem, for now at least. If we get more cases that can benefit from a more general refcount based pattern, we can revisit (unless you feel strongly about exploring this more now). I'm fine with the double locking approach for now.
I see now that `wayland_surface_clear_role` is probably fine and I mixed up the object (which validity is guaranteed by libwayland) and the user data (which is not). The user data is only freed in `wayland_surface_destroy`. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2944#note_35489
participants (3)
-
Alexandros Frantzis -
Alexandros Frantzis (＠afrantzis)
-
Rémi Bernon