Creating RGB Surface With Format

SDL Development
1

Hello.

Well, a few days ago I was asking for some help here regarding this subject, but I backed out, as I hate asking for help as much as most people abhor giving it. Since then I have gone through the docs and turned my brain to pineapple pulp trying to figure this out. I have only had limited success. Yes, there is a similar thread around here, but it is not specifically what I am doing. Sorry about that!

So… please help!

THE OBJECTIVE:

  1. Load a 32 bit BMP into an SDL_Surface structure.
  2. Recover the data on the image (format, bytes_pp, width, height, depth, etcetera).
  3. Save the pixel data from the surface (cast Uint32*) to a binary file with SDL_RWops.
  4. Create a new pixel array, identical to the one saved.
  5. Load the pixel data into it with SDL_RWops.
  6. Create a new SDL_Surface with SDL_CreateRGBSurfaceWithFormatFrom(), using the new array pixel data.

When I finally got some results, the data (pixel position and abgr8888) seems to be all mangled, as if there is some keying that I am not applying, or something. Or maybe the array I am storing the data in is inadequate for the format expected? I cannot find any deeper details on that (*pixels), it is a bit vague in the docs. In the printf output, I am noticing there is no consistency in the color order. I notice this because I see the alpha jumping around, from array element to array element.

I am baffled. Is what I am trying to do even possible with SDL?

Anyway, for posterity, here is the code.

#include <SDL.h>
#include <stdio.h>

typedef enum { false, true } bool;

int main(int argc, char* argv[])
{
	if (SDL_Init(SDL_INIT_VIDEO) != 0)
	{
		return -1;
	}
	
	SDL_RWops* the_file = NULL;
	SDL_Surface* new_surface = NULL;
	SDL_Surface* remake_surface = NULL;
	const char* new_image = "bird.bmp";
	new_surface = SDL_LoadBMP_RW(SDL_RWFromFile(new_image, "rb"), 1);

	Uint8 image_bytes_pp = new_surface->format->BytesPerPixel;
	int image_pitch = new_surface->pitch;
	int image_width = new_surface->w;
	int image_height = new_surface->h;
	int image_size = image_width * image_height;
	Uint8 image_depth = new_surface->format->BitsPerPixel;
	SDL_PixelFormat* image_pixel_format = new_surface->format;
	Uint32 pixel_format_enum = new_surface->format->format;
	const char* pixel_format_name = SDL_GetPixelFormatName(pixel_format_enum);
	SDL_Color abgr;
	int x = 0;
	int y = 0;
	int i = 0;
	int x_coord_address_index = x * image_pitch;
	int y_coord_address_index = y * image_bytes_pp;
	int coord_address_index = x_coord_address_index + y_coord_address_index;
	Uint8* pixel_array_channels = NULL;
	Uint8* pixel_address_ptr = NULL;

	printf("%s\n", SDL_GetPixelFormatName(pixel_format_enum));
	printf("%u\n", image_bytes_pp);
	printf("%u\n", image_depth);
	printf("%d\n", image_pitch);
	printf("%d\n", coord_address_index);
	printf("Should this surface be locked? %s", SDL_MUSTLOCK(new_surface) == 0 ? "NO" : "YES");

	SDL_Delay(3500);

	Uint32* pixel_array = SDL_calloc(image_size, sizeof(Uint32));

	//for(y = 0; y < image_height; y++) 
	//for (x = 0; x < image_width; x++)
	//for (x = (image_width - 1); x > -1; x--)
	for (y = (image_height - 1); y > -1; y--)
	{
		//for (x = 0; x < image_width; x++) 
		//for (y = 0; y < image_height; y++)
		//for (y = (image_height - 1); y > -1; y--)
		for (x = (image_width - 1); x > -1; x--)
		{
			x_coord_address_index = x * image_pitch;
			y_coord_address_index = y * image_bytes_pp;
			coord_address_index = x_coord_address_index + y_coord_address_index;
			pixel_address_ptr = (Uint8*)new_surface->pixels + coord_address_index;
			// Something weird is happening here with the following function...
			SDL_GetRGBA(*(Uint32*)pixel_address_ptr, image_pixel_format, &abgr.r, &abgr.g, &abgr.b, &abgr.a);

			if (pixel_array != NULL)
			{
				// Or something weird is happening here. Debugger is not placing it as it is not a programming error, 
				// but a format order and/or scanline storage error, it would seem.
				pixel_array_channels = (Uint8*)&pixel_array[i];
				pixel_array_channels[0] = (Uint8)abgr.r;
				pixel_array_channels[1] = (Uint8)abgr.g;
				pixel_array_channels[2] = (Uint8)abgr.b;
				pixel_array_channels[3] = (Uint8)abgr.a;
				
				// The order of the 

				printf("Red: 0x%02x,  ", pixel_array_channels[0]);
				printf("Green: 0x%02x,  ", pixel_array_channels[1]);
				printf("Blue: 0x%02x,  ", pixel_array_channels[2]);
				printf("Alpha: 0x%02x\n", pixel_array_channels[3]);
				printf("Pixel: 0x%08x\n", pixel_array[i]);

				i++;
			}
			else
			{
				return -1;
			}
		}
	}
	i = 0;
	const char* file_name = "new_bird.bin";

	the_file = SDL_RWFromFile(file_name, "w+b");
	if (the_file != NULL)
	{
		SDL_RWwrite(the_file, pixel_array, sizeof(Uint32), image_size);
		SDL_RWclose(the_file);
		the_file = NULL;
	}

	// There is one last possibility here, though I doubt it.
	// Perhaps the data is getting mangled between the writing and the reading.
	// I assume that SDL_RWread is configured to "read" as SDL_RWwrite "writes".
	// It would not make any sense if it did not, but I do not know.

	Uint32* new_pixel_array = SDL_calloc(image_size, sizeof(Uint32));
	the_file = SDL_RWFromFile(file_name, "r+b");
	if (the_file != NULL)
	{
		SDL_RWread(the_file, new_pixel_array, sizeof(Uint32), image_size);
		SDL_RWclose(the_file);
	}

	remake_surface = SDL_CreateRGBSurfaceWithFormatFrom((Uint32*)new_pixel_array, 
		image_width, image_height, (int)image_depth, image_pitch, pixel_format_enum);

	SDL_Window* local_window = NULL;
	SDL_Surface* local_window_surface = NULL;

	local_window = SDL_CreateWindow("Image", 
		SDL_WINDOWPOS_CENTERED, SDL_WINDOWPOS_CENTERED, 
		image_width, image_height, SDL_WINDOW_SHOWN);

	local_window_surface = SDL_GetWindowSurface(local_window);

	SDL_BlitSurface(remake_surface, NULL, local_window_surface, NULL);
	SDL_UpdateWindowSurface(local_window);

	
	
	bool quit = false;
	SDL_Event loop_event;
	while (quit == false)
	{
		while (SDL_PollEvent(&loop_event) != true)
		{
			if (loop_event.type == SDL_QUIT)
			{
				quit = true;
			}
		}
	}

	SDL_FreeSurface(new_surface);
	SDL_FreeSurface(remake_surface);
	SDL_FreeSurface(local_window_surface);
	SDL_DestroyWindow(local_window);
	SDL_free(pixel_array);
	SDL_free(new_pixel_array);
	SDL_Quit();
	return 0;
}

Here is the image I want to store, on the left. The results, after being put through the program, are on the right. Kind of getting there, after many failures, but not quite there yet…

image
image801×428 115 KB

Is there a trick? Any ideas?

My sincere thanks in advance, if you can spare a little time to de-fuddle me.

2

Ha! Ha! Well, this post was hidden by the Counter Spam Bot for some 24 hours, and during this time I put on the thinking cap again and finally got to the bottom of my errors.

The color issues were solved by ordering the channels properly, according to the recovered format of the 32 bit BMP…

				pixel_array_channels = (Uint8*)&pixel_array[i];
				pixel_array_channels[0] = (Uint8)abgr.b;
				pixel_array_channels[1] = (Uint8)abgr.g;
				pixel_array_channels[2] = (Uint8)abgr.r;
				pixel_array_channels[3] = (Uint8)abgr.a;

My bad!

And the segmentation of the image was solved by fixing the coord_address_index components, and setting up the for loop correctly…

for (y = 0; y < image_height; y++)
	{
		for (x = 0; x < image_width; x++)
		{
			y_coord_address_index = y * image_pitch;
			x_coord_address_index = x * image_bytes_pp;
			coord_address_index = x_coord_address_index + y_coord_address_index;
			pixel_address_ptr = (Uint8*)new_surface->pixels + coord_address_index;
			
			// rest of the code suppressed, as it is generally unchanged. 
		}
	}

