Re: [PATCH] DRI2/GLX: fix swap event handling

[Jesse Barnes <jbarnes@virtuousgeek.org>]

On Fri, 29 Apr 2011 09:20:31 +0200
Julien Cristau <jcristau@debian.org> wrote:

> On Thu, Apr 28, 2011 at 13:27:22 -0700, Jesse Barnes wrote:
> 
> > diff --git a/glx/glxdri2.c b/glx/glxdri2.c
> > index d979717..654b4ae 100644
> > --- a/glx/glxdri2.c
> > +++ b/glx/glxdri2.c
> > @@ -192,8 +192,17 @@ __glXdriSwapEvent(ClientPtr client, void *data, int type, CARD64 ust,
> >      wire.ust_lo = ust & 0xffffffff;
> >      wire.msc_hi = msc >> 32;
> >      wire.msc_lo = msc & 0xffffffff;
> > -    wire.sbc_hi = sbc >> 32;
> > -    wire.sbc_lo = sbc & 0xffffffff;
> > +    wire.sbc_hi = 0;
> > +
> > +    if (DRI2ClientSupportsSBC(client)) {
> > +	wire.sbc_lo0 = sbc & 0xff;
> > +	wire.sbc_lo8 = (sbc >> 8) & 0xff;
> > +	wire.sbc_lo16 = (sbc >> 16) & 0xff;
> 
> This one should be wire.sbc_lo16 = (sbc >> 16) & 0xffff;

Ah thanks, good catch. In this case, too much typing and not enough
cult & paste. :)

> 
> > +    } else {
> > +	wire.sbc_lo0 = 0;
> > +	wire.sbc_lo8 = 0;
> > +	wire.sbc_lo16 = 0;
> > +    }
> >  
> >      WriteEventsToClient(client, 1, (xEvent *) &wire);
> >  }
> 
> Also I think big endian clients (when built against the old protocol
> header) will expect the low byte of event_type where you now have
> sbc_lo8, so you would need to swap them.  Same for the dri2 event.

Right.  I should probably add swapping for the whole thing for swapped
clients as well.

> 
> [...]
> > diff --git a/include/protocol-versions.h b/include/protocol-versions.h
> > index 8692ded..8fde917 100644
> > --- a/include/protocol-versions.h
> > +++ b/include/protocol-versions.h
> > @@ -57,7 +57,7 @@
> >  
> >  /* GLX */
> >  #define SERVER_GLX_MAJOR_VERSION		1
> > -#define SERVER_GLX_MINOR_VERSION		4
> > +#define SERVER_GLX_MINOR_VERSION		5
> >  
> Do we get to bump the GLX protocol version?  Somehow I thought that was
> khronos business.

Maybe it is, but unless we're going to ignore this problem for indirect
glx clients, we need a way of knowing which type of event structure to
expect.  Is there another version I could use instead?

-- 
Jesse Barnes, Intel Open Source Technology Center