xserver: Branch 'xwayland-21.1' - 2 commits

GitLab Mirror gitlab-mirror at kemper.freedesktop.org
Wed Jun 30 08:49:16 UTC 2021


 hw/xwayland/xwayland-glamor-eglstream.c |  167 +++++---------------------------
 1 file changed, 29 insertions(+), 138 deletions(-)

New commits:
commit d9005a02e9c109e57dcc81b51de3b5778915af26
Author: Olivier Fourdan <ofourdan at redhat.com>
Date:   Fri Apr 30 09:56:05 2021 +0200

    xwayland/eglstream: Remove stream validity
    
    To avoid an EGL stream in the wrong state, if the window pixmap changed
    before the stream was connected, we would still keep the pending stream
    but mark it as invalid. Once the callback is received, the pending would
    be simply discarded.
    
    But all of this is actually to avoid a bug in egl-wayland, there should
    not be any problem with Xwayland destroying an EGL stream while the
    compositor is still using it.
    
    With that bug now fixed in egl-wayland 1.1.7, we can safely drop all
    that logic from Xwayland EGLstream backend.
    
    Signed-off-by: Olivier Fourdan <ofourdan at redhat.com>
    Reviewed-by: Michel Dänzer <mdaenzer at redhat.com>
    Closes: https://gitlab.freedesktop.org/xorg/xserver/-/issues/1189
    (cherry picked from commit 7d509b6f342d9732b567dc4efa867ea24919853b)

diff --git a/hw/xwayland/xwayland-glamor-eglstream.c b/hw/xwayland/xwayland-glamor-eglstream.c
index 2eae9494c..8d18caaf5 100644
--- a/hw/xwayland/xwayland-glamor-eglstream.c
+++ b/hw/xwayland/xwayland-glamor-eglstream.c
@@ -52,15 +52,6 @@
 #include "wayland-eglstream-controller-client-protocol.h"
 #include "linux-dmabuf-unstable-v1-client-protocol.h"
 
-struct xwl_eglstream_pending_stream {
-    PixmapPtr pixmap;
-    WindowPtr window;
-
-    struct wl_callback *cb;
-
-    Bool is_valid;
-};
-
 struct xwl_eglstream_private {
     EGLDeviceEXT egl_device;
     struct wl_eglstream_display *display;
@@ -90,7 +81,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;
+    struct wl_callback *pending_cb;
     Bool wait_for_buffer_release;
 
     /* XWL_PIXMAP_EGLSTREAM. */
@@ -301,20 +292,12 @@ xwl_eglstream_destroy_pixmap_stream(struct xwl_pixmap *xwl_pixmap)
     free(xwl_pixmap);
 }
 
-static void
-xwl_glamor_eglstream_destroy_pending_stream(struct xwl_eglstream_pending_stream *pending)
-{
-    if (pending->cb)
-        wl_callback_destroy(pending->cb);
-    free(pending);
-}
-
 static void
 xwl_glamor_eglstream_remove_pending_stream(struct xwl_pixmap *xwl_pixmap)
 {
-    if (xwl_pixmap->pending_stream) {
-        xwl_glamor_eglstream_destroy_pending_stream(xwl_pixmap->pending_stream);
-        xwl_pixmap->pending_stream = NULL;
+    if (xwl_pixmap->pending_cb) {
+        wl_callback_destroy(xwl_pixmap->pending_cb);
+        xwl_pixmap->pending_cb = NULL;
     }
 }
 
@@ -338,27 +321,13 @@ xwl_glamor_eglstream_get_wl_buffer_for_pixmap(PixmapPtr pixmap)
 }
 
 static void
-xwl_eglstream_maybe_set_pending_stream_invalid(PixmapPtr pixmap)
+xwl_eglstream_cancel_pending_stream(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;
-
-    /* The compositor may still be using the stream, so we can't destroy
-     * it yet. We'll only have a guarantee that the stream is safe to
-     * destroy once we receive the pending wl_display_sync() for this
-     * stream
-     */
-    pending->pixmap->refcnt++;
+    if (xwl_pixmap)
+        xwl_glamor_eglstream_remove_pending_stream(xwl_pixmap);
 }
 
 static void
