From 53870fb7fe83f5ab54a0e37a553f3dd769e2fbbf Mon Sep 17 00:00:00 2001 From: Richard Grenville Date: Fri, 5 Apr 2013 21:05:19 +0800 Subject: [PATCH] Improvement: GLX: Cache region contents & --glx-no-rebind-pixmap - Cache region contents in is_region_empty(), mostly useful only for GLX backend to save one roundtrip to X. - GLX backend: Add --glx-no-rebind-pixmap, which prevents rebinding of GLX texture to pixmap on content change. It doesn't work on some drivers, but it saves some CPU on those where it does. - Wrap XFree() with a new function cxfree() since its man page claims NULL pointers are not acceptable (although in fact it does...). - Use macro to save some code in get_cfg(). Code clean-up. --- c2.c | 2 +- common.h | 23 +++++- compton.c | 213 ++++++++++++++++++++---------------------------------- compton.h | 23 ++++-- opengl.c | 35 +++++---- 5 files changed, 132 insertions(+), 164 deletions(-) diff --git a/c2.c b/c2.c index 4e2b0dfb3..13c194050 100644 --- a/c2.c +++ b/c2.c @@ -1183,7 +1183,7 @@ c2_match_once_leaf(session_t *ps, win *w, const c2_l_t *pleaf, // Free the string after usage, if necessary if (tgt_free) { if (C2_L_TATOM == pleaf->type) - XFree(tgt_free); + cxfree(tgt_free); else free(tgt_free); } diff --git a/common.h b/common.h index 2f5b831d1..eb57250b0 100644 --- a/common.h +++ b/common.h @@ -357,12 +357,15 @@ typedef struct { char *display; /// The backend in use. enum backend backend; - /// Whether to avoid using stencil buffer under GLX backend. Might be unsafe. + /// Whether to avoid using stencil buffer under GLX backend. Might be + /// unsafe. bool glx_no_stencil; /// Whether to copy unmodified regions from front buffer. bool glx_copy_from_front; /// Whether to use glXCopySubBufferMESA() to update screen. bool glx_use_copysubbuffermesa; + /// Whether to avoid rebinding pixmap on window damage. + bool glx_no_rebind_pixmap; /// Whether to try to detect WM windows and mark them as focused. bool mark_wmwin_focused; /// Whether to mark override-redirect windows as focused. @@ -1424,6 +1427,17 @@ fds_poll(session_t *ps, struct timeval *ptv) { } #undef CPY_FDS +/** + * Wrapper of XFree() for convenience. + * + * Because a NULL pointer cannot be passed to XFree(), its man page says. + */ +static inline void +cxfree(void *data) { + if (data) + XFree(data); +} + /** * Wrapper of XInternAtom() for convenience. */ @@ -1544,7 +1558,7 @@ wid_has_prop(const session_t *ps, Window w, Atom atom) { if (Success == XGetWindowProperty(ps->dpy, w, atom, 0, 0, False, AnyPropertyType, &type, &format, &nitems, &after, &data)) { - XFree(data); + cxfree(data); if (type) return true; } @@ -1598,7 +1612,7 @@ static inline void free_winprop(winprop_t *pprop) { // Empty the whole structure to avoid possible issues if (pprop->data.p8) { - XFree(pprop->data.p8); + cxfree(pprop->data.p8); pprop->data.p8 = NULL; } pprop->nitems = 0; @@ -1647,7 +1661,8 @@ glx_tex_binded(const glx_texture_t *ptex, Pixmap pixmap) { } void -glx_set_clip(session_t *ps, XserverRegion reg); +glx_set_clip(session_t *ps, XserverRegion reg, + const XRectangle * const cache_rects, const int cache_nrects); bool glx_blur_dst(session_t *ps, int dx, int dy, int width, int height, float z, diff --git a/compton.c b/compton.c index 981ee0c68..e86d74951 100644 --- a/compton.c +++ b/compton.c @@ -614,7 +614,7 @@ wid_get_prop_adv(const session_t *ps, Window w, Atom atom, long offset, }; } - XFree(data); + cxfree(data); return (winprop_t) { .data.p8 = NULL, @@ -658,12 +658,12 @@ win_rounded_corners(session_t *ps, win *w) { for (i = 0; i < nrects; ++i) if (rects[i].width >= minwidth && rects[i].height >= minheight) { w->rounded_corners = true; - XFree(rects); + cxfree(rects); return; } w->rounded_corners = false; - XFree(rects); + cxfree(rects); } /** @@ -762,7 +762,7 @@ find_toplevel2(session_t *ps, Window wid) { break; } - if (tchildren) XFree(tchildren); + cxfree(tchildren); wid = parent; } @@ -1026,7 +1026,7 @@ find_client_win(session_t *ps, Window w) { break; } - XFree(children); + cxfree(children); return ret; } @@ -1448,7 +1448,8 @@ win_paint_win(session_t *ps, win *w, XserverRegion reg_paint) { // 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)) { + if (!paint_bind_tex(ps, &w->paint, 0, 0, 0, + (!ps->o.glx_no_rebind_pixmap && w->pixmap_damaged))) { printf_errf("(%#010lx): Failed to bind texture. Expect troubles.", w->id); } w->pixmap_damaged = false; @@ -1665,7 +1666,7 @@ paint_all(session_t *ps, XserverRegion region, win *t) { reg_paint = region; } - set_tgt_clip(ps, reg_paint); + set_tgt_clip(ps, reg_paint, NULL, 0); paint_root(ps, reg_paint); // Create temporary regions for use during painting @@ -1716,10 +1717,16 @@ paint_all(session_t *ps, XserverRegion region, win *t) { XFixesSubtractRegion(ps->dpy, reg_paint, reg_paint, w->border_size); // Detect if the region is empty before painting - if (region == reg_paint || !is_region_empty(ps, reg_paint)) { - set_tgt_clip(ps, reg_paint); + { + int nrects = 0; + XRectangle *rects = NULL; + if (region == reg_paint + || !is_region_empty(ps, reg_paint, &rects, &nrects)) { + set_tgt_clip(ps, reg_paint, rects, nrects); - win_paint_shadow(ps, w, reg_paint); + win_paint_shadow(ps, w, reg_paint); + } + cxfree(rects); } } @@ -1743,16 +1750,21 @@ paint_all(session_t *ps, XserverRegion region, win *t) { reg_paint = region; } - if (!is_region_empty(ps, reg_paint)) { - set_tgt_clip(ps, reg_paint); - // Blur window background - if (w->blur_background && (WMODE_SOLID != w->mode - || (ps->o.blur_background_frame && w->frame_opacity))) { - win_blur_background(ps, w, ps->tgt_buffer, reg_paint); - } + { + int nrects = 0; + XRectangle *rects = NULL; + if (!is_region_empty(ps, reg_paint, &rects, &nrects)) { + set_tgt_clip(ps, reg_paint, rects, nrects); + // Blur window background + if (w->blur_background && (WMODE_SOLID != w->mode + || (ps->o.blur_background_frame && w->frame_opacity))) { + win_blur_background(ps, w, ps->tgt_buffer, reg_paint); + } - // Painting the window - win_paint_win(ps, w, reg_paint); + // Painting the window + win_paint_win(ps, w, reg_paint); + } + cxfree(rects); } } @@ -1762,7 +1774,7 @@ paint_all(session_t *ps, XserverRegion region, win *t) { // Do this as early as possible if (!ps->o.dbe) - set_tgt_clip(ps, None); + set_tgt_clip(ps, None, NULL, 0); if (ps->o.vsync) { // Make sure all previous requests are processed to achieve best @@ -2751,7 +2763,7 @@ restack_win(session_t *ps, win *w, Window new_above) { printf("-> "); if (to_free) { - XFree(window_name); + cxfree(window_name); window_name = NULL; } } @@ -3251,11 +3263,11 @@ wid_get_text_prop(session_t *ps, Window wid, Atom prop, *pnstr = 0; if (*pstrlst) XFreeStringList(*pstrlst); - XFree(text_prop.value); + cxfree(text_prop.value); return false; } - XFree(text_prop.value); + cxfree(text_prop.value); return true; } @@ -3281,10 +3293,10 @@ wid_get_name(session_t *ps, Window wid, char **name) { || !nstr || !strlst) { if (strlst) XFreeStringList(strlst); - XFree(text_prop.value); + cxfree(text_prop.value); return false; } - XFree(text_prop.value); + cxfree(text_prop.value); } *name = mstrcpy(strlst[0]); @@ -3766,8 +3778,7 @@ ev_property_notify(session_t *ps, XPropertyEvent *ev) { // Print out changed atom char *name = XGetAtomName(ps->dpy, ev->atom); printf_dbg(" { atom = %s }\n", name); - if (name) - XFree(name); + cxfree(name); } #endif @@ -3987,7 +3998,7 @@ ev_handle(session_t *ps, XEvent *ev) { ev_name(ps, ev), ev_serial(ev), wid, window_name); if (to_free) { - XFree(window_name); + cxfree(window_name); window_name = NULL; } } @@ -4228,6 +4239,10 @@ usage(void) { " My tests on nouveau shows a 200% performance boost when only 1/4 of\n" " the screen is updated. May break VSync and is not available on some\n" " drivers. Overrides --glx-copy-from-front.\n" + "--glx-no-rebind-pixmap\n" + " GLX backend: Avoid rebinding pixmap on window damage. Probably\n" + " could improve performance on rapid window content changes, but is\n" + " known to break things on some drivers.\n" #undef WARNING #ifndef CONFIG_DBUS #define WARNING WARNING_DISABLED @@ -4702,6 +4717,7 @@ get_cfg(session_t *ps, int argc, char *const *argv, bool first_pass) { { "glx-use-copysubbuffermesa", no_argument, NULL, 295 }, { "blur-background-exclude", required_argument, NULL, 296 }, { "active-opacity", required_argument, NULL, 297 }, + { "glx-no-rebind-pixmap", no_argument, NULL, 298 }, // Must terminate with a NULL entry { NULL, 0, NULL, 0 }, }; @@ -4759,6 +4775,7 @@ get_cfg(session_t *ps, int argc, char *const *argv, bool first_pass) { while (-1 != (o = getopt_long(argc, argv, shortopts, longopts, &longopt_idx))) { switch (o) { +#define P_CASEBOOL(idx, option) case idx: ps->o.option = true; break // Short options case 'h': usage(); @@ -4790,9 +4807,7 @@ get_cfg(session_t *ps, int argc, char *const *argv, bool first_pass) { case 'F': fading_enable = true; break; - case 'S': - ps->o.synchronize = true; - break; + P_CASEBOOL('S', synchronize); case 'r': ps->o.shadow_radius = atoi(optarg); break; @@ -4811,17 +4826,13 @@ get_cfg(session_t *ps, int argc, char *const *argv, bool first_pass) { case 'e': ps->o.frame_opacity = atof(optarg); break; - case 'z': - ps->o.clear_shadow = true; - break; + P_CASEBOOL('z', clear_shadow); case 'n': case 'a': case 's': printf_errfq(1, "(): -n, -a, and -s have been removed."); break; - case 'b': - ps->o.fork_after_register = true; - break; + P_CASEBOOL('b', fork_after_register); // Long options case 256: // --config @@ -4838,42 +4849,21 @@ get_cfg(session_t *ps, int argc, char *const *argv, bool first_pass) { // --shadow-blue ps->o.shadow_blue = atof(optarg); break; - case 260: - // --inactive-opacity-override - ps->o.inactive_opacity_override = true; - break; + P_CASEBOOL(260, inactive_opacity_override); case 261: // --inactive-dim ps->o.inactive_dim = atof(optarg); break; - case 262: - // --mark-wmwin-focused - ps->o.mark_wmwin_focused = true; - break; + P_CASEBOOL(262, mark_wmwin_focused); case 263: // --shadow-exclude condlst_add(ps, &ps->o.shadow_blacklist, optarg); break; - case 264: - // --mark-ovredir-focused - ps->o.mark_ovredir_focused = true; - break; - case 265: - // --no-fading-openclose - ps->o.no_fading_openclose = true; - break; - case 266: - // --shadow-ignore-shaped - ps->o.shadow_ignore_shaped = true; - break; - case 267: - // --detect-rounded-corners - ps->o.detect_rounded_corners = true; - break; - case 268: - // --detect-client-opacity - ps->o.detect_client_opacity = true; - break; + P_CASEBOOL(264, mark_ovredir_focused); + P_CASEBOOL(265, no_fading_openclose); + P_CASEBOOL(266, shadow_ignore_shaped); + P_CASEBOOL(267, detect_rounded_corners); + P_CASEBOOL(268, detect_client_opacity); case 269: // --refresh-rate ps->o.refresh_rate = atoi(optarg); @@ -4887,66 +4877,24 @@ get_cfg(session_t *ps, int argc, char *const *argv, bool first_pass) { // --alpha-step ps->o.alpha_step = atof(optarg); break; - case 272: - // --dbe - ps->o.dbe = true; - break; - case 273: - // --paint-on-overlay - ps->o.paint_on_overlay = true; - break; - case 274: - // --sw-opti - ps->o.sw_opti = true; - break; - case 275: - // --vsync-aggressive - ps->o.vsync_aggressive = true; - break; - case 276: - // --use-ewmh-active-win - ps->o.use_ewmh_active_win = true; - break; - case 277: - // --respect-prop-shadow - ps->o.respect_prop_shadow = true; - break; - case 278: - // --unredir-if-possible - ps->o.unredir_if_possible = true; - break; + P_CASEBOOL(272, dbe); + P_CASEBOOL(273, paint_on_overlay); + P_CASEBOOL(274, sw_opti); + P_CASEBOOL(275, vsync_aggressive); + P_CASEBOOL(276, use_ewmh_active_win); + P_CASEBOOL(277, respect_prop_shadow); + P_CASEBOOL(278, unredir_if_possible); case 279: // --focus-exclude condlst_add(ps, &ps->o.focus_blacklist, optarg); break; - case 280: - // --inactive-dim-fixed - ps->o.inactive_dim_fixed = true; - break; - case 281: - // --detect-transient - ps->o.detect_transient = true; - break; - case 282: - // --detect-client-leader - ps->o.detect_client_leader = true; - break; - case 283: - // --blur-background - ps->o.blur_background = true; - break; - case 284: - // --blur-background-frame - ps->o.blur_background_frame = true; - break; - case 285: - // --blur-background-fixed - ps->o.blur_background_fixed = true; - break; - case 286: - // --dbus - ps->o.dbus = true; - break; + P_CASEBOOL(280, inactive_dim_fixed); + P_CASEBOOL(281, detect_transient); + P_CASEBOOL(282, detect_client_leader); + P_CASEBOOL(283, blur_background); + P_CASEBOOL(284, blur_background_frame); + P_CASEBOOL(285, blur_background_fixed); + P_CASEBOOL(286, dbus); case 287: // --logpath ps->o.logpath = mstrcpy(optarg); @@ -4964,14 +4912,8 @@ get_cfg(session_t *ps, int argc, char *const *argv, bool first_pass) { if (!parse_backend(ps, optarg)) exit(1); break; - case 291: - // --glx-no-stencil - ps->o.glx_no_stencil = true; - break; - case 292: - // --glx-copy-from-front - ps->o.glx_copy_from_front = true; - break; + P_CASEBOOL(291, glx_no_stencil); + P_CASEBOOL(292, glx_copy_from_front); case 293: // --benchmark ps->o.benchmark = atoi(optarg); @@ -4980,10 +4922,7 @@ get_cfg(session_t *ps, int argc, char *const *argv, bool first_pass) { // --benchmark-wid ps->o.benchmark_wid = strtol(optarg, NULL, 0); break; - case 295: - // --glx-use-copysubbuffermesa - ps->o.glx_use_copysubbuffermesa = true; - break; + P_CASEBOOL(295, glx_use_copysubbuffermesa); case 296: // --blur-background-exclude condlst_add(ps, &ps->o.blur_background_blacklist, optarg); @@ -4992,9 +4931,11 @@ get_cfg(session_t *ps, int argc, char *const *argv, bool first_pass) { // --active-opacity ps->o.active_opacity = (normalize_d(atof(optarg)) * OPAQUE); break; + P_CASEBOOL(298, glx_no_rebind_pixmap); default: usage(); break; +#undef P_CASEBOOL } } @@ -5492,7 +5433,7 @@ init_filters(session_t *ps) { ps->xrfilter_convolution_exists = true; } } - XFree(pf); + cxfree(pf); // Turn features off if any required filter is not present if (!ps->xrfilter_convolution_exists) { @@ -6195,7 +6136,7 @@ session_init(session_t *ps_old, int argc, char **argv) { add_win(ps, children[i], i ? children[i-1] : None); } - XFree(children); + cxfree(children); } @@ -6445,7 +6386,7 @@ session_run(session_t *ps) { if (!ps->redirected) free_region(ps, &ps->all_damage); - if (ps->all_damage && !is_region_empty(ps, ps->all_damage)) { + if (ps->all_damage && !is_region_empty(ps, ps->all_damage, NULL, NULL)) { static int paint = 0; paint_all(ps, ps->all_damage, t); ps->reg_ignore_expire = false; diff --git a/compton.h b/compton.h index 77d3b6bc3..06bede6a2 100644 --- a/compton.h +++ b/compton.h @@ -269,8 +269,7 @@ make_text_prop(session_t *ps, char *str) { printf_errfq(1, "(): Failed to allocate memory."); if (XmbTextListToTextProperty(ps->dpy, &str, 1, XStringStyle, pprop)) { - if (pprop->value) - XFree(pprop->value); + cxfree(pprop->value); free(pprop); pprop = NULL; } @@ -547,14 +546,15 @@ win_render(session_t *ps, win *w, int x, int y, int wid, int hei, double opacity } static inline void -set_tgt_clip(session_t *ps, XserverRegion reg) { +set_tgt_clip(session_t *ps, XserverRegion reg, + const XRectangle * const cache_rects, const int cache_nrects) { switch (ps->o.backend) { case BKEND_XRENDER: XFixesSetPictureClipRegion(ps->dpy, ps->tgt_buffer, 0, 0, reg); break; #ifdef CONFIG_VSYNC_OPENGL case BKEND_GLX: - glx_set_clip(ps, reg); + glx_set_clip(ps, reg, cache_rects, cache_nrects); break; #endif } @@ -854,7 +854,7 @@ dump_region(const session_t *ps, XserverRegion region) { printf("Rect #%d: %8d, %8d, %8d, %8d\n", i, rects[i].x, rects[i].y, rects[i].width, rects[i].height); - XFree(rects); + cxfree(rects); } /** @@ -865,13 +865,22 @@ dump_region(const session_t *ps, XserverRegion region) { * * @param ps current session * @param region region to check for + * @param pcache_rects a place to cache the dumped rectangles + * @param ncache_nrects a place to cache the number of dumped rectangles */ static inline bool -is_region_empty(const session_t *ps, XserverRegion region) { +is_region_empty(const session_t *ps, XserverRegion region, + XRectangle **pcache_rects, int *pcache_nrects) { int nrects = 0; XRectangle *rects = XFixesFetchRegion(ps->dpy, region, &nrects); - XFree(rects); + if (pcache_rects) + *pcache_rects = rects; + else + cxfree(rects); + + if (pcache_nrects) + *pcache_nrects = nrects; return !nrects; } diff --git a/opengl.c b/opengl.c index 45f7b5f32..a660ab5c5 100644 --- a/opengl.c +++ b/opengl.c @@ -135,8 +135,7 @@ glx_init(session_t *ps, bool need_render) { success = true; glx_init_end: - if (pvis) - XFree(pvis); + cxfree(pvis); if (!success) glx_destroy(ps); @@ -324,8 +323,7 @@ glx_update_fbconfig(session_t *ps) { } } - if (pfbcfgs) - XFree(pfbcfgs); + cxfree(pfbcfgs); // Sanity checks if (!ps->glx_fbconfigs[ps->depth]) { @@ -564,7 +562,7 @@ glx_paint_pre(session_t *ps, XserverRegion *preg) { { XserverRegion reg_copy = XFixesCreateRegion(ps->dpy, NULL, 0); XFixesSubtractRegion(ps->dpy, reg_copy, ps->screen_reg, *preg); - glx_set_clip(ps, reg_copy); + glx_set_clip(ps, reg_copy, NULL, 0); free_region(ps, ®_copy); } @@ -579,14 +577,15 @@ glx_paint_pre(session_t *ps, XserverRegion *preg) { } } - glx_set_clip(ps, *preg); + glx_set_clip(ps, *preg, NULL, 0); } /** * Set clipping region on the target window. */ void -glx_set_clip(session_t *ps, XserverRegion reg) { +glx_set_clip(session_t *ps, XserverRegion reg, + const XRectangle * const cache_rects, const int cache_nrects) { // Quit if we aren't using stencils if (ps->o.glx_no_stencil) return; @@ -601,10 +600,16 @@ glx_set_clip(session_t *ps, XserverRegion reg) { if (!reg) return; - int nrects = 0; - XRectangle *rects = XFixesFetchRegion(ps->dpy, reg, &nrects); + int nrects = cache_nrects; + XRectangle *rects_free = NULL; + const XRectangle *rects = cache_rects; + if (!rects) { + nrects = 0; + rects = rects_free = XFixesFetchRegion(ps->dpy, reg, &nrects); + } if (!nrects) { - if (rects) XFree(rects); + cxfree(rects_free); + rects_free = NULL; nrects = 1; rects = &rect_blank; } @@ -649,8 +654,7 @@ glx_set_clip(session_t *ps, XserverRegion reg) { glDepthMask(GL_TRUE); } - if (rects && &rect_blank != rects) - XFree(rects); + cxfree(rects_free); } bool @@ -954,7 +958,7 @@ glx_render(session_t *ps, const glx_texture_t *ptex, glEnd(); if (rects && rects != &rec_all) - XFree(rects); + cxfree(rects); free_region(ps, ®_new); } @@ -989,7 +993,7 @@ glx_swap_copysubbuffermesa(session_t *ps, XserverRegion reg) { glXSwapBuffers(ps->dpy, get_tgt_window(ps)); } else { - glx_set_clip(ps, None); + glx_set_clip(ps, None, NULL, 0); for (int i = 0; i < nrects; ++i) { const int x = rects[i].x; const int y = ps->root_height - rects[i].y - rects[i].height; @@ -1003,8 +1007,7 @@ glx_swap_copysubbuffermesa(session_t *ps, XserverRegion reg) { } } - if (rects) - XFree(rects); + cxfree(rects); } #ifdef CONFIG_VSYNC_OPENGL_GLSL