compiz team mailing list archive

[Daniel van Vugt <vanvugt@xxxxxxxxx>]

Daniel van Vugt has proposed merging lp:~vanvugt/compiz/fix-763005-trunk into lp:compiz.

Requested reviews:
  compiz packagers (compiz)
  Sam Spilsbury (smspillaz)
Related bugs:
  Bug #763005 in fglrx-installer (Ubuntu): "Compiz's "Sync to Vblank" makes display stutter/slow with some drivers (like fglrx)"
  https://bugs.launchpad.net/ubuntu/+source/fglrx-installer/+bug/763005

For more details, see:
https://code.launchpad.net/~vanvugt/compiz/fix-763005-trunk/+merge/71307

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 patch addresses both possible problems, providing optimal framerates with sync-to-vblank support, most of the time.

It is possible this might also fix similar nvidia issues where DynamicTwinView reduces the framerate; bug 762749 and maybe bug 92599...
-- 
https://code.launchpad.net/~vanvugt/compiz/fix-763005-trunk/+merge/71307
Your team compiz packagers is requested to review the proposed merge of lp:~vanvugt/compiz/fix-763005-trunk into lp:compiz.

=== added file 'debian/patches/fix_slow_vsync_lp763005.patch'
--- debian/patches/fix_slow_vsync_lp763005.patch	1970-01-01 00:00:00 +0000
+++ debian/patches/fix_slow_vsync_lp763005.patch	2011-08-12 07:46:22 +0000
@@ -0,0 +1,175 @@
+Description: Fix slow/stuttering display when sync to Vblank is enabled.
+ 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 patch addresses both possible problems, providing optimal framerates
+ with sync-to-vblank support, most of the time.
+Author: Daniel van Vugt <vanvugt@xxxxxxxxx>
+Bug-Ubuntu: https://launchpad.net/bugs/763005
+
+===================================================================
+--- old/plugins/opengl/include/opengl/opengl.h	2011-08-12 15:00:38.809906000 +0800
++++ new/plugins/opengl/include/opengl/opengl.h	2011-08-12 15:09:05.000000000 +0800
+@@ -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;
+===================================================================
+--- old/plugins/opengl/src/screen.cpp	2011-08-12 15:00:38.809906000 +0800
++++ new/plugins/opengl/src/screen.cpp	2011-08-12 15:09:05.000000000 +0800
+@@ -36,8 +36,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;
+@@ -227,13 +227,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);
+@@ -1002,19 +1005,30 @@
+ }
+ 
+ void
+-PrivateGLScreen::waitForVideoSync ()
++PrivateGLScreen::controlSwapVideoSync ()
+ {
+-    unsigned int sync;
+-
+-    if (!optionGetSyncToVblank ())
+-	return;
++    bool sync = optionGetSyncToVblank ();
++    if (GL::swapInterval)
++    {   // Docs: http://www.opengl.org/registry/specs/SGI/swap_control.txt
++	(*GL::swapInterval) (sync ? 1 : 0);
++    }
++    else if (sync)
++    {   // ugly fallback - blocks the CPU.
++	waitForVideoSync ();
++    }
++}
+ 
+-    if (GL::getVideoSync)
++void
++PrivateGLScreen::waitForVideoSync ()
++{
++    if (optionGetSyncToVblank () && GL::waitVideoSync)
+     {
+-	glFlush ();
++	if (GL::swapInterval)
++	    (*GL::swapInterval) (0); // Don't wait twice. Just in case.
+ 
+-	(*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);
+     }
+ }
+ 
+@@ -1087,10 +1101,23 @@
+ 
+     targetOutput = &screen->outputDevs ()[0];
+ 
+-    waitForVideoSync ();
++    glFlush ();  // used to be called from waitForVideoSync
+ 
++    //
++    // 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
+@@ -1098,6 +1125,7 @@
+ 	BoxPtr pBox;
+ 	int    nBox, y;
+ 
++	waitForVideoSync ();
+ 	pBox = const_cast <Region> (tmpRegion.handle ())->rects;
+ 	nBox = const_cast <Region> (tmpRegion.handle ())->numRects;
+ 
+@@ -1153,7 +1181,7 @@
+ bool
+ PrivateGLScreen::hasVSync ()
+ {
+-   return (GL::getVideoSync && optionGetSyncToVblank ());
++   return (GL::waitVideoSync && optionGetSyncToVblank ());
+ }
+ 
+ void
+@@ -1161,7 +1189,7 @@
+ {
+     if (pendingCommands)
+     {
+-	glFinish ();
++	glFlush ();  // glFlush! glFinish would block the CPU, which is bad.
+ 	pendingCommands = false;
+     }
+ }
+===================================================================
+--- old/plugins/opengl/src/privates.h	2011-08-12 15:10:58.007681085 +0800
++++ new/plugins/opengl/src/privates.h	2011-08-12 15:13:02.617681069 +0800
+@@ -70,6 +70,7 @@
+ 
+ 	void prepareDrawing ();
+ 
++	void controlSwapVideoSync ();
+ 	void waitForVideoSync ();
+ 
+ 	void paintBackground (const CompRegion &region,

=== modified file 'debian/patches/series'
--- debian/patches/series	2011-08-12 07:07:23 +0000
+++ debian/patches/series	2011-08-12 07:46:22 +0000
@@ -1,2 +1,3 @@
 01_don_t_init_a11y.patch
 091_no_use_gnome_but_desktop_file.patch
+fix_slow_vsync_lp763005.patch