GP2X Rgb888 To Sdl_surface?


No, that's the kind of profiling I meant. For large scale operations it can be good to just time it, though, to get a time in milliseconds - percentages are good for finding hotspots, but absolute times are best when you're actually optimising a routine. (Percentages tell you what to optimise, times tell you how much you did optimise it.) gprof can tell you cumulative times as well, or you can just use gettimeofday and do the maths!

Your loops are already ordered well for access to your own array, but it doesn't match the ordering of the actual video memory. Swapping the order of X and Y brings your array in line with video memory.

The aliasing would look something like this:
Code:
void surf_array_o(SDL_Surface *surface, uint8_t ***image, uint16_t width, uint16_t height)	//puts pixels from an array to a surface (optimized)
{
	uint16_t ix, ix3, iy, pitch_value=surface->pitch;
	uint8_t *p;
	void *pixels_start = surface->pixels;

	for (iy=0; iy<height; iy++)
	{
		uint8_t **image_iy = image[iy];
		for (ix=0; ix<width; ix++)
		{
			   uint8_t *image_iy_ix = image_iy[ix];
			   ix3 = ix*3;
			   p = (uint8_t *) pixels_start + (iy * pitch_value) + ix3;		//p == address to the pixel
			   p[0] = image_iy_ix[0];
			   p[1] = image_iy_ix[1];
			   p[2] = image_iy_ix[2];
		}
	}
}

Regarding the change in storage format, you could still create your top level pointer structure in order to access the data as image[iy][ix], just pointing the result at the right bit of the single block you're allocating, but any access through that structure is going to be quite inefficient. It's probably at least as efficient to calculate the offset into the single array as it is to look up the offsets in several different memory locations - when you write image[iy][ix], the compiler has to load image (unless it's already loaded), then load image[iy], then load image[iy][ix], just to get the pointer to the pixel data. Each of these loads is dependent on the result of the previous load, so they end up taking effectively two cycles each. On the other hand, if your data alignments are easy to calculate (powers of two are great for this architecture, and padding your pixel data to 4 bytes will also help) the multiplies are really cheap to calculate, and I don't think there's a penalty for using the results straight away, unlike with loads from memory. Plus there's nothing to alias - bonus!

Another approach to avoiding the multiply is to still use aliases, but increment to advance to the next pixel instead of multiplying from scratch each time. You can also advance a pointer instead of an index, but if it makes sense to do so then the compiler is likely to do this one for you. If you get at least a passing familiarity with the assembler of the system you're targetting then you can inspect the code the compiler generates and check for this kind of thing - it can also highlight cases where the compiler reloads from memory unnecessarily, where you should use an alias.
 
gfoot posted on Sep 3 2006 at 09:14 PM said:
No, that's the kind of profiling I meant. For large scale operations it can be good to just time it, though, to get a time in milliseconds - percentages are good for finding hotspots, but absolute times are best when you're actually optimising a routine. (Percentages tell you what to optimise, times tell you how much you did optimise it.) gprof can tell you cumulative times as well, or you can just use gettimeofday and do the maths!

Your loops are already ordered well for access to your own array, but it doesn't match the ordering of the actual video memory. Swapping the order of X and Y brings your array in line with video memory.

The aliasing would look something like this:
Code:
void surf_array_o(SDL_Surface *surface, uint8_t ***image, uint16_t width, uint16_t height)	//puts pixels from an array to a surface (optimized)
{
	uint16_t ix, ix3, iy, pitch_value=surface->pitch;
	uint8_t *p;
	void *pixels_start = surface->pixels;

	for (iy=0; iy<height; iy++)
	{
		uint8_t **image_iy = image[iy];
		for (ix=0; ix<width; ix++)
		{
			   uint8_t *image_iy_ix = image_iy[ix];
			   ix3 = ix*3;
			   p = (uint8_t *) pixels_start + (iy * pitch_value) + ix3;		//p == address to the pixel
			   p[0] = image_iy_ix[0];
			   p[1] = image_iy_ix[1];
			   p[2] = image_iy_ix[2];
		}
	}
}

Regarding the change in storage format, you could still create your top level pointer structure in order to access the data as image[iy][ix], just pointing the result at the right bit of the single block you're allocating, but any access through that structure is going to be quite inefficient. It's probably at least as efficient to calculate the offset into the single array as it is to look up the offsets in several different memory locations - when you write image[iy][ix], the compiler has to load image (unless it's already loaded), then load image[iy], then load image[iy][ix], just to get the pointer to the pixel data. Each of these loads is dependent on the result of the previous load, so they end up taking effectively two cycles each. On the other hand, if your data alignments are easy to calculate (powers of two are great for this architecture, and padding your pixel data to 4 bytes will also help) the multiplies are really cheap to calculate, and I don't think there's a penalty for using the results straight away, unlike with loads from memory. Plus there's nothing to alias - bonus!

Another approach to avoiding the multiply is to still use aliases, but increment to advance to the next pixel instead of multiplying from scratch each time. You can also advance a pointer instead of an index, but if it makes sense to do so then the compiler is likely to do this one for you. If you get at least a passing familiarity with the assembler of the system you're targetting then you can inspect the code the compiler generates and check for this kind of thing - it can also highlight cases where the compiler reloads from memory unnecessarily, where you should use an alias.

You think I should pad my pixel values to 4 bytes so I do <<2 instead of *3? in spite of the extra memory usage?

anyways, incrementing the pointer progressively sounds like a great idea, multiplication or shifting never involved, then it's (i think) all about calling image[++index] everytime to systematically increment it, right? (i've never used the prefix increment operator before, so i'm not sure)
 
Last edited by a moderator:
You'd want to post-increment, so it increments after reading. It might be clearer to just add 1 to the second lookup, 2 to the third, then add 3 to the pointer at the end.

Padding the pixels would be a memory tradeoff, yes.. but with padded data there's more scope for the CPU loading all three components in one load operation. It might not affect the gcc-generated code, though - it depends how clever gcc is about this - perhaps it's only beneficial if you're willing to use assembly.

Actually, I just noticed that this function now just copies the data from your image array to a 24-bit SDL surface. Maybe you could just write your data directly to the surface in the first place. I thought you were originally writing it to a 16-bit surface... it's the copy to 16-bit that needs this kind of function.
 
gfoot posted on Sep 3 2006 at 10:49 PM said:
You'd want to post-increment, so it increments after reading. It might be clearer to just add 1 to the second lookup, 2 to the third, then add 3 to the pointer at the end.

Padding the pixels would be a memory tradeoff, yes.. but with padded data there's more scope for the CPU loading all three components in one load operation. It might not affect the gcc-generated code, though - it depends how clever gcc is about this - perhaps it's only beneficial if you're willing to use assembly.

Actually, I just noticed that this function now just copies the data from your image array to a 24-bit SDL surface. Maybe you could just write your data directly to the surface in the first place. I thought you were originally writing it to a 16-bit surface... it's the copy to 16-bit that needs this kind of function.

There's a problem, my screen surface is defined to be 16-bit, and I'm writing 24-bit data to it, so I can't use that surface directly.
 
Last edited by a moderator:
I just realized something, if I'm making my own blitter, why not write directly to the frame buffer instead of bothering with SDL? Wouldn't it be faster?
 
Back
Top