Blob Blame History Raw
From 90188c2648edb137f3564bdea53012355473f105 Mon Sep 17 00:00:00 2001
From: Olivier Fourdan <ofourdan@redhat.com>
Date: Fri, 16 Apr 2021 10:38:23 +0200
Subject: [PATCH xserver 13/27] xwayland/eglstream: Dissociate pending stream
 from window
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Previously, we would have pending streams associated with top level X11
windows, keeping temporary accounting for the pending streams before
they get fully initialized for the xwl_pixmap which would be associated
with X11 pixmaps.

If the window content changes before the stream is ready, the
corresponding pending stream would be marked as invalid and the pending
stream would be eventually removed once the stream becomes ready.

Since commit affc47452 - "xwayland: Drop the separate refcount for the
xwl_pixmap", we no longer keep a separate reference counter for the
xwl_pixmap, but rather tie it to the X11 pixmap lifespan. Yet, the
pending stream would still be associated with the X11 toplevel window.

Dissociate the pending streams from the X11 toplevel window, to keep it
tied only to the xwl_pixmap so that we can have:

 - pixmap <-> xwl_pixmap
 - xwl_pixmap <-> pending stream

Of course, the pending streams remain temporary and get removed as soon
as the ready callback is triggered, but the pending streams are not
linked to the X11 window anymore which can change their content, and
therefore their X11 pixmap at any time.

Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Reviewed-by: Michel Dänzer <mdaenzer@redhat.com>
https://gitlab.freedesktop.org/xorg/xserver/-/issues/1156
(cherry picked from commit cb61ecc7291cfbed2f76d4437cd7450b8e4dab00)
---
 hw/xwayland/xwayland-glamor-eglstream.c | 131 +++++++++++-------------
 1 file changed, 60 insertions(+), 71 deletions(-)

diff --git a/hw/xwayland/xwayland-glamor-eglstream.c b/hw/xwayland/xwayland-glamor-eglstream.c
index 77b24a4b4..32f9a326f 100644
--- a/hw/xwayland/xwayland-glamor-eglstream.c
+++ b/hw/xwayland/xwayland-glamor-eglstream.c
@@ -93,6 +93,7 @@ struct xwl_pixmap {
     /* add any new <= 4-byte member here to avoid holes on 64-bit */
     struct xwl_screen *xwl_screen;
     struct wl_buffer *buffer;
+    struct xwl_eglstream_pending_stream *pending_stream;
 
     /* XWL_PIXMAP_EGLSTREAM. */
     EGLStreamKHR stream;
@@ -103,7 +104,6 @@ struct xwl_pixmap {
 };
 
 static DevPrivateKeyRec xwl_eglstream_private_key;
-static DevPrivateKeyRec xwl_eglstream_window_private_key;
 
 static inline struct xwl_eglstream_private *
 xwl_eglstream_get(struct xwl_screen *xwl_screen)
@@ -112,21 +112,6 @@ xwl_eglstream_get(struct xwl_screen *xwl_screen)
                             &xwl_eglstream_private_key);
 }
 
