BMP and it's weird upside-down storage

[PiMaster]

I'm making a BMP loader/editor, and I've run into a big dilemma... While I can load the BMP, to change a pixel I have to account for the fact that BMP image data is stored "upside-down" compared to most raster graphics. The first index is bottom-left, and it goes from left to right, bottom to top. I tried to come up with a simple equation, but I'm getting a SEGFAULT error, so it must be wrong. How do I calculate the index of the array for this?

//so far I have this (frame is for possible expansion to GIFs) (data is void*)
color& sprite::operator()(int x,int y,int frame=0){
	return *(((color*)frames[frame]->data)+(width*(height-y-1)+x));}

[Disch]

Code clarity is more important than compactness. Instead of cramming everything onto one line like that, break it up so the logic is easier to follow. That way these kinds of errors are less likely.

Having multiple lines doesn't make your code any slower.

Anyway I don't see anything wrong as long as sizeof(color) is the same size as the size of 1 pixel. My guess is that your pixels are smaller than sizeof(color) and that's causing this code to go passed the end of the buffer.



Also.... even if the bitmap file format has the pixels "upside-down", that doesn't mean your code has to. You can flip the data when you load it so that you don't have to access it upside-down like that.

Plus, not all bmps are upside down. Only those with a positive height are. Negative heights are stored "normally".


EDIT: also, why are you using void* at all? If you're casting to a color* all the time... why not just use color*? safer/less code/etc/etc...

[PiMaster]

1.) I'm making it so compact because I don't think it's that complicated. Cast data as color*, add the index, and dereference it. But then, we all have different preferences for style.
2.) Derp, why didn't I think of that? Check if height is positive, and if it is, change it to negative in the BITMAPINFO struct and flip all the data.
3.) I was originally using void* because I was planning on supporting more than just 24-bit color, but now that I think about it, it's probably easier to just convert every bpp to 24-bit.

Thanks for your help!

[Disch]

1) Fair enough. Personally I find 3-deep nested parenthesis with no whitespace to be somewhat cluttered, but whatever.

3) This is 24 bit? Well there's your problem.

sizeof one pixel is going to be 3 bytes if it's 24-bit, but sizeof(color) is likely 4 bytes -- which is causing you to access incorrect data, and is making you step outside the bounds of the array.



Also note that bitmap rows are padded to 4 byte boundaries, so width*y isn't necessarily correct. You'd need pitch*y where 'pitch' is the number of bytes for each row.

For example... a 24-bit bitmap with a width of 17 will have 51 bytes of pixel data (17*3 = 51).
However the actual bitmap rows will be spaced 52 bytes apart (padded to next 4-byte boundary).


The lesson here is don't try to cast raw binary data to a specific type. What I would do instead is have an array of colors that you index normally, and when you load the file, instead of just reading all of the pixel data into a buffer -- read it into your color array appropriately.

This will make the loading code a little more complicated, but will add more flexibility and safety (and speed) to the actual pixel data access.

[Duthomhas]

1) It isn't that complicated, but it is complicated enough that you made an error. Disch's instruction to be clear instead of compact is not just style, it is wisdom that comes from industry.


Having raster-up data is actually the norm -- for older formats. It is only more modern graphics formats that have image data stored in top-to-bottom rasters. But either way, it is easier to just load it into a representation in memory that matches your hardware and/or bitblit routines.

Good luck!