[Mesa-dev] [PATCH] xserver/glx/dri2: use new GLX/DRI2 swap event types

[Jesse Barnes]

On Tue, 10 May 2011 10:46:17 +0200
Michel Dänzer <michel at daenzer.net> wrote:

> On Fre, 2011-05-06 at 14:01 -0700, Jesse Barnes wrote: 
> > On Fri, 6 May 2011 13:00:19 -0700
> > Jeremy Huddleston <jeremyhu at apple.com> wrote:
> > 
> > > Yeah, that looks about right.
> > > 
> > > This in combination with the latest version of "xserver/glx/dri2: use new GLX/DRI2 swap event types"
> > > 
> > > Reviewed-by: Jeremy Huddleston <jeremyhu at apple.com>
> > 
> > Ok here's a more complete patch.  It touches GLX and involves drawable
> > lifetimes, which I'm not that familiar with, so careful review
> > appreciated.  Note the X vs GLX drawable ID switching in the DRI2 event
> > handler (DRI2 just deals with X IDs).
> > 
> > Kristian and Jeremy, is this a good basis for moving the Apple stuff
> > over to a client GLX drawable type?
> > 
> > -- 
> > Jesse Barnes, Intel Open Source Technology Center
> > 
> > From fae63609dd4fd20ccd84d2211787136bb9a1da05 Mon Sep 17 00:00:00 2001
> > From: Jesse Barnes <jbarnes at virtuousgeek.org>
> > Date: Fri, 6 May 2011 10:31:24 -0700
> > Subject: [PATCH] GLX/DRI2: handle swap event swap count wrapping
> > 
> > Create a new GLX drawable struct to track client related info, and add a
> > wrap counter to it drawable and track it as we receive events.  This
> > allows us to support the full 64 bits of the event structure we pass to
> > the client even though the server only gives us a 32 bit count.
> > 
> > Signed-off-by: Jesse Barnes <jbarnes at virtuousgeek.org>
> 
> [...]
> 
> > @@ -582,6 +584,14 @@ struct glx_display
> >  #endif
> >  };
> >  
> > +struct glx_drawable {
> > +   XID xDrawable;
> > +   XID drawable;
> > +
> > +   uint32_t lastEventSbc;
> > +   int64_t eventSbcWrap;
> 
> Shouldn't eventSbcWrap be unsigned?

The aevent sbc field is signed, so I thought I'd match it.  Making it
unsigned should be fine though too; client already have to check for
wraparound on this value.

> > diff --git a/src/glx/glxext.c b/src/glx/glxext.c
> > index 02652cb..03c05a3 100644
> > --- a/src/glx/glxext.c
> > +++ b/src/glx/glxext.c
> > @@ -106,7 +106,7 @@ XEXT_GENERATE_ERROR_STRING(__glXErrorString, __glXExtensionName,
> >  static Bool
> >  __glXWireToEvent(Display *dpy, XEvent *event, xEvent *wire)
> >  {
> > -     struct glx_display *glx_dpy = __glXInitialize(dpy);
> > +   struct glx_display *glx_dpy = __glXInitialize(dpy);
> >  
> >     if (glx_dpy == NULL)
> >        return False;
> 
> Superfluous whitespace-only change.

Yeah I couldn't resist.  I'll push that separately.

> Looks good to me otherwise, but I'm not really familiar with the
> client-side GLX drawable lifetime either.

Ok, I'll double check things; I think it's ok since the new struct
matches the DRI drawable lifetime, but I'll look at those paths again
and make sure I haven't missed something.

Now anyone care to test?

-- 
Jesse Barnes, Intel Open Source Technology Center