-static inline struct xwl_eglstream_pending_stream *
-xwl_eglstream_window_get_pending(WindowPtr window)
-{
-    return dixLookupPrivate(&window->devPrivates,
-                            &xwl_eglstream_window_private_key);
-}
-
-static inline void
-xwl_eglstream_window_set_pending(WindowPtr window,
-                                 struct xwl_eglstream_pending_stream *stream)
-{
-    dixSetPrivate(&window->devPrivates,
-                  &xwl_eglstream_window_private_key, stream);
-}
-
 static GLint
 xwl_eglstream_compile_glsl_prog(GLenum type, const char *source)
 {
@@ -323,7 +308,6 @@ xwl_glamor_eglstream_destroy_pending_stream(struct xwl_eglstream_pending_stream
 {
     if (pending->cb)
         wl_callback_destroy(pending->cb);
-    xwl_eglstream_window_set_pending(pending->window, NULL);
     xorg_list_del(&pending->link);
     free(pending);
 }
@@ -331,16 +315,9 @@ xwl_glamor_eglstream_destroy_pending_stream(struct xwl_eglstream_pending_stream
 static void
 xwl_glamor_eglstream_remove_pending_stream(struct xwl_pixmap *xwl_pixmap)
 {
-    struct xwl_eglstream_private *xwl_eglstream =
-        xwl_eglstream_get(xwl_pixmap->xwl_screen);
-    struct xwl_eglstream_pending_stream *pending;
-
-    xorg_list_for_each_entry(pending,
-                             &xwl_eglstream->pending_streams, link) {
-        if (pending->xwl_pixmap == xwl_pixmap) {
-            xwl_glamor_eglstream_destroy_pending_stream(pending);
-            break;
-        }
+    if (xwl_pixmap->pending_stream) {
+        xwl_glamor_eglstream_destroy_pending_stream(xwl_pixmap->pending_stream);
+        xwl_pixmap->pending_stream = NULL;
     }
 }
 
@@ -363,23 +340,41 @@ xwl_glamor_eglstream_get_wl_buffer_for_pixmap(PixmapPtr pixmap)
     return xwl_pixmap_get(pixmap)->buffer;
 }
 
+static void
+xwl_eglstream_maybe_set_pending_stream_invalid(PixmapPtr pixmap)
+{
+    struct xwl_pixmap *xwl_pixmap;
+    struct xwl_eglstream_pending_stream *pending;
+
+    xwl_pixmap = xwl_pixmap_get(pixmap);
+    if (!xwl_pixmap)
+        return;
+
+    pending = xwl_pixmap->pending_stream;
+    if (!pending)
+        return;
+
+    pending->is_valid = FALSE;
+}
+
 static void
 xwl_eglstream_set_window_pixmap(WindowPtr window, PixmapPtr pixmap)
 {
-    struct xwl_screen *xwl_screen = xwl_screen_get(window->drawable.pScreen);
+    ScreenPtr screen = window->drawable.pScreen;
+    struct xwl_screen *xwl_screen = xwl_screen_get(screen);
     struct xwl_eglstream_private *xwl_eglstream =
         xwl_eglstream_get(xwl_screen);
-    struct xwl_eglstream_pending_stream *pending;
+    PixmapPtr old_pixmap;
 
-    pending = xwl_eglstream_window_get_pending(window);
-    if (pending) {
-        /* The pixmap for this window has changed before the compositor
-         * finished attaching the consumer for the window's pixmap's original
-         * eglstream. A producer can no longer be attached, so the stream's
-         * useless
-         */
-        pending->is_valid = FALSE;
-    }
+    /* The pixmap for this window has changed.
+     * If that occurs while there is a stream pending, i.e. before the
+     * compositor has finished attaching the consumer for the window's
+     * pixmap's original eglstream, then a producer could no longer be
+     * attached, so the stream would be useless.
+     */
+    old_pixmap = (*screen->GetWindowPixmap) (window);
+    if (old_pixmap)
+        xwl_eglstream_maybe_set_pending_stream_invalid(old_pixmap);
 
     xwl_screen->screen->SetWindowPixmap = xwl_eglstream->SetWindowPixmap;
     (*xwl_screen->screen->SetWindowPixmap)(window, pixmap);
@@ -489,16 +484,18 @@ xwl_eglstream_print_error(EGLDisplay egl_display,
  * - Receive pixmap B's stream callback, fall over and fail because the
  *   window's surface now incorrectly has pixmap A's stream attached to it.
  *
- * We work around this problem by keeping a queue of pending streams, and
- * only allowing one queue entry to exist for each window. In the scenario
- * listed above, this should happen:
+ * We work around this problem by keeping a pending stream associated with
+ * the xwl_pixmap, which itself is associated with the window pixmap.
+ * In the scenario listed above, this should happen:
  *
  * - Begin processing X events...
- * - A window is resized, causing us to add an eglstream (known as eglstream
- *   A) waiting for its consumer to finish attachment to be added to the
- *   queue.
+ * - A window is resized, a new window pixmap is created causing us to
+ *   add an eglstream (known as eglstream A) waiting for its consumer
+ *   to finish attachment.
  * - Resize on same window happens. We invalidate the previously pending
- *   stream and add another one to the pending queue (known as eglstream B).
+ *   stream on the old window pixmap.
+ *   A new window pixmap is attached to the window and another pending
+ *   stream is created for that new pixmap (known as eglstream B).
  * - Begin processing Wayland events...
  * - Receive invalidated callback from compositor for eglstream A, destroy
  *   stream.
@@ -515,6 +512,7 @@ xwl_eglstream_consumer_ready_callback(void *data,
         xwl_eglstream_get(xwl_screen);
     struct xwl_pixmap *xwl_pixmap;
     struct xwl_eglstream_pending_stream *pending;
+    PixmapPtr pixmap;
     Bool found = FALSE;
 
     xorg_list_for_each_entry(pending, &xwl_eglstream->pending_streams, link) {
@@ -528,6 +526,9 @@ xwl_eglstream_consumer_ready_callback(void *data,
     wl_callback_destroy(callback);
     pending->cb = NULL;
 
+    xwl_pixmap = pending->xwl_pixmap;
+    pixmap = pending->pixmap;
+
     if (!pending->is_valid) {
         xwl_eglstream_destroy_pixmap_stream(pending->xwl_pixmap);
         goto out;
@@ -535,12 +536,11 @@ xwl_eglstream_consumer_ready_callback(void *data,
 
     xwl_glamor_egl_make_current(xwl_screen);
 
-    xwl_pixmap = pending->xwl_pixmap;
     xwl_pixmap->surface = eglCreateStreamProducerSurfaceKHR(
         xwl_screen->egl_display, xwl_eglstream->config,
         xwl_pixmap->stream, (int[]) {
-            EGL_WIDTH,  pending->pixmap->drawable.width,
-            EGL_HEIGHT, pending->pixmap->drawable.height,
+            EGL_WIDTH,  pixmap->drawable.width,
+            EGL_HEIGHT, pixmap->drawable.height,
             EGL_NONE
         });
 
@@ -555,14 +555,14 @@ xwl_eglstream_consumer_ready_callback(void *data,
            pending->window->drawable.id, pending->pixmap);
 
 out:
-    xwl_glamor_eglstream_destroy_pending_stream(pending);
+    xwl_glamor_eglstream_remove_pending_stream(xwl_pixmap);
 }
 
 static const struct wl_callback_listener consumer_ready_listener = {
     xwl_eglstream_consumer_ready_callback
 };
 
-static void
+static struct xwl_eglstream_pending_stream *
 xwl_eglstream_queue_pending_stream(struct xwl_screen *xwl_screen,
                                    WindowPtr window, PixmapPtr pixmap)
 {
@@ -570,14 +570,8 @@ xwl_eglstream_queue_pending_stream(struct xwl_screen *xwl_screen,
         xwl_eglstream_get(xwl_screen);
     struct xwl_eglstream_pending_stream *pending_stream;
 
-#ifdef DEBUG
-    if (!xwl_eglstream_window_get_pending(window))
-        DebugF("eglstream: win %d begins new eglstream for pixmap %p\n",
-               window->drawable.id, pixmap);
-    else
-        DebugF("eglstream: win %d interrupts and replaces pending eglstream for pixmap %p\n",
-               window->drawable.id, pixmap);
-#endif
+    DebugF("eglstream: win %d queues new pending stream for pixmap %p\n",
+           window->drawable.id, pixmap);
 
     pending_stream = malloc(sizeof(*pending_stream));
     pending_stream->window = window;
@@ -586,11 +580,12 @@ xwl_eglstream_queue_pending_stream(struct xwl_screen *xwl_screen,
     pending_stream->is_valid = TRUE;
     xorg_list_init(&pending_stream->link);
     xorg_list_add(&pending_stream->link, &xwl_eglstream->pending_streams);
-    xwl_eglstream_window_set_pending(window, pending_stream);
 
     pending_stream->cb = wl_display_sync(xwl_screen->display);
     wl_callback_add_listener(pending_stream->cb, &consumer_ready_listener,
                              xwl_screen);
+
+    return pending_stream;
 }
 
 static void
@@ -663,7 +658,8 @@ xwl_eglstream_create_pixmap_and_stream(struct xwl_screen *xwl_screen,
     wl_eglstream_controller_attach_eglstream_consumer(
         xwl_eglstream->controller, xwl_window->surface, xwl_pixmap->buffer);
 
-    xwl_eglstream_queue_pending_stream(xwl_screen, window, pixmap);
+    xwl_pixmap->pending_stream =
+        xwl_eglstream_queue_pending_stream(xwl_screen, window, pixmap);
 
 fail:
     if (stream_fd >= 0)
@@ -674,22 +670,19 @@ static Bool
 xwl_glamor_eglstream_allow_commits(struct xwl_window *xwl_window)
 {
     struct xwl_screen *xwl_screen = xwl_window->xwl_screen;
-    struct xwl_eglstream_pending_stream *pending =
-        xwl_eglstream_window_get_pending(xwl_window->window);
     PixmapPtr pixmap =
         (*xwl_screen->screen->GetWindowPixmap)(xwl_window->window);
     struct xwl_pixmap *xwl_pixmap = xwl_pixmap_get(pixmap);
 
     if (xwl_pixmap) {
+        struct xwl_eglstream_pending_stream *pending = xwl_pixmap->pending_stream;
+
         if (pending) {
             /* Wait for the compositor to finish connecting the consumer for
              * this eglstream */
-            if (pending->is_valid)
-                return FALSE;
+            assert(pending->is_valid);
 
-            /* The pixmap for this window was changed before the compositor
-             * finished connecting the eglstream for the window's previous
-             * pixmap. Begin creating a new eglstream. */
+            return FALSE;
         } else {
             return TRUE;
         }
@@ -1190,10 +1183,6 @@ xwl_glamor_eglstream_init_screen(struct xwl_screen *xwl_screen)
     xwl_eglstream->SetWindowPixmap = screen->SetWindowPixmap;
     screen->SetWindowPixmap = xwl_eglstream_set_window_pixmap;
 
-    if (!dixRegisterPrivateKey(&xwl_eglstream_window_private_key,
-                               PRIVATE_WINDOW, 0))
-        return FALSE;
-
     return TRUE;
 }
 
-- 
2.31.1