On Mon, 2 Aug 2010 08:24:04 -0400, Kristian Høgsberg <[email protected]> wrote:
> On Sun, Aug 1, 2010 at 3:20 PM, Keith Packard <[email protected]> wrote:
> > On Sun, 1 Aug 2010 14:39:46 -0400, Kristian Høgsberg <[email protected]> 
> > 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...

-- 
[email protected]

Attachment: pgphuM5Ppnn0u.pgp
Description: PGP signature

_______________________________________________
[email protected]: X.Org support
Archives: http://lists.freedesktop.org/archives/xorg
Info: http://lists.freedesktop.org/mailman/listinfo/xorg
Your subscription address: [email protected]

Reply via email to