On 16 March 2012 15:00, Paul Berry <span dir="ltr">&lt;<a href="mailto:stereotype441@gmail.com">stereotype441@gmail.com</a>&gt;</span> wrote:<br><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 <span dir="ltr">&lt;<a href="mailto:jfonseca@vmware.com" target="_blank">jfonseca@vmware.com</a>&gt;</span> wrote:<br><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">


<div><div><br>
<br>
----- Original Message -----<br>
&gt; On 03/12/2012 02:41 PM, Paul Berry wrote:<br>
&gt; &gt; Previous patches set up the piglit-dispatch infrastructure but did<br>
&gt; &gt; not<br>
&gt; &gt; use it.  This patch enables piglit-dispatch and uses it instead of<br>
&gt; &gt; GLEW.<br>
&gt; &gt;<br>
&gt; &gt; No piglit regressions on Intel SandyBridge (Linux) or Mac OSX.<br>
&gt;<br>
&gt; Nice. Only one more platform to conquer.<br>
&gt;<br>
&gt; &gt;  #if defined(USE_OPENGL)<br>
&gt; &gt; -#  include &quot;glew.h&quot;<br>
&gt; &gt; -   /* Include the real headers too, in case GLEW misses something.<br>
&gt; &gt; */<br>
&gt; &gt; +#  include &quot;piglit-dispatch.h&quot;<br>
&gt; &gt; +   /* Include the real headers too, in case piglit-dispatch misses<br>
&gt; &gt; something. */<br>
&gt; &gt;  #  ifdef __APPLE__<br>
&gt; &gt;  #          include &lt;OpenGL/gl.h&gt;<br>
&gt; &gt;  #          include &lt;OpenGL/glu.h&gt;<br>
&gt;<br>
&gt; Shouldn&#39;t Apple&#39;s &lt;OpenGL/gl.h&gt; be removed too?<br>
&gt; I think we discussed this before, but I don&#39;t remember the<br>
&gt; conclusion.<br>
<br>
</div></div>It&#39;s probably pointless if all GL defitions are done before.<br>
<br>
But note that Apple&#39;s glext.h is non standard:<br>
<br>
  <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><br>
<br>
It&#39;s probably better to replicate this on piglit-dispatch too.<br>
<span><font color="#888888"><br>
Jose<br>
</font></span></blockquote></div><br></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.<br>

<br>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&#39;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).<br>

<br>I&#39;ll do some investigation and see what&#39;s really going on.<br>
</blockquote></div><br>Ok, I did some investigation and the first startling fact I discovered was that glew.h doesn&#39;t define GLhandleARB correctly for Mac OSX--it defines it as an unsigned int regardless of the OS you&#39;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.<br>
<br>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&#39;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.<br>
<br>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&#39;t a problem), and all of the functions that take GLhandleARB as a parameter ignore the upper 32 bits (so (b) isn&#39;t a problem).  That means that the only harm in getting GLhandleARB wrong is that you break GetAttachedObjectsARB, and fortunately Piglit never exercises GetAttachedObjectsARB.<br>
<br><br>So, based on the fact that GLEW already gets GLhandleARB wrong, and the consequences are minimal, my proposal is to replicate GLEW&#39;s mistake in the initial commit of piglit-dispatch, and address this inconsistency in a later patch series.<br>