vsync: choose vsync method automatically

Choose the best vsync method for the user, instead of asking them to
frustratingly try every one of the options to see what works.

With this commit, the `vsync` option will take only a boolean value.

Signed-off-by: Yuxuan Shui <yshuiv7@gmail.com>

compton.sample.conf

@@ -56,7 +56,7 @@ mark-ovredir-focused = true;
 detect-rounded-corners = true;
 detect-client-opacity = true;
 refresh-rate = 0;
-vsync = "none";
+vsync = true;
 # sw-opti = true;
 # unredir-if-possible = true;
 # unredir-if-possible-delay = 5000;

man/compton.1.asciidoc

@@ -130,19 +130,8 @@ OPTIONS
 *--refresh-rate* 'REFRESH_RATE'::
 	Specify refresh rate of the screen. If not specified or 0, compton will try detecting this with X RandR extension.
 
-*--vsync* 'VSYNC_METHOD'::
+*--vsync*::
-	Set VSync method. VSync methods currently available:
+	Enable VSync.
-+
---
-* 'none': No VSync
-* 'drm': VSync with 'DRM_IOCTL_WAIT_VBLANK'. May only work on some (DRI-based) drivers.
-* 'opengl': Try to VSync with 'SGI_video_sync' OpenGL extension. Only work on some drivers.
-* 'opengl-oml': Try to VSync with 'OML_sync_control' OpenGL extension. Only work on some drivers.
-* 'opengl-swc': Try to VSync with 'MESA_swap_control' or 'SGI_swap_control' (in order of preference) OpenGL extension. Works only with GLX backend. Known to be most effective on many drivers. Does not guarantee to control paint timing.
-* 'opengl-mswc': Deprecated, use 'opengl-swc' instead.
-
-(Note some VSync methods may not be enabled at compile time.)
---
 
 *--sw-opti*::
 	Limit compton to repaint at most once every 1 / 'refresh_rate' second to boost performance. This should not be used with *--vsync* drm/opengl/opengl-oml as they essentially does *--sw-opti*'s job already, unless you wish to specify a lower refresh rate than the actual value.

src/backend/xrender.c

@@ -503,7 +503,7 @@ backend_t *backend_xrender_init(session_t *ps) {
 		abort();
 	}
 