@@ -378,7 +347,7 @@ xwl_eglstream_set_window_pixmap(WindowPtr window, PixmapPtr pixmap)
      */
     old_pixmap = (*screen->GetWindowPixmap) (window);
     if (old_pixmap && old_pixmap != pixmap)
-        xwl_eglstream_maybe_set_pending_stream_invalid(old_pixmap);
+        xwl_eglstream_cancel_pending_stream(old_pixmap);
 
     xwl_screen->screen->SetWindowPixmap = xwl_eglstream->SetWindowPixmap;
     (*xwl_screen->screen->SetWindowPixmap)(window, pixmap);
@@ -464,68 +433,19 @@ xwl_eglstream_print_error(EGLDisplay egl_display,
     }
 }
 
-/* Because we run asynchronously with our wayland compositor, it's possible
- * that an X client event could cause us to begin creating a stream for a
- * pixmap/window combo before the stream for the pixmap this window
- * previously used has been fully initialized. An example:
- *
- * - Start processing X client events.
- * - X window receives resize event, causing us to create a new pixmap and
- *   begin creating the corresponding eglstream. This pixmap is known as
- *   pixmap A.
- * - X window receives another resize event, and again changes its current
- *   pixmap causing us to create another corresponding eglstream for the same
- *   window. This pixmap is known as pixmap B.
- * - Start handling events from the wayland compositor.
- *
- * Since both pixmap A and B will have scheduled wl_display_sync events to
- * indicate when their respective streams are connected, we will receive each
- * callback in the original order the pixmaps were created. This means the
- * following would happen:
- *
- * - Receive pixmap A's stream callback, attach its stream to the surface of
- *   the window that just orphaned it.
- * - 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 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, 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 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.
- * - Receive callback from compositor for eglstream B, create producer.
- * - Success!
- */
 static void
 xwl_eglstream_consumer_ready_callback(void *data,
                                       struct wl_callback *callback,
                                       uint32_t time)
 {
-    struct xwl_eglstream_pending_stream *pending = data;
-    PixmapPtr pixmap = pending->pixmap;
+    PixmapPtr pixmap = data;
     struct xwl_pixmap *xwl_pixmap = xwl_pixmap_get(pixmap);
     struct xwl_screen *xwl_screen = xwl_pixmap->xwl_screen;
     struct xwl_eglstream_private *xwl_eglstream =
         xwl_eglstream_get(xwl_screen);
 
     wl_callback_destroy(callback);
-    pending->cb = NULL;
-
-    if (!pending->is_valid) {
-        xwl_glamor_eglstream_remove_pending_stream(xwl_pixmap);
-        dixDestroyPixmap(pixmap, 0);
-        return;
-    }
+    xwl_pixmap->pending_cb = NULL;
 
     xwl_glamor_egl_make_current(xwl_screen);
 
@@ -541,42 +461,16 @@ xwl_eglstream_consumer_ready_callback(void *data,
         ErrorF("eglstream: Failed to create EGLSurface for pixmap\n");
         xwl_eglstream_print_error(xwl_screen->egl_display,
                                   xwl_pixmap->stream, eglGetError());
-        goto out;
+    } else {
+        DebugF("eglstream: completes eglstream for pixmap %p, congrats!\n",
+               pixmap);
     }
-
-    DebugF("eglstream: win %d completes eglstream for pixmap %p, congrats!\n",
-           pending->window->drawable.id, pending->pixmap);
-
-out:
-    xwl_glamor_eglstream_remove_pending_stream(xwl_pixmap);
 }
 
 static const struct wl_callback_listener consumer_ready_listener = {
     xwl_eglstream_consumer_ready_callback
 };
 