Works fine, now. But while I am here;

My system is Little Endian, FWIW to mention. Now, as this is working with the SDL_Surface format itself, and exporting that to the bin file, I have the idea that the Endian-ness of the system is irrelevant. However, even if it is, it is no big deal to test in code and rig accordingly, regarding the color channels. Nonetheless, it would be nice to know if Endian-ness affects an SDL_Surface, once it is actually made.

My thanks. Sorry for the commotion. All the best!

3

For future reference, if you’re trying to copy pixels to/from a surface with the same pixel format, use memcpy() instead of manually copying them one pixel at a time.

And if you want to index into your array of pixels, instead of a bunch of variables and pointer math, use (y * pitch_in_pixels) + x, like this:

for(int y = 0; y < image_height; y++) {
    for(int x = 0; x < image_width; x++) {
        size_t offset = (y * pitch_in_pixels) + x;
        uint32_t pixel = your_pixel_array[offset];
        // do something with pixel
        your_pixel_array[offset] = pixel;
    }
}

Kinda weird that SDL_LoadBMP_RW gave you a BGRA image instead of RGBA. Guess to save you the step of converting it to BGRA yourself for faster blitting, LOL.

4

Thank you, @sjr .

Indeed, my intrinsic apprehension of pointer arithmetic invariably tends to cause me to face it by doing a Fred Astaire tap-dance with a top hat in front of it, when there are often more practical ways, QED.

I suspect this is probably because of my obligatory utilization of the source 32 bit BMP images in the program. In future I will most likely expand the functionality to include other formats. The snippet code was minimum compile code for the demonstration, functions and most error checking suppressed; there is a little bit more to the whole project, part of which at present rejects other depth images, if you try to load them.

Thanks again, very helpful!

5

I just tested it myself and SDL_LoadBMP() does indeed return an image with BGRA pixel format instead of RGBA (even though the BMP file format is RGBA order).

...
SDL_Surface *bmp = SDL_LoadBMP("bitmap.bmp");    // a 32-bit BMP
SDL_PixelFormat *format = bmp->format;
printf("Pixel format: %s\n", SDL_GetPixelFormatName(pixelFormat->format));

Gives

Pixel format: SDL_PIXELFORMAT_ARGB8888
6

BMP files can be any order, if the BI_ALPHABITFIELDS compression type is used. In that case four masks are used to specify the order of R,G,B,A.

7

I just tested it myself and SDL_LoadBMP() does indeed return an image with BGRA pixel format

vs

Pixel format: SDL_PIXELFORMAT_ARGB8888

So apparently it returns ARGB and not BGRA (for your bitmap).

One thing that’s a bit confusing here is that SDL_PIXELFORMAT_ARGB8888 means that each pixel is a 32bit (u)int and:

Uint32 color = ...;
Uint8 alphaByte = color & 0xff; // "least significant" byte of the Uint32
Uint8 redByte   = (color >> 8) & 0xff;
Uint8 greenByte = (color >> 16) & 0xff;
Uint8 blueByte  = (color >> 24) & 0xff; // "most significant" byte of the Uint32

but on Little Endian machines, this means that the lowest address byte is blue, and the highest address byte is alpha:

Uint32 color = ...;
Uint8 colorAsBytes[4];
// copy the raw memory of the Uint32 color into byte array
memcpy(colorAsBytes, &color, 4);
Uint8 alphaByte = colorAsBytes[3];
Uint8 redByte   = colorAsBytes[2];
Uint8 greenByte = colorAsBytes[1];
Uint8 blueByte  = colorAsBytes[0];

So on Little Endian machines, SDL’s SDL_PIXELFORMAT_ARGB8888 is BGRA when you look at the pixeldata as (groups of four) bytes instead of as Uint32.

Your BMP using byte-wise BGRA is not unexpected: 24bit BMPs use (byte-wise) BGR for the pixel-data, see https://docs.microsoft.com/en-us/windows/win32/api/wingdi/ns-wingdi-bitmapv5header:

Each 3-byte triplet in the bitmap array represents the relative intensities of blue, green, and red, respectively, for a pixel.

For 32bit it’s a bit unclear, because strictly speaking it only supports an alpha channel if bV5AlphaMask, i.e. a bitmask for alpha, is specified (see also @rtrussell’s comment), but at least for 32bit data without alpha (where that byte is just a filler), the spec says:

