SDL: Support returning <8bpp surfaces in SDL_LoadBMP_RW

From 05e7dcf8f80724c46d44e736c9b7d3714529c761 Mon Sep 17 00:00:00 2001
From: Cameron Cawley <[EMAIL REDACTED]>
Date: Fri, 17 Nov 2023 14:10:32 +0000
Subject: [PATCH] Support returning <8bpp surfaces in SDL_LoadBMP_RW

---
 src/video/SDL_bmp.c     | 101 +++++++++++-----------------------------
 test/testcustomcursor.c |   7 ++-
 test/testutils.c        |   7 ++-
 3 files changed, 38 insertions(+), 77 deletions(-)

diff --git a/src/video/SDL_bmp.c b/src/video/SDL_bmp.c
index c9fdcdbaf518..c5fe6c7deb01 100644
--- a/src/video/SDL_bmp.c
+++ b/src/video/SDL_bmp.c
@@ -197,7 +197,6 @@ SDL_Surface *SDL_LoadBMP_RW(SDL_RWops *src, SDL_bool freesrc)
 {
     SDL_bool was_error = SDL_TRUE;
     Sint64 fp_offset = 0;
-    int bmpPitch;
     int i, pad;
     SDL_Surface *surface;
     Uint32 Rmask = 0;
@@ -208,7 +207,6 @@ SDL_Surface *SDL_LoadBMP_RW(SDL_RWops *src, SDL_bool freesrc)
     Uint8 *bits;
     Uint8 *top, *end;
     SDL_bool topDown;
-    int ExpandBMP;
     SDL_bool haveRGBMasks = SDL_FALSE;
     SDL_bool haveAlphaMask = SDL_FALSE;
     SDL_bool correctAlpha = SDL_FALSE;
@@ -365,14 +363,8 @@ SDL_Surface *SDL_LoadBMP_RW(SDL_RWops *src, SDL_bool freesrc)
         goto done;
     }
 
-    /* Expand 1, 2 and 4 bit bitmaps to 8 bits per pixel */
+    /* Reject invalid bit depths */
     switch (biBitCount) {
-    case 1:
-    case 2:
-    case 4:
-        ExpandBMP = biBitCount;
-        biBitCount = 8;
-        break;
     case 0:
     case 3:
     case 5:
@@ -381,7 +373,6 @@ SDL_Surface *SDL_LoadBMP_RW(SDL_RWops *src, SDL_bool freesrc)
         SDL_SetError("%d-bpp BMP images are not supported", biBitCount);
         goto done;
     default:
-        ExpandBMP = 0;
         break;
     }
 
@@ -514,89 +505,49 @@ SDL_Surface *SDL_LoadBMP_RW(SDL_RWops *src, SDL_bool freesrc)
     }
     top = (Uint8 *)surface->pixels;
     end = (Uint8 *)surface->pixels + (surface->h * surface->pitch);
-    switch (ExpandBMP) {
-    case 1:
-        bmpPitch = (biWidth + 7) >> 3;
-        pad = (((bmpPitch) % 4) ? (4 - ((bmpPitch) % 4)) : 0);
-        break;
-    case 2:
-        bmpPitch = (biWidth + 3) >> 2;
-        pad = (((bmpPitch) % 4) ? (4 - ((bmpPitch) % 4)) : 0);
-        break;
-    case 4:
-        bmpPitch = (biWidth + 1) >> 1;
-        pad = (((bmpPitch) % 4) ? (4 - ((bmpPitch) % 4)) : 0);
-        break;
-    default:
-        pad = ((surface->pitch % 4) ? (4 - (surface->pitch % 4)) : 0);
-        break;
-    }
+    pad = ((surface->pitch % 4) ? (4 - (surface->pitch % 4)) : 0);
     if (topDown) {
         bits = top;
     } else {
         bits = end - surface->pitch;
     }
     while (bits >= top && bits < end) {
-        switch (ExpandBMP) {
-        case 1:
-        case 2:
-        case 4:
-        {
-            Uint8 pixel = 0;
-            int shift = (8 - ExpandBMP);
+        if (SDL_RWread(src, bits, surface->pitch) != (size_t)surface->pitch) {
+            goto done;
+        }
+        if (biBitCount == 8 && palette && biClrUsed < (1u << biBitCount)) {
             for (i = 0; i < surface->w; ++i) {
-                if (i % (8 / ExpandBMP) == 0) {
-                    if (!SDL_ReadU8(src, &pixel)) {
-                        goto done;
-                    }
-                }
-                bits[i] = (pixel >> shift);
                 if (bits[i] >= biClrUsed) {
                     SDL_SetError("A BMP image contains a pixel with a color out of the palette");
                     goto done;
                 }
-                pixel <<= ExpandBMP;
-            }
-        } break;
-
-        default:
-            if (SDL_RWread(src, bits, surface->pitch) != (size_t)surface->pitch) {
-                goto done;
-            }
-            if (biBitCount == 8 && 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");
-                        goto done;
-                    }
-                }
             }
+        }
 #if SDL_BYTEORDER == SDL_BIG_ENDIAN
