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 <yshuiv7@gmail.com>

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.
  */

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 "

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

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) {

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;

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;
 }
 
 /**