Skip to content

Commit aafbf71

Browse files
sloukenslouken
committed
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 #14369
1 parent 3a59163 commit aafbf71

File tree

3 files changed

+57
-13
lines changed

3 files changed

+57
-13
lines changed

‎src/render/opengl/SDL_render_gl.c‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1671,6 +1671,7 @@ static void GL_DestroyTexture(SDL_Renderer *renderer, SDL_Texture *texture)
16711671

16721672
if (renderdata->drawstate.texture == texture) {
16731673
renderdata->drawstate.texture = NULL;
1674+
renderdata->drawstate.shader_params = NULL;
16741675
}
16751676
if (renderdata->drawstate.target == texture) {
16761677
renderdata->drawstate.target = NULL;

‎src/render/opengl/SDL_shaders_gl.c‎

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ struct GL_ShaderContext
5959
bool GL_ARB_texture_rectangle_supported;
6060

6161
GL_ShaderData shaders[NUM_SHADERS];
62-
const float *shader_params[NUM_SHADERS];
62+
float *shader_params[NUM_SHADERS];
6363
};
6464

6565
/* *INDENT-OFF* */ // clang-format off
@@ -703,7 +703,22 @@ void GL_SelectShader(GL_ShaderContext *ctx, GL_Shader shader, const float *shade
703703

704704
ctx->glUseProgramObjectARB(program);
705705

706-
if (shader_params && shader_params != ctx->shader_params[shader]) {
706+
int shader_params_len = 0;
707+
if (shader == SHADER_PALETTE_LINEAR ||
708+
shader == SHADER_PALETTE_PIXELART ||
709+
shader == SHADER_RGB_PIXELART ||
710+
shader == SHADER_RGBA_PIXELART) {
711+
shader_params_len = 4 * sizeof(float);
712+
#ifdef SDL_HAVE_YUV
713+
} else if (shader >= SHADER_YUV) {
714+
shader_params_len = 16 * sizeof(float);
715+
#endif
716+
}
717+
SDL_assert(!shader_params || shader_params_len > 0);
718+
719+
if (shader_params &&
720+
(!ctx->shader_params[shader] ||
721+
SDL_memcmp(shader_params, ctx->shader_params[shader], shader_params_len) != 0)) {
707722
if (shader == SHADER_PALETTE_LINEAR ||
708723
shader == SHADER_PALETTE_PIXELART ||
709724
shader == SHADER_RGB_PIXELART ||
@@ -736,7 +751,12 @@ void GL_SelectShader(GL_ShaderContext *ctx, GL_Shader shader, const float *shade
736751
}
737752
#endif // SDL_HAVE_YUV
738753

739-
ctx->shader_params[shader] = shader_params;
754+
if (!ctx->shader_params[shader]) {
755+
ctx->shader_params[shader] = (float *)SDL_malloc(shader_params_len);
756+
}
757+
if (ctx->shader_params[shader]) {
758+
SDL_memcpy(ctx->shader_params[shader], shader_params, shader_params_len);
759+
}
740760
}
741761
}
742762

@@ -746,6 +766,7 @@ void GL_DestroyShaderContext(GL_ShaderContext *ctx)
746766

747767
for (i = 0; i < NUM_SHADERS; ++i) {
748768
DestroyShaderProgram(ctx, &ctx->shaders[i]);
769+
SDL_free(ctx->shader_params[i]);
749770
}
750771
SDL_free(ctx);
751772
}

‎src/render/opengles2/SDL_render_gles2.c‎

Lines changed: 32 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ typedef struct GLES2_ProgramCacheEntry
127127
GLuint fragment_shader;
128128
GLuint uniform_locations[NUM_GLES2_UNIFORMS];
129129
GLfloat projection[4][4];
130-
const float *shader_params;
130+
float *shader_params;
131131
struct GLES2_ProgramCacheEntry *prev;
132132
struct GLES2_ProgramCacheEntry *next;
133133
} GLES2_ProgramCacheEntry;
@@ -466,6 +466,7 @@ static GLES2_ProgramCacheEntry *GLES2_CacheProgram(GLES2_RenderData *data, GLuin
466466
data->glGetProgramiv(entry->id, GL_LINK_STATUS, &linkSuccessful);
467467
if (!linkSuccessful) {
468468
data->glDeleteProgram(entry->id);
469+
SDL_free(entry->shader_params);
469470
SDL_free(entry);
470471
SDL_SetError("Failed to link shader program");
471472
return NULL;
@@ -505,12 +506,11 @@ static GLES2_ProgramCacheEntry *GLES2_CacheProgram(GLES2_RenderData *data, GLuin
505506

506507
// Evict the last entry from the cache if we exceed the limit
507508
if (data->program_cache.count > GLES2_MAX_CACHED_PROGRAMS) {
508-
data->glDeleteProgram(data->program_cache.tail->id);
509-
data->program_cache.tail = data->program_cache.tail->prev;
510-
if (data->program_cache.tail) {
511-
SDL_free(data->program_cache.tail->next);
512-
data->program_cache.tail->next = NULL;
513-
}
509+
GLES2_ProgramCacheEntry *oldest = data->program_cache.tail;
510+
data->program_cache.tail = oldest->prev;
511+
data->glDeleteProgram(oldest->id);
512+
SDL_free(oldest->shader_params);
513+
SDL_free(oldest);
514514
--data->program_cache.count;
515515
}
516516
return entry;
@@ -629,6 +629,7 @@ static bool GLES2_SelectProgram(GLES2_RenderData *data, SDL_Texture *texture, GL
629629
GLES2_ProgramCacheEntry *program;
630630
GLES2_TextureData *tdata = texture ? (GLES2_TextureData *)texture->internal : NULL;
631631
const float *shader_params = NULL;
632+
int shader_params_len = 0;
632633

633634
// Select an appropriate shader pair for the specified modes
634635
vtype = GLES2_SHADER_VERTEX_DEFAULT;
@@ -644,10 +645,12 @@ static bool GLES2_SelectProgram(GLES2_RenderData *data, SDL_Texture *texture, GL
644645
case SDL_SCALEMODE_LINEAR:
645646
ftype = GLES2_SHADER_FRAGMENT_TEXTURE_PALETTE_LINEAR;
646647
shader_params = tdata->texel_size;
648+
shader_params_len = 4 * sizeof(float);
647649
break;
648650
case SDL_SCALEMODE_PIXELART:
649651
ftype = GLES2_SHADER_FRAGMENT_TEXTURE_PALETTE_PIXELART;
650652
shader_params = tdata->texel_size;
653+
shader_params_len = 4 * sizeof(float);
651654
break;
652655
default:
653656
SDL_assert(!"Unknown scale mode");
@@ -658,6 +661,7 @@ static bool GLES2_SelectProgram(GLES2_RenderData *data, SDL_Texture *texture, GL
658661
if (scale_mode == SDL_SCALEMODE_PIXELART) {
659662
ftype = GLES2_SHADER_FRAGMENT_TEXTURE_ABGR_PIXELART;
660663
shader_params = tdata->texel_size;
664+
shader_params_len = 4 * sizeof(float);
661665
} else {
662666
ftype = GLES2_SHADER_FRAGMENT_TEXTURE_ABGR;
663667
}
@@ -666,6 +670,7 @@ static bool GLES2_SelectProgram(GLES2_RenderData *data, SDL_Texture *texture, GL
666670
if (scale_mode == SDL_SCALEMODE_PIXELART) {
667671
ftype = GLES2_SHADER_FRAGMENT_TEXTURE_ARGB_PIXELART;
668672
shader_params = tdata->texel_size;
673+
shader_params_len = 4 * sizeof(float);
669674
} else {
670675
ftype = GLES2_SHADER_FRAGMENT_TEXTURE_ARGB;
671676
}
@@ -674,6 +679,7 @@ static bool GLES2_SelectProgram(GLES2_RenderData *data, SDL_Texture *texture, GL
674679
if (scale_mode == SDL_SCALEMODE_PIXELART) {
675680
ftype = GLES2_SHADER_FRAGMENT_TEXTURE_RGB_PIXELART;
676681
shader_params = tdata->texel_size;
682+
shader_params_len = 4 * sizeof(float);
677683
} else {
678684
ftype = GLES2_SHADER_FRAGMENT_TEXTURE_RGB;
679685
}
@@ -682,6 +688,7 @@ static bool GLES2_SelectProgram(GLES2_RenderData *data, SDL_Texture *texture, GL
682688
if (scale_mode == SDL_SCALEMODE_PIXELART) {
683689
ftype = GLES2_SHADER_FRAGMENT_TEXTURE_BGR_PIXELART;
684690
shader_params = tdata->texel_size;
691+
shader_params_len = 4 * sizeof(float);
685692
} else {
686693
ftype = GLES2_SHADER_FRAGMENT_TEXTURE_BGR;
687694
}
@@ -694,6 +701,7 @@ static bool GLES2_SelectProgram(GLES2_RenderData *data, SDL_Texture *texture, GL
694701
SDL_SetError("Unsupported YUV colorspace");
695702
goto fault;
696703
}
704+
shader_params_len = 16 * sizeof(float);
697705
break;
698706
case GLES2_IMAGESOURCE_TEXTURE_NV12:
699707
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
706714
SDL_SetError("Unsupported YUV colorspace");
707715
goto fault;
708716
}
717+
shader_params_len = 16 * sizeof(float);
709718
break;
710719
case GLES2_IMAGESOURCE_TEXTURE_NV21:
711720
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
718727
SDL_SetError("Unsupported YUV colorspace");
719728
goto fault;
720729
}
730+
shader_params_len = 16 * sizeof(float);
721731
break;
722732
#endif // SDL_HAVE_YUV
723733
case GLES2_IMAGESOURCE_TEXTURE_EXTERNAL_OES:
@@ -762,7 +772,11 @@ static bool GLES2_SelectProgram(GLES2_RenderData *data, SDL_Texture *texture, GL
762772
// Select that program in OpenGL
763773
data->glUseProgram(program->id);
764774

765-
if (shader_params && shader_params != program->shader_params) {
775+
SDL_assert(!shader_params || shader_params_len > 0);
776+
777+
if (shader_params &&
778+
(!program->shader_params ||
779+
SDL_memcmp(shader_params, program->shader_params, shader_params_len) != 0)) {
766780
#ifdef SDL_HAVE_YUV
767781
if (ftype >= GLES2_SHADER_FRAGMENT_TEXTURE_YUV) {
768782
// 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
789803
if (shader_params) {
790804
data->glUniform4f(program->uniform_locations[GLES2_UNIFORM_TEXEL_SIZE], shader_params[0], shader_params[1], shader_params[2], shader_params[3]);
791805
}
792-
program->shader_params = shader_params;
806+
807+
if (!program->shader_params) {
808+
program->shader_params = (float *)SDL_malloc(shader_params_len);
809+
}
810+
if (program->shader_params) {
811+
SDL_memcpy(program->shader_params, shader_params, shader_params_len);
812+
}
793813
}
794814

795815
// Set the current program
@@ -1636,8 +1656,9 @@ static void GLES2_DestroyRenderer(SDL_Renderer *renderer)
16361656
GLES2_ProgramCacheEntry *next;
16371657
entry = data->program_cache.head;
16381658
while (entry) {
1639-
data->glDeleteProgram(entry->id);
16401659
next = entry->next;
1660+
data->glDeleteProgram(entry->id);
1661+
SDL_free(entry->shader_params);
16411662
SDL_free(entry);
16421663
entry = next;
16431664
}
@@ -2196,6 +2217,7 @@ static void GLES2_DestroyTexture(SDL_Renderer *renderer, SDL_Texture *texture)
21962217

21972218
if (data->drawstate.texture == texture) {
21982219
data->drawstate.texture = NULL;
2220+
data->drawstate.shader_params = NULL;
21992221
}
22002222
if (data->drawstate.target == texture) {
22012223
data->drawstate.target = NULL;

0 commit comments

Comments
 (0)