Sign in to follow this  
Undeadlnsanity

Bitmap file saving problem.

Recommended Posts

Undeadlnsanity    233
Hi all, I have the following code:
unsigned int swidth = getMainScreen()->getWidth();
unsigned int sheight = getMainScreen()->getHeight();
std::vector< unsigned char > pixeldata;
		
pixeldata.resize( swidth * sheight * 3 );

// Clear the pixeldata variable
memset(&pixeldata.front(), 0, swidth * sheight * 3);

glReadPixels(0, 0, swidth, sheight, GL_RGB, GL_BYTE, &pixeldata.front());

// Used for debugging purposes
for (int i = 0; i < pixeldata.size(); ++i) {
	std::cout << pixeldata[i];
}
std::cout << "\n\n";
		
SDL_Surface* image = SDL_CreateRGBSurfaceFrom(&pixeldata.front(), swidth, sheight, 24, 0,
// I'm a bit lost with the masks - hope this will work!
	255U << (16),
	255 << (8),
	255 << (0),
	0 /* no alpha! */);

// Used for debugging purposes
if ( image == NULL ) {
	std::cout << "SDL_Surface creation failed!\n\n";
}
		
SDL_SaveBMP(image, filename.c_str());

By the looks of the std::cout output the glReadPixels() function is doing its job. The SDL_Surface also appears to be being created correctly as the if statement isn't entered. The problem is that the BMP file that is saved isn't readable by any image editing application and it's also 54 bytes which can't be right as the stdout.txt file output by the std::cout calls is well over 1.5 megs. Any help in solving this problem is appreciated. Thanks and take care.

Share this post


Link to post
Share on other sites
Boder    938
Here is some code.


GLubyte *data = new GLubyte[pPreferences->Screen_W * pPreferences->Screen_H * 3];

// read gl screen
glReadPixels( 0, 0, pPreferences->Screen_W, pPreferences->Screen_H,
GL_RGB, GL_UNSIGNED_BYTE, (GLvoid *)data );

// create SDL surface
SDL_Surface *temp = SDL_CreateRGBSurface( SDL_SWSURFACE, pPreferences->Screen_W,
pPreferences->Screen_H, 32, 0x000000ff, 0x0000ff00, 0x00ff0000, 0xff000000 );

SDL_LockSurface( temp );

Uint32 *pdata = (Uint32 *)temp->pixels;

// copy into SDL surface
for( int y = temp->h - 1, y2 = 0; y >= 0 && y2 < pPreferences->Screen_H; --y, ++y2 )
{
for( int x = temp->w -1; x >= 0; --x )
{
pdata[y * temp->pitch/4 + x] =
SDL_MapRGBA( temp->format,
data[ y2 * pPreferences->Screen_W * 3 + x * 3 + 0 ],
data[ y2 * pPreferences->Screen_W * 3 + x * 3 + 1 ],
data[ y2 * pPreferences->Screen_W * 3 + x * 3 + 2 ],
255 );
}
}

SDL_UnlockSurface( temp );

// save the surface
SDL_SaveBMP( temp, filename );

// delete screen data
SDL_FreeSurface( temp );
delete[] data;

Share this post


Link to post
Share on other sites
Undeadlnsanity    233
It works - sort of.

I'm getting the following output:
http://www.arronbailiss.co.uk/junk/fife/screenshot1.jpg

But it should look like this:
http://www.arronbailiss.co.uk/junk/fife/screenshot2.jpg

Here's my code:
GLubyte *data = new GLubyte[swidth * sheight * 3];

// Read the screen and store it into "data"
glReadPixels( 0, 0, swidth, sheight,
GL_RGB, GL_UNSIGNED_BYTE, (GLvoid *)data );

// Create the temporary SDL surface
SDL_Surface *temp = SDL_CreateRGBSurface( SDL_SWSURFACE, swidth, sheight,
24, // Bit depth
255U << (16), // Red channel
255 << (8), // Green channel
255 << (0), // Blue channel
0 /* no alpha! */ );

