On Wed, Jun 05, 2013 at 12:34:24PM -0700, Eric Anholt wrote: > This reverts commit 3209b094a3b1466b579e8020e12a4f3fa78a5f3f. After a > long debug session by Paul Berry, it appears that this was the commit > that has been producing sporadic failures in piglit front buffer > rendering tests for the last several years. > > GetBuffers may return fresh buffers with invalid contents at a couple > reasonable times: > > - When first asked for a non-fake-front buffer. > - When the drawable size is changed, an Invalidate has been sent, and > obviously the app needs to redraw the whole buffer. > - After a glXSwapBuffers(), GL allows the backbuffer to be undefined, > and an Invalidate was sent to tell the GL that it should grab these > appropriate new buffers to avoid stalling. > > But with the patch being reverted, GetBuffers would also return fresh > invalid buffers when the drawable serial number changed, which is > approximately "whenever, for any reason". The app is not expecting > invalid buffer contents "whenever", nor is it valid. Because the GL > usually only GetBuffers after an Invalidate is sent, and the new > buffer allocation only happened during a GetBuffers, most apps saw no > problems. But apps that do (fake-)frontbuffer rendering do frequently > ask the server for the front buffer (since we drop the fake front > allocation when we're not doing front buffer rendering), and if the > drawable serial got bumped midway through a draw, the server would > pointlessly ditch the front *and* backbuffer full of important > drawing, resulting in bad rendering. > > The patch was originally to fix bugzilla: > https://bugs.freedesktop.org/show_bug.cgi?id=28365 > Specifically: > > To reproduce, start with a large-ish display (i.e. 1680x1050 on my > laptop), use the patched glxgears from bug 28252 to add the > -override option. Then run glxgears -override -geometry 640x480 > to create a 640x480 window in the top left corner, which will work > fine. Next, run xrandr -s 640x480 and watch the fireworks. > > I've tested with an override-redirect glxgears, both with vblank sync > enabled and disabled, both with gnome-shell and no window manager at > all, before and after this patch. The only problem observed was that > before and after the revert, sometimes when alt-tabbing to kill my > gears after completing the test gnome-shell would get confused about > override-redirectness of the glxgears window (according to a log > message) and apparently not bother doing any further compositing.
The fix for https://bugs.freedesktop.org/show_bug.cgi?id=28365 is now provided by 6f916ffec7767eeab62132eb6575043969104c81, and so all 3209b09 does is randomly recreate all the other buffers after a Pixmap change. We still miss sending InvalidateNotify for those, but that is an orthogonal issue. Reviewed-by: Chris Wilson <[email protected]> Tested-by: Chris Wilson <[email protected]> -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
