Fix static GLEW linkage #54
Conversation
In glew.h, it is required to define GLEW_STATIC when using the static library, however USD never sets this flag, thus previously all glew linkage was assumed to be dynamic with "dllimport" linkage. Now the CMake FindGlew module sets this flag when running in Win32 and the static library is discovered (this is determined by convention of the "s" or "sd" suffix). Also, GLEW debug libraries are now supported (e.g. glew32[s]d.lib).
|
Don't merge this yet, it needs another tweak to be correct. |
| @@ -62,6 +62,12 @@ if (WIN32) | |||
| PATH_SUFFIXES | |||
| Release/${ARCH} | |||
| DOC "The GLEW library") | |||
|
|
|||
| if ("${GLEW_LIBRARY}" MATCHES "glew32s(d|)") | |||
| add_definitions("/DGLEW_STATIC") | |||
There was a problem hiding this comment.
I believe this should be GLEW_BUILD (not GLEW_STATIC) -- I will confirm and update the PR.
There was a problem hiding this comment.
GLEW_BUILD is supposed to be defined when you are building the GLEW library itself IIRC.
BTW, usd-build-club is currently working around this issue by building the glew dll instead of the static lib. Previously, it was creating static glew...
There was a problem hiding this comment.
Yeah, that's what I thought from the docs too, however Windows has a quirk where global variables declared "extern" will not be shared over library boundaries unless they are declared with dllexport declspec. Looking at the GLEW_STATIC/GLEW_BUILD preprocessor logic, the way to get dllexport is by declaring GLEW_BUILD.
So it seems this is the only way to make it work, unless you are ok with unique glew state per-library, but the USD imaging init pattern requires shared state, since Glf calls glewInit() (via GlfGlewInit); so without shared state, Glf will be the only library that gets initialized correctly.
There was a problem hiding this comment.
Oh, the unstated bit in my comment is that glew stores GL functions as global function pointer variables of the form __glewApiName -- e.g. glDrawBuffers is internally stored as a global function pointer called __glewDrawBuffers, thus hitting the scenario I described above.
There was a problem hiding this comment.
/*
* GLEW_STATIC is defined for static library.
* GLEW_BUILD is defined for building the DLL library.
*/
#ifdef GLEW_STATIC
# define GLEWAPI extern
#else
# ifdef GLEW_BUILD
# define GLEWAPI extern __declspec(dllexport)
# else
# define GLEWAPI extern __declspec(dllimport)
# endif
#endif
If you don't have GLEW_STATIC, and you do have GLEW_BUILD, USD will attempt to re-export the symbols, so if you use GLEW yourself, and link glf, then the linker will report duplicate symbols? I agree that these macros in glew.h are bogus, extern is seldom what you want in this case.
In Zeno, I worked around this issue by requiring Zeno libs to link the PlatformGL library, and never glew directly. Maybe this hiccough needs to be documented...
There was a problem hiding this comment.
Yeah, seems wrong -- and thinking about this more this morning, I believe the real problem is static linkage of Glew into a dynamic shared library. We should really just be linking Glew dynamically, then everything would work as expected... aside from the linker errors I get from dynamic linking when combining USD and OpenSubdiv (but that's just something that needs to be fixed).
There was a problem hiding this comment.
Yeah, that's why build-club is using dynamic now. Yesterday, I did a build of Pixar's dev_win_ip with @JamieKenyonFoundry's latest dev_win_ip merged on top, and usd-build-club built GLEW and OpenSubdiv and it all worked :) What link problem are you seeing in OSD?
There was a problem hiding this comment.
Linker error (symbol not found for __glewXXX) when building Hd, the error originates from OpenSubdiv Osd classes.
|
I'm getting opensubdiv related errors (missing__glesXxxxx symbols) when Was the build working without Jamie's changes? On Monday, October 24, 2016, Nick Porcino [email protected] wrote:
|
|
Hmmm, I had a local change. When I back it out, as shown in the diff below, I get the same failure. This is complicated :\ |
|
What was your local change? |
|
Yeah, I wondered about that too, but given the fact that Windows DLLs On Saturday, October 22, 2016, Nick Porcino [email protected]
|
|
I'm seeing this now too! |
|
@JamieKenyonFoundry The OpenSubdiv fix is here: PixarAnimationStudios/OpenSubdiv#905 |
|
Thats awesome! Thanks @jcowles and George! |
|
Closing this pull request along with #53, please reopen if this is still an issue. Thanks! |
…mni_plugin) (PixarAnimationStudios#54) * Work in progress on UsdDeltaBuilder * Incremental delta tests in test_omni_plugin * Incremental delta code clean-up
In glew.h, it is required to define GLEW_STATIC when using the static library,
however USD never sets this flag, thus previously all glew linkage was assumed
to be dynamic with "dllimport" linkage.
Now the CMake FindGlew module sets this flag when running in Win32 and the
static library is discovered (this is determined by convention of the "s" or "sd"
suffix).
Also, GLEW debug libraries are now supported (e.g. glew32[s]d.lib).
Addresses Issue #53