Opinion sought on a bunch of fixes

[Nigel Stewart-2]

Hello all,

This proposed patch is to resolve various bugs
tracked on the GLEW sourceforge bug tracker:

Bugs:

2237657 - Compilation warnings for OSX
2272725 - aglSetCurrentContext compiler warning on OSX
2544715 - Workaround for Mesa glut compatibility
2237650 - Cygwin support for opengl32

Patches:

2489303 - [PATCH] Deprecation fix for Mac OS X (*)
2320783 - [PATCH] Removes a g++ warning

(*) Patch 2489303 is a duplicate of bug 2237657

Of these, probably the most interesting is the Cygwin
handling.  In my view, GLEW should default to the Win32
OpenGL conventions on Cygwin, rather than GLX.  But changing
this will certainly break code that depends on the status
quo.  Essentially, the change will treat Cygwin and Ming32
builds the same way.

 > The GLEW handling of __CYGWIN__ appears to be intended for linking with the
 > unix-style GL, rather than the native windows opengl32.
 >
 > This affects such things as the APIENTRY, GLAPI and CALLBACK definitions
 > which are not compatible between GLX and WGL OpenGL on Cygwin.
 >
 > My preference would be that native windows WGL OpenGL be supported by
 > Cygwin, with perhaps a conditional GLEW_GLX for Cygwin/X11/GLX.

Thoughts?

- Nigel

------------

Index: src/visualinfo.c
===================================================================
--- src/visualinfo.c (revision 545)
+++ src/visualinfo.c (working copy)
@@ -1056,7 +1056,7 @@
    aglDestroyPixelFormat(pf);
    /*aglSetDrawable(ctx, GetWindowPort(wnd));*/
    ctx->octx = aglGetCurrentContext();
-  if (NULL == aglSetCurrentContext(ctx->ctx)) return GL_TRUE;
+  if (GL_FALSE == aglSetCurrentContext(ctx->ctx)) return GL_TRUE;
    return GL_FALSE;
  }

Index: auto/src/wglew_head.h
===================================================================
--- auto/src/wglew_head.h (revision 545)
+++ auto/src/wglew_head.h (working copy)
@@ -8,7 +8,7 @@

  #define __wglext_h_

-#if !defined(APIENTRY) && !defined(__CYGWIN__)
+#if !defined(APIENTRY)
  #  ifndef WIN32_LEAN_AND_MEAN
  #    define WIN32_LEAN_AND_MEAN 1
  #  endif
Index: auto/src/glew_head.c
===================================================================
--- auto/src/glew_head.c (revision 545)
+++ auto/src/glew_head.c (working copy)
@@ -35,12 +35,29 @@
  #endif /* GLEW_MX */

  #if defined(__APPLE__)
-#include <mach-o/dyld.h>
  #include <stdlib.h>
  #include <string.h>
+#include <AvailabilityMacros.h>

+#ifdef MAC_OS_X_VERSION_10_3
+
+#include <dlfcn.h>
+
  void* NSGLGetProcAddress (const GLubyte *name)
  {
+  static void* image = NULL;
+  if (NULL == image)
+  {
+    image = dlopen("/System/Library/Frameworks/OpenGL.framework/Versions/Current/OpenGL", RTLD_LAZY);
+  }
+  return image ? dlsym(image, (const char*)name) : NULL;
+}
+#else
+
+#include <mach-o/dyld.h>
+
+void* NSGLGetProcAddress (const GLubyte *name)
+{
    static const struct mach_header* image = NULL;
    NSSymbol symbol;
    char* symbolName;
@@ -59,6 +76,7 @@
    free(symbolName);
    return symbol ? NSAddressOfSymbol(symbol) : NULL;
  }
+#endif /* MAC_OS_X_VERSION_10_3 */
  #endif /* __APPLE__ */

  #if defined(__sgi) || defined (__sun)
Index: auto/src/glewinfo_tail.c
===================================================================
--- auto/src/glewinfo_tail.c (revision 545)
+++ auto/src/glewinfo_tail.c (working copy)
@@ -199,7 +199,7 @@
    aglDestroyPixelFormat(pf);
    /*aglSetDrawable(ctx, GetWindowPort(wnd));*/
    octx = aglGetCurrentContext();
