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

[Paul Berry]

On 7 June 2013 17:42, Chris Wilson <chris at chris-wilson.co.uk> wrote:

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

Can this patch get pushed upstream now?  I'd like to rule it out as a
possible cause for https://bugs.freedesktop.org/show_bug.cgi?id=65744.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.x.org/archives/xorg-devel/attachments/20130618/e4858545/attachment.html>