From 50e2259404344a5a3932b8e3873e1c047f31056b Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Sun, 28 Oct 2018 23:14:26 +0000 Subject: [PATCH] Remove xrender-sync and xrender-sync-fence This was a dubious "fix" for a Nvidia driver problem. The problem was never fully understood, and the then developers took a shotgun approach and implemented xsync fences as a fix. Which somehow fixed the problem. Again, I don't see any indication that the developers understood why this "fix" worked. (for details, see chjj/compton#152 and chjj/compton#181) The driver problem should have been fixed almost 5 years ago. So this shouldn't be needed anymore. In addition the way compton uses xsync fences is apparently wrong according to the xsync spec (fences are attached to screen, but compton uses them as if they were attached to drawables). So, I will try removing it and see if anyone will complain. If there are real concrete reasons why fences are needed, it will be brought back. Signed-off-by: Yuxuan Shui --- .gitignore | 3 ++ compton.sample.conf | 2 - man/compton.1.asciidoc | 8 +--- src/common.h | 95 ------------------------------------------ src/compton.c | 76 ++++----------------------------- src/compton.h | 1 - src/config_libconfig.c | 8 ++-- src/meson.build | 2 +- src/opengl.c | 19 --------- src/win.h | 7 ---- src/x.c | 11 ----- 11 files changed, 17 insertions(+), 215 deletions(-) diff --git a/.gitignore b/.gitignore index cab03277..caa3b22e 100644 --- a/.gitignore +++ b/.gitignore @@ -12,6 +12,8 @@ stamp-h1 compton *.o build/ +compile_commands.json +build.ninja # CMake files compton-*.deb @@ -45,3 +47,4 @@ doxygen/ .ycm_extra_conf.pyc /src/backtrace-symbols.[ch] /compton*.trace +*.orig diff --git a/compton.sample.conf b/compton.sample.conf index bc6ad51a..024a7aff 100644 --- a/compton.sample.conf +++ b/compton.sample.conf @@ -77,8 +77,6 @@ invert-color-include = [ ]; # glx-no-rebind-pixmap = true; glx-swap-method = "undefined"; # glx-use-gpushader4 = true; -# xrender-sync = true; -# xrender-sync-fence = true; # Window type settings wintypes: diff --git a/man/compton.1.asciidoc b/man/compton.1.asciidoc index dd896814..5dc2fd7b 100644 --- a/man/compton.1.asciidoc +++ b/man/compton.1.asciidoc @@ -242,7 +242,7 @@ May also be one of the predefined kernels: `3x3box` (default), `5x5box`, `7x7box + -- * `xrender` backend performs all rendering operations with X Render extension. It is what `xcompmgr` uses, and is generally a safe fallback when you encounter rendering artifacts or instability. -* `glx` (OpenGL) backend performs all rendering operations with OpenGL. It is more friendly to some VSync methods, and has significantly superior performance on color inversion (`--invert-color-include`) or blur (`--blur-background`). It requires proper OpenGL 2.0 support from your driver and hardware. You may wish to look at the GLX performance optimization options below. `--xrender-sync` and `--xrender-sync-fence` might be needed on some systems to avoid delay in changes of screen contents. +* `glx` (OpenGL) backend performs all rendering operations with OpenGL. It is more friendly to some VSync methods, and has significantly superior performance on color inversion (`--invert-color-include`) or blur (`--blur-background`). It requires proper OpenGL 2.0 support from your driver and hardware. You may wish to look at the GLX performance optimization options below. * `xr_glx_hybrid` backend renders the updated screen contents with X Render and presents it on the screen with GLX. It attempts to address the rendering issues some users encountered with GLX backend and enables the better VSync of GLX backends. `--vsync-use-glfinish` might fix some rendering issues with this backend. -- @@ -258,12 +258,6 @@ May also be one of the predefined kernels: `3x3box` (default), `5x5box`, `7x7box *--glx-use-gpushader4*:: GLX backend: Use 'GL_EXT_gpu_shader4' for some optimization on blur GLSL code. My tests on GTX 670 show no noticeable effect. -*--xrender-sync*:: - Attempt to synchronize client applications' draw calls with `XSync()`, used on GLX backend to ensure up-to-date window content is painted. - -*--xrender-sync-fence*:: - Additionally use X Sync fence to sync clients' draw calls. Needed on nvidia-drivers with GLX backend for some users. May be disabled at compile time with `NO_XSYNC=1`. - *--glx-fshader-win* 'SHADER':: GLX backend: Use specified GLSL fragment shader for rendering window contents. See `compton-default-fshader-win.glsl` and `compton-fake-transparency-fshader-win.glsl` in the source tree for examples. diff --git a/src/common.h b/src/common.h index 29d40fae..8c64401b 100644 --- a/src/common.h +++ b/src/common.h @@ -49,14 +49,6 @@ // #define CONFIG_OPENGL 1 // Whether to enable DBus support with libdbus. // #define CONFIG_DBUS 1 -// Whether to enable X Sync support. -// #define CONFIG_XSYNC 1 -// Whether to enable GLX Sync support. -// #define CONFIG_GLX_XSYNC 1 - -#if (!defined(CONFIG_XSYNC) || !defined(CONFIG_OPENGL)) && defined(CONFIG_GLX_SYNC) -#error Cannot enable GL sync without X Sync / OpenGL support. -#endif #ifndef COMPTON_VERSION #define COMPTON_VERSION "unknown" @@ -82,9 +74,6 @@ #include #include -#ifdef CONFIG_XSYNC -#include -#endif #include #include @@ -460,11 +449,6 @@ typedef struct options_t { char *display_repr; /// The backend in use. enum backend backend; - /// Whether to sync X drawing to avoid certain delay issues with - /// GLX backend. - bool xrender_sync; - /// Whether to sync X drawing with X Sync fence. - bool xrender_sync_fence; /// Whether to avoid using stencil buffer under GLX backend. Might be /// unsafe. bool glx_no_stencil; @@ -740,9 +724,6 @@ typedef struct session { xcb_render_picture_t tgt_picture; /// Temporary buffer to paint to before sending to display. paint_t tgt_buffer; -#ifdef CONFIG_XSYNC - XSyncFence tgt_buffer_fence; -#endif /// Window ID of the window we register as a symbol. Window reg_win; #ifdef CONFIG_OPENGL @@ -904,14 +885,6 @@ typedef struct session { region_t *xinerama_scr_regs; /// Number of Xinerama screens. int xinerama_nscrs; -#endif -#ifdef CONFIG_XSYNC - /// Whether X Sync extension exists. - bool xsync_exists; - /// Event base number for X Sync extension. - int xsync_event; - /// Error base number for X Sync extension. - int xsync_error; #endif /// Whether X Render convolution filter exists. bool xrfilter_convolution_exists; @@ -1491,20 +1464,6 @@ free_all_damage_last(session_t *ps) { pixman_region32_clear(&ps->all_damage_last[i]); } -#ifdef CONFIG_XSYNC -/** - * Free a XSync fence. - */ -static inline void -free_fence(session_t *ps, XSyncFence *pfence) { - if (*pfence) - XSyncDestroyFence(ps->dpy, *pfence); - *pfence = None; -} -#else -#define free_fence(ps, pfence) ((void) 0) -#endif - /** * Check if a rectangle includes the whole screen. */ @@ -1743,60 +1702,6 @@ glx_mark_frame(session_t *ps) { ///@} -#ifdef CONFIG_XSYNC -#define xr_sync(ps, d, pfence) xr_sync_(ps, d, pfence) -#else -#define xr_sync(ps, d, pfence) xr_sync_(ps, d) -#endif - -/** - * Synchronizes a X Render drawable to ensure all pending painting requests - * are completed. - */ -static inline void -xr_sync_(session_t *ps, Drawable d -#ifdef CONFIG_XSYNC - , XSyncFence *pfence -#endif - ) { - if (!ps->o.xrender_sync) - return; - - x_sync(ps->c); -#ifdef CONFIG_XSYNC - if (ps->o.xrender_sync_fence && ps->xsync_exists) { - // TODO: If everybody just follows the rules stated in X Sync prototype, - // we need only one fence per screen, but let's stay a bit cautious right - // now - XSyncFence tmp_fence = None; - if (!pfence) - pfence = &tmp_fence; - assert(pfence); - if (!*pfence) - *pfence = XSyncCreateFence(ps->dpy, d, False); - if (*pfence) { - Bool __attribute__((unused)) triggered = False; - /* if (XSyncQueryFence(ps->dpy, *pfence, &triggered) && triggered) - XSyncResetFence(ps->dpy, *pfence); */ - // The fence may fail to be created (e.g. because of died drawable) - assert(!XSyncQueryFence(ps->dpy, *pfence, &triggered) || !triggered); - XSyncTriggerFence(ps->dpy, *pfence); - XSyncAwaitFence(ps->dpy, pfence, 1); - assert(!XSyncQueryFence(ps->dpy, *pfence, &triggered) || triggered); - } - else { - printf_errf("(%#010lx): Failed to create X Sync fence.", d); - } - free_fence(ps, &tmp_fence); - if (*pfence) - XSyncResetFence(ps->dpy, *pfence); - } -#endif -#ifdef CONFIG_GLX_SYNC - xr_glx_sync(ps, d, pfence); -#endif -} - /** @name DBus handling */ ///@{ diff --git a/src/compton.c b/src/compton.c index 9b7dfaca..c9b3606f 100644 --- a/src/compton.c +++ b/src/compton.c @@ -701,9 +701,6 @@ win_build_shadow(session_t *ps, win *w, double opacity) { assert(!w->shadow_paint.pict); w->shadow_paint.pict = shadow_picture_argb; - // Sync it once and only once - xr_sync(ps, w->shadow_paint.pixmap, NULL); - xcb_free_gc(ps->c, gc); xcb_image_destroy(shadow_image); xcb_free_pixmap(ps->c, shadow_pixmap); @@ -1445,8 +1442,6 @@ paint_one(session_t *ps, win *w, const region_t *reg_paint) { w->paint.pixmap = xcb_generate_id(ps->c); set_ignore_cookie(ps, xcb_composite_name_window_pixmap(ps->c, w->id, w->paint.pixmap)); - if (w->paint.pixmap) - free_fence(ps, &w->fence); } Drawable draw = w->paint.pixmap; @@ -1463,9 +1458,6 @@ paint_one(session_t *ps, win *w, const region_t *reg_paint) { draw, XCB_RENDER_CP_SUBWINDOW_MODE, &pa); } - if (IsViewable == w->a.map_state) - xr_sync(ps, draw, &w->fence); - // GLX: Build texture // 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, @@ -1846,12 +1838,9 @@ paint_all(session_t *ps, region_t *region, const region_t *region_real, win * co glFlush(); glXWaitX(); assert(ps->tgt_buffer.pixmap); - xr_sync(ps, ps->tgt_buffer.pixmap, &ps->tgt_buffer_fence); paint_bind_tex(ps, &ps->tgt_buffer, ps->root_width, ps->root_height, ps->depth, !ps->o.glx_no_rebind_pixmap); - // See #163 - xr_sync(ps, ps->tgt_buffer.pixmap, &ps->tgt_buffer_fence); if (ps->o.vsync_use_glfinish) glFinish(); else @@ -2079,11 +2068,6 @@ unmap_win(session_t *ps, win **_w) { if (w->destroyed) return; - // One last synchronization - if (w->paint.pixmap) - xr_sync(ps, w->paint.pixmap, &w->fence); - free_fence(ps, &w->fence); - // Set focus out win_set_focused(ps, w, false); @@ -2545,16 +2529,6 @@ ev_name(session_t *ps, xcb_generic_event_t *ev) { if (ps->shape_exists && ev->response_type == ps->shape_event) return "ShapeNotify"; -#ifdef CONFIG_XSYNC - if (ps->xsync_exists) { - int o = ev->response_type - ps->xsync_event; - switch (o) { - CASESTRRET(XSyncCounterNotify); - CASESTRRET(XSyncAlarmNotify); - } - } -#endif - sprintf(buf, "Event %d", ev->response_type); return buf; @@ -3402,6 +3376,7 @@ usage(int ret) { "--backend backend\n" " Choose backend. Possible choices are xrender, glx, and\n" " xr_glx_hybrid" WARNING ".\n" +#undef WARNING "\n" "--glx-no-stencil\n" " GLX backend: Avoid using stencil buffer. Might cause issues\n" @@ -3426,20 +3401,6 @@ usage(int ret) { " GLX backend: Use GL_EXT_gpu_shader4 for some optimization on blur\n" " GLSL code. My tests on GTX 670 show no noticeable effect.\n" "\n" - "--xrender-sync\n" - " Attempt to synchronize client applications' draw calls with XSync(),\n" - " used on GLX backend to ensure up-to-date window content is painted.\n" -#undef WARNING -#ifndef CONFIG_XSYNC -#define WARNING WARNING_DISABLED -#else -#define WARNING -#endif - "\n" - "--xrender-sync-fence\n" - " Additionally use X Sync fence to sync clients' draw calls. Needed\n" - " on nvidia-drivers with GLX backend for some users." WARNING "\n" - "\n" "--glx-fshader-win shader\n" " GLX backend: Use specified GLSL fragment shader for rendering window\n" " contents.\n" @@ -3457,6 +3418,7 @@ usage(int ret) { "--dbus\n" " Enable remote control via D-Bus. See the D-BUS API section in the\n" " man page for more details." WARNING "\n" +#undef WARNING "\n" "--benchmark cycles\n" " Benchmark mode. Repeatedly paint until reaching the specified cycles.\n" @@ -3470,7 +3432,6 @@ usage(int ret) { ; FILE *f = (ret ? stderr: stdout); fputs(usage_text, f); -#undef WARNING #undef WARNING_DISABLED exit(ret); @@ -3996,8 +3957,12 @@ get_cfg(session_t *ps, int argc, char *const *argv, bool first_pass) { ps->o.write_pid_path = mstrcpy(optarg); break; P_CASEBOOL(311, vsync_use_glfinish); - P_CASEBOOL(312, xrender_sync); - P_CASEBOOL(313, xrender_sync_fence); + case 312: + printf_errf("(): --xrender-sync %s", deprecation_message); + break; + case 313: + printf_errf("(): --xrender-sync-fence %s", deprecation_message); + break; P_CASEBOOL(315, no_fading_destroyed_argb); P_CASEBOOL(316, force_win_blend); case 317: @@ -4051,9 +4016,6 @@ get_cfg(session_t *ps, int argc, char *const *argv, bool first_pass) { if (ps->o.blur_background_frame) ps->o.blur_background = true; - if (ps->o.xrender_sync_fence) - ps->o.xrender_sync = true; - // Other variables determined by options // Determine whether we need to track focus changes @@ -5164,27 +5126,6 @@ session_init(session_t *ps_old, int argc, char **argv) { ps->shape_exists = true; } - if (ps->o.xrender_sync_fence) { -#ifdef CONFIG_XSYNC - // Query X Sync - if (XSyncQueryExtension(ps->dpy, &ps->xsync_event, &ps->xsync_error)) { - // TODO: Fencing may require version >= 3.0? - int major_version_return = 0, minor_version_return = 0; - if (XSyncInitialize(ps->dpy, &major_version_return, &minor_version_return)) - ps->xsync_exists = true; - } - if (!ps->xsync_exists) { - printf_errf("(): X Sync extension not found. No X Sync fence sync is " - "possible."); - exit(1); - } -#else - printf_errf("(): X Sync support not compiled in. --xrender-sync-fence " - "can't work."); - exit(1); -#endif - } - // Query X RandR if ((ps->o.sw_opti && !ps->o.refresh_rate) || ps->o.xinerama_shadow_crop) { ext_info = xcb_get_extension_data(ps->c, &xcb_randr_id); @@ -5498,7 +5439,6 @@ session_destroy(session_t *ps) { ps->tgt_picture = None; else free_picture(ps, &ps->tgt_picture); - free_fence(ps, &ps->tgt_buffer_fence); free_picture(ps, &ps->root_picture); free_paint(ps, &ps->tgt_buffer); diff --git a/src/compton.h b/src/compton.h index 040e495a..f89b543b 100644 --- a/src/compton.h +++ b/src/compton.h @@ -209,7 +209,6 @@ free_paint(session_t *ps, paint_t *ppaint) { static inline void free_wpaint(session_t *ps, win *w) { free_paint(ps, &w->paint); - free_fence(ps, &w->fence); } /** diff --git a/src/config_libconfig.c b/src/config_libconfig.c index 2a6f4e11..13d612a2 100644 --- a/src/config_libconfig.c +++ b/src/config_libconfig.c @@ -355,10 +355,6 @@ parse_config(session_t *ps, struct options_tmp *pcfgtmp) { exit(1); // --glx-use-gpushader4 lcfg_lookup_bool(&cfg, "glx-use-gpushader4", &ps->o.glx_use_gpushader4); - // --xrender-sync - lcfg_lookup_bool(&cfg, "xrender-sync", &ps->o.xrender_sync); - // --xrender-sync-fence - lcfg_lookup_bool(&cfg, "xrender-sync-fence", &ps->o.xrender_sync_fence); if (lcfg_lookup_bool(&cfg, "clear-shadow", &bval)) printf_errf("(): \"clear-shadow\" is removed as an option, and is always" @@ -370,6 +366,10 @@ parse_config(session_t *ps, struct options_tmp *pcfgtmp) { printf_errf("(): \"glx-use-copysubbuffermesa\" %s", deprecation_message); if (lcfg_lookup_bool(&cfg, "glx-copy-from-front", &bval) && bval) printf_errf("(): \"glx-copy-from-front\" %s", deprecation_message); + if (lcfg_lookup_bool(&cfg, "xrender-sync", &bval) && bval) + printf_errf("(): \"xrender-sync\" %s", deprecation_message); + if (lcfg_lookup_bool(&cfg, "xrender-sync-fence", &bval) && bval) + printf_errf("(): \"xrender-sync-fence\" %s", deprecation_message); // Wintype settings diff --git a/src/meson.build b/src/meson.build index 79b55fbd..cd249399 100644 --- a/src/meson.build +++ b/src/meson.build @@ -1,6 +1,6 @@ deps = [ cc.find_library('m'), cc.find_library('ev') ] -cflags = [ '-DCONFIG_XSYNC' ] +cflags = [ ] srcs = ['compton.c', 'win.c', 'c2.c', 'x.c', 'config.c'] diff --git a/src/opengl.c b/src/opengl.c index 64abe5e7..fa8a9dd7 100644 --- a/src/opengl.c +++ b/src/opengl.c @@ -185,25 +185,6 @@ get_visualinfo_from_visual(session_t *ps, xcb_visualid_t visual) { return XGetVisualInfo(ps->dpy, VisualIDMask, &vreq, &nitems); } -#ifdef CONFIG_GLX_SYNC -void -xr_glx_sync(session_t *ps, Drawable d, XSyncFence *pfence) { - if (*pfence) { - // GLsync sync = ps->psglx->glFenceSyncProc(GL_SYNC_GPU_COMMANDS_COMPLETE, 0); - GLsync sync = ps->psglx->glImportSyncEXT(GL_SYNC_X11_FENCE_EXT, *pfence, 0); - /* GLenum ret = ps->psglx->glClientWaitSyncProc(sync, GL_SYNC_FLUSH_COMMANDS_BIT, - 1000); - assert(GL_CONDITION_SATISFIED == ret); */ - XSyncTriggerFence(ps->dpy, *pfence); - XFlush(ps->dpy); - ps->psglx->glWaitSyncProc(sync, 0, GL_TIMEOUT_IGNORED); - // ps->psglx->glDeleteSyncProc(sync); - // XSyncResetFence(ps->dpy, *pfence); - } - glx_check_err(ps); -} -#endif - #ifdef DEBUG_GLX_DEBUG_CONTEXT static inline GLXFBConfig get_fbconfig_from_visualinfo(session_t *ps, const XVisualInfo *visualinfo) { diff --git a/src/win.h b/src/win.h index e19af2cc..9b11b6b2 100644 --- a/src/win.h +++ b/src/win.h @@ -4,9 +4,6 @@ #pragma once #include #include -#ifdef CONFIG_XSYNC -#include -#endif #include // FIXME shouldn't need this @@ -104,10 +101,6 @@ struct win { winmode_t mode; /// Whether the window has been damaged at least once. bool ever_damaged; -#ifdef CONFIG_XSYNC - /// X Sync fence of drawable. - XSyncFence fence; -#endif /// Whether the window was damaged after last paint. bool pixmap_damaged; /// Damage of the window. diff --git a/src/x.c b/src/x.c index 2d34500c..7e8aa3d7 100644 --- a/src/x.c +++ b/src/x.c @@ -317,17 +317,6 @@ x_print_error(unsigned long serial, uint8_t major, uint8_t minor, uint8_t error_ } #endif -#ifdef CONFIG_XSYNC - if (ps->xsync_exists) { - o = error_code - ps->xsync_error; - switch (o) { - CASESTRRET2(XSyncBadCounter); - CASESTRRET2(XSyncBadAlarm); - CASESTRRET2(XSyncBadFence); - } - } -#endif - switch (error_code) { CASESTRRET2(BadAccess); CASESTRRET2(BadAlloc);