From aafbf7183f9fc91d17bc87f14cd6dac2dd6a3b05 Mon Sep 17 00:00:00 2001 From: Sam Lantinga Date: Sat, 1 Nov 2025 10:24:01 -0700 Subject: [PATCH] Copy shader params instead of caching a pointer to them It's possible for a new texture to be allocated with the same address as a previous one, so we can't just cache the pointer value. Fixes https://github.com/libsdl-org/SDL/issues/14369 --- src/render/opengl/SDL_render_gl.c | 1 + src/render/opengl/SDL_shaders_gl.c | 27 ++++++++++++++-- src/render/opengles2/SDL_render_gles2.c | 42 +++++++++++++++++++------ 3 files changed, 57 insertions(+), 13 deletions(-) diff --git a/src/render/opengl/SDL_render_gl.c b/src/render/opengl/SDL_render_gl.c index 1b195d5312..9a868a6a69 100644 --- a/src/render/opengl/SDL_render_gl.c +++ b/src/render/opengl/SDL_render_gl.c @@ -1671,6 +1671,7 @@ static void GL_DestroyTexture(SDL_Renderer *renderer, SDL_Texture *texture) if (renderdata->drawstate.texture == texture) { renderdata->drawstate.texture = NULL; + renderdata->drawstate.shader_params = NULL; } if (renderdata->drawstate.target == texture) { renderdata->drawstate.target = NULL; diff --git a/src/render/opengl/SDL_shaders_gl.c b/src/render/opengl/SDL_shaders_gl.c index 1648751271..d518703e28 100644 --- a/src/render/opengl/SDL_shaders_gl.c +++ b/src/render/opengl/SDL_shaders_gl.c @@ -59,7 +59,7 @@ struct GL_ShaderContext bool GL_ARB_texture_rectangle_supported; GL_ShaderData shaders[NUM_SHADERS]; - const float *shader_params[NUM_SHADERS]; + float *shader_params[NUM_SHADERS]; }; /* *INDENT-OFF* */ // clang-format off @@ -703,7 +703,22 @@ void GL_SelectShader(GL_ShaderContext *ctx, GL_Shader shader, const float *shade ctx->glUseProgramObjectARB(program); - if (shader_params && shader_params != ctx->shader_params[shader]) { + int shader_params_len = 0; + if (shader == SHADER_PALETTE_LINEAR || + shader == SHADER_PALETTE_PIXELART || + shader == SHADER_RGB_PIXELART || + shader == SHADER_RGBA_PIXELART) { + shader_params_len = 4 * sizeof(float); +#ifdef SDL_HAVE_YUV + } else if (shader >= SHADER_YUV) { + shader_params_len = 16 * sizeof(float); +#endif + } + SDL_assert(!shader_params || shader_params_len > 0); + + if (shader_params && + (!ctx->shader_params[shader] || + SDL_memcmp(shader_params, ctx->shader_params[shader], shader_params_len) != 0)) { if (shader == SHADER_PALETTE_LINEAR || shader == SHADER_PALETTE_PIXELART || shader == SHADER_RGB_PIXELART || @@ -736,7 +751,12 @@ void GL_SelectShader(GL_ShaderContext *ctx, GL_Shader shader, const float *shade } #endif // SDL_HAVE_YUV - ctx->shader_params[shader] = shader_params; + if (!ctx->shader_params[shader]) { + ctx->shader_params[shader] = (float *)SDL_malloc(shader_params_len); + } + if (ctx->shader_params[shader]) { + SDL_memcpy(ctx->shader_params[shader], shader_params, shader_params_len); + } } } @@ -746,6 +766,7 @@ void GL_DestroyShaderContext(GL_ShaderContext *ctx) for (i = 0; i < NUM_SHADERS; ++i) { DestroyShaderProgram(ctx, &ctx->shaders[i]); + SDL_free(ctx->shader_params[i]); } SDL_free(ctx); } diff --git a/src/render/opengles2/SDL_render_gles2.c b/src/render/opengles2/SDL_render_gles2.c index 3f7e37f1fb..4ea7151503 100644 --- a/src/render/opengles2/SDL_render_gles2.c +++ b/src/render/opengles2/SDL_render_gles2.c @@ -127,7 +127,7 @@ typedef struct GLES2_ProgramCacheEntry GLuint fragment_shader; GLuint uniform_locations[NUM_GLES2_UNIFORMS]; GLfloat projection[4][4]; - const float *shader_params; + float *shader_params; struct GLES2_ProgramCacheEntry *prev; struct GLES2_ProgramCacheEntry *next; } GLES2_ProgramCacheEntry; @@ -466,6 +466,7 @@ static GLES2_ProgramCacheEntry *GLES2_CacheProgram(GLES2_RenderData *data, GLuin data->glGetProgramiv(entry->id, GL_LINK_STATUS, &linkSuccessful); if (!linkSuccessful) { data->glDeleteProgram(entry->id); + SDL_free(entry->shader_params); SDL_free(entry); SDL_SetError("Failed to link shader program"); return NULL; @@ -505,12 +506,11 @@ static GLES2_ProgramCacheEntry *GLES2_CacheProgram(GLES2_RenderData *data, GLuin // Evict the last entry from the cache if we exceed the limit if (data->program_cache.count > GLES2_MAX_CACHED_PROGRAMS) { - data->glDeleteProgram(data->program_cache.tail->id); - data->program_cache.tail = data->program_cache.tail->prev; - if (data->program_cache.tail) { - SDL_free(data->program_cache.tail->next); - data->program_cache.tail->next = NULL; - } + GLES2_ProgramCacheEntry *oldest = data->program_cache.tail; + data->program_cache.tail = oldest->prev; + data->glDeleteProgram(oldest->id); + SDL_free(oldest->shader_params); + SDL_free(oldest); --data->program_cache.count; } return entry; @@ -629,6 +629,7 @@ static bool GLES2_SelectProgram(GLES2_RenderData *data, SDL_Texture *texture, GL GLES2_ProgramCacheEntry *program; GLES2_TextureData *tdata = texture ? (GLES2_TextureData *)texture->internal : NULL; const float *shader_params = NULL; + int shader_params_len = 0; // Select an appropriate shader pair for the specified modes vtype = GLES2_SHADER_VERTEX_DEFAULT; @@ -644,10 +645,12 @@ static bool GLES2_SelectProgram(GLES2_RenderData *data, SDL_Texture *texture, GL case SDL_SCALEMODE_LINEAR: ftype = GLES2_SHADER_FRAGMENT_TEXTURE_PALETTE_LINEAR; shader_params = tdata->texel_size; + shader_params_len = 4 * sizeof(float); break; case SDL_SCALEMODE_PIXELART: ftype = GLES2_SHADER_FRAGMENT_TEXTURE_PALETTE_PIXELART; shader_params = tdata->texel_size; + shader_params_len = 4 * sizeof(float); break; default: SDL_assert(!"Unknown scale mode"); @@ -658,6 +661,7 @@ static bool GLES2_SelectProgram(GLES2_RenderData *data, SDL_Texture *texture, GL if (scale_mode == SDL_SCALEMODE_PIXELART) { ftype = GLES2_SHADER_FRAGMENT_TEXTURE_ABGR_PIXELART; shader_params = tdata->texel_size; + shader_params_len = 4 * sizeof(float); } else { ftype = GLES2_SHADER_FRAGMENT_TEXTURE_ABGR; } @@ -666,6 +670,7 @@ static bool GLES2_SelectProgram(GLES2_RenderData *data, SDL_Texture *texture, GL if (scale_mode == SDL_SCALEMODE_PIXELART) { ftype = GLES2_SHADER_FRAGMENT_TEXTURE_ARGB_PIXELART; shader_params = tdata->texel_size; + shader_params_len = 4 * sizeof(float); } else { ftype = GLES2_SHADER_FRAGMENT_TEXTURE_ARGB; } @@ -674,6 +679,7 @@ static bool GLES2_SelectProgram(GLES2_RenderData *data, SDL_Texture *texture, GL if (scale_mode == SDL_SCALEMODE_PIXELART) { ftype = GLES2_SHADER_FRAGMENT_TEXTURE_RGB_PIXELART; shader_params = tdata->texel_size; + shader_params_len = 4 * sizeof(float); } else { ftype = GLES2_SHADER_FRAGMENT_TEXTURE_RGB; } @@ -682,6 +688,7 @@ static bool GLES2_SelectProgram(GLES2_RenderData *data, SDL_Texture *texture, GL if (scale_mode == SDL_SCALEMODE_PIXELART) { ftype = GLES2_SHADER_FRAGMENT_TEXTURE_BGR_PIXELART; shader_params = tdata->texel_size; + shader_params_len = 4 * sizeof(float); } else { ftype = GLES2_SHADER_FRAGMENT_TEXTURE_BGR; } @@ -694,6 +701,7 @@ static bool GLES2_SelectProgram(GLES2_RenderData *data, SDL_Texture *texture, GL SDL_SetError("Unsupported YUV colorspace"); goto fault; } + shader_params_len = 16 * sizeof(float); break; case GLES2_IMAGESOURCE_TEXTURE_NV12: if (SDL_GetHintBoolean("SDL_RENDER_OPENGL_NV12_RG_SHADER", false)) { @@ -706,6 +714,7 @@ static bool GLES2_SelectProgram(GLES2_RenderData *data, SDL_Texture *texture, GL SDL_SetError("Unsupported YUV colorspace"); goto fault; } + shader_params_len = 16 * sizeof(float); break; case GLES2_IMAGESOURCE_TEXTURE_NV21: if (SDL_GetHintBoolean("SDL_RENDER_OPENGL_NV12_RG_SHADER", false)) { @@ -718,6 +727,7 @@ static bool GLES2_SelectProgram(GLES2_RenderData *data, SDL_Texture *texture, GL SDL_SetError("Unsupported YUV colorspace"); goto fault; } + shader_params_len = 16 * sizeof(float); break; #endif // SDL_HAVE_YUV case GLES2_IMAGESOURCE_TEXTURE_EXTERNAL_OES: @@ -762,7 +772,11 @@ static bool GLES2_SelectProgram(GLES2_RenderData *data, SDL_Texture *texture, GL // Select that program in OpenGL data->glUseProgram(program->id); - if (shader_params && shader_params != program->shader_params) { + SDL_assert(!shader_params || shader_params_len > 0); + + if (shader_params && + (!program->shader_params || + SDL_memcmp(shader_params, program->shader_params, shader_params_len) != 0)) { #ifdef SDL_HAVE_YUV if (ftype >= GLES2_SHADER_FRAGMENT_TEXTURE_YUV) { // YUV shader params are Yoffset, 0, Rcoeff, 0, Gcoeff, 0, Bcoeff, 0 @@ -789,7 +803,13 @@ static bool GLES2_SelectProgram(GLES2_RenderData *data, SDL_Texture *texture, GL if (shader_params) { data->glUniform4f(program->uniform_locations[GLES2_UNIFORM_TEXEL_SIZE], shader_params[0], shader_params[1], shader_params[2], shader_params[3]); } - program->shader_params = shader_params; + + if (!program->shader_params) { + program->shader_params = (float *)SDL_malloc(shader_params_len); + } + if (program->shader_params) { + SDL_memcpy(program->shader_params, shader_params, shader_params_len); + } } // Set the current program @@ -1636,8 +1656,9 @@ static void GLES2_DestroyRenderer(SDL_Renderer *renderer) GLES2_ProgramCacheEntry *next; entry = data->program_cache.head; while (entry) { - data->glDeleteProgram(entry->id); next = entry->next; + data->glDeleteProgram(entry->id); + SDL_free(entry->shader_params); SDL_free(entry); entry = next; } @@ -2196,6 +2217,7 @@ static void GLES2_DestroyTexture(SDL_Renderer *renderer, SDL_Texture *texture) if (data->drawstate.texture == texture) { data->drawstate.texture = NULL; + data->drawstate.shader_params = NULL; } if (data->drawstate.target == texture) { data->drawstate.target = NULL;