xwayland: clear pixmaps after creation in rootless mode

When a pixmap is created with a backing FBO, the FBO should be cleared
to avoid rendering uninitialized memory. This could happen when the
pixmap is rendered without being filled in its entirety.

One example is when a top-level window without a background is
resized. The pixmap would be reallocated to prepare for more pixels,
but uninitialized memory would be rendered in the resize offset until
the client sends a frame that fills these additional pixels.

Another example is when a new top-level window is created without a
background. Uninitialized memory would be rendered after the pixmap is
allocated and before the client sends its first frame.

This issue is only apparent in OpenGL implementations that don't zero
the VRAM of allocated buffers by default, such as RadeonSI.

Signed-off-by: Dor Askayo <dor.askayo@gmail.com>
Closes: https://gitlab.freedesktop.org/xorg/xserver/issues/636
Reviewed-by: Michel Dänzer <mdaenzer@redhat.com>

5 files changed, 49 insertions, 1 deletions

diff --git a/glamor/glamor.c b/glamor/glamor.c
index 4b935de59..4d6a8394e 100644
--- a/glamor/glamor.c
+++ b/glamor/glamor.c
@@ -129,6 +129,23 @@ glamor_set_pixmap_texture(PixmapPtr pixmap, unsigned int tex)
     return TRUE;
 }
 
+_X_EXPORT void
+glamor_clear_pixmap(PixmapPtr pixmap)
+{
+    ScreenPtr screen = pixmap->drawable.pScreen;
+    glamor_screen_private *glamor_priv;
+    glamor_pixmap_private *pixmap_priv;
+    const struct glamor_format *pixmap_format;
+
+    glamor_priv = glamor_get_screen_private(screen);
+    pixmap_priv = glamor_get_pixmap_private(pixmap);
+    pixmap_format = glamor_format_for_pixmap(pixmap);
+
+    assert(pixmap_priv->fbo != NULL);
+
+    glamor_pixmap_clear_fbo(glamor_priv, pixmap_priv->fbo, pixmap_format);
+}
+
 uint32_t
 glamor_get_pixmap_texture(PixmapPtr pixmap)
 {
@@ -781,6 +798,9 @@ glamor_init(ScreenPtr screen, unsigned int flags)
         epoxy_has_gl_extension("GL_NV_pack_subimage");
     glamor_priv->has_dual_blend =
         epoxy_has_gl_extension("GL_ARB_blend_func_extended");
+    glamor_priv->has_clear_texture =
+        epoxy_gl_version() >= 44 ||
+        epoxy_has_gl_extension("GL_ARB_clear_texture");
 
     glamor_priv->can_copyplane = (gl_version >= 30);
 
diff --git a/glamor/glamor.h b/glamor/glamor.h
index c972694e3..038d4d80d 100644
--- a/glamor/glamor.h
+++ b/glamor/glamor.h
@@ -115,6 +115,9 @@ extern _X_EXPORT Bool glamor_set_pixmap_texture(PixmapPtr pixmap,
 
 extern _X_EXPORT void glamor_set_pixmap_type(PixmapPtr pixmap,
                                              glamor_pixmap_type_t type);
+
+extern _X_EXPORT void glamor_clear_pixmap(PixmapPtr pixmap);
+
 extern _X_EXPORT void glamor_block_handler(ScreenPtr screen);
 
 extern _X_EXPORT PixmapPtr glamor_create_pixmap(ScreenPtr screen, int w, int h,
diff --git a/glamor/glamor_fbo.c b/glamor/glamor_fbo.c
index d8c54f471..4b3338b43 100644
--- a/glamor/glamor_fbo.c
+++ b/glamor/glamor_fbo.c
@@ -241,6 +241,24 @@ glamor_create_fbo_array(glamor_screen_private *glamor_priv,
     return NULL;
 }
 
+void
+glamor_pixmap_clear_fbo(glamor_screen_private *glamor_priv, glamor_pixmap_fbo *fbo,
+                        const struct glamor_format *pixmap_format)
+{
+    glamor_make_current(glamor_priv);
+
+    assert(fbo->fb != 0 && fbo->tex != 0);
+
+    if (glamor_priv->has_clear_texture) {
+        glClearTexImage(fbo->tex, 0, pixmap_format->format, pixmap_format->type, NULL);
+    }
+    else {
+        glDrawBuffer(GL_COLOR_ATTACHMENT0); /* assumes fbo->fb was attached as GL_COLOR_ATTACHMENT0 */
+        glClearColor(0.0, 0.0, 0.0, 0.0);
+        glClear(GL_COLOR_BUFFER_BIT);
+    }
+}
+
 glamor_pixmap_fbo *
 glamor_pixmap_detach_fbo(glamor_pixmap_private *pixmap_priv)
 {
diff --git a/glamor/glamor_priv.h b/glamor/glamor_priv.h
index ea7a8bc96..6ccc1c04c 100644
--- a/glamor/glamor_priv.h
+++ b/glamor/glamor_priv.h
@@ -209,6 +209,7 @@ typedef struct glamor_screen_private {
     Bool has_rw_pbo;
     Bool use_quads;
     Bool has_dual_blend;
+    Bool has_clear_texture;
     Bool has_texture_swizzle;
     Bool is_core_profile;
     Bool can_copyplane;
@@ -551,6 +552,8 @@ void glamor_destroy_fbo(glamor_screen_private *glamor_priv,
                         glamor_pixmap_fbo *fbo);
 void glamor_pixmap_destroy_fbo(PixmapPtr pixmap);
 Bool glamor_pixmap_fbo_fixup(ScreenPtr screen, PixmapPtr pixmap);
+void glamor_pixmap_clear_fbo(glamor_screen_private *glamor_priv, glamor_pixmap_fbo *fbo,
+                             const struct glamor_format *pixmap_format);
 
 const struct glamor_format *glamor_format_for_pixmap(PixmapPtr pixmap);
 
diff --git a/hw/xwayland/xwayland-glamor-gbm.c b/hw/xwayland/xwayland-glamor-gbm.c
index bfcf81dbc..9cbce5f55 100644
--- a/hw/xwayland/xwayland-glamor-gbm.c
+++ b/hw/xwayland/xwayland-glamor-gbm.c
@@ -238,8 +238,12 @@ xwl_glamor_gbm_create_pixmap(ScreenPtr screen,
         if (bo) {
             pixmap = xwl_glamor_gbm_create_pixmap_for_bo(screen, bo, depth);
 
-            if (!pixmap)
+            if (!pixmap) {
                 gbm_bo_destroy(bo);
+            }
+            else if (xwl_screen->rootless && hint == CREATE_PIXMAP_USAGE_BACKING_PIXMAP) {
+                glamor_clear_pixmap(pixmap);
+            }
         }
     }