-  if (NULL == aglSetCurrentContext(ctx)) return GL_TRUE;
+  if (GL_FALSE == aglSetCurrentContext(ctx)) return GL_TRUE;
    return GL_FALSE;
  }

Index: auto/src/glew_head.h
===================================================================
--- auto/src/glew_head.h (revision 545)
+++ auto/src/glew_head.h (working copy)
@@ -28,7 +28,7 @@
  /* <windef.h> */
  #ifndef APIENTRY
  #define GLEW_APIENTRY_DEFINED
-#  if defined(__MINGW32__)
+#  if defined(__MINGW32__) || defined(__CYGWIN__)
  #    define APIENTRY __stdcall
  #  elif (_MSC_VER >= 800) || defined(_STDCALL_SUPPORTED) || defined(__BORLANDC__)
  #    define APIENTRY __stdcall
@@ -37,14 +37,14 @@
  #  endif
  #endif
  #ifndef GLAPI
-#  if defined(__MINGW32__)
+#  if defined(__MINGW32__) || defined(__CYGWIN__)
  #    define GLAPI extern
  #  endif
  #endif
  /* <winnt.h> */
  #ifndef CALLBACK
  #define GLEW_CALLBACK_DEFINED
-#  if defined(__MINGW32__)
+#  if defined(__MINGW32__) || defined(__CYGWIN__)
  #    define CALLBACK __attribute__ ((__stdcall__))
  #  elif (defined(_M_MRX000) || defined(_M_IX86) || defined(_M_ALPHA) || defined(_M_PPC)) && !defined(MIDL_PASS)
  #    define CALLBACK __stdcall
@@ -81,7 +81,7 @@
  #endif

  #ifndef GLAPI
-#  if defined(__MINGW32__)
+#  if defined(__MINGW32__) || defined(__CYGWIN__)
  #    define GLAPI extern
  #  else
  #    define GLAPI WINGDIAPI
@@ -168,7 +168,7 @@
  typedef unsigned long long GLuint64EXT;
  #  endif
  #else
-#  if defined(__MINGW32__)
+#  if defined(__MINGW32__) || defined(__CYGWIN__)
  #include <inttypes.h>
  #  endif
  typedef int64_t GLint64EXT;
Index: auto/src/glew_init_glx.c
===================================================================
--- auto/src/glew_init_glx.c (revision 545)
+++ auto/src/glew_init_glx.c (working copy)
@@ -4,10 +4,12 @@
  {
    GLubyte* p;
    GLubyte* end;
+
+  if (glXGetCurrentDisplay == NULL) return GL_FALSE;
    GLuint len = _glewStrLen((const GLubyte*)name);
  /*   if (glXQueryExtensionsString == NULL || glXGetCurrentDisplay == NULL) return GL_FALSE; */
  /*   p = (GLubyte*)glXQueryExtensionsString(glXGetCurrentDisplay(), DefaultScreen(glXGetCurrentDisplay())); */
-  if (glXGetClientString == NULL || glXGetCurrentDisplay == NULL) return GL_FALSE;
+/*  if (glXGetClientString == NULL || glXGetCurrentDisplay == NULL) return GL_FALSE; */
    p = (GLubyte*)glXGetClientString(glXGetCurrentDisplay(), GLX_EXTENSIONS);
    if (0 == p) return GL_FALSE;
    end = p + _glewStrLen(p);


-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------

Index: src/visualinfo.c
===================================================================
--- src/visualinfo.c (revision 545)
+++ src/visualinfo.c (working copy)
@@ -1056,7 +1056,7 @@
   aglDestroyPixelFormat(pf);
   /*aglSetDrawable(ctx, GetWindowPort(wnd));*/
   ctx->octx = aglGetCurrentContext();
-  if (NULL == aglSetCurrentContext(ctx->ctx)) return GL_TRUE;
+  if (GL_FALSE == aglSetCurrentContext(ctx->ctx)) return GL_TRUE;
   return GL_FALSE;
 }
 
Index: auto/src/wglew_head.h
===================================================================
--- auto/src/wglew_head.h (revision 545)
+++ auto/src/wglew_head.h (working copy)
@@ -8,7 +8,7 @@
 
 #define __wglext_h_
 
-#if !defined(APIENTRY) && !defined(__CYGWIN__)
+#if !defined(APIENTRY)
 #  ifndef WIN32_LEAN_AND_MEAN
 #    define WIN32_LEAN_AND_MEAN 1
 #  endif
Index: auto/src/glew_head.c
===================================================================
--- auto/src/glew_head.c (revision 545)
+++ auto/src/glew_head.c (working copy)
@@ -35,12 +35,29 @@
 #endif /* GLEW_MX */
 
 #if defined(__APPLE__)
-#include <mach-o/dyld.h>
 #include <stdlib.h>
 #include <string.h>
+#include <AvailabilityMacros.h>
 
+#ifdef MAC_OS_X_VERSION_10_3
+
+#include <dlfcn.h>
+
 void* NSGLGetProcAddress (const GLubyte *name)
 {
+  static void* image = NULL;
+  if (NULL == image)
+  {
+    image = dlopen("/System/Library/Frameworks/OpenGL.framework/Versions/Current/OpenGL", RTLD_LAZY);
+  }
+  return image ? dlsym(image, (const char*)name) : NULL;
+}
+#else
+
+#include <mach-o/dyld.h>
+
+void* NSGLGetProcAddress (const GLubyte *name)
+{
   static const struct mach_header* image = NULL;
   NSSymbol symbol;
   char* symbolName;
@@ -59,6 +76,7 @@
   free(symbolName);
   return symbol ? NSAddressOfSymbol(symbol) : NULL;
 }
+#endif /* MAC_OS_X_VERSION_10_3 */
 #endif /* __APPLE__ */
 
 #if defined(__sgi) || defined (__sun)
Index: auto/src/glewinfo_tail.c
===================================================================
--- auto/src/glewinfo_tail.c (revision 545)
+++ auto/src/glewinfo_tail.c (working copy)
@@ -199,7 +199,7 @@
   aglDestroyPixelFormat(pf);
   /*aglSetDrawable(ctx, GetWindowPort(wnd));*/
   octx = aglGetCurrentContext();
-  if (NULL == aglSetCurrentContext(ctx)) return GL_TRUE;
+  if (GL_FALSE == aglSetCurrentContext(ctx)) return GL_TRUE;
   return GL_FALSE;
 }
 
Index: auto/src/glew_head.h
===================================================================
--- auto/src/glew_head.h (revision 545)
+++ auto/src/glew_head.h (working copy)
@@ -28,7 +28,7 @@
 /* <windef.h> */
 #ifndef APIENTRY
 #define GLEW_APIENTRY_DEFINED
-#  if defined(__MINGW32__)
+#  if defined(__MINGW32__) || defined(__CYGWIN__)
 #    define APIENTRY __stdcall
 #  elif (_MSC_VER >= 800) || defined(_STDCALL_SUPPORTED) || defined(__BORLANDC__)
 #    define APIENTRY __stdcall
@@ -37,14 +37,14 @@
 #  endif
 #endif
 #ifndef GLAPI
-#  if defined(__MINGW32__)
+#  if defined(__MINGW32__) || defined(__CYGWIN__)
 #    define GLAPI extern
 #  endif
 #endif
 /* <winnt.h> */
 #ifndef CALLBACK
 #define GLEW_CALLBACK_DEFINED
-#  if defined(__MINGW32__)
+#  if defined(__MINGW32__) || defined(__CYGWIN__)
 #    define CALLBACK __attribute__ ((__stdcall__))
 #  elif (defined(_M_MRX000) || defined(_M_IX86) || defined(_M_ALPHA) || defined(_M_PPC)) && !defined(MIDL_PASS)
 #    define CALLBACK __stdcall
@@ -81,7 +81,7 @@
 #endif
 
 #ifndef GLAPI
-#  if defined(__MINGW32__)
+#  if defined(__MINGW32__) || defined(__CYGWIN__)
 #    define GLAPI extern
 #  else
 #    define GLAPI WINGDIAPI
@@ -168,7 +168,7 @@
 typedef unsigned long long GLuint64EXT;
 #  endif
 #else
