Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SGL_GL_MakeCurrent fails for multiple contexts / multiple threads when using EGL #2495

Closed
SDLBugzilla opened this issue Feb 11, 2021 · 0 comments

Comments

@SDLBugzilla
Copy link
Collaborator

This bug report was migrated from our old Bugzilla tracker.

These attachments are available in the static archive:

Reported in version: HG 2.1
Reported for operating system, platform: Linux, x86_64

Comments on the original bug report:

On 2017-07-06 04:34:57 +0000, Anthony Pesch wrote:

According to the EGL spec, eglMakeCurrent fails with EGL_BAD_ACCESS if:

  • If ctx is current to some other thread, or if either draw or read are bound
    to contexts in another thread, an EGL_BAD_ACCESS error is generated.

Unfortunately, this means SDLs current EGL implementation can't be used with multiple contexts / threads as every context is bound with the window's surface for draw / read. As soon as the second thread binds a context it will receive EGL_BAD_ACCESS as the first thread also bound the window's surface for draw / read.

From digging around the code, I'm not sure I see a great fix. For my use case (using a secondary thread / context for offscreen rendering), binding a NULL surface for the secondary contexts is acceptable. I'm not sure of all the other use cases however.

For my own purposes, I've added in an awful hack to disable binding a surface through SDL_GL_SetAttribute and SDL_EGL_MakeCurrent.

I wonder if a more proper solution would be to add an extra field to the EGLContext struct, say a EGLSurface **egl_surface (right now EGLSurface is a typedef, I'm not sure if it's common that this is instead a wrapper struct with private data). This new field would be assigned &windowdata->egl_surface or NULL at context creation time based on a new flag SDL_GL_CONTEXT_USE_WINDOW_SURFACE. Finally, SDL_EGL_MakeCurrent would use that pointer to figure out the current surface to bind instead of being passed the window's current surface.

If anyone has some input on how to go about fixing this, I'd love to get a patch together that could be upstreamed.

On 2017-08-09 05:25:36 +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 2019-10-25 15:50:49 +0000, Martin Fiedler wrote:

Chiming in here, because we hit this issue as well.

It's indeed a rather complex problem. Some OpenGL window system integration layers allow sharing the same window/drawable/surface/DC from multiple active contexts at the same time, some don't. Same goes for passing "null" drawables:

  • GLX: sharing a drawable is OK, null drawables are forbidden
  • WGL: sharing a DC is OK, null DCs are forbidden
  • EGL: sharing a surface is forbidden, but using EGL_NO_SURFACE is OK with most client APIs (desktop OpenGL >= 3.0; GL_OES_surfaceless_context on GLES)

Any application that uses multithreading with OpenGL (e.g. to load assets in the background on a second thread) and wants to be portable will hit problems with that. Currently, SDL doesn't abstract these platform differences away, but it should. The question is how the API should look like.

Here are some ideas, none of them perfect:

Option 1: SDL_GL_CONTEXT_NO_SURFACE attribute

  • introduce a new SDL_GL attribute: if not set (default), this is a normal rendering context that wants to draw onto the screen; if set, it's a background context that doesn't require a surface
  • the attribute gets stored for every context
  • in SDL_GL_MakeCurrent(), if the context is set to "no surface" and the system's *MakeCurrent() doesn't allow sharing surfaces, ignore the window parameter and use EGL_NO_SURFACE instead
    Caveats:
  • the context is always either no-surface or with-surface; can't use the same context for both

Option 2: allow window=NULL in SDL_GL_MakeCurrent

  • if window=NULL, but the platform requires a target surface, use the surface that was most recently used with this context (defaulting to the one specified in SDL_GL_CreateContext)
    Caveats:
  • application code needs to explicitly use window=NULL when activating a context for background tasks
  • scenarios where the "which surface to select if none was specified" logic fails may exist

Option 3: new flag for SDL_GL_MakeCurrent, or new function

  • when activating a context, the application specifies explicitly whether a target surface is required or not
  • if the platform doesn't allow surface sharing, but the application said it doesn't need a surface anyway, use EGL_NO_SURFACE
    Caveats:
  • application needs to explicitly select what it wants, like with option 2 above
  • adding a parameter breaks all existing application code

On 2020-02-04 17:03:35 +0000, Martin Fiedler wrote:

Created attachment 4189
allow_surfaceless_egl_context

Here's a patch that enables passing NULL windows to SDL_GL_MakeCurrent, if the involved APIs allow it. Currently, this is only the case for EGL, and even then only if some specific extensions are present (which they usually are).

If "surfaceless" contexts are not supported, SDL_GL_MakeCurrent continues to generate an error (albeit with a more specific error message than it used to), so this should not break anything that wasn't broken before.

On 2020-04-13 18:46:40 +0000, Ryan C. Gordon wrote:

Martin's patch is now https://hg.libsdl.org/SDL/rev/636561e494f7, thanks!

--ryan.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant