Merge lp:~vanvugt/compiz-core/fix-763005 into lp:compiz-core/0.9.5

[Daniel van Vugt]

Fix slow/stuttering display when Sync To VBlank is enabled. (LP: #763005)

For fglrx the cause appears to be an ATI/AMD driver bug where glXGetVideoSyncSGI and glXWaitVideoSyncSGI are particularly slow, so we can't afford to call both on every redraw or we end up with 1/3-1/2 framerate. For intel and really all drivers, another cause appears to be excessive waiting in the compiz architecture not allowing enough CPU time to draw the full frame before the next vertical refresh.

This proposal addresses both possible problems, providing optimal framerates with sync-to-vblank support, most of the time.

[Didier Roche-Tolomelli (didrocks)]

Sam, can you look at this? Thanks!

[Sam Spilsbury (smspillaz)]

Sorry I got to this so late.

This looks good. Could you possibly retarget this against lp:compiz-core and make sure comments are on a new line ?

review: Approve

[Daniel van Vugt (vanvugt)]

Modified comment style as requested and retargeted to lp:compiz-core.

[Sam Spilsbury (smspillaz)]

Approve as per our discussion last time

review: Approve

=== modified file 'plugins/opengl/include/opengl/opengl.h'
--- plugins/opengl/include/opengl/opengl.h	2011-02-24 07:52:09 +0000
+++ plugins/opengl/include/opengl/opengl.h	2011-09-01 03:50:21 +0000
@@ -98,11 +98,13 @@
 					  int	  width,
 					  int	  height);
 
-    typedef int (*GLXGetVideoSyncProc)  (unsigned int *count);
     typedef int (*GLXWaitVideoSyncProc) (int	  divisor,
 					 int	  remainder,
 					 unsigned int *count);
 
+    // http://www.opengl.org/registry/specs/SGI/swap_control.txt
+    typedef int (*GLXSwapIntervalProc) (int interval);
+
     #ifndef GLX_VERSION_1_3
     typedef struct __GLXFBConfigRec *GLXFBConfig;
     #endif
@@ -163,8 +165,8 @@
     extern GLXReleaseTexImageProc   releaseTexImage;
     extern GLXQueryDrawableProc     queryDrawable;
     extern GLXCopySubBufferProc     copySubBuffer;
-    extern GLXGetVideoSyncProc      getVideoSync;
     extern GLXWaitVideoSyncProc     waitVideoSync;
+    extern GLXSwapIntervalProc      swapInterval;
     extern GLXGetFBConfigsProc      getFBConfigs;
     extern GLXGetFBConfigAttribProc getFBConfigAttrib;
     extern GLXCreatePixmapProc      createPixmap;

=== modified file 'plugins/opengl/src/privates.h'
--- plugins/opengl/src/privates.h	2009-03-16 09:18:16 +0000
+++ plugins/opengl/src/privates.h	2011-09-01 03:50:21 +0000
@@ -70,6 +70,7 @@
 
 	void prepareDrawing ();
 
