Merge lp:~vanvugt/compiz/fix-763005-trunk into lp:~compiz/compiz/ubuntu

Proposed by Daniel van Vugt
Status: Superseded
Proposed branch: lp:~vanvugt/compiz/fix-763005-trunk
Merge into: lp:~compiz/compiz/ubuntu
Diff against target: 187 lines (+176/-0)
2 files modified
debian/patches/fix_slow_vsync_lp763005.patch (+175/-0)
debian/patches/series (+1/-0)
To merge this branch: bzr merge lp:~vanvugt/compiz/fix-763005-trunk
Reviewer Review Type Date Requested Status
Sam Spilsbury Approve
compiz packagers Pending
Review via email: mp+71307@code.launchpad.net

This proposal has been superseded by a proposal from 2011-09-01.

Description of the change

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...

To post a comment you must log in.
Revision history for this message
Didier Roche-Tolomelli (didrocks) wrote :

Sam, can you look at this? Thanks!

Revision history for this message
Sam Spilsbury (smspillaz) wrote :

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

Unmerged revisions

648. By Daniel van Vugt

Fix slow/stuttering display when sync to Vblank is enabled. (LP: #763005)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== 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 @@
1Description: Fix slow/stuttering display when sync to Vblank is enabled.
2 For fglrx the cause appears to be an ATI/AMD driver bug where
3 glXGetVideoSyncSGI and glXWaitVideoSyncSGI are particularly slow, so we
4 can't afford to call both on every redraw or we end up with 1/3-1/2 framerate.
5 For intel and really all drivers, another cause appears to be excessive
6 waiting in the compiz architecture not allowing enough CPU time to draw the
7 full frame before the next vertical refresh.
8 This patch addresses both possible problems, providing optimal framerates
9 with sync-to-vblank support, most of the time.
10Author: Daniel van Vugt <vanvugt@gmail.com>
11Bug-Ubuntu: https://launchpad.net/bugs/763005
12
13===================================================================
14--- old/plugins/opengl/include/opengl/opengl.h 2011-08-12 15:00:38.809906000 +0800
15+++ new/plugins/opengl/include/opengl/opengl.h 2011-08-12 15:09:05.000000000 +0800
16@@ -98,11 +98,13 @@
17 int width,
18 int height);
19
20- typedef int (*GLXGetVideoSyncProc) (unsigned int *count);
21 typedef int (*GLXWaitVideoSyncProc) (int divisor,
22 int remainder,
23 unsigned int *count);
24
25+ // http://www.opengl.org/registry/specs/SGI/swap_control.txt
26+ typedef int (*GLXSwapIntervalProc) (int interval);
27+
28 #ifndef GLX_VERSION_1_3
29 typedef struct __GLXFBConfigRec *GLXFBConfig;
30 #endif
31@@ -163,8 +165,8 @@
32 extern GLXReleaseTexImageProc releaseTexImage;
33 extern GLXQueryDrawableProc queryDrawable;
34 extern GLXCopySubBufferProc copySubBuffer;
35- extern GLXGetVideoSyncProc getVideoSync;
36 extern GLXWaitVideoSyncProc waitVideoSync;
37+ extern GLXSwapIntervalProc swapInterval;
38 extern GLXGetFBConfigsProc getFBConfigs;
39 extern GLXGetFBConfigAttribProc getFBConfigAttrib;
40 extern GLXCreatePixmapProc createPixmap;
41===================================================================
42--- old/plugins/opengl/src/screen.cpp 2011-08-12 15:00:38.809906000 +0800
43+++ new/plugins/opengl/src/screen.cpp 2011-08-12 15:09:05.000000000 +0800
44@@ -36,8 +36,8 @@
45 GLXReleaseTexImageProc releaseTexImage = NULL;
46 GLXQueryDrawableProc queryDrawable = NULL;
47 GLXCopySubBufferProc copySubBuffer = NULL;
48- GLXGetVideoSyncProc getVideoSync = NULL;
49 GLXWaitVideoSyncProc waitVideoSync = NULL;
50+ GLXSwapIntervalProc swapInterval = NULL;
51 GLXGetFBConfigsProc getFBConfigs = NULL;
52 GLXGetFBConfigAttribProc getFBConfigAttrib = NULL;
53 GLXCreatePixmapProc createPixmap = NULL;
54@@ -227,13 +227,16 @@
55
56 if (strstr (glxExtensions, "GLX_SGI_video_sync"))
57 {
58- GL::getVideoSync = (GL::GLXGetVideoSyncProc)
59- getProcAddress ("glXGetVideoSyncSGI");
60-
61 GL::waitVideoSync = (GL::GLXWaitVideoSyncProc)
62 getProcAddress ("glXWaitVideoSyncSGI");
63 }
64
65+ if (strstr (glxExtensions, "GLX_SGI_swap_control"))
66+ {
67+ GL::swapInterval = (GL::GLXSwapIntervalProc)
68+ getProcAddress ("glXSwapIntervalSGI");
69+ }
70+
71 glXMakeCurrent (dpy, CompositeScreen::get (s)->output (), priv->ctx);
72
73 glExtensions = (const char *) glGetString (GL_EXTENSIONS);
74@@ -1002,19 +1005,30 @@
75 }
76
77 void
78-PrivateGLScreen::waitForVideoSync ()
79+PrivateGLScreen::controlSwapVideoSync ()
80 {
81- unsigned int sync;
82-
83- if (!optionGetSyncToVblank ())
84- return;
85+ bool sync = optionGetSyncToVblank ();
86+ if (GL::swapInterval)
87+ { // Docs: http://www.opengl.org/registry/specs/SGI/swap_control.txt
88+ (*GL::swapInterval) (sync ? 1 : 0);
89+ }
90+ else if (sync)
91+ { // ugly fallback - blocks the CPU.
92+ waitForVideoSync ();
93+ }
94+}
95
96- if (GL::getVideoSync)
97+void
98+PrivateGLScreen::waitForVideoSync ()
99+{
100+ if (optionGetSyncToVblank () && GL::waitVideoSync)
101 {
102- glFlush ();
103+ if (GL::swapInterval)
104+ (*GL::swapInterval) (0); // Don't wait twice. Just in case.
105
106- (*GL::getVideoSync) (&sync);
107- (*GL::waitVideoSync) (2, (sync + 1) % 2, &sync);
108+ // Docs: http://www.opengl.org/registry/specs/SGI/video_sync.txt
109+ unsigned int frameno;
110+ (*GL::waitVideoSync) (1, 0, &frameno);
111 }
112 }
113
114@@ -1087,10 +1101,23 @@
115
116 targetOutput = &screen->outputDevs ()[0];
117
118- waitForVideoSync ();
119+ glFlush (); // used to be called from waitForVideoSync
120
121+ //
122+ // FIXME: Actually fix the composite plugin to be more efficient;
123+ // If GL::swapInterval == NULL && GL::waitVideoSync != NULL then the
124+ // composite plugin should not be imposing any framerate restriction
125+ // (ie. blocking the CPU) at all. Because the framerate will be controlled
126+ // and optimized here:
127+ //
128 if (mask & COMPOSITE_SCREEN_DAMAGE_ALL_MASK)
129 {
130+ //
131+ // controlSwapVideoSync is much faster than waitForVideoSync because
132+ // it won't block the CPU. The waiting is offloaded to the GPU.
133+ // Unfortunately it only works with glXSwapBuffers in most drivers.
134+ //
135+ controlSwapVideoSync ();
136 glXSwapBuffers (screen->dpy (), cScreen->output ());
137 }
138 else
139@@ -1098,6 +1125,7 @@
140 BoxPtr pBox;
141 int nBox, y;
142
143+ waitForVideoSync ();
144 pBox = const_cast <Region> (tmpRegion.handle ())->rects;
145 nBox = const_cast <Region> (tmpRegion.handle ())->numRects;
146
147@@ -1153,7 +1181,7 @@
148 bool
149 PrivateGLScreen::hasVSync ()
150 {
151- return (GL::getVideoSync && optionGetSyncToVblank ());
152+ return (GL::waitVideoSync && optionGetSyncToVblank ());
153 }
154
155 void
156@@ -1161,7 +1189,7 @@
157 {
158 if (pendingCommands)
159 {
160- glFinish ();
161+ glFlush (); // glFlush! glFinish would block the CPU, which is bad.
162 pendingCommands = false;
163 }
164 }
165===================================================================
166--- old/plugins/opengl/src/privates.h 2011-08-12 15:10:58.007681085 +0800
167+++ new/plugins/opengl/src/privates.h 2011-08-12 15:13:02.617681069 +0800
168@@ -70,6 +70,7 @@
169
170 void prepareDrawing ();
171
172+ void controlSwapVideoSync ();
173 void waitForVideoSync ();
174
175 void paintBackground (const CompRegion &region,
0176
=== 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 @@
101_don_t_init_a11y.patch101_don_t_init_a11y.patch
2091_no_use_gnome_but_desktop_file.patch2091_no_use_gnome_but_desktop_file.patch
3fix_slow_vsync_lp763005.patch

Subscribers

People subscribed via source and target branches

to all changes: