Iterating through SDL_Surface pixels

SDL Development
1

Is using surface->w and surface->h in nested for loops a sensible way to access the pixel data in an image?

Code:

for(y = 0; y < source->h; y++){
	for(x = 0; x < source->w ;x++){

		SDL_LockSurface(source);
		pixel = getPixel(source, x, y);
		SDL_UnlockSurface(source);
	
		/* Get Red component */
		temp = pixel & fmt->Rmask; /* Isolate red component */
		temp = temp >> fmt->Rshift; /* Shift it down to 8-bit */
		temp = temp << fmt->Rloss; /* Expand to a full 8-bit number */
		red = (Uint8)temp;
		/* Get Green component */
		temp = pixel & fmt->Gmask; /* Isolate green component */
		temp = temp >> fmt->Gshift; /* Shift it down to 8-bit */
		temp = temp << fmt->Gloss; /* Expand to a full 8-bit number */
		green = (Uint8)temp;
		/* Get Blue component */
		temp = pixel & fmt->Bmask; /* Isolate blue component */
		temp = temp >> fmt->Bshift; /* Shift it down to 8-bit */
		temp = temp << fmt->Bloss; /* Expand to a full 8-bit number */	
		blue = (Uint8)temp;
		
		redData[(y * source->w) + x] = red;
		greenData[(y * source->w) + x] = green;
		blueData[(y * source->w) + x] = blue;
	}
}

Also, I am writing the pixels like so:

Code:

for(x = 0; x < source->w; x++){
	for(y = 0; y < source->h ;y++){
		for(index = 0; index < 256; index++){
			if(redData[(y * source->w) + x] == index){
				redData[(y * source->w) + x] = redCdf[index];
			}
			if(greenData[(y * source->w) + x] == index){
				greenData[(y * source->w) + x] = greenCdf[index];
			}
			if(blueData[(y * source->w) + x] == index){
				blueData[(y * source->w) + x] = blueCdf[index];
			}
		}
		/* Bit shifting the colour data back into pixel format */
		pixel = (redData[(y * width) + x] << 16) | (greenData[(y * width) + x] << 8) | (blueData[(y * width) + x]);
		putPixel(filteredImage, x, y, pixel);
	}
}

The reason I am asking is because I am drawing vertical black lines through my image …which is otherwise looking as it should.[/code]

2

I think you should lock the surface before the whole loop, and unlock it after the whole loop.

If you’re worrying about surface ->w changed in the loop, you can copy the value to another variable before the loop. I don’t think it would changed, btw.

3

honeyspider wrote:

Code:

for(x = 0; x < source->w; x++){
for(y = 0; y < source->h ;y++){
for(index = 0; index < 256; index++){
if(redData[(y * source->w) + x] == index){
redData[(y * source->w) + x] = redCdf[index];
}
if(greenData[(y * source->w) + x] == index){
greenData[(y * source->w) + x] = greenCdf[index];
}
if(blueData[(y * source->w) + x] == index){
blueData[(y * source->w) + x] = blueCdf[index];
}
}
/* Bit shifting the colour data back into pixel format */
pixel = (redData[(y * width) + x] << 16) | (greenData[(y * width) + x] << 8) | (blueData[(y * width) + x]);
putPixel(filteredImage, x, y, pixel);
}
}

The reason I am asking is because I am drawing vertical black lines through my image …which is otherwise looking as it should.[/code]

Performance-wise you should be changing this loop so that it is the same as the first ie. For Y, then for X within it.
For the reason why this is, read the following stackoverflow link:


(C# but the logic is the same)

4

mr_tawan wrote:

I think you should lock the surface before the whole loop, and unlock it after the whole loop.

If you’re worrying about surface ->w changed in the loop, you can copy the value to another variable before the loop. I don’t think it would changed, btw.

I was worried the values might have changed somehow, alas that is not the case. I tried again with variables as you suggested and I still have red lines.

Perhaps I am using the bit shifting incorrectly? Even when I try and rebuild an image without modifying the pixel data, I get a grey scale image with black vertical lines.

5

The old wiki had some code for putting and getting pixels that handles
supported pixel formats. You should check yours against that:
http://sdl.beuc.net/sdl.wiki/Pixel_Access

Jonny DOn Sun, May 11, 2014 at 7:50 AM, honeyspider <svladd_cjelik at yahoo.com.au>wrote:

mr_tawan wrote:

I think you should lock the surface before the whole loop, and unlock it
after the whole loop.

If you’re worrying about surface ->w changed in the loop, you can copy the
value to another variable before the loop. I don’t think it would changed,
btw.

I was worried the values might have changed somehow, alas that is not the
case. I tried again with variables as you suggested and I still have red
lines.

Perhaps I am using the bit shifting incorrectly? Even when I try and
rebuild an image without modifying the pixel data, I get a grey scale image
with black vertical lines.

SDL mailing list
SDL at lists.libsdl.org
http://lists.libsdl.org/listinfo.cgi/sdl-libsdl.org