emscripten-ports / SDL2

Other
166 stars 64 forks source link

SDL_GL_CONTEXT_FLAGS, SDL_GL_FRAMEBUFFER_SRGB_CAPABLE issues #86

Open kripken opened 5 years ago

kripken commented 5 years ago

See

https://groups.google.com/d/msg/emscripten-discuss/iZId9geA_9E/wcfQPHQXAwAJ

It sounds like SDL_GL_CONTEXT_FLAGS and SDL_GL_FRAMEBUFFER_SRGB_CAPABLE can make things silently fail?

If there isn't a good fix, perhaps this could be turned into an error at least?

cc @gabrielcuvillier who may have more info

gabrielcuvillier commented 5 years ago

Yes, it looks like setting some SDL_GL* flags (in particular SDL_GL_CONTEXT_FLAGS) after having set SDL_GL_CONTEXT_PROFILE can make things fail.

gabrielcuvillier commented 5 years ago

To me, the issue probably come from here: SDL_egl.c, line 590, function SDL_EGL_CreateContext:

/* Set the context version and other attributes. */
if ((major_version < 3 || (minor_version == 0 && profile_es)) && `         
         _this->gl_config.flags == 0 &&
        (profile_mask == 0 || profile_es)) {
// context creation
}
else {
// fail 
}

The "if" expression is a bit cryptic to understand, but in the end, it look likes that if there is some gl_config.flags, it will just fail...

Daft-Freak commented 5 years ago

That else fail is more like "try to use EGL_KHR_create_context and fail because it isn't supported" (It isn't possible to set context flags without that extension or EGL 1.5). Though, it should already set an error saying that (https://github.com/emscripten-ports/SDL2/blob/master/src/video/SDL_egl.c#L721).

SRGB_CAPABLE fails because we don't support the extension for that either (https://github.com/emscripten-ports/SDL2/blob/master/src/video/SDL_egl.c#L888).

Edit: I think I've hit this on Android as well.

gabrielcuvillier commented 5 years ago

I guess nobody is checking for SDL_Errors ;) Though, in that case, the reason why the error happens is a bit misleading: SDL implementation for GL context creation is delegated to the SDL EGL backend implementation, which in turn rely on Emscripten EGL implementation. As emscripten support for EGL is incomplete (missing extensions or EGL 1.5), this is failing. It is very difficult to figure out things.

Maybe the "flags" should be filtered out before calling the SDL EGL backend ?

Daft-Freak commented 5 years ago

It happens on any platform that uses EGL and only supports ~GLES2.0, not just Emscripten though. SDL_GL_CONTEXT_FLAGS also fails with an error on other platforms without the relevant extensions (WGL/GLX_ARB_create_context).

gabrielcuvillier commented 5 years ago

Okay. I think this is nevertheless and issue, because the documentation of SDL_GL_SetAttribute says these settings are requests (https://wiki.libsdl.org/SDL_GL_SetAttribute#Remarks) and apps must use SDL_GL_GetAttribute after window and context creation to see if those features are available.

But if it is not only Emscripten specific, then this is an issue for upstream SDL.

aardappel commented 4 years ago

Just ran into SDL_GL_FRAMEBUFFER_SRGB_CAPABLE causing SDL_CreateWindow to fail as well. It appears WebGL2 supports SRGB FBO's generally, so would be good to make this work? https://developer.mozilla.org/en-US/docs/Web/API/EXT_sRGB

Daft-Freak commented 4 years ago

There's no way to request an sRGB default framebuffer though, which is what we would need to implement that flag. (Plus a bunch of work in emscripten's EGL implementation).

aardappel commented 4 years ago

@Daft-Freak yes, looks like SRGB conversion would need to be done manually for WebGL2: https://stackoverflow.com/questions/51032480/how-do-you-implement-a-gamma-correct-workflow-in-webgl2

That's a shame, I had though that thanks to ES 3 supporting it, engines could move to a default linear color space model by default, but WebGL is holding that back. Or I guess put up with a pow() at the end of every shader, which would be unfortunate. WebGPU where art thou?