[PATCH] Revert "Set DamageSetReportAfterOp to true for the damage extension" (#30260)

[Aaron Plattner]

On Wed, Oct 20, 2010 at 03:49:47PM -0700, Keith Packard wrote:
> On Wed, 20 Oct 2010 15:03:59 -0700, Aaron Plattner <aplattner at nvidia.com> wrote:
> 
> > Fine, but will you be willing to move this call to the drivers that need it
> > when we have a real sync extension?
> 
> I don't see the point -- it doesn't change anything visible to clients
> or drivers, other than to eliminate the tiny window where some damage
> events might be delivered in a slightly different order, aside from the
> bug present within the Damage code.

I meant that with proper synchronization, clients would benefit from
receiving the events as early as possible to give them more time to
process the events.  I realize that the events get buffered most of the
time currently so it's not a big change right now.

> Also, there's no way, given the currents server API, to have drivers
> make this call -- they can't know which of the many damage objects
> within the server are created by the damage extension.

That's a good point.  That seems like it might be worth adding.

> I guess I'm curious how these other drivers manage to use the composite
> extension today; are they just forcing cross-queue synchronization at
> every flush call in the X server?

No, at least for our driver.  Like I said, GLX does not specify any
synchronization and adding implicit synchronization would kill performance.
Compositors just kick off their rendering and hope for the best, and
sometimes it doesn't work.  Compiz added some sort of "force
synchronization" option recently, but I haven't looked at how it works.

> In any case, here's what I think we should ensure that any release on
> either 1.9 or 1.10 branch doesn't contain the compiz bug.
> 
> For the 1.9 branch, unless the fix for the Damage code is trivial, I'd
> recommend reverting this patch there.
> 
> For the 1.10 master branch, I'd strongly prefer to see the Damage bug
> get fixed before the 1.10 release. It's likely to be causing other
> problems within the server in places which use the post-op path. If no
> such fix is forthcoming before the 1.10 release, we should revert this
> patch as the known bugs it causes are worse than the known bugs it
> fixes.

Fair enough, and this change isn't that huge of a deal, I'm just worried
that papering over it by adding an inferred ordering between queues will
mean that nobody will review James's sync extension stuff and the Damage
sync problems will go unfixed for a third of your users for another few
years
( http://www.phoronix.com/data/img/results/lgs_2010_results/3.png )