This bug report was migrated from our old Bugzilla tracker.

These attachments are available in the static archive:

• preload library source (preload.c, text/plain, 2017-08-04 11:38:00 +0000, 2181 bytes)

Reported in version: 2.0.5
Reported for operating system, platform: Linux, x86_64

Comments on the original bug report:

On 2017-08-04 11:38:00 +0000, Baldur Karlsson wrote:

Created attachment 2818
preload library source

In 36f40b8cc979 there was a call added in X11_GL_LoadLibrary to SDL_GL_DeduceMaxSupportedESProfile, which then calls SDL_GL_ExtensionSupported to check for ES compatibility, which ends up calling glGetString (for the version to see if it should use glGetStringi) and other things.

However this happens before any context is created or made current, so I don't believe it's valid to do. Probably in most cases these simple queries aren't context-dependent but just return fixed compile-time strings, so nothing blows up, but I ran into a crash with renderdoc because I require a context at that point.

To repro/prove this, compile a simple program like this:

#include <SDL2/SDL.h>

int main(int argc, char **argv)
{;
SDL_Init(SDL_INIT_VIDEO);
SDL_Window *win = SDL_CreateWindow("test", -32, -32, 32, 32, SDL_WINDOW_OPENGL|SDL_WINDOW_HIDDEN);
SDL_GLContext ctx = SDL_GL_CreateContext(win);
return 0;
}

And then run with SDL_OPENGL_LIBRARY=./libpreload.so pointing to the compiled preload.c from the attachment. It's a bit of an arbitrary repro case but the real one involves a full GL interceptor so this is nice and simple. It shows the order of the calls:

$ SDL_OPENGL_LIBRARY=./libpreload.so ./SDLtest
Inside hooked glGetString(1f02)
Inside hooked glGetString(1f03)
Inside hooked glGetString(1f02)
Inside hooked glGetString(1f03)
Inside hooked glGetString(1f02)
Inside hooked glGetString(1f03)
Inside hooked glXCreateContext
Inside hooked glGetString(1f00)
Inside hooked glGetString(1f02)
Inside hooked glGetString(1f03)
Inside hooked glGetString(1f02)
Inside hooked glGetString(1f03)
Inside hooked glGetString(1f02)
Inside hooked glGetString(1f03)
Inside hooked glXCreateContext

On 2017-08-08 00:49:08 +0000, Mark Callow wrote:

One fix is to remove calls to SDL_GL_DeduceMaxSupportedESProfile and instead in the *_GL_InitExtensions functions separately test for {GLX,WGL}_EXT_create_context_es_profile and {GLX,WGL}_EXT_create_context_es2_profile in that order.

This will fail if someone is trying to create an ES 3.1 or 3.2 context and only 3.0 & 2.0 are available.

To fully fix this, in the event that {GLX,WGL}_EXT_create_context_es_profile is found and the app is requesting an ES 3.1 or 3.2 context, *_GL_LoadLibrary will need to attempt to create a sacrificial context of the requested version which it can then delete. If it fails, it can go ahead and load the OpenGL ES library.

For now I've linked this to the bugs associated with 36f40b8cc979 which Sam reopened. However I think it is simpler to discuss the fix here. I'm not sure what is the utility of reopening those others.

On 2017-08-08 20:07:49 +0000, Mark Callow wrote:

Note that WIN_GL_InitExtensions is already creating a sacrificial context, for other reasons, which is why this issue only happens on Linux/GLX.

On 2017-08-08 20:37:07 +0000, Mark Callow wrote:

I don't understand why the code sample ends up calling glXCreateContext twice. The only call I have found in the source is from SDL_GL_CreateContext. There are no calls to it from any code invoked by SDL_CreateWindow. Anyone know?

On 2017-08-08 20:42:59 +0000, Baldur Karlsson wrote:

If you mean the original code sample I posted, it's because underneath SDL_Init in SDL_video.c there's a function called ShouldUseTextureFramebuffer() that creates a temporary window & context to check the vendor string on linux, then the code sample itself creates a context (the second call).

On 2017-08-09 05:25:35 +0000, Ryan C. Gordon wrote:

(Sorry if you get a lot of copies of this email, we're touching dozens of bug reports right now.)

Tagging a bunch of bugs as target-2.0.6.

This means we're in the final stretch for an official SDL 2.0.6 release! These are the bugs we really want to fix before shipping if humanly possible.

That being said, we don't promise to fix them because of this tag, we just want to make sure we don't forget to deal with them before we bless a final 2.0.6 release, and generally be organized about what we're aiming to ship. After some debate, we might just remove this tag again and deal with it for a later release.

Hopefully you'll hear more about this bug soon. If you have more information (including "this got fixed at some point, nevermind"), we would love to have you come add more information to the bug report when you have a moment.

Thanks!
--ryan.

On 2017-08-09 20:44:07 +0000, Mark Callow wrote:

(In reply to Baldur Karlsson from comment # 4)

If you mean the original code sample I posted, it's because underneath
SDL_Init in SDL_video.c there's a function called
ShouldUseTextureFramebuffer() that creates a temporary window & context to
check the vendor string on linux, then the code sample itself creates a
context (the second call).

Thanks.

It's unrelated to this issue but I think that code needs to be revisited. A very large number of OpenGL h/w accelerated devices are not being recognized as such. Intel's has been providing good open source drivers for their embedded graphics h/w for some years as a standard part of the Mesa distribution.

Related to this issue, it would probably be good to use the same sacrificial context to query the vendor string and finding out which es_compatibility extensions are supported. That would mean moving the vendor query.

On 2017-08-09 23:17:37 +0000, Sam Lantinga wrote:

Mark, would you mind creating a tested patch that covers this issue and the various OpenGL ES use cases?

Thanks!

On 2017-08-10 00:46:56 +0000, Mark Callow wrote:

Sam, I do not have suitable hardware, i.e. a GPU whose driver supports GLX_EXT_create_context_es{,2}_profile, which is only NVIDIA as far as I am aware. So, although I can probably create a patch, I can only test the paths used when the extension is not found, which will traverse none of the code to fix this issue. Also because I am sick at present it might be some time before I can get to it.

On 2017-08-10 01:50:01 +0000, Sam Lantinga wrote:

Okay, thanks for letting us know.

On 2017-09-01 17:58:34 +0000, Ryan C. Gordon wrote:

There used to be a sacrificial context on X11, too, but previously, the only reason we had this on Windows was because your function pointers from wglGetProcAddress() are context-specific and we needed to look up a context-specific function to then create a modern OpenGL context; it's wild, but that's how it works. glX does not have this requirement and I decided taunting the GL driver gods by creating and immediately destroying the extra context was worth avoiding.

I've added this back, in https://hg.libsdl.org/SDL/rev/1717e5011161, and https://hg.libsdl.org/SDL/rev/37688701d4ce makes this more robust.

I can confirm this definitely creates a context, makes it current, correctly looks up the extensions, and then destroys the temporary context.

--ryan.

On 2017-09-01 18:11:39 +0000, Ryan C. Gordon wrote:

(additional fixes https://hg.libsdl.org/SDL/rev/71be1b793327 and https://hg.libsdl.org/SDL/rev/f9cf3fb0b2f7 because I'm too quick on the trigger when pushing to revision control...)

--ryan.