From cebe1b18d664504e88fd67ac44f0dcbdce702ee7 Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Sun, 10 Mar 2019 14:55:17 +0000 Subject: [PATCH] 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 --- compton.sample.conf | 2 +- man/compton.1.asciidoc | 15 +---- src/backend/xrender.c | 2 +- src/common.h | 2 + src/compton.c | 10 --- src/config.h | 30 +++------ src/config_libconfig.c | 9 +-- src/dbus.c | 6 +- src/options.c | 43 ++++--------- src/render.c | 4 ++ src/vsync.c | 135 ++++++++++++++--------------------------- src/vsync.h | 2 - 12 files changed, 81 insertions(+), 179 deletions(-) diff --git a/compton.sample.conf b/compton.sample.conf index 814e2b74..cfceb45b 100644 --- a/compton.sample.conf +++ b/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; diff --git a/man/compton.1.asciidoc b/man/compton.1.asciidoc index 1d69b9b5..f0f6c700 100644 --- a/man/compton.1.asciidoc +++ b/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':: - Set VSync method. VSync methods currently available: -+ --- -* '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.) --- +*--vsync*:: + Enable VSync. *--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. diff --git a/src/backend/xrender.c b/src/backend/xrender.c index d7dd5722..e2cf3e89 100644 --- a/src/backend/xrender.c +++ b/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 = diff --git a/src/common.h b/src/common.h index 4690ee7b..0776a014 100644 --- a/src/common.h +++ b/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 diff --git a/src/compton.c b/src/compton.c index 36bd7608..931a1ce1 100644 --- a/src/compton.c +++ b/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, diff --git a/src/config.h b/src/config.h index 65a736a1..e818e3c5 100644 --- a/src/config.h +++ b/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; - /// Whether to do VSync aggressively. - bool vsync_aggressive; + bool vsync; /// 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) { - for (vsync_t i = 0; VSYNC_STRS[i]; ++i) - if (!strcasecmp(str, VSYNC_STRS[i])) { - return i; - } - - log_error("Invalid vsync argument: %s", str); - return NUM_VSYNC; +static inline bool parse_vsync(const char *str) { + if (strcmp(str, "no") == 0 || strcmp(str, "none") == 0 || + strcmp(str, "false") == 0 || strcmp(str, "nah") == 0) { + return false; + } + return true; } // vim: set noet sw=8 ts=8 : diff --git a/src/config_libconfig.c b/src/config_libconfig.c index b03e601c..d4df3adb 100644 --- a/src/config_libconfig.c +++ b/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_fatal("Cannot parse vsync"); - goto err; - } + log_warn("vsync option will take a boolean from now on. \"%s\" is " + "interpreted as \"%s\" for compatibility, but this will stop " + "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); diff --git a/src/dbus.c b/src/dbus.c index b2f7e538..a6602e3f 100644 --- a/src/dbus.c +++ b/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)) { - assert(ps->o.vsync < sizeof(VSYNC_STRS) / sizeof(VSYNC_STRS[0])); - cdbus_reply_string(ps, msg, VSYNC_STRS[ps->o.vsync]); - return true; - } + cdbus_m_opts_get_do(vsync, cdbus_reply_bool); 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]); diff --git a/src/options.c b/src/options.c index 0af7684f..dd7a1889 100644 --- a/src/options.c +++ b/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" - " Set VSync method. There are (up to) 5 VSync methods currently\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" + "--vsync\n" + " Enable VSync\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 - opt->vsync = parse_vsync(optarg); - if (opt->vsync >= NUM_VSYNC) - exit(1); + if (optarg) { + opt->vsync = parse_vsync(optarg); + log_warn("--vsync doesn't take argument anymore. \"%s\" " + "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 diff --git a/src/render.c b/src/render.c index 6f4ba146..9d93df2d 100644 --- a/src/render.c +++ b/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) { diff --git a/src/vsync.c b/src/vsync.c index 2525a7df..fba4ebf8 100644 --- a/src/vsync.c +++ b/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)) { - ps->o.vsync = VSYNC_NONE; - return false; - } else + if (!ps->o.vsync) { return true; -} + } -/** - * Wait for next VSync. - */ -void vsync_wait(session_t *ps) { - if (!ps->o.vsync) - return; +#ifdef CONFIG_OPENGL + if (bkend_use_glx(ps)) { + if (!vsync_opengl_swc_init(ps)) { + return false; + } + 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]) - VSYNC_FUNCS_WAIT[ps->o.vsync](ps); -} + // Oh no, we are not using glx backend. + // 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; + } -/** - * Deinitialize current VSync method. - */ -void vsync_deinit(session_t *ps) { - if (ps->o.vsync && VSYNC_FUNCS_DEINIT[ps->o.vsync]) - VSYNC_FUNCS_DEINIT[ps->o.vsync](ps); + if (vsync_opengl_init(ps)) { + log_info("Using the opengl vsync method"); + ps->vsync_wait = vsync_opengl_wait; + return true; + } +#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; } diff --git a/src/vsync.h b/src/vsync.h index e50f6d0d..076bc260 100644 --- a/src/vsync.h +++ b/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);