From 4401666cfb06f9d76a1bf109feda42730a6da9aa Mon Sep 17 00:00:00 2001 From: Maxim Solovyov Date: Thu, 1 Feb 2024 02:31:29 +0300 Subject: [PATCH 1/3] x: add the x_get_visual_for_depth function it returns the first found visual for the given depth --- src/x.c | 15 +++++++++++++++ src/x.h | 2 ++ 2 files changed, 17 insertions(+) diff --git a/src/x.c b/src/x.c index 06c8b939d6..c4de9113f9 100644 --- a/src/x.c +++ b/src/x.c @@ -321,6 +321,21 @@ xcb_visualid_t x_get_visual_for_standard(struct x_connection *c, xcb_pict_standa return x_get_visual_for_pictfmt(g_pictfmts, pictfmt->id); } +xcb_visualid_t x_get_visual_for_depth(struct x_connection *c, uint8_t depth) { + xcb_screen_iterator_t screen_it = xcb_setup_roots_iterator(xcb_get_setup(c->c)); + for (; screen_it.rem; xcb_screen_next(&screen_it)) { + xcb_depth_iterator_t depth_it = + xcb_screen_allowed_depths_iterator(screen_it.data); + for (; depth_it.rem; xcb_depth_next(&depth_it)) { + if (depth_it.data->depth == depth) { + return xcb_depth_visuals_iterator(depth_it.data).data->visual_id; + } + } + } + + return XCB_NONE; +} + xcb_render_pictformat_t x_get_pictfmt_for_standard(struct x_connection *c, xcb_pict_standard_t std) { x_get_server_pictfmts(c); diff --git a/src/x.h b/src/x.h index df45b5cca7..fe44313789 100644 --- a/src/x.h +++ b/src/x.h @@ -408,6 +408,8 @@ struct xvisual_info x_get_visual_info(struct x_connection *c, xcb_visualid_t vis xcb_visualid_t x_get_visual_for_standard(struct x_connection *c, xcb_pict_standard_t std); +xcb_visualid_t x_get_visual_for_depth(struct x_connection *c, uint8_t depth); + xcb_render_pictformat_t x_get_pictfmt_for_standard(struct x_connection *c, xcb_pict_standard_t std); From a655730e490a57f53bb38a740aa6dbaf8f872ecc Mon Sep 17 00:00:00 2001 From: Maxim Solovyov Date: Thu, 1 Feb 2024 06:25:38 +0300 Subject: [PATCH 2/3] picom: fix binding the root background pixmap in case of depth mismatch if the root background pixmap's depth doesn't match the root window's depth, find a suitable visual for the root background pixmap's depth and use it instead of the root window's visual --- src/picom.c | 33 +++++++++++++++++++++++++++++++-- 1 file changed, 31 insertions(+), 2 deletions(-) diff --git a/src/picom.c b/src/picom.c index feac750e4a..4e7a1b9af5 100644 --- a/src/picom.c +++ b/src/picom.c @@ -1158,14 +1158,43 @@ void root_damaged(session_t *ps) { } auto pixmap = x_get_root_back_pixmap(&ps->c, ps->atoms); if (pixmap != XCB_NONE) { + xcb_get_geometry_reply_t *r = xcb_get_geometry_reply( + ps->c.c, xcb_get_geometry(ps->c.c, pixmap), NULL); + if (!r) { + goto err; + } + + // We used to assume that pixmaps pointed by the root background + // pixmap atoms are owned by the root window and have the same + // depth and hence the same visual that we can use to bind them. + // However, some applications break this assumption, e.g. the + // Xfce's desktop manager xfdesktop that sets the _XROOTPMAP_ID + // atom to a pixmap owned by it that seems to always have 32 bpp + // depth when the common root window's depth is 24 bpp. So use the + // root window's visual only if the root background pixmap's depth + // matches the root window's depth. Otherwise, find a suitable + // visual for the root background pixmap's depth and use it. + // + // We can't obtain a suitable visual for the root background + // pixmap the same way as the win_bind_pixmap function because it + // requires a window and we have only a pixmap. We also can't not + // bind the root background pixmap in case of depth mismatch + // because some options rely on it's content, e.g. + // transparent-clipping. + xcb_visualid_t visual = + r->depth == ps->c.screen_info->root_depth + ? ps->c.screen_info->root_visual + : x_get_visual_for_depth(&ps->c, r->depth); + free(r); + ps->root_image = ps->backend_data->ops->bind_pixmap( - ps->backend_data, pixmap, - x_get_visual_info(&ps->c, ps->c.screen_info->root_visual), false); + ps->backend_data, pixmap, x_get_visual_info(&ps->c, visual), false); if (ps->root_image) { ps->backend_data->ops->set_image_property( ps->backend_data, IMAGE_PROPERTY_EFFECTIVE_SIZE, ps->root_image, (int[]){ps->root_width, ps->root_height}); } else { + err: log_error("Failed to bind root back pixmap"); } } From 4a79e7b7779297db9a622523953a6f207fe38cde Mon Sep 17 00:00:00 2001 From: Maxim Solovyov Date: Thu, 1 Feb 2024 07:13:16 +0300 Subject: [PATCH 3/3] render: fix binding the root background pixmap in case of depth mismatch fix the same issue in the legacy backends, see the previous commit for details. this commit also removes the x_validate_pixmap function because it was used only in the get_root_tile function and the fix pretty much implies and embeds it. --- src/render.c | 20 +++++++++++++------- src/x.c | 21 --------------------- src/x.h | 2 -- 3 files changed, 13 insertions(+), 30 deletions(-) diff --git a/src/render.c b/src/render.c index fadd833586..371a9be7d4 100644 --- a/src/render.c +++ b/src/render.c @@ -604,20 +604,27 @@ static bool get_root_tile(session_t *ps) { bool fill = false; xcb_pixmap_t pixmap = x_get_root_back_pixmap(&ps->c, ps->atoms); - // Make sure the pixmap we got is valid - if (pixmap && !x_validate_pixmap(&ps->c, pixmap)) { - pixmap = XCB_NONE; + xcb_get_geometry_reply_t *r; + if (pixmap) { + r = xcb_get_geometry_reply(ps->c.c, xcb_get_geometry(ps->c.c, pixmap), NULL); } // Create a pixmap if there isn't any - if (!pixmap) { + xcb_visualid_t visual; + if (!pixmap || !r) { pixmap = x_create_pixmap(&ps->c, (uint8_t)ps->c.screen_info->root_depth, 1, 1); if (pixmap == XCB_NONE) { log_error("Failed to create pixmaps for root tile."); return false; } + visual = ps->c.screen_info->root_visual; fill = true; + } else { + visual = r->depth == ps->c.screen_info->root_depth + ? ps->c.screen_info->root_visual + : x_get_visual_for_depth(&ps->c, r->depth); + free(r); } // Create Picture @@ -625,7 +632,7 @@ static bool get_root_tile(session_t *ps) { .repeat = true, }; ps->root_tile_paint.pict = x_create_picture_with_visual_and_pixmap( - &ps->c, ps->c.screen_info->root_visual, pixmap, XCB_RENDER_CP_REPEAT, &pa); + &ps->c, visual, pixmap, XCB_RENDER_CP_REPEAT, &pa); // Fill pixmap if needed if (fill) { @@ -646,8 +653,7 @@ static bool get_root_tile(session_t *ps) { ps->root_tile_paint.pixmap = pixmap; #ifdef CONFIG_OPENGL if (BKEND_GLX == ps->o.backend) { - return paint_bind_tex(ps, &ps->root_tile_paint, 0, 0, true, 0, - ps->c.screen_info->root_visual, false); + return paint_bind_tex(ps, &ps->root_tile_paint, 0, 0, true, 0, visual, false); } #endif diff --git a/src/x.c b/src/x.c index c4de9113f9..d48ae96ba7 100644 --- a/src/x.c +++ b/src/x.c @@ -704,27 +704,6 @@ xcb_pixmap_t x_create_pixmap(struct x_connection *c, uint8_t depth, int width, i return XCB_NONE; } -/** - * Validate a pixmap. - * - * Detect whether the pixmap is valid with XGetGeometry. Well, maybe there - * are better ways. - */ -bool x_validate_pixmap(struct x_connection *c, xcb_pixmap_t pixmap) { - if (pixmap == XCB_NONE) { - return false; - } - - auto r = xcb_get_geometry_reply(c->c, xcb_get_geometry(c->c, pixmap), NULL); - if (!r) { - return false; - } - - bool ret = r->width && r->height; - free(r); - return ret; -} - /// We don't use the _XSETROOT_ID root window property as a source of the background /// pixmap because it most likely points to a dummy pixmap used to keep the colormap /// associated with the background pixmap alive but we listen for it's changes and update diff --git a/src/x.h b/src/x.h index fe44313789..f320e8696e 100644 --- a/src/x.h +++ b/src/x.h @@ -356,8 +356,6 @@ const char *x_strerror(xcb_generic_error_t *e); xcb_pixmap_t x_create_pixmap(struct x_connection *, uint8_t depth, int width, int height); -bool x_validate_pixmap(struct x_connection *, xcb_pixmap_t pxmap); - /** * Free a winprop_t. *