Deal with various .bmp file format variants in SDL_LoadBMP_RW().

This helps when modern versions of The Gimp (and lots of other things) produces a 32-bit bitmap with an alpha channel, or anything with "BI_BITFIELDS" format, since that data is now embedded in the bitmap info header instead of directly following it and we would accidentally skip over embedded versions of it. Fixes Bugzilla #2714.
libsdl-org · Sep 8, 2014 · bcaae0b · bcaae0b
1 parent 0b98d0c
commit bcaae0b
Showing 1 changed file with 84 additions and 69 deletions.
diff --git a/src/video/SDL_bmp.c b/src/video/SDL_bmp.c
@@ -85,15 +85,17 @@ SDL_LoadBMP_RW(SDL_RWops * src, int freesrc)
     int bmpPitch;
     int i, pad;
     SDL_Surface *surface;
-    Uint32 Rmask;
-    Uint32 Gmask;
-    Uint32 Bmask;
-    Uint32 Amask;
+    Uint32 Rmask = 0;
+    Uint32 Gmask = 0;
+    Uint32 Bmask = 0;
+    Uint32 Amask = 0;
     SDL_Palette *palette;
     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;
 
     /* The Win32 BMP file header (14 bytes) */
@@ -144,15 +146,14 @@ SDL_LoadBMP_RW(SDL_RWops * src, int freesrc)
 
     /* Read the Win32 BITMAPINFOHEADER */
     biSize = SDL_ReadLE32(src);
