[PATCH 1/2] Always call the flush callback chain when we might send out damage events

[Keith Packard]

On Mon, 2 Aug 2010 08:24:04 -0400, Kristian Høgsberg <krh at bitplanet.net> wrote:
> On Sun, Aug 1, 2010 at 3:20 PM, Keith Packard <keithp at keithp.com> wrote:
> > On Sun, 1 Aug 2010 14:39:46 -0400, Kristian Høgsberg <krh at bitplanet.net> wrote:
> >> Before this gets drowned out in janitorial patches... Keith, do you
> >> want a pull request for this and the damageext patch?  Did you have
> >> have a look at the damage change?
> >
> > I read through the change and reviewed the potential impacts on the
> > server. I'm a little concerned about having the semantics of the damage
> > extension accidentally change as the code paths for the post-activity
> > damage region processing.
> 
> Half a sentence missing here?

Yeah, missed the 'are completely different than the pre-activity code path'

> The patch is not changing the
> semantics, just making sure that the semantics that we effectively
> provide and clients expect is what they get (post rendering
> notification).

Sure, my concern is that within the damage code itself, the code to
perform post-activity notification is quite convoluted and may easily
introduce subtle changes in the events delivered to applications. I just
don't know; I haven't really reviewed that in depth.

> Yup, it is the kind of change that requires a bit of review.

I'll merge your change and then probably spend some time looking at the
damage code and seeing if I can't clean it up a bit; right now it's
messy with lots of different options about when and how the callbacks
are made.

Just for fun, I briefly considered having the damage extension use the
pre-activity mode and simply pend all output to the client until the
FlushAllOutput call from the main loop. That would have been a
smaller change in the code paths than your patch, but would expose the X
server to potentially buffering a fairly hefty set of events, which
didn't seem like a great plan -- in case of malloc failure in that code
path, the client is disconnected from the X server...

-- 
keith.packard at intel.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <http://lists.x.org/archives/xorg/attachments/20100802/7e96d81d/attachment.pgp>