Improvement #7: Add GLX_OML_sync_control VSync support

- Add "vsync-oml" VSync method, using GLX_OML_sync_control. Untested,
  because it's not supported by my driver.

- Unredirect ps->reg_win, because DRI wiki says it's related to the
  behavior of OpenGL VSync extensions.

- Add glFlush() and glXWaitX() calls, in hope they are slightly helpful
  for VSync.

- Change a few functions to make error handling more graceful. Make some
  errors fatal. Code clean-up.

- Add unused function make_text_prop().

Makefile

@@ -44,6 +44,7 @@ endif
 # ==== OpenGL VSync ====
 ifeq "$(NO_VSYNC_OPENGL)" ""
   CFG += -DCONFIG_VSYNC_OPENGL
+  # -lGL must precede some other libraries, or it segfaults on FreeBSD (#74)
   LIBS := -lGL $(LIBS)
 endif
 

src/common.h

@@ -248,12 +248,16 @@ typedef enum {
   VSYNC_NONE,
   VSYNC_DRM,
   VSYNC_OPENGL,
+  VSYNC_OPENGL_OML,
   NUM_VSYNC,
 } vsync_t;
 
 #ifdef CONFIG_VSYNC_OPENGL
 typedef int (*f_WaitVideoSync) (int, int, unsigned *);
 typedef int (*f_GetVideoSync) (unsigned *);
