Use a dither palette when converting RGB images to indexed formats

include/SDL3/SDL_surface.h

@@ -742,7 +742,7 @@ extern SDL_DECLSPEC SDL_Surface *SDLCALL SDL_ConvertSurface(SDL_Surface *surface
  * \sa SDL_ConvertSurface
  * \sa SDL_DestroySurface
  */
-extern SDL_DECLSPEC SDL_Surface *SDLCALL SDL_ConvertSurfaceAndColorspace(SDL_Surface *surface, SDL_PixelFormat format, const SDL_Palette *palette, SDL_Colorspace colorspace, SDL_PropertiesID props);
+extern SDL_DECLSPEC SDL_Surface *SDLCALL SDL_ConvertSurfaceAndColorspace(SDL_Surface *surface, SDL_PixelFormat format, SDL_Palette *palette, SDL_Colorspace colorspace, SDL_PropertiesID props);
 
 /**
  * Copy a block of pixels of one format to another format.

src/dynapi/SDL_dynapi_procs.h

@@ -94,7 +94,7 @@ SDL_DYNAPI_PROC(int,SDL_ConvertEventToRenderCoordinates,(SDL_Renderer *a, SDL_Ev
 SDL_DYNAPI_PROC(int,SDL_ConvertPixels,(int a, int b, SDL_PixelFormat c, const void *d, int e, SDL_PixelFormat f, void *g, int h),(a,b,c,d,e,f,g,h),return)
 SDL_DYNAPI_PROC(int,SDL_ConvertPixelsAndColorspace,(int a, int b, SDL_PixelFormat c, SDL_Colorspace d, SDL_PropertiesID e, const void *f, int g, SDL_PixelFormat h, SDL_Colorspace i, SDL_PropertiesID j, void *k, int l),(a,b,c,d,e,f,g,h,i,j,k,l),return)
 SDL_DYNAPI_PROC(SDL_Surface*,SDL_ConvertSurface,(SDL_Surface *a, SDL_PixelFormat b),(a,b),return)
-SDL_DYNAPI_PROC(SDL_Surface*,SDL_ConvertSurfaceAndColorspace,(SDL_Surface *a, SDL_PixelFormat b, const SDL_Palette *c, SDL_Colorspace d, SDL_PropertiesID e),(a,b,c,d,e),return)
+SDL_DYNAPI_PROC(SDL_Surface*,SDL_ConvertSurfaceAndColorspace,(SDL_Surface *a, SDL_PixelFormat b, SDL_Palette *c, SDL_Colorspace d, SDL_PropertiesID e),(a,b,c,d,e),return)
 SDL_DYNAPI_PROC(int,SDL_CopyProperties,(SDL_PropertiesID a, SDL_PropertiesID b),(a,b),return)
 SDL_DYNAPI_PROC(SDL_AudioStream*,SDL_CreateAudioStream,(const SDL_AudioSpec *a, const SDL_AudioSpec *b),(a,b),return)
 SDL_DYNAPI_PROC(SDL_Cursor*,SDL_CreateColorCursor,(SDL_Surface *a, int b, int c),(a,b,c),return)

src/video/SDL_pixels.c

@@ -1077,28 +1077,28 @@ void SDL_DestroyPalette(SDL_Palette *palette)
 /*
  * Calculate an 8-bit (3 red, 3 green, 2 blue) dithered palette of colors
  */
-void SDL_DitherColors(SDL_Color *colors, int bpp)
+void SDL_DitherPalette(SDL_Palette *palette)
 {
     int i;
-    if (bpp != 8) {
+    if (palette->ncolors != 256) {
         return; /* only 8bpp supported right now */
     }
 
-    for (i = 0; i < 256; i++) {
+    for (i = 0; i < palette->ncolors; i++) {
         int r, g, b;
         /* map each bit field to the full [0, 255] interval,
            so 0 is mapped to (0, 0, 0) and 255 to (255, 255, 255) */
         r = i & 0xe0;
         r |= r >> 3 | r >> 6;
-        colors[i].r = (Uint8)r;
+        palette->colors[i].r = (Uint8)r;
         g = (i << 3) & 0xe0;
         g |= g >> 3 | g >> 6;
-        colors[i].g = (Uint8)g;
+        palette->colors[i].g = (Uint8)g;
         b = i & 0x3;
         b |= b << 2;
         b |= b << 4;
-        colors[i].b = (Uint8)b;
-        colors[i].a = SDL_ALPHA_OPAQUE;
+        palette->colors[i].b = (Uint8)b;
+        palette->colors[i].a = SDL_ALPHA_OPAQUE;
     }
 }
 
@@ -1414,9 +1414,9 @@ static Uint8 *MapNto1(const SDL_PixelFormatDetails *src, const SDL_Palette *pal,
         return NULL;
     }
 
-    dithered.ncolors = 256;
-    SDL_DitherColors(colors, 8);
     dithered.colors = colors;
+    dithered.ncolors = SDL_arraysize(colors);
+    SDL_DitherPalette(&dithered);
     return Map1to1(&dithered, pal, identical);
 }
 

src/video/SDL_pixels_c.h

@@ -48,7 +48,7 @@ extern int SDL_MapSurface(SDL_Surface *src, SDL_Surface *dst);
 extern void SDL_InvalidateAllBlitMap(SDL_Surface *surface);
 
 /* Miscellaneous functions */
-extern void SDL_DitherColors(SDL_Color *colors, int bpp);
+extern void SDL_DitherPalette(SDL_Palette *palette);
 extern Uint8 SDL_FindColor(const SDL_Palette *pal, Uint8 r, Uint8 g, Uint8 b, Uint8 a);
 extern void SDL_DetectPalette(const SDL_Palette *pal, SDL_bool *is_opaque, SDL_bool *has_alpha_channel);
 extern SDL_Surface *SDL_DuplicatePixels(int width, int height, SDL_PixelFormat format, SDL_Colorspace colorspace, void *pixels, int pitch);

src/video/SDL_surface.c

@@ -1342,9 +1342,10 @@ int SDL_FlipSurface(SDL_Surface *surface, SDL_FlipMode flip)
     }
 }
 