Each DWORD [Uint32] in the bitmap array represents the relative intensities of blue, green, and red for a pixel. The value for blue is in the least significant 8 bits, followed by 8 bits each for green and red. The high byte in each DWORD is not used.

so it’s byte-wise BGRX (remember, BMP is a Little Endian file format, so when they say “least significant 8 bits”, that’s the byte with the lowest address).
So if 32bit data without alpha is byte-wise BGRX by default, it’s reasonable to expect that many image writers use byte-wise BGRA for 32bit data with alpha.
And byte-wise BGRA is SDL_PIXELFORMAT_ARGB8888 (on Little Endian machines).

But of course it’s entirely possible (and legal) that the image writer that produced @William-K’s test image uses byte-wise RGBA data for 32bit BMPs with alpha channel, which (on Little Endian machines) corresponds to SDL_PIXELFORMAT_ABGR8888.

Side-Note: Since SDL 2.0.5 it has a few pixelformat aliases for 32bit data that are independent of platform endianess: SDL_PIXELFORMAT_RGBA32 and friends, see SDL_PixelFormatEnum - SDL Wiki (and even before that it had SDL_PIXELFORMAT_RGB24 / SDL_PIXELFORMAT_BGR24 for byte-wise 24bit RGB / BGR pixeldata)

8

Yeah, I meant BGRA as in that was the in-memory order. AFAIK SDL is kinda the odd one out in terms of defining pixel formats in their logical order, such as SDL_PIXELFORMAT_ARGB8888.

I wasn’t aware that the byte order for BMPs was typically BGRA. For some reason I assumed it’d be RGBA.

9

