2007-06-14 Matthew Allum <mallum@openedhand.com>

* clutter/cogl/gl/cogl.c: (cogl_check_extension):
        Actually populate this func and remove the static alternate
        named one. Means GLX actually checks for available extensions.
        Other minor tidy ups.

        * clutter/glx/clutter-backend-glx.c:
        * clutter/glx/clutter-backend-glx.h:
        Add support for GLX_SGI_swap_control to do vblanking prefering
        over glXGetVideoSyncSGI. Should fix issues on Intel chips with
        very slow frame rates due to vblank problems.
        Thanks to  Michel Danzer for tips, see;
        http://bugs.freedesktop.org/show_bug.cgi?id=10542

        * test/test-actors.c:
        Change FPS to 60.

ChangeLog

@@ -1,3 +1,21 @@
+2007-06-14  Matthew Allum  <mallum@openedhand.com>
+
+	* clutter/cogl/gl/cogl.c: (cogl_check_extension):
+	Actually populate this func and remove the static alternate
+	named one. Means GLX actually checks for available extensions.
+	Other minor tidy ups.
+	
+	* clutter/glx/clutter-backend-glx.c:
+	* clutter/glx/clutter-backend-glx.h:
+	Add support for GLX_SGI_swap_control to do vblanking prefering 
+        over glXGetVideoSyncSGI. Should fix issues on Intel chips with 
+        very slow frame rates due to vblank problems. 
+	Thanks to  Michel Danzer for tips, see;
+        http://bugs.freedesktop.org/show_bug.cgi?id=10542  
+
+	* test/test-actors.c:
+	Change FPS to 60.
+
 2007-06-14  Emmanuele Bassi  <ebassi@openedhand.com>
 
 	* clutter/clutter-timeout-pool.c: Make ClutterTimeoutPool

clutter/cogl/gl/cogl.c

@@ -78,9 +78,15 @@ error_string(GLenum errorCode)
 #define GE(x) (x);
 #endif
 
