Make swap event handling match the spec

Make swap event handling match the spec

[Jesse Barnes]

Ian reminded me that we changed the spec to fit within an XEvent, but we
never updated the code to match.  This set of patches (much simpler than
the last) does just that.  Wrapping support can be added to Mesa if we
really want 64 bit values, but that means checking the drawable sbc and
adding whenver sbc hits 0.

Thanks,
Jesse

[PATCH] glxproto: make GLX swap event struct match spec

[Jesse Barnes]

We only spec a 32 bit swap count, so drop the high sbc field.

Signed-off-by: Jesse Barnes <jbarnes-Y1mF5jBUw70BENJcbMCuUQ@public.gmane.org>
---
 configure.ac |    2 +-
 glxproto.h   |    3 +--
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/configure.ac b/configure.ac
index d88e6df..a3047e4 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1,5 +1,5 @@
 AC_PREREQ([2.60])
-AC_INIT([GLProto], [1.4.12], [https://bugs.freedesktop.org/enter_bug.cgi?product=xorg])
+AC_INIT([GLProto], [1.4.13], [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 0ff44e3..a6018a1 100644
--- a/glxproto.h
+++ b/glxproto.h
@@ -1380,8 +1380,7 @@ typedef struct {
     CARD32 ust_lo B32;
     CARD32 msc_hi B32;
     CARD32 msc_lo B32;
-    CARD32 sbc_hi B32;
-    CARD32 sbc_lo B32;
+    CARD32 sbc B32;
 } xGLXBufferSwapComplete;
 
 /************************************************************************/
-- 
1.7.4.1

_______________________________________________
xorg-devel-go0+a7rfsptAfugRpC6u6w@public.gmane.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

[PATCH] dri2proto: make DRI2 swap event match GLX spec

[Jesse Barnes]

We only spec a 32 bit sbc count, so drop the high bits.

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 configure.ac |    2 +-
 dri2proto.h  |    3 +--
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/configure.ac b/configure.ac
index 5b78d6b..9505f56 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1,5 +1,5 @@
 AC_PREREQ([2.60])
-AC_INIT([DRI2Proto], [2.3], [https://bugs.freedesktop.org/enter_bug.cgi?product=xorg])
+AC_INIT([DRI2Proto], [2.4], [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 9708a4a..12d834f 100644
--- a/dri2proto.h
+++ b/dri2proto.h
@@ -295,8 +295,7 @@ typedef struct {
     CARD32 ust_lo B32;
     CARD32 msc_hi B32;
     CARD32 msc_lo B32;
-    CARD32 sbc_hi B32;
-    CARD32 sbc_lo B32;
+    CARD32 sbc B32;
 } xDRI2BufferSwapComplete;
 #define sz_xDRI2BufferSwapComplete 32
 
-- 
1.7.4.1

[PATCH 3/4] GLX/DRI2: pass drawable correctly for indirect swap events

[Jesse Barnes]

Pass the right drawable pointer as data to the swap complete function.

Signed-off-by: Jesse Barnes <jbarnes-Y1mF5jBUw70BENJcbMCuUQ@public.gmane.org>
---
 glx/glxdri2.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/glx/glxdri2.c b/glx/glxdri2.c
index d979717..93c5e5b 100644
--- a/glx/glxdri2.c
+++ b/glx/glxdri2.c
@@ -222,7 +222,7 @@ __glXDRIdrawableSwapBuffers(ClientPtr client, __GLXdrawable *drawable)
 #endif
 
     if (DRI2SwapBuffers(client, drawable->pDraw, 0, 0, 0, &unused,
-			__glXdriSwapEvent, drawable->pDraw) != Success)
+			__glXdriSwapEvent, drawable) != Success)
 	return FALSE;
 
     return TRUE;
-- 
1.7.4.1

_______________________________________________
xorg-devel-go0+a7rfsptAfugRpC6u6w@public.gmane.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

[PATCH 4/4] GLX/DRI2: make GLX swap event handling match spec

[Jesse Barnes]

Only send a 32 bit swap count out to the client.

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 configure.ac              |    4 ++--
 glx/glxdri2.c             |    5 ++---
 hw/xfree86/dri2/dri2.c    |    2 +-
 hw/xfree86/dri2/dri2.h    |    2 +-
 hw/xfree86/dri2/dri2ext.c |    5 ++---
 5 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/configure.ac b/configure.ac
index 6eb780c..87194a5 100644
--- a/configure.ac
+++ b/configure.ac
@@ -771,11 +771,11 @@ RECORDPROTO="recordproto >= 1.13.99.1"
 SCRNSAVERPROTO="scrnsaverproto >= 1.1"
 RESOURCEPROTO="resourceproto"
 DRIPROTO="xf86driproto >= 2.1.0"
-DRI2PROTO="dri2proto >= 2.3"
+DRI2PROTO="dri2proto >= 2.4"
 XINERAMAPROTO="xineramaproto"
 BIGFONTPROTO="xf86bigfontproto >= 1.2.0"
 DGAPROTO="xf86dgaproto >= 2.0.99.1"
-GLPROTO="glproto >= 1.4.10"
+GLPROTO="glproto >= 1.4.13"
 DMXPROTO="dmxproto >= 2.2.99.1"
 VIDMODEPROTO="xf86vidmodeproto >= 2.2.99.1"
 WINDOWSWMPROTO="windowswmproto"
diff --git a/glx/glxdri2.c b/glx/glxdri2.c
index 93c5e5b..450d8c9 100644
--- a/glx/glxdri2.c
+++ b/glx/glxdri2.c
@@ -163,7 +163,7 @@ __glXDRIdrawableWaitGL(__GLXdrawable *drawable)
 
 static void
 __glXdriSwapEvent(ClientPtr client, void *data, int type, CARD64 ust,
-		  CARD64 msc, CARD64 sbc)
+		  CARD64 msc, CARD32 sbc)
 {
     __GLXdrawable *drawable = data;
     xGLXBufferSwapComplete wire;
@@ -192,8 +192,7 @@ __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 = sbc;
 
     WriteEventsToClient(client, 1, (xEvent *) &wire);
 }
diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c
index 5c42a51..40829c2 100644
--- a/hw/xfree86/dri2/dri2.c
+++ b/hw/xfree86/dri2/dri2.c
@@ -76,7 +76,7 @@ typedef struct _DRI2Drawable {
     ClientPtr		 blockedClient;
     Bool		 blockedOnMsc;
     int			 swap_interval;
-    CARD64		 swap_count;
+    CARD32		 swap_count;
     int64_t		 target_sbc; /* -1 means no SBC wait outstanding */
     CARD64		 last_swap_target; /* most recently queued swap target */
     CARD64		 last_swap_msc; /* msc at completion of most recent swap */
diff --git a/hw/xfree86/dri2/dri2.h b/hw/xfree86/dri2/dri2.h
index fe0bf6c..2a41ead 100644
--- a/hw/xfree86/dri2/dri2.h
+++ b/hw/xfree86/dri2/dri2.h
@@ -51,7 +51,7 @@ extern CARD8 dri2_minor;
 
 typedef DRI2BufferRec DRI2Buffer2Rec, *DRI2Buffer2Ptr;
 typedef void (*DRI2SwapEventPtr)(ClientPtr client, void *data, int type,
-				 CARD64 ust, CARD64 msc, CARD64 sbc);
+				 CARD64 ust, CARD64 msc, CARD32 sbc);
 
 
 typedef DRI2BufferPtr	(*DRI2CreateBuffersProcPtr)(DrawablePtr pDraw,
diff --git a/hw/xfree86/dri2/dri2ext.c b/hw/xfree86/dri2/dri2ext.c
index 4e48e65..2bb515f 100644
--- a/hw/xfree86/dri2/dri2ext.c
+++ b/hw/xfree86/dri2/dri2ext.c
@@ -357,7 +357,7 @@ vals_to_card64(CARD32 lo, CARD32 hi)
 
 static void
 DRI2SwapEvent(ClientPtr client, void *data, int type, CARD64 ust, CARD64 msc,
-	      CARD64 sbc)
+	      CARD32 sbc)
 {
     xDRI2BufferSwapComplete event;
     DrawablePtr pDrawable = data;
@@ -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);
 }
-- 
1.7.4.1

[PATCH 5/6] GLX: don't crash when indirect clients try to set an event mask

[Jesse Barnes]

After sending the GLXChangeDrawableAttributes request, we also set a
local set of attributes on the DRI drawable.  But in the indirect case
this array won't be present, so skip the setting in that case to avoid a
crash.

Signed-off-by: Jesse Barnes <jbarnes-Y1mF5jBUw70BENJcbMCuUQ@public.gmane.org>
---
 src/glx/glx_pbuffer.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/src/glx/glx_pbuffer.c b/src/glx/glx_pbuffer.c
index 5f91bc6..ec54f1e 100644
--- a/src/glx/glx_pbuffer.c
+++ b/src/glx/glx_pbuffer.c
@@ -137,6 +137,9 @@ ChangeDrawableAttribute(Display * dpy, GLXDrawable drawable,
 #ifdef GLX_DIRECT_RENDERING
    pdraw = GetGLXDRIDrawable(dpy, drawable);
 
+   if (!pdraw)
+      return;
+
    for (i = 0; i < num_attribs; i++) {
       switch(attribs[i * 2]) {
       case GLX_EVENT_MASK:
-- 
1.7.4.1

_______________________________________________
xorg-devel-go0+a7rfsptAfugRpC6u6w@public.gmane.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

[PATCH 6/6] DRI2/GLX: make swap event handling match spec

[Jesse Barnes]

We only handle a 32 bit swap count, so use the new structure definitions.

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 configure.ac     |    4 ++--
 src/glx/dri2.c   |    2 +-
 src/glx/glxext.c |    2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/configure.ac b/configure.ac
index 3b05ca3..94fb6f7 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.4
+GLPROTO_REQUIRED=1.4.13
 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 adfd3d1..2f18ca0 100644
--- a/src/glx/dri2.c
+++ b/src/glx/dri2.c
@@ -124,7 +124,7 @@ DRI2WireToEvent(Display *dpy, XEvent *event, xEvent *wire)
       }
       aevent->ust = ((CARD64)awire->ust_hi << 32) | awire->ust_lo;
       aevent->msc = ((CARD64)awire->msc_hi << 32) | awire->msc_lo;
-      aevent->sbc = ((CARD64)awire->sbc_hi << 32) | awire->sbc_lo;
+      aevent->sbc = awire->sbc;
       return True;
    }
 #endif
diff --git a/src/glx/glxext.c b/src/glx/glxext.c
index 278c719..831d83f 100644
--- a/src/glx/glxext.c
+++ b/src/glx/glxext.c
@@ -138,7 +138,7 @@ __glXWireToEvent(Display *dpy, XEvent *event, xEvent *wire)
       aevent->drawable = awire->drawable;
       aevent->ust = ((CARD64)awire->ust_hi << 32) | awire->ust_lo;
       aevent->msc = ((CARD64)awire->msc_hi << 32) | awire->msc_lo;
-      aevent->sbc = ((CARD64)awire->sbc_hi << 32) | awire->sbc_lo;
+      aevent->sbc = awire->sbc;
       return True;
    }
    default:
-- 
1.7.4.1

Re: Make swap event handling match the spec

[Jesse Barnes]

On Tue,  3 May 2011 12:21:23 -0700
Jesse Barnes <jbarnes-Y1mF5jBUw70BENJcbMCuUQ@public.gmane.org> wrote:

> Ian reminded me that we changed the spec to fit within an XEvent, but we
> never updated the code to match.  This set of patches (much simpler than
> the last) does just that.  Wrapping support can be added to Mesa if we
> really want 64 bit values, but that means checking the drawable sbc and
> adding whenver sbc hits 0.

Apparently I've confused git send-email...  3/4 and 4/4 are for the
server and 5/6 and 6/6 are for Mesa.

-- 
Jesse Barnes, Intel Open Source Technology Center
_______________________________________________
xorg-devel-go0+a7rfsptAfugRpC6u6w@public.gmane.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH] glxproto: make GLX swap event struct match spec

[Keith Packard]

[-- Attachment #1.1: Type: text/plain, Size: 453 bytes --]

On Tue,  3 May 2011 12:21:24 -0700, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:

> We only spec a 32 bit swap count, so drop the high sbc field.

You're missing the explicit 16-bit padding field after 'event_type'

The documented encoding
http://www.opengl.org/registry/specs/INTEL/swap_event.txt needs to be
fixed to match this, it has the padding at the end which leaves most of
the structure mis-aligned.

-- 
keith.packard@intel.com

[-- Attachment #1.2: Type: application/pgp-signature, Size: 189 bytes --]

[-- Attachment #2: Type: text/plain, Size: 156 bytes --]

_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [PATCH] glxproto: make GLX swap event struct match spec

[Jesse Barnes]

On Tue, 03 May 2011 13:54:38 -0700
Keith Packard <keithp@keithp.com> wrote:

> On Tue,  3 May 2011 12:21:24 -0700, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> 
> > We only spec a 32 bit swap count, so drop the high sbc field.
> 
> You're missing the explicit 16-bit padding field after 'event_type'
> 
> The documented encoding
> http://www.opengl.org/registry/specs/INTEL/swap_event.txt needs to be
> fixed to match this, it has the padding at the end which leaves most of
> the structure mis-aligned.

Right, another case where we updated the spec incorrectly then failed
to make the code match the broken definition (the complete enums also
need to match the final values, which are correct in the first part of
the spec).  Yay for divergence.

-- 
Jesse Barnes, Intel Open Source Technology Center

Re: [PATCH] glxproto: make GLX swap event struct match spec

[Jesse Barnes]

On Tue, 3 May 2011 14:02:31 -0700
Jesse Barnes <jbarnes-Y1mF5jBUw70BENJcbMCuUQ@public.gmane.org> wrote:

> On Tue, 03 May 2011 13:54:38 -0700
> Keith Packard <keithp-aN4HjG94KOLQT0dZR+AlfA@public.gmane.org> wrote:
> 
> > On Tue,  3 May 2011 12:21:24 -0700, Jesse Barnes <jbarnes-Y1mF5jBUw70BENJcbMCuUQ@public.gmane.org> wrote:
> > 
> > > We only spec a 32 bit swap count, so drop the high sbc field.
> > 
> > You're missing the explicit 16-bit padding field after 'event_type'
> > 
> > The documented encoding
> > http://www.opengl.org/registry/specs/INTEL/swap_event.txt needs to be
> > fixed to match this, it has the padding at the end which leaves most of
> > the structure mis-aligned.
> 
> Right, another case where we updated the spec incorrectly then failed
> to make the code match the broken definition (the complete enums also
> need to match the final values, which are correct in the first part of
> the spec).  Yay for divergence.

Fixed version below.

-- 
Jesse Barnes, Intel Open Source Technology Center

>From f5403828c68af0f12c79c0504df0a4781ca84b2b Mon Sep 17 00:00:00 2001
From: Jesse Barnes <jbarnes-Y1mF5jBUw70BENJcbMCuUQ@public.gmane.org>
Date: Tue, 3 May 2011 12:14:10 -0700
Subject: [PATCH] glxproto: make GLX swap event struct match spec

We only spec a 32 bit swap count, so drop the high sbc field.  Also make
the padding explicit (most compilers would have already done this since
it's not a packed structure, but making it explicit should prevent
surprises in the future).

Signed-off-by: Jesse Barnes <jbarnes-Y1mF5jBUw70BENJcbMCuUQ@public.gmane.org>
---
 configure.ac |    2 +-
 glxproto.h   |    4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/configure.ac b/configure.ac
index d88e6df..a3047e4 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1,5 +1,5 @@
 AC_PREREQ([2.60])
-AC_INIT([GLProto], [1.4.12], [https://bugs.freedesktop.org/enter_bug.cgi?product=xorg])
+AC_INIT([GLProto], [1.4.13], [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 0ff44e3..dfa0647 100644
--- a/glxproto.h
+++ b/glxproto.h
@@ -1375,13 +1375,13 @@ typedef struct {
     BYTE pad;
     CARD16 sequenceNumber B16;
     CARD16 event_type B16;
+    CARD16 pad2;
     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;
+    CARD32 sbc B32;
 } xGLXBufferSwapComplete;
 
 /************************************************************************/
-- 
1.7.4.1


_______________________________________________
xorg-devel-go0+a7rfsptAfugRpC6u6w@public.gmane.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH] dri2proto: make DRI2 swap event match GLX spec

[Jesse Barnes]

Updated with explicit padding.

-- 
Jesse Barnes, Intel Open Source Technology Center

>From 0ca3778de195a82087d0f07415a1cf8fc94f5b0a Mon Sep 17 00:00:00 2001
From: Jesse Barnes <jbarnes@virtuousgeek.org>
Date: Tue, 3 May 2011 12:14:58 -0700
Subject: [PATCH] dri2proto: make DRI2 swap event match GLX spec

We only spec a 32 bit sbc count, so drop the high bits.  Also make the
padding explicit.

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 configure.ac |    2 +-
 dri2proto.h  |    4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/configure.ac b/configure.ac
index 5b78d6b..9505f56 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1,5 +1,5 @@
 AC_PREREQ([2.60])
-AC_INIT([DRI2Proto], [2.3], [https://bugs.freedesktop.org/enter_bug.cgi?product=xorg])
+AC_INIT([DRI2Proto], [2.4], [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 9708a4a..ff76355 100644
--- a/dri2proto.h
+++ b/dri2proto.h
@@ -290,13 +290,13 @@ typedef struct {
     CARD8 pad;
     CARD16 sequenceNumber B16;
     CARD16 event_type B16;
+    CARD16 pad2;
     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;
+    CARD32 sbc B32;
 } xDRI2BufferSwapComplete;
 #define sz_xDRI2BufferSwapComplete 32
 
-- 
1.7.4.1

Re: [PATCH] glxproto: make GLX swap event struct match spec

[Keith Packard]

[-- Attachment #1.1: Type: text/plain, Size: 266 bytes --]

On Tue, 3 May 2011 14:08:58 -0700, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:

> Fixed version below.

Reviewed-by: Keith Packard <keithp@keithp.com>

(assuming that the GLX protocol specification gets updated to match :-)

-- 
keith.packard@intel.com

[-- Attachment #1.2: Type: application/pgp-signature, Size: 189 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] glxproto: make GLX swap event struct match spec

[Jesse Barnes]

On Tue, 03 May 2011 14:15:30 -0700
Keith Packard <keithp@keithp.com> wrote:

> On Tue, 3 May 2011 14:08:58 -0700, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> 
> > Fixed version below.
> 
> Reviewed-by: Keith Packard <keithp@keithp.com>
> 
> (assuming that the GLX protocol specification gets updated to match :-)

Yeah, Ian is fixing up the padding there to match what our existing gcc
compiled implementations actually do (along with fixing the enums in
the second half to match the first :)

I'll push this out.

Thanks,
-- 
Jesse Barnes, Intel Open Source Technology Center

Re: [PATCH] glxproto: make GLX swap event struct match spec

[Ian Romanick]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 05/03/2011 02:08 PM, Jesse Barnes wrote:
> On Tue, 3 May 2011 14:02:31 -0700
> Jesse Barnes <jbarnes-Y1mF5jBUw70BENJcbMCuUQ@public.gmane.org> wrote:
> 
>> On Tue, 03 May 2011 13:54:38 -0700
>> Keith Packard <keithp-aN4HjG94KOLQT0dZR+AlfA@public.gmane.org> wrote:
>>
>>> On Tue,  3 May 2011 12:21:24 -0700, Jesse Barnes <jbarnes-Y1mF5jBUw70BENJcbMCuUQ@public.gmane.org> wrote:
>>>
>>>> We only spec a 32 bit swap count, so drop the high sbc field.
>>>
>>> You're missing the explicit 16-bit padding field after 'event_type'
>>>
>>> The documented encoding
>>> http://www.opengl.org/registry/specs/INTEL/swap_event.txt needs to be
>>> fixed to match this, it has the padding at the end which leaves most of
>>> the structure mis-aligned.
>>
>> Right, another case where we updated the spec incorrectly then failed
>> to make the code match the broken definition (the complete enums also
>> need to match the final values, which are correct in the first part of
>> the spec).  Yay for divergence.
> 
> Fixed version below.
> 

Does this need the "B16" cruft?
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iEYEARECAAYFAk3BzwQACgkQX1gOwKyEAw/SowCfaAm1gxkowryhD2ku9oPMlaUY
YyoAniZgXMFszpUBwnJH17JDyZwtsOw+
=k7tr
-----END PGP SIGNATURE-----
_______________________________________________
xorg-devel-go0+a7rfsptAfugRpC6u6w@public.gmane.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [Mesa-dev] [PATCH] glxproto: make GLX swap event struct match spec

[Ian Romanick]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 05/03/2011 12:21 PM, Jesse Barnes wrote:
> We only spec a 32 bit swap count, so drop the high sbc field.
> 
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>

Is there any way we could do this and NOT break building older versions
of Mesa?  I'd like to be able to build 7.9, 7.10, and master on my
system without having two different versions of glproto.

> ---
>  configure.ac |    2 +-
>  glxproto.h   |    3 +--
>  2 files changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/configure.ac b/configure.ac
> index d88e6df..a3047e4 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -1,5 +1,5 @@
>  AC_PREREQ([2.60])
> -AC_INIT([GLProto], [1.4.12], [https://bugs.freedesktop.org/enter_bug.cgi?product=xorg])
> +AC_INIT([GLProto], [1.4.13], [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 0ff44e3..a6018a1 100644
> --- a/glxproto.h
> +++ b/glxproto.h
> @@ -1380,8 +1380,7 @@ typedef struct {
>      CARD32 ust_lo B32;
>      CARD32 msc_hi B32;
>      CARD32 msc_lo B32;
> -    CARD32 sbc_hi B32;
> -    CARD32 sbc_lo B32;
> +    CARD32 sbc B32;
>  } xGLXBufferSwapComplete;
>  
>  /************************************************************************/

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iEYEARECAAYFAk3B0HsACgkQX1gOwKyEAw+27gCeLgSzv2Yjq7NQF+3QjeoXS8J0
qoQAn3n+Q8ujE3JFwpAyCM9TYtZ13wy0
=OkZ2
-----END PGP SIGNATURE-----

Re: [Mesa-dev] Make swap event handling match the spec

[Ian Romanick]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 05/03/2011 12:21 PM, Jesse Barnes wrote:
> Ian reminded me that we changed the spec to fit within an XEvent, but we
> never updated the code to match.  This set of patches (much simpler than
> the last) does just that.  Wrapping support can be added to Mesa if we
> really want 64 bit values, but that means checking the drawable sbc and
> adding whenver sbc hits 0.

3/4, 4/4, 5/6, and 6/6:

Reviewed-by: Ian Romanick <ian.d.romanick@intel.com>
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iEYEARECAAYFAk3B0pAACgkQX1gOwKyEAw8JmwCeKgOPMC733wwz5zCP1OuSYX7f
h+wAn05XZhh6akmfUyuaJ2THREQEaDsw
=FzuP
-----END PGP SIGNATURE-----

Re: [PATCH] glxproto: make GLX swap event struct match spec

[Jesse Barnes]

On Wed, 04 May 2011 15:17:31 -0700
Ian Romanick <idr@freedesktop.org> wrote:

> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> On 05/03/2011 12:21 PM, Jesse Barnes wrote:
> > We only spec a 32 bit swap count, so drop the high sbc field.
> > 
> > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> 
> Is there any way we could do this and NOT break building older versions
> of Mesa?  I'd like to be able to build 7.9, 7.10, and master on my
> system without having two different versions of glproto.
> 

We did that the last time glproto bumped (kept the req at 1.4.10 and
added ifdefs), but that added bugs that we didn't find for awhile, so I
wanted to try to avoid it this time.  Another option for you would be
to build 7.9, 7.10, and master against different install roots with
PKG_CONFIG_PATH set appropriately...

-- 
Jesse Barnes, Intel Open Source Technology Center

Re: [PATCH] glxproto: make GLX swap event struct match spec

[Jesse Barnes]

On Wed, 4 May 2011 16:16:37 -0700
Jesse Barnes <jbarnes@virtuousgeek.org> wrote:

> On Wed, 04 May 2011 15:17:31 -0700
> Ian Romanick <idr@freedesktop.org> wrote:
> 
> > -----BEGIN PGP SIGNED MESSAGE-----
> > Hash: SHA1
> > 
> > On 05/03/2011 12:21 PM, Jesse Barnes wrote:
> > > We only spec a 32 bit swap count, so drop the high sbc field.
> > > 
> > > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> > 
> > Is there any way we could do this and NOT break building older versions
> > of Mesa?  I'd like to be able to build 7.9, 7.10, and master on my
> > system without having two different versions of glproto.
> > 
> 
> We did that the last time glproto bumped (kept the req at 1.4.10 and
> added ifdefs), but that added bugs that we didn't find for awhile, so I
> wanted to try to avoid it this time.  Another option for you would be
> to build 7.9, 7.10, and master against different install roots with
> PKG_CONFIG_PATH set appropriately...

Or just backport the fix to 7.x :)  The server is only sending 32 bytes
regardless, so having the fix in older client library versions will
give either the right sbc number (if the server is new) or 0 if the
server is old (unless you've wrapped the sbc_lo field and sbc_hi is
set).  So an improvement either way.

-- 
Jesse Barnes, Intel Open Source Technology Center

Re: [Mesa-dev] [PATCH] glxproto: make GLX swap event struct match spec

[Dave Airlie]

On Wed, 2011-05-04 at 16:16 -0700, Jesse Barnes wrote:
> On Wed, 04 May 2011 15:17:31 -0700
> Ian Romanick <idr@freedesktop.org> wrote:
> 
> > -----BEGIN PGP SIGNED MESSAGE-----
> > Hash: SHA1
> > 
> > On 05/03/2011 12:21 PM, Jesse Barnes wrote:
> > > We only spec a 32 bit swap count, so drop the high sbc field.
> > > 
> > > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> > 
> > Is there any way we could do this and NOT break building older versions
> > of Mesa?  I'd like to be able to build 7.9, 7.10, and master on my
> > system without having two different versions of glproto.
> > 
> 
> We did that the last time glproto bumped (kept the req at 1.4.10 and
> added ifdefs), but that added bugs that we didn't find for awhile, so I
> wanted to try to avoid it this time.  Another option for you would be
> to build 7.9, 7.10, and master against different install roots with
> PKG_CONFIG_PATH set appropriately...
> 

How about you try again, with an increased emphasis on not adding bugs,
now that you know what you did wrong the first time?

Dave.

Re: [Mesa-dev] [PATCH] glxproto: make GLX swap event struct match spec

[Jesse Barnes]

On Thu, 05 May 2011 09:32:46 +1000
Dave Airlie <airlied@redhat.com> wrote:

> On Wed, 2011-05-04 at 16:16 -0700, Jesse Barnes wrote:
> > On Wed, 04 May 2011 15:17:31 -0700
> > Ian Romanick <idr@freedesktop.org> wrote:
> > 
> > > -----BEGIN PGP SIGNED MESSAGE-----
> > > Hash: SHA1
> > > 
> > > On 05/03/2011 12:21 PM, Jesse Barnes wrote:
> > > > We only spec a 32 bit swap count, so drop the high sbc field.
> > > > 
> > > > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> > > 
> > > Is there any way we could do this and NOT break building older versions
> > > of Mesa?  I'd like to be able to build 7.9, 7.10, and master on my
> > > system without having two different versions of glproto.
> > > 
> > 
> > We did that the last time glproto bumped (kept the req at 1.4.10 and
> > added ifdefs), but that added bugs that we didn't find for awhile, so I
> > wanted to try to avoid it this time.  Another option for you would be
> > to build 7.9, 7.10, and master against different install roots with
> > PKG_CONFIG_PATH set appropriately...
> > 
> 
> How about you try again, with an increased emphasis on not adding bugs,
> now that you know what you did wrong the first time?

How about you look at git and see what happened last time?

We added some dri2 proto requests, and people wanted to build with old
versions w/o the new requests.  So they added some ifdefs but didn't
check all the combos (now not just old server/new server, but
multiplied by two) and things were broken for awhile, and it was easy
to get breakage without even noticing (I found several bugs for people
related to invalidation that were solely due to bad builds).

-- 
Jesse Barnes, Intel Open Source Technology Center

Re: [Mesa-dev] [PATCH] glxproto: make GLX swap event struct match spec

[Jesse Barnes]

On Wed, 4 May 2011 17:49:37 -0700
Jesse Barnes <jbarnes-Y1mF5jBUw70BENJcbMCuUQ@public.gmane.org> wrote:
> How about you look at git and see what happened last time?
> 
> We added some dri2 proto requests, and people wanted to build with old
> versions w/o the new requests.  So they added some ifdefs but didn't
> check all the combos (now not just old server/new server, but
> multiplied by two) and things were broken for awhile, and it was easy
> to get breakage without even noticing (I found several bugs for people
> related to invalidation that were solely due to bad builds).

(For those who don't want to look through git and the history: the
problem is that making the builds use old and new means you can build
client and server with different proto versions and not even notice.
That makes debugging all the harder because everything seems to be ok
but you're now taking untested paths on the client and/or server side
due to #ifdefs and protocol mismatches.)

-- 
Jesse Barnes, Intel Open Source Technology Center
_______________________________________________
xorg-devel-go0+a7rfsptAfugRpC6u6w@public.gmane.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel