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

[Daniel van Vugt]

Reduced screen tearing still visible with "Sync To VBlank" enabled. (LP: #880707)

The composite plugin's frame timing logic did not correctly synchronize with the vblank support of other plugins (opengl). This would lead to excessive visible tearing even when "Sync To VBlank" is enabled in opengl.

This fix does NOT claim to eliminate all tearing, but it does eliminate most of it. To completely eliminate tearing more work has to be done to make the glXSwapBuffers code (opengl plugin) fast enough to use on every frame.

[Daniel van Vugt (vanvugt)]

I suspect this will fix bug 92599 too, so long as "Sync To VBlank" is enabled in OpenGL. Because composite will now ignore the rubbish framerate it gets from NVIDIA-Xrandr and will always use the natural refresh rate of the monitor instead.

[Sam Spilsbury (smspillaz)]

Hi Daniel,

Just to clarify, this returns to using glXWaitVideoSync and glXGetVideoSync rather than glXSwapInterval?

As for moving to using glXSwapBuffers - I agree with that, however the plugins generally expect that the damaged regions are going to be the ones copied over on to the frontbuffer from the backbuffer, rather than the entire back-to-front copy. Our current paint system allows for sort of "spill overs" on to the backbuffer, a much larger work item here is to make every plugin set its paint clipping region explicitly for each thing that it paints, so that the backbuffer is "sacred".

review: Approve

[Daniel van Vugt (vanvugt)]

Returning back to WaitVideoSync was necessary, because SwapInterval doesn't block. And we need the blocking in WaitVideoSync for accurate frame timing and to always stay in sync with the monitor. Composite's reasoning that it can magically stay in sync with the monitor's vsync with artificial timer delays was very misguided. If you have Sync to VBlank enabled then the only blocking should be the call to WaitVideoSync. This solves the problem of compiz painting out of phase, and even slightly out of frequency.

SwapInterval was originally used to try and "unblock the CPU". However I now realize that the main CPU blocking issue was this one -- bad timing in composite. SwapInterval made little to no difference and can be safely removed.

The critical fix for bug 763005 is still present:
  (*GL::waitVideoSync) (1, 0, &frameno);

[Daniel van Vugt (vanvugt)]

Tearing is further reduced by combining this proposal with another:
https://code.launchpad.net/~vanvugt/compiz-core/fix-priority/+merge/81952

[Daniel van Vugt (vanvugt)]

Work in progress again. I think I can improve on this significantly.

Unmerged revisions

=== modified file 'plugins/composite/src/screen.cpp'
--- plugins/composite/src/screen.cpp	2011-10-15 11:00:51 +0000
+++ plugins/composite/src/screen.cpp	2011-11-09 14:27:01 +0000
@@ -706,7 +706,7 @@
 	(FPSLimiterMode == CompositeFPSLimiterModeVSyncLike ||
 	 (pHnd && pHnd->hasVSync ()));
 
-    if (idle || hasVSyncBehavior)
+    if (idle)
     {
 	if (timeMult > 1)
 	{
@@ -715,6 +715,11 @@
 	    timeMult--;
 	}
     }
+    else if (hasVSyncBehavior)
+    {
+	frameStatus = 0;
+	redrawTime = diff;
+    }
     else
     {
 	int next;
@@ -755,13 +760,7 @@
 	}
     }
     
-    if (diff >= redrawTime)
-	return 1;
-
-    if (hasVSyncBehavior)
-	return (redrawTime - diff) * 0.7;
-
-    return redrawTime - diff;
+    return (diff >= redrawTime) ? 1 : redrawTime - diff;
 }
 
 int

=== modified file 'plugins/opengl/src/screen.cpp'
--- plugins/opengl/src/screen.cpp	2011-10-26 23:15:02 +0000
+++ plugins/opengl/src/screen.cpp	2011-11-09 14:27:01 +0000
@@ -31,9 +31,6 @@
 #include <math.h>
 
 namespace GL {
-    typedef int (*GLXSwapIntervalProc) (int interval);
-
-    GLXSwapIntervalProc      swapInterval = NULL;
     GLXBindTexImageProc      bindTexImage = NULL;
     GLXReleaseTexImageProc   releaseTexImage = NULL;
     GLXQueryDrawableProc     queryDrawable = NULL;
@@ -413,12 +410,6 @@
 	    getProcAddress ("glXWaitVideoSyncSGI");
     }
 
-    if (strstr (glxExtensions, "GLX_SGI_swap_control"))
-    {
-	GL::swapInterval = (GL::GLXSwapIntervalProc)
-	    getProcAddress ("glXSwapIntervalSGI");
-    }
-
     fbConfigs = (*GL::getFBConfigs) (dpy, s->screenNum (), &nElements);
 
     for (i = 0; i <= MAX_DEPTH; i++)
@@ -1076,44 +1067,18 @@
 		priv->lastViewport.height);
 }
 
-namespace GL
-{
-
 void
-waitForVideoSync ()
+PrivateGLScreen::waitForVideoSync ()
 {
+    // Docs: http://www.opengl.org/registry/specs/SGI/video_sync.txt
     if (GL::waitVideoSync)
     {
-	// Don't wait twice. Just in case.
-	if (GL::swapInterval)
-	    (*GL::swapInterval) (0);
-
-	// Docs: http://www.opengl.org/registry/specs/SGI/video_sync.txt
 	unsigned int frameno;
 	(*GL::waitVideoSync) (1, 0, &frameno);
     }
 }
 
 void
-controlSwapVideoSync (bool sync)
-{
-    // Docs: http://www.opengl.org/registry/specs/SGI/swap_control.txt
-    if (GL::swapInterval)
-	(*GL::swapInterval) (sync ? 1 : 0);
-    else if (sync)
-	waitForVideoSync ();
-}
-
-} // namespace GL
-
-void
-PrivateGLScreen::waitForVideoSync ()
-{
-    if (optionGetSyncToVblank ())
-        GL::waitForVideoSync ();
-}
-
-void
 PrivateGLScreen::paintOutputs (CompOutput::ptrList &outputs,
 			       unsigned int        mask,
 			       const CompRegion    &region)
@@ -1183,22 +1148,11 @@
     targetOutput = &screen->outputDevs ()[0];
 
     glFlush ();
+    if (optionGetSyncToVblank ())
+	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.
-	 */
-	GL::controlSwapVideoSync (optionGetSyncToVblank ());
 	glXSwapBuffers (screen->dpy (), cScreen->output ());
     }
     else
@@ -1206,7 +1160,6 @@
 	BoxPtr pBox;
 	int    nBox, y;
 
-	waitForVideoSync ();
 	pBox = const_cast <Region> (tmpRegion.handle ())->rects;
 	nBox = const_cast <Region> (tmpRegion.handle ())->numRects;