Merge pull request #130 from yshui/vsync

vsync: choose vsync method automatically

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