Improved color accuracy blitting to 8-bit indexed surfaces

Fixes https://github.com/libsdl-org/SDL/issues/10519

src/video/SDL_blit.h

@@ -23,6 +23,8 @@
 #ifndef SDL_blit_h_
 #define SDL_blit_h_
 
+#include "../SDL_hashtable.h"
+
 /* Table to do pixel byte expansion */
 extern const Uint8 *SDL_expand_byte[9];
 extern const Uint16 SDL_expand_byte_10[];
@@ -70,6 +72,7 @@ typedef struct
     const SDL_PixelFormatDetails *dst_fmt;
     const SDL_Palette *dst_pal;
     Uint8 *table;
+    SDL_HashTable *palette_map;
     int flags;
     Uint32 colorkey;
     Uint8 r, g, b, a;

src/video/SDL_blit_N.c

@@ -933,234 +933,6 @@ static void Blit_RGB444_XRGB8888ARMSIMD(SDL_BlitInfo *info)
 #define LO 1
 #endif
 
-/* Special optimized blit for RGB 8-8-8 --> RGB 3-3-2 */
-#define RGB888_RGB332(dst, src)                    \
-    {                                              \
-        dst = (Uint8)((((src)&0x00E00000) >> 16) | \
-                      (((src)&0x0000E000) >> 11) | \
-                      (((src)&0x000000C0) >> 6));  \
-    }
-static void Blit_XRGB8888_index8(SDL_BlitInfo *info)
-{
-#ifndef USE_DUFFS_LOOP
-    int c;
-#endif
-    int width, height;
-    Uint32 *src;
-    const Uint8 *map;
-    Uint8 *dst;
-    int srcskip, dstskip;
-
-    /* Set up some basic variables */
-    width = info->dst_w;
-    height = info->dst_h;
-    src = (Uint32 *)info->src;
-    srcskip = info->src_skip / 4;
-    dst = info->dst;
-    dstskip = info->dst_skip;
-    map = info->table;
-
-    if (!map) {
-        while (height--) {
-#ifdef USE_DUFFS_LOOP
-            /* *INDENT-OFF* */ /* clang-format off */
-            DUFFS_LOOP(
-                RGB888_RGB332(*dst++, *src);
-            , width);
-            /* *INDENT-ON* */ /* clang-format on */
-#else
-            for (c = width / 4; c; --c) {
-                /* Pack RGB into 8bit pixel */
-                ++src;
-                RGB888_RGB332(*dst++, *src);
-                ++src;
-                RGB888_RGB332(*dst++, *src);
-                ++src;
-                RGB888_RGB332(*dst++, *src);
-                ++src;
-            }
-            switch (width & 3) {
-            case 3:
-                RGB888_RGB332(*dst++, *src);
-                ++src;
-                SDL_FALLTHROUGH;
-            case 2:
-                RGB888_RGB332(*dst++, *src);
-                ++src;
-                SDL_FALLTHROUGH;
-            case 1:
-                RGB888_RGB332(*dst++, *src);
-                ++src;
-            }
-#endif /* USE_DUFFS_LOOP */
-            src += srcskip;
-            dst += dstskip;
-        }
-    } else {
-        int Pixel;
-
-        while (height--) {
-#ifdef USE_DUFFS_LOOP
-            /* *INDENT-OFF* */ /* clang-format off */
-            DUFFS_LOOP(
-                RGB888_RGB332(Pixel, *src);
-                *dst++ = map[Pixel];
-                ++src;
-            , width);
-            /* *INDENT-ON* */ /* clang-format on */
-#else
-            for (c = width / 4; c; --c) {
-                /* Pack RGB into 8bit pixel */
-                RGB888_RGB332(Pixel, *src);
-                *dst++ = map[Pixel];
-                ++src;
-                RGB888_RGB332(Pixel, *src);
-                *dst++ = map[Pixel];
-                ++src;
-                RGB888_RGB332(Pixel, *src);
-                *dst++ = map[Pixel];
-                ++src;
-                RGB888_RGB332(Pixel, *src);
-                *dst++ = map[Pixel];
-                ++src;
-            }
-            switch (width & 3) {
-            case 3:
-                RGB888_RGB332(Pixel, *src);
-                *dst++ = map[Pixel];
-                ++src;
-                SDL_FALLTHROUGH;
-            case 2:
-                RGB888_RGB332(Pixel, *src);
-                *dst++ = map[Pixel];
-                ++src;
-                SDL_FALLTHROUGH;
-            case 1:
-                RGB888_RGB332(Pixel, *src);
-                *dst++ = map[Pixel];
-                ++src;
-            }
-#endif /* USE_DUFFS_LOOP */
-            src += srcskip;
-            dst += dstskip;
-        }
-    }
-}
-
-/* Special optimized blit for RGB 10-10-10 --> RGB 3-3-2 */
-#define RGB101010_RGB332(dst, src)                 \
-    {                                              \
-        dst = (Uint8)((((src)&0x38000000) >> 22) | \
-                      (((src)&0x000E0000) >> 15) | \
-                      (((src)&0x00000300) >> 8));  \
-    }
-static void Blit_RGB101010_index8(SDL_BlitInfo *info)
-{
-#ifndef USE_DUFFS_LOOP
-    int c;
-#endif
-    int width, height;
-    Uint32 *src;
-    const Uint8 *map;
-    Uint8 *dst;
-    int srcskip, dstskip;
-
-    /* Set up some basic variables */
-    width = info->dst_w;
-    height = info->dst_h;
-    src = (Uint32 *)info->src;
-    srcskip = info->src_skip / 4;
-    dst = info->dst;
-    dstskip = info->dst_skip;
-    map = info->table;
-
-    if (!map) {
-        while (height--) {
-#ifdef USE_DUFFS_LOOP
-            /* *INDENT-OFF* */ /* clang-format off */
-            DUFFS_LOOP(
-                RGB101010_RGB332(*dst++, *src);
-            , width);
-            /* *INDENT-ON* */ /* clang-format on */
-#else
-            for (c = width / 4; c; --c) {
-                /* Pack RGB into 8bit pixel */
-                ++src;
-                RGB101010_RGB332(*dst++, *src);
-                ++src;
-                RGB101010_RGB332(*dst++, *src);
-                ++src;
-                RGB101010_RGB332(*dst++, *src);
-                ++src;
-            }
-            switch (width & 3) {
-            case 3:
-                RGB101010_RGB332(*dst++, *src);
-                ++src;
-                SDL_FALLTHROUGH;
-            case 2:
-                RGB101010_RGB332(*dst++, *src);
-                ++src;
-                SDL_FALLTHROUGH;
-            case 1:
-                RGB101010_RGB332(*dst++, *src);
-                ++src;
-            }
-#endif /* USE_DUFFS_LOOP */
-            src += srcskip;
-            dst += dstskip;
-        }
-    } else {
-        int Pixel;
-
-        while (height--) {
-#ifdef USE_DUFFS_LOOP
-            /* *INDENT-OFF* */ /* clang-format off */
-            DUFFS_LOOP(
-                RGB101010_RGB332(Pixel, *src);
-                *dst++ = map[Pixel];
-                ++src;
-            , width);
-            /* *INDENT-ON* */ /* clang-format on */
-#else
-            for (c = width / 4; c; --c) {
-                /* Pack RGB into 8bit pixel */
-                RGB101010_RGB332(Pixel, *src);
-                *dst++ = map[Pixel];
-                ++src;
-                RGB101010_RGB332(Pixel, *src);
-                *dst++ = map[Pixel];
-                ++src;
-                RGB101010_RGB332(Pixel, *src);
-                *dst++ = map[Pixel];
-                ++src;
-                RGB101010_RGB332(Pixel, *src);
-                *dst++ = map[Pixel];
-                ++src;
-            }
-            switch (width & 3) {
-            case 3:
-                RGB101010_RGB332(Pixel, *src);
-                *dst++ = map[Pixel];
-                ++src;
-                SDL_FALLTHROUGH;
-            case 2:
-                RGB101010_RGB332(Pixel, *src);
-                *dst++ = map[Pixel];
-                ++src;
-                SDL_FALLTHROUGH;
-            case 1:
-                RGB101010_RGB332(Pixel, *src);
-                *dst++ = map[Pixel];
-                ++src;
-            }
-#endif /* USE_DUFFS_LOOP */
-            src += srcskip;
-            dst += dstskip;
-        }
-    }
-}
-
 /* Special optimized blit for RGB 8-8-8 --> RGB 5-5-5 */
 #define RGB888_RGB555(dst, src)                                    \
     {                                                              \
@@ -2072,97 +1844,6 @@ static void Blit_RGB555_ARGB1555(SDL_BlitInfo *info)
     }
 }
 
