From 264b436dbaaa8f3f031837dfe947cbede42c21b5 Mon Sep 17 00:00:00 2001 From: Sam Lantinga Date: Sat, 27 Sep 2025 09:45:03 -0700 Subject: [PATCH] Fixed changing palettes while rendering is in flight --- src/render/SDL_render.c | 11 ++++ src/render/SDL_sysrender.h | 1 + src/render/software/SDL_render_sw.c | 96 ++++++++++++++++++++++++++--- test/testpalette.c | 70 +++++++++++++++------ 4 files changed, 152 insertions(+), 26 deletions(-) diff --git a/src/render/SDL_render.c b/src/render/SDL_render.c index d2d47f3e44..fe1d0e8a7d 100644 --- a/src/render/SDL_render.c +++ b/src/render/SDL_render.c @@ -1896,6 +1896,17 @@ bool SDL_SetTexturePalette(SDL_Texture *texture, SDL_Palette *palette) } if (palette != texture->palette) { + if (!FlushRenderCommandsIfTextureNeeded(texture)) { + return false; + } + + if (!texture->native) { + SDL_Renderer *renderer = texture->renderer; + if (!renderer->ChangeTexturePalette(renderer, texture, palette)) { + return false; + } + } + if (texture->palette) { SDL_DestroyPalette(texture->palette); } diff --git a/src/render/SDL_sysrender.h b/src/render/SDL_sysrender.h index 660d022d53..ea23c5abf4 100644 --- a/src/render/SDL_sysrender.h +++ b/src/render/SDL_sysrender.h @@ -235,6 +235,7 @@ struct SDL_Renderer void (*InvalidateCachedState)(SDL_Renderer *renderer); bool (*RunCommandQueue)(SDL_Renderer *renderer, SDL_RenderCommand *cmd, void *vertices, size_t vertsize); + bool (*ChangeTexturePalette)(SDL_Renderer *renderer, SDL_Texture *texture, SDL_Palette *palette); bool (*UpdateTexturePalette)(SDL_Renderer *renderer, SDL_Texture *texture); bool (*UpdateTexture)(SDL_Renderer *renderer, SDL_Texture *texture, const SDL_Rect *rect, const void *pixels, diff --git a/src/render/software/SDL_render_sw.c b/src/render/software/SDL_render_sw.c index 12a5f0e5c4..1da66115c2 100644 --- a/src/render/software/SDL_render_sw.c +++ b/src/render/software/SDL_render_sw.c @@ -37,6 +37,13 @@ // SDL surface based renderer implementation +typedef struct +{ + SDL_Palette *palette; + Uint32 version; + int refcount; +} SW_Palette; + typedef struct { const SDL_Rect *viewport; @@ -49,6 +56,7 @@ typedef struct { SDL_Surface *surface; SDL_Surface *window; + SDL_HashTable *palettes; } SW_RenderData; static SDL_Surface *SW_ActivateRenderer(SDL_Renderer *renderer) @@ -126,18 +134,87 @@ static bool SW_CreateTexture(SDL_Renderer *renderer, SDL_Texture *texture, SDL_P return true; } -static bool SW_UpdateTexturePalette(SDL_Renderer *renderer, SDL_Texture *texture) +static void SW_DestroyPalette(void *unused, const void *key, const void *value) { - SDL_Surface *surface = (SDL_Surface *)texture->internal; - SDL_Palette *palette = texture->palette; + SW_Palette *internal = (SW_Palette *)value; + if (internal->palette) { + SDL_DestroyPalette(internal->palette); + } + SDL_free(internal); +} - if (!surface->palette) { - surface->palette = SDL_CreatePalette(1 << SDL_BITSPERPIXEL(surface->format)); - if (!surface->palette) { +static bool SW_ChangeTexturePalette(SDL_Renderer *renderer, SDL_Texture *texture, SDL_Palette *palette) +{ + SW_RenderData *data = (SW_RenderData *)renderer->internal; + SDL_Surface *surface = (SDL_Surface *)texture->internal; + + // We can't use the palette directly since it may change while drawing is in flight, + // so we'll keep a shadow of the palette that our texture surfaces will use instead. + if (!data->palettes) { + data->palettes = SDL_CreateHashTable(0, false, SDL_HashPointer, SDL_KeyMatchPointer, SW_DestroyPalette, NULL); + if (!data->palettes) { return false; } } - return SDL_SetPaletteColors(surface->palette, palette->colors, 0, palette->ncolors); + + // Unreference our internal palette + SW_Palette *internal = NULL; + if (texture->palette) { + if (SDL_FindInHashTable(data->palettes, texture->palette, (const void **)&internal)) { + --internal->refcount; + if (internal->refcount == 0) { + SDL_RemoveFromHashTable(data->palettes, texture->palette); + } + } + } + + if (palette) { + if (SDL_FindInHashTable(data->palettes, palette, (const void **)&internal)) { + ++internal->refcount; + } else { + internal = (SW_Palette *)SDL_calloc(1, sizeof(*internal)); + if (!internal) { + return false; + } + internal->refcount = 1; + + if (!SDL_InsertIntoHashTable(data->palettes, palette, internal, false)) { + SW_DestroyPalette(NULL, palette, internal); + return false; + } + } + return SDL_SetSurfacePalette(surface, internal->palette); + } else { + return SDL_SetSurfacePalette(surface, NULL); + } +} + +static bool SW_UpdateTexturePalette(SDL_Renderer *renderer, SDL_Texture *texture) +{ + SW_RenderData *data = (SW_RenderData *)renderer->internal; + SDL_Surface *surface = (SDL_Surface *)texture->internal; + SDL_Palette *palette = texture->palette; + Uint32 version = palette->version; + + SW_Palette *internal = NULL; + if (!SDL_FindInHashTable(data->palettes, palette, (const void **)&internal)) { + return SDL_SetError("Couldn't find internal palette"); + } + if (internal->version != version) { + // Cycle the palette as some drawing might be in flight using the old colors + if (internal->palette) { + SDL_DestroyPalette(internal->palette); + } + internal->palette = SDL_CreatePalette(palette->ncolors); + if (!internal->palette) { + return false; + } + if (!SDL_SetPaletteColors(internal->palette, palette->colors, 0, palette->ncolors)) { + return false; + } + internal->version = version; + } + return SDL_SetSurfacePalette(surface, internal->palette); } static bool SW_UpdateTexture(SDL_Renderer *renderer, SDL_Texture *texture, @@ -1030,6 +1107,10 @@ static void SW_DestroyRenderer(SDL_Renderer *renderer) SDL_Window *window = renderer->window; SW_RenderData *data = (SW_RenderData *)renderer->internal; + if (data->palettes) { + SDL_assert(SDL_HashTableEmpty(data->palettes)); + SDL_DestroyHashTable(data->palettes); + } if (window) { SDL_DestroyWindowSurface(window); } @@ -1159,6 +1240,7 @@ bool SW_CreateRendererForSurface(SDL_Renderer *renderer, SDL_Surface *surface, S renderer->WindowEvent = SW_WindowEvent; renderer->GetOutputSize = SW_GetOutputSize; renderer->CreateTexture = SW_CreateTexture; + renderer->ChangeTexturePalette = SW_ChangeTexturePalette; renderer->UpdateTexturePalette = SW_UpdateTexturePalette; renderer->UpdateTexture = SW_UpdateTexture; renderer->LockTexture = SW_LockTexture; diff --git a/test/testpalette.c b/test/testpalette.c index f1efd96e9d..5ec61f6301 100644 --- a/test/testpalette.c +++ b/test/testpalette.c @@ -12,6 +12,7 @@ /* Simple program: Move N sprites around on the screen as fast as possible */ #include +#include #include #ifdef SDL_PLATFORM_EMSCRIPTEN @@ -284,12 +285,26 @@ static const SDL_Color Palette[256] = { static SDL_Renderer *renderer; static SDL_Palette *palette; static SDL_Texture *texture; -static SDL_Texture *black_texture; -static SDL_Texture *white_texture; +static SDL_Texture *black_texture1; +static SDL_Texture *black_texture2; +static SDL_Texture *white_texture1; +static SDL_Texture *white_texture2; static int palettePos = 0; static int paletteDir = -1; static bool done; +static SDL_Texture *CreateTexture(const void *pixels, int pitch) +{ + SDL_Texture *tex = SDL_CreateTexture(renderer, SDL_PIXELFORMAT_INDEX8, SDL_TEXTUREACCESS_STATIC, 256, 1); + if (!tex) { + return NULL; + } + SDL_SetTextureBlendMode(tex, SDL_BLENDMODE_NONE); + SDL_UpdateTexture(tex, NULL, pixels, pitch); + SDL_SetTexturePalette(tex, palette); + return tex; +} + static bool CreateTextures() { Uint8 data[256]; @@ -304,26 +319,30 @@ static bool CreateTextures() data[i] = i; } - texture = SDL_CreateTexture(renderer, SDL_PIXELFORMAT_INDEX8, SDL_TEXTUREACCESS_STATIC, 256, 1); + texture = CreateTexture(data, SDL_arraysize(data)); if (!texture) { return false; } - SDL_UpdateTexture(texture, NULL, data, SDL_arraysize(data)); - SDL_SetTexturePalette(texture, palette); - black_texture = SDL_CreateTexture(renderer, SDL_PIXELFORMAT_INDEX8, SDL_TEXTUREACCESS_STATIC, 1, 1); - if (!black_texture) { + black_texture1 = CreateTexture(data, SDL_arraysize(data)); + if (!black_texture1) { return false; } - SDL_UpdateTexture(black_texture, NULL, data, SDL_arraysize(data)); - SDL_SetTexturePalette(black_texture, palette); - white_texture = SDL_CreateTexture(renderer, SDL_PIXELFORMAT_INDEX8, SDL_TEXTUREACCESS_STATIC, 1, 1); - if (!white_texture) { + black_texture2 = CreateTexture(data, SDL_arraysize(data)); + if (!black_texture2) { + return false; + } + + white_texture1 = CreateTexture(data, SDL_arraysize(data)); + if (!white_texture1) { + return false; + } + + white_texture2 = CreateTexture(data, SDL_arraysize(data)); + if (!white_texture2) { return false; } - SDL_UpdateTexture(white_texture, NULL, data, SDL_arraysize(data)); - SDL_SetTexturePalette(white_texture, palette); return true; } @@ -343,9 +362,11 @@ static void UpdatePalette(int pos) static void loop(void) { SDL_Event event; - SDL_FRect src = { 0.0f, 0.0f, 1.0f, 1.0f }; + SDL_FRect src = { 1.0f, 0.0f, 1.0f, 1.0f }; SDL_FRect dst1 = { 0.0f, 0.0f, 32.0f, 32.0f }; - SDL_FRect dst2 = { WINDOW_WIDTH - 32.0f, 0.0f, 32.0f, 32.0f }; + SDL_FRect dst2 = { 0.0f, WINDOW_HEIGHT - 32.0f, 32.0f, 32.0f }; + SDL_FRect dst3 = { WINDOW_WIDTH - 32.0f, 0.0f, 32.0f, 32.0f }; + SDL_FRect dst4 = { WINDOW_WIDTH - 32.0f, WINDOW_HEIGHT - 32.0f, 32.0f, 32.0f }; const SDL_Color black = { 0, 0, 0, SDL_ALPHA_OPAQUE }; const SDL_Color white = { 255, 255, 255, SDL_ALPHA_OPAQUE }; @@ -390,10 +411,18 @@ static void loop(void) /* Draw one square with black, and one square with white * This tests changing palette colors within a single frame */ - SDL_SetPaletteColors(palette, &black, 0, 1); - SDL_RenderTexture(renderer, black_texture, &src, &dst1); - SDL_SetPaletteColors(palette, &white, 0, 1); - SDL_RenderTexture(renderer, white_texture, &src, &dst2); + SDL_SetPaletteColors(palette, &black, 1, 1); + SDL_SetRenderDrawColor(renderer, 0, 0, 0, SDL_ALPHA_OPAQUE); + SDL_RenderDebugText(renderer, dst1.x + 32.0f + 2.0f, dst1.y + 12, "Black"); + SDL_RenderTexture(renderer, black_texture1, &src, &dst1); + SDL_RenderDebugText(renderer, dst2.x + 32.0f + 2.0f, dst2.y + 12, "Black"); + SDL_RenderTexture(renderer, black_texture2, &src, &dst2); + SDL_SetPaletteColors(palette, &white, 1, 1); + SDL_SetRenderDrawColor(renderer, 255, 255, 255, SDL_ALPHA_OPAQUE); + SDL_RenderDebugText(renderer, dst3.x - 40.0f - 2.0f, dst3.y + 12, "White"); + SDL_RenderTexture(renderer, white_texture1, &src, &dst3); + SDL_RenderDebugText(renderer, dst4.x - 40.0f - 2.0f, dst4.y + 12, "White"); + SDL_RenderTexture(renderer, white_texture2, &src, &dst4); SDL_RenderPresent(renderer); SDL_Delay(10); @@ -410,6 +439,8 @@ int main(int argc, char *argv[]) SDL_Window *window = NULL; int return_code = -1; + SDLTest_TrackAllocations(); + if (argc > 1) { SDL_LogError(SDL_LOG_CATEGORY_APPLICATION, "USAGE: %s", argv[0]); return_code = 1; @@ -444,5 +475,6 @@ quit: SDL_DestroyRenderer(renderer); SDL_DestroyWindow(window); SDL_Quit(); + SDLTest_LogAllocations(); return return_code; }