-	xd->vsync = ps->o.vsync != VSYNC_NONE;
+	xd->vsync = ps->o.vsync;
 	if (ps->present_exists) {
 		auto eid = xcb_generate_id(ps->c);
 		auto e =

src/common.h

@@ -526,6 +526,8 @@ typedef struct session {
 	// === DBus related ===
 	void *dbus_data;
 #endif
+
+	int (*vsync_wait)(session_t *);
 } session_t;
 
 /// Temporary structure used for communication between

src/compton.c

@@ -93,15 +93,6 @@ const char *const WINTYPES[NUM_WINTYPES] = {
     "popup_menu", "tooltip", "notify", "combo",   "dnd",
 };
 
-/// Names of VSync modes.
-const char *const VSYNC_STRS[NUM_VSYNC + 1] = {"none",               // VSYNC_NONE
-                                               "drm",                // VSYNC_DRM
-                                               "opengl",             // VSYNC_OPENGL
-                                               "opengl-oml",         // VSYNC_OPENGL_OML
-                                               "opengl-swc",         // VSYNC_OPENGL_SWC
-                                               "opengl-mswc",        // VSYNC_OPENGL_MSWC
-                                               NULL};
-
 /// Names of backends.
 const char *const BACKEND_STRS[NUM_BKEND + 1] = {"xrender",              // BKEND_XRENDER
                                                  "glx",                  // BKEND_GLX
@@ -2226,7 +2217,6 @@ static session_t *session_init(int argc, char **argv, Display *dpy,
 
 	            .refresh_rate = 0,
 	            .sw_opti = false,
-	            .vsync = VSYNC_NONE,
 
 	            .shadow_red = 0.0,
 	            .shadow_green = 0.0,

src/config.h

@@ -28,17 +28,6 @@
 
 typedef struct session session_t;
 
-/// VSync modes.
-typedef enum {
-	VSYNC_NONE,
-	VSYNC_DRM,
-	VSYNC_OPENGL,
-	VSYNC_OPENGL_OML,
-	VSYNC_OPENGL_SWC,
-	VSYNC_OPENGL_MSWC,
-	NUM_VSYNC,
-} vsync_t;
-
 /// @brief Possible backends of compton.
 enum backend {
 	BKEND_XRENDER,
@@ -138,9 +127,7 @@ typedef struct options_t {
 	/// Whether to enable refresh-rate-based software optimization.
 	bool sw_opti;
 	/// VSync method to use;
-	vsync_t vsync;
+	bool vsync;
-	/// Whether to do VSync aggressively.
-	bool vsync_aggressive;
 	/// Whether to use glFinish() instead of glFlush() for (possibly) better
 	/// VSync yet probably higher CPU usage.
 	bool vsync_use_glfinish;
@@ -238,7 +225,6 @@ typedef struct options_t {
 	bool track_leader;
 } options_t;
 
-extern const char *const VSYNC_STRS[NUM_VSYNC + 1];
 extern const char *const BACKEND_STRS[NUM_BKEND + 1];
 
 attr_warn_unused_result bool parse_long(const char *, long *);
@@ -347,14 +333,12 @@ static inline attr_pure int parse_glx_swap_method(const char *str) {
 /**
  * Parse a VSync option argument.
  */
-static inline vsync_t parse_vsync(const char *str) {
+static inline bool parse_vsync(const char *str) {
-	for (vsync_t i = 0; VSYNC_STRS[i]; ++i)
+	if (strcmp(str, "no") == 0 || strcmp(str, "none") == 0 ||
-		if (!strcasecmp(str, VSYNC_STRS[i])) {
+	    strcmp(str, "false") == 0 || strcmp(str, "nah") == 0) {
-			return i;
+		return false;
-		}
+	}
-
+	return true;
-	log_error("Invalid vsync argument: %s", str);
-	return NUM_VSYNC;
 }
 
 // vim: set noet sw=8 ts=8 :

src/config_libconfig.c

@@ -299,11 +299,12 @@ char *parse_config_libconfig(options_t *opt, const char *config_file, bool *shad
 	// --vsync
 	if (config_lookup_string(&cfg, "vsync", &sval)) {
 		opt->vsync = parse_vsync(sval);
-		if (opt->vsync >= NUM_VSYNC) {
+		log_warn("vsync option will take a boolean from now on. \"%s\" is "
-			log_fatal("Cannot parse vsync");
+		         "interpreted as \"%s\" for compatibility, but this will stop "
-			goto err;
+		         "working soon",
-		}
+		         sval, opt->vsync ? "true" : "false");
 	}
+	lcfg_lookup_bool(&cfg, "vsync", &opt->vsync);
 	// --backend
 	if (config_lookup_string(&cfg, "backend", &sval)) {
 		opt->backend = parse_backend(sval);

src/dbus.c

@@ -993,11 +993,7 @@ static bool cdbus_process_opts_get(session_t *ps, DBusMessage *msg) {
 
 	cdbus_m_opts_get_do(refresh_rate, cdbus_reply_int32);
 	cdbus_m_opts_get_do(sw_opti, cdbus_reply_bool);
-	if (!strcmp("vsync", target)) {
+	cdbus_m_opts_get_do(vsync, cdbus_reply_bool);
-		assert(ps->o.vsync < sizeof(VSYNC_STRS) / sizeof(VSYNC_STRS[0]));
-		cdbus_reply_string(ps, msg, VSYNC_STRS[ps->o.vsync]);
-		return true;
-	}
 	if (!strcmp("backend", target)) {
 		assert(ps->o.backend < sizeof(BACKEND_STRS) / sizeof(BACKEND_STRS[0]));
 		cdbus_reply_string(ps, msg, BACKEND_STRS[ps->o.backend]);

src/options.c

@@ -154,32 +154,8 @@ static void usage(int ret) {
 	    "  Specify refresh rate of the screen. If not specified or 0, compton\n"
 	    "  will try detecting this with X RandR extension.\n"
 	    "\n"
-	    "--vsync vsync-method\n"
+	    "--vsync\n"
-	    "  Set VSync method. There are (up to) 5 VSync methods currently\n"
+	    "  Enable VSync\n"
-	    "  available:\n"
-	    "    none = No VSync\n"
-	    "    drm = VSync with DRM_IOCTL_WAIT_VBLANK. May only work on some\n"
-	    "      (DRI-based) drivers."
-#ifndef CONFIG_VSYNC_DRM
-	    WARNING_DISABLED
-#endif
-	    "\n\n"
-#ifndef CONFIG_OPENGL
-#define WARNING WARNING_DISABLED
-#else
-#define WARNING
-#endif
-	    "    opengl = Try to VSync with SGI_video_sync OpenGL extension. Only\n"
-	    "      work on some drivers." WARNING "\n"
-	    "    opengl-oml = Try to VSync with OML_sync_control OpenGL extension.\n"
-	    "      Only work on some drivers." WARNING "\n"
-	    "    opengl-swc = Enable driver-level VSync. Works only with GLX "
-	    "backend." WARNING "\n"
-#undef WARNING
-	    "\n"
-	    "--vsync-aggressive\n"
-	    "  Attempt to send painting request before VBlank and do XFlush()\n"
-	    "  during VBlank. This switch may be lifted out at any moment.\n"
 	    "\n"
 	    "--paint-on-overlay\n"
 	    "  Painting on X Composite overlay window.\n"
@@ -377,7 +353,7 @@ static const struct option longopts[] = {
     {"detect-rounded-corners", no_argument, NULL, 267},
     {"detect-client-opacity", no_argument, NULL, 268},
     {"refresh-rate", required_argument, NULL, 269},
-    {"vsync", required_argument, NULL, 270},
+    {"vsync", optional_argument, NULL, 270},
     {"alpha-step", required_argument, NULL, 271},
     {"dbe", no_argument, NULL, 272},
     {"paint-on-overlay", no_argument, NULL, 273},
@@ -615,10 +591,15 @@ void get_cfg(options_t *opt, int argc, char *const *argv, bool shadow_enable,
 		P_CASEBOOL(268, detect_client_opacity);
 		P_CASELONG(269, refresh_rate);
 		case 270:
-			// --vsync
+			if (optarg) {
-			opt->vsync = parse_vsync(optarg);
+				opt->vsync = parse_vsync(optarg);
-			if (opt->vsync >= NUM_VSYNC)
+				log_warn("--vsync doesn't take argument anymore. \"%s\" "
-				exit(1);
+					 "is interpreted as \"%s\" for compatibility, but "
+					 "this will stop working soon",
+					 optarg, opt->vsync ? "true" : "false");
+			} else {
+				opt->vsync = true;
+			}
 			break;
 		case 271:
 			// --alpha-step

src/render.c

@@ -950,6 +950,10 @@ void paint_all(session_t *ps, win *const t, bool ignore_damage) {
 #endif
 	}
 
+	if (ps->vsync_wait) {
+		ps->vsync_wait(ps);
+	}
+
 	switch (ps->o.backend) {
 	case BKEND_XRENDER:
 		if (ps->o.monitor_repaint) {

src/vsync.c

@@ -46,7 +46,6 @@ static int vsync_drm_wait(session_t *ps) {
 
 	return ret;
 }
-#endif
 
 /**
  * Initialize DRM VSync.
@@ -54,7 +53,6 @@ static int vsync_drm_wait(session_t *ps) {
  * @return true for success, false otherwise
  */
 static bool vsync_drm_init(session_t *ps) {
-#ifdef CONFIG_VSYNC_DRM
 	// Should we always open card0?
 	if (ps->drm_fd < 0 && (ps->drm_fd = open("/dev/dri/card0", O_RDWR)) < 0) {
 		log_error("Failed to open device.");
@@ -65,12 +63,10 @@ static bool vsync_drm_init(session_t *ps) {
 		return false;
 
 	return true;
-#else
-	log_error("compton is not compiled with DRM VSync support.");
-	return false;
-#endif
 }
+#endif
 
+#ifdef CONFIG_OPENGL
 /**
  * Initialize OpenGL VSync.
  *
@@ -81,30 +77,19 @@ static bool vsync_drm_init(session_t *ps) {
  * @return true for success, false otherwise
  */
 static bool vsync_opengl_init(session_t *ps) {
-#ifdef CONFIG_OPENGL
 	if (!ensure_glx_context(ps))
 		return false;
 
 	return glxext.has_GLX_SGI_video_sync;
-#else
-	log_error("compton is not compiled with OpenGL VSync support.");
-	return false;
-#endif
 }
 
 static bool vsync_opengl_oml_init(session_t *ps) {
-#ifdef CONFIG_OPENGL
 	if (!ensure_glx_context(ps))
 		return false;
 
 	return glxext.has_GLX_OML_sync_control;
-#else
-	log_error("compton is not compiled with OpenGL VSync support.");
-	return false;
-#endif
 }
 
-#ifdef CONFIG_OPENGL
 static inline bool vsync_opengl_swc_swap_interval(session_t *ps, unsigned int interval) {
 	if (glxext.has_GLX_MESA_swap_control)
 		return glXSwapIntervalMESA(interval) == 0;
@@ -121,12 +106,10 @@ static inline bool vsync_opengl_swc_swap_interval(session_t *ps, unsigned int in
 	}
 	return false;
 }
-#endif
 
 static bool vsync_opengl_swc_init(session_t *ps) {
-#ifdef CONFIG_OPENGL
 	if (!bkend_use_glx(ps)) {
-		log_warn("OpenGL swap control requires the GLX backend.");
+		log_error("OpenGL swap control requires the GLX backend.");
 		return false;
 	}
 
@@ -136,26 +119,8 @@ static bool vsync_opengl_swc_init(session_t *ps) {
 	}
 
 	return true;
-#else
-	log_error("compton is not compiled with OpenGL VSync support.");
-	return false;
-#endif
 }
 
-static bool vsync_opengl_mswc_init(session_t *ps) {
-	log_warn("opengl-mswc is deprecated, please use opengl-swc instead.");
-	return vsync_opengl_swc_init(ps);
-}
-
-bool (*const VSYNC_FUNCS_INIT[NUM_VSYNC])(session_t *ps) = {
-    [VSYNC_DRM] = vsync_drm_init,
-    [VSYNC_OPENGL] = vsync_opengl_init,
-    [VSYNC_OPENGL_OML] = vsync_opengl_oml_init,
-    [VSYNC_OPENGL_SWC] = vsync_opengl_swc_init,
-    [VSYNC_OPENGL_MSWC] = vsync_opengl_mswc_init,
-};
-
-#ifdef CONFIG_OPENGL
 /**
  * Wait for next VSync, OpenGL method.
  */
@@ -164,8 +129,6 @@ static int vsync_opengl_wait(session_t *ps) {
 
 	glXGetVideoSyncSGI(&vblank_count);
 	glXWaitVideoSyncSGI(2, (vblank_count + 1) % 2, &vblank_count);
-	// I see some code calling glXSwapIntervalSGI(1) afterwards, is it required?
-
 	return 0;
 }
 
@@ -179,69 +142,63 @@ static int vsync_opengl_oml_wait(session_t *ps) {
 
 	glXGetSyncValuesOML(ps->dpy, ps->reg_win, &ust, &msc, &sbc);
 	glXWaitForMscOML(ps->dpy, ps->reg_win, 0, 2, (msc + 1) % 2, &ust, &msc, &sbc);
-
 	return 0;
 }
-
 #endif
 
-/// Function pointers to wait for VSync.
-int (*const VSYNC_FUNCS_WAIT[NUM_VSYNC])(session_t *ps) = {
-#ifdef CONFIG_VSYNC_DRM
-    [VSYNC_DRM] = vsync_drm_wait,
-#endif
-#ifdef CONFIG_OPENGL
-    [VSYNC_OPENGL] = vsync_opengl_wait,
-    [VSYNC_OPENGL_OML] = vsync_opengl_oml_wait,
-#endif
-};
-
-#ifdef CONFIG_OPENGL
-static void vsync_opengl_swc_deinit(session_t *ps) {
-	vsync_opengl_swc_swap_interval(ps, 0);
-}
-#endif
-
-/// Function pointers to deinitialize VSync.
-void (*const VSYNC_FUNCS_DEINIT[NUM_VSYNC])(session_t *ps) = {
-#ifdef CONFIG_OPENGL
-    [VSYNC_OPENGL_SWC] = vsync_opengl_swc_deinit,
-    [VSYNC_OPENGL_MSWC] = vsync_opengl_swc_deinit,
-#endif
-};
-
 /**
  * Initialize current VSync method.
  */
 bool vsync_init(session_t *ps) {
-	// Mesa turns on swap control by default, undo that
 #ifdef CONFIG_OPENGL
-	if (bkend_use_glx(ps))
+	if (bkend_use_glx(ps)) {
+		// Mesa turns on swap control by default, undo that
 		vsync_opengl_swc_swap_interval(ps, 0);
+	}
+#endif
+#ifdef CONFIG_VSYNC_DRM
+	log_warn("The DRM vsync method is deprecated, please don't enable it.");
 #endif
 
-	if (ps->o.vsync && VSYNC_FUNCS_INIT[ps->o.vsync] && !VSYNC_FUNCS_INIT[ps->o.vsync](ps)) {
+	if (!ps->o.vsync) {
-		ps->o.vsync = VSYNC_NONE;
-		return false;
-	} else
 		return true;
-}
+	}
 
-/**
+#ifdef CONFIG_OPENGL
- * Wait for next VSync.
+	if (bkend_use_glx(ps)) {
- */
+		if (!vsync_opengl_swc_init(ps)) {
-void vsync_wait(session_t *ps) {
+			return false;
-	if (!ps->o.vsync)
+		}
-		return;
+		ps->vsync_wait = NULL;        // glXSwapBuffers will automatically wait
+		                              // for vsync, we don't need to do anything.
+		return true;
+	}
+#endif
 
-	if (VSYNC_FUNCS_WAIT[ps->o.vsync])
+	// Oh no, we are not using glx backend.
-		VSYNC_FUNCS_WAIT[ps->o.vsync](ps);
+	// Throwing things at wall.
-}
+#ifdef CONFIG_OPENGL
+	if (vsync_opengl_oml_init(ps)) {
+		log_info("Using the opengl-oml vsync method");
+		ps->vsync_wait = vsync_opengl_oml_wait;
+		return true;
+	}
 
-/**
+	if (vsync_opengl_init(ps)) {
- * Deinitialize current VSync method.
+		log_info("Using the opengl vsync method");
- */
+		ps->vsync_wait = vsync_opengl_wait;
-void vsync_deinit(session_t *ps) {
+		return true;
-	if (ps->o.vsync && VSYNC_FUNCS_DEINIT[ps->o.vsync])
+	}
-		VSYNC_FUNCS_DEINIT[ps->o.vsync](ps);
+#endif
+
+#ifdef CONFIG_VSYNC_DRM
+	if (vsync_drm_init(ps)) {
+		log_info("Using the drm vsync method");
+		ps->vsync_wait = vsync_drm_wait;
+		return true;
+	}
+#endif
+
+	log_error("No supported vsync method found for this backend");
+	return false;
 }

src/vsync.h

@@ -5,5 +5,3 @@
 typedef struct session session_t;
 
 bool vsync_init(session_t *ps);
-void vsync_wait(session_t *ps);
-void vsync_deinit(session_t *ps);