[PATCH] Revert "DRI2: re-allocate DRI2 drawable if pixmap serial changes"

[Chris Wilson]

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 <chris at chris-wilson.co.uk>
Tested-by: Chris Wilson <chris at chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre