# Off-Topic Discussion > The Lounge > Tech Talk >  >  What's wrong with this?

## ninja9578

Everything is set up right in OpenGL and the glut window is 800x600.  The engine works, this is the code that doesn't work.  GLUT is in RGB mode:





> glutInitDisplayMode (GLUT_DOUBLE|GLUT_RGB|GLUT_DEPTH);



It creates a file (usually about 1.4MB, but Preview won't open it, saying that it's not a supported format.  Everything is commented so that you can see what everything is.

This is the part of the header that's important:




> #  ifndef WIN32
> typedef struct                       /**** BMP file header structure ****/
>     {
>     unsigned short bfType;           /* Magic number for file */
>     unsigned int   bfSize;           /* Size of file */
>     unsigned short bfReserved1;      /* Reserved */
>     unsigned short bfReserved2;      /* ... */
>     unsigned int   bfOffBits;        /* Offset to bitmap data */
>     } BITMAPFILEHEADER;
> ...



And here's my method to take a picture.




> void snapshot(int windowWidth, int windowHeight, char * filename){
> 	char * bmpBuffer = (char *)malloc(windowWidth * windowHeight * 3);
> 	if  (!bmpBuffer) return;
> 
> 	glReadPixels((GLint)0, (GLint)0, (GLint)windowWidth - 1, (GLint)windowHeight - 1, GL_RGB,  GL_UNSIGNED_BYTE, bmpBuffer);
> 
> 	FILE *filePtr = fopen(filename,  "wb");
> 	if (!filePtr) return;
> 
> ...



I've been looking at it for an hour, I don't see it.

----------


## Replicon

I'm not sure if the 'b' in wb does this, but be certain you are opening the file in binary mode (and not text mode).

Check the exact byte size of your file to see if it matches what it should be.

----------


## Ynot

I *think* because you've opened the file in "wb" (write binary) that the multiple fwrites at the bottom are overwritting each other

try opening the file with "ab" (append binary)




```
......
	FILE *filePtr = fopen(filename,  "ab");
......
```


Each call to write to the file should now append the write to the end, rather than overwrite what came before

----------


## ninja9578

As far as I know fwrites don't overwrite each other.  The "wb" style only overwrites the file if it already exists, each fwrite appends to that.  "ab" opens a file that already exists and adds to it.  

Anyway, I tried your solution and it's still come out and unsupported format.

----------


## Ynot

try adding fflush after each write (this will commit the write to disk
then sleep for 30 seconds, then onto the next write + flush

you can then have the file open (in a hex editor, or whatever) and see exactly what each write does

If you're sure all the data structures are correct, the problem's got to be how they are being written to file

----------


## Ynot

the only other thing I can see, is the way you've stored the data size in the file header

you've got



```
.....
char * bmpBuffer = (char *)malloc(windowWidth * windowHeight * 3);
.....
bitmapFileHeader.bfSize = windowWidth * windowHeight * 3;
.....
```


it's possible the data type and/or value is wrong
try using sizeof() instead

----------


## Ynot

Also, try changing



```
bitmapFileHeader.bfType = 0x424D;  //"MB"  - remember bitmap data is bottom to top, right to left
```


to



```
bitmapFileHeader.bfType = 0x4D42;  //"MB"  - remember bitmap data is bottom to top, right to left
```



0x424D = BM
0x4D42 = MB

Now, MB is mentioned in the headers, (possibly backwards BM, to show processing is backwards - bottom right to top left) - don't know

----------


## Replicon

Yeah, append is actually going to break the file if the first run of the software wrote it correctly  :smiley: 

General advice, though: Learn to use streams.  ::D: 

And if this is part of a bigger project, why reinvent the wheel? There are plenty of lightweight libraries to do what you want to do. What else is dangerous in there...

char * bmpBuffer = (char *)malloc(windowWidth * windowHeight * 3);

How do you know about that 3? I assume you're assuming 24-bit colour... what if it's actually 32? Or 16? Your code should check that before allocating blindly. I realize that's probably how you've initialized it, but.... ugh!

To fix your problem: How much of that code is copied directly from a book (or perhaps nehe or where ever), and how much of it did you customize?

----------


## ninja9578

I tried both an "MB" and "BM" type header, neither one worked.  

I looked for two hours for a library to do this for me and came up with either too big of a library with unnecessary functions I only needed this one method.  I copied it directly (for the most part,) but the header wasn't including so I took that from one of my professor's pages.  That's why I think an error lies somewhere in compatibility in the structs, but I don't see any.

I know that it's three because as I said in the first post I set up GLUT with the GL_RGB mode.  This is always 24 bits, 32 bits would be the GL_RGBA mode.  GLUT relays the information to OpenGL which sets up it's frame buffer accordingly.

What did you mean by use stream?  This is technically a file stream?  You think I should use fprintf and fflush()?

----------


## Replicon

This will be useful for you: http://en.wikipedia.org/wiki/BMP_file_format

So, you definitely want to use "BM" (0x424D).

Open up your trusty hex editor (which I assume you have - if not, you can write a hex dumper in maybe 10-20 minutes time), and compare what's in your file header with what you expect in the bitmap, based on the values you input in your code. Check if one of your offsets is off... check if your data structures match exactly what's going on in the link I put up there (I checked real quick and it looks fine at first glance).

One question:




```
bitmapInfoHeader.biWidth = windowWidth - 1;
bitmapInfoHeader.biHeight = windowHeight - 1;
```


Why the "-1"?

By streams for file i/o, I meant in general, fstream is preferred these days for these things, assuming you're using c++. I once ran into a nasty cross-platform bug (well, annoyance) that in Linux, it was binary by default, but in windows, it wasn't, so I had to just have O_BINARY be zero if undefined... yuck... I switched it to fstream, and it's much cleaner now. In a nutshell:




```
std::ofstream out("filename", std::ios::out | std::ios::trunc | std::ios::binary);

if( ! out ) {
    // throw, etc.
}

out.write( addressOfData, sizeOfData );

// ...

out.close();
```


But like I said above, there are two things I would do right away:

1) Figure out if your file size matches exactly what it should be, which I think is:

sizeof(BITMAPFILEHEADER) + sizeof(BITMAPINFOHEADER) + windowWidth * windowHeight * 3



If it's slightly larger, you're probably not writing in binary (even if you have the right flag set, I dunno).

2) Look into that "-1" issue - I think it's a bug. Your width is still windowWidth - you just read "from pixel 0 to pixel windowWidth-1", which is windowWidth pixels.

----------


## ninja9578

I printed out the size of the information to be 1440056 and the Inspector says that the files are 1.4MB, so that checks out.

This is a GNU C file, no C++ style streams.  

You are right about the -1, that was a bug, but not the one that I'm looking for.  That wouldn't cause the file to be unreadable, that would just skew everything a little bit.  

I checked the Wiki and all my headers seem to line up.  My headers seemed to line up, but I printed out their sizes to make sure.  The file headers is printing out as 16 bytes, it's supposed to be 14.  There's the problem.  What is causing it?  The format of the struct is above, three unsigned shorts and two ints, that should be 14.

----------


## Replicon

> I printed out the size of the information to be 1440056 and the Inspector says that the files are 1.4MB, so that checks out.



That's not good enough.  :tongue2:  What is the file size in bytes? It needs to be exactly equal to what you quoted above.





> This is a GNU C file, no C++ style streams.



tsk tsk  :wink2:  (just messin)





> You are right about the -1, that was a bug, but not the one that I'm looking for.  That wouldn't cause the file to be unreadable, that would just skew everything a little bit.



One less bug  :smiley: 






> I checked the Wiki and all my headers seem to line up.  My headers seemed to line up, but I printed out their sizes to make sure.  The file headers is printing out as 16 bytes, it's supposed to be 14.  There's the problem.  What is causing it?  The format of the struct is above, three unsigned shorts and two ints, that should be 14.



That might explain it. This means reading the next header is offset, which breaks things. just for the sake of sanity, you can output the sizes of all your data types manually.

It might actually be caused by internal padding for memory alignment. Try writing the fields manually to the file instead of dumping the entire data structure.

EDIT: hey, what if WIN32 IS defined, and you're using some other data structure?  :tongue2:

----------


## ninja9578

> tsk tsk  (just messin)



 :tongue2:  this is a fluid simulation, which is insanely slow.  I need to do hand optimization.  I've never had a C++ file beat any of my C files.  C++ gets converted to C before compilation you know  :tongue2: 





> It might actually be caused by internal padding for memory alignment. Try writing the fields manually to the file instead of dumping the entire data structure.



That's what I thought when I saw it too.  I didn't think the PPC architecture did that, I thought only MIPS and Play Station still did memory alignment.





> EDIT: hey, what if WIN32 IS defined, and you're using some other data structure?



I'm not planning on running this on a Windows computer, that was left over from my professor's code.  He used something that was built into the window.h header.

So I shortened the dump by two bytes because I assumed that the padding was at the end of the struct.  It didn't work.  Then I moved the starting point up by two bytes and it still didn't work.  Then I tried moving the starting point 1 byte (1 one each side) and it still failed.  Yes I changed the offset part of the file structure too.  Grrrrrrr...

It's definitely something in the file header because I tried an IBM info header and it still fails.  And if there is a problem with the way that the bitmap is stored then it should still open, it'll just look weird.

----------


## Replicon

Don't just guess at it - look at your output file in a hex editor and figure out what's going on. Do it quick cause I'm getting antsy about knowing what happened  ::D:

----------


## Replicon

I bet your int (bfSize) is what's misaligned. It comes after a short, so it probably wants to be off by 2 bytes. The rest works out fine.

Try writing each member one at a time, just for the hell of it.

Here's an interesting blog posting about memory alignment in PPC: http://www.moythreads.com/wordpress/...ory-alignment/

----------


## ninja9578

You were right about bfSize being offset by 2 bytes.  To fix it I got rid of the struct and just used regular primatives, but it still didn't solve the problem.

I re-factored the bfSize to include the size of the headers  :tongue2:  like it's supposed to be.

Also, how come nobody called me on this little bug: sizeof(bmpBuffer)?  sizeof(a pointer) will always return 8  :tongue2: 

I opened my hex editor and meticulously translated every single thing.  It looks right to me, here is a screenshot of all the code that I'm using and the hex editor itself.

http://img225.imageshack.us/img225/2020/codemf7.png



Neither the Windows or OS2 headers work and yes I changed all the number correctly when I tried it.

----------


## Replicon

Probably because you don't have sizeof(bmpBuffer) in your code above  :tongue2: 

In your current code, you're not writing the data, for one. (you have it commented out)

I just noticed that bfSize is supposed to be the size of the entire file... what a crappy definition!!!

If all else fails, try the following:

Take a 24-bit screenshot of something exactly the same size as your window, and check that everything matches in your known, working bitmap  :smiley:

----------


## Replicon

In fact, here, I'll do it for you  :smiley: 

Happy debugging!

edit: sorry, stupid forum software resized it. It's still readable, though kinda... barely hehe.

----------


## ninja9578

> Probably because you don't have sizeof(bmpBuffer) in your code above



Whoops, that was something that I added later.  My mistake.





> In your current code, you're not writing the data, for one. (you have it commented out)



I commented it out so that you guys could see just the header in the hex.





> Take a 24-bit screenshot of something exactly the same size as your window, and check that everything matches in your known, working bitmap



Wow, they weren't even close to the ones that Photoshop created.  I'm using the brute force method.  I'm ripping the header out of Photoshop and writing that in.  Everything is tinted a weird shade of red, but I know why it does that.

----------