-static gboolean 
+CoglFuncPtr
-check_gl_extension (const gchar *name,
+cogl_get_proc_address (const gchar* name)
-                    const gchar *ext)
+{
+  /* FIXME: This very likely needs to be handled in the backend */
+  return NULL;
+}
+
+gboolean 
+cogl_check_extension (const gchar *name, const gchar *ext)
 {
   gchar *end;
   gint name_len, n;
@@ -104,40 +110,6 @@ check_gl_extension (const gchar *name,
   return FALSE;
 }
 
-#if 0
-static gboolean 
-is_gl_version_at_least_12 (void)
-{
-  /* FIXME: This likely needs to live elsewhere in features or cogl */
-  return 
-    (g_ascii_strtod ((const gchar*) glGetString (GL_VERSION), NULL) >= 1.2);
-
-
-  /* At least GL 1.2 is needed for CLAMP_TO_EDGE */
-  /* FIXME: move to cogl... */
-  if (!is_gl_version_at_least_12 ())
-    {
-      g_set_error (error, CLUTTER_INIT_ERROR,
-                   CLUTTER_INIT_ERROR_BACKEND,
-                   "Clutter needs at least version 1.2 of OpenGL");
-      return FALSE;
-    }
-}
-#endif
-
-CoglFuncPtr
-cogl_get_proc_address (const gchar* name)
-{
-  /* FIXME */
-  return NULL;
-}
-
-gboolean 
-cogl_check_extension (const gchar *name, const gchar *ext)
-{
-  return FALSE;
-}
-
 void
 cogl_paint_init (const ClutterColor *color)
 {
@@ -552,34 +524,18 @@ cogl_get_features ()
 
   gl_extensions = (const gchar*) glGetString (GL_EXTENSIONS);
 
-  if (check_gl_extension ("GL_ARB_texture_rectangle", gl_extensions) ||
+  if (cogl_check_extension ("GL_ARB_texture_rectangle", gl_extensions) ||
-      check_gl_extension ("GL_EXT_texture_rectangle", gl_extensions))
+      cogl_check_extension ("GL_EXT_texture_rectangle", gl_extensions))
     {
       flags |= CLUTTER_FEATURE_TEXTURE_RECTANGLE;
     }
 
 #ifdef GL_YCBCR_MESA
-  if (check_gl_extension ("GL_MESA_ycbcr_texture", gl_extensions))
+  if (cogl_check_extension ("GL_MESA_ycbcr_texture", gl_extensions))
     {
       flags |= CLUTTER_FEATURE_TEXTURE_YUV;
     }
 #endif
-
-#if 0
-  CLUTTER_NOTE (GL,
-		"\n"
-		"===========================================\n"
-		"GL_VENDOR: %s\n"
-		"GL_RENDERER: %s\n"
-		"GL_VERSION: %s\n"
-		"GL_EXTENSIONS: %s\n"
-		"===========================================\n",
-		glGetString (GL_VENDOR),
-		glGetString (GL_RENDERER),
-		glGetString (GL_VERSION),
-		glGetString (GL_EXTENSIONS),
-		: "no");
-#endif
   
   return flags;
 }

clutter/glx/clutter-backend-glx.c

@@ -415,19 +415,69 @@ clutter_backend_glx_get_features (ClutterBackend *backend)
 
   /* FIXME: we really need to check if gl context is set */
 
+  CLUTTER_NOTE (BACKEND, "Checking features\n"
+		"GL_VENDOR: %s\n"
+		"GL_RENDERER: %s\n"
+		"GL_VERSION: %s\n"
+		"GL_EXTENSIONS: %s\n",
+		glGetString (GL_VENDOR),
+		glGetString (GL_RENDERER),
+		glGetString (GL_VERSION),
+		glGetString (GL_EXTENSIONS));
+
   glx_extensions 
     = glXQueryExtensionsString (clutter_glx_get_default_display (),
 				clutter_glx_get_default_screen ());
 
+  CLUTTER_NOTE (BACKEND, "GLX Extensions: %s", glx_extensions);
+
+  /* First check for explicit disabling or it set elsewhere (eg NVIDIA) */
   if (getenv("__GL_SYNC_TO_VBLANK") || check_vblank_env ("none"))
     {
-      CLUTTER_NOTE (MISC, "vblank sync: disabled at user request");
+      CLUTTER_NOTE (BACKEND, "vblank sync: disabled at user request");
     }
   else
     {
+      /* We try two GL vblank syncing mechanisms.  
+       * glXSwapIntervalSGI is tried first, then glXGetVideoSyncSGI.
+       *
+       * glXSwapIntervalSGI is known to work with Mesa and in particular
+       * the Intel drivers. glXGetVideoSyncSGI has serious problems with
+       * Intel drivers causing terrible frame rate so it only tried as a
+       * fallback.
+       *
+       * How well glXGetVideoSyncSGI works with other driver (ATI etc) needs
+       * to be investigated. glXGetVideoSyncSGI on ATI at least seems to have
+       * no effect.
+      */
       if (!check_vblank_env ("dri") && 
+	  cogl_check_extension ("GLX_SGI_swap_control", glx_extensions))
+	{
+	  backend_glx->swap_interval = 
+	    (SwapIntervalProc) get_proc_address ("glXSwapIntervalSGI");
+
+	  CLUTTER_NOTE (BACKEND, "attempting glXSwapIntervalSGI vblank setup");
+
+	  if (backend_glx->swap_interval != NULL)
+	    {
+	      if (backend_glx->swap_interval (1) == 0)
+		{
+		  backend_glx->vblank_type = CLUTTER_VBLANK_GLX_SWAP;
+		  flags |= CLUTTER_FEATURE_SYNC_TO_VBLANK;
+		  CLUTTER_NOTE (BACKEND, "glXSwapIntervalSGI setup success");
+		}
+	    }
+
+	  if (!(flags & CLUTTER_FEATURE_SYNC_TO_VBLANK))
+	    CLUTTER_NOTE (BACKEND, "glXSwapIntervalSGI vblank setup failed");
+	}
+
+      if (!check_vblank_env ("dri") && 
+	  !(flags & CLUTTER_FEATURE_SYNC_TO_VBLANK) &&
 	  cogl_check_extension ("GLX_SGI_video_sync", glx_extensions))
 	{
+	  CLUTTER_NOTE (BACKEND, "attempting glXGetVideoSyncSGI vblank setup");
+
 	  backend_glx->get_video_sync = 
 	    (GetVideoSyncProc) get_proc_address ("glXGetVideoSyncSGI");
 
@@ -437,29 +487,39 @@ clutter_backend_glx_get_features (ClutterBackend *backend)
 	  if ((backend_glx->get_video_sync != NULL) &&
 	      (backend_glx->wait_video_sync != NULL))
 	    {
-	      CLUTTER_NOTE (MISC, "vblank sync: using glx");
+	      CLUTTER_NOTE (BACKEND, 
+			    "glXGetVideoSyncSGI vblank setup success");
 	      
               backend_glx->vblank_type = CLUTTER_VBLANK_GLX;
 	      flags |= CLUTTER_FEATURE_SYNC_TO_VBLANK;
 	    }
+
+	  if (!(flags & CLUTTER_FEATURE_SYNC_TO_VBLANK))
+	    CLUTTER_NOTE (BACKEND, "glXGetVideoSyncSGI vblank setup failed");
 	}
 #ifdef __linux__
+      /* 
+       * DRI is really an extreme fallback -rumoured to work with Via chipsets
+      */
       if (!(flags & CLUTTER_FEATURE_SYNC_TO_VBLANK))
 	{
+	  CLUTTER_NOTE (BACKEND, "attempting DRI vblank setup");
 	  backend_glx->dri_fd = open("/dev/dri/card0", O_RDWR);
 	  if (backend_glx->dri_fd >= 0)
 	    {
-	      CLUTTER_NOTE (MISC, "vblank sync: using dri");
+	      CLUTTER_NOTE (BACKEND, "DRI vblank setup success");
-
 	      backend_glx->vblank_type = CLUTTER_VBLANK_DRI;
 	      flags |= CLUTTER_FEATURE_SYNC_TO_VBLANK;
 	    }
+
+	  if (!(flags & CLUTTER_FEATURE_SYNC_TO_VBLANK))
+	    CLUTTER_NOTE (BACKEND, "DRI vblank setup failed");
 	}
 #endif
       if (!(flags & CLUTTER_FEATURE_SYNC_TO_VBLANK))
         {
-          CLUTTER_NOTE (MISC,
+          CLUTTER_NOTE (BACKEND,
-                        "vblank sync: no use-able mechanism found");
+                        "no use-able vblank mechanism found");
         }
     }
 
@@ -545,6 +605,11 @@ clutter_backend_glx_wait_for_vblank (ClutterBackendGLX *backend_glx)
 {
   switch (backend_glx->vblank_type)
     {
+    case CLUTTER_VBLANK_GLX_SWAP:
+      {
+	/* Nothing */
+	break;
+      }
     case CLUTTER_VBLANK_GLX:
       {
 	unsigned int retraceCount;

clutter/glx/clutter-backend-glx.h

@@ -47,6 +47,7 @@ typedef struct _ClutterBackendGLXClass  ClutterBackendGLXClass;
 typedef enum ClutterGLXVBlankType
 {
   CLUTTER_VBLANK_NONE = 0,
+  CLUTTER_VBLANK_GLX_SWAP,
   CLUTTER_VBLANK_GLX,
   CLUTTER_VBLANK_DRI
 
@@ -56,6 +57,7 @@ typedef int (*GetVideoSyncProc)  (unsigned int *count);
 typedef int (*WaitVideoSyncProc) (int           divisor,
 				  int          remainder,
 				  unsigned int *count);
+typedef int (*SwapIntervalProc) (int interval);
 
 typedef struct _ClutterGLXEventFilter
 {
@@ -82,9 +84,10 @@ struct _ClutterBackendGLX
   GSList  *event_filters;
 
   /* Vblank stuff */
-  GetVideoSyncProc  get_video_sync;
+  GetVideoSyncProc       get_video_sync;
-  WaitVideoSyncProc wait_video_sync;
+  WaitVideoSyncProc      wait_video_sync;
-  gint                dri_fd;
+  SwapIntervalProc       swap_interval;
+  gint                   dri_fd;
   ClutterGLXVBlankType   vblank_type;
 };
 

tests/test-actors.c

@@ -151,7 +151,7 @@ main (int argc, char *argv[])
   oh = g_new(SuperOH, 1);
 
   /* Create a timeline to manage animation */
-  timeline = clutter_timeline_new (360, 120); /* num frames, fps */
+  timeline = clutter_timeline_new (360, 60); /* num frames, fps */
   g_object_set(timeline, "loop", TRUE, 0);   /* have it loop */
 
   /* fire a callback for frame change */