+	void controlSwapVideoSync ();
 	void waitForVideoSync ();
 
 	void paintBackground (const CompRegion &region,

=== modified file 'plugins/opengl/src/screen.cpp'
--- plugins/opengl/src/screen.cpp	2011-02-24 07:52:09 +0000
+++ plugins/opengl/src/screen.cpp	2011-09-01 03:50:21 +0000
@@ -35,8 +35,8 @@
     GLXReleaseTexImageProc   releaseTexImage = NULL;
     GLXQueryDrawableProc     queryDrawable = NULL;
     GLXCopySubBufferProc     copySubBuffer = NULL;
-    GLXGetVideoSyncProc      getVideoSync = NULL;
     GLXWaitVideoSyncProc     waitVideoSync = NULL;
+    GLXSwapIntervalProc      swapInterval = NULL;
     GLXGetFBConfigsProc      getFBConfigs = NULL;
     GLXGetFBConfigAttribProc getFBConfigAttrib = NULL;
     GLXCreatePixmapProc      createPixmap = NULL;
@@ -217,13 +217,16 @@
 
     if (strstr (glxExtensions, "GLX_SGI_video_sync"))
     {
-	GL::getVideoSync = (GL::GLXGetVideoSyncProc)
-	    getProcAddress ("glXGetVideoSyncSGI");
-
 	GL::waitVideoSync = (GL::GLXWaitVideoSyncProc)
 	    getProcAddress ("glXWaitVideoSyncSGI");
     }
 
+    if (strstr (glxExtensions, "GLX_SGI_swap_control"))
+    {
+	GL::swapInterval = (GL::GLXSwapIntervalProc)
+	    getProcAddress ("glXSwapIntervalSGI");
+    }
+
     glXMakeCurrent (dpy, CompositeScreen::get (s)->output (), priv->ctx);
 
     glExtensions = (const char *) glGetString (GL_EXTENSIONS);
@@ -986,19 +989,28 @@
 }
 
 void
+PrivateGLScreen::controlSwapVideoSync ()
+{
+    bool sync = optionGetSyncToVblank ();
+    // Docs: http://www.opengl.org/registry/specs/SGI/swap_control.txt
+    if (GL::swapInterval)
+	(*GL::swapInterval) (sync ? 1 : 0);
+    else if (sync)
+	waitForVideoSync ();
+}
+
+void
 PrivateGLScreen::waitForVideoSync ()
 {
-    unsigned int sync;
-
-    if (!optionGetSyncToVblank ())
-	return;
-
-    if (GL::getVideoSync)
+    if (optionGetSyncToVblank () && GL::waitVideoSync)
     {
-	glFlush ();
+	// Don't wait twice. Just in case.
+	if (GL::swapInterval)
+	    (*GL::swapInterval) (0);
 
-	(*GL::getVideoSync) (&sync);
-	(*GL::waitVideoSync) (2, (sync + 1) % 2, &sync);
+	// Docs: http://www.opengl.org/registry/specs/SGI/video_sync.txt
+	unsigned int frameno;
+	(*GL::waitVideoSync) (1, 0, &frameno);
     }
 }
 
@@ -1071,10 +1083,23 @@
 
     targetOutput = &screen->outputDevs ()[0];
 
-    waitForVideoSync ();
+    glFlush ();
 
+    //
+    // FIXME: Actually fix the composite plugin to be more efficient;
+    // If GL::swapInterval == NULL && GL::waitVideoSync != NULL then the
+    // composite plugin should not be imposing any framerate restriction
+    // (ie. blocking the CPU) at all. Because the framerate will be controlled
+    // and optimized here:
+    //
     if (mask & COMPOSITE_SCREEN_DAMAGE_ALL_MASK)
     {
+	//
+	// controlSwapVideoSync is much faster than waitForVideoSync because
+	// it won't block the CPU. The waiting is offloaded to the GPU.
+	// Unfortunately it only works with glXSwapBuffers in most drivers.
+	//
+	controlSwapVideoSync ();
 	glXSwapBuffers (screen->dpy (), cScreen->output ());
     }
     else
@@ -1082,6 +1107,7 @@
 	BoxPtr pBox;
 	int    nBox, y;
 
+	waitForVideoSync ();
 	pBox = const_cast <Region> (tmpRegion.handle ())->rects;
 	nBox = const_cast <Region> (tmpRegion.handle ())->numRects;
 
@@ -1137,7 +1163,7 @@
 bool
 PrivateGLScreen::hasVSync ()
 {
-   return (GL::getVideoSync && optionGetSyncToVblank ());
+   return (GL::waitVideoSync && optionGetSyncToVblank ());
 }
 
 void
@@ -1145,7 +1171,8 @@
 {
     if (pendingCommands)
     {
-	glFinish ();
+	// glFlush! glFinish would block the CPU, which is bad.
+	glFlush ();
 	pendingCommands = false;
     }
 }