xserver: Branch 'master' - 15 commits

GitLab Mirror gitlab-mirror at kemper.freedesktop.org
Tue May 11 13:03:34 UTC 2021


 glamor/glamor.c                         |    1 
 hw/xwayland/xwayland-cursor.c           |   11 
 hw/xwayland/xwayland-glamor-eglstream.c |  366 +++++++++++++++++++++-----------
 hw/xwayland/xwayland-glamor.c           |    6 
 hw/xwayland/xwayland-glamor.h           |    4 
 hw/xwayland/xwayland-present.c          |    7 
 hw/xwayland/xwayland-window.c           |   13 -
 7 files changed, 276 insertions(+), 132 deletions(-)

New commits:
commit 012350e3db47fef0404346f55968032e62004fcf
Author: Olivier Fourdan <ofourdan at redhat.com>
Date:   Fri Apr 30 16:23:10 2021 +0200

    xwayland/eglstream: Set ALU to GXCopy for blitting
    
    The EGLstream backend's post damage function uses a shader and
    glDrawArrays() to copy the data from the glamor's pixmap texture prior
    to do the eglSwapBuffers().
    
    However, glDrawArrays() can be affected by the GL state, and therefore
    not reliably produce the expected copy, causing the content of the
    buffer to be corrupted.
    
    Make sure to set the ALU to GXCopy prior to call glDrawArrays() to get
    the expected result.
    
    Signed-off-by: Olivier Fourdan <ofourdan at redhat.com>
    Suggested-by: Michel Dänzer <mdaenzer at redhat.com>

diff --git a/hw/xwayland/xwayland-glamor-eglstream.c b/hw/xwayland/xwayland-glamor-eglstream.c
index 64f4e31f5..2094d293a 100644
--- a/hw/xwayland/xwayland-glamor-eglstream.c
+++ b/hw/xwayland/xwayland-glamor-eglstream.c
@@ -33,6 +33,7 @@
 #define EGL_NO_X11
 #include <glamor_egl.h>
 #include <glamor.h>
+#include <glamor_priv.h>
 #include <glamor_transform.h>
 #include <glamor_transfer.h>
 
