<div dir="ltr">On 7 June 2013 17:42, Chris Wilson <<a href="mailto:chris@chris-wilson.co.uk" target="_blank">chris@chris-wilson.co.uk</a>> wrote: <div class="gmail_extra"><div class="gmail_quote"> <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div class=""><div class="h5">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: > <a href="https://bugs.freedesktop.org/show_bug.cgi?id=28365" target="_blank">https://bugs.freedesktop.org/show_bug.cgi?id=28365</a> > 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. </div></div>The fix for <a href="https://bugs.freedesktop.org/show_bug.cgi?id=28365" target="_blank">https://bugs.freedesktop.org/show_bug.cgi?id=28365</a> 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 <<a href="mailto:chris@chris-wilson.co.uk">chris@chris-wilson.co.uk</a>> Tested-by: Chris Wilson <<a href="mailto:chris@chris-wilson.co.uk">chris@chris-wilson.co.uk</a>> -Chris -- Chris Wilson, Intel Open Source Technology Centre </blockquote></div> </div><div class="gmail_extra">Can this patch get pushed upstream now? I'd like to rule it out as a possible cause for <a href="https://bugs.freedesktop.org/show_bug.cgi?id=65744">https://bugs.freedesktop.org/show_bug.cgi?id=65744</a>. </div></div>