-#  if defined(__MINGW32__)
+#  if defined(__MINGW32__) || defined(__CYGWIN__)
 #include <inttypes.h>
 #  endif
 typedef int64_t GLint64EXT;
Index: auto/src/glew_init_glx.c
===================================================================
--- auto/src/glew_init_glx.c (revision 545)
+++ auto/src/glew_init_glx.c (working copy)
@@ -4,10 +4,12 @@
 {    
   GLubyte* p;
   GLubyte* end;
+
+  if (glXGetCurrentDisplay == NULL) return GL_FALSE;
   GLuint len = _glewStrLen((const GLubyte*)name);
 /*   if (glXQueryExtensionsString == NULL || glXGetCurrentDisplay == NULL) return GL_FALSE; */
 /*   p = (GLubyte*)glXQueryExtensionsString(glXGetCurrentDisplay(), DefaultScreen(glXGetCurrentDisplay())); */
-  if (glXGetClientString == NULL || glXGetCurrentDisplay == NULL) return GL_FALSE;
+/*  if (glXGetClientString == NULL || glXGetCurrentDisplay == NULL) return GL_FALSE; */
   p = (GLubyte*)glXGetClientString(glXGetCurrentDisplay(), GLX_EXTENSIONS);
   if (0 == p) return GL_FALSE;
   end = p + _glewStrLen(p);

------------------------------------------------------------------------------
Open Source Business Conference (OSBC), March 24-25, 2009, San Francisco, CA
-OSBC tackles the biggest issue in open source: Open Sourcing the Enterprise
-Strategies to boost innovation and cut costs with open source participation
-Receive a $600 discount off the registration fee with the source code: SFAD
http://p.sf.net/sfu/XcvMzF8H
_______________________________________________
glew-coders mailing list
glew-coders@...
https://lists.sourceforge.net/lists/listinfo/glew-coders

[tom fogal-3]

Nigel Stewart <nstewart@...> writes:
> This proposed patch is to resolve various bugs
> tracked on the GLEW sourceforge bug tracker:
[snip]
> Of these, probably the most interesting is the Cygwin
> handling.  In my view, GLEW should default to the Win32
> OpenGL conventions on Cygwin, rather than GLX.  But changing
> this will certainly break code that depends on the status
> quo.  Essentially, the change will treat Cygwin and Ming32
> builds the same way.
[snip]

Snipped patches I have no opinion on / don't feel qualified to talk
about.

> Index: auto/src/glew_head.h
> ===================================================================
> --- auto/src/glew_head.h (revision 545)
> +++ auto/src/glew_head.h (working copy)
> @@ -28,7 +28,7 @@
>   /* <windef.h> */
>   #ifndef APIENTRY
>   #define GLEW_APIENTRY_DEFINED
> -#  if defined(__MINGW32__)
> +#  if defined(__MINGW32__) || defined(__CYGWIN__)

Since this is now `if either MinGW or Cygwin', does turning it into `if
Windows' (i.e. _WIN32) make sense?

There might still be symbol exporting issues, but that seems like
something that should key on __GNUC__ and _MSC_VER (IIRC) -- that is,
it'd be compiler specific code, not OS/environment specific code.

> Index: auto/src/glew_init_glx.c
> ===================================================================
> --- auto/src/glew_init_glx.c (revision 545)
> +++ auto/src/glew_init_glx.c (working copy)
> @@ -4,10 +4,12 @@
>   {
>     GLubyte* p;
>     GLubyte* end;
> +
> +  if (glXGetCurrentDisplay == NULL) return GL_FALSE;
>     GLuint len = _glewStrLen((const GLubyte*)name);

This would require a C99-compliant compiler (mixed declaration and
code), yes?

I'm not saying that's a bad/harsh requirement (I don't think it'll
effect me/my users), but thought I'd mention.

>   /*   if (glXQueryExtensionsString == NULL || glXGetCurrentDisplay == NULL)
> return GL_FALSE; */
>   /*   p = (GLubyte*)glXQueryExtensionsString(glXGetCurrentDisplay(), Default
> Screen(glXGetCurrentDisplay())); */
> -  if (glXGetClientString == NULL || glXGetCurrentDisplay == NULL) return GL_
> FALSE;
> +/*  if (glXGetClientString == NULL || glXGetCurrentDisplay == NULL) return G
> L_FALSE; */

FWIW, I'd rather see these comment lines + the one commented in the
patch simply disappear.  It's a bit confusing, and they're immortalized
in version control anyway.

-tom

------------------------------------------------------------------------------
Open Source Business Conference (OSBC), March 24-25, 2009, San Francisco, CA
-OSBC tackles the biggest issue in open source: Open Sourcing the Enterprise
-Strategies to boost innovation and cut costs with open source participation
-Receive a $600 discount off the registration fee with the source code: SFAD
http://p.sf.net/sfu/XcvMzF8H
_______________________________________________
glew-coders mailing list
glew-coders@...
https://lists.sourceforge.net/lists/listinfo/glew-coders

[Nigel Stewart-2]

>> -#  if defined(__MINGW32__)
>> +#  if defined(__MINGW32__) || defined(__CYGWIN__)
>
> Since this is now `if either MinGW or Cygwin', does turning it into `if
> Windows' (i.e. _WIN32) make sense?

I think better granularity is a feature - in the sense that
__CYGWIN__ specifics can be grepped and tweaked more easily.
And I'd be concerned about polluting the preprocessor namespace
with things like _WIN32, that wouldn't otherwise be necessary.

>>     GLubyte* p;
>>     GLubyte* end;
>> +
>> +  if (glXGetCurrentDisplay == NULL) return GL_FALSE;
>>     GLuint len = _glewStrLen((const GLubyte*)name);
>
> This would require a C99-compliant compiler (mixed declaration and
> code), yes?

Good catch!  Yes indeed, that would be a bug.  There is somewhere
else that isn't -ansi compliant either, you just reminded me.

>>   /*   if (glXQueryExtensionsString == NULL || glXGetCurrentDisplay == NULL)
>> return GL_FALSE; */
>>   /*   p = (GLubyte*)glXQueryExtensionsString(glXGetCurrentDisplay(), Default
>> Screen(glXGetCurrentDisplay())); */
>> -  if (glXGetClientString == NULL || glXGetCurrentDisplay == NULL) return GL_
>> FALSE;
>> +/*  if (glXGetClientString == NULL || glXGetCurrentDisplay == NULL) return G
>> L_FALSE; */
>
> FWIW, I'd rather see these comment lines + the one commented in the
> patch simply disappear.  It's a bit confusing, and they're immortalized
> in version control anyway.

Yes, I agree entirely.

- Nigel

-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------

------------------------------------------------------------------------------
Open Source Business Conference (OSBC), March 24-25, 2009, San Francisco, CA
-OSBC tackles the biggest issue in open source: Open Sourcing the Enterprise
-Strategies to boost innovation and cut costs with open source participation
-Receive a $600 discount off the registration fee with the source code: SFAD
http://p.sf.net/sfu/XcvMzF8H
_______________________________________________
glew-coders mailing list
glew-coders@...
https://lists.sourceforge.net/lists/listinfo/glew-coders

[Nigel Stewart-2]

Going once, going twice...

- Nigel

-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------

------------------------------------------------------------------------------
Open Source Business Conference (OSBC), March 24-25, 2009, San Francisco, CA
-OSBC tackles the biggest issue in open source: Open Sourcing the Enterprise
-Strategies to boost innovation and cut costs with open source participation
-Receive a $600 discount off the registration fee with the source code: SFAD
http://p.sf.net/sfu/XcvMzF8H
_______________________________________________
glew-coders mailing list
glew-coders@...
https://lists.sourceforge.net/lists/listinfo/glew-coders

[Nigel Stewart-2]

> Going once, going twice...

The change finally went in today as Revision 548.

- Nigel

-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------

------------------------------------------------------------------------------
Apps built with the Adobe(R) Flex(R) framework and Flex Builder(TM) are
powering Web 2.0 with engaging, cross-platform capabilities. Quickly and
easily build your RIAs with Flex Builder, the Eclipse(TM)based development
software that enables intelligent coding and step-through debugging.
Download the free 60 day trial. http://p.sf.net/sfu/www-adobe-com
_______________________________________________
glew-coders mailing list
glew-coders@...
https://lists.sourceforge.net/lists/listinfo/glew-coders