Condense GLX extension lookup

Put them all into one function, and move the function pointers out of
glx_session_t, making them global variables (because them don't change
after initialized). Remove the function pointer typedefs and replace
them with the ones in glxext.h

We also only lookup the functions once per a lifetime of compton.

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

src/backend/gl/glx.c

@@ -265,7 +265,7 @@ static void *glx_init(session_t *ps) {
 	}
 
 	// Ensure GLX_EXT_texture_from_pixmap exists
-	if (!glx_has_extension(ps->dpy, ps->scr, "GLX_EXT_texture_from_pixmap"))
+	if (!glxext.has_GLX_EXT_texture_from_pixmap)
 		goto end;
 
 	// Initialize GLX data structure
@@ -542,3 +542,87 @@ backend_info_t glx_backend = {
     .buffer_age = glx_buffer_age,
     .max_buffer_age = 5,        // XXX why?
 };
+
+/**
+ * Check if a GLX extension exists.
+ */
+static inline bool glx_has_extension(Display *dpy, int screen, const char *ext) {
+	const char *glx_exts = glXQueryExtensionsString(dpy, screen);
+	if (!glx_exts) {
+		log_error("Failed get GLX extension list.");
+		return false;
+	}
+
+	long inlen = strlen(ext);
+	const char *curr = glx_exts;
+	bool match = false;
+	while (curr && !match) {
+		const char *end = strchr(curr, ' ');
+		if (!end) {
+			// Last extension string
+			match = strcmp(ext, curr) == 0;
+		} else if (end - curr == inlen) {
+			// Length match, do match string
+			match = strncmp(ext, curr, end - curr) == 0;
+		}
+		curr = end ? end + 1 : NULL;
+	}
+
+	if (!match) {
+		log_info("Missing GLX extension %s.", ext);
+	} else {
+		log_info("Found GLX extension %s.", ext);
+	}
+
+	return match;
+}
+
+struct glxext_info glxext = {0};
+PFNGLXGETVIDEOSYNCSGIPROC glXGetVideoSyncSGI;
+PFNGLXWAITVIDEOSYNCSGIPROC glXWaitVideoSyncSGI;
+PFNGLXGETSYNCVALUESOMLPROC glXGetSyncValuesOML;
+PFNGLXWAITFORMSCOMLPROC glXWaitForMscOML;
+PFNGLXSWAPINTERVALEXTPROC glXSwapIntervalEXT;
+PFNGLXSWAPINTERVALSGIPROC glXSwapIntervalSGI;
+PFNGLXSWAPINTERVALMESAPROC glXSwapIntervalMESA;
+PFNGLXBINDTEXIMAGEEXTPROC glXBindTexImageEXT;
+PFNGLXRELEASETEXIMAGEEXTPROC glXReleaseTexImageEXT;
+
+void glxext_init(Display *dpy, int screen) {
+	if (glxext.initialized) {
+		return;
+	}
+	glxext.initialized = true;
+#define check_ext(name) glxext.has_##name = glx_has_extension(dpy, screen, #name)
+	check_ext(GLX_SGI_video_sync);
+	check_ext(GLX_SGI_swap_control);
+	check_ext(GLX_OML_sync_control);
+	check_ext(GLX_MESA_swap_control);
+	check_ext(GLX_EXT_swap_control);
+	check_ext(GLX_EXT_texture_from_pixmap);
+#undef check_ext
+
+#define lookup(name) (name = (__typeof__(name))glXGetProcAddress((GLubyte *)#name))
+	// Checking if the returned function pointer is NULL is not really necessary,
+	// or maybe not even useful, since glXGetProcAddress might always return something.
+	// We are doing it just for completeness' sake.
+	if (!lookup(glXGetVideoSyncSGI) || !lookup(glXWaitVideoSyncSGI)) {
+		glxext.has_GLX_SGI_video_sync = false;
+	}
+	if (!lookup(glXSwapIntervalEXT)) {
+		glxext.has_GLX_EXT_swap_control = false;
+	}
+	if (!lookup(glXSwapIntervalMESA)) {
+		glxext.has_GLX_MESA_swap_control = false;
+	}
+	if (!lookup(glXSwapIntervalSGI)) {
+		glxext.has_GLX_SGI_swap_control = false;
+	}
+	if (!lookup(glXWaitForMscOML) || !lookup(glXGetSyncValuesOML)) {
+		glxext.has_GLX_OML_sync_control = false;
+	}
+	if (!lookup(glXBindTexImageEXT) || !lookup(glXReleaseTexImageEXT)) {
+		glxext.has_GLX_EXT_texture_from_pixmap = false;
+	}
+#undef lookup
+}

src/backend/gl/glx.h

@@ -1,7 +1,9 @@
 // SPDX-License-Identifier: MPL-2.0
 // Copyright (c) Yuxuan Shui <yshuiv7@gmail.com>
 #pragma once
