Fixed changing palettes while rendering is in flight

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);
         }

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,

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 void SW_DestroyPalette(void *unused, const void *key, const void *value)
+{
+    SW_Palette *internal = (SW_Palette *)value;
+    if (internal->palette) {
+        SDL_DestroyPalette(internal->palette);
+    }
+    SDL_free(internal);
+}
+
+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;
+        }
+    }
+
+    // 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;
 
-    if (!surface->palette) {
-        surface->palette = SDL_CreatePalette(1 << SDL_BITSPERPIXEL(surface->format));
-        if (!surface->palette) {
+    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_SetPaletteColors(surface->palette, palette->colors, 0, palette->ncolors);
+    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;

test/testpalette.c

@@ -12,6 +12,7 @@
 /* Simple program:  Move N sprites around on the screen as fast as possible */
 
 #include <SDL3/SDL.h>
+#include <SDL3/SDL_test_memory.h>
 #include <SDL3/SDL_main.h>
 
 #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;
 }