SDL_LockSurface( temp );

Uint32 *pdata = (Uint32 *)temp->pixels;

// copy into SDL surface
for( int y = temp->h - 1, y2 = 0; y >= 0 && y2 < sheight; --y, ++y2 )
{
for( int x = temp->w -1; x >= 0; --x )
{
pdata[y * temp->pitch/4 + x] =
SDL_MapRGBA( temp->format,
data[ y2 * swidth * 3 + x * 3 + 0 ],
data[ y2 * swidth * 3 + x * 3 + 1 ],
data[ y2 * swidth * 3 + x * 3 + 2 ],
255 );
}
}

SDL_UnlockSurface( temp );

// Save the screenshot
SDL_SaveBMP( temp, filename.c_str() );

// Free up memory
SDL_FreeSurface( temp );
delete[] data;


Thanks

Share this post


Link to post
Share on other sites
Boder    938
Your alignment is probably messed up by casting to an array like

Uint32,

you should cast to a BYTE array

You might be able to read pixels right into the surface. Try this [smile]


// Create the temporary SDL surface
SDL_Surface *temp = SDL_CreateRGBSurface( SDL_SWSURFACE, swidth, sheight,
24, // Bit depth
255U << (16), // Red channel
255 << (8), // Green channel
255 << (0), // Blue channel
0 /* no alpha! */ );

SDL_LockSurface( temp );

// Read the screen and store it into "data"
glReadPixels( 0, 0, swidth, sheight,
GL_RGB, GL_UNSIGNED_BYTE, (GLvoid *)temp->pixels );

SDL_UnlockSurface( temp );

// Save the screenshot
SDL_SaveBMP( temp, filename.c_str() );

// Free up memory
SDL_FreeSurface( temp );

Share this post


Link to post
Share on other sites
Zahlman    1682
Stick with the vector (although make it a vector of GLubyte rather than of unsigned char) rather than doing the allocation yourself - you had a perfectly good idea there. Also, with the resize() call, you don't then need to memset: the vector will be implicitly filled with "default-constructed" GLubytes (i.e. with zero value):

Quote:

teh sgi docz0r (emphasis mine)
[std::vector::resize prototype:] void resize(n, t = T())

[semantics of a.resize(n, t):] ... [m]odifies the container so that it has exactly n elements, inserting elements at the end or erasing elements from the end if necessary. If any elements are inserted, they are copies of t.


That old C stuff is ugly anyway; in C++, prefer std::fill when you need to "memset".

Share this post


Link to post
Share on other sites
Undeadlnsanity    233
Alright now it's creating the full image, but the RGB channels are wrong and the image is upside-down.

I've fiddled with it a little and this is what I've achieved:

http://www.arronbailiss.co.uk/junk/fife/screenshot5.jpg

Here's the current code:
unsigned int swidth = getMainScreen()->getWidth();
unsigned int sheight = getMainScreen()->getHeight();
std::vector< GLubyte > pixeldata;

pixeldata.resize( swidth * sheight * 3 );

SDL_Surface* image = SDL_CreateRGBSurface(SDL_SWSURFACE, swidth, sheight, 24,
// I'm a bit lost with the masks - hope this will work!
255U << (16),
255 << (8),
255 << (0),
0 /* no alpha! */);

SDL_LockSurface( image );

glReadPixels(0, 0, swidth, sheight, GL_RGB, GL_UNSIGNED_BYTE, (GLvoid *)image->pixels);

SDL_UnlockSurface( image );

SDL_SaveBMP(image, filename.c_str());
SDL_FreeSurface( image );



Does anyone have any idea what the problem is and am I defining the RGB channels correctly (255U << (16), 255 << (8) and 255 << (0))?

Thanks a lot for all of your help.

Share this post


Link to post
Share on other sites
Undeadlnsanity    233
Hi again all,

The screwed colours have been fixed by switching the B and R values around. I've tried to flip the image by copying the pixel data from "reversed_image" to "image" in reverse order but I'm receiving the compile error.

Current code:
void RenderBackendOpenGL::captureScreen(const std::string& filename) {
unsigned int swidth = getMainScreen()->getWidth();
unsigned int sheight = getMainScreen()->getHeight();
unsigned char *pixels = malloc (swidth * sheight * 3);

SDL_Surface* reversed_image = SDL_CreateRGBSurface(SDL_SWSURFACE, swidth, sheight, 24,
255U << (0), // Blue channel
255 << (8), // Green channel
255 << (16), // Red channel
0 /* no alpha! */);

SDL_LockSurface( reversed_image );

// Read in the pixel data
glReadPixels(0, 0, swidth, sheight, GL_RGB, GL_UNSIGNED_BYTE, pixels);

SDL_UnlockSurface( reversed_image );

/* At this point the image has been reversed, so we need to re-reverse it so that
it is the correct way around. We do this by copying the "image" pixels to another
surface in reverse order */

SDL_Surface* image = SDL_CreateRGBSurface(SDL_SWSURFACE, swidth, sheight, 24,
255U << (0), // Blue channel
255 << (8), // Green channel
255 << (16), // Red channel
0 /* no alpha! */);

// Copy the "reversed_image" memory to the "image" memory
for (int i = 0; i < sheight; ++i) {
memcpy(image->pixels + image->pitch*i, pixels + image->pitch*(sheight-i-1), swidth * 3); // This is where I'm getting the compile error.
}

// Save file
SDL_SaveBMP(image, filename.c_str());

// Clear memory
SDL_FreeSurface( reversed_image );
SDL_FreeSurface( image );
}



The error is: pointer of type 'void *' used in arithmetic

Thanks again :-).

Share this post


Link to post
Share on other sites
Zahlman    1682
Quote:
Original post by Undeadlnsanity
Hi again all,

The screwed colours have been fixed by switching the B and R values around. I've tried to flip the image by copying the pixel data from "reversed_image" to "image" in reverse order but I'm receiving the compile error.


Don't do that. Instead, reverse the bounds of the loop that iterates over rows.

(The reason you need to do this is that the BMP format actually stores the row data bottom to top. Apparently, this was thought to be a pretty slick hack back when computers were really slow, because it reduced flicker when the image was being drawn and loaded at the same time and took a few refresh cycles to load. Or something like that, anyway.)

And like I said, please stick with the vector, and things like std::fill - or in this case, std::copy. You'll be thankful you did sooner or later.

Share this post


Link to post
Share on other sites
Zahlman    1682
When you are loading the data into the array with the nested loop, you have an "iteration" over rows, and over columns. You want to go over the rows backwards, so that ... er, never mind, the order you do it in doesn't matter; rather, the idea is to copy row X to row (N-X) instead.

Er, wait. Then again, now that I look at it, you already have:


for( int y = temp->h - 1, y2 = 0; y >= 0 && y2 < sheight; --y, ++y2 )


The idea being that 'y' counts backwards over rows, and 'y2' counts forwards over them, and you thus reverse the rows as you copy them across. Apparently (given your results) you're supposed to *not* do that, as a consequence of whatever SDL is doing internally to be BMP-friendly.

Share this post


Link to post
Share on other sites
Undeadlnsanity    233
The problem was fixed with this code:
uint8_t *imagepixels = reinterpret_cast<uint8_t*>(image->pixels);
// Copy the "reversed_image" memory to the "image" memory
for (int y = (sheight - 1); y >= 0; --y) {
uint8_t *row_begin = pixels + y * swidth * 3;
uint8_t *row_end = row_begin + swidth * 3;

std::copy(row_begin, row_end, imagepixels);

// Advance a row in the output surface.
imagepixels += image->pitch;
}


Thanks for your help everyone!

rate++

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this