[PATCH] remove damagePostOp() from DamageDamageRegion()

[Adam Jackson]

On Wed, 2008-08-27 at 09:56 -0700, Aaron Plattner wrote:

> Now might be a good time to reopen the discussion about fixing the Damage
> extension for asynchronous hardware.  There are two ways to solve this:
> 
>   1. Add extensions to allow GL client rendering to wait for X rendering
>      associated with a particular Damage event using some sort of sync
>      barrier.
>   2. Defer sending Damage events until the damage has actually occurred.
> 
> While 1 would be ideal, it's probably too complicated to spec and implement
> in a reasonable timeframe so it might be a good idea to think about
> implementing 2.  What we need for that is
> 
>   a. reportAfter-like semantics for all damage objects, not just ones that
>      were explicitly marked with DamageSetReportAfterOp.
>   b. Deferring damageReportPostOp to after the rendering has actually
>      occurred, not just after it's been submitted to the hardware.

As Michel notes, we need report-before too.  Software cursor is the big
user of this, which does among other things:

if the incoming damage overlaps the glyph position
    mark cursor removed
    restore the pixels under the glyph
call lower rendering layer
if cursor was removed
    re-paint the cursor

This is overkill in the common case of GXcopy, but any rop that reads
out the destination needs this for correctness.  Software cursor is not
really something we can break, since hardware still rarely has more than
one and MPX will require arbitrarily many.  Cursor plane plz.

I think the tension here is that there's actually three events of
interest - damage pending, damage posted to hardware, and damage
completed - and we only expose two.  Of course, for software and
single-queue hardware, the latter two are pretty much identical.

I lean towards defining the existing DamageNotify event - and the
embarassingly named ReportPostOp inside the server - as "posted to
hardware", on the basis that naïve hardware shouldn't have to care, and
complicated hardware will have to do something custom to get the
completion event anyway.  The question is how to add the completion
event to the protocol.  And yes, I realize this is walking straight into
the "too complicated to spec and implement" warning above, but I don't
think it's actually that bad.  My first thought would be to add it as a
new sync counter, but I think that fails because you'd need one per
drawable.  I think this feels more like a flag in the high bit of the
damage report level: DamageReportNonEmpty | DamageReportCompletion would
give you a simple edge trigger when rendering hit the framebuffer.  As a
bonus, this gives us per-drawable completions, and a way to know about
them that isn't XGetImage.

The internal API could go a couple of ways.  My first guess is something
like a DamageRegisterCompletionCallback() for the screen, which if
present causes miext/damage/ to register a WakeupHandler that calls into
the screen for events (which it then pulls from the DRM or whatever).
There's possibly a wrapping order issue there but I think it's
manageable.  Screens without such a callback would just generate
completion-class events immediately.  Maybe this is bad API though?  I'm
open to suggestions.

---

Assuming all the above, the question is "what does DamageDamageRegion
mean".  I think the original intention of adding it as driver API was as
a way to generate DamageNotify events for things that happened outside
X.  I think it still works with no changes in the above clarified model:
A region got smashed from the outside, you didn't get the chance to save
any bits on it, clients are about to get told so now's your chance to
prepare for that if you need to.  (For example, swcursor would put the
glyph up again and just deal with any  resulting flicker.)  Since it
still calls ReportPostOp, you either get completion events immediately,
or the poll for same gets scheduled.

That still makes it bad _internal_ API.  The server core has no excuse
for calling it, because internally we need to do things between
damage-pend and damage-post.  But that seems trivial:

void
damageDamageRegionPending(DrawablePtr pDraw, RegionPtr pReg)
{
    damageDamageRegion(pDraw, pReg, FALSE, -1);
}

void
damageDamageRegionPosted(DrawablePtr pDraw)
{
    damageReportPostOp(pDraw);
}

And then fix up the call sites to do things in the right order.
(Actually I'd really like to just rename ReportPostOp entirely.)

- ajax