[PATCH v2 0/5] MR2944: winewayland.drv: First window contents on screen

[Alexandros Frantzis (＠afrantzis)]

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!

--
  v2: winewayland.drv: Implement a simple window_surface flush.
 
https://gitlab.winehq.org/wine/wine/-/merge_requests/2944

[PATCH v2 1/5] winewayland.drv: Introduce per-window driver data.

[Alexandros Frantzis]

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          | 120 +++++++++++++++++++++++++
 3 files changed, 127 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..23add167abd 100644
--- a/dlls/winewayland.drv/window.c
+++ b/dlls/winewayland.drv/window.c
@@ -28,8 +28,128 @@
 
 #include "wine/debug.h"
 
+#include <stdlib.h>
+
 WINE_DEFAULT_DEBUG_CHANNEL(waylanddrv);
 
+/* private window data */
+struct wayland_win_data
+{
+    /* hwnd that this private data belongs to */
+    HWND hwnd;
+};
+
+static pthread_mutex_t win_data_mutex = PTHREAD_MUTEX_INITIALIZER;
+static struct wayland_win_data *win_data_context[32768];
+
+static inline int context_idx(HWND hwnd)
+{
+    return LOWORD(hwnd) >> 1;
+}
+
+/***********************************************************************
+ *           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;
+    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);
+    win_data_context[context_idx(hwnd)] = data;
+
+    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);
+
+    win_data_context[context_idx(data->hwnd)] = NULL;
+
+    free(data);
+
+    pthread_mutex_unlock(&win_data_mutex);
+}
+
+/***********************************************************************
+ *           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 wayland_win_data *data;
+
+    if (!hwnd) return NULL;
+
+    pthread_mutex_lock(&win_data_mutex);
+    if ((data = win_data_context[context_idx(hwnd)]) && data->hwnd == hwnd)
+        return data;
+    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)
+{
+    if (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

[PATCH v2 2/5] winewayland.drv: Basic Wayland toplevel surface support.

[Alexandros Frantzis]

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 |  195 ++++
 dlls/winewayland.drv/waylanddrv.h      |   27 +
 dlls/winewayland.drv/waylanddrv_main.c |    1 +
 dlls/winewayland.drv/window.c          |  100 ++
 dlls/winewayland.drv/xdg-shell.xml     | 1351 ++++++++++++++++++++++++
 7 files changed, 1718 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..82e6ce869e6
--- /dev/null
+++ b/dlls/winewayland.drv/wayland_surface.c
@@ -0,0 +1,195 @@
+/*
+ * 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.
+ */
+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 23add167abd..d73f3b8dd69 100644
--- a/dlls/winewayland.drv/window.c
+++ b/dlls/winewayland.drv/window.c
@@ -37,6 +37,12 @@ struct wayland_win_data
 {
     /* hwnd that this private data belongs to */
     HWND hwnd;
+    /* parent hwnd for child windows */
+    HWND parent;
+    /* wayland surface (if any) for this window */
+    struct wayland_surface *wayland_surface;
+    /* whether this window is visible */
+    BOOL visible;
 };
 
 static pthread_mutex_t win_data_mutex = PTHREAD_MUTEX_INITIALIZER;
@@ -83,6 +89,7 @@ static void wayland_win_data_destroy(struct wayland_win_data *data)
 
     win_data_context[context_idx(data->hwnd)] = NULL;
 
+    if (data->wayland_surface) wayland_surface_destroy(data->wayland_surface);
     free(data);
 
     pthread_mutex_unlock(&win_data_mutex);
@@ -117,6 +124,67 @@ static void wayland_win_data_release(struct wayland_win_data *data)
     if (data) pthread_mutex_unlock(&win_data_mutex);
 }
 
