[PATCH 11/14] damageext: Xineramify (v6)

[Adam Jackson]

On Tue, 2013-12-03 at 15:31 -0800, Keith Packard wrote:
> Adam Jackson <ajax at redhat.com> writes:
> > No.  Accumulate -> DamageReportDamage on screen 0 -> Queue.  Even if
> > clipping means there's no damage to screen 0's logical aperture, screen
> > 0's DamageExtPtr tracks the whole protocol screen, and its callback
> > always gets invoked.
> 
> Oh, right, it's calling DamageReportDamage on the screen 0 damage
> object. That should work. I didn't expect that, which is why I missed
> it.

It's mentioned in the commit message, though not especially clear in the
comments.  I'm happy to either add better commentary or unify the two
callbacks so it's a little more obvious.

> > Yeah, I didn't like it either.  I had hoped to be able to just do the
> > event emission from screen 0's callback, but not all the rendering paths
> > do FOR_NSCREENS_BACKWARD.  And in a previous iteration of this patch I'd
> > used FlushCallback, but WriteEventsToClient can itself flush so that can
> > be infinite recursion.
> 
> Oh. Is there some reason that just calling DamageExtReport directly from
> the screen 0 handler every time wouldn't work just fine? You'll get
> multiple damage events for windows which span screens, but clients
> should deal with that just fine. Would greatly simplify your code, at
> the cost of not generating minimal damage reports.

I can think of two semantic issues that would introduce.

NonEmpty reports would effectively hear multiple edge triggers for the
same logical event.  If the client does not compressing consecutive
damage events - and why would they, this hasn't been a problem before -
then at best they'd do redundant work in reacting to the damage.  The
spec does say we're allowed to be sloppy with the actual emitted
rectangles, but the criteria for NonEmpty events is "each time the
damage rectangle changes from empty to non-empty", not "one or more
times when".

BoundingBox is worse, because the event stream would become nonsense.
Imagine a 100x100 window, straddling horizontally adjacent screens, such
that a 50x100 strip is on each.  A client creates a Damage on it.  The
initial report kicks back with two events, one saying the bounding box
is 50x100+0+0, and one saying it's 50x100+50+0.  I don't think we could
call that a "bounding box" with a straight face.  To handle this
correctly the client would need to union successive events, which is
fairly ridiculous since the existing definition promises that successive
events will only ever grow the damaged region.

I'd prefer not to introduce a change that requires clients to know how
poorly the server is implemented.  Granted it wouldn't be the first
semantic issue with Xinerama (check out how CopyArea from window to
pixmap just plain doesn't emit GraphicsExpose), but why make it worse.

- ajax