-static void BlitNto1(SDL_BlitInfo *info)
-{
-#ifndef USE_DUFFS_LOOP
-    int c;
-#endif
-    int width, height;
-    Uint8 *src;
-    const Uint8 *map;
-    Uint8 *dst;
-    int srcskip, dstskip;
-    int srcbpp;
-    Uint32 Pixel;
-    int sR, sG, sB;
-    const SDL_PixelFormatDetails *srcfmt;
-
-    /* Set up some basic variables */
-    width = info->dst_w;
-    height = info->dst_h;
-    src = info->src;
-    srcskip = info->src_skip;
-    dst = info->dst;
-    dstskip = info->dst_skip;
-    map = info->table;
-    srcfmt = info->src_fmt;
-    srcbpp = srcfmt->bytes_per_pixel;
-
-    if (!map) {
-        while (height--) {
-#ifdef USE_DUFFS_LOOP
-            /* *INDENT-OFF* */ /* clang-format off */
-            DUFFS_LOOP(
-                DISEMBLE_RGB(src, srcbpp, srcfmt, Pixel,
-                                sR, sG, sB);
-                if ( 1 ) {
-                    /* Pack RGB into 8bit pixel */
-                    *dst = (Uint8)(((sR>>5)<<(3+2)) | ((sG>>5)<<(2)) | ((sB>>6)<<(0)));
-                }
-                dst++;
-                src += srcbpp;
-            , width);
-            /* *INDENT-ON* */ /* clang-format on */
-#else
-            for (c = width; c; --c) {
-                DISEMBLE_RGB(src, srcbpp, srcfmt, Pixel, sR, sG, sB);
-                if (1) {
-                    /* Pack RGB into 8bit pixel */
-                    *dst = ((sR >> 5) << (3 + 2)) |
-                           ((sG >> 5) << (2)) | ((sB >> 6) << (0));
-                }
-                dst++;
-                src += srcbpp;
-            }
-#endif
-            src += srcskip;
-            dst += dstskip;
-        }
-    } else {
-        while (height--) {
-#ifdef USE_DUFFS_LOOP
-            /* *INDENT-OFF* */ /* clang-format off */
-            DUFFS_LOOP(
-                DISEMBLE_RGB(src, srcbpp, srcfmt, Pixel,
-                                sR, sG, sB);
-                if ( 1 ) {
-                    /* Pack RGB into 8bit pixel */
-                    *dst = map[((sR>>5)<<(3+2))|
-                           ((sG>>5)<<(2))  |
-                           ((sB>>6)<<(0))  ];
-                }
-                dst++;
-                src += srcbpp;
-            , width);
-            /* *INDENT-ON* */ /* clang-format on */
-#else
-            for (c = width; c; --c) {
-                DISEMBLE_RGB(src, srcbpp, srcfmt, Pixel, sR, sG, sB);
-                if (1) {
-                    /* Pack RGB into 8bit pixel */
-                    *dst = map[((sR >> 5) << (3 + 2)) |
-                               ((sG >> 5) << (2)) | ((sB >> 6) << (0))];
-                }
-                dst++;
-                src += srcbpp;
-            }
-#endif /* USE_DUFFS_LOOP */
-            src += srcskip;
-            dst += dstskip;
-        }
-    }
-}
-
 /* blits 32 bit RGB<->RGBA with both surfaces having the same R,G,B fields */
 static void Blit4to4MaskAlpha(SDL_BlitInfo *info)
 {
@@ -2474,71 +2155,6 @@ static void BlitNtoNCopyAlpha(SDL_BlitInfo *info)
     }
 }
 