+
+typedef Bool (*f_GetSyncValuesOML) (Display* dpy, GLXDrawable drawable, int64_t* ust, int64_t* msc, int64_t* sbc);
+typedef Bool (*f_WaitForMscOML) (Display* dpy, GLXDrawable drawable, int64_t target_msc, int64_t divisor, int64_t remainder, int64_t* ust, int64_t* msc, int64_t* sbc);
 #endif
 
 typedef struct {
@@ -536,9 +540,13 @@ typedef struct {
   /// GLX context.
   GLXContext glx_context;
   /// Pointer to glXGetVideoSyncSGI function.
-  f_GetVideoSync glx_get_video_sync;
+  f_GetVideoSync glXGetVideoSyncSGI;
   /// Pointer to glXWaitVideoSyncSGI function.
-  f_WaitVideoSync glx_wait_video_sync;
+  f_WaitVideoSync glXWaitVideoSyncSGI;
+  /// Pointer to glXGetSyncValuesOML function.
+  f_GetSyncValuesOML glXGetSyncValuesOML;
+  /// Pointer to glXWaitForMscOML function.
+  f_WaitForMscOML glXWaitForMscOML;
 #endif
 
   // === X extension related ===

src/compton.c

@@ -31,11 +31,30 @@ const char * const WINTYPES[NUM_WINTYPES] = {
   "dnd",
 };
 
-/// Names of VSync modes
+/// Names of VSync modes.
 const char * const VSYNC_STRS[NUM_VSYNC] = {
-  "none",   // VSYNC_NONE
+  "none",         // VSYNC_NONE
-  "drm",    // VSYNC_DRM
+  "drm",          // VSYNC_DRM
-  "opengl", // VSYNC_OPENGL
+  "opengl",       // VSYNC_OPENGL
+  "opengl-oml",   // VSYNC_OPENGL_OML
+};
+
+/// Function pointers to init VSync modes.
+static 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,
+};
+
+/// Function pointers to wait for VSync.
+static int (* const (VSYNC_FUNCS_WAIT[NUM_VSYNC]))(session_t *ps) = {
+#ifdef CONFIG_VSYNC_DRM
+  [VSYNC_DRM        ] = vsync_drm_wait,
+#endif
+#ifdef CONFIG_VSYNC_OPENGL
+  [VSYNC_OPENGL     ] = vsync_opengl_wait,
+  [VSYNC_OPENGL_OML ] = vsync_opengl_oml_wait,
+#endif
 };
 
 /// Names of root window properties that could point to a pixmap of
@@ -1651,10 +1670,16 @@ paint_all(session_t *ps, XserverRegion region, win *t) {
   if (!ps->o.dbe)
     XFixesSetPictureClipRegion(ps->dpy, ps->tgt_buffer, 0, 0, None);
 
-  if (VSYNC_NONE != ps->o.vsync) {
+  if (ps->o.vsync) {
     // Make sure all previous requests are processed to achieve best
     // effect
     XSync(ps->dpy, False);
+#ifdef CONFIG_VSYNC_OPENGL
+    if (ps->glx_context) {
+      glFlush();
+      glXWaitX();
+    }
+#endif
   }
 
   // Wait for VBlank. We could do it aggressively (send the painting
@@ -1685,6 +1710,13 @@ paint_all(session_t *ps, XserverRegion region, win *t) {
 
   XFlush(ps->dpy);
 
+#ifdef CONFIG_VSYNC_OPENGL
+  if (ps->glx_context) {
+    glFlush();
+    glXWaitX();
+  }
+#endif
+
 #ifdef DEBUG_REPAINT
   print_timestamp(ps);
   struct timespec now = get_time_timespec();
@@ -3952,84 +3984,85 @@ usage(void) {
 /**
  * Register a window as symbol, and initialize GLX context if wanted.
  */
-static void
+static bool
-register_cm(session_t *ps, bool want_glxct) {
+register_cm(session_t *ps, bool glx) {
-  Atom a;
+  XVisualInfo *pvi = NULL;
-  char *buf;
 
 #ifdef CONFIG_VSYNC_OPENGL
   // Create a window with the wanted GLX visual
-  if (want_glxct) {
+  if (glx) {
-    XVisualInfo *pvi = NULL;
-    bool ret = false;
     // Get visual for the window
     int attribs[] = { GLX_RGBA, GLX_RED_SIZE, 1, GLX_GREEN_SIZE, 1, GLX_BLUE_SIZE, 1, None };
     pvi = glXChooseVisual(ps->dpy, ps->scr, attribs);
 
     if (!pvi) {
-      fprintf(stderr, "register_cm(): Failed to choose visual required "
+      printf_errf("(): Failed to choose GLX visual.");
-          "by fake OpenGL VSync window. OpenGL VSync turned off.\n");
+      return false;
     }
-    else {
-      // Create the window
-      XSetWindowAttributes swa = {
-        .colormap = XCreateColormap(ps->dpy, ps->root, pvi->visual, AllocNone),
-        .border_pixel = 0,
-      };
 
-      pvi->screen = ps->scr;
+    // Create the window
-      ps->reg_win = XCreateWindow(ps->dpy, ps->root, 0, 0, 1, 1, 0, pvi->depth,
+    XSetWindowAttributes swa = {
-          InputOutput, pvi->visual, CWBorderPixel | CWColormap, &swa);
+      .colormap = XCreateColormap(ps->dpy, ps->root, pvi->visual, AllocNone),
+      .border_pixel = 0,
+    };
 
-      if (!ps->reg_win)
+    pvi->screen = ps->scr;
-        fprintf(stderr, "register_cm(): Failed to create window required "
+    ps->reg_win = XCreateWindow(ps->dpy, ps->root, 0, 0, 1, 1, 0, pvi->depth,
-            "by fake OpenGL VSync. OpenGL VSync turned off.\n");
+        InputOutput, pvi->visual, CWBorderPixel | CWColormap, &swa);
-      else {
+  }
-        // Get GLX context
+  // Otherwise, create a simple window
-        ps->glx_context = glXCreateContext(ps->dpy, pvi, None, GL_TRUE);
+  else
-        if (!ps->glx_context) {
+#endif
-          fprintf(stderr, "register_cm(): Failed to get GLX context. "
+  {
-              "OpenGL VSync turned off.\n");
+    ps->reg_win = XCreateSimpleWindow(ps->dpy, ps->root, 0, 0, 1, 1, 0,
-          ps->o.vsync = VSYNC_NONE;
+        None, None);
-        }
+  }
-        else {
+
-          // Attach GLX context
+  if (!ps->reg_win) {
-          if (!(ret = glXMakeCurrent(ps->dpy, ps->reg_win, ps->glx_context)))
+    printf_errf("(): Failed to create window.");
-            fprintf(stderr, "register_cm(): Failed to attach GLX context."
+    return false;
-              " OpenGL VSync turned off.\n");
+  }
-        }
+
-      }
+#ifdef CONFIG_VSYNC_OPENGL
+  if (glx) {
+    // Get GLX context
+    ps->glx_context = glXCreateContext(ps->dpy, pvi, None, GL_TRUE);
+    if (!ps->glx_context) {
+      printf_errf("(): Failed to get GLX context.");
+      return false;
     }
-    if (pvi)
-      XFree(pvi);
 
-    if (!ret)
+    // Attach GLX context
-      ps->o.vsync = VSYNC_NONE;
+    if (!glXMakeCurrent(ps->dpy, ps->reg_win, ps->glx_context)) {
+      printf_errf("(): Failed to attach GLX context.");
+      return false;
+    }
   }
 #endif
 
-  if (!ps->reg_win)
+  if (pvi)
-    ps->reg_win = XCreateSimpleWindow(ps->dpy, ps->root, 0, 0, 1, 1, 0,
+    XFree(pvi);
-        None, None);
 
   Xutf8SetWMProperties(ps->dpy, ps->reg_win, "xcompmgr", "xcompmgr",
-    NULL, 0, NULL, NULL, NULL);
+      NULL, 0, NULL, NULL, NULL);
 
-  unsigned len = strlen(REGISTER_PROP) + 2;
+  {
-  int s = ps->scr;
+    unsigned len = strlen(REGISTER_PROP) + 2;
+    int s = ps->scr;
 
-  while (s >= 10) {
+    while (s >= 10) {
-    ++len;
+      ++len;
-    s /= 10;
+      s /= 10;
+    }
+
+    char *buf = malloc(len);
+    snprintf(buf, len, REGISTER_PROP "%d", ps->scr);
+    buf[len - 1] = '\0';
+    XSetSelectionOwner(ps->dpy, get_atom(ps, buf), ps->reg_win, 0);
+    free(buf);
   }
 
-  buf = malloc(len);
+  return true;
-  snprintf(buf, len, REGISTER_PROP"%d", ps->scr);
-
-  a = get_atom(ps, buf);
-  free(buf);
-
-  XSetSelectionOwner(ps->dpy, a, ps->reg_win, 0);
 }
 
 /**
@@ -4873,7 +4906,7 @@ 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) {
-    fprintf(stderr, "vsync_drm_init(): Failed to open device.\n");
+    printf_errf("(): Failed to open device.");
     return false;
   }
 
@@ -4882,7 +4915,7 @@ vsync_drm_init(session_t *ps) {
 
   return true;
 #else
-  fprintf(stderr, "Program not compiled with DRM VSync support.\n");
+  printf_errf("(): Program not compiled with DRM VSync support.");
   return false;
 #endif
 }
@@ -4927,19 +4960,38 @@ static bool
 vsync_opengl_init(session_t *ps) {
 #ifdef CONFIG_VSYNC_OPENGL
   // Get video sync functions
-  ps->glx_get_video_sync = (f_GetVideoSync)
+  ps->glXGetVideoSyncSGI = (f_GetVideoSync)
     glXGetProcAddress ((const GLubyte *) "glXGetVideoSyncSGI");
-  ps->glx_wait_video_sync = (f_WaitVideoSync)
+  ps->glXWaitVideoSyncSGI = (f_WaitVideoSync)
     glXGetProcAddress ((const GLubyte *) "glXWaitVideoSyncSGI");
-  if (!ps->glx_wait_video_sync || !ps->glx_get_video_sync) {
+  if (!ps->glXWaitVideoSyncSGI || !ps->glXGetVideoSyncSGI) {
-    fprintf(stderr, "vsync_opengl_init(): "
+    printf_errf("(): Failed to get glXWait/GetVideoSyncSGI function.");
-        "Failed to get glXWait/GetVideoSyncSGI function.\n");
     return false;
   }
 
   return true;
 #else
-  fprintf(stderr, "Program not compiled with OpenGL VSync support.\n");
+  printf_errfq(1, "Program not compiled with OpenGL VSync support.");
+  return false;
+#endif
+}
+
+static bool
+vsync_opengl_oml_init(session_t *ps) {
+#ifdef CONFIG_VSYNC_OPENGL
+  // Get video sync functions
+  ps->glXGetSyncValuesOML= (f_GetSyncValuesOML)
+    glXGetProcAddress ((const GLubyte *) "glXGetSyncValuesOML");
+  ps->glXWaitForMscOML = (f_WaitForMscOML)
+    glXGetProcAddress ((const GLubyte *) "glXWaitForMscOML");
+  if (!ps->glXGetSyncValuesOML || !ps->glXWaitForMscOML) {
+    printf_errf("(): Failed to get OML_sync_control functions.");
+    return false;
+  }
+
+  return true;
+#else
+  printf_errfq(1, "Program not compiled with OpenGL VSync support.");
   return false;
 #endif
 }
@@ -4948,13 +5000,31 @@ vsync_opengl_init(session_t *ps) {
 /**
  * Wait for next VSync, OpenGL method.
  */
-static void
+static int
 vsync_opengl_wait(session_t *ps) {
-  unsigned vblank_count;
+  unsigned vblank_count = 0;
 
-  ps->glx_get_video_sync(&vblank_count);
+  ps->glXGetVideoSyncSGI(&vblank_count);
-  ps->glx_wait_video_sync(2, (vblank_count + 1) % 2, &vblank_count);
+  ps->glXWaitVideoSyncSGI(2, (vblank_count + 1) % 2, &vblank_count);
   // I see some code calling glXSwapIntervalSGI(1) afterwards, is it required?
+
+  return 0;
+}
+
+/**
+ * Wait for next VSync, OpenGL OML method.
+ *
+ * https://mail.gnome.org/archives/clutter-list/2012-November/msg00031.html
+ */
+static int
+vsync_opengl_oml_wait(session_t *ps) {
+  int64_t ust = 0, msc = 0, sbc = 0;
+
+  ps->glXGetSyncValuesOML(ps->dpy, ps->reg_win, &ust, &msc, &sbc);
+  ps->glXWaitForMscOML(ps->dpy, ps->reg_win, 0, 2, (msc + 1) % 2,
+      &ust, &msc, &sbc);
+
+  return 0;
 }
 #endif
 
@@ -4963,27 +5033,13 @@ vsync_opengl_wait(session_t *ps) {
  */
 static void
 vsync_wait(session_t *ps) {
-  if (VSYNC_NONE == ps->o.vsync)
+  if (!ps->o.vsync)
     return;
 
-#ifdef CONFIG_VSYNC_DRM
+  assert(VSYNC_FUNCS_WAIT[ps->o.vsync]);
-  if (VSYNC_DRM == ps->o.vsync) {
-    vsync_drm_wait(ps);
-    return;
-  }
-#endif
 
-#ifdef CONFIG_VSYNC_OPENGL
+  if (VSYNC_FUNCS_WAIT[ps->o.vsync])
-  if (VSYNC_OPENGL == ps->o.vsync) {
+    VSYNC_FUNCS_WAIT[ps->o.vsync](ps);
-    vsync_opengl_wait(ps);
-    return;
-  }
-#endif
-
-  // This place should not reached!
-  assert(0);
-
-  return;
 }
 
 /**
@@ -5008,15 +5064,16 @@ init_alpha_picts(session_t *ps) {
 /**
  * Initialize double buffer.
  */
-static void
+static bool
 init_dbe(session_t *ps) {
   if (!(ps->root_dbe = XdbeAllocateBackBufferName(ps->dpy,
           (ps->o.paint_on_overlay ? ps->overlay: ps->root), XdbeCopied))) {
-    fprintf(stderr, "Failed to create double buffer. Double buffering "
+    printf_errf("(): Failed to create double buffer. Double buffering "
-        "turned off.\n");
+        "cannot work.");
-    ps->o.dbe = false;
+    return false;
-    return;
   }
+
+  return true;
 }
 
 /**
@@ -5090,6 +5147,10 @@ redir_start(session_t *ps) {
 
     XCompositeRedirectSubwindows(ps->dpy, ps->root, CompositeRedirectManual);
 
+    // Unredirect reg_win as this may have an effect on VSync:
+    // < http://dri.freedesktop.org/wiki/CompositeSwap >
+    XCompositeUnredirectWindow(ps->dpy, ps->reg_win, CompositeRedirectManual);
+
     // Must call XSync() here
     XSync(ps->dpy, False);
 
@@ -5478,8 +5539,10 @@ session_init(session_t *ps_old, int argc, char **argv) {
 
 #ifdef CONFIG_VSYNC_OPENGL
     .glx_context = None,
-    .glx_get_video_sync = NULL,
+    .glXGetVideoSyncSGI = NULL,
-    .glx_wait_video_sync = NULL,
+    .glXWaitVideoSyncSGI = NULL,
+    .glXGetSyncValuesOML = NULL,
+    .glXWaitForMscOML = NULL,
 #endif
 
     .xfixes_event = 0,
@@ -5567,6 +5630,9 @@ 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");
@@ -5609,21 +5675,22 @@ session_init(session_t *ps_old, int argc, char **argv) {
     if (XRRQueryExtension(ps->dpy, &ps->randr_event, &ps->randr_error))
       ps->randr_exists = true;
     else
-      fprintf(stderr, "No XRandR extension, automatic refresh rate "
+      printf_errf("(): No XRandR extension, automatic refresh rate "
-          "detection impossible.\n");
+          "detection impossible.");
   }
 
-#ifdef CONFIG_VSYNC_OPENGL
   // Query X GLX extension
-  if (VSYNC_OPENGL == ps->o.vsync) {
+  if (want_glx) {
+#ifdef CONFIG_VSYNC_OPENGL
     if (glXQueryExtension(ps->dpy, &ps->glx_event, &ps->glx_error))
       ps->glx_exists = true;
     else {
-      fprintf(stderr, "No GLX extension, OpenGL VSync impossible.\n");
+      printf_errfq(1, "(): No GLX extension, OpenGL VSync impossible.");
-      ps->o.vsync = VSYNC_NONE;
     }
-  }
+#else
+    printf_errfq(1, "(): OpenGL VSync support not compiled in.");
 #endif
+  }
 
   // Query X DBE extension
   if (ps->o.dbe) {
@@ -5641,23 +5708,24 @@ session_init(session_t *ps_old, int argc, char **argv) {
       ps->o.dbe = false;
   }
 
-  register_cm(ps, (VSYNC_OPENGL == ps->o.vsync));
+  if (!register_cm(ps, want_glx))
+    exit(1);
 
   // Initialize software optimization
   if (ps->o.sw_opti)
     ps->o.sw_opti = swopti_init(ps);
 
-  // Initialize DRM/OpenGL VSync
+  // Initialize VSync
-  if ((VSYNC_DRM == ps->o.vsync && !vsync_drm_init(ps))
+  if (ps->o.vsync && VSYNC_FUNCS_INIT[ps->o.vsync]
-      || (VSYNC_OPENGL == ps->o.vsync && !vsync_opengl_init(ps)))
+      && !VSYNC_FUNCS_INIT[ps->o.vsync](ps))
-    ps->o.vsync = VSYNC_NONE;
+    exit(1);
 
   // Overlay must be initialized before double buffer
   if (ps->o.paint_on_overlay)
     init_overlay(ps);
 
-  if (ps->o.dbe)
+  if (ps->o.dbe && !init_dbe(ps))
-    init_dbe(ps);
+    exit(1);
 
   init_atoms(ps);
   init_alpha_picts(ps);

src/compton.h

@@ -216,6 +216,25 @@ ms_to_tv(int timeout) {
   };
 }
 
+/**
+ * Create a XTextProperty of a single string.
+ */
+static inline XTextProperty *
+make_text_prop(session_t *ps, char *str) {
+  XTextProperty *pprop = malloc(sizeof(XTextProperty));
+  if (!pprop)
+    printf_errfq(1, "(): Failed to allocate memory.");
+
+  if (XmbTextListToTextProperty(ps->dpy, &str, 1,  XStringStyle, pprop)) {
+    if (pprop->value)
+      XFree(pprop->value);
+    free(pprop);
+    pprop = NULL;
+  }
+
+  return pprop;
+}
+
 static void
 run_fade(session_t *ps, win *w, unsigned steps);
 
@@ -685,8 +704,8 @@ ev_window(session_t *ps, XEvent *ev);
 static void __attribute__ ((noreturn))
 usage(void);
 
-static void
+static bool
-register_cm(session_t *ps, bool want_glxct);
+register_cm(session_t *ps, bool glx);
 
 inline static void
 ev_focus_in(session_t *ps, XFocusChangeEvent *ev);
@@ -878,9 +897,15 @@ vsync_drm_wait(session_t *ps);
 static bool
 vsync_opengl_init(session_t *ps);
 
+static bool
+vsync_opengl_oml_init(session_t *ps);
+
 #ifdef CONFIG_VSYNC_OPENGL
-static void
+static int
 vsync_opengl_wait(session_t *ps);
+
+static int
+vsync_opengl_oml_wait(session_t *ps);
 #endif
 
 static void
@@ -889,7 +914,7 @@ vsync_wait(session_t *ps);
 static void
 init_alpha_picts(session_t *ps);
 
-static void
+static bool
 init_dbe(session_t *ps);
 
 static void