From dd240d0576c3287eeb5851698dafd5fce9974ef9 Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Tue, 12 Mar 2019 00:29:59 +0000 Subject: [PATCH] Deprecate --glx-swap-method Setting glx-swap-method to value other than "undefined" and "buffer-age" could potentially cause rendering problems. So remove them, the meaning of the remaining options can be more precisely captured by "use-damage", so create a new option under that name. --glx-swap-method is deprecated in favor of the new option --use-damage. Signed-off-by: Yuxuan Shui --- compton.sample.conf | 2 +- man/compton.1.asciidoc | 6 +++--- src/config.h | 49 ++---------------------------------------- src/config_libconfig.c | 20 +++++++++++++---- src/dbus.c | 2 +- src/opengl.c | 5 ----- src/options.c | 36 +++++++++++++++++++++---------- src/render.c | 14 +++++++----- 8 files changed, 57 insertions(+), 77 deletions(-) diff --git a/compton.sample.conf b/compton.sample.conf index cfceb45b..0ed48ab0 100644 --- a/compton.sample.conf +++ b/compton.sample.conf @@ -70,8 +70,8 @@ invert-color-include = [ ]; # GLX backend # glx-no-stencil = true; # glx-no-rebind-pixmap = true; -glx-swap-method = "undefined"; # xrender-sync-fence = true; +use-damage = true; # Window type settings wintypes: diff --git a/man/compton.1.asciidoc b/man/compton.1.asciidoc index 7428fcb0..ec1439ee 100644 --- a/man/compton.1.asciidoc +++ b/man/compton.1.asciidoc @@ -203,7 +203,7 @@ May also be one of the predefined kernels: `3x3box` (default), `5x5box`, `7x7box Exclude conditions for background blur. *--resize-damage* 'INTEGER':: - Resize damaged region by a specific number of pixels. A positive value enlarges it while a negative one shrinks it. If the value is positive, those additional pixels will not be actually painted to screen, only used in blur calculation, and such. (Due to technical limitations, with *--glx-swap-method*, those pixels will still be incorrectly painted to screen.) Primarily used to fix the line corruption issues of blur, in which case you should use the blur radius value here (e.g. with a 3x3 kernel, you should use *--resize-damage* 1, with a 5x5 one you use *--resize-damage* 2, and so on). May or may not work with `--glx-no-stencil`. Shrinking doesn't function correctly. + Resize damaged region by a specific number of pixels. A positive value enlarges it while a negative one shrinks it. If the value is positive, those additional pixels will not be actually painted to screen, only used in blur calculation, and such. (Due to technical limitations, with *--use-damage*, those pixels will still be incorrectly painted to screen.) Primarily used to fix the line corruption issues of blur, in which case you should use the blur radius value here (e.g. with a 3x3 kernel, you should use *--resize-damage* 1, with a 5x5 one you use *--resize-damage* 2, and so on). May or may not work with `--glx-no-stencil`. Shrinking doesn't function correctly. *--invert-color-include* 'CONDITION':: Specify a list of conditions of windows that should be painted with inverted color. Resource-hogging, and is not well tested. @@ -232,8 +232,8 @@ May also be one of the predefined kernels: `3x3box` (default), `5x5box`, `7x7box *--glx-no-rebind-pixmap*:: GLX backend: Avoid rebinding pixmap on window damage. Probably could improve performance on rapid window content changes, but is known to break things on some drivers (LLVMpipe, xf86-video-intel, etc.). Recommended if it works. -*--glx-swap-method* undefined/exchange/copy/3/4/5/6/buffer-age:: - GLX backend: GLX buffer swap method we assume. Could be `undefined` (0), `copy` (1), `exchange` (2), 3-6, or `buffer-age` (-1). `undefined` is the slowest and the safest, and the default value. `copy` is fastest, but may fail on some drivers, 2-6 are gradually slower but safer (6 is still faster than 0). Usually, double buffer means 2, triple buffer means 3. `buffer-age` means auto-detect using 'GLX_EXT_buffer_age', supported by some drivers. Partially breaks `--resize-damage`. Defaults to `undefined`. +*-use-damage*:: + Use the damage information to limit rendering to parts of the screen that has actually changed. Potentially improves the performance. *--xrender-sync-fence*:: Use X Sync fence to sync clients' draw calls, to make sure all draw calls are finished before compton starts drawing. Needed on nvidia-drivers with GLX backend for some users. diff --git a/src/config.h b/src/config.h index e818e3c5..4c88ad0b 100644 --- a/src/config.h +++ b/src/config.h @@ -82,8 +82,6 @@ typedef struct options_t { bool glx_no_stencil; /// Whether to avoid rebinding pixmap on window damage. bool glx_no_rebind_pixmap; - /// GLX swap method we assume OpenGL uses. - int glx_swap_method; /// Custom fragment shader for painting windows, as a string. char *glx_fshader_win_str; /// Whether to detect rounded corners. @@ -131,6 +129,8 @@ typedef struct options_t { /// Whether to use glFinish() instead of glFlush() for (possibly) better /// VSync yet probably higher CPU usage. bool vsync_use_glfinish; + /// Whether use damage information to help limit the area to paint + bool use_damage; // === Shadow === /// Red, green and blue tone of the shadow. @@ -285,51 +285,6 @@ static inline attr_pure enum backend parse_backend(const char *str) { return NUM_BKEND; } -/** - * Parse a glx_swap_method option argument. - * - * Returns -2 on failure - */ -static inline attr_pure int parse_glx_swap_method(const char *str) { - // Parse alias - if (!strcmp("undefined", str)) { - return 0; - } - - if (!strcmp("copy", str)) { - return 1; - } - - if (!strcmp("exchange", str)) { - return 2; - } - - if (!strcmp("buffer-age", str)) { - return -1; - } - - // Parse number - char *pc = NULL; - int age = strtol(str, &pc, 0); - if (!pc || str == pc) { - log_error("glx-swap-method is an invalid number: %s", str); - return -2; - } - - for (; *pc; ++pc) - if (!isspace(*pc)) { - log_error("Trailing characters in glx-swap-method option: %s", str); - return -2; - } - - if (age < -1) { - log_error("Number for glx-swap-method is too small: %s", str); - return -2; - } - - return age; -} - /** * Parse a VSync option argument. */ diff --git a/src/config_libconfig.c b/src/config_libconfig.c index 0c1c7733..5cbcba7d 100644 --- a/src/config_libconfig.c +++ b/src/config_libconfig.c @@ -380,12 +380,24 @@ char *parse_config_libconfig(options_t *opt, const char *config_file, bool *shad lcfg_lookup_bool(&cfg, "glx-no-rebind-pixmap", &opt->glx_no_rebind_pixmap); // --glx-swap-method if (config_lookup_string(&cfg, "glx-swap-method", &sval)) { - opt->glx_swap_method = parse_glx_swap_method(sval); - if (opt->glx_swap_method == -2) { - log_fatal("Cannot parse \"glx-swap-method\""); - goto err; + char *endptr; + long val = strtol(sval, &endptr, 10); + if (*endptr || !(*sval)) { + // sval is not a number, or an empty string + val = -1; } + if (strcmp(sval, "undefined") != 0 && val != 0) { + // If not undefined, we will use damage and buffer-age to limit + // the rendering area. + opt->use_damage = true; + } + log_warn("glx-swap-method has been deprecated since v6, your setting " + "\"%s\" should be %s.", + sval, + opt->use_damage ? "replaced by `use-damage = true`" : "removed"); } + // --use-damage + lcfg_lookup_bool(&cfg, "use-damage", &opt->use_damage); // --glx-use-gpushader4 if (config_lookup_bool(&cfg, "glx-use-gpushader4", &ival) && ival) { log_warn("glx-use-gpushader4 is deprecated since v6, please remove it " diff --git a/src/dbus.c b/src/dbus.c index a6602e3f..2cdf4659 100644 --- a/src/dbus.c +++ b/src/dbus.c @@ -1028,7 +1028,7 @@ static bool cdbus_process_opts_get(session_t *ps, DBusMessage *msg) { #ifdef CONFIG_OPENGL cdbus_m_opts_get_do(glx_no_stencil, cdbus_reply_bool); cdbus_m_opts_get_do(glx_no_rebind_pixmap, cdbus_reply_bool); - cdbus_m_opts_get_do(glx_swap_method, cdbus_reply_int32); + cdbus_m_opts_get_do(use_damage, cdbus_reply_bool); #endif cdbus_m_opts_get_do(track_focus, cdbus_reply_bool); diff --git a/src/opengl.c b/src/opengl.c index 1ff59ac9..b29ba72c 100644 --- a/src/opengl.c +++ b/src/opengl.c @@ -53,11 +53,6 @@ bool glx_init(session_t *ps, bool need_render) { } } - if (ps->o.glx_swap_method > CGLX_MAX_BUFFER_AGE) { - log_error("glx-swap-method is too big"); - goto glx_init_end; - } - // Get XVisualInfo pvis = get_visualinfo_from_visual(ps, ps->vis); if (!pvis) { diff --git a/src/options.c b/src/options.c index 95098471..ff59e562 100644 --- a/src/options.c +++ b/src/options.c @@ -283,13 +283,9 @@ static void usage(int ret) { " known to break things on some drivers (LLVMpipe, xf86-video-intel,\n" " etc.).\n" "\n" - "--glx-swap-method undefined/copy/exchange/3/4/5/6/buffer-age\n" - " GLX backend: GLX buffer swap method we assume. Could be\n" - " undefined (0), copy (1), exchange (2), 3-6, or buffer-age (-1).\n" - " \"undefined\" is the slowest and the safest, and the default value.\n" - " 1 is fastest, but may fail on some drivers, 2-6 are gradually slower\n" - " but safer (6 is still faster than 0). -1 means auto-detect using\n" - " GLX_EXT_buffer_age, supported by some drivers. \n" + "--use-damage\n" + " Use the damage information to limit rendering to parts of the screen\n" + " that has actually changed. Potentially improves the performance.\n" "\n" "--xrender-sync-fence\n" " Additionally use X Sync fence to sync clients' draw calls. Needed\n" @@ -405,6 +401,7 @@ static const struct option longopts[] = { {"no-name-pixmap", no_argument, NULL, 320}, {"log-level", required_argument, NULL, 321}, {"log-file", required_argument, NULL, 322}, + {"use-damage", no_argument, NULL, 323}, {"experimental-backends", no_argument, NULL, 733}, {"monitor-repaint", no_argument, NULL, 800}, {"diagnostics", no_argument, NULL, 801}, @@ -676,12 +673,28 @@ void get_cfg(options_t *opt, int argc, char *const *argv, bool shadow_enable, opt->active_opacity = normalize_d(atof(optarg)); break; P_CASEBOOL(298, glx_no_rebind_pixmap); - case 299: + case 299: { // --glx-swap-method - opt->glx_swap_method = parse_glx_swap_method(optarg); - if (opt->glx_swap_method == -2) - exit(1); + char *endptr; + long tmpval = strtol(optarg, &endptr, 10); + bool should_remove = true; + if (*endptr || !(*optarg)) { + // optarg is not a number, or an empty string + tmpval = -1; + } + if (strcmp(optarg, "undefined") != 0 && tmpval != 0) { + // If not undefined, we will use damage and buffer-age to + // limit the rendering area. + opt->use_damage = true; + should_remove = false; + } + log_warn("--glx-swap-method has been deprecated, your setting " + "\"%s\" should be %s.", + optarg, + !should_remove ? "replaced by `--use-damage`" : + "removed"); break; + } case 300: // --fade-exclude condlst_add(&opt->fade_blacklist, optarg); @@ -749,6 +762,7 @@ void get_cfg(options_t *opt, int argc, char *const *argv, bool shadow_enable, break; } P_CASEBOOL(319, no_x_selection); + P_CASEBOOL(323, use_damage); P_CASEBOOL(733, experimental_backends); P_CASEBOOL(800, monitor_repaint); case 801: opt->print_diagnostics = true; break; diff --git a/src/render.c b/src/render.c index 9d93df2d..a371456e 100644 --- a/src/render.c +++ b/src/render.c @@ -96,7 +96,7 @@ static inline bool bkend_use_xrender(session_t *ps) { } int maximum_buffer_age(session_t *ps) { - if (bkend_use_glx(ps) && ps->o.glx_swap_method == SWAPM_BUFFER_AGE) { + if (bkend_use_glx(ps) && ps->o.use_damage) { return CGLX_MAX_BUFFER_AGE; } return 1; @@ -105,17 +105,21 @@ int maximum_buffer_age(session_t *ps) { static int get_buffer_age(session_t *ps) { #ifdef CONFIG_OPENGL if (bkend_use_glx(ps)) { - if (ps->o.glx_swap_method == SWAPM_BUFFER_AGE) { + if (!glxext.has_GLX_EXT_buffer_age && ps->o.use_damage) { + log_warn("GLX_EXT_buffer_age not supported by your driver," + "`use-damage` has to be disabled"); + ps->o.use_damage = false; + } + if (ps->o.use_damage) { unsigned int val; glXQueryDrawable(ps->dpy, get_tgt_window(ps), GLX_BACK_BUFFER_AGE_EXT, &val); return (int)val ?: -1; - } else { - return -1; } + return -1; } #endif - return 1; + return ps->o.use_damage ? 1 : -1; } /**