+static BOOL wayland_win_data_wayland_surface_needs_update(struct wayland_win_data *data)
+{
+    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. */
+    if (data->wayland_surface && !data->parent)
+    {
+        BOOL visible = data->wayland_surface->xdg_toplevel != NULL;
+        if (data->visible != 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);
+
+    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 && !data->parent) 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
  */
@@ -138,6 +206,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),
@@ -145,11 +215,41 @@ BOOL WAYLAND_WindowPosChanging(HWND hwnd, HWND insert_after, UINT swp_flags,
 
     if (!data && !(data = wayland_win_data_create(hwnd))) return TRUE;
 
+    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

[PATCH v2 3/5] winewayland.drv: Ensure Wayland surface handlers don't access invalid data.

[Alexandros Frantzis]

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 82e6ce869e6..b23a1694e4a 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

[PATCH v2 4/5] winewayland.drv: Introduce window_surface for Wayland.

[Alexandros Frantzis]

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 d73f3b8dd69..16df33a4ab4 100644
--- a/dlls/winewayland.drv/window.c
+++ b/dlls/winewayland.drv/window.c
@@ -43,6 +43,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 pthread_mutex_t win_data_mutex = PTHREAD_MUTEX_INITIALIZER;
@@ -89,6 +91,7 @@ static void wayland_win_data_destroy(struct wayland_win_data *data)
 
     win_data_context[context_idx(data->hwnd)] = NULL;
 
+    if (data->window_surface) window_surface_release(data->window_surface);
     if (data->wayland_surface) wayland_surface_destroy(data->wayland_surface);
     free(data);
 
@@ -208,6 +211,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),
@@ -220,6 +224,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;
@@ -241,6 +268,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

[PATCH v2 5/5] winewayland.drv: Implement a simple window_surface flush.

[Alexandros Frantzis]

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 b23a1694e4a..a8e79514fe9 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)
 {
@@ -197,3 +205,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 16df33a4ab4..32056612fe9 100644
--- a/dlls/winewayland.drv/window.c
+++ b/dlls/winewayland.drv/window.c
@@ -91,7 +91,11 @@ static void wayland_win_data_destroy(struct wayland_win_data *data)
 
     win_data_context[context_idx(data->hwnd)] = NULL;
 
-    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);
 
@@ -278,6 +282,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

[Rémi Bernon]

Rémi Bernon (@rbernon) commented about dlls/winewayland.drv/window.c:
> {
>      /* hwnd that this private data belongs to */
>      HWND hwnd;
> +    /* parent hwnd for child windows */
> +    HWND parent;
> Unless there is a specific reason I'm not sure you want surface for every Win32 windows.

I believe it is unnecessary and may cause more harm than good. Wine already handles compositing of child windows onto their parent surface (though there's possibly some tricks with cross-process rendering), and winex11 for instance only realize top-level windows into X11 windows.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/2944#note_34245

[Rémi Bernon]

Rémi Bernon (@rbernon) commented about dlls/winewayland.drv/window.c:
> +    pthread_mutex_lock(&win_data_mutex);
> +    if ((data = win_data_context[context_idx(hwnd)]) && data->hwnd == hwnd)
> +        return data;
> +    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)
> +{
> +    if (data) pthread_mutex_unlock(&win_data_mutex);
> I think NULL should not be accepted here.

I can see that the code is adapted pretty much pristine from winex11, but in my opinion it is not necessarily a good reference. Having a new driver should be an occasion to write better code, not one to duplicate (potentially not great) code. The winex11 driver works well, yes, but it is also extremely difficult to refactor. It would be great not to spread that difficulty.

For instance, the window data creation pattern `if (!get && !create)` looks awkward and racy at first as it is unguarded. It is only working because of how `WindowPosChanging` is called, and I don't think we should rely on this.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/2944#note_34248

[Rémi Bernon]

Rémi Bernon (@rbernon) commented about dlls/winewayland.drv/window.c:
> +
> +    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);
> +
> +    win_data_context[context_idx(data->hwnd)] = NULL;
> +
> +    free(data);
> You should probably free outside of the mutex.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/2944#note_34249

[Rémi Bernon]

Rémi Bernon (@rbernon) commented about dlls/winewayland.drv/window.c:
> WINE_DEFAULT_DEBUG_CHANNEL(waylanddrv);
>
> +/* private window data */
> +struct wayland_win_data
> +{
> +    /* hwnd that this private data belongs to */
> +    HWND hwnd;
> +};
> +
> +static pthread_mutex_t win_data_mutex = PTHREAD_MUTEX_INITIALIZER;
> +static struct wayland_win_data *win_data_context[32768];
> +
> +static inline int context_idx(HWND hwnd)
> +{
> +    return LOWORD(hwnd) >> 1;
> +}
> I like the simplicity but I'm tempted to say that this is wasting memory. I also don't like very much that this relies on some implicit property of HWND values. Maybe it is alright, but I don't know how stable or correct they are.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/2944#note_34250

[Rémi Bernon]

Rémi Bernon (@rbernon) commented about dlls/winewayland.drv/wayland.c:
> +        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)
> This looks quite complicated (and a lot like the event dispatch mechanism that was not well received in the previous MR) just to make sure the surfaces aren't used anymore. There's maybe an easier way to do?

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/2944#note_34251