-static struct xwl_eglstream_pending_stream *
-xwl_eglstream_queue_pending_stream(WindowPtr window, PixmapPtr pixmap)
-{
-    struct xwl_pixmap *xwl_pixmap = xwl_pixmap_get(pixmap);
-    struct xwl_screen *xwl_screen = xwl_pixmap->xwl_screen;
-    struct xwl_eglstream_pending_stream *pending_stream;
-
-    DebugF("eglstream: win %d queues new pending stream for pixmap %p\n",
-           window->drawable.id, pixmap);
-
-    pending_stream = calloc(1, sizeof(*pending_stream));
-    pending_stream->window = window;
-    pending_stream->pixmap = pixmap;
-    pending_stream->is_valid = TRUE;
-
-    pending_stream->cb = wl_display_sync(xwl_screen->display);
-    wl_callback_add_listener(pending_stream->cb, &consumer_ready_listener,
-                             pending_stream);
-
-    return pending_stream;
-}
-
 static void
 xwl_eglstream_buffer_release_callback(void *data)
 {
@@ -656,9 +550,9 @@ 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_pixmap->pending_stream =
-        xwl_eglstream_queue_pending_stream(window, pixmap);
-
+    xwl_pixmap->pending_cb = wl_display_sync(xwl_screen->display);
+    wl_callback_add_listener(xwl_pixmap->pending_cb, &consumer_ready_listener,
+                             pixmap);
 fail:
     if (stream_fd >= 0)
         close(stream_fd);
@@ -673,25 +567,22 @@ xwl_glamor_eglstream_allow_commits(struct xwl_window *xwl_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) {
+        if (xwl_pixmap->pending_cb) {
             /* Wait for the compositor to finish connecting the consumer for
              * this eglstream */
-            assert(pending->is_valid);
-
             return FALSE;
-        } else {
-            if (xwl_pixmap->surface != EGL_NO_SURFACE ||
-                xwl_pixmap->type == XWL_PIXMAP_DMA_BUF)
-                return TRUE;
-
-            /* The pending stream got removed, we have a xwl_pixmap and
-             * yet we do not have a surface.
-             * So something went wrong with the surface creation, retry.
-             */
-            xwl_eglstream_destroy_pixmap_stream(xwl_pixmap);
         }
+
+        if (xwl_pixmap->surface != EGL_NO_SURFACE ||
+            xwl_pixmap->type == XWL_PIXMAP_DMA_BUF) {
+            return TRUE;
+        }
+
+        /* The pending stream got removed, we have a xwl_pixmap and
+         * yet we do not have a surface.
+         * So something went wrong with the surface creation, retry.
+         */
+         xwl_eglstream_destroy_pixmap_stream(xwl_pixmap);
     }
 
     /* Glamor pixmap has no backing stream yet; begin making one and disallow
commit 8c74023712a9e0d79ec669738b726e7dc821de68
Author: Olivier Fourdan <ofourdan at redhat.com>
Date:   Wed Jun 23 11:35:17 2021 +0200

    xwayland/eglstream: Keep pending stream if the pixmap didn't change
    
    If the pixmap does not actually change in set_window_pixmap(), there is
    no need to invalidate the pending stream, if there's one.
    
    Signed-off-by: Olivier Fourdan <ofourdan at redhat.com>
    Suggested-by: Michel Dänzer <mdaenzer at redhat.com>
    Reviewed-by: Michel Dänzer <mdaenzer at redhat.com>
    (cherry picked from commit 2be9f795bc15012dc912f595003e01bb6aa66f54)

diff --git a/hw/xwayland/xwayland-glamor-eglstream.c b/hw/xwayland/xwayland-glamor-eglstream.c
index 0affc954c..2eae9494c 100644
--- a/hw/xwayland/xwayland-glamor-eglstream.c
+++ b/hw/xwayland/xwayland-glamor-eglstream.c
@@ -377,7 +377,7 @@ xwl_eglstream_set_window_pixmap(WindowPtr window, PixmapPtr pixmap)
      * attached, so the stream would be useless.
      */
     old_pixmap = (*screen->GetWindowPixmap) (window);
-    if (old_pixmap)
+    if (old_pixmap && old_pixmap != pixmap)
         xwl_eglstream_maybe_set_pending_stream_invalid(old_pixmap);
 
     xwl_screen->screen->SetWindowPixmap = xwl_eglstream->SetWindowPixmap;


More information about the xorg-commit mailing list