-            /* Byte-swap the pixels if needed. Note that the 24bpp
-               case has already been taken care of above. */
-            switch (biBitCount) {
-            case 15:
-            case 16:
-            {
-                Uint16 *pix = (Uint16 *)bits;
-                for (i = 0; i < surface->w; i++) {
-                    pix[i] = SDL_Swap16(pix[i]);
-                }
-                break;
+        /* Byte-swap the pixels if needed. Note that the 24bpp
+           case has already been taken care of above. */
+        switch (biBitCount) {
+        case 15:
+        case 16:
+        {
+            Uint16 *pix = (Uint16 *)bits;
+            for (i = 0; i < surface->w; i++) {
+                pix[i] = SDL_Swap16(pix[i]);
             }
+            break;
+        }
 
-            case 32:
-            {
-                Uint32 *pix = (Uint32 *)bits;
-                for (i = 0; i < surface->w; i++) {
-                    pix[i] = SDL_Swap32(pix[i]);
-                }
-                break;
-            }
+        case 32:
+        {
+            Uint32 *pix = (Uint32 *)bits;
+            for (i = 0; i < surface->w; i++) {
+                pix[i] = SDL_Swap32(pix[i]);
             }
-#endif
             break;
         }
+        }
+#endif
+
         /* Skip padding bytes, ugh */
         if (pad) {
             Uint8 padbyte;
diff --git a/test/testcustomcursor.c b/test/testcustomcursor.c
index ee90c782dba2..afd74b121135 100644
--- a/test/testcustomcursor.c
+++ b/test/testcustomcursor.c
@@ -72,7 +72,12 @@ init_color_cursor(const char *file)
     SDL_Surface *surface = SDL_LoadBMP(file);
     if (surface) {
         if (surface->format->palette) {
-            SDL_SetSurfaceColorKey(surface, 1, *(Uint8 *)surface->pixels);
+            const Uint8 bpp = surface->format->BitsPerPixel;
+            const Uint8 mask = (1 << bpp) - 1;
+            if (SDL_PIXELORDER(surface->format->format) == SDL_BITMAPORDER_4321)
+                SDL_SetSurfaceColorKey(surface, 1, (*(Uint8 *)surface->pixels) & mask);
+            else
+                SDL_SetSurfaceColorKey(surface, 1, ((*(Uint8 *)surface->pixels) >> (8 - bpp)) & mask);
         } else {
             switch (surface->format->BitsPerPixel) {
             case 15:
diff --git a/test/testutils.c b/test/testutils.c
index f6eb3780eeab..e9d505297bb8 100644
--- a/test/testutils.c
+++ b/test/testutils.c
@@ -116,7 +116,12 @@ LoadTexture(SDL_Renderer *renderer, const char *file, SDL_bool transparent,
         /* Set transparent pixel as the pixel at (0,0) */
         if (transparent) {
             if (temp->format->palette) {
-                SDL_SetSurfaceColorKey(temp, SDL_TRUE, *(Uint8 *)temp->pixels);
+                const Uint8 bpp = temp->format->BitsPerPixel;
+                const Uint8 mask = (1 << bpp) - 1;
+                if (SDL_PIXELORDER(temp->format->format) == SDL_BITMAPORDER_4321)
+                    SDL_SetSurfaceColorKey(temp, SDL_TRUE, (*(Uint8 *)temp->pixels) & mask);
+                else
+                    SDL_SetSurfaceColorKey(temp, SDL_TRUE, ((*(Uint8 *)temp->pixels) >> (8 - bpp)) & mask);
             } else {
                 switch (temp->format->BitsPerPixel) {
                 case 15: