orangeduck / Corange

Pure C Game Engine
http://www.youtube.com/watch?v=482GxqTWXtA
Other
1.79k stars 199 forks source link

SDL_GL_GetProcAddress incorrectly used in SDL_local.c macro. #25

Open ghost opened 7 years ago

ghost commented 7 years ago

The SDL_GL_LoadExtension macro is looking for NULL return to determine extension availability, that could have undefined behavior.

From the SDL2 reference manual "if you look up a function that doesn't exist, you'll get a non-NULL result that is NOT safe to call", for Linux at least.

A more correct form would be something like:

define SDL_GL_LoadExtension(type, name) \

if (SDL_GL_ExtensionSupported(#name)) \
    name = (type)SDL_GL_GetProcAddress(#name); \
else if (SDL_GL_ExtensionSupported(#name"EXT")) \
    name = (type)SDL_GL_GetProcAddress(#name"EXT"); \
else if (SDL_GL_ExtensionSupported(#name"ARB")) \
    name = (type)SDL_GL_GetProcAddress(#name"ARB"); \
else \
    fprintf(stderr, "Failed to load any of function '%s*')\n", #name); 

In any case, I found this problem because when trying Corange I can't load any of the extensions. For instance, glCreateProgram* does not exist in the extension list of glGetString(GL_EXTENSIONS) and SDL_GL_ExtensionSupported() returns false for it as well.

The context is a 4.5.0, im on nvidias latest propietary drivers (gtx880). Any idea why?

orangeduck commented 7 years ago

Opps didn't know that - very suprising that this is the API since both dlsym and GetProcAddress return NULL on failure. So I guess the only way to check if an extension is present is by using the extensions list?

What platform are you running? I've only seen this when people are using the Mesa drivers - but OpenGL can fail in all kinds of bizzare and wonderful ways so I am not that suprised.

ghost commented 7 years ago

Just call SDL_GL_ExtensionSupported on all platforms. It will give you a guaranteed result, if it returns true then the getproc must return a valid value.

Yes, this is one of many issues addressed in Vulkan; the instance creation will fail if any of the requested extensions are not available. The API user must first check availability before creating the instance and then provide a list of guaranteed "will work" extensions and layers.

EDIT: archlinux, i7, nvidia gtx880, proprietary driver 375.26-6.

ghost commented 7 years ago

By the way, SDL does most of this already, I'm not sure if maybe SDL is outdated or why you opted to fetch the prototypes manually. It's already using glext, check out SDL_opengl_glext.h.

You can also include glext manually and add #define GL_GLEXT_PROTOTYPES 1 before including it. I have no idea how it works on different platforms or for what extensions, but it works for all my opengl uses. Vulkan has a similar macro check for VK_PROTOTYPES.

orangeduck commented 7 years ago

If I recally correctly, SDL_opengl_glext.h only provides some of the prototypes for the most common extensions but not all. Either way - this is how it used to have to be done in SDL1 but probably the situation has been updated for SDL2 and I've not updated the code properly.