On 16 March 2012 15:00, Paul Berry <<a href="mailto:stereotype441@gmail.com">stereotype441@gmail.com</a>> wrote: <div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> <div class="HOEnZb"><div class="h5">On 16 March 2012 12:04, Jose Fonseca <<a href="mailto:jfonseca@vmware.com" target="_blank">jfonseca@vmware.com</a>> wrote: <div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> <div><div> ----- Original Message ----- > On 03/12/2012 02:41 PM, Paul Berry wrote: > > Previous patches set up the piglit-dispatch infrastructure but did > > not > > use it. This patch enables piglit-dispatch and uses it instead of > > GLEW. > > > > No piglit regressions on Intel SandyBridge (Linux) or Mac OSX. > > Nice. Only one more platform to conquer. > > > #if defined(USE_OPENGL) > > -# include "glew.h" > > - /* Include the real headers too, in case GLEW misses something. > > */ > > +# include "piglit-dispatch.h" > > + /* Include the real headers too, in case piglit-dispatch misses > > something. */ > > # ifdef __APPLE__ > > # include <OpenGL/gl.h> > > # include <OpenGL/glu.h> > > Shouldn't Apple's <OpenGL/gl.h> be removed too? > I think we discussed this before, but I don't remember the > conclusion. </div></div>It's probably pointless if all GL defitions are done before. But note that Apple's glext.h is non standard: <a href="https://github.com/apitrace/apitrace/blob/master/thirdparty/khronos/GL/glext.patch" target="_blank">https://github.com/apitrace/apitrace/blob/master/thirdparty/khronos/GL/glext.patch</a> It's probably better to replicate this on piglit-dispatch too. Jose </blockquote></div> </div></div>Interesting: on my mac, GLhandleARB is defined as a void *. Your patch defines it as an unsigned long (which is equivalent to void * from an ABI perspective). GL defines it as an unsigned int, which is certainly *not* equivalent on 64-bit systems. This throws a bit of a monkey wrench into things, since it breaks some of the aliases defined in the official gl.spec file. for example GetAttachedObjectsARB is marked as an alias for GetAttachedShaders, but this can't possibly work on mac if one of them takes a GLhandleARB * as its 4th argument, and the other takes a GLuint * as its 4th argument (since the two types have different sizes). I'll do some investigation and see what's really going on. </blockquote></div> Ok, I did some investigation and the first startling fact I discovered was that glew.h doesn't define GLhandleARB correctly for Mac OSX--it defines it as an unsigned int regardless of the OS you're building on. This means that on 64-bit Mac OSX, if you use GLEW to call any function that uses a GLhandleARB in its signature, the caller and callee disagree on the size of the function arguments. On 32-bit x86, that would have been a nightmare, since function arguments are passed on the stack with 32 bit alignment, so the caller and callee wouldn't even agree on the size of the stack frame. However, on 64-bit x86, the first 6 integer arguments to a function are passed in 64-bit registers, the situation is not so bad. The only things that can go wrong are: (a) when calling a function that returns a GLhandleARB, the upper 32 bits will be dropped, (b) when calling a function that takes a GLhandleARB as a parameter, the upper 32 bits will contain undefined data, and (c) GetAttachedObjectsARB totally fails to work properly, because one of its arguments is a pointer to an array of GLhandleARB. Fortunately, the good folks at Apple seem to have anticipated this (or they got lucky): on Mac OSX, all the handles generated are small integers (so (a) isn't a problem), and all of the functions that take GLhandleARB as a parameter ignore the upper 32 bits (so (b) isn't a problem). That means that the only harm in getting GLhandleARB wrong is that you break GetAttachedObjectsARB, and fortunately Piglit never exercises GetAttachedObjectsARB. So, based on the fact that GLEW already gets GLhandleARB wrong, and the consequences are minimal, my proposal is to replicate GLEW's mistake in the initial commit of piglit-dispatch, and address this inconsistency in a later patch series.