Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix binding the root background pixmap in case of depth mismatch #984

Merged
merged 3 commits into from
Feb 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 31 additions & 2 deletions src/picom.c
Original file line number Diff line number Diff line change
Expand Up @@ -1158,14 +1158,43 @@
}
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(

Check warning on line 1161 in src/picom.c

View check run for this annotation

Codecov / codecov/patch

src/picom.c#L1161

Added line #L1161 was not covered by tests
ps->c.c, xcb_get_geometry(ps->c.c, pixmap), NULL);
if (!r) {
goto err;

Check warning on line 1164 in src/picom.c

View check run for this annotation

Codecov / codecov/patch

src/picom.c#L1163-L1164

Added lines #L1163 - L1164 were not covered by tests
}

// 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);

Check warning on line 1188 in src/picom.c

View check run for this annotation

Codecov / codecov/patch

src/picom.c#L1184-L1188

Added lines #L1184 - L1188 were not covered by tests

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:

Check warning on line 1197 in src/picom.c

View check run for this annotation

Codecov / codecov/patch

src/picom.c#L1197

Added line #L1197 was not covered by tests
log_error("Failed to bind root back pixmap");
}
}
Expand Down
20 changes: 13 additions & 7 deletions src/render.c
Original file line number Diff line number Diff line change
Expand Up @@ -604,28 +604,35 @@
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);

Check warning on line 609 in src/render.c

View check run for this annotation

Codecov / codecov/patch

src/render.c#L608-L609

Added lines #L608 - L609 were not covered by tests
}

// Create a pixmap if there isn't any
if (!pixmap) {
xcb_visualid_t visual;
if (!pixmap || !r) {

Check warning on line 614 in src/render.c

View check run for this annotation

Codecov / codecov/patch

src/render.c#L614

Added line #L614 was not covered by tests
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;

Check warning on line 621 in src/render.c

View check run for this annotation

Codecov / codecov/patch

src/render.c#L621

Added line #L621 was not covered by tests
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);

Check warning on line 627 in src/render.c

View check run for this annotation

Codecov / codecov/patch

src/render.c#L624-L627

Added lines #L624 - L627 were not covered by tests
}

// Create Picture
xcb_render_create_picture_value_list_t pa = {
.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) {
Expand All @@ -646,8 +653,7 @@
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);

Check warning on line 656 in src/render.c

View check run for this annotation

Codecov / codecov/patch

src/render.c#L656

Added line #L656 was not covered by tests
}
#endif

Expand Down
36 changes: 15 additions & 21 deletions src/x.c
Original file line number Diff line number Diff line change
Expand Up @@ -321,6 +321,21 @@
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)) {

Check warning on line 326 in src/x.c

View check run for this annotation

Codecov / codecov/patch

src/x.c#L324-L326

Added lines #L324 - L326 were not covered by tests
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;

Check warning on line 331 in src/x.c

View check run for this annotation

Codecov / codecov/patch

src/x.c#L328-L331

Added lines #L328 - L331 were not covered by tests
}
}
}

return XCB_NONE;

Check warning on line 336 in src/x.c

View check run for this annotation

Codecov / codecov/patch

src/x.c#L336

Added line #L336 was not covered by tests
}

xcb_render_pictformat_t
x_get_pictfmt_for_standard(struct x_connection *c, xcb_pict_standard_t std) {
x_get_server_pictfmts(c);
Expand Down Expand Up @@ -689,27 +704,6 @@
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
Expand Down
4 changes: 2 additions & 2 deletions src/x.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 <code>winprop_t</code>.
*
Expand Down Expand Up @@ -408,6 +406,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);

Expand Down
Loading