diff mbox

glxproto: make GLX swap event struct match spec

Message ID 1304450489-31415-2-git-send-email-jbarnes@virtuousgeek.org (mailing list archive)
State New, archived
Headers show

Commit Message

Jesse Barnes May 3, 2011, 7:21 p.m. UTC
We only spec a 32 bit swap count, so drop the high sbc field.

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

Comments

Keith Packard May 3, 2011, 8:54 p.m. UTC | #1
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.
Jesse Barnes May 3, 2011, 9:02 p.m. UTC | #2
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.
Ian Romanick May 4, 2011, 10:17 p.m. UTC | #3
-----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-----
Jesse Barnes May 4, 2011, 11:16 p.m. UTC | #4
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 May 4, 2011, 11:21 p.m. UTC | #5
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.
David Airlie May 4, 2011, 11:32 p.m. UTC | #6
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.
Jesse Barnes May 5, 2011, 12:49 a.m. UTC | #7
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 May 5, 2011, 12:54 a.m. UTC | #8
On Wed, 4 May 2011 17:49:37 -0700
Jesse Barnes <jbarnes@virtuousgeek.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.)
diff mbox

Patch

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;
 
 /************************************************************************/