Improvement: Change VSync mode with D-Bus & Makefile update & Misc

- Add on-the-fly VSync option modification via D-Bus, as requested by
  kunitoki (#80). Expose parse_vsync(), create vsync_init() and
  ensure_glx_context().

- Change default value of ps->drm_fd to -1.

- Update Makefile. Change the install/uninstall rules and add doc
  installation, requested by hasufell in #85.

- Mark window not damaged in map_win(). It helps in reducing flickering
  with inverted window color, but I'm not completely sure if it's safe.

- Avoid modifying w->invert_color when window is unmapped.

- Update documentation. Thanks to hasufell for pointing out.

Makefile

@@ -73,6 +73,8 @@ LIBS += $(shell pkg-config --libs $(PACKAGES))
 INCS += $(shell pkg-config --cflags $(PACKAGES))
 
 CFLAGS += -Wall -std=c99
+
+BINS = compton bin/compton-trans
 MANPAGES = man/compton.1 man/compton-trans.1
 MANPAGES_HTML = $(addsuffix .html,$(MANPAGES))
 
@@ -96,17 +98,22 @@ man/%.1.html: man/%.1.asciidoc
 
 docs: $(MANPAGES) $(MANPAGES_HTML)
 
-install: compton docs
+install: $(BINS) docs
-	@install -Dm755 compton "$(DESTDIR)$(BINDIR)"/compton
+	@install -d "$(DESTDIR)$(BINDIR)" "$(DESTDIR)$(MANDIR)"
-	@install -Dm755 bin/compton-trans "$(DESTDIR)$(BINDIR)"/compton-trans
+	@install -D -m755 $(BINS) "$(DESTDIR)$(BINDIR)"/ 
-	@install -Dm644 man/compton.1 "$(DESTDIR)$(MANDIR)"/compton.1
+	@install -D -m644 $(MANPAGES) "$(DESTDIR)$(MANDIR)"/
-	@install -Dm644 man/compton-trans.1 "$(DESTDIR)$(MANDIR)"/compton-trans.1
+ifneq "$(DOCDIR)" ""
+	@install -d "$(DESTDIR)$(DOCDIR)"
+	@install -D -m644 README.md compton.sample.conf "$(DESTDIR)$(DOCDIR)"/
+	@install -D -m755 dbus-examples/cdbus-driver.sh "$(DESTDIR)$(DOCDIR)"/
+endif
 
 uninstall:
-	@rm -f "$(DESTDIR)$(BINDIR)/compton"
+	@rm -f "$(DESTDIR)$(BINDIR)/compton" "$(DESTDIR)$(BINDIR)/compton-trans"
-	@rm -f "$(DESTDIR)$(BINDIR)/compton-trans"
+	@rm -f $(addprefix "$(DESTDIR)$(MANDIR)"/, compton.1 compton-trans.1)
-	@rm -f "$(DESTDIR)$(MANDIR)/compton.1"
+ifneq "$(DOCDIR)" ""
-	@rm -f "$(DESTDIR)$(MANDIR)/compton-trans.1"
+	@rm -f $(addprefix "$(DESTDIR)$(DOCDIR)"/, README.md compton.sample.conf cdbus-driver.sh)
+endif
 
 clean:
 	@rm -f $(OBJS) compton $(MANPAGES) $(MANPAGES_HTML) .clang_complete

README.md

@@ -50,7 +50,7 @@ __R__ for runtime
 * pkg-config (B)
 * make (B)
 * xproto / x11proto (B)
-* bash (R)
+* sh (R)
 * xprop,xwininfo / x11-utils (R)
 * libpcre (B,R) (Can be disabled with `NO_REGEX_PCRE` at compile time)
 * libconfig (B,R) (Can be disabled with `NO_LIBCONFIG` at compile time)

man/compton.1.asciidoc

@@ -120,12 +120,13 @@ OPTIONS
 	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. There are 2 VSync methods currently available:
+	Set VSync method. There are 3 VSync methods currently available:
 +
 --
 * 'none': No VSync
 * 'drm': VSync with 'DRM_IOCTL_WAIT_VBLANK'. May only work on some drivers. Experimental.
 * 'opengl': Try to VSync with 'SGI_swap_control' OpenGL extension. Only work on some drivers. Experimental.
+* 'opengl-oml': Try to VSync with 'OML_sync_control' OpenGL extension. Only work on some drivers. Experimental.
 
 (Note some VSync methods may not be enabled at compile time.)
 --

src/common.h

@@ -1167,6 +1167,20 @@ normalize_d(double d) {
   return normalize_d_range(d, 0.0, 1.0);
 }
 
+/**
+ * Parse a VSync option argument.
+ */
+static inline bool
+parse_vsync(session_t *ps, const char *str) {
+  for (vsync_t i = 0; i < (sizeof(VSYNC_STRS) / sizeof(VSYNC_STRS[0])); ++i)
+    if (!strcasecmp(str, VSYNC_STRS[i])) {
+      ps->o.vsync = i;
+      return true;
+    }
+  printf_errf("(\"%s\"): Invalid vsync argument.", str);
+  return false;
+}
+
 timeout_t *
 timeout_insert(session_t *ps, time_ms_t interval,
     bool (*callback)(session_t *ps, timeout_t *ptmout), void *data);
@@ -1421,6 +1435,9 @@ free_winprop(winprop_t *pprop) {
 void
 force_repaint(session_t *ps);
 
+bool
+vsync_init(session_t *ps);
+
 #ifdef CONFIG_DBUS
 /** @name DBus handling
  */

src/compton.c

@@ -1875,7 +1875,7 @@ map_win(session_t *ps, Window id) {
   }
   win_determine_fade(ps, w);
 
-  w->damaged = true;
+  w->damaged = false;
 
   /* if any configure events happened while
      the window was unmapped, then configure
@@ -2184,6 +2184,11 @@ win_determine_shadow(session_t *ps, win *w) {
  */
 static void
 win_determine_invert_color(session_t *ps, win *w) {
+  // Do not change window invert color state when the window is unmapped,
+  // unless it comes from w->invert_color_force.
+  if (UNSET == w->invert_color_force && IsViewable != w->a.map_state)
+    return;
+
   bool invert_color_old = w->invert_color;
 
   if (UNSET != w->invert_color_force)
@@ -3896,7 +3901,7 @@ usage(void) {
     "  Specify refresh rate of the screen. If not specified or 0, compton\n"
     "  will try detecting this with X RandR extension.\n"
     "--vsync vsync-method\n"
-    "  Set VSync method. There are up to 2 VSync methods currently available\n"
+    "  Set VSync method. There are up to 3 VSync methods currently available\n"
     "  depending on your compile time settings:\n"
     "    none = No VSync\n"
 #undef WARNING
@@ -3915,6 +3920,8 @@ usage(void) {
 #endif
     "    opengl = Try to VSync with SGI_swap_control OpenGL extension. Only\n"
     "      work on some drivers. Experimental." WARNING"\n"
+    "    opengl-oml = Try to VSync with OML_sync_control OpenGL extension.\n"
+    "      Only work on some drivers. Experimental." WARNING"\n"
     "--alpha-step val\n"
     "  Step for pregenerating alpha pictures. 0.01 - 1.0. Defaults to\n"
     "  0.03.\n"
@@ -3986,6 +3993,8 @@ usage(void) {
  */
 static bool
 register_cm(session_t *ps, bool glx) {
+  assert(!ps->reg_win);
+
   XVisualInfo *pvi = NULL;
 
 #ifdef CONFIG_VSYNC_OPENGL
@@ -4023,6 +4032,10 @@ register_cm(session_t *ps, bool glx) {
     return false;
   }
 
+  // Unredirect the window if it's redirected, just in case
+  if (ps->redirected)
+    XCompositeUnredirectWindow(ps->dpy, ps->reg_win, CompositeRedirectManual);
+
 #ifdef CONFIG_VSYNC_OPENGL
   if (glx) {
     // Get GLX context
@@ -4197,23 +4210,6 @@ open_config_file(char *cpath, char **ppath) {
   return NULL;
 }
 
-/**
- * Parse a VSync option argument.
- */
-static inline void
-parse_vsync(session_t *ps, const char *optarg) {
-  vsync_t i;
-
-  for (i = 0; i < (sizeof(VSYNC_STRS) / sizeof(VSYNC_STRS[0])); ++i)
-    if (!strcasecmp(optarg, VSYNC_STRS[i])) {
-      ps->o.vsync = i;
-      break;
-    }
-  if ((sizeof(VSYNC_STRS) / sizeof(VSYNC_STRS[0])) == i) {
-    printf_errfq(1, "(\"%s\"): Invalid --vsync argument.", optarg);
-  }
-}
-
 /**
  * Parse a condition list in configuration file.
  */
@@ -4357,8 +4353,8 @@ parse_config(session_t *ps, struct options_tmp *pcfgtmp) {
   // --refresh-rate
   lcfg_lookup_int(&cfg, "refresh-rate", &ps->o.refresh_rate);
   // --vsync
-  if (config_lookup_string(&cfg, "vsync", &sval))
+  if (config_lookup_string(&cfg, "vsync", &sval) && !parse_vsync(ps, sval))
-    parse_vsync(ps, sval);
+    exit(1);
   // --alpha-step
   config_lookup_float(&cfg, "alpha-step", &ps->o.alpha_step);
   // --dbe
@@ -4634,7 +4630,8 @@ get_cfg(session_t *ps, int argc, char *const *argv, bool first_pass) {
         break;
       case 270:
         // --vsync
-        parse_vsync(ps, optarg);
+        if (!parse_vsync(ps, optarg))
+          exit(1);
         break;
       case 271:
         // --alpha-step
@@ -4905,7 +4902,7 @@ static bool
 vsync_drm_init(session_t *ps) {
 #ifdef CONFIG_VSYNC_DRM
   // Should we always open card0?
-  if ((ps->drm_fd = open("/dev/dri/card0", O_RDWR)) < 0) {
+  if (ps->drm_fd < 0 && (ps->drm_fd = open("/dev/dri/card0", O_RDWR)) < 0) {
     printf_errf("(): Failed to open device.");
     return false;
   }
@@ -4959,11 +4956,16 @@ vsync_drm_wait(session_t *ps) {
 static bool
 vsync_opengl_init(session_t *ps) {
 #ifdef CONFIG_VSYNC_OPENGL
+  if (!ensure_glx_context(ps))
+    return false;
+
   // Get video sync functions
-  ps->glXGetVideoSyncSGI = (f_GetVideoSync)
+  if (!ps->glXWaitVideoSyncSGI)
-    glXGetProcAddress ((const GLubyte *) "glXGetVideoSyncSGI");
+    ps->glXGetVideoSyncSGI = (f_GetVideoSync)
-  ps->glXWaitVideoSyncSGI = (f_WaitVideoSync)
+      glXGetProcAddress ((const GLubyte *) "glXGetVideoSyncSGI");
-    glXGetProcAddress ((const GLubyte *) "glXWaitVideoSyncSGI");
+  if (!ps->glXWaitVideoSyncSGI)
+    ps->glXWaitVideoSyncSGI = (f_WaitVideoSync)
+      glXGetProcAddress ((const GLubyte *) "glXWaitVideoSyncSGI");
   if (!ps->glXWaitVideoSyncSGI || !ps->glXGetVideoSyncSGI) {
     printf_errf("(): Failed to get glXWait/GetVideoSyncSGI function.");
     return false;
@@ -4971,7 +4973,7 @@ vsync_opengl_init(session_t *ps) {
 
   return true;
 #else
-  printf_errfq(1, "Program not compiled with OpenGL VSync support.");
+  printf_errf("(): Program not compiled with OpenGL VSync support.");
   return false;
 #endif
 }
@@ -4979,11 +4981,16 @@ vsync_opengl_init(session_t *ps) {
 static bool
 vsync_opengl_oml_init(session_t *ps) {
 #ifdef CONFIG_VSYNC_OPENGL
+  if (!ensure_glx_context(ps))
+    return false;
+
   // Get video sync functions
-  ps->glXGetSyncValuesOML= (f_GetSyncValuesOML)
+  if (!ps->glXGetSyncValuesOML)
-    glXGetProcAddress ((const GLubyte *) "glXGetSyncValuesOML");
+    ps->glXGetSyncValuesOML = (f_GetSyncValuesOML)
-  ps->glXWaitForMscOML = (f_WaitForMscOML)
+      glXGetProcAddress ((const GLubyte *) "glXGetSyncValuesOML");
-    glXGetProcAddress ((const GLubyte *) "glXWaitForMscOML");
+  if (!ps->glXWaitForMscOML)
+    ps->glXWaitForMscOML = (f_WaitForMscOML)
+      glXGetProcAddress ((const GLubyte *) "glXWaitForMscOML");
   if (!ps->glXGetSyncValuesOML || !ps->glXWaitForMscOML) {
     printf_errf("(): Failed to get OML_sync_control functions.");
     return false;
@@ -4991,7 +4998,7 @@ vsync_opengl_oml_init(session_t *ps) {
 
   return true;
 #else
-  printf_errfq(1, "Program not compiled with OpenGL VSync support.");
+  printf_errf("(): Program not compiled with OpenGL VSync support.");
   return false;
 #endif
 }
@@ -5028,6 +5035,20 @@ vsync_opengl_oml_wait(session_t *ps) {
 }
 #endif
 
+/**
+ * Initialize current VSync method.
+ */
+bool
+vsync_init(session_t *ps) {
+  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
+    return true;
+}
+
 /**
  * Wait for next VSync.
  */
@@ -5534,7 +5555,7 @@ session_init(session_t *ps_old, int argc, char **argv) {
     .paint_tm_offset = 0L,
 
 #ifdef CONFIG_VSYNC_DRM
-    .drm_fd = 0,
+    .drm_fd = -1,
 #endif
 
 #ifdef CONFIG_VSYNC_OPENGL
@@ -5630,9 +5651,6 @@ session_init(session_t *ps_old, int argc, char **argv) {
   ps->vis = DefaultVisual(ps->dpy, ps->scr);
   ps->depth = DefaultDepth(ps->dpy, ps->scr);
 
-  bool want_glx = (VSYNC_OPENGL == ps->o.vsync
-      || VSYNC_OPENGL_OML == ps->o.vsync);
-
   if (!XRenderQueryExtension(ps->dpy,
         &ps->render_event, &ps->render_error)) {
     fprintf(stderr, "No render extension\n");
@@ -5679,19 +5697,6 @@ session_init(session_t *ps_old, int argc, char **argv) {
           "detection impossible.");
   }
 
-  // Query X GLX extension
-  if (want_glx) {
-#ifdef CONFIG_VSYNC_OPENGL
-    if (glXQueryExtension(ps->dpy, &ps->glx_event, &ps->glx_error))
-      ps->glx_exists = true;
-    else {
-      printf_errfq(1, "(): No GLX extension, OpenGL VSync impossible.");
-    }
-#else
-    printf_errfq(1, "(): OpenGL VSync support not compiled in.");
-#endif
-  }
-
   // Query X DBE extension
   if (ps->o.dbe) {
     int dbe_ver_major = 0, dbe_ver_minor = 0;
@@ -5708,16 +5713,12 @@ session_init(session_t *ps_old, int argc, char **argv) {
       ps->o.dbe = false;
   }
 
-  if (!register_cm(ps, want_glx))
-    exit(1);
-
   // Initialize software optimization
   if (ps->o.sw_opti)
     ps->o.sw_opti = swopti_init(ps);
 
   // Initialize VSync
-  if (ps->o.vsync && VSYNC_FUNCS_INIT[ps->o.vsync]
+  if (!vsync_init(ps))
-      && !VSYNC_FUNCS_INIT[ps->o.vsync](ps))
     exit(1);
 
   // Overlay must be initialized before double buffer
@@ -5727,6 +5728,11 @@ session_init(session_t *ps_old, int argc, char **argv) {
   if (ps->o.dbe && !init_dbe(ps))
     exit(1);
 
+  // Create registration window
+  // Must not precede VSync init functions because they may create the window
+  if (!ps->reg_win && !register_cm(ps, false))
+    exit(1);
+
   init_atoms(ps);
   init_alpha_picts(ps);
 
@@ -5970,9 +5976,9 @@ session_destroy(session_t *ps) {
 
 #ifdef CONFIG_VSYNC_DRM
   // Close file opened for DRM VSync
-  if (ps->drm_fd) {
+  if (ps->drm_fd >= 0) {
     close(ps->drm_fd);
-    ps->drm_fd = 0;
+    ps->drm_fd = -1;
   }
 #endif
 

src/compton.h

@@ -707,6 +707,39 @@ usage(void);
 static bool
 register_cm(session_t *ps, bool glx);
 
+#ifdef CONFIG_VSYNC_OPENGL
+/**
+ * Ensure we have a GLX context.
+ */
+static inline bool
+ensure_glx_context(session_t *ps) {
+  if (ps->glx_context)
+    return true;
+
+  // Check for GLX extension
+  if (!ps->glx_exists) {
+    if (glXQueryExtension(ps->dpy, &ps->glx_event, &ps->glx_error))
+      ps->glx_exists = true;
+    else {
+      printf_errf("(): No GLX extension.");
+      return false;
+    }
+  }
+
+  // Create GLX context
+  if (ps->reg_win) {
+    XDestroyWindow(ps->dpy, ps->reg_win);
+    ps->reg_win = None;
+  }
+  if (!register_cm(ps, true) || !ps->glx_context) {
+    printf_errf("(): Failed to acquire GLX context.");
+    return false;
+  }
+
+  return true;
+}
+#endif
+
 inline static void
 ev_focus_in(session_t *ps, XFocusChangeEvent *ev);
 

src/dbus.c

@@ -952,6 +952,25 @@ cdbus_process_opts_set(session_t *ps, DBusMessage *msg) {
     }
     goto cdbus_process_opts_set_success;
   }
+
+  // vsync
+  if (!strcmp("vsync", target)) {
+    const char * val = NULL;
+    if (!cdbus_msg_get_arg(msg, 1, DBUS_TYPE_STRING, &val))
+      return false;
+    if (!parse_vsync(ps, val)) {
+      printf_errf("(): " CDBUS_ERROR_BADARG_S, 1, "Value invalid.");
+      cdbus_reply_err(ps, msg, CDBUS_ERROR_BADARG, CDBUS_ERROR_BADARG_S, 1, "Value invalid.");
+    }
+    else if (!vsync_init(ps)) {
+      printf_errf("(): " CDBUS_ERROR_CUSTOM_S, "Failed to initialize specified VSync method.");
+      cdbus_reply_err(ps, msg, CDBUS_ERROR_CUSTOM, CDBUS_ERROR_CUSTOM_S, "Failed to initialize specified VSync method.");
+    }
+    else
+      goto cdbus_process_opts_set_success;
+    return true;
+  }
+
 #undef cdbus_m_opts_set_do
 
   printf_errf("(): " CDBUS_ERROR_BADTGT_S, target);

src/dbus.h

@@ -21,13 +21,15 @@
 #define CDBUS_ERROR_BADMSG_S    "Unrecognized command. Beware compton " \
                                 "cannot make you a sandwich."
 #define CDBUS_ERROR_BADARG      CDBUS_ERROR_PREFIX ".bad_argument"
-#define CDBUS_ERROR_BADARG_S    "Something wrong in arguments?"
+#define CDBUS_ERROR_BADARG_S    "Failed to parse argument %d: %s"
 #define CDBUS_ERROR_BADWIN      CDBUS_ERROR_PREFIX ".bad_window"
 #define CDBUS_ERROR_BADWIN_S    "Requested window %#010lx not found."
 #define CDBUS_ERROR_BADTGT      CDBUS_ERROR_PREFIX ".bad_target"
 #define CDBUS_ERROR_BADTGT_S    "Target \"%s\" not found."
 #define CDBUS_ERROR_FORBIDDEN   CDBUS_ERROR_PREFIX ".forbidden"
 #define CDBUS_ERROR_FORBIDDEN_S "Incorrect password, access denied."
+#define CDBUS_ERROR_CUSTOM      CDBUS_ERROR_PREFIX ".custom"
+#define CDBUS_ERROR_CUSTOM_S    "%s"
 
 // Window type
 typedef uint32_t cdbus_window_t;