-static void BlitNto1Key(SDL_BlitInfo *info)
-{
-    int width = info->dst_w;
-    int height = info->dst_h;
-    Uint8 *src = info->src;
-    int srcskip = info->src_skip;
-    Uint8 *dst = info->dst;
-    int dstskip = info->dst_skip;
-    const SDL_PixelFormatDetails *srcfmt = info->src_fmt;
-    const Uint8 *palmap = info->table;
-    Uint32 ckey = info->colorkey;
-    Uint32 rgbmask = ~srcfmt->Amask;
-    int srcbpp;
-    Uint32 Pixel;
-    unsigned sR, sG, sB;
-
-    /* Set up some basic variables */
-    srcbpp = srcfmt->bytes_per_pixel;
-    ckey &= rgbmask;
-
-    if (!palmap) {
-        while (height--) {
-            /* *INDENT-OFF* */ /* clang-format off */
-            DUFFS_LOOP(
-            {
-                DISEMBLE_RGB(src, srcbpp, srcfmt, Pixel,
-                                sR, sG, sB);
-                if ( (Pixel & rgbmask) != ckey ) {
-                    /* Pack RGB into 8bit pixel */
-                    *dst = (Uint8)(((sR>>5)<<(3+2))|
-                                   ((sG>>5)<<(2)) |
-                                   ((sB>>6)<<(0)));
-                }
-                dst++;
-                src += srcbpp;
-            },
-            width);
-            /* *INDENT-ON* */ /* clang-format on */
-            src += srcskip;
-            dst += dstskip;
-        }
-    } else {
-        while (height--) {
-            /* *INDENT-OFF* */ /* clang-format off */
-            DUFFS_LOOP(
-            {
-                DISEMBLE_RGB(src, srcbpp, srcfmt, Pixel,
-                                sR, sG, sB);
-                if ( (Pixel & rgbmask) != ckey ) {
-                    /* Pack RGB into 8bit pixel */
-                    *dst = (Uint8)palmap[((sR>>5)<<(3+2))|
-                                         ((sG>>5)<<(2))  |
-                                         ((sB>>6)<<(0))  ];
-                }
-                dst++;
-                src += srcbpp;
-            },
-            width);
-            /* *INDENT-ON* */ /* clang-format on */
-            src += srcskip;
-            dst += dstskip;
-        }
-    }
-}
-
 static void Blit2to2Key(SDL_BlitInfo *info)
 {
     int width = info->dst_w;
@@ -3343,22 +2959,7 @@ SDL_BlitFunc SDL_CalculateBlitN(SDL_Surface *surface)
     switch (surface->internal->map.info.flags & ~SDL_COPY_RLE_MASK) {
     case 0:
         blitfun = NULL;
-        if (dstfmt->bits_per_pixel == 8) {
-            if ((srcfmt->bytes_per_pixel == 4) &&
-                (srcfmt->Rmask == 0x00FF0000) &&
-                (srcfmt->Gmask == 0x0000FF00) &&
-                (srcfmt->Bmask == 0x000000FF)) {
-                blitfun = Blit_XRGB8888_index8;
-            } else if ((srcfmt->bytes_per_pixel == 4) &&
-                       (srcfmt->Rmask == 0x3FF00000) &&
-                       (srcfmt->Gmask == 0x000FFC00) &&
-                       (srcfmt->Bmask == 0x000003FF)) {
-                blitfun = Blit_RGB101010_index8;
-            } else {
-                blitfun = BlitNto1;
-            }
-        } else {
-            /* Now the meat, choose the blitter we want */
+        if (dstfmt->bits_per_pixel > 8) {
             Uint32 a_need = NO_ALPHA;
             if (dstfmt->Amask) {
                 a_need = srcfmt->Amask ? COPY_ALPHA : SET_ALPHA;
@@ -3418,15 +3019,13 @@ SDL_BlitFunc SDL_CalculateBlitN(SDL_Surface *surface)
 
         if (srcfmt->bytes_per_pixel == 2 && surface->internal->map.identity != 0) {
             return Blit2to2Key;
-        } else if (dstfmt->bytes_per_pixel == 1) {
-            return BlitNto1Key;
         } else {
 #ifdef SDL_ALTIVEC_BLITTERS
             if ((srcfmt->bytes_per_pixel == 4) && (dstfmt->bytes_per_pixel == 4) && SDL_HasAltiVec()) {
                 return Blit32to32KeyAltivec;
             } else
 #endif
-                if (srcfmt->Amask && dstfmt->Amask) {
+            if (srcfmt->Amask && dstfmt->Amask) {
                 return BlitNtoNKeyCopyAlpha;
             } else {
                 return BlitNtoNKey;

src/video/SDL_blit_slow.c

@@ -69,15 +69,21 @@ void SDL_Blit_Slow(SDL_BlitInfo *info)
     const SDL_Palette *src_pal = info->src_pal;
     const SDL_PixelFormatDetails *dst_fmt = info->dst_fmt;
     const SDL_Palette *dst_pal = info->dst_pal;
+    SDL_HashTable *palette_map = info->palette_map;
     int srcbpp = src_fmt->bytes_per_pixel;
     int dstbpp = dst_fmt->bytes_per_pixel;
     SlowBlitPixelAccess src_access;
     SlowBlitPixelAccess dst_access;
     Uint32 rgbmask = ~src_fmt->Amask;
     Uint32 ckey = info->colorkey & rgbmask;
+    Uint32 last_pixel = 0;
+    Uint8 last_index = 0;
 
     src_access = GetPixelAccessMethod(src_fmt->format);
     dst_access = GetPixelAccessMethod(dst_fmt->format);
+    if (dst_access == SlowBlitPixelAccess_Index8) {
+        last_index = SDL_LookupRGBAColor(palette_map, last_pixel, dst_pal);
+    }
 
     incy = ((Uint64)info->src_h << 16) / info->dst_h;
     incx = ((Uint64)info->src_w << 16) / info->dst_w;
@@ -275,12 +281,12 @@ void SDL_Blit_Slow(SDL_BlitInfo *info)
 
             switch (dst_access) {
             case SlowBlitPixelAccess_Index8:
-                RGB332_FROM_RGB(dstpixel, dstR, dstG, dstB);
-                if (info->table) {
-                    *dst = info->table[dstpixel];
-                } else {
-                    *dst = dstpixel;
+                dstpixel = (dstR << 24 | dstG << 16 | dstB << 8 | dstA);
+                if (dstpixel != last_pixel) {
+                    last_pixel = dstpixel;
+                    last_index = SDL_LookupRGBAColor(palette_map, dstpixel, dst_pal);
                 }
+                *dst = last_index;
                 break;
             case SlowBlitPixelAccess_RGB:
                 ASSEMBLE_RGB(dst, dstbpp, dst_fmt, dstR, dstG, dstB);

src/video/SDL_pixels.c

@@ -1131,6 +1131,23 @@ Uint8 SDL_FindColor(const SDL_Palette *pal, Uint8 r, Uint8 g, Uint8 b, Uint8 a)
     return pixel;
 }
 
+Uint8 SDL_LookupRGBAColor(SDL_HashTable *palette_map, Uint32 pixel, const SDL_Palette *pal)
+{
+    Uint8 color_index = 0;
+    const void *value;
+    if (SDL_FindInHashTable(palette_map, (const void *)(uintptr_t)pixel, &value)) {
+        color_index = (Uint8)(uintptr_t)value;
+    } else {
+        Uint8 r = (Uint8)((pixel >> 24) & 0xFF);
+        Uint8 g = (Uint8)((pixel >> 16) & 0xFF);
+        Uint8 b = (Uint8)((pixel >>  8) & 0xFF);
+        Uint8 a = (Uint8)((pixel >>  0) & 0xFF);
+        color_index = SDL_FindColor(pal, r, g, b, a);
+        SDL_InsertIntoHashTable(palette_map, (const void *)(uintptr_t)pixel, (const void *)(uintptr_t)color_index);
+    }
+    return color_index;
+}
+
 /* Tell whether palette is opaque, and if it has an alpha_channel */
 void SDL_DetectPalette(const SDL_Palette *pal, SDL_bool *is_opaque, SDL_bool *has_alpha_channel)
 {
@@ -1401,24 +1418,6 @@ static Uint8 *Map1toN(const SDL_Palette *pal, Uint8 Rmod, Uint8 Gmod, Uint8 Bmod
     return map;
 }
 
-/* Map from BitField to Dithered-Palette to Palette */
-static Uint8 *MapNto1(const SDL_PixelFormatDetails *src, const SDL_Palette *pal, int *identical)
-{
-    /* Generate a 256 color dither palette */
-    SDL_Palette dithered;
-    SDL_Color colors[256];
-
-    if (!pal) {
-        SDL_SetError("dst does not have a palette set");
-        return NULL;
-    }
-
-    dithered.colors = colors;
-    dithered.ncolors = SDL_arraysize(colors);
-    SDL_DitherPalette(&dithered);
-    return Map1to1(&dithered, pal, identical);
-}
-
 int SDL_ValidateMap(SDL_Surface *src, SDL_Surface *dst)
 {
     SDL_BlitMap *map = &src->internal->map;
@@ -1449,8 +1448,14 @@ void SDL_InvalidateMap(SDL_BlitMap *map)
     map->info.dst_pal = NULL;
     map->src_palette_version = 0;
     map->dst_palette_version = 0;
-    SDL_free(map->info.table);
-    map->info.table = NULL;
+    if (map->info.table) {
+        SDL_free(map->info.table);
+        map->info.table = NULL;
+    }
+    if (map->info.palette_map) {
+        SDL_DestroyHashTable(map->info.palette_map);
+        map->info.palette_map = NULL;
+    }
 }
 
 int SDL_MapSurface(SDL_Surface *src, SDL_Surface *dst)
@@ -1504,13 +1509,7 @@ int SDL_MapSurface(SDL_Surface *src, SDL_Surface *dst)
     } else {
         if (SDL_ISPIXELFORMAT_INDEXED(dstfmt->format)) {
             /* BitField --> Palette */
-            map->info.table = MapNto1(srcfmt, dstpal, &map->identity);
-            if (!map->identity) {
-                if (!map->info.table) {
-                    return -1;
-                }
-            }
-            map->identity = 0; /* Don't optimize to copy */
+            map->info.palette_map = SDL_CreateHashTable(NULL, 32, SDL_HashID, SDL_KeyMatchID, NULL, SDL_FALSE);
         } else {
             /* BitField --> BitField */
             if (srcfmt == dstfmt) {

src/video/SDL_pixels_c.h

@@ -50,6 +50,7 @@ extern int SDL_MapSurface(SDL_Surface *src, SDL_Surface *dst);
 /* Miscellaneous functions */
 extern void SDL_DitherPalette(SDL_Palette *palette);
 extern Uint8 SDL_FindColor(const SDL_Palette *pal, Uint8 r, Uint8 g, Uint8 b, Uint8 a);
+extern Uint8 SDL_LookupRGBAColor(SDL_HashTable *palette_map, Uint32 pixel, const SDL_Palette *pal);
 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);