We are currently migrating Bugzilla to GitHub issues.
Any changes made to the bug tracker now will be lost, so please do not post new bugs or make changes to them.
When we're done, all bug URLs will redirect to their equivalent location on the new bug tracker.

Bug 1943

Summary: Wrong handling of legacy 32bpp BMP files
Product: SDL_image Reporter: Kang Seonghoon <public+libsdl>
Component: miscAssignee: Sam Lantinga <slouken>
Status: RESOLVED FIXED QA Contact: Sam Lantinga <slouken>
Severity: normal    
Priority: P2    
Version: 1.2.12   
Hardware: All   
OS: All   
Attachments: test image
test image with padding bytes of zeroes
32-bit BMP with unused bits set to 0

Description Kang Seonghoon 2013-07-07 13:35:47 UTC
Created attachment 1211 [details]
test image

While BMP format supports alpha channel, it is enabled only when the header is at least 56 bytes long (BITMAPV3INFOHEADER and later). For very common 40-byte-long header (BITMAPINFOHEADER) 32bpp format should be interpreted as BGRX format, but currently SDL_image interprets them as BGRA format and causes a significant compatibility problem as many 32bpp files use a padding byte of 0 ("transparent" in BGRA interpretation).

I've attached a test image that exhibits this problem. I haven't tested with the SDL_image code, but my suggestion is to change the following lines in IMG_bmp.c from:

~~~~
                    case 32:
                        Amask = 0xFF000000;
                        Rmask = 0x00FF0000;
                        Gmask = 0x0000FF00;
                        Bmask = 0x000000FF;
                        break;
~~~~

to:

~~~~
                    case 32:
                        Amask = (biSize < 56 ? 0 : 0xFF000000); /* no alpha before BITMAPV3INFOHEADER */
                        Rmask = 0x00FF0000;
                        Gmask = 0x0000FF00;
                        Bmask = 0x000000FF;
                        break;
~~~~

(As a reference, this corresponds to lines 325--330 in hg rev c5eb519b5161.)
Comment 1 Sam Lantinga 2013-07-07 17:05:11 UTC
The GIMP saves out images with an alpha channel and a 40 byte header. In fact the test image you provide has an alpha gradient in it, and Chrome displays it as such.

Do you have a sample image that is 32-bit and pads the fourth byte with 0?
Comment 2 Kang Seonghoon 2013-07-07 20:36:07 UTC
Created attachment 1212 [details]
test image with padding bytes of zeroes

Ah, that test image is artificially constructed to illustrate the problem. And your point is very interesting: Chrome uses the alpha chanel *only when* the padding bytes are entirely zeroes! Apparantly there are some bogus BMPs out there with padding bytes indeed used as alpha channel. Relevant Blink code is available at:

https://chromium.googlesource.com/chromium/blink/+/ac49bb2/Source/core/platform/image-decoders/bmp/BMPImageReader.cpp (line 694)

I've attached newly constructed test image. As a matter of fact, the original image was constructed with the following Python script:

~~~~
import colorsys
with open('test.bmp', 'wb') as f:
    f.write('424d38c0120000000000360000002800000080020000e0010000010020000000000002c01200120b0000120b00000000000000000000'.decode('hex'))
    horiz = [''.join(chr(int(round(j*255))) for j in colorsys.hsv_to_rgb(i/640., 1, 1)[::-1]) for i in xrange(640)]
    vert = [chr(int(round(j/480.*255))) for j in xrange(480)]
    for y in xrange(480):
        for x in xrange(640):
            f.write(horiz[x] + vert[y])
~~~~
Comment 3 Sam Lantinga 2013-07-07 21:20:09 UTC
Created attachment 1213 [details]
32-bit BMP with unused bits set to 0
Comment 4 Sam Lantinga 2013-07-07 21:25:24 UTC
Okay, this is fixed, thanks!
http://hg.libsdl.org/SDL_image/rev/830b565dee54