@@ -727,6 +728,8 @@ xwl_glamor_eglstream_post_damage(struct xwl_window *xwl_window,
      * won't actually draw to it
      */
     xwl_glamor_egl_make_current(xwl_screen);
+    glamor_set_alu(xwl_screen->screen, GXcopy);
+
     glBindFramebuffer(GL_FRAMEBUFFER, 0);
 
     if (eglGetCurrentSurface(EGL_READ) != xwl_pixmap->surface ||
commit d85bfa6ab7495281516f3a4b05dc1ff0b2c4bf91
Author: Olivier Fourdan <ofourdan at redhat.com>
Date:   Tue May 4 10:56:38 2021 +0200

    xwayland/eglstream: Do not always increment pixmap refcnt on commit
    
    Currently, the EGLstream backend would increment the pixmap refcount for
    each commit, and decrease that refcount on the wl_buffer release
    callback.
    
    But that's relying on the compositor sending us a release callback for
    each commit, otherwise the pixmap refcount will keep increasing and the
    pixmap will be leaked.
    
    So instead, increment the refcount on the pixmap only when we have not
    received a release notification for the wl_buffer, to avoid increasing
    the pixmap refcount more than once without a corresponding release
    event.
    
    This way, if the pixmap is still in use when released on the X11 side,
    the EGL stream will be kept until the compositor releases it.
    
    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>

diff --git a/hw/xwayland/xwayland-glamor-eglstream.c b/hw/xwayland/xwayland-glamor-eglstream.c
index 6721acfe8..64f4e31f5 100644
--- a/hw/xwayland/xwayland-glamor-eglstream.c
+++ b/hw/xwayland/xwayland-glamor-eglstream.c
@@ -89,6 +89,7 @@ struct xwl_pixmap {
     struct xwl_screen *xwl_screen;
     struct wl_buffer *buffer;
     struct xwl_eglstream_pending_stream *pending_stream;
+    Bool wait_for_buffer_release;
 
     /* XWL_PIXMAP_EGLSTREAM. */
     EGLStreamKHR stream;
@@ -577,8 +578,16 @@ xwl_eglstream_queue_pending_stream(WindowPtr window, PixmapPtr pixmap)
 static void
 xwl_eglstream_buffer_release_callback(void *data)
 {
-    /* drop the reference we took in post_damage, freeing if necessary */
-    dixDestroyPixmap(data, 0);
+    PixmapPtr pixmap = data;
+    struct xwl_pixmap *xwl_pixmap = xwl_pixmap_get(pixmap);
+
+    assert(xwl_pixmap);
+
+    if (xwl_pixmap->wait_for_buffer_release) {
+        xwl_pixmap->wait_for_buffer_release = FALSE;
+        /* drop the reference we took in the ready callback, freeing if necessary */
+        dixDestroyPixmap(pixmap, 0);
+    }
 }
 
 static const struct wl_buffer_listener xwl_eglstream_buffer_release_listener = {
@@ -606,6 +615,7 @@ xwl_eglstream_create_pixmap_and_stream(struct xwl_screen *xwl_screen,
 
     xwl_glamor_egl_make_current(xwl_screen);
 
+    xwl_pixmap->wait_for_buffer_release = FALSE;
     xwl_pixmap->xwl_screen = xwl_screen;
     xwl_pixmap->surface = EGL_NO_SURFACE;
     xwl_pixmap->stream = eglCreateStreamKHR(xwl_screen->egl_display, NULL);
@@ -762,8 +772,11 @@ xwl_glamor_eglstream_post_damage(struct xwl_window *xwl_window,
         goto out;
     }
 
-    /* hang onto the pixmap until the compositor has released it */
-    pixmap->refcnt++;
+    if (!xwl_pixmap->wait_for_buffer_release) {
+        /* hang onto the pixmap until the compositor has released it */
+        pixmap->refcnt++;
+        xwl_pixmap->wait_for_buffer_release = TRUE;
+    }
 
 out:
     /* Restore previous state */
commit b583395cd38ad101c7541bd8b0e91143ced44703
Author: Olivier Fourdan <ofourdan at redhat.com>
Date:   Fri Apr 30 09:02:29 2021 +0200

    xwayland/eglstream: Check eglSwapBuffers()
    
    EGLstream's post_damage() would unconditionally return success
    regardless of the actual status of the eglSwapBuffers().
    
    Yet, if eglSwapBuffers() fails, we should not post the corresponding
    damage as they wouldn't match the actual content of the buffer.
    
    Use the eglSwapBuffers() return value as the return value for
    post_damage() and do not take a refrence on the pixmap if it fails.
    
    Signed-off-by: Olivier Fourdan <ofourdan at redhat.com>
    Reviewed-by: Michel Dänzer <mdaenzer at redhat.com>

diff --git a/hw/xwayland/xwayland-glamor-eglstream.c b/hw/xwayland/xwayland-glamor-eglstream.c
index 3a3caa976..6721acfe8 100644
--- a/hw/xwayland/xwayland-glamor-eglstream.c
+++ b/hw/xwayland/xwayland-glamor-eglstream.c
@@ -750,14 +750,20 @@ xwl_glamor_eglstream_post_damage(struct xwl_window *xwl_window,
     glDrawArrays(GL_TRIANGLE_FAN, 0, 4);
 
     if (xwl_eglstream->have_egl_damage)
-        eglSwapBuffersWithDamageKHR(xwl_screen->egl_display,
-                                    xwl_pixmap->surface, egl_damage, 1);
+        status = eglSwapBuffersWithDamageKHR(xwl_screen->egl_display,
+                                             xwl_pixmap->surface,
+                                             egl_damage, 1);
     else
-        eglSwapBuffers(xwl_screen->egl_display, xwl_pixmap->surface);
+        status = eglSwapBuffers(xwl_screen->egl_display,
+                                xwl_pixmap->surface);
+
+    if (!status) {
+        ErrorF("eglstream: buffer swap failed, not posting damage\n");
+        goto out;
+    }
 
     /* hang onto the pixmap until the compositor has released it */
     pixmap->refcnt++;
-    status = TRUE;
 
 out:
     /* Restore previous state */
commit a45799971083c47082d085d3732a5b12692cf75b
Author: Olivier Fourdan <ofourdan at redhat.com>
Date:   Mon Apr 19 14:52:38 2021 +0200

    xwayland/eglstream: Fix calloc/malloc
    
    Use calloc() instead of malloc() like the rest of the code.
    
    Also fix the arguments of calloc() calls to match the definition which
    is calloc(size_t nmemb, size_t size).
    
    This is a cleanup patch, no functional change.
    
    Signed-off-by: Olivier Fourdan <ofourdan at redhat.com>
    Reviewed-by: Michel Dänzer <mdaenzer at redhat.com>

diff --git a/hw/xwayland/xwayland-glamor-eglstream.c b/hw/xwayland/xwayland-glamor-eglstream.c
index ce066cd9e..3a3caa976 100644
--- a/hw/xwayland/xwayland-glamor-eglstream.c
+++ b/hw/xwayland/xwayland-glamor-eglstream.c
@@ -562,7 +562,7 @@ xwl_eglstream_queue_pending_stream(WindowPtr window, PixmapPtr pixmap)
     DebugF("eglstream: win %d queues new pending stream for pixmap %p\n",
            window->drawable.id, pixmap);
 
-    pending_stream = malloc(sizeof(*pending_stream));
+    pending_stream = calloc(1, sizeof(*pending_stream));
     pending_stream->window = window;
     pending_stream->pixmap = pixmap;
     pending_stream->is_valid = TRUE;
@@ -596,7 +596,7 @@ xwl_eglstream_create_pixmap_and_stream(struct xwl_screen *xwl_screen,
     struct wl_array stream_attribs;
     int stream_fd = -1;
 
-    xwl_pixmap = calloc(sizeof(*xwl_pixmap), 1);
+    xwl_pixmap = calloc(1, sizeof(*xwl_pixmap));
     if (!xwl_pixmap)
         FatalError("Not enough memory to create pixmap\n");
     xwl_pixmap_set_private(pixmap, xwl_pixmap);
commit 098e0f52c088c6eb52c7e54c5a11cefabd480908
Author: Olivier Fourdan <ofourdan at redhat.com>
Date:   Mon Apr 19 18:11:19 2021 +0200

    xwayland/eglstream: Do not commit without surface
    
    The EGL surface for the xwl_pixmap is created once the stream is ready
    and valid.
    
    If the pixmap's EGL surface fails, for whatever reason, the xwl_pixmap
    will be unusable and will end up as an invalid wl_buffer.
    
    Make sure we do not allow commits in that case and recreate the
    xwl_pixmap/stream.
    
    Signed-off-by: Olivier Fourdan <ofourdan at redhat.com>
    Reviewed-by: Michel Dänzer <mdaenzer at redhat.com>
    https://gitlab.freedesktop.org/xorg/xserver/-/issues/1156

diff --git a/hw/xwayland/xwayland-glamor-eglstream.c b/hw/xwayland/xwayland-glamor-eglstream.c
index 399a691d3..ce066cd9e 100644
--- a/hw/xwayland/xwayland-glamor-eglstream.c
+++ b/hw/xwayland/xwayland-glamor-eglstream.c
@@ -670,7 +670,14 @@ xwl_glamor_eglstream_allow_commits(struct xwl_window *xwl_window)
 
             return FALSE;
         } else {
-            return TRUE;
+            if (xwl_pixmap->surface != EGL_NO_SURFACE)
+                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);
         }
     }
 
commit bee2ebb29f0999862ab39af26c673c00af40b082
Author: Olivier Fourdan <ofourdan at redhat.com>
Date:   Tue Apr 27 14:17:19 2021 +0200

    xwayland/eglstream: Drop the list of pending streams
    
    Now that the pending stream is associated with the xwl_pixmap for
    EGLStream and the xwl_pixmap itself is associated to the pixmap, we have
    a reliable way to get to those data from any pending stream.
    
    As a result, the list of pending streams that we keep in the EGLStream
    global structure becomes useless.
    
    So we can drop the pending stream's xwl_pixmap and also the list of
    pending streams altogether, and save us a walk though that list for each
    callback.
    
    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>

diff --git a/hw/xwayland/xwayland-glamor-eglstream.c b/hw/xwayland/xwayland-glamor-eglstream.c
index 807bfcb1d..399a691d3 100644
--- a/hw/xwayland/xwayland-glamor-eglstream.c
+++ b/hw/xwayland/xwayland-glamor-eglstream.c
@@ -55,12 +55,9 @@ struct xwl_eglstream_pending_stream {
     PixmapPtr pixmap;
     WindowPtr window;
 
-    struct xwl_pixmap *xwl_pixmap;
     struct wl_callback *cb;
 
     Bool is_valid;
-
-    struct xorg_list link;
 };
 
 struct xwl_eglstream_private {
@@ -73,8 +70,6 @@ struct xwl_eglstream_private {
 
     SetWindowPixmapProcPtr SetWindowPixmap;
 
-    struct xorg_list pending_streams;
-
     Bool have_egl_damage;
 
     GLint blit_prog;
@@ -308,7 +303,6 @@ xwl_glamor_eglstream_destroy_pending_stream(struct xwl_eglstream_pending_stream
 {
     if (pending->cb)
         wl_callback_destroy(pending->cb);
-    xorg_list_del(&pending->link);
     free(pending);
 }
 
@@ -514,28 +508,16 @@ xwl_eglstream_consumer_ready_callback(void *data,
                                       struct wl_callback *callback,
                                       uint32_t time)
 {
-    struct xwl_screen *xwl_screen = data;
+    struct xwl_eglstream_pending_stream *pending = data;
+    PixmapPtr pixmap = pending->pixmap;
+    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);
-    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) {
-        if (pending->cb == callback) {
-            found = TRUE;
-            break;
-        }
-    }
-    assert(found);
 
     wl_callback_destroy(callback);
     pending->cb = NULL;
 
-    xwl_pixmap = pending->xwl_pixmap;
-    pixmap = pending->pixmap;
-
     if (!pending->is_valid) {
         xwl_glamor_eglstream_remove_pending_stream(xwl_pixmap);
         dixDestroyPixmap(pixmap, 0);
@@ -571,11 +553,10 @@ static const struct wl_callback_listener consumer_ready_listener = {
 };
 
 static struct xwl_eglstream_pending_stream *
-xwl_eglstream_queue_pending_stream(struct xwl_screen *xwl_screen,
-                                   WindowPtr window, PixmapPtr pixmap)
+xwl_eglstream_queue_pending_stream(WindowPtr window, PixmapPtr pixmap)
 {
-    struct xwl_eglstream_private *xwl_eglstream =
-        xwl_eglstream_get(xwl_screen);
+    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",
@@ -584,14 +565,11 @@ xwl_eglstream_queue_pending_stream(struct xwl_screen *xwl_screen,
     pending_stream = malloc(sizeof(*pending_stream));
     pending_stream->window = window;
     pending_stream->pixmap = pixmap;
-    pending_stream->xwl_pixmap = xwl_pixmap_get(pixmap);
     pending_stream->is_valid = TRUE;
-    xorg_list_init(&pending_stream->link);
-    xorg_list_add(&pending_stream->link, &xwl_eglstream->pending_streams);
 
     pending_stream->cb = wl_display_sync(xwl_screen->display);
     wl_callback_add_listener(pending_stream->cb, &consumer_ready_listener,
-                             xwl_screen);
+                             pending_stream);
 
     return pending_stream;
 }
@@ -667,7 +645,7 @@ xwl_eglstream_create_pixmap_and_stream(struct xwl_screen *xwl_screen,
         xwl_eglstream->controller, xwl_window->surface, xwl_pixmap->buffer);
 
     xwl_pixmap->pending_stream =
-        xwl_eglstream_queue_pending_stream(xwl_screen, window, pixmap);
+        xwl_eglstream_queue_pending_stream(window, pixmap);
 
 fail:
     if (stream_fd >= 0)
@@ -1249,7 +1227,6 @@ xwl_glamor_init_eglstream(struct xwl_screen *xwl_screen)
                   &xwl_eglstream_private_key, xwl_eglstream);
 
     xwl_eglstream->egl_device = egl_device;
-    xorg_list_init(&xwl_eglstream->pending_streams);
 
     xwl_screen->eglstream_backend.init_egl = xwl_glamor_eglstream_init_egl;
     xwl_screen->eglstream_backend.init_wl_registry = xwl_glamor_eglstream_init_wl_registry;
commit e19bf86c17ef9c802fea24410cc6b1f51a19ce7f
Author: Olivier Fourdan <ofourdan at redhat.com>
Date:   Wed Apr 14 17:31:08 2021 +0200

    xwayland/eglstream: Keep a reference to the pixmap
    
    Commit affc47452 - "xwayland: Drop the separate refcount for the
    xwl_pixmap" removed the separate reference counter for the xwl_pixmap
    which holds the EGLStream.
    
    While that works fine for the common case, if the window's pixmap is
    changed before the stream is ready, the older pixmap will be destroyed
    and the xwl_pixmap along with it, even if the compositor is still using
    the stream.
    
    The code that was removed with commit affc47452 was taking care of that
    by increasing the separate reference counter for the xwl_pixmap, but it
    no longer the case.
    
    As a result, we may end up with the EGL stream in the wrong state when
    trying to use it, which will cascade down into all sort of issues.
    
    To avoid the problem, increase the reference count on the pixmap when it
    is marked as invalid in EGLStream's SetWindowPixmap().
    
    This way, the xwl_pixmap and the EGLStream are kept until released by
    the compositor, even when the pixmap changes before stream is ready.
    
    Signed-off-by: Olivier Fourdan <ofourdan at redhat.com>
    Reviewed-by: Michel Dänzer <mdaenzer at redhat.com>
    Fixes: affc47452 xwayland: Drop the separate refcount for the xwl_pixmap
    https://gitlab.freedesktop.org/xorg/xserver/-/issues/1156

diff --git a/hw/xwayland/xwayland-glamor-eglstream.c b/hw/xwayland/xwayland-glamor-eglstream.c
index 32f9a326f..807bfcb1d 100644
--- a/hw/xwayland/xwayland-glamor-eglstream.c
+++ b/hw/xwayland/xwayland-glamor-eglstream.c
@@ -355,6 +355,13 @@ xwl_eglstream_maybe_set_pending_stream_invalid(PixmapPtr pixmap)
         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++;
 }
 
 static void
@@ -530,8 +537,9 @@ xwl_eglstream_consumer_ready_callback(void *data,
     pixmap = pending->pixmap;
 
     if (!pending->is_valid) {
-        xwl_eglstream_destroy_pixmap_stream(pending->xwl_pixmap);
-        goto out;
+        xwl_glamor_eglstream_remove_pending_stream(xwl_pixmap);
+        dixDestroyPixmap(pixmap, 0);
+        return;
     }
 
     xwl_glamor_egl_make_current(xwl_screen);
commit cb61ecc7291cfbed2f76d4437cd7450b8e4dab00
Author: Olivier Fourdan <ofourdan at redhat.com>
Date:   Fri Apr 16 10:38:23 2021 +0200

    xwayland/eglstream: Dissociate pending stream from window
    
    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 at redhat.com>
    Reviewed-by: Michel Dänzer <mdaenzer at redhat.com>
    https://gitlab.freedesktop.org/xorg/xserver/-/issues/1156

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;
 }
 
commit cc596bcfb273eeab82ac3d59867668af8bad2abf
Author: Olivier Fourdan <ofourdan at redhat.com>
Date:   Thu Apr 1 08:46:52 2021 +0200

    xwayland/eglstream: Add more error checking
    
    eglCreateStreamKHR() can fail and return EGL_NO_STREAM_KHR, in which
    case there is no point in trying to create a buffer from it.
    
    Similarly, eglCreateStreamProducerSurfaceKHR() also fail and return
    EGL_NO_SURFACE, which in turn will be used in eglMakeCurrent() as
    draw/read surface, and therefore would mean no draw/read buffer.
    
    In those cases, log the error, and bail out early. That won't solve the
    issue but will help with investigating the root cause of issues with
    EGLStream backend.
    
    Signed-off-by: Olivier Fourdan <ofourdan at redhat.com>
    Reviewed-by: Michel Dänzer <mdaenzer at redhat.com>
    https://gitlab.freedesktop.org/xorg/xserver/-/issues/1156

diff --git a/hw/xwayland/xwayland-glamor-eglstream.c b/hw/xwayland/xwayland-glamor-eglstream.c
index 9abb7b779..77b24a4b4 100644
--- a/hw/xwayland/xwayland-glamor-eglstream.c
+++ b/hw/xwayland/xwayland-glamor-eglstream.c
@@ -387,6 +387,84 @@ xwl_eglstream_set_window_pixmap(WindowPtr window, PixmapPtr pixmap)
     xwl_screen->screen->SetWindowPixmap = xwl_eglstream_set_window_pixmap;
 }
 
+static const char *
+xwl_eglstream_get_error_str(EGLint error)
+{
+    switch (error) {
+    case EGL_BAD_PARAMETER:
+        return "EGL_BAD_PARAMETER";
+    case EGL_BAD_ATTRIBUTE:
+        return "EGL_BAD_ATTRIBUTE";
+    case EGL_BAD_MATCH:
+        return "EGL_BAD_MATCH";
+    case EGL_BAD_ACCESS:
+        return "EGL_BAD_ACCESS";
+    case EGL_BAD_STATE_KHR:
+        return "EGL_BAD_STATE_KHR";
+    case EGL_BAD_STREAM_KHR:
+        return "EGL_BAD_STREAM_KHR";
+    case EGL_BAD_DISPLAY:
+        return "EGL_BAD_DISPLAY";
+    case EGL_NOT_INITIALIZED:
+        return "EGL_NOT_INITIALIZED";
+    default:
+        return "Unknown error";
+    }
+}
+
+static const char *
+xwl_eglstream_get_stream_state_str(EGLint state)
+{
+    switch (state) {
+    case EGL_STREAM_STATE_CREATED_KHR:
+        return "EGL_STREAM_STATE_CREATED_KHR";
+    case EGL_STREAM_STATE_CONNECTING_KHR:
+        return "EGL_STREAM_STATE_CONNECTING_KHR";
+    case EGL_STREAM_STATE_EMPTY_KHR:
+        return "EGL_STREAM_STATE_EMPTY_KHR";
+    case EGL_STREAM_STATE_NEW_FRAME_AVAILABLE_KHR:
+        return "EGL_STREAM_STATE_NEW_FRAME_AVAILABLE_KHR";
+    case EGL_STREAM_STATE_OLD_FRAME_AVAILABLE_KHR:
+        return "EGL_STREAM_STATE_OLD_FRAME_AVAILABLE_KHR";
+    case EGL_STREAM_STATE_DISCONNECTED_KHR:
+        return "EGL_STREAM_STATE_DISCONNECTED_KHR";
+    default:
+        return "Unknown state";
+    }
+}
+
+static EGLint
+xwl_eglstream_get_state(EGLDisplay egl_display, EGLStreamKHR egl_stream)
+{
+    EGLint state;
+
+    eglQueryStreamKHR(egl_display, egl_stream, EGL_STREAM_STATE_KHR, &state);
+    if (!eglQueryStreamKHR(egl_display, egl_stream,
+                           EGL_STREAM_STATE_KHR, &state)) {
+        EGLint state_error = eglGetError();
+        ErrorF("eglstream: Failed to query state - error 0x%X: %s\n",
+               state_error, xwl_eglstream_get_error_str(state_error));
+        return EGL_FALSE;
+    }
+
+    return state;
+}
+
+
+static void
+xwl_eglstream_print_error(EGLDisplay egl_display,
+                          EGLStreamKHR egl_stream, EGLint error)
+{
+    ErrorF("eglstream: error 0x%X: %s\n", error,
+           xwl_eglstream_get_error_str(error));
+
+    if (error == EGL_BAD_STATE_KHR) {
+        EGLint state = xwl_eglstream_get_state(egl_display, egl_stream);
+        ErrorF("eglstream: stream state 0x%X: %s\n", state,
+               xwl_eglstream_get_stream_state_str(state));
+    }
+}
+
 /* 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
@@ -466,6 +544,13 @@ xwl_eglstream_consumer_ready_callback(void *data,
             EGL_NONE
         });
 
+    if (xwl_pixmap->surface == EGL_NO_SURFACE) {
+        ErrorF("eglstream: Failed to create EGLSurface for pixmap\n");
+        xwl_eglstream_print_error(xwl_screen->egl_display,
+                                  xwl_pixmap->stream, eglGetError());
+        goto out;
+    }
+
     DebugF("eglstream: win %d completes eglstream for pixmap %p, congrats!\n",
            pending->window->drawable.id, pending->pixmap);
 
@@ -543,8 +628,16 @@ xwl_eglstream_create_pixmap_and_stream(struct xwl_screen *xwl_screen,
     xwl_pixmap->xwl_screen = xwl_screen;
     xwl_pixmap->surface = EGL_NO_SURFACE;
     xwl_pixmap->stream = eglCreateStreamKHR(xwl_screen->egl_display, NULL);
+    if (xwl_pixmap->stream == EGL_NO_STREAM_KHR) {
+        ErrorF("eglstream: Couldn't create EGL stream.\n");
+        goto fail;
+    }
     stream_fd = eglGetStreamFileDescriptorKHR(xwl_screen->egl_display,
                                               xwl_pixmap->stream);
+    if (stream_fd == EGL_NO_FILE_DESCRIPTOR_KHR) {
+        ErrorF("eglstream: Couldn't get EGL stream file descriptor.\n");
+        goto fail;
+    }
 
     wl_array_init(&stream_attribs);
     xwl_pixmap->buffer =
commit 823f3254fabd16e5e721da57cd260beac9b8f8bd
Author: Olivier Fourdan <ofourdan at redhat.com>
Date:   Thu Apr 15 10:59:36 2021 +0200

    xwayland/eglstream: Small refactoring
    
    Some functions are called "callback" whereas they are not longer
    callback functions or "unref" while they no longer deal with a reference
    counter anymore, which is quite confusing. Rename those functions to be
    more explicit.
    
    Also, the pending streams can be destroyed in different places, move the
    common code to separate function to avoid duplicating code and help with
    readability of the code.
    
    Signed-off-by: Olivier Fourdan <ofourdan at redhat.com>
    Acked-by: Michel Dänzer <mdaenzer at redhat.com>

diff --git a/hw/xwayland/xwayland-glamor-eglstream.c b/hw/xwayland/xwayland-glamor-eglstream.c
index 64fe93b7c..9abb7b779 100644
--- a/hw/xwayland/xwayland-glamor-eglstream.c
+++ b/hw/xwayland/xwayland-glamor-eglstream.c
@@ -287,7 +287,7 @@ xwl_glamor_egl_device_has_egl_extensions(void *device,
 }
 
 static void
-xwl_eglstream_unref_pixmap_stream(struct xwl_pixmap *xwl_pixmap)
+xwl_eglstream_destroy_pixmap_stream(struct xwl_pixmap *xwl_pixmap)
 {
     struct xwl_screen *xwl_screen = xwl_pixmap->xwl_screen;
 
@@ -319,7 +319,17 @@ xwl_eglstream_unref_pixmap_stream(struct xwl_pixmap *xwl_pixmap)
 }
 
 static void
-xwl_glamor_eglstream_del_pending_stream_cb(struct xwl_pixmap *xwl_pixmap)
+xwl_glamor_eglstream_destroy_pending_stream(struct xwl_eglstream_pending_stream *pending)
+{
+    if (pending->cb)
+        wl_callback_destroy(pending->cb);
+    xwl_eglstream_window_set_pending(pending->window, NULL);
+    xorg_list_del(&pending->link);
+    free(pending);
+}
+
+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);
@@ -328,10 +338,7 @@ xwl_glamor_eglstream_del_pending_stream_cb(struct xwl_pixmap *xwl_pixmap)
     xorg_list_for_each_entry(pending,
                              &xwl_eglstream->pending_streams, link) {
         if (pending->xwl_pixmap == xwl_pixmap) {
-            wl_callback_destroy(pending->cb);
-            xwl_eglstream_window_set_pending(pending->window, NULL);
-            xorg_list_del(&pending->link);
-            free(pending);
+            xwl_glamor_eglstream_destroy_pending_stream(pending);
             break;
         }
     }
@@ -343,9 +350,9 @@ xwl_glamor_eglstream_destroy_pixmap(PixmapPtr pixmap)
     struct xwl_pixmap *xwl_pixmap = xwl_pixmap_get(pixmap);
 
     if (xwl_pixmap && pixmap->refcnt == 1) {
-        xwl_glamor_eglstream_del_pending_stream_cb(xwl_pixmap);
+        xwl_glamor_eglstream_remove_pending_stream(xwl_pixmap);
+        xwl_eglstream_destroy_pixmap_stream(xwl_pixmap);
         xwl_pixmap_del_buffer_release_cb(pixmap);
-        xwl_eglstream_unref_pixmap_stream(xwl_pixmap);
     }
     return glamor_destroy_pixmap(pixmap);
 }
@@ -432,8 +439,6 @@ xwl_eglstream_consumer_ready_callback(void *data,
     struct xwl_eglstream_pending_stream *pending;
     Bool found = FALSE;
 
-    wl_callback_destroy(callback);
-
     xorg_list_for_each_entry(pending, &xwl_eglstream->pending_streams, link) {
         if (pending->cb == callback) {
             found = TRUE;
@@ -442,8 +447,11 @@ xwl_eglstream_consumer_ready_callback(void *data,
     }
     assert(found);
 
+    wl_callback_destroy(callback);
+    pending->cb = NULL;
+
     if (!pending->is_valid) {
-        xwl_eglstream_unref_pixmap_stream(pending->xwl_pixmap);
+        xwl_eglstream_destroy_pixmap_stream(pending->xwl_pixmap);
         goto out;
     }
 
@@ -462,9 +470,7 @@ xwl_eglstream_consumer_ready_callback(void *data,
            pending->window->drawable.id, pending->pixmap);
 
 out:
-    xwl_eglstream_window_set_pending(pending->window, NULL);
-    xorg_list_del(&pending->link);
-    free(pending);
+    xwl_glamor_eglstream_destroy_pending_stream(pending);
 }
 
 static const struct wl_callback_listener consumer_ready_listener = {
@@ -514,8 +520,8 @@ static const struct wl_buffer_listener xwl_eglstream_buffer_release_listener = {
 };
 
 static void
-xwl_eglstream_create_pending_stream(struct xwl_screen *xwl_screen,
-                                    WindowPtr window, PixmapPtr pixmap)
+xwl_eglstream_create_pixmap_and_stream(struct xwl_screen *xwl_screen,
+                                       WindowPtr window, PixmapPtr pixmap)
 {
     struct xwl_eglstream_private *xwl_eglstream =
         xwl_eglstream_get(xwl_screen);
@@ -599,8 +605,8 @@ xwl_glamor_eglstream_allow_commits(struct xwl_window *xwl_window)
     /* Glamor pixmap has no backing stream yet; begin making one and disallow
      * commits until then
      */
-    xwl_eglstream_create_pending_stream(xwl_screen, xwl_window->window,
-                                        pixmap);
+    xwl_eglstream_create_pixmap_and_stream(xwl_screen, xwl_window->window,
+                                           pixmap);
 
     return FALSE;
 }
commit 85244d2a2081d61a2e4a06e847041f638de01e3f
Author: Olivier Fourdan <ofourdan at redhat.com>
Date:   Wed Mar 31 09:49:35 2021 +0200

    xwayland/eglstream: Check framebuffer status
    
    The EGLStream backend would sometime generate GL errors trying to draw
    to the framebuffer, which gives an invalid buffer, which in turn would
    generate a Wayland error from the compositor which is fatal to the
    client.
    
    Check the framebuffer status and bail out early if it's not complete,
    to avoid getting into trouble later.
    
    Signed-off-by: Olivier Fourdan <ofourdan at redhat.com>
    Reviewed-by: Michel Dänzer <mdaenzer at redhat.com>
    https://gitlab.freedesktop.org/xorg/xserver/-/issues/1156

diff --git a/hw/xwayland/xwayland-glamor-eglstream.c b/hw/xwayland/xwayland-glamor-eglstream.c
index f64d05064..64fe93b7c 100644
--- a/hw/xwayland/xwayland-glamor-eglstream.c
+++ b/hw/xwayland/xwayland-glamor-eglstream.c
@@ -619,6 +619,7 @@ xwl_glamor_eglstream_post_damage(struct xwl_window *xwl_window,
         box->x2 - box->x1, box->y2 - box->y1
     };
     GLint saved_vao;
+    int status;
 
     if (xwl_pixmap->type != XWL_PIXMAP_EGLSTREAM)
         /* This can happen if a client does X11 rendering on a
@@ -652,6 +653,13 @@ xwl_glamor_eglstream_post_damage(struct xwl_window *xwl_window,
     glUniform1i(xwl_eglstream->blit_is_rgba_pos,
                 pixmap->drawable.depth >= 32);
 
+    status = glCheckFramebufferStatus(GL_FRAMEBUFFER);
+    if (status != GL_FRAMEBUFFER_COMPLETE) {
+        ErrorF("eglstream: Framebuffer incomplete 0x%X, not posting damage\n", status);
+        status = FALSE;
+        goto out;
+    }
+
     /* Blit rendered image into EGLStream surface */
     glDrawBuffer(GL_BACK);
     glDrawArrays(GL_TRIANGLE_FAN, 0, 4);
@@ -662,14 +670,16 @@ xwl_glamor_eglstream_post_damage(struct xwl_window *xwl_window,
     else
         eglSwapBuffers(xwl_screen->egl_display, xwl_pixmap->surface);
 
+    /* hang onto the pixmap until the compositor has released it */
+    pixmap->refcnt++;
+    status = TRUE;
+
+out:
     /* Restore previous state */
     glBindVertexArray(saved_vao);
     glBindTexture(GL_TEXTURE_2D, 0);
 
-    /* hang onto the pixmap until the compositor has released it */
-    pixmap->refcnt++;
-
-    return TRUE;
+    return status;
 }
 
 static Bool
commit 252cbad316f43edc08aa5c844789398a58ba270c
Author: Olivier Fourdan <ofourdan at redhat.com>
Date:   Wed Mar 31 13:57:45 2021 +0200

    xwayland/glamor: Add return status to post_damage
    
    If the glamor backend failed to post damage, the caller should do the
    same to avoid a failure to attach the buffer to the Wayland surface.
    
    Change the API of Xwayland's glamor backend post_damage() to return a
    status so that xwl_window_post_damage() can tell whether the callee
    failed.
    
    Signed-off-by: Olivier Fourdan <ofourdan at redhat.com>
    Reviewed-by: Michel Dänzer <mdaenzer at redhat.com>
    https://gitlab.freedesktop.org/xorg/xserver/-/issues/1156

diff --git a/hw/xwayland/xwayland-glamor-eglstream.c b/hw/xwayland/xwayland-glamor-eglstream.c
index c6e17bf8b..f64d05064 100644
--- a/hw/xwayland/xwayland-glamor-eglstream.c
+++ b/hw/xwayland/xwayland-glamor-eglstream.c
@@ -605,7 +605,7 @@ xwl_glamor_eglstream_allow_commits(struct xwl_window *xwl_window)
     return FALSE;
 }
 
-static void
+static Bool
 xwl_glamor_eglstream_post_damage(struct xwl_window *xwl_window,
                                  PixmapPtr pixmap, RegionPtr region)
 {
@@ -625,7 +625,7 @@ xwl_glamor_eglstream_post_damage(struct xwl_window *xwl_window,
          * flipping OpenGL or Vulkan window. In that case, we don't
          * need to do the copy below.
          */
-        return;
+        return TRUE;
 
     /* Unbind the framebuffer BEFORE binding the EGLSurface, otherwise we
      * won't actually draw to it
@@ -668,6 +668,8 @@ xwl_glamor_eglstream_post_damage(struct xwl_window *xwl_window,
 
     /* hang onto the pixmap until the compositor has released it */
     pixmap->refcnt++;
+
+    return TRUE;
 }
 
 static Bool
diff --git a/hw/xwayland/xwayland-glamor.c b/hw/xwayland/xwayland-glamor.c
index 9e44d5106..e940f9fd7 100644
--- a/hw/xwayland/xwayland-glamor.c
+++ b/hw/xwayland/xwayland-glamor.c
@@ -304,14 +304,16 @@ xwl_glamor_pixmap_get_wl_buffer(PixmapPtr pixmap)
     return NULL;
 }
 
-void
+Bool
 xwl_glamor_post_damage(struct xwl_window *xwl_window,
                        PixmapPtr pixmap, RegionPtr region)
 {
     struct xwl_screen *xwl_screen = xwl_window->xwl_screen;
 
     if (xwl_screen->egl_backend->post_damage)
-        xwl_screen->egl_backend->post_damage(xwl_window, pixmap, region);
+        return xwl_screen->egl_backend->post_damage(xwl_window, pixmap, region);
+
+    return TRUE;
 }
 
 Bool
diff --git a/hw/xwayland/xwayland-glamor.h b/hw/xwayland/xwayland-glamor.h
index 26ab78f04..cf3c4fba3 100644
--- a/hw/xwayland/xwayland-glamor.h
+++ b/hw/xwayland/xwayland-glamor.h
@@ -83,7 +83,7 @@ struct xwl_egl_backend {
      * you should implement blitting from the glamor pixmap to the wayland
      * pixmap here. Otherwise, this callback is optional.
      */
-    void (*post_damage)(struct xwl_window *xwl_window,
+    Bool (*post_damage)(struct xwl_window *xwl_window,
                         PixmapPtr pixmap, RegionPtr region);
 
     /* Called by Xwayland to confirm with the egl backend that the given
@@ -117,7 +117,7 @@ void xwl_glamor_init_wl_registry(struct xwl_screen *xwl_screen,
                                  uint32_t version);
 Bool xwl_glamor_has_wl_interfaces(struct xwl_screen *xwl_screen,
                                  struct xwl_egl_backend *xwl_egl_backend);
-void xwl_glamor_post_damage(struct xwl_window *xwl_window,
+Bool xwl_glamor_post_damage(struct xwl_window *xwl_window,
                             PixmapPtr pixmap, RegionPtr region);
 Bool xwl_glamor_allow_commits(struct xwl_window *xwl_window);
 void xwl_glamor_egl_make_current(struct xwl_screen *xwl_screen);
diff --git a/hw/xwayland/xwayland-window.c b/hw/xwayland/xwayland-window.c
index af4290ec7..00f161eda 100644
--- a/hw/xwayland/xwayland-window.c
+++ b/hw/xwayland/xwayland-window.c
@@ -811,8 +811,12 @@ xwl_window_post_damage(struct xwl_window *xwl_window)
     }
 
 #ifdef XWL_HAS_GLAMOR
-    if (xwl_screen->glamor)
-        xwl_glamor_post_damage(xwl_window, pixmap, region);
+    if (xwl_screen->glamor) {
+        if (!xwl_glamor_post_damage(xwl_window, pixmap, region)) {
+            ErrorF("glamor: Failed to post damage\n");
+            return;
+        }
+    }
 #endif
 
     wl_surface_attach(xwl_window->surface, buffer, 0, 0);
commit 3b265c59a6456f6e4abfb9e1694237bc56f1776a
Author: Olivier Fourdan <ofourdan at redhat.com>
Date:   Tue Mar 30 08:48:25 2021 +0200

    glamor: Dump backtrace on GL error
    
    Currrently, when a GL error is triggered, glamor would log the error
    which may not be sufficient to trace it back to the cause of the error.
    
    Also dump the backtrace which may give more information as to where the
    error comes from.
    
    Signed-off-by: Olivier Fourdan <ofourdan at redhat.com>
    Reviewed-by: Martin Peres <martin.peres at mupuf.org>
    Reviewed-by: Michel Dänzer <mdaenzer at redhat.com>
    https://gitlab.freedesktop.org/xorg/xserver/-/issues/1156

diff --git a/glamor/glamor.c b/glamor/glamor.c
index 3baef4b9f..b8406f42d 100644
--- a/glamor/glamor.c
+++ b/glamor/glamor.c
@@ -414,6 +414,7 @@ glamor_debug_output_callback(GLenum source,
 
     LogMessageVerb(X_ERROR, 0, "glamor%d: GL error: %*s\n",
                screen->myNum, length, message);
+    xorg_backtrace();
 }
 
 /**
commit 25d2f4948f0abd39e099b8ac69b7cb1dab38a10a
Author: Olivier Fourdan <ofourdan at redhat.com>
Date:   Mon Mar 29 15:01:15 2021 +0200

    xwayland: Check buffer prior to attaching it
    
    If the buffer is NULL, do not even try to attach it, and risk a Wayland
    protocol error which would be fatal to us.
    
    Signed-off-by: Olivier Fourdan <ofourdan at redhat.com>
    Reviewed-by: Martin Peres <martin.peres at mupuf.org>
    Reviewed-by: Michel Dänzer <mdaenzer at redhat.com>
    https://gitlab.freedesktop.org/xorg/xserver/-/issues/1156

diff --git a/hw/xwayland/xwayland-cursor.c b/hw/xwayland/xwayland-cursor.c
index fac8840e6..c4457cc2a 100644
--- a/hw/xwayland/xwayland-cursor.c
+++ b/hw/xwayland/xwayland-cursor.c
@@ -162,8 +162,15 @@ static void
 xwl_cursor_attach_pixmap(struct xwl_seat *xwl_seat,
                          struct xwl_cursor *xwl_cursor, PixmapPtr pixmap)
 {
-    wl_surface_attach(xwl_cursor->surface,
-                      xwl_shm_pixmap_get_wl_buffer(pixmap), 0, 0);
+    struct wl_buffer *buffer;
+
+    buffer = xwl_shm_pixmap_get_wl_buffer(pixmap);
+    if (!buffer) {
+        ErrorF("cursor: Error getting buffer\n");
+        return;
+    }
+
+    wl_surface_attach(xwl_cursor->surface, buffer, 0, 0);
     xwl_surface_damage(xwl_seat->xwl_screen, xwl_cursor->surface, 0, 0,
                        xwl_seat->x_cursor->bits->width,
                        xwl_seat->x_cursor->bits->height);
diff --git a/hw/xwayland/xwayland-present.c b/hw/xwayland/xwayland-present.c
index 7ba7efc11..83d67517a 100644
--- a/hw/xwayland/xwayland-present.c
+++ b/hw/xwayland/xwayland-present.c
@@ -443,6 +443,12 @@ xwl_present_flip(WindowPtr present_window,
     if (!xwl_window)
         return FALSE;
 
+    buffer = xwl_glamor_pixmap_get_wl_buffer(pixmap);
+    if (!buffer) {
+        ErrorF("present: Error getting buffer\n");
+        return FALSE;
+    }
+
     damage_box = RegionExtents(damage);
 
     event = malloc(sizeof *event);
@@ -450,7 +456,6 @@ xwl_present_flip(WindowPtr present_window,
         return FALSE;
 
     pixmap->refcnt++;
-    buffer = xwl_glamor_pixmap_get_wl_buffer(pixmap);
 
     event->event_id = event_id;
     event->xwl_present_window = xwl_present_window;
diff --git a/hw/xwayland/xwayland-window.c b/hw/xwayland/xwayland-window.c
index d0c7c581d..af4290ec7 100644
--- a/hw/xwayland/xwayland-window.c
+++ b/hw/xwayland/xwayland-window.c
@@ -805,6 +805,11 @@ xwl_window_post_damage(struct xwl_window *xwl_window)
 #endif
         buffer = xwl_shm_pixmap_get_wl_buffer(pixmap);
 
+    if (!buffer) {
+        ErrorF("Error getting buffer\n");
+        return;
+    }
+
 #ifdef XWL_HAS_GLAMOR
     if (xwl_screen->glamor)
         xwl_glamor_post_damage(xwl_window, pixmap, region);
commit 4f0889e98306d30a37aba0fadb1fd3790c13205a
Author: Olivier Fourdan <ofourdan at redhat.com>
Date:   Mon Mar 29 14:22:56 2021 +0200

    xwayland/eglstream: Check buffer creation
    
    EGLStream wl_eglstream_display_create_stream() may fail, yet Xwayland
    would try to attach the buffer which may cause a fatal Wayland protocol
    error raised by the compositor.
    
    Check if the buffer creation worked, and fail gracefully otherwise (like
    wayland-eglsurface does).
    
    Signed-off-by: Olivier Fourdan <ofourdan at redhat.com>
    Reviewed-by: Martin Peres <martin.peres at mupuf.org>
    Reviewed-by: Michel Dänzer <mdaenzer at redhat.com>
    https://gitlab.freedesktop.org/xorg/xserver/-/issues/1156

diff --git a/hw/xwayland/xwayland-glamor-eglstream.c b/hw/xwayland/xwayland-glamor-eglstream.c
index 17295f3bd..c6e17bf8b 100644
--- a/hw/xwayland/xwayland-glamor-eglstream.c
+++ b/hw/xwayland/xwayland-glamor-eglstream.c
@@ -548,6 +548,10 @@ xwl_eglstream_create_pending_stream(struct xwl_screen *xwl_screen,
                                            stream_fd,
                                            WL_EGLSTREAM_HANDLE_TYPE_FD,
                                            &stream_attribs);
+    if (!xwl_pixmap->buffer) {
+        ErrorF("eglstream: Failed to create buffer\n");
+        goto fail;
+    }
 
     wl_buffer_add_listener(xwl_pixmap->buffer,
                            &xwl_eglstream_buffer_release_listener,
@@ -562,7 +566,9 @@ xwl_eglstream_create_pending_stream(struct xwl_screen *xwl_screen,
 
     xwl_eglstream_queue_pending_stream(xwl_screen, window, pixmap);
 
-    close(stream_fd);
+fail:
+    if (stream_fd >= 0)
+        close(stream_fd);
 }
 
 static Bool


More information about the xorg-commit mailing list