Problem with initialising dynamically allocated memory via pointer

[acerzw]

Hi,

I am using a dynamically allocated array to store the addresses of screen lines for a graphics application that uses the Graphics SDL Library. I am using the following code to create and initialise the array:

Uint8 *ScreenAddress;
Uint16 BytesPerLine;
Uint8 *LineAddress;

LineAddress=new Uint8[Height];

for (int index=0; index<Height; index++) LineAddress[index]=ScreenAddress+(index*BytesPerLine);

The types of the variables are those required by SDL, however I get the following error:

error: invalid conversion from ‘Uint8*’ to ‘unsigned char’

and the compiler indicates that the error is in the 'for' loop line. I have tried many different ways of casting the expression including putting (Uint8 *) after the '=' but it does not work. What am I doing wrong?

Many Thanks

Acerzw

[jsmith]

Casting isn't going to help you here.

If LineAddress is to be an array that stores pointers-to-Uint8s then
LineAddress needs to be declared as

Uint8* *LineAddress;

You are currently defining it to be a pointer-to-Uint8 or an array of Uint8.

[acerzw]

Thanks Jsmith,

However when I do that I get:

error: cannot convert ‘Uint8*’ to ‘Uint8**’ in assignment

indicating this line is now an error:

LineAddress=new Uint8[Height];

[jsmith]

You don't want to allocate an array of Uint8, you want an array of Uint8 pointers.

new Uint8*[Height];

[acerzw]

Thanks again,

Well it compiles now but i get a segmentation fault when the program runs, so it must be changing the way the pointer works. The code that uses the array is probably contributing to the segmentation fault and is as follows:

Uint8 *PixelAddress;

void SetPixelAddress(int x,int y) {PixelAddress=(Uint8 *)LineAddress[y]+x*BytesPerPixel;}

// and further on i use the pointer to set a pixel
*PixelAddress=DrawColour;

The curious thing is that if I use the code, as I have it originally in my first post, and change the array so it 's size is set when it is declared and get rid of the dynamic allocation it works... so not sure what is going on... unfortunately I need the dynamic allocation, any more ideas?

[jsmith]

Remove the unneeded (or else it's wrong) cast and see what the compiler says

void SetPixelAddress( int x, int y ) {
    LineAddress[ y ][ x * BytesPerPixel ] = DrawColor;
}

Incidentally, why do you even have the LineAddress array? Are not the raster lines
contiguous in memory?

[acerzw]

I have removed the cast as suggested, I still get the segmentation error. I am storing the LineAddress to avoid multiplying it each time. I have not combined the address calculation and pixel setting functions as you have suggested.

What I don't understand is why this works without the dynamic allocation:

Uint8 *ScreenAddress;
Uint16 BytesPerLine;
Uint8 *LineAddress[800];
Uint8 *PixelAddress;

for (int index=0; index<Height; index++) LineAddress[index]=ScreenAddress+(index*BytesPerLine);

// calculating the address
void SetPixelAddress(int x,int y) {PixelAddress=(Uint8 *)LineAddress[y]+x*BytesPerPixel;}

// and further on i use the pointer to set a pixel
*PixelAddress=DrawColour;

but stops working as soon as I do this:

Uint8 *LineAddress;

LineAddress = new Uint8[Height];

using your suggested changes:

Uint8* *LineAddress;

LineAddress = new Uint8*[Height];

void SetPixelAddress(int x,int y) {PixelAddress=LineAddress[y]+x*BytesPerPixel;}

I get the segmentation fault, so not sure why the non-dynamic code works, but this doesn't?

[jsmith]

I think your "optimization" is actually slower than if you didn't optimize. You've replaced a
multiply with an add and a memory access. I think it would be better if you avoid the problem
altogether and remove your LineAddress array.

But, if you really want to keep it, and it crashes the first time into SetPixelAddress, then I'd
suggest printing out the values of x, y, LineAddress[y], and the calculated address to make
sure they all look right.

[acerzw]

Thanks Jsmith

Your right, I am doing optimisations from the old days when multiplies were very costly, with faster processors now I need to update my optimisation practises! Removed it an it works fine. Thanks.

[Disch]

from the old days when multiplies were very costly

Welcome to 1995... er wait... 2010.

hehe

[acerzw]

Sorry, what's that in Octal? Lol

[acerzw]

For the record, the 'optimised' version does seem to work faster, the other version stutters noticeably, not sure why. The final version as follows worked:

Uint8 *ScreenAddress;
Uint8 *PixelAddress;
Uint16 BytesPerLine;
int BytesPerPixel;

Uint8** LineAddress;

LineAddress = new Uint8*[Height];

for(int index=0;index<Height;index++) LineAddress[index]=ScreenAddress+(index*BytesPerLine);

void SetPixelAddress(int x,int y) {PixelAddress=LineAddress[y]+(x*BytesPerPixel);}

[helios]

This is for pixel-level access, isn't it? The way I do this is

uchar *pix=/*...*/;
for (ulong y=0;y<h;y++){
	uchar *pix0=pix;
	for (ulong x=0;x<w;x++){
		pix[Roffset]=/*...*/;
		pix[Goffset]=/*...*/;
		pix[Boffset]=/*...*/;
		pix+=advance; //advance: size of a pixel in bytes
	}
	pix=pix0+pitch; //pitch: size of a scan line in bytes (padding included)
}

I always work with 24- and 32-bit surfaces in memory.