+#include <stdbool.h>
 #include <GL/glx.h>
+#include <GL/glxext.h>
 #include <X11/Xlib.h>
 #include <xcb/xcb.h>
 #include <xcb/render.h>
@@ -64,36 +66,26 @@ x_visual_to_fbconfig_criteria(xcb_connection_t *c, xcb_visualid_t visual) {
 	};
 }
 
-/**
+struct glxext_info {
- * Check if a GLX extension exists.
+	bool initialized;
- */
+	bool has_GLX_SGI_video_sync;
-static inline bool glx_has_extension(Display *dpy, int screen, const char *ext) {
+	bool has_GLX_SGI_swap_control;
-	const char *glx_exts = glXQueryExtensionsString(dpy, screen);
+	bool has_GLX_OML_sync_control;
-	if (!glx_exts) {
+	bool has_GLX_MESA_swap_control;
-		log_error("Failed get GLX extension list.");
+	bool has_GLX_EXT_swap_control;
-		return false;
+	bool has_GLX_EXT_texture_from_pixmap;
-	}
+};
 
-	long inlen = strlen(ext);
+extern struct glxext_info glxext;
-	const char *curr = glx_exts;
-	bool match = false;
-	while (curr && !match) {
-		const char *end = strchr(curr, ' ');
-		if (!end) {
-			// Last extension string
-			match = strcmp(ext, curr) == 0;
-		} else if (end - curr == inlen) {
-			// Length match, do match string
-			match = strncmp(ext, curr, end - curr) == 0;
-		}
-		curr = end ? end + 1 : NULL;
-	}
 
-	if (!match) {
+extern PFNGLXGETVIDEOSYNCSGIPROC glXGetVideoSyncSGI;
-		log_info("Missing GLX extension %s.", ext);
+extern PFNGLXWAITVIDEOSYNCSGIPROC glXWaitVideoSyncSGI;
-	} else {
+extern PFNGLXGETSYNCVALUESOMLPROC glXGetSyncValuesOML;
-		log_info("Found GLX extension %s.", ext);
+extern PFNGLXWAITFORMSCOMLPROC glXWaitForMscOML;
-	}
+extern PFNGLXSWAPINTERVALEXTPROC glXSwapIntervalEXT;
+extern PFNGLXSWAPINTERVALSGIPROC glXSwapIntervalSGI;
+extern PFNGLXSWAPINTERVALMESAPROC glXSwapIntervalMESA;
+extern PFNGLXBINDTEXIMAGEEXTPROC glXBindTexImageEXT;
+extern PFNGLXRELEASETEXIMAGEEXTPROC glXReleaseTexImageEXT;
 
-	return match;
+void glxext_init(Display *, int screen);
-}

src/common.h

@@ -189,18 +189,6 @@ typedef void (*GLDEBUGPROC) (GLenum source, GLenum type,
 typedef void (*f_DebugMessageCallback) (GLDEBUGPROC, void *userParam);
 #endif
 
-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);
-
-typedef int (*f_SwapIntervalSGI) (int interval);
-typedef int (*f_SwapIntervalMESA) (unsigned int interval);
-
-typedef void (*f_BindTexImageEXT) (Display *display, GLXDrawable drawable, int buffer, const int *attrib_list);
-typedef void (*f_ReleaseTexImageEXT) (Display *display, GLXDrawable drawable, int buffer);
-
 #ifdef CONFIG_OPENGL
 typedef GLsync (*f_FenceSync) (GLenum condition, GLbitfield flags);
 typedef GLboolean (*f_IsSync) (GLsync sync);