-SDL_Surface *SDL_ConvertSurfaceAndColorspace(SDL_Surface *surface, SDL_PixelFormat format, const SDL_Palette *palette, SDL_Colorspace colorspace, SDL_PropertiesID props)
+SDL_Surface *SDL_ConvertSurfaceAndColorspace(SDL_Surface *surface, SDL_PixelFormat format, SDL_Palette *palette, SDL_Colorspace colorspace, SDL_PropertiesID props)
 {
-    SDL_Surface *convert;
+    SDL_Palette *temp_palette = NULL;
+    SDL_Surface *convert = NULL;
     SDL_Colorspace src_colorspace;
     SDL_PropertiesID src_properties;
     Uint32 copy_flags;
@@ -1359,12 +1360,12 @@ SDL_Surface *SDL_ConvertSurfaceAndColorspace(SDL_Surface *surface, SDL_PixelForm
 
     if (!SDL_SurfaceValid(surface)) {
         SDL_InvalidParamError("surface");
-        return NULL;
+        goto error;
     }
 
     if (format == SDL_PIXELFORMAT_UNKNOWN) {
         SDL_InvalidParamError("format");
-        return NULL;
+        goto error;
     }
 
     /* Check for empty destination palette! (results in empty image) */
@@ -1377,7 +1378,14 @@ SDL_Surface *SDL_ConvertSurfaceAndColorspace(SDL_Surface *surface, SDL_PixelForm
         }
         if (i == palette->ncolors) {
             SDL_SetError("Empty destination palette");
-            return NULL;
+            goto error;
+        }
+    } else if (SDL_ISPIXELFORMAT_INDEXED(format)) {
+        // Create a dither palette for conversion
+        temp_palette = SDL_CreatePalette(1 << SDL_BITSPERPIXEL(format));
+        if (temp_palette) {
+            SDL_DitherPalette(temp_palette);
+            palette = temp_palette;
         }
     }
 
@@ -1387,7 +1395,10 @@ SDL_Surface *SDL_ConvertSurfaceAndColorspace(SDL_Surface *surface, SDL_PixelForm
     /* Create a new surface with the desired format */
     convert = SDL_CreateSurface(surface->w, surface->h, format);
     if (!convert) {
-        return NULL;
+        goto error;
+    }
+    if (SDL_ISPIXELFORMAT_INDEXED(format)) {
+        SDL_SetSurfacePalette(convert, palette);
     }
 
     if (colorspace == SDL_COLORSPACE_UNKNOWN) {
@@ -1397,8 +1408,7 @@ SDL_Surface *SDL_ConvertSurfaceAndColorspace(SDL_Surface *surface, SDL_PixelForm
 
     if (SDL_ISPIXELFORMAT_FOURCC(format) || SDL_ISPIXELFORMAT_FOURCC(surface->format)) {
         if (SDL_ConvertPixelsAndColorspace(surface->w, surface->h, surface->format, src_colorspace, src_properties, surface->pixels, surface->pitch, convert->format, colorspace, props, convert->pixels, convert->pitch) < 0) {
-            SDL_DestroySurface(convert);
-            return NULL;
+            goto error;
         }
 
         /* Save the original copy flags */
@@ -1407,14 +1417,6 @@ SDL_Surface *SDL_ConvertSurfaceAndColorspace(SDL_Surface *surface, SDL_PixelForm
         goto end;
     }
 
-    /* Copy the palette if any */
-    if (palette && convert->internal->palette) {
-        SDL_memcpy(convert->internal->palette->colors,
-                   palette->colors,
-                   palette->ncolors * sizeof(SDL_Color));
-        convert->internal->palette->ncolors = palette->ncolors;
-    }
-
     /* Save the original copy flags */
     copy_flags = surface->internal->map.info.flags;
     copy_color.r = surface->internal->map.info.r;
@@ -1509,8 +1511,7 @@ SDL_Surface *SDL_ConvertSurfaceAndColorspace(SDL_Surface *surface, SDL_PixelForm
 
     /* SDL_BlitSurfaceUnchecked failed, and so the conversion */
     if (ret < 0) {
-        SDL_DestroySurface(convert);
-        return NULL;
+        goto error;
     }
 
     if (copy_flags & SDL_COPY_COLORKEY) {
@@ -1547,8 +1548,7 @@ SDL_Surface *SDL_ConvertSurfaceAndColorspace(SDL_Surface *surface, SDL_PixelForm
             /* Create a dummy surface to get the colorkey converted */
             tmp = SDL_CreateSurface(1, 1, surface->format);
             if (!tmp) {
-                SDL_DestroySurface(convert);
-                return NULL;
+                goto error;
             }
 
             /* Share the palette, if any */
@@ -1564,8 +1564,7 @@ SDL_Surface *SDL_ConvertSurfaceAndColorspace(SDL_Surface *surface, SDL_PixelForm
             tmp2 = SDL_ConvertSurfaceAndColorspace(tmp, format, palette, colorspace, props);
             if (!tmp2) {
                 SDL_DestroySurface(tmp);
-                SDL_DestroySurface(convert);
-                return NULL;
+                goto error;
             }
 
             /* Get the converted colorkey */
@@ -1585,6 +1584,9 @@ SDL_Surface *SDL_ConvertSurfaceAndColorspace(SDL_Surface *surface, SDL_PixelForm
     }
 
 end:
+    if (temp_palette) {
+        SDL_DestroyPalette(temp_palette);
+    }
 
     SDL_SetSurfaceClipRect(convert, &surface->internal->clip_rect);
 
@@ -1601,6 +1603,15 @@ end:
 
     /* We're ready to go! */
     return convert;
+
+error:
+    if (temp_palette) {
+        SDL_DestroyPalette(temp_palette);
+    }
+    if (convert) {
+        SDL_DestroySurface(convert);
+    }
+    return NULL;
 }
 
 SDL_Surface *SDL_DuplicateSurface(SDL_Surface *surface)