-    if (biSize == 12) {
+    if (biSize == 12) {   /* really old BITMAPCOREHEADER */
         biWidth = (Uint32) SDL_ReadLE16(src);
         biHeight = (Uint32) SDL_ReadLE16(src);
         /* biPlanes = */ SDL_ReadLE16(src);
         biBitCount = SDL_ReadLE16(src);
         biCompression = BI_RGB;
-    } else {
-        const unsigned int headerSize = 40;
-
+    } else if (biSize >= 40) {  /* some version of BITMAPINFOHEADER */
+        Uint32 headerSize;
         biWidth = SDL_ReadLE32(src);
         biHeight = SDL_ReadLE32(src);
         /* biPlanes = */ SDL_ReadLE16(src);
@@ -164,6 +165,54 @@ SDL_LoadBMP_RW(SDL_RWops * src, int freesrc)
         biClrUsed = SDL_ReadLE32(src);
         /* biClrImportant = */ SDL_ReadLE32(src);
 
+        /* 64 == BITMAPCOREHEADER2, an incompatible OS/2 2.x extension. Skip this stuff for now. */
+        if (biSize == 64) {
+            /* ignore these extra fields. */
+            if (biCompression == BI_BITFIELDS) {
+                /* this value is actually huffman compression in this variant. */
+                SDL_SetError("Compressed BMP files not supported");
+                was_error = SDL_TRUE;
+                goto done;
+            }
+        } else {
+            /* This is complicated. If compression is BI_BITFIELDS, then
+               we have 3 DWORDS that specify the RGB masks. This is either
+               stored here in an BITMAPV2INFOHEADER (which only differs in
+               that it adds these RGB masks) and biSize >= 52, or we've got
+               these masks stored in the exact same place, but strictly
+               speaking, this is the bmiColors field in BITMAPINFO immediately
+               following the legacy v1 info header, just past biSize. */
+            if (biCompression == BI_BITFIELDS) {
+                haveRGBMasks = SDL_TRUE;
+                Rmask = SDL_ReadLE32(src);
+                Gmask = SDL_ReadLE32(src);
+                Bmask = SDL_ReadLE32(src);
+
+                /* ...v3 adds an alpha mask. */
+                if (biSize >= 56) {  /* BITMAPV3INFOHEADER; adds alpha mask */
+                    haveAlphaMask = SDL_TRUE;
+                    Amask = SDL_ReadLE32(src);
+                }
+            } else {
+                /* the mask fields are ignored for v2+ headers if not BI_BITFIELD. */
+                if (biSize >= 52) {  /* BITMAPV2INFOHEADER; adds RGB masks */
+                    /*Rmask = */ SDL_ReadLE32(src);
+                    /*Gmask = */ SDL_ReadLE32(src);
+                    /*Bmask = */ SDL_ReadLE32(src);
+                }
+                if (biSize >= 56) {  /* BITMAPV3INFOHEADER; adds alpha mask */
+                    /*Amask = */ SDL_ReadLE32(src);
+                }
+            }
+
+            /* Insert other fields here; Wikipedia and MSDN say we're up to
+               v5 of this header, but we ignore those for now (they add gamma,
+               color spaces, etc). Ignoring the weird OS/2 2.x format, we
+               currently parse up to v3 correctly (hopefully!). */
+        }
+
+        /* skip any header bytes we didn't handle... */
+        headerSize = (Uint32) (SDL_RWtell(src) - (fp_offset + 14));
         if (biSize > headerSize) {
             SDL_RWseek(src, (biSize - headerSize), RW_SEEK_CUR);
         }
@@ -194,80 +243,46 @@ SDL_LoadBMP_RW(SDL_RWops * src, int freesrc)
     }
 
     /* We don't support any BMP compression right now */
-    Rmask = Gmask = Bmask = Amask = 0;
     switch (biCompression) {
     case BI_RGB:
         /* If there are no masks, use the defaults */
-        if (bfOffBits == (14 + biSize)) {
-            /* Default values for the BMP format */
-            switch (biBitCount) {
-            case 15:
-            case 16:
-                Rmask = 0x7C00;
-                Gmask = 0x03E0;
-                Bmask = 0x001F;
-                break;
-            case 24:
-#if SDL_BYTEORDER == SDL_BIG_ENDIAN
-                Rmask = 0x000000FF;
-                Gmask = 0x0000FF00;
-                Bmask = 0x00FF0000;
-#else
-                Rmask = 0x00FF0000;
-                Gmask = 0x0000FF00;
-                Bmask = 0x000000FF;
-#endif
-                break;
-            case 32:
-                /* We don't know if this has alpha channel or not */
-                correctAlpha = SDL_TRUE;
-                Amask = 0xFF000000;
-                Rmask = 0x00FF0000;
-                Gmask = 0x0000FF00;
-                Bmask = 0x000000FF;
-                break;
-            default:
-                break;
-            }
-            break;
-        }
-        /* Fall through -- read the RGB masks */
-
-    case BI_BITFIELDS:
+        SDL_assert(!haveRGBMasks);
+        SDL_assert(!haveAlphaMask);
+        /* Default values for the BMP format */
         switch (biBitCount) {
         case 15:
         case 16:
-            Rmask = SDL_ReadLE32(src);
-            Gmask = SDL_ReadLE32(src);
-            Bmask = SDL_ReadLE32(src);
+            Rmask = 0x7C00;
+            Gmask = 0x03E0;
+            Bmask = 0x001F;
+            break;
+        case 24:
+#if SDL_BYTEORDER == SDL_BIG_ENDIAN
+            Rmask = 0x000000FF;
+            Gmask = 0x0000FF00;
+            Bmask = 0x00FF0000;
+#else
+            Rmask = 0x00FF0000;
+            Gmask = 0x0000FF00;
+            Bmask = 0x000000FF;
+#endif
             break;
         case 32:
-            Rmask = SDL_ReadLE32(src);
-            Gmask = SDL_ReadLE32(src);
-            Bmask = SDL_ReadLE32(src);
-            Amask = SDL_ReadLE32(src);
-
-            /* ImageMagick seems to put out bogus masks here. Pick a default. */
-            if ((Rmask == 0xFFFFFF) && (Gmask == 0xFFFFFF) &&
-                (Bmask == 0xFFFFFF) && (Amask == 0xFFFFFF) ) {
-                Amask = 0xFF000000;
-                Rmask = 0x00FF0000;
-                Gmask = 0x0000FF00;
-                Bmask = 0x000000FF;
-            } else if ((Rmask == 0xFFFFFF00) && (Gmask == 0xFFFFFF00) &&
-                       (Bmask == 0xFFFFFF00) && (Amask == 0xFFFFFF00) ) {
-                /* argh, The Gimp seems to put out different bogus masks! */
-                Amask = 0x000000FF;
-                Rmask = 0xFF000000;
-                Gmask = 0x00FF0000;
-                Bmask = 0x0000FF00;
-            }
-
+            /* We don't know if this has alpha channel or not */
+            correctAlpha = SDL_TRUE;
+            Amask = 0xFF000000;
+            Rmask = 0x00FF0000;
+            Gmask = 0x0000FF00;
+            Bmask = 0x000000FF;
             break;
         default:
             break;
         }
         break;
+
+    case BI_BITFIELDS:
+        break;  /* we handled this in the info header. */
+
     default:
         SDL_SetError("Compressed BMP files not supported");
         was_error = SDL_TRUE;