@@ -279,22 +267,6 @@ typedef struct {
   GLXContext context;
   /// Whether we have GL_ARB_texture_non_power_of_two.
   bool has_texture_non_power_of_two;
-  /// Pointer to glXGetVideoSyncSGI function.
-  f_GetVideoSync glXGetVideoSyncSGI;
-  /// Pointer to glXWaitVideoSyncSGI function.
-  f_WaitVideoSync glXWaitVideoSyncSGI;
-   /// Pointer to glXGetSyncValuesOML function.
-  f_GetSyncValuesOML glXGetSyncValuesOML;
-  /// Pointer to glXWaitForMscOML function.
-  f_WaitForMscOML glXWaitForMscOML;
-  /// Pointer to glXSwapIntervalSGI function.
-  f_SwapIntervalSGI glXSwapIntervalProc;
-  /// Pointer to glXSwapIntervalMESA function.
-  f_SwapIntervalMESA glXSwapIntervalMESAProc;
-  /// Pointer to glXBindTexImageEXT function.
-  f_BindTexImageEXT glXBindTexImageProc;
-  /// Pointer to glXReleaseTexImageEXT function.
-  f_ReleaseTexImageEXT glXReleaseTexImageProc;
   /// Pointer to the glFenceSync() function.
   f_FenceSync glFenceSyncProc;
   /// Pointer to the glIsSync() function.

src/opengl.c

@@ -83,7 +83,7 @@ glx_init(session_t *ps, bool need_render) {
   }
 
   // Ensure GLX_EXT_texture_from_pixmap exists
-  if (need_render && !glx_has_extension(ps->dpy, ps->scr, "GLX_EXT_texture_from_pixmap"))
+  if (need_render && !glxext.has_GLX_EXT_texture_from_pixmap)
     goto glx_init_end;
 
   // Initialize GLX data structure
@@ -175,18 +175,6 @@ glx_init(session_t *ps, bool need_render) {
     psglx->has_texture_non_power_of_two = gl_has_extension(
         "GL_ARB_texture_non_power_of_two");
 
-  // Acquire function addresses
-  if (need_render) {
-    psglx->glXBindTexImageProc = (f_BindTexImageEXT)
-      glXGetProcAddress((const GLubyte *) "glXBindTexImageEXT");
-    psglx->glXReleaseTexImageProc = (f_ReleaseTexImageEXT)
-      glXGetProcAddress((const GLubyte *) "glXReleaseTexImageEXT");
-    if (!psglx->glXBindTexImageProc || !psglx->glXReleaseTexImageProc) {
-      log_error("Failed to acquire glXBindTexImageEXT() / glXReleaseTexImageEXT().");
-      goto glx_init_end;
-    }
-  }
-
   // Render preparations
   if (need_render) {
     glx_on_root_change(ps);
@@ -626,9 +614,9 @@ glx_bind_pixmap(session_t *ps, glx_texture_t **pptex, xcb_pixmap_t pixmap,
   // The specification requires rebinding whenever the content changes...
   // We can't follow this, too slow.
   if (need_release)
-    ps->psglx->glXReleaseTexImageProc(ps->dpy, ptex->glpixmap, GLX_FRONT_LEFT_EXT);
+    glXReleaseTexImageEXT(ps->dpy, ptex->glpixmap, GLX_FRONT_LEFT_EXT);
 
-  ps->psglx->glXBindTexImageProc(ps->dpy, ptex->glpixmap, GLX_FRONT_LEFT_EXT, NULL);
+  glXBindTexImageEXT(ps->dpy, ptex->glpixmap, GLX_FRONT_LEFT_EXT, NULL);
 
   // Cleanup
   glBindTexture(ptex->target, 0);
@@ -647,7 +635,7 @@ glx_release_pixmap(session_t *ps, glx_texture_t *ptex) {
   // Release binding
   if (ptex->glpixmap && ptex->texture) {
     glBindTexture(ptex->target, ptex->texture);
-    ps->psglx->glXReleaseTexImageProc(ps->dpy, ptex->glpixmap, GLX_FRONT_LEFT_EXT);
+    glXReleaseTexImageEXT(ps->dpy, ptex->glpixmap, GLX_FRONT_LEFT_EXT);
     glBindTexture(ptex->target, 0);
   }
 

src/render.c

@@ -1121,6 +1121,9 @@ static bool init_alpha_picts(session_t *ps) {
 
 bool init_render(session_t *ps) {
 	// Initialize OpenGL as early as possible
+#ifdef CONFIG_OPENGL
+	glxext_init(ps->dpy, ps->scr);
+#endif
 	if (bkend_use_glx(ps)) {
 #ifdef CONFIG_OPENGL
 		if (!glx_init(ps, true))

src/vsync.c

@@ -90,24 +90,7 @@ vsync_opengl_init(session_t *ps) {
   if (!ensure_glx_context(ps))
     return false;
 
-  if (!glx_has_extension(ps->dpy, ps->scr, "GLX_SGI_video_sync")) {
+  return glxext.has_GLX_SGI_video_sync;
-    log_error("Your driver doesn't support SGI_video_sync, giving up.");
-    return false;
-  }
-
-  // Get video sync functions
-  if (!ps->psglx->glXGetVideoSyncSGI)
-    ps->psglx->glXGetVideoSyncSGI = (f_GetVideoSync)
-      glXGetProcAddress((const GLubyte *) "glXGetVideoSyncSGI");
-  if (!ps->psglx->glXWaitVideoSyncSGI)
-    ps->psglx->glXWaitVideoSyncSGI = (f_WaitVideoSync)
-      glXGetProcAddress((const GLubyte *) "glXWaitVideoSyncSGI");
-  if (!ps->psglx->glXWaitVideoSyncSGI || !ps->psglx->glXGetVideoSyncSGI) {
-    log_error("Failed to get glXWait/GetVideoSyncSGI function.");
-    return false;
-  }
-
-  return true;
 #else
   log_error("compton is not compiled with OpenGL VSync support.");
   return false;
@@ -120,59 +103,32 @@ vsync_opengl_oml_init(session_t *ps) {
   if (!ensure_glx_context(ps))
     return false;
 
-  if (!glx_has_extension(ps->dpy, ps->scr, "GLX_OML_sync_control")) {
+  return glxext.has_GLX_OML_sync_control;
-    log_error("Your driver doesn't support OML_sync_control, giving up.");
-    return false;
-  }
-
-  // Get video sync functions
-  if (!ps->psglx->glXGetSyncValuesOML)
-    ps->psglx->glXGetSyncValuesOML = (f_GetSyncValuesOML)
-      glXGetProcAddress ((const GLubyte *) "glXGetSyncValuesOML");
-  if (!ps->psglx->glXWaitForMscOML)
-    ps->psglx->glXWaitForMscOML = (f_WaitForMscOML)
-      glXGetProcAddress ((const GLubyte *) "glXWaitForMscOML");
-  if (!ps->psglx->glXGetSyncValuesOML || !ps->psglx->glXWaitForMscOML) {
-    log_error("Failed to get OML_sync_control functions.");
-    return false;
-  }
-
-  return true;
 #else
   log_error("compton is not compiled with OpenGL VSync support.");
   return false;
 #endif
 }
 
-static bool
-vsync_opengl_swc_swap_interval(session_t *ps, unsigned int interval) {
 #ifdef CONFIG_OPENGL
-  if (!ensure_glx_context(ps))
+static inline bool
-    return false;
+vsync_opengl_swc_swap_interval(session_t *ps, unsigned int interval) {
-
+  if (glxext.has_GLX_MESA_swap_control)
-  if (!ps->psglx->glXSwapIntervalProc && !ps->psglx->glXSwapIntervalMESAProc) {
+    return glXSwapIntervalMESA(interval) == 0;
-    if (glx_has_extension(ps->dpy, ps->scr, "GLX_MESA_swap_control")) {
+  else if (glxext.has_GLX_SGI_swap_control)
-      ps->psglx->glXSwapIntervalMESAProc = (f_SwapIntervalMESA)
+    return glXSwapIntervalSGI(interval) == 0;
-        glXGetProcAddress ((const GLubyte *) "glXSwapIntervalMESA");
+  else if (glxext.has_GLX_EXT_swap_control) {
-    } else if (glx_has_extension(ps->dpy, ps->scr, "GLX_SGI_swap_control")) {
+    GLXDrawable d = glXGetCurrentDrawable();
-      ps->psglx->glXSwapIntervalProc = (f_SwapIntervalSGI)
+    if (d == None) {
-        glXGetProcAddress ((const GLubyte *) "glXSwapIntervalSGI");
+      // We don't have a context??
-    } else {
-      log_error("Your driver doesn't support SGI_swap_control nor MESA_swap_control, giving up.");
       return false;
     }
+    glXSwapIntervalEXT(ps->dpy, glXGetCurrentDrawable(), interval);
+    return true;
   }
-
+  return false;
-  if (ps->psglx->glXSwapIntervalMESAProc)
-    ps->psglx->glXSwapIntervalMESAProc(interval);
-  else if (ps->psglx->glXSwapIntervalProc)
-    ps->psglx->glXSwapIntervalProc(interval);
-  else
-    return false;
-#endif
-
-  return true;
 }
+#endif
 
 static bool
 vsync_opengl_swc_init(session_t *ps) {
@@ -216,8 +172,8 @@ static int
 vsync_opengl_wait(session_t *ps) {
   unsigned vblank_count = 0;
 
-  ps->psglx->glXGetVideoSyncSGI(&vblank_count);
+  glXGetVideoSyncSGI(&vblank_count);
-  ps->psglx->glXWaitVideoSyncSGI(2, (vblank_count + 1) % 2, &vblank_count);
+  glXWaitVideoSyncSGI(2, (vblank_count + 1) % 2, &vblank_count);
   // I see some code calling glXSwapIntervalSGI(1) afterwards, is it required?
 
   return 0;
@@ -232,8 +188,8 @@ static int
 vsync_opengl_oml_wait(session_t *ps) {
   int64_t ust = 0, msc = 0, sbc = 0;
 
-  ps->psglx->glXGetSyncValuesOML(ps->dpy, ps->reg_win, &ust, &msc, &sbc);
+  glXGetSyncValuesOML(ps->dpy, ps->reg_win, &ust, &msc, &sbc);
-  ps->psglx->glXWaitForMscOML(ps->dpy, ps->reg_win, 0, 2, (msc + 1) % 2,
+  glXWaitForMscOML(ps->dpy, ps->reg_win, 0, 2, (msc + 1) % 2,
       &ust, &msc, &sbc);
 
   return 0;