Re: [Mesa-dev] glproto changes
On Wed, 4 May 2011 18:21:20 -0700, Jesse Barnes jbar...@virtuousgeek.org wrote: On Thu, 5 May 2011 11:17:02 +1000 Dave Airlie airl...@gmail.com wrote: So I wasn't watching and glproto broke its interface, and I think its bad. Why? You can no longer bisect things across this point without now moving glproto. glxproto.h:xGLXBufferSwapComplete was a released header file definition, you cannot go back and change history. This should have been handled with xGLXBufferSwapComplete2 struct that newer mesa and X server could would use instead of the older code. Old code would build against the old broken defintion but since its broken it wouldn't matter, and new code would build against the new fixed definition. This doesn't address the I need the latest glproto to build mesa and my distro doesn't have which concerns me less than the I can't bisect anymore and I fully agree with Jesse that the last time we tried using #ifdef for this sort of thing it led to a number of untested combos resulting in impossible to debug issues. Yes, in hindsight I was too shellshocked by our previous experience with dri2 invalidate, ifdefs, and untested paths to even consider allowing new code to build with old proto. But breaking bisect is bad, no doubt. But in this case adding a separate struct is probably the right thing to do, and I'd be happy to do it if people are willing to put up with the churn (glproto 1.4.14, dri2proto 2.5 plus changes to Mesa and X to use the new struct). Yes, please do a new struct. I'm happy to see the churn to make it happen if it means developers are likely to do better at testing stable branches. pgpJQDb5VCzXL.pgp Description: PGP signature ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [Mesa-dev] glproto changes
On Wed, 4 May 2011 21:29:23 -0700
Jeremy Huddleston jerem...@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
diff --git a/configure.ac b/configure.ac
index 9505f56..297be0e 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1,5 +1,5 @@
AC_PREREQ([2.60])
-AC_INIT([DRI2Proto], [2.4], [https://bugs.freedesktop.org/enter_bug.cgi?product=xorg])
+AC_INIT([DRI2Proto], [2.5], [https://bugs.freedesktop.org/enter_bug.cgi?product=xorg])
AM_INIT_AUTOMAKE([foreign dist-bzip2])
AM_MAINTAINER_MODE
diff --git a/dri2proto.h b/dri2proto.h
index ff76355..ab25565 100644
--- a/dri2proto.h
+++ b/dri2proto.h
@@ -290,6 +290,21 @@ typedef struct {
CARD8 pad;
CARD16 sequenceNumber B16;
CARD16 event_type B16;
+CARD32 drawable B32;
+CARD32 ust_hi B32;
+CARD32 ust_lo B32;
+CARD32 msc_hi B32;
+CARD32 msc_lo B32;
+CARD32 sbc_hi B32;
+CARD32 sbc_lo B32;
+} xDRI2BufferSwapComplete;
+#define sz_xDRI2BufferSwapComplete 32
+
+typedef struct {
+CARD8 type;
+CARD8 pad;
+CARD16 sequenceNumber B16;
+CARD16 event_type B16;
CARD16 pad2;
CARD32 drawable B32;
CARD32 ust_hi B32;
@@ -297,7 +312,7 @@ typedef struct {
CARD32 msc_hi B32;
CARD32 msc_lo B32;
CARD32 sbc B32;
-} xDRI2BufferSwapComplete;
+} xDRI2BufferSwapComplete2;
#define sz_xDRI2BufferSwapComplete 32
typedef struct {
diff --git a/configure.ac b/configure.ac
index a3047e4..a6c301c 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1,5 +1,5 @@
AC_PREREQ([2.60])
-AC_INIT([GLProto], [1.4.13], [https://bugs.freedesktop.org/enter_bug.cgi?product=xorg])
+AC_INIT([GLProto], [1.4.14], [https://bugs.freedesktop.org/enter_bug.cgi?product=xorg])
AM_INIT_AUTOMAKE([foreign dist-bzip2])
AM_MAINTAINER_MODE
diff --git a/glxproto.h b/glxproto.h
index dfa0647..3f9e837 100644
--- a/glxproto.h
+++ b/glxproto.h
@@ -1375,6 +1375,20 @@ typedef struct {
BYTE pad;
CARD16 sequenceNumber B16;
CARD16 event_type B16;
+CARD32 drawable;
+CARD32 ust_hi B32;
+CARD32 ust_lo B32;
+CARD32 msc_hi B32;
+CARD32 msc_lo B32;
+CARD32 sbc_hi B32;
+CARD32 sbc_lo B32;
+} xGLXBufferSwapComplete;
+
+typedef struct {
+BYTE type;
+BYTE pad;
+CARD16 sequenceNumber B16;
+CARD16 event_type B16;
CARD16 pad2;
CARD32 drawable;
CARD32 ust_hi B32;
@@ -1382,7 +1396,8 @@ typedef struct {
CARD32 msc_hi B32;
CARD32 msc_lo B32;
CARD32 sbc B32;
-} xGLXBufferSwapComplete;
+} xGLXBufferSwapComplete2;
+
//
diff --git a/configure.ac b/configure.ac
index 54d9c29..fb38a4e 100644
--- a/configure.ac
+++ b/configure.ac
@@ -21,8 +21,8 @@ dnl Versions for external dependencies
LIBDRM_REQUIRED=2.4.24
LIBDRM_RADEON_REQUIRED=2.4.24
LIBDRM_INTEL_REQUIRED=2.4.24
-DRI2PROTO_REQUIRED=2.1
-GLPROTO_REQUIRED=1.4.11
+DRI2PROTO_REQUIRED=2.5
+GLPROTO_REQUIRED=1.4.14
LIBDRM_XORG_REQUIRED=2.4.24
LIBKMS_XORG_REQUIRED=1.0.0
diff --git a/src/glx/dri2.c b/src/glx/dri2.c
index
Re: [Mesa-dev] glproto changes
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 0x;
event.msc_hi = (CARD64)msc 32;
event.msc_lo = msc 0x;
-event.sbc_hi = (CARD64)sbc 32;
-event.sbc_lo = sbc 0x;
+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 jerem...@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.patchglproto-compat-fix.patchmesa-glx-compat-fix.patchxserver-glproto-compat-fix.patch
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [Mesa-dev] glproto changes
On Thu, 5 May 2011 10:07:24 -0700 Jeremy Huddleston jerem...@freedesktop.org wrote: 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? No because XEvents are only 32 bytes. We actually noticed that when publishing the spec, but we never updated the code. And yes, I should change the CARD64s to CARD32s as in the previous patch. Accidentally dropped that part when reverting. Thanks, Jesse ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [Mesa-dev] glproto changes
On Thu, 5 May 2011 11:17:02 +1000 Dave Airlie airl...@gmail.com wrote: So I wasn't watching and glproto broke its interface, and I think its bad. Why? You can no longer bisect things across this point without now moving glproto. glxproto.h:xGLXBufferSwapComplete was a released header file definition, you cannot go back and change history. This should have been handled with xGLXBufferSwapComplete2 struct that newer mesa and X server could would use instead of the older code. Old code would build against the old broken defintion but since its broken it wouldn't matter, and new code would build against the new fixed definition. This doesn't address the I need the latest glproto to build mesa and my distro doesn't have which concerns me less than the I can't bisect anymore and I fully agree with Jesse that the last time we tried using #ifdef for this sort of thing it led to a number of untested combos resulting in impossible to debug issues. Yes, in hindsight I was too shellshocked by our previous experience with dri2 invalidate, ifdefs, and untested paths to even consider allowing new code to build with old proto. But breaking bisect is bad, no doubt. But in this case adding a separate struct is probably the right thing to do, and I'd be happy to do it if people are willing to put up with the churn (glproto 1.4.14, dri2proto 2.5 plus changes to Mesa and X to use the new struct). -- Jesse Barnes, Intel Open Source Technology Center ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [Mesa-dev] glproto changes
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.
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?
Either way, I think we should retract the glproto 1.4.13 release until we can
get this straightened out.
On May 4, 2011, at 18:17, Dave Airlie wrote:
So I wasn't watching and glproto broke its interface, and I think its bad.
Why?
You can no longer bisect things across this point without now moving glproto.
glxproto.h:xGLXBufferSwapComplete was a released header file
definition, you cannot go back and change history.
This should have been handled with xGLXBufferSwapComplete2 struct that
newer mesa and X server could would use
instead of the older code. Old code would build against the old broken
defintion but since its broken it wouldn't matter,
and new code would build against the new fixed definition.
This doesn't address the I need the latest glproto to build mesa and
my distro doesn't have which concerns me less
than the I can't bisect anymore and I fully agree with Jesse that the
last time we tried using #ifdef for this sort of thing it led
to a number of untested combos resulting in impossible to debug issues.
Dave.
___
mesa-dev mailing list
mesa-...@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel
