It might be better to send this as a separate patch series as the "Signed-off-by" is misleading here. You can perhaps change it to "Co-authored-by" and then add your signoff.
> On Nov 4, 2025, at 12:23 AM, startergo <[email protected]> wrote: > > SDL_GL_CONTEXT_PROFILE_MASK needs to be set before creating the window > so that SDL can load the ANGLE library for GL ES support. The shader > source version also needs to match the library used. Avoid falling back > to GL ES on macOS since it will not be available if ANGLE was not the > library loaded initially. > > Enhancements in v3: > - Add better error handling and diagnostics > - Add GL context validation after creation > - Improve shader version detection for better compatibility > - Add conditional precision qualifiers for GLES vs GL Perhaps this can be added as: #ifdef GL_FRAGMENT_PRECISION_HIGH precision highp float; #else precision mediump float; #endif to the fragment shader itself. I’m not entirely sure why it’s needed though. > - Add runtime ANGLE detection > - Improve code comments and documentation > - Fix potential memory issues with shader compilation The code was using g_autofree which handled that. It’s not clear if returning NULL from qemu_gl_init_shader instead of exiting would not cause issues. > - Add trace points for debugging I can’t speak for the maintainers but it might be easier to have a more minimal patch get merged first before further enhancements. > > Signed-off-by: Mohamed Akram <[email protected]> > --- > ui/sdl2-gl.c | 54 ++++++++++++++++++++++----- > ui/sdl2.c | 18 +++++++++ > ui/shader.c | 64 ++++++++++++++++++++++++++++++-- > ui/shader/texture-blit-flip.vert | 2 - > ui/shader/texture-blit.frag | 2 - > ui/shader/texture-blit.vert | 2 - > ui/trace-events | 7 ++++ > 7 files changed, 130 insertions(+), 19 deletions(-) > > diff --git a/ui/sdl2-gl.c b/ui/sdl2-gl.c > index 3be17d1..1db85d2 100644 > --- a/ui/sdl2-gl.c > +++ b/ui/sdl2-gl.c > @@ -30,6 +30,11 @@ > #include "ui/input.h" > #include "ui/sdl2.h" > > +#ifdef CONFIG_DARWIN > +#include <TargetConditionals.h> > +#endif > +#include "trace.h" > + > static void sdl2_set_scanout_mode(struct sdl2_console *scon, bool scanout) > { > if (scon->scanout_mode == scanout) { > @@ -147,27 +152,56 @@ QEMUGLContext sdl2_gl_create_context(DisplayGLCtx *dgc, > SDL_GL_MakeCurrent(scon->real_window, scon->winctx); > > SDL_GL_SetAttribute(SDL_GL_SHARE_WITH_CURRENT_CONTEXT, 1); > - if (scon->opts->gl == DISPLAY_GL_MODE_ON || > - scon->opts->gl == DISPLAY_GL_MODE_CORE) { > - SDL_GL_SetAttribute(SDL_GL_CONTEXT_PROFILE_MASK, > - SDL_GL_CONTEXT_PROFILE_CORE); > - } else if (scon->opts->gl == DISPLAY_GL_MODE_ES) { > - SDL_GL_SetAttribute(SDL_GL_CONTEXT_PROFILE_MASK, > - SDL_GL_CONTEXT_PROFILE_ES); > - } > + > + /* > + * Note: SDL_GL_CONTEXT_PROFILE_MASK should be set before window creation > + * (see sdl2_window_create) so SDL can load the correct GL library (ANGLE > + * for ES support on macOS). Setting it here only affects shared > contexts. > + */ > + > SDL_GL_SetAttribute(SDL_GL_CONTEXT_MAJOR_VERSION, params->major_ver); > SDL_GL_SetAttribute(SDL_GL_CONTEXT_MINOR_VERSION, params->minor_ver); > > ctx = SDL_GL_CreateContext(scon->real_window); > > - /* If SDL fail to create a GL context and we use the "on" flag, > - * then try to fallback to GLES. > + if (ctx) { > + /* Validate the created context */ > + SDL_GL_MakeCurrent(scon->real_window, ctx); > + int profile = 0; > + SDL_GL_GetAttribute(SDL_GL_CONTEXT_PROFILE_MASK, &profile); > + > + trace_sdl2_gl_context_created(profile, params->major_ver, > + params->minor_ver); > + > + /* Check if ANGLE is being used */ > + const char *renderer = (const char *)glGetString(GL_RENDERER); > + if (renderer && strstr(renderer, "ANGLE")) { > + trace_sdl2_gl_using_angle(renderer); > + } > + } else { > + const char *err = SDL_GetError(); > + warn_report("Failed to create GL context (v%d.%d): %s", > + params->major_ver, params->minor_ver, > + err ? err : "unknown error"); > + } > + > + /* > + * If SDL fail to create a GL context and we use the "on" flag, > + * then try to fallback to GLES. On macOS, this will not work because > + * SDL must load ANGLE at window creation time for ES support. If the > + * Core profile was requested initially, ANGLE was not loaded and this > + * fallback will fail. > */ > +#if !TARGET_OS_OSX > if (!ctx && scon->opts->gl == DISPLAY_GL_MODE_ON) { > SDL_GL_SetAttribute(SDL_GL_CONTEXT_PROFILE_MASK, > SDL_GL_CONTEXT_PROFILE_ES); > ctx = SDL_GL_CreateContext(scon->real_window); > + if (ctx) { > + trace_sdl2_gl_fallback_to_es(); > + } > } > +#endif > return (QEMUGLContext)ctx; > } > > diff --git a/ui/sdl2.c b/ui/sdl2.c > index 032dc14..5522bcc 100644 > --- a/ui/sdl2.c > +++ b/ui/sdl2.c > @@ -34,6 +34,7 @@ > #include "system/system.h" > #include "qemu/log.h" > #include "qemu-main.h" > +#include "trace.h" > > static int sdl2_num_outputs; > static struct sdl2_console *sdl2_console; > @@ -96,6 +97,23 @@ void sdl2_window_create(struct sdl2_console *scon) > #ifdef CONFIG_OPENGL > if (scon->opengl) { > flags |= SDL_WINDOW_OPENGL; > + > + /* > + * Set GL context profile before creating the window. > + * This is critical on macOS where SDL needs to know which GL library > + * to load: the native OpenGL framework (for Core profile) or ANGLE > + * (for ES profile). This cannot be changed after window creation. > + */ > + if (scon->opts->gl == DISPLAY_GL_MODE_ON || > + scon->opts->gl == DISPLAY_GL_MODE_CORE) { > + SDL_GL_SetAttribute(SDL_GL_CONTEXT_PROFILE_MASK, > + SDL_GL_CONTEXT_PROFILE_CORE); > + trace_sdl2_gl_request_core_profile(); > + } else if (scon->opts->gl == DISPLAY_GL_MODE_ES) { > + SDL_GL_SetAttribute(SDL_GL_CONTEXT_PROFILE_MASK, > + SDL_GL_CONTEXT_PROFILE_ES); > + trace_sdl2_gl_request_es_profile(); > + } > } > #endif > > diff --git a/ui/shader.c b/ui/shader.c > index ab448c4..9a19ed4 100644 > --- a/ui/shader.c > +++ b/ui/shader.c > @@ -26,6 +26,8 @@ > */ > #include "qemu/osdep.h" > #include "ui/shader.h" > +#include "ui/shader/texture-blit-frag.h" > +#include "qemu/error-report.h" > > #include "ui/shader/texture-blit-vert.h" > #include "ui/shader/texture-blit-flip-vert.h" > @@ -153,12 +155,68 @@ QemuGLShader *qemu_gl_init_shader(void) > { > QemuGLShader *gls = g_new0(QemuGLShader, 1); > > + /* > + * Detect GL version and set appropriate shader version. > + * Desktop GL: Use GLSL 1.40 (OpenGL 3.1) for broad compatibility > + * ES: Use GLSL ES 3.00 (OpenGL ES 3.0) > + */ > + bool is_desktop = epoxy_is_desktop_gl(); > + const char *version = is_desktop ? "140" : "300 es"; > + > + /* > + * Add precision qualifiers for GLES (required), but not for desktop GL > + * where they may cause warnings on some drivers. > + */ > + const char *precision = is_desktop ? "" : "precision mediump float;\n"; > + > + /* Log GL context information for debugging */ > + int gl_version = epoxy_gl_version(); > + const char *vendor = (const char *)glGetString(GL_VENDOR); > + const char *renderer = (const char *)glGetString(GL_RENDERER); > + const char *gl_version_str = (const char *)glGetString(GL_VERSION); > + > + info_report("Initializing shaders: %s GL %d.%d (%s / %s / %s)", > + is_desktop ? "Desktop" : "ES", > + gl_version / 10, gl_version % 10, > + vendor ? vendor : "unknown", > + renderer ? renderer : "unknown", > + gl_version_str ? gl_version_str : "unknown"); > + > + /* Check for required GL features */ > + if (is_desktop && > + !epoxy_has_gl_extension("GL_ARB_vertex_array_object")) { > + warn_report("GL_ARB_vertex_array_object not available, " > + "rendering may fail"); > + } > + > + /* Build shader source with appropriate version and precision */ > + const char *vert_fmt = "#version %s\n%s"; > + const char *frag_fmt = "#version %s\n%s%s"; > + > + char *blit_vert_src = g_strdup_printf( > + vert_fmt, version, texture_blit_vert_src); > + char *blit_flip_vert_src = g_strdup_printf( > + vert_fmt, version, texture_blit_flip_vert_src); > + char *blit_frag_src = g_strdup_printf( > + frag_fmt, version, precision, texture_blit_frag_src); > + > + /* Compile and link shader programs */ > gls->texture_blit_prog = qemu_gl_create_compile_link_program > - (texture_blit_vert_src, texture_blit_frag_src); > + (blit_vert_src, blit_frag_src); > gls->texture_blit_flip_prog = qemu_gl_create_compile_link_program > - (texture_blit_flip_vert_src, texture_blit_frag_src); > + (blit_flip_vert_src, blit_frag_src); > + > + /* Clean up temporary shader source strings */ > + g_free(blit_vert_src); > + g_free(blit_flip_vert_src); > + g_free(blit_frag_src); > + > if (!gls->texture_blit_prog || !gls->texture_blit_flip_prog) { > - exit(1); > + error_report("Failed to compile GL shaders (GL %s %d.%d)", > + is_desktop ? "Desktop" : "ES", > + gl_version / 10, gl_version % 10); > + qemu_gl_fini_shader(gls); > + return NULL; > } > gls->texture_blit_vao = > diff --git a/ui/shader/texture-blit-flip.vert > b/ui/shader/texture-blit-flip.vert > index f7a448d..1e4ac4c 100644 > --- a/ui/shader/texture-blit-flip.vert > +++ b/ui/shader/texture-blit-flip.vert > @@ -1,5 +1,3 @@ > -#version 300 es > - > in vec2 in_position; > out vec2 ex_tex_coord; > > diff --git a/ui/shader/texture-blit.frag b/ui/shader/texture-blit.frag > index 8ed95a4..bd296a2 100644 > --- a/ui/shader/texture-blit.frag > +++ b/ui/shader/texture-blit.frag > @@ -1,5 +1,3 @@ > -#version 300 es > - > uniform sampler2D image; > in mediump vec2 ex_tex_coord; > out mediump vec4 out_frag_color; > diff --git a/ui/shader/texture-blit.vert b/ui/shader/texture-blit.vert > index fb48d70..ae205f6 100644 > --- a/ui/shader/texture-blit.vert > +++ b/ui/shader/texture-blit.vert > @@ -1,5 +1,3 @@ > -#version 300 es > - > in vec2 in_position; > out vec2 ex_tex_coord; > > diff --git a/ui/trace-events b/ui/trace-events > index 3eba9ca..5effcc0 100644 > --- a/ui/trace-events > +++ b/ui/trace-events > @@ -182,5 +182,12 @@ dbus_gl_gfx_switch(void *p) "surf: %p" > dbus_filter(unsigned int serial, unsigned int filter) "serial=%u (<= %u)" > dbus_can_share_map(bool share) "can_share_map: %d" > > +# sdl2-gl.c > +sdl2_gl_context_created(int profile, int major, int minor) "profile=%d > version=%d.%d" > +sdl2_gl_using_angle(const char *renderer) "renderer=%s" > +sdl2_gl_fallback_to_es(void) "Falling back to OpenGL ES" > +sdl2_gl_request_core_profile(void) "Requesting Core profile" > +sdl2_gl_request_es_profile(void) "Requesting ES profile" > + > # egl-helpers.c > egl_init_d3d11_device(void *p) "d3d device: %p" > -- > 2.51.1 > >
