From 1a88e3d0c51287e72fdfdddd529670f01858acd8 Mon Sep 17 00:00:00 2001 From: Richard Grenville Date: Mon, 18 Mar 2013 11:48:28 +0800 Subject: [PATCH] Bug fix: GLX: ARGB texture too dark & Jitter when resize & others - GLX backend: Fix a bug that ARGB windows / shadows are rendered too dark. Thanks to derhass in FreeNode/##opengl for help. - GLX backend: Fix a problem that during window resize the content looks jittering, by letting compton fetch pixmap sizes with XGetGeometry() instead of relying on window width/height, which could be inaccurate during window resize. Negative effect on performance. Thanks to M4he for reporting. (#7) - Add .desktop file. Thanks to quequotion for providing it. (#97) - Avoid checking presence of window pixmap, because they may not exist with very old X Composite implementations. - Add workaround for a strange window restack issue when compton receieves a ConfigureNotify with non-existent new above window. - Add debugging function hexdump(). Extra sanity checks on various places. --- .gitignore | 1 + compton.desktop | 10 ++++++++ src/common.h | 44 +++++++++++++++++++++++++++++++---- src/compton.c | 36 +++++++++++++++++++++-------- src/compton.h | 18 +++++++++------ src/opengl.c | 61 ++++++++++++++++++++++++++++++++----------------- 6 files changed, 128 insertions(+), 42 deletions(-) create mode 100644 compton.desktop diff --git a/.gitignore b/.gitignore index ef61fd7..a63a74a 100644 --- a/.gitignore +++ b/.gitignore @@ -43,3 +43,4 @@ man/*.1 doxygen/ .clang_complete /src/backtrace-symbols.[ch] +/compton*.trace diff --git a/compton.desktop b/compton.desktop new file mode 100644 index 0000000..c6b2d7b --- /dev/null +++ b/compton.desktop @@ -0,0 +1,10 @@ +[Desktop Entry] +Version=1.0 +Type=Application +Name=compton +Comment=A X compositor +Categories=Utility; +TryExec=compton +Exec=compton +Icon=xcompmgr +# Thanks to quequotion for providing this file! diff --git a/src/common.h b/src/common.h index 77aa582..3b66208 100644 --- a/src/common.h +++ b/src/common.h @@ -297,9 +297,9 @@ struct _glx_texture { GLuint texture; GLXPixmap glpixmap; Pixmap pixmap; - int width; - int height; - int depth; + unsigned width; + unsigned height; + unsigned depth; bool y_inverted; }; #endif @@ -1573,7 +1573,7 @@ glx_init_blur(session_t *ps); bool glx_bind_pixmap(session_t *ps, glx_texture_t **pptex, Pixmap pixmap, - int width, int height, int depth); + unsigned width, unsigned height, unsigned depth); void glx_release_pixmap(session_t *ps, glx_texture_t *ptex); @@ -1686,3 +1686,39 @@ c2_match(session_t *ps, win *w, const c2_lptr_t *condlst, ///@} #endif + +/** + * @brief Dump raw bytes in HEX format. + * + * @param data pointer to raw data + * @param len length of data + */ +static inline void +hexdump(const char *data, int len) { + static const int BYTE_PER_LN = 16; + + if (len <= 0) + return; + + // Print header + printf("%10s:", "Offset"); + for (int i = 0; i < BYTE_PER_LN; ++i) + printf(" %2d", i); + putchar('\n'); + + // Dump content + for (int offset = 0; offset < len; ++offset) { + if (!(offset % BYTE_PER_LN)) + printf("0x%08x:", offset); + + printf(" %02hhx", data[offset]); + + if ((BYTE_PER_LN - 1) == offset % BYTE_PER_LN) + putchar('\n'); + } + if (len % BYTE_PER_LN) + putchar('\n'); + + fflush(stdout); +} + diff --git a/src/compton.c b/src/compton.c index 591fc70..539f8d2 100644 --- a/src/compton.c +++ b/src/compton.c @@ -474,6 +474,7 @@ win_build_shadow(session_t *ps, win *w, double opacity) { w->shadow_paint.pixmap = shadow_pixmap_argb; w->shadow_paint.pict = shadow_picture_argb; + bool success = paint_bind_tex(ps, &w->shadow_paint, shadow_image->width, shadow_image->height, 32, true); XFreeGC(ps->dpy, gc); @@ -863,8 +864,7 @@ get_root_tile(session_t *ps) { ps->root_tile_paint.pixmap = pixmap; #ifdef CONFIG_VSYNC_OPENGL if (BKEND_GLX == ps->o.backend) - return glx_bind_pixmap(ps, &ps->root_tile_paint.ptex, ps->root_tile_paint.pixmap, - ps->root_width, ps->root_height, ps->depth); + return glx_bind_pixmap(ps, &ps->root_tile_paint.ptex, ps->root_tile_paint.pixmap, 0, 0, 0); #endif return true; @@ -1437,8 +1437,10 @@ win_paint_win(session_t *ps, win *w, XserverRegion reg_paint) { } } // GLX: Build texture - if (!paint_bind_tex(ps, &w->paint, w->widthb, w->heightb, - w->pictfmt->depth, w->pixmap_damaged)) { + // Let glx_bind_pixmap() determine pixmap size, because if the user + // is resizing windows, the width and height we get may not be up-to-date, + // causing the jittering issue M4he reported in #7. + if (!paint_bind_tex(ps, &w->paint, 0, 0, 0, w->pixmap_damaged)) { printf_errf("(%#010lx): Failed to bind texture. Expect troubles.", w->id); } w->pixmap_damaged = false; @@ -2633,21 +2635,33 @@ restack_win(session_t *ps, win *w, Window new_above) { } if (old_above != new_above) { - win **prev; + win **prev = NULL, **prev_old = NULL; - /* unhook */ + // unhook for (prev = &ps->list; *prev; prev = &(*prev)->next) { if ((*prev) == w) break; } - *prev = w->next; + prev_old = prev; - /* rehook */ + bool found = false; + + // rehook for (prev = &ps->list; *prev; prev = &(*prev)->next) { - if ((*prev)->id == new_above && !(*prev)->destroyed) + if ((*prev)->id == new_above && !(*prev)->destroyed) { + found = true; break; + } } + if (!found) { + printf_errf("(%#010lx, %#010lx): " + "Failed to found new above window.", w->id, new_above); + return; + } + + *prev_old = w->next; + w->next = *prev; *prev = w; @@ -2668,7 +2682,9 @@ restack_win(session_t *ps, win *w, Window new_above) { desc = ""; if (c->destroyed) desc = "(D) "; - printf("%#010lx \"%s\" %s-> ", c->id, window_name, desc); + printf("%#010lx \"%s\" %s", c->id, window_name, desc); + if (c->next) + printf("-> "); if (to_free) { XFree(window_name); diff --git a/src/compton.h b/src/compton.h index 9c4bd0a..c16c596 100644 --- a/src/compton.h +++ b/src/compton.h @@ -162,7 +162,9 @@ free_wincondlst(c2_lptr_t **pcondlst) { */ static inline bool paint_isvalid(session_t *ps, const paint_t *ppaint) { - if (!ppaint || !ppaint->pixmap) + // Don't check for presence of Pixmap here, because older X Composite doesn't + // provide it + if (!ppaint) return false; if (BKEND_XRENDER == ps->o.backend && !ppaint->pict) @@ -179,13 +181,15 @@ paint_isvalid(session_t *ps, const paint_t *ppaint) { * Bind texture in paint_t if we are using GLX backend. */ static inline bool -paint_bind_tex(session_t *ps, paint_t *ppaint, int wid, int hei, int depth, - bool force) { +paint_bind_tex(session_t *ps, paint_t *ppaint, + unsigned wid, unsigned hei, unsigned depth, bool force) { #ifdef CONFIG_VSYNC_OPENGL - // TODO: Make sure we have the same Pixmap binded? - if (BKEND_GLX == ps->o.backend - && (force || !glx_tex_binded(ppaint->ptex, ppaint->pixmap))) { - return glx_bind_pixmap(ps, &ppaint->ptex, ppaint->pixmap, wid, hei, depth); + if (BKEND_GLX == ps->o.backend) { + if (!ppaint->pixmap) + return false; + + if (force || !glx_tex_binded(ppaint->ptex, ppaint->pixmap)) + return glx_bind_pixmap(ps, &ppaint->ptex, ppaint->pixmap, wid, hei, depth); } #endif diff --git a/src/opengl.c b/src/opengl.c index b32368e..21b3c18 100644 --- a/src/opengl.c +++ b/src/opengl.c @@ -107,7 +107,6 @@ glx_init(session_t *ps, bool need_render) { // glEnable(GL_DEPTH_TEST); glTexEnvi(GL_TEXTURE_ENV, GL_TEXTURE_ENV_MODE, GL_REPLACE); - glBlendFunc(GL_SRC_ALPHA, GL_ONE_MINUS_SRC_ALPHA); glDisable(GL_BLEND); if (!ps->o.glx_no_stencil) { @@ -322,15 +321,10 @@ glx_cmp_fbconfig(session_t *ps, */ bool glx_bind_pixmap(session_t *ps, glx_texture_t **pptex, Pixmap pixmap, - int width, int height, int depth) { - if (depth > OPENGL_MAX_DEPTH) { - printf_errf("(%d): Requested depth higher than %d.", depth, - OPENGL_MAX_DEPTH); - return false; - } - const glx_fbconfig_t *pcfg = ps->glx_fbconfigs[depth]; - if (!pcfg) { - printf_errf("(%d): Couldn't find FBConfig with requested depth.", depth); + unsigned width, unsigned height, unsigned depth) { + if (!pixmap) { + printf_errf("(%#010lx): Binding to an empty pixmap. This can't work.", + pixmap); return false; } @@ -359,8 +353,7 @@ glx_bind_pixmap(session_t *ps, glx_texture_t **pptex, Pixmap pixmap, glEnable(target); // Release pixmap if parameters are inconsistent - if (ptex->texture && !(width == ptex->width && height == ptex->height - && ptex->pixmap == pixmap && depth == ptex->depth)) { + if (ptex->texture && ptex->pixmap != pixmap) { glx_release_pixmap(ps, ptex); } @@ -380,7 +373,6 @@ glx_bind_pixmap(session_t *ps, glx_texture_t **pptex, Pixmap pixmap, glBindTexture(target, 0); ptex->texture = texture; - ptex->y_inverted = pcfg->y_inverted; } if (!ptex->texture) { printf_errf("(): Failed to allocate texture."); @@ -393,6 +385,29 @@ glx_bind_pixmap(session_t *ps, glx_texture_t **pptex, Pixmap pixmap, if (!ptex->glpixmap) { need_release = false; + // Retrieve pixmap parameters, if they aren't provided + if (!(width && height && depth)) { + Window rroot = None; + int rx = 0, ry = 0; + unsigned rbdwid = 0; + if (!XGetGeometry(ps->dpy, pixmap, &rroot, &rx, &ry, + &width, &height, &rbdwid, &depth)) { + printf_errf("(%#010lx): Failed to query Pixmap info.", pixmap); + return false; + } + if (depth > OPENGL_MAX_DEPTH) { + printf_errf("(%d): Requested depth higher than %d.", depth, + OPENGL_MAX_DEPTH); + return false; + } + } + + const glx_fbconfig_t *pcfg = ps->glx_fbconfigs[depth]; + if (!pcfg) { + printf_errf("(%d): Couldn't find FBConfig with requested depth.", depth); + return false; + } + // Determine texture target, copied from compiz GLint tex_tgt = 0; if (GLX_TEXTURE_2D_BIT_EXT & pcfg->texture_tgts @@ -419,6 +434,11 @@ glx_bind_pixmap(session_t *ps, glx_texture_t **pptex, Pixmap pixmap, }; ptex->glpixmap = glXCreatePixmap(ps->dpy, pcfg->cfg, pixmap, attrs); + ptex->pixmap = pixmap; + ptex->width = width; + ptex->height = height; + ptex->depth = depth; + ptex->y_inverted = pcfg->y_inverted; } if (!ptex->glpixmap) { printf_errf("(): Failed to allocate GLX pixmap."); @@ -436,11 +456,6 @@ glx_bind_pixmap(session_t *ps, glx_texture_t **pptex, Pixmap pixmap, glBindTexture(target, 0); glDisable(target); - ptex->width = width; - ptex->height = height; - ptex->depth = depth; - ptex->pixmap = pixmap; - return true; } @@ -493,7 +508,7 @@ glx_set_clip(session_t *ps, XserverRegion reg) { GLint z = 0; #ifdef DEBUG_GLX - printf_dbgf("(): Rect %d: %f, %f, %f, %f\n", i, rx, ry, rxe, rye); + printf_dbgf("(): Rect %d: %d, %d, %d, %d\n", i, rx, ry, rxe, rye); #endif glVertex3i(rx, ry, z); @@ -532,10 +547,14 @@ glx_render(session_t *ps, const glx_texture_t *ptex, if (opacity < 1.0 || GLX_TEXTURE_FORMAT_RGBA_EXT == ps->glx_fbconfigs[ptex->depth]->texture_fmt) { glEnable(GL_BLEND); - glBlendFunc(GL_SRC_ALPHA, GL_ONE_MINUS_SRC_ALPHA); + // Needed for handling opacity of ARGB texture glTexEnvi(GL_TEXTURE_ENV, GL_TEXTURE_ENV_MODE, GL_MODULATE); - glColor4f(1.0f, 1.0f, 1.0f, opacity); + + // This is all weird, but X Render is using a strange ARGB format, and + // we need to use those things to correct it. Thanks to derhass for help. + glBlendFunc(GL_ONE, GL_ONE_MINUS_SRC_ALPHA); + glColor4f(opacity, opacity, opacity, opacity); } // Color negation