Support indexed surfaces without palettes (thanks @sulix!)

Currently, all SDL_Surfaces with an indexed pixel format have an
associated SDL_Palette. This palette either consists of entirely the
colour black, or -- in the special case of 1-bit surfaces, black and
white.

When an indexed surface is blitted to another indexed surface, a 'map'
is generated from the source surface's palette to the destination
surfaces palette, in order to preserve the look of the image if the
palettes differ.

However, in most cases, applications will want to blit the raw index
values, rather than translate to make the colours as similar as
possible. For instance, the destination surface's palette may have been
modified to fade the screen out.

This change allows an indexed surface to have no associated palette. If
either the source or destination surface of a blit do not have a
palette, then the raw indices are copied (assuming both have an indexed
format).

This mimics better what happens with most other APIs (such as
DirectDraw), where most users do not set a palette on any surface but
the screen, whose palette is implicitly used for the whole application.

docs/README-migration.md

@@ -1632,6 +1632,8 @@ The `format` member of SDL_Surface is now an enumerated pixel format value. You
 
 The userdata member of SDL_Surface has been replaced with a more general properties interface, which can be queried with SDL_GetSurfaceProperties()
 
+Indexed format surfaces no longer have a palette by default. Surfaces without a palette will copy the pixels untranslated between surfaces. You should use SDL_CreatePalette() to create a palette and call SDL_SetSurfacePalette() to associate it with the final indexed surface before copying it to color pixels.
+
 Removed the unused 'flags' parameter from SDL_ConvertSurface.
 
 SDL_CreateRGBSurface() and SDL_CreateRGBSurfaceWithFormat() have been combined into a new function SDL_CreateSurface().

src/video/SDL_bmp.c

@@ -203,7 +203,6 @@ SDL_Surface *SDL_LoadBMP_IO(SDL_IOStream *src, SDL_bool closeio)
     Uint32 Gmask = 0;
     Uint32 Bmask = 0;
     Uint32 Amask = 0;
-    SDL_Palette *palette;
     Uint8 *bits;
     Uint8 *top, *end;
     SDL_bool topDown;
@@ -439,8 +438,10 @@ SDL_Surface *SDL_LoadBMP_IO(SDL_IOStream *src, SDL_bool closeio)
     }
 
     /* Load the palette, if any */
