[Piglit] [PATCH v2 10/13] piglit-dispatch: Switch to using piglit-dispatch instead of GLEW.

[Paul Berry]

On 16 March 2012 15:00, Paul Berry <stereotype441 at gmail.com> wrote:

> On 16 March 2012 12:04, Jose Fonseca <jfonseca at vmware.com> wrote:
>
>>
>>
>> ----- 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.
>>
>> It's probably pointless if all GL defitions are done before.
>>
>> But note that Apple's glext.h is non standard:
>>
>>
>> https://github.com/apitrace/apitrace/blob/master/thirdparty/khronos/GL/glext.patch
>>
>> It's probably better to replicate this on piglit-dispatch too.
>>
>> Jose
>>
>
> 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.
>

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.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/piglit/attachments/20120320/98296083/attachment.htm>