[Mesa-dev] glproto changes

[Jeremy Huddleston]

Why is sbc a 32bit field in xGLXBufferSwapComplete2 and xDRI2BufferSwapComplete2 when it is a 64bit field in GLXBufferSwapComplete?

The hunk at the bottom will result in casting a 64bit int to a 32bit int.  If you're going to change this, shouldn't you also change GLXBufferSwapComplete, DRI2SwapEvent() and everything else that has a 64bit swap count?

Isn't it easier to correct the spec to match reality?

--- a/hw/xfree86/dri2/dri2ext.c
+++ b/hw/xfree86/dri2/dri2ext.c
@@ -359,7 +359,7 @@ static void
 DRI2SwapEvent(ClientPtr client, void *data, int type, CARD64 ust, CARD64 msc,
 	      CARD64 sbc)
 {
-    xDRI2BufferSwapComplete event;
+    xDRI2BufferSwapComplete2 event;
     DrawablePtr pDrawable = data;
 
     event.type = DRI2EventBase + DRI2_BufferSwapComplete;
@@ -369,8 +369,7 @@ DRI2SwapEvent(ClientPtr client, void *data, int type, CARD64 ust, CARD64 msc,
     event.ust_lo = ust & 0xffffffff;
     event.msc_hi = (CARD64)msc >> 32;
     event.msc_lo = msc & 0xffffffff;
-    event.sbc_hi = (CARD64)sbc >> 32;
-    event.sbc_lo = sbc & 0xffffffff;
+    event.sbc = sbc;
 
     WriteEventsToClient(client, 1, (xEvent *)&event);
 }



On May 5, 2011, at 08:28, Jesse Barnes wrote:

> On Wed, 4 May 2011 21:29:23 -0700
> Jeremy Huddleston <jeremyhu at freedesktop.org> wrote:
> 
>> Yeah... so considering the comments in mesa-dev earlier today, I was really surprised to see that glproto and dri2proto were tagged today.  I think we need to brownbag retract and rethink this.
> 
> Damnit; right when Dave mentioned this last night I knew I had gone too
> far in trying to make sure the fix was propagated.  I hate it when he's
> right!
> 
> Yeah, having a rule that we can't touch existing proto structs makes
> sense unless we want to do a major break.  It certainly makes pushing
> out updates eaiser and preserves bisection...
> 
>> These changes break API.  I'm all for fixing the structs, but anything that breaks API (or worse, protocol) certainly warrants much more than the +0.0.1 version bump.  This also makes it impossible to build the current dev and current stable with the same protos installed.  I haven't looked at the details specifically, but I suspect that it also changes what is on the wire, meaning clients and the server may disagree depending on which glproto version they were using.  Is that the case?  If not, can't we solve this with some creative union{}ing?
> 
> In this case, what's on the wire is pretty much the same; if the client
> and server don't match, you may get a different kind of corruption in
> the affected field (0 vs some other value), but things are no worse.
> 
>> Either way, I think we should retract the glproto 1.4.13 release until we can get this straightened out.
> 
> Ok; fwiw my rationale was twofold:
>  1) make sure master requires the new proto headers to avoid some of
>     the trouble we've had in the past with ifdefs and untested paths
>     (though again, the failure mode is benign in this case)
>  2) make the proto breakage obvious even for older code; the fix is
>     trivial
> 
> But I guess (2) is a bit much... after pushing I started having
> nightmares about the input proto changes from awhile back.
> 
> Here are what the backwards compatible changes look like...  Look
> better?
> 
> Thanks,
> -- 
> Jesse "Breaker of Builds" Barnes
> <dri2proto-compat-fix.patch><glproto-compat-fix.patch><mesa-glx-compat-fix.patch><xserver-glproto-compat-fix.patch>