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 - Wrong handling of legacy 32bpp BMP files
Summary: Wrong handling of legacy 32bpp BMP files
Status: RESOLVED FIXED
Alias: None
Product: SDL_image
Classification: Unclassified
Component: misc (show other bugs)
Version: 1.2.12
Hardware: All All
: P2 normal
Assignee: Sam Lantinga
QA Contact: Sam Lantinga
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-07-07 13:35 UTC by Kang Seonghoon
Modified: 2013-07-07 21:25 UTC (History)
0 users

See Also:

Attachments
test image (1.17 MB, image/bmp)
2013-07-07 13:35 UTC, Kang Seonghoon 		Details
test image with padding bytes of zeroes (1.17 MB, image/bmp)
2013-07-07 20:36 UTC, Kang Seonghoon 		Details
32-bit BMP with unused bits set to 0 (31.80 KB, image/bmp)
2013-07-07 21:20 UTC, Sam Lantinga 		Details
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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