We are currently migrating Bugzilla to GitHub issues.
Any changes made to the bug tracker now will be lost, so please do not post new bugs or make changes to them.
When we're done, all bug URLs will redirect to their equivalent location on the new bug tracker.

Bug 3145 - SDL creates OpenGL context for OpenGL ES 3.x without checking if ES3 profile is supported
Summary: SDL creates OpenGL context for OpenGL ES 3.x without checking if ES3 profile ...
Status: RESOLVED FIXED
Alias: None
Product: SDL
Classification: Unclassified
Component: video (show other bugs)
Version: HG 2.0
Hardware: x86_64 Other
: P2 normal
Assignee: Ryan C. Gordon
QA Contact: Sam Lantinga
URL:
Keywords: target-2.0.6
Depends on:
Blocks: 2570 3725
  Show dependency treegraph
 
Reported: 2015-10-09 23:26 UTC by Mark Callow
Modified: 2017-09-01 18:28 UTC (History)
2 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Callow 2015-10-09 23:26:09 UTC
The opening code in WIN_GL_CreateContext() in SDL_windowsopengl.c does

    if (_this->gl_config.profile_mask == SDL_GL_CONTEXT_PROFILE_ES &&
        !_this->gl_data->HAS_WGL_EXT_create_context_es2_profile) {
#if SDL_VIDEO_OPENGL_EGL        
        /* Switch to EGL based functions */
        ...
#endif
    /* Use OpenGL context */


SDL_GL_CONTEXT_PROFILE_ES is set iff the extension WGL_EXT_create_context_es2_profile is available. This means that when an ES3 context is requested, SDL will use an OpenGL context even though the context might well not support WGL_EXT_create_context_es3_profile, in which case the application will almost certainly fail.

This needs to be fixed together with bug #2570.
Comment 1 Mark Callow 2015-10-10 00:24:27 UTC
Clarification:

Presence of WGL_EXT_create_context_es{,2}_profile could mean that an ES 3.x profile is supported. The problem is that if it is not, the OpenGL context creation fails and there is no attempt to load a libegl.dll or libGLESv2.dll.
Comment 2 Mark Callow 2015-10-28 09:59:56 UTC
I am working on fixing this issue and have questions about the expected usage of SDL by apps. I need to understand this in order to create the best solution.

In most of the *CreateWindow, *_GL{,ES}_CreateContext and *_GL{,ES}_LoadLibrary functions, for platforms that support both OpenGL and OpenGL ES, there is code that checks the profile mask, unloading the current library and loading the other, if it doesn't match.

For example in SDL_x11opengl.c X11_GL_LoadLibrary

    if (_this->gl_config.profile_mask == SDL_GL_CONTEXT_PROFILE_ES && 
        ! _this->gl_data->HAS_GLX_EXT_create_context_es2_profile ) {
#if SDL_VIDEO_OPENGL_EGL
        X11_GL_UnloadLibrary(_this);
        ...
#endif
    }

and in SDL_x11opengles.c in X11_GLES_LoadLibrary

    if (_this->gl_config.profile_mask != SDL_GL_CONTEXT_PROFILE_ES) {
        #if SDL_VIDEO_OPENGL_GLX
        X11_GLES_UnloadLibrary(_this);
        ...
        #endif
    }

For the case of the *LoadLibrary functions, I presume this is because the application could call SDL_GL_LoadLibrary at anytime and you are trying to protect applications from themselves. Is this true? Is there another reason?

For the case of the *_CreateWindow I suppose this is because you do not know which library might be loaded at the time SDL_CreateWindow is called.

For the *{GL,ES}_CreateContext functions the reason is not clear at all. The documentation says that the PROFILE_MASK, MAJOR_VERSION and MINOR_VERSION attributes must be set *before* the window is created and the window must be created before SDL_GL_CreateContext is called. So why are the checks necessary?

An application that changed attributes between SDL_CreateWindow and SDL_GL_CreateContext is not following the rules.

If the application was to create a second window with different attributes, these checks would result in a different library being loaded, breaking rendering in the first window.
Comment 3 Mark Callow 2015-10-29 09:24:40 UTC
I have just spotted an inconsistency between X11_GL_LoadLibrary and WIN_GL_LoadLibrary. The former, after calling GL_InitExtensions() checks for PROFILE_ES and if so is the necessary extension supported. If not, it unloads the GL library and loads the GLES library. The latter simply loads the GL library.

Because WIN_GL_CreateContext does the test and unload/load the end result for the application is much the same. But it is confusing.

It seems simpler to just test this in LoadLibrary and ensure the correct library is loaded and remove the tests elsewhere. Hopefully I will get answers to my questions in comment 2 that can guide me.
Comment 4 Mark Callow 2015-12-11 11:18:54 UTC
I have just submitted a patch in bug #2570 that fixes this. Please incorporate it into SDL.

While working on the fix I found some additional problems which I also fixed:

- The correct extension name to check is EXT_create_context_es_profile
  EXT_create_context_es2_profile only indicates support for ES2 profiles.
- EXT_create_context_es_profile does not support OpenGL ES 1 profiles.
  The patch makes SDL load libGLES_CM and libEGL if ES 1 is specified.


I also spotted several differences between the WIN and X11 implementations that cause application portability problems for several corner cases. That is an app may work on WIN and fail or X11 or vice-versa. I may file a new bug for those.
Comment 5 Sam Lantinga 2017-01-10 17:00:54 UTC
Your patch is in, please see my comments on bug 2570
https://hg.libsdl.org/SDL/rev/36f40b8cc979

Please do file bugs for the other portability problems you noticed.

I'm going to ping Ryan to chime in on comment 2, as I'm not sure of the intricacies of that.

Thanks!
Comment 6 Gabriel Jacobo 2017-01-10 20:34:57 UTC
I think the loading/unloading of libraries enables the chance to open a window in OpenGL, if that fails (no OpenGL of x version available for example), you can try with OpenGL ES, or the other way around, or have a test app that deals with several types of OpenGL contexts in sequence (even though as you mentioned having them loaded concurrently will result in a problem).
Comment 7 Mark Callow 2017-01-11 07:28:22 UTC
(In reply to Sam Lantinga from comment #5)
> Please do file bugs for the other portability problems you noticed.
> 

It was a long time ago. I will look for the notes I made at the time. If I find them, I'll file the bugs.
Comment 8 Ryan C. Gordon 2017-01-12 05:25:05 UTC
(In reply to Sam Lantinga from comment #5)
> I'm going to ping Ryan to chime in on comment 2, as I'm not sure of the
> intricacies of that.

I think Mark's reading of this is correct and we've probably never had an app that needed to switch libraries at all, let alone after creating a window, so it's probably just a well-intentioned bug.

--ryan.
Comment 9 Sam Lantinga 2017-08-04 20:09:37 UTC
The fix to this bug introduced bug 3725
Comment 10 Ryan C. Gordon 2017-09-01 18:28:31 UTC
(In reply to Sam Lantinga from comment #9)
> The fix to this bug introduced bug 3725

Bug #3725 is resolved now, so I'm resolving this one too (or should we leave this open?).


--ryan.