BMP loader cannot handle inverted row data

[GoogleCodeExporter]

The Problem

The BMP format specifies that the height of the BMP can either be positive or 
negative.  If the height is positive, rows are stored in reverse order from 
bottom to top.  If the height is negative, rows are stored from top to bottom.

It seems that Grafx2's BMP loader can only handle BMPs with positive heights.  
It pulls in the value as an unsigned dword, which means that negative heights 
are represented as their two's complement positive value.

The upshot of this is that attempting to load BMPs saved by Acorn, for example, 
will cause malloc to fail when Grafx2 attempts to allocate massive amounts of 
RAM for itself.  Worse, because the program remembers which image was loaded 
last and reloads it on startup, loading a BMP with a negative height will 
render the program completely useless.  Every attempt to start it causes it to 
try to load the bitmap, which causes it to crash.

Suggested Solutions

1. Replace the BMP loader with a less naive solution.  In addition to this 
problem, it doesn't seem to support all header types, nor does it work around 
issues common in Mac image editors, such as the fact that they rarely set the 
image size value correctly.  This is the ideal.

2. Detect negative heights by reading the value in as signed and throw up an 
"unsupported BMP format" alert.  This would be a good interim solution.

Version Info

Using Grafx2 svn 1759 for OSX.

Original issue reported on code.google.com by antdze...@gmail.com on 17 May 2011 at 8:51

[GoogleCodeExporter]

I see the problem with negative height, it seems easy to fix. Do you know where 
we can find such BMP file to test? 
Can you provide some details about the other problems you refer ? ("issues 
common in Mac image editors (...) they rarely set the image size value 
correctly")

Original comment by yrizoud on 17 May 2011 at 10:02

• Changed state: Accepted
• Added labels: OpSys-All
• Removed labels: OpSys-

[GoogleCodeExporter]

There are some test images here:

http://ant.simianzombie.com/testbmps/

They have all been created using Acorn v2 in OSX.

Regarding the other issues, if you open one of those files up in a hex editor 
you'll notice that the value for image size (called Size_3 in your BMP header) 
is 0.  It should be equal to the size of the file minus the size of the header, 
and represents the total amount of non-header data in the image (note that it's 
not the same as width * height * bits per pixel, as the image will undoubtedly 
include extra padding between rows and at the end of the file).  However, none 
of the editors I have here - Pixen, Acorn, Paintbrush - set the value 
correctly.  They all leave it as 0, which makes me think that they're using an 
official BMP API from Apple that is mangling the header data.

Original comment by antdze...@gmail.com on 17 May 2011 at 10:30

[GoogleCodeExporter]

Thanks, samples help a lot.

About "Size3", I just found a source that says about it: "If there is no 
compression, it is valid to set this member to zero." 
(http://www.fortunecity.com/skyscraper/windows/364/bmpffrmt.html)
It seems that each time I have to look up some details on BMP format, I find 
contradicting information. Grafx2 will actually ignore this value, even for 
compressed bitmaps. And Grafx2 itself has always saved a zero for this field. 
--'

Original comment by yrizoud on 17 May 2011 at 11:55

[GoogleCodeExporter]

No prob.  Thanks for clarifying the Size3 field.

You're right about the amount of contradictory information around the BMP spec. 
 The only reason I spotted this bug in the first place is because I'm fixing 
the same problem in my own Python BMP loader...

Original comment by antdze...@gmail.com on 17 May 2011 at 12:00

[GoogleCodeExporter]

I think this was fixed in a recent changeset. Can anyone confirm ? :)

Original comment by pulkoma...@gmail.com on 19 Jun 2011 at 8:38

[GoogleCodeExporter]

Fixed in r1787 (release branch) for 2.3, and merged to trunk for 2.4wip.

Original comment by yrizoud on 20 Jun 2011 at 10:32

• Changed state: Fixed