Bug 28438

More mesa commit information are as following:

(master)commit a86e4852f463c289eba019a4d231fbd6cae82f27
Author: Kristian Høgsberg <krh@bitplanet.net>
Date:   Mon May 10 16:18:55 2010 -0400

    intel: Only flush fake front buffer on API level glFlush()

    Without this patch, any old intel_flush() call will cause a round trip to
    the server and do a copy from fake to real front.  We only actually
    guarantee that frontbuffer results show up when glFlush() ia called, so
    move the flushing to intel_glFlush().

    We also need to flush fake to front before getting new buffers, but
    we just handle that manually.


(master)commit 0bf1ddd60a3660974ad54e410533eeb481949364
Author: Vinson Lee <vlee@vmware.com>
Date:   Mon May 10 15:41:22 2010 -0700

    i915: Drop intelFlush().

    This was missed in commit c4775a27e3aaa2006b98f225387499b79bc609ef.
    Fixes i915 build.

Kristian, can you reproduce?

The issue also exists on the 7.8 branch. With bisect, we found the culprit commit is: 7c50d29f7ced3d60e52ee0146d982b49ea421de2 
And the one following it is the first good commit:
c642f3941ba2ab68135037e1fcb1c29dcb820de7 

More commit information is as following:

commit 7c50d29f7ced3d60e52ee0146d982b49ea421de2
Author: Kristian Høgsberg <krh@bitplanet.net>
Date:   Fri Jan 8 12:35:47 2010 -0500

    intel/DRI2: add DRI2flushExtension support with invalidate hook

    Needed to support the SwapBuffers code properly.

    Signed-off-by: Kristian Høgsberg <krh@bitplanet.net>

commit c642f3941ba2ab68135037e1fcb1c29dcb820de7
Author: Jesse Barnes <jbarnes@virtuousgeek.org>
Date:   Mon Jan 4 16:26:17 2010 -0500

    xdemos/glsync: handle no sync method better

    Print out count, finish rendering, etc.  Makes the -sn option more useful.

Although the bisect seems to imply a regression in mesa, I am curious if the flushing fixes for i965 in 2.12 were perhaps the underlying bug?

I don't actually see anything with xterm, but I can get gnome-terminal to skip rendering in some cases.  Are you using xterm or gnome-terminal?  Can you attach a screenshot of the bad rendering?  Are you running compiz in direct rendering mode and does it happen if you run it in indirect mode?

Created attachment 37297 [details]
incorrect character in gnome-terminal  under compiz

we are using gnome-terminal to reproduce the issue. 
And this issue only turns out in direct rendering mode. When we ran compiz in indirect rendering mode(compiz --indirect-rendering), the bug disappeared.

I have a patch at:

http://cgit.freedesktop.org/~anholt/xf86-video-intel/log/?h=compositing-fix

that may fix the problem -- if someone can reproduce the problem can test with with compositing-fix~1 and compositing-fix, that would be nice.

(In reply to comment #8)
> I have a patch at:
> http://cgit.freedesktop.org/~anholt/xf86-video-intel/log/?h=compositing-fix
> that may fix the problem -- if someone can reproduce the problem can test with
> with compositing-fix~1 and compositing-fix, that would be nice.

We modified the patch a little, and then build 2D driver with the new patch.
The bug doesn't come out again.

Created attachment 37411 [details]
the modified patch

(In reply to comment #8)
> I have a patch at:
> 
> http://cgit.freedesktop.org/~anholt/xf86-video-intel/log/?h=compositing-fix
> 
> that may fix the problem -- if someone can reproduce the problem can test with
> with compositing-fix~1 and compositing-fix, that would be nice.

I don't think this is the patch we're looking for.  We used to have a getbuffers round trip for each glXBindTexImage, but with the invalidate events we can cache the bo name in the client and avoid that.  The round trip was probably what kept things in sync, and tfp probably broke when we stopped doing that.  But it has to work without it.

The way tfp works is that the server submits rendering to against a drawable and sends out the damage events.  The server does write the event to the client output buffer before the driver emits the rendering to the batch buffer, but the client output buffer isn't actually written to the client until after the block handler, where we submit the batch buffer.

So the ordering on the server side seems to be OK, except in the case where the client output buffer overflows and we flush it without submitting the batch buffer.  This sounds like it could be the problem, but it isn't since the corruption happens even without the overflow condition triggering.

(In reply to comment #11)
> (In reply to comment #8)
> > I have a patch at:
> > 
> > http://cgit.freedesktop.org/~anholt/xf86-video-intel/log/?h=compositing-fix
> > 
> > that may fix the problem -- if someone can reproduce the problem can test with
> > with compositing-fix~1 and compositing-fix, that would be nice.
> 
> I don't think this is the patch we're looking for.  We used to have a
> getbuffers round trip for each glXBindTexImage, but with the invalidate events
> we can cache the bo name in the client and avoid that.  The round trip was
> probably what kept things in sync, and tfp probably broke when we stopped doing
> that.  But it has to work without it.
> 
> The way tfp works is that the server submits rendering to against a drawable
> and sends out the damage events.  The server does write the event to the client
> output buffer before the driver emits the rendering to the batch buffer, but
> the client output buffer isn't actually written to the client until after the
> block handler, where we submit the batch buffer.
> 
> So the ordering on the server side seems to be OK, except in the case where the
> client output buffer overflows and we flush it without submitting the batch
> buffer.  This sounds like it could be the problem, but it isn't since the
> corruption happens even without the overflow condition triggering.

Indeed, I just bisected the problem myself and the offending commit is:

commit f829e76d8835382b8a52224dfbb2556360e41ffc
Author: Kristian Høgsberg <krh@bitplanet.net>
Date:   Tue May 11 10:32:40 2010 -0400

    intel: Don't update renderbuffers in intelSetTexBuffer2 if we have invalidat

which is the commit where I removed the getbuffers roundtrip on glXBindTexImage.

Created attachment 37447 [details] [review]
Submit batch buffers from flush callback chain 

This is the fix for the root cause.  It's a patch against xf86-video-intel.  Sent to intel-gfx for review.

Patch committed to xf86-video-intel, closing bug.


commit 69d65f9184006eac790efcff78a0e425160e95aa
Author: Kristian Høgsberg <krh@bitplanet.net>
Date:   Thu Jul 29 18:31:48 2010 -0400

    Submit batch buffers from flush callback chain
    
    There are a few cases where the server will flush client output buffers
    but our block handler only catches the most common (before going into select
    If the server flushes client buffers before we submit our batch buffer,
    the client may receive a damage event for rendering that hasn't happened yet
    
    Instead, we can hook into the flush callback chain, which the server will
    invoke just before flushing output.  This lets us submit batch buffers
    before sending out events, preserving ordering.
    
    Fixes 28438: [bisected] incorrect character in gnome-terminal under compiz
    https://bugs.freedesktop.org/show_bug.cgi?id=28438
    
    Signed-off-by: Kristian Høgsberg <krh@bitplanet.net>

Already fixed.