[Alexandros Frantzis (＠afrantzis)]

On Wed May 31 20:42:18 2023 +0000, Rémi Bernon wrote:
> I like the simplicity but I'm tempted to say that this is wasting
> memory. I also don't like very much that this relies on some implicit
> property of HWND values. Maybe it is alright, but I don't know how
> stable or correct they are.
> This is another case of me copying what another driver is doing (wineandroid), expecting it to be a reasonable approach (and I think it basically is) :) If there are any concerns about this code, I am happy to replace it with a simple map implementation. Since I don't want to implement a custom hashtable just for this, I am tempted to use `rb_tree`. Although `rb_tree` won't be ideal for a small number of windows (high constant factors), it's readily available and has a good asymptotic behavior, so we won't choke if any app decides to create thousands of windows. As a side note, I am curious if there have ever been any thoughts or discussions about introducing a hashtable implementation in include/wine, since that would been a good match for this use case.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/2944#note_34456

[Alexandros Frantzis (＠afrantzis)]

On Wed May 31 20:42:17 2023 +0000, Rémi Bernon wrote:
> I think NULL should not be accepted here.
> I can see that the code is adapted pretty much pristine from winex11,
> but in my opinion it is not necessarily a good reference. Having a new
> driver should be an occasion to write better code, not one to duplicate
> (potentially not great) code. The winex11 driver works well, yes, but it
> is also extremely difficult to refactor. It would be great not to spread
> that difficulty.
> For instance, the window data creation pattern `if (!get && !create)`
> looks awkward and racy at first as it is unguarded. It is only working
> because of how `WindowPosChanging` is called, and I don't think we
> should rely on this.
> Seems very sensible. I will move to a more traditional/explicit locking pattern, getting rid of `wayland_win_data_release()` completely and making `wayland_win_data_get()` not perform any internal locking. For example for the data creation could look like (in pseudo-code) :

```c
lock;
if (!get && !create) { unlock; return; }
...
unlock;
```

Let me know if you had some other approach in mind.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/2944#note_34454

[Alexandros Frantzis (＠afrantzis)]

On Wed May 31 20:42:17 2023 +0000, Rémi Bernon wrote:
> You should probably free outside of the mutex.
> Will do.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/2944#note_34455

[Alexandros Frantzis (＠afrantzis)]