Not sure how common one or the other is, but it definitely would be less confusing if the default pixelformat for 1-byte-per-channel RGBA data would have RGBA in the name (instead of just having those aliases, which won’t turn up in SDL_GetPixelFormatName().

However, I think it makes sense that “RGBA8888” specifically is based on an Uint32 instead of a byte-array, because there’s also SDL_PIXELFORMAT_ARGB2101010 where the color channels aren’t full bytes but a bunch of bits of an Uint32 (and similarly the Uint16-based formats like SDL_PIXELFORMAT_RGB555 or SDL_PIXELFORMAT_BGRA5551).
Maybe it would’ve been nicer if the Uint-based formats always assumed Big Endian Uints, so a pixelformat always corresponds to the same underlying data (when looking at raw byte-wise memory), and then SDL_PIXELFORMAT_RGBA8888 always would mean “byte-wise RGBA” - but of course it would have caused confusion when actually working on those Uints with bit-operations…

10

Excellent supplementary information. For my part, much gratitude your way.

Some cerebral breaking of wind was caused by exactly this topic of the color channels. The sharp eyed will see my mistake, which was at the root of at least the mismatched color channels problem in my image on the first post. I will take the forbidden liberty of quoting myself, the only mitigating condition being that it is to point out said error, by my assumptions…

This was the result of my transient dyslexia, jumping to conclusions reading…

I also assumed RGBA, backwards. During my reflection between the two first posts on this thread, I took the trouble to read the format carefully… and face-palmed.

As mentioned earlier, one of my hopes had been that the format, saved from the pixel array off the SDL_Surface would be “Endian independent”, and would save me the routine of swapping bytes for platform. Even though this is not a problem, it would be a little less coding in some header somewhere. In any case, I am under the impression (maybe erroneously?) that most platforms are Little Endian these days (I think the Commodore Amiga was Big Endian. Not sure now, it was a while back, and I was not all that concerned, TBH). I am therefore very pleased to read this…

…as it answers the remaining question in my second post.

Great stuff, thanks again!

11

Just for the record:

So, where using SDL_memcpy() is concerned, the code modifies to…

int image_size = surface->h * surface->w * surface->format->BytesPerPixel;
Uint32* pixel_array = SDL_calloc(image_size, sizeof(Uint32));
SDL_memcpy(/*(Uint8*)*/pixel_array, /*(Uint8**)*/surface->pixels, (size_t)image_size);
// Open the RWops file...
SDL_RWwrite(RWops_file, pixel_array, sizeof(Uint32), image_size);
// Close the RWops file...

The suppressed casting was yet another result of transient confusion, completely ignoring the fact that, regardless, memcpy() is byte by byte, which places perfect logic on the image_size being (w * h * bpp). At one stage I even had that cast as (Uint32*), which is pretty “Doh!”, in retrospect, considering what I just finished reasoning above.

See, where the confusion with casting arose was with that pointer to a pointer in the surface->pixels structure, convincing me very erroneously, it seems, that it would be necessary. It is not, for all intents and purposes, but would it be syntactically valid to do so, at least on the pointer to pointer argument? I think not, but cannot be 100% sure of it. The prerogative, I believe, is that if it compiles and works fine without the casting, you are good. But I have had enough experience with this not always being the case before, especially where pointers are concerned with other functions, to justify some wariness as I explore this way about accomplishing the objective.

The output *.bin file recovers the image perfectly with the code…

SDL_RWread(the_file, new_pixel_array, sizeof(Uint32), image_size);
// Image restored correctly with SDL_CreateRGBSurfaceWithFormatFrom()...

Being a bit more adventurous; I experimented with 24 bit images using the same code (ie; the Uint32 array elements would not be “full”). It handles it perfectly well, and obtains the format as…

SDL_PIXELFORMAT_BGR24

Which is not a surprise, really. But my worry comes back to the same Endian-ness issue as before, as this is no longer covered by a convenient SDL_PIXELFORMAT alias. I think, with this, I finally get to the bottom of my remaining doubt:

Would it be necessary to check Endian-ness while creating a surface with SDL_CreateRGBSurfaceWithFormatFrom(), when it comes to saved pixel_arrays of 24 bit bitmaps from an existing SDL_Surface?

In other words, if I port the same saved binary file of a 24 bit bitmap (which was saved on a Little Endian machine) to a Big Endian computer, would something like the following snippet of code be enough to resolve any Endian-ness issues that might arise? Or would I necessarily have to dive programmatically into the loaded array and swap the bytes around manually?

Uint32 Endian_test = 1;
if (*(Uint8*)&Endian_test == 1)
{
	// Little Endian, Set the format to SDL_PIXELFORMAT_BGR24
}
else
{
	// Big Endian, Set the format to SDL_PIXELFORMAT_RGB24
}

// Use the resultant format with SDL_CreateRGBSurfaceWithFormatFrom()...

I leave the question(s) floating about out there. I think I am right, but that is not enough for me, as I have been wrong many, many times before. As I do not have any Big Endian computers to test this on, and am in the process of putting these functions into my own static lib (as they are useful to me), I would like to be positive I am going the right way about this.

Thank you to all, as always, much respect, and all the best.

12

SDL_PIXELFORMAT_BGR24 is endian-safe, it’s just three bytes per pixel in BGR order (thus the similarity to the SDL_PIXELFORMAT_RGBA32 name).

If you ever need to write code that depenends on the machine’s endianess, use

#if SDL_BYTEORDER == SDL_BIG_ENDIAN
  // your big endian code
#else
  // your little endian code
#endif
13

@Daniel_Gibson , thank you very much!

14

Yep. Caught my own mistake up there, in practice. Just for my own future reference, I add…

Considering that I tended to think of memcpy() as a bit of a string only function, because of its byte size sequence handling, it was quite innovative and imaginative (from my perspective, at least) of @sjr to use it for this purpose. But I warn myself…

When dealing with Uint8* into Uint32*, the last (size_t) argument should always be times 4 of the array dimension. This leaves me with some confusion as to how and why my program worked okay when I converted the image to a 24 bit BMP, with only 3 bytes per pixel obtained from the Surface structure->BytesPerPixel, and used that value multiplied by the pixel array base dimension. That should have made the memcpy() exactly 3/4 the required number of elements for the Uint32* array (as it was only copying 3 ea Uint8, contiguously, into the Uint32). In short, the image should have been a mess, if it even appeared at all when blitting was attempted.

But it was not. Anyway… one of those things I will just have to shrug at, and move on from knowing better, now. Seems the great God C forgave me that trespass in the foresight that I would consider it later. Hah!

(Unless there is something I am missing…)

15

It just depends on how you’re converting from RGB to RGBA. If you’re using SDL’s functions for color conversion etc it should be fine. Using memcpy() shouldn’t have worked, and I suspect something else is happening.

16

Actually not, @sjr. Quite the contrary…

It does seem to work. When I said that the suggestion to use it was innovative and imaginative, from my point of view, I meant it in the best possible way. However, always benefit of the doubt. Once liberated from the shackles of my own assumptions regarding memcpy(), it became a conduit to the better understanding of how SDL_Surface actually implements that pixel pointer. It was that specific that sent me around the block several times, looking for elusive bandicoots in the dark with merely a candle of an IQ.

Part 1: Initial experimentation
Perhaps I should include some code that presents the culmination (at last) of this adventure. Minimum functioning, with error checking suppressed. Here is “version 1.0”, commentary left in, as it is part of the description for this post:

(Incidentally, the images were exported from paint net as 32 and 24 bit RGB BMPs, respectively).

#include <SDL.h>

int main(int argc, char* argv[])
{
	SDL_Window* local_window = NULL;
	SDL_Surface* local_window_surface = NULL;
	SDL_Surface* remake_32_image = NULL;
	SDL_Surface* remake_24_image = NULL;
	SDL_Surface* original_32_image = SDL_LoadBMP_RW(SDL_RWFromFile("battle32.bmp", "rb"), 1);
	SDL_Surface* original_24_image = SDL_LoadBMP_RW(SDL_RWFromFile("battle24.bmp", "rb"), 1);

	SDL_Init(SDL_INIT_VIDEO);

	size_t image_size = (size_t)original_32_image->w * (size_t)original_32_image->h;

	// 
	Uint32* pixel_32_array = SDL_calloc(image_size, sizeof(Uint32));
	Uint32* pixel_24_array = SDL_calloc(image_size, sizeof(Uint32)); 
	// As there is no such thing as a Uint24, obviously :)
	// Which means even a 24 bit image has to be a Uint32 array, with one Uint8 ignored, per pixel.
	// Or one would think, but this does not happen in reality. See what happens when this gets to the
	// SDL_memcpy part...
	
	SDL_memcpy(pixel_32_array, original_32_image->pixels, (image_size * 4)); 
	// to be clear, 4 is bytes_pp as would be obtained from a 32 bit image from SDL_Surface

	SDL_memcpy(pixel_24_array, original_24_image->pixels, (image_size * 3)); //<- see, HERE is where it gets weird.
	// to be clear, 3 is bytes_pp as would be obtained from a 24 bit image from SDL_Surface
	// It is copying 3 bytes contiguously into the array, which leaves the last 1/4 of the Uint32 array "unoccupied".
	// Which brings me to this...
	//
	// Uint32* pixel_24_array = SDL_calloc((size_t)(image_size * 0.751), sizeof(Uint32));
	// 
	// If I use that line up where I create the dynamic array for the 24 bit data, the program still works perfectly.
	// Thereby, the array remains a Uint32, completely populated, creeping a Uint8 to the left inside each Uint32, 
	// per each iteration through the size specified in memcpy().
	// It seems strange, but I am having to accept this as normal, now, because it is the way it works.
	// I reiterate that I honestly thought a 24 bit image on an SDL_Surface was still a complete Uint32 per element with
	// one ignored Uint8 channel. Live and learn. Huh! 

	remake_32_image = SDL_CreateRGBSurfaceWithFormatFrom((Uint32*)pixel_32_array, original_32_image->w, original_32_image->h,
		32, original_32_image->pitch, original_32_image->format->format);
	// to be clear, 32 is bits_pp as would be obtained from a 32 bit image from SDL_Surface.
	// I know you can get this from SDL_Surface->format->BitsPerPixel, but I put it in manually here for clarity.
	
	remake_24_image = SDL_CreateRGBSurfaceWithFormatFrom((Uint32*)pixel_24_array, original_24_image->w, original_24_image->h,
		24, original_24_image->pitch, original_24_image->format->format);
	// to be clear, 24 is bits_pp as would be obtained from a 24 bit image from SDL_Surface


	local_window = SDL_CreateWindow("Test", SDL_WINDOWPOS_UNDEFINED, SDL_WINDOWPOS_UNDEFINED,
		remake_32_image->w, remake_32_image->h, SDL_WINDOW_SHOWN);

	local_window_surface = SDL_GetWindowSurface(local_window);

	SDL_BlitSurface(remake_32_image, NULL, local_window_surface, NULL);
	SDL_UpdateWindowSurface(local_window);
	SDL_Delay(3000);

	SDL_BlitSurface(remake_24_image, NULL, local_window_surface, NULL);
	SDL_UpdateWindowSurface(local_window);
	SDL_Delay(3000);

	SDL_FreeSurface(original_32_image);
	SDL_FreeSurface(remake_32_image);
	SDL_FreeSurface(original_24_image);
	SDL_FreeSurface(remake_24_image);

	SDL_FreeSurface(local_window_surface);
	SDL_DestroyWindow(local_window);
	SDL_free(pixel_32_array);
	SDL_free(pixel_24_array);
	SDL_Quit();

	return 0;
}

This works fine. Here are the two images that were blitted sequentially…

image
image718×312 65.2 KB

All good, works satisfactorily, but it was clearly not the whole story. I realize many an SDL pundit will be laughing at this, as they will already know better.

Part 2: Understanding the pixel array
As can be seen in the code, I make a comment about the surplus channel in each Uint32 pixel element of the 24 bit RGB BMP. It had become clear that SDL_Surface->pixels actually does employ contiguous initialization, and in fact does not leave a blank Uint8 (the unused Alpha channel) between every three color bytes, when referring to a 24 bit surface. Understanding that caused me to realize that a Uint32 array made to the size (ie; Width * Height, in pixels) of the image was 25% too big, wasting memory. A hack to test this idea was as such…

	Uint32* pixel_24_array = SDL_calloc((size_t)(image_size * 0.751), sizeof(Uint32));

That is to say, a Uint32 array 3/4 the size_t of the WxH. Testing this, proved the point. It still worked fine. But this leads to yet another question answered by a question.

Does the dynamic pixel array for data storage have to be Uint32?

So, I scrubbed the untidy idea of making a 3/4 size Uint32 pixel array, and instead made the array of Uint8 to the size of the bytes per pixel expected in the format. Like this…

	Uint8* pixel_32_array = SDL_calloc(image_size * 4, sizeof(Uint8));
	Uint8* pixel_24_array = SDL_calloc(image_size * 3, sizeof(Uint8));

After all, that is what memcpy() is expecting; it makes sense play its game?

	SDL_memcpy(pixel_32_array, original_32_image->pixels, (image_size * 4)); 
	SDL_memcpy(pixel_24_array, original_24_image->pixels, (image_size * 3));

Part 3: Final resolution
This left one final question…
Is it really necessary to cast the void* pixels in SDL_CreateRGBSurfaceWithFormatFrom(), now that we know it, too, seems to deal with the pixel array contiguously at byte level?

Therefore, I tried it:

remake_32_image = SDL_CreateRGBSurfaceWithFormatFrom(pixel_32_array, original_32_image->w, original_32_image->h,
		32, original_32_image->pitch, original_32_image->format->format);

Instead of…

remake_32_image = SDL_CreateRGBSurfaceWithFormatFrom((Uint32*)pixel_32_array, original_32_image->w, original_32_image->h,
		32, original_32_image->pitch, original_32_image->format->format);

It has absolutely no objections. To me at any rate, incorporating these modifications, the code looks cleaner and is (more importantly) tailor made to the requirements.

Conclusion
Well, yeah. I have played around with programming languages and APIs for some years, now, off and on. I never dabbled so deeply into bitmaps or “surfaces” as on this occasion. It has been fun. I only hope I am on the right track, with the above.

Kudos @sjr. You helped me think things out there, and thanks to everyone else for the helpful comments. Unless I have made any obvious glaring errors, I think we are done, here.

Your patience was sincerely appreciated.

17

First off, using memcpy() isn’t really “innovative”, it’s what it’s for and what everyone uses. It’s also almost guaranteed to be faster than doing a byte-by-byte copy yourself.

Secondly, it’s ultimately all just bytes in memory. Having the pointer type be a uint32 just makes the code the compiler generates treat it that way if you access the actual elements of the array. It’s good you realized you shouldn’t be casting things that aren’t uint32s to uint32 when passing them to SDL.

memcpy() isn’t “expecting” a uint8 or anything. The source and destination are both void pointers because it doesn’t care, and it doesn’t matter what type each pointer is. memcpy() certainly isn’t doing the copy one byte at a time, if that’s what you’re thinking.

You’re off to a good start, but I would definitely spend some more time reading up on pointers, types, and memory layout.

18

Thank you. They are all very good points.

Regarding my misconceptions about memcpy(), I am reluctantly obliged to point the finger at some documentation…

The C library function **void *memcpy(void dest, const void src, size_t n) copies n characters from memory area src to memory area dest .

…from Tutorialspoint. See, it says “characters”, which naturally caused me to assume it was primarily for string handling. That is; a sort of rougher version of strcpy(), for whatever purpose someone might want that. This is not a recent misconception; that site was my primary source of information as from a very long time ago, now.

Other documentation (C++ Reference) that I looked at, these last few days, clarified this misconception somewhat…

The underlying type of the objects pointed to by both the source and destination pointers are irrelevant for this function; The result is a binary copy of the data.

In any case, I do not think that this is the right place to get into all of that. It is far too fundamental. My bad, my problem.

Indeed. You see, that is why I was doing it the long way, originally. You cannot learn to ski just reading a book about it. The same principle applies here.

Pointers are darn fickle. I do struggle to understand why this…

pixel_address_ptr = (Uint8*)new_surface->pixels + coord_address_index;

…should be fine in one program (the one we have been looking at above, with 24 bit and 32 bit images), and yet not for another program that attempts to obtain pixel data off an 8 bit surface, when it is exactly the same thing. The compiler forces me to cast that, but does not like it in the 8 bit version. Even before compiling, the IDE is underlining that cast in red. So I am stumped, there. However, I am not asking for help with that. I need to figure that one out on my own.

But, yes. A lot to learn, and also to relearn. I am picking up programming again, as a hobby like before, after 12 years. I am not important, therefore, so my thanks for your time in adding the very constructive commentary, which I am going to be taking seriously in the immediate future, is profound and sincere.

All the best!

19

This comes from C calling its single-byte numeric data type “char,” a bizarre conflation of two unlike things that was wrongheaded even in the 1970s and has only gotten worse since the invention of Unicode.

2 Likes
20

Greetings. I am just revisiting this topic once more, as in the course of grinding out solutions I met with some areas of uncertainty.

That said, the utility is complete, and functioning to my entire satisfaction, able to process 32, 24 and 8 bit images, in png, jpg and bmp format. It has been a wholesome experience, as many long winded procedures have been whittled down. Perhaps it is still a little over programmed, but going the long way about it certainly had its benefits. Anyway…

The areas of uncertainty, for which I beg some reassurance, please…

I see a number of examples / tutorials in which SDL_Surface and SDL_Window are global. I am not fond of globals, but it seems (please correct me if I am wrong) there is something to it. In the various experiments along the way, I found that there is often some difficulty in properly freeing and NULLing the SDL_Surface pointer, if it resides in main(), from a function. In pseudocode:

main()
{
     SDL_Surface* Surf = CreateTheSurface;
     func_free_the_surface( Surf );

}

func_free_the_surface( Surf )
{
     SDL_FreeSurface(Surf);
     Surf = NULL;
}

Does not properly NULL the pointer, and subsequently can cause program crashes, if you attempt to use the NULL value, if it is still dangling, to test for existence of the Surf.

Very obviously, this is avoided if the Surface is global, and all is good.

The question: Is it advisable to maintain globally declared Windows and Surfaces if you are going to be using them for multiple surface creations?

The other question I have more or less answered myself, but again, like to be as certain as possible that I am on the right track.

In C; is there any issue with creating temporary structures in a function?

For example, I may have a custom structure, created “somewhere else” (ie; not in the function):

typedef struct image_header
{
    int w;
    int h;
} surface_header;

Then, in the function…

the_function()
{
    surface_header local_header;

    local_header.w = 10;
    // etcetera...
}

Will this generate any problems when the function terminates, such as memory leaks? I assume not, and I specifically make it with typedef so that it is treated as any other type would be in a local, temporary life variable in a function. But I am not sure. This has been different in at least one other language, and I wondered if I can “get away with it”, or better, if it is completely legal.

Other areas of slight concern continue to be endianness. During this experience, I have noted with interest one of the alternative ways of RGB Surface Creation, with SDL_CreateRGBSurfaceFrom(), utilizing SDL_PixelFormatEnumToMasks() and SDL_MasksToPixelFormatEnum(), which provides some flexibility to dealing with endianness practically. But, alas, the proof of the pudding is always in the eating, and I have not a functioning big endian machine to prove the concept, so it remains hypothetical (for me).

Just for the record, the program source is over at: GitHub , if anyone is interested in some mild humor.

All the same, thanks once more for all the help, here.

Take care.