-    palette = SDL_GetSurfacePalette(surface);
-    if (palette) {
+    if (SDL_ISPIXELFORMAT_INDEXED(surface->format)) {
+        int max_colors = (1 << SDL_BITSPERPIXEL(surface->format));
+        SDL_Palette *palette;
+
         if (SDL_SeekIO(src, fp_offset + 14 + biSize, SDL_IO_SEEK_SET) < 0) {
             SDL_SetError("Error seeking in datastream");
             goto done;
@@ -455,14 +456,19 @@ SDL_Surface *SDL_LoadBMP_IO(SDL_IOStream *src, SDL_bool closeio)
             biClrUsed = 1 << biBitCount;
         }
 
-        if (biClrUsed > (Uint32)palette->ncolors) {
+        if (biClrUsed > (Uint32)max_colors) {
             biClrUsed = 1 << biBitCount; /* try forcing it? */
-            if (biClrUsed > (Uint32)palette->ncolors) {
+            if (biClrUsed > (Uint32)max_colors) {
                 SDL_SetError("Unsupported or incorrect biClrUsed field");
                 goto done;
             }
         }
 
+        palette = SDL_CreatePalette(biClrUsed);
+        if (!palette) {
+            goto done;
+        }
+
         if (biSize == 12) {
             for (i = 0; i < (int)biClrUsed; ++i) {
                 if (!SDL_ReadU8(src, &palette->colors[i].b) ||
@@ -488,7 +494,9 @@ SDL_Surface *SDL_LoadBMP_IO(SDL_IOStream *src, SDL_bool closeio)
                 palette->colors[i].a = SDL_ALPHA_OPAQUE;
             }
         }
-        palette->ncolors = biClrUsed;
+
+        SDL_SetSurfacePalette(surface, palette);
+        SDL_DestroyPalette(palette);
     }
 
     /* Read the surface pixels.  Note that the bmp image is upside down */
@@ -515,7 +523,7 @@ SDL_Surface *SDL_LoadBMP_IO(SDL_IOStream *src, SDL_bool closeio)
         if (SDL_ReadIO(src, bits, surface->pitch) != (size_t)surface->pitch) {
             goto done;
         }
-        if (biBitCount == 8 && palette && biClrUsed < (1u << biBitCount)) {
+        if (biBitCount == 8 && surface->internal->palette && biClrUsed < (1u << biBitCount)) {
             for (i = 0; i < surface->w; ++i) {
                 if (bits[i] >= biClrUsed) {
                     SDL_SetError("A BMP image contains a pixel with a color out of the palette");

src/video/SDL_pixels.c

@@ -1022,8 +1022,7 @@ SDL_Palette *SDL_CreatePalette(int ncolors)
     if (!palette) {
         return NULL;
     }
-    palette->colors =
-        (SDL_Color *)SDL_malloc(ncolors * sizeof(*palette->colors));
+    palette->colors = (SDL_Color *)SDL_malloc(ncolors * sizeof(*palette->colors));
     if (!palette->colors) {
         SDL_free(palette);
         return NULL;
@@ -1386,6 +1385,11 @@ static Uint8 *Map1toN(const SDL_Palette *pal, Uint8 Rmod, Uint8 Gmod, Uint8 Bmod
         return NULL;
     }
 
+    /* An all-zero map for surfaces without a palette. */
+    if (!pal) {
+        return map;
+    }
+
     /* We memory copy to the pixel map so the endianness is preserved */
     for (i = 0; i < pal->ncolors; ++i) {
         Uint8 R = (Uint8)((pal->colors[i].r * Rmod) / 255);
@@ -1405,6 +1409,11 @@ static Uint8 *MapNto1(const SDL_PixelFormatDetails *src, const SDL_Palette *pal,
     SDL_Palette dithered;
     SDL_Color colors[256];
 
+    if (!pal) {
+        *identical = 1;
+        return NULL;
+    }
+
     dithered.ncolors = 256;
     SDL_DitherColors(colors, 8);
     dithered.colors = colors;
@@ -1467,8 +1476,11 @@ int SDL_MapSurface(SDL_Surface *src, SDL_Surface *dst)
     if (SDL_ISPIXELFORMAT_INDEXED(srcfmt->format)) {
         if (SDL_ISPIXELFORMAT_INDEXED(dstfmt->format)) {
             /* Palette --> Palette */
-            map->info.table =
-                Map1to1(srcpal, dstpal, &map->identity);
+            if (srcpal && dstpal) {
+                map->info.table = Map1to1(srcpal, dstpal, &map->identity);
+            } else {
+                map->identity = 1;
+            }
             if (!map->identity) {
                 if (!map->info.table) {
                     return -1;

src/video/SDL_surface.c

@@ -158,25 +158,6 @@ static SDL_Surface *SDL_InitializeSurface(SDL_InternalSurface *mem, int width, i
     surface->internal->map.info.b = 0xFF;
     surface->internal->map.info.a = 0xFF;
 
-    if (SDL_ISPIXELFORMAT_INDEXED(surface->format)) {
-        SDL_Palette *palette = SDL_CreatePalette((1 << SDL_BITSPERPIXEL(surface->format)));
-        if (!palette) {
-            SDL_DestroySurface(surface);
-            return NULL;
-        }
-        if (palette->ncolors == 2) {
-            /* Create a black and white bitmap palette */
-            palette->colors[0].r = 0xFF;
-            palette->colors[0].g = 0xFF;
-            palette->colors[0].b = 0xFF;
-            palette->colors[1].r = 0x00;
-            palette->colors[1].g = 0x00;
-            palette->colors[1].b = 0x00;
-        }
-        SDL_SetSurfacePalette(surface, palette);
-        SDL_DestroyPalette(palette);
-    }
-
     if (colorspace != SDL_COLORSPACE_UNKNOWN &&
         colorspace != SDL_GetDefaultColorspaceForFormat(format)) {
         SDL_SetSurfaceColorspace(surface, colorspace);

test/testautomation_surface.c

@@ -854,37 +854,54 @@ static int surface_testFlip(void *arg)
 
 static int surface_testPalette(void *arg)
 {
-    SDL_Surface *surface, *output;
+    SDL_Surface *source, *surface, *output;
     SDL_Palette *palette;
     Uint8 *pixels;
-    int offset;
+
+    palette = SDL_CreatePalette(2);
+    SDLTest_AssertCheck(palette != NULL, "SDL_CreatePalette()");
+
+    source = SDL_CreateSurface(1, 1, SDL_PIXELFORMAT_INDEX8);
+    SDLTest_AssertCheck(source != NULL, "SDL_CreateSurface()");
 
     surface = SDL_CreateSurface(1, 1, SDL_PIXELFORMAT_INDEX8);
     SDLTest_AssertCheck(surface != NULL, "SDL_CreateSurface()");
 
+    pixels = (Uint8 *)surface->pixels;
+    SDLTest_AssertCheck(*pixels == 0, "Expected *pixels == 0 got %u", *pixels);
+
+    /* Identity copy between indexed surfaces without a palette */
+    *(Uint8 *)source->pixels = 1;
+    SDL_BlitSurface(source, NULL, surface, NULL);
+    SDLTest_AssertCheck(*pixels == 1, "Expected *pixels == 1 got %u", *pixels);
+
+    /* Identity copy between indexed surfaces where the destination has a palette */
+    palette->colors[0].r = 0;
+    palette->colors[0].g = 0;
+    palette->colors[0].b = 0;
+    palette->colors[1].r = 0xFF;
+    palette->colors[1].g = 0;
+    palette->colors[1].b = 0;
+    SDL_SetSurfacePalette(surface, palette);
+    *pixels = 0;
+    SDL_BlitSurface(source, NULL, surface, NULL);
+    SDLTest_AssertCheck(*pixels == 1, "Expected *pixels == 1 got %u", *pixels);
+
     output = SDL_CreateSurface(1, 1, SDL_PIXELFORMAT_RGBA32);
     SDLTest_AssertCheck(output != NULL, "SDL_CreateSurface()");
 
-    *(Uint8 *)surface->pixels = 255;
-
     pixels = (Uint8 *)output->pixels;
-    offset = 0;
-
     SDL_BlitSurface(surface, NULL, output, NULL);
-    SDLTest_AssertCheck(pixels[offset] == 0xFF,
-                        "Expected pixels[%d] == 0xFF got 0x%.2X", offset, pixels[offset]);
+    SDLTest_AssertCheck(*pixels == 0xFF, "Expected *pixels == 0xFF got 0x%.2X", *pixels);
 
     /* Set the palette color and blit again */
-    palette = SDL_GetSurfacePalette(surface);
-    SDLTest_AssertCheck(palette != NULL, "Expected palette != NULL, got %p", palette);
-    if (palette) {
-        palette->colors[255].r = 0xAA;
-    }
+    palette->colors[1].r = 0xAA;
     SDL_SetSurfacePalette(surface, palette);
     SDL_BlitSurface(surface, NULL, output, NULL);
-    SDLTest_AssertCheck(pixels[offset] == 0xAA,
-                        "Expected pixels[%d] == 0xAA got 0x%.2X", offset, pixels[offset]);
+    SDLTest_AssertCheck(*pixels == 0xAA, "Expected *pixels == 0xAA got 0x%.2X", *pixels);
 
+    SDL_DestroyPalette(palette);
+    SDL_DestroySurface(source);
     SDL_DestroySurface(surface);
     SDL_DestroySurface(output);