On Wed May 31 20:27:36 2023 +0000, Rémi Bernon wrote:
> Unless there is a specific reason I'm not sure you want surface for
> every Win32 windows.
> I believe it is unnecessary and may cause more harm than good. Wine
> already handles compositing of child windows onto their parent surface
> (though there's possibly some tricks with cross-process rendering), and
> winex11 for instance only realize top-level windows into X11 windows.
> `wl_surface` for child windows are not currently used, but will be when we want to enable rendering GL/Vulkan content into child windows. Although the existence of `wl_surface` doesn't really hurt at the moment (except in terms of unused wayland objects, but perhaps I am missing something?), I am fine explicitly not creating them for now, until the accelerated rendering use case becomes a reality.

Note that regardless of the existence or not of a `wl_surface`, the current code will composite all child windows into the parent as you describe (i.e., we always return a NULL `window_surface` for them).

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/2944#note_34457

[Alexandros Frantzis (＠afrantzis)]

On Wed May 31 20:43:50 2023 +0000, Rémi Bernon wrote:
> This looks quite complicated (and a lot like the event dispatch
> mechanism that was not well received in the previous MR) just to make
> sure the surfaces aren't used anymore. There's maybe an easier way to do?
> TL;DR: Although I would also prefer to avoid any additional complexity, I haven't been able to find a better approach, given the constraints of the libwayland API and the impedance mismatch between the win32/wayland multi-threading dispatch models.

Due to the way multi-threading support works in libwayland, there are cases related to object destruction, like this one (see commit message for more details), where we need to ensure that any event handler currently executing will be finished before it's safe to continue 100% race-free (this is trivially true in single-threaded applications). Let me know if you would like me to go into more detail about this issue (we will need to dive into libwayland dispatch internals).

The high-level blocking API `wl_display_dispatch_queue` we used before doesn't provide any (direct, but see alternative below) mechanism to enforce this guarantee, so the currently proposed approach is to use the lower-level libwayland APIs that allow us to exercise more control on when events are dispatched (or not dispatched, in this case) in a thread.

The proposed mechanism might seem complex and arcane (and, to be fair, it is), but it is a standard libwayland idiom recommended by the documentation (see https://gitlab.freedesktop.org/wayland/wayland/-/blob/main/src/wayland-clien...) to enable a dispatch thread to handle multiple event sources. In fact, the `wl_display_dispatch_queue` function we used before uses this exact prepare/poll/read dance internally (https://gitlab.freedesktop.org/wayland/wayland/-/blob/main/src/wayland-clien...), but only reads from one source, the compositor fd. Our custom dispatch function just extends this mechanism so that we poll for an additional fd (the callback pipe), to allow us to ensure that after calling our sync function, all previous events dispatches have finished.

I have experimented with several other approaches and the only one I have found that is somewhat simpler in terms of client code while still being correct/race-free is: https://gitlab.winehq.org/afrantzis/wine/-/commit/9af3e50c856432922dce5c9567.... However, despite the apparent simplicity there is a lot of complexity involved in proving the correctness of this alternative. I have been in contact with libwayland upstream about this, and it was not a trivial exercise for us to be fully convinced that this is correct, involving digging into the libwayland internals and nuances. It's also less performant since it involves a possible roundtrip to the compositor. For these reasons, and, FWIW, the upstream recommendation was against this approach, and rather strongly in favor of the battle-tested prepare/poll/read dance.
> and a lot like the event dispatch mechanism that was not well received in the previous MR

Although some parts of the mechanism are similar (e.g., using pipe fds for thread notification/wakeup), this is distinctly different in that it doesn't interact at all with wine event/message dispatch, it's a just a way to interject callbacks into the internal Wayland event dispatch thread.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/2944#note_34458

[Rémi Bernon]

On Fri Jun  2 15:38:26 2023 +0000, Alexandros Frantzis wrote:
> Seems very sensible. I will move to a more traditional/explicit locking
> pattern, getting rid of `wayland_win_data_release()` completely and
> making `wayland_win_data_get()` not perform any internal locking. For
> example for the data creation could look like (in pseudo-code) :
> ```c
> lock;
> if (!get && !create) { unlock; return; }
> ...
> unlock;
> ```
> Let me know if you had some other approach in mind.
> Sure. You can also keep the internal locking, as it may be more convenient, as long as `create` checks first if is has been beaten to it.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/2944#note_34463

[Rémi Bernon]

On Fri Jun  2 15:38:27 2023 +0000, Alexandros Frantzis wrote:
> This is another case of me copying what another driver is doing
> (wineandroid), expecting it to be a reasonable approach (and I think it
> basically is) :) If there are any concerns about this code, I am happy
> to replace it with a simple map implementation. Since I don't want to
> implement a custom hashtable just for this, I am tempted to use
> `rb_tree`. Although `rb_tree` won't be ideal for a small number of
> windows (high constant factors), it's readily available and has a good
> asymptotic behavior, so we won't choke if any app decides to create
> thousands of windows. As a side note, I am curious if there have ever
> been any thoughts or discussions about introducing a hashtable
> implementation in include/wine, since that would been a good match for
> this use case.
> 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.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/2944#note_34466

[Rémi Bernon]

On Fri Jun  2 15:38:27 2023 +0000, Alexandros Frantzis wrote:
> `wl_surface` for child windows are not currently used, but will be when
> we want to enable rendering GL/Vulkan content into child windows.
> Although the existence of `wl_surface` doesn't really hurt at the moment
> (except in terms of unused wayland objects, but perhaps I am missing
> something?), I am fine explicitly not creating them for now, until the
> accelerated rendering use case becomes a reality.
> Note that regardless of the existence or not of a `wl_surface`, the
> current code will composite all child windows into the parent as you
> describe (i.e., we always return a NULL `window_surface` for them).
> Yeah I think it's best to wait for accelerated surface before introducing them, and then only do it on a as-needed basis. Fwiw there can sometimes be a huge number of child Win32 windows created, none of which will need any acceleration, and you probably don't want to create host objects needlessly.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/2944#note_34467

[Rémi Bernon]

On Fri Jun  2 16:24:35 2023 +0000, Rémi Bernon 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. Fwiw there
> can sometimes be a huge number of child Win32 windows created, none of
> which will need any acceleration, and you probably don't want to create
> host objects needlessly.
> Also note that, from the winex11 experience at least, it looks like accelerated surfaces should not be exactly a one-to-one match between HWND and host windows. 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.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/2944#note_34468

[Rémi Bernon]

On Fri Jun  2 15:38:27 2023 +0000, Alexandros Frantzis wrote:
> TL;DR: Although I would also prefer to avoid any additional complexity,
> I haven't been able to find a better approach, given the constraints of
> the libwayland API and the impedance mismatch between the win32/wayland
> multi-threading dispatch models.
> Due to the way multi-threading support works in libwayland, there are
> cases related to object destruction, like this one (see commit message
> for more details), where we need to ensure that any event handler
> currently executing will be finished before it's safe to continue 100%
> race-free (this is trivially true in single-threaded applications). Let
> me know if you would like me to go into more detail about this issue (we
> will need to dive into libwayland dispatch internals).
> The high-level blocking API `wl_display_dispatch_queue` we used before
> doesn't provide any (direct, but see alternative below) mechanism to
> enforce this guarantee, so the currently proposed approach is to use the
> lower-level libwayland APIs that allow us to exercise more control on
> when events are dispatched (or not dispatched, in this case) in a thread.
> The proposed mechanism might seem complex and arcane (and, to be fair,
> it is), but it is a standard libwayland idiom recommended by the
> documentation (see
> https://gitlab.freedesktop.org/wayland/wayland/-/blob/main/src/wayland-clien...)
> to enable a dispatch thread to handle multiple event sources. In fact,
> the `wl_display_dispatch_queue` function we used before uses this exact
> prepare/poll/read dance internally
> (https://gitlab.freedesktop.org/wayland/wayland/-/blob/main/src/wayland-clien...),
> but only reads from one source, the compositor fd. Our custom dispatch
> function just extends this mechanism so that we poll for an additional
> fd (the callback pipe), to allow us to ensure that after calling our
> sync function, all previous events dispatches have finished.
> I have experimented with several other approaches and the only one I
> have found that is somewhat simpler in terms of client code while still
> being correct/race-free is:
> https://gitlab.winehq.org/afrantzis/wine/-/commit/9af3e50c856432922dce5c9567....
> However, despite the apparent simplicity there is a lot of complexity
> involved in proving the correctness of this alternative. I have been in
> contact with libwayland upstream about this, and it was not a trivial
> exercise for us to be fully convinced that this is correct, involving
> digging into the libwayland internals and nuances. It's also less
> performant since it involves a possible roundtrip to the compositor. For
> these reasons, and, FWIW, the upstream recommendation was against this
> approach, and rather strongly in favor of the battle-tested
> prepare/poll/read dance.
> > and a lot like the event dispatch mechanism that was not well received
> > in the previous MR
> > Although some parts of the mechanism are similar (e.g., using pipe fds
> > for thread notification/wakeup), this is distinctly different in that it
> > doesn't interact at all with wine event/message dispatch, it's a just a
> > way to interject callbacks into the internal Wayland event dispatch thread.
> > 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.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/2944#note_34469