Page 1 of 1

Crazy pointer arithmetic issues

Posted: Thu Feb 24, 2011 2:29 am
by mankrip
I'm trying to get TGAs to load in Makaqu, but the code I've ported from GLQuake refuses to work.

More specifically, this:

Code: Select all

typedef struct //_TargaHeader
{
	byte
		id_length
	,	colormap_type
	,	image_type
		;
	unsigned short
		colormap_index
	,	colormap_length
		;
	byte
		colormap_size
		;
	unsigned short
		x_origin
	,	y_origin
	,	width
	,	height
		;
	byte
		pixel_size
	,	attributes
		;
} TargaHeader;

void LoadTGA_as8bit (char *filename, byte **pic, int *width, int *height)
{
	TargaHeader		*filedata;					//pcx_t	*pcx;
	byte			*input, *output;			//*out,

	int				row, column;				//int		x, y;
	int				columns, rows, numPixels;	//int		dataByte, runLength;
	byte			*pixbuf;					//byte	*pix;
	loadedfile_t	*fileinfo;

	*pic = NULL;

	fileinfo = COM_LoadTempFile (filename);
	if (!fileinfo)
		return;

	// parse the file
	filedata = (TargaHeader *) fileinfo->data;

	if (filedata->image_type != 2 && filedata->image_type != 10)
		Sys_Error ("LoadTGA: Only type 2 and 10 targa RGB images supported\n");

	if (filedata->colormap_type != 0)
		Sys_Error ("LoadTGA: No colormaps supported\non \"%s\"\n", filename);

	if (filedata->pixel_size != 32)
	if (filedata->pixel_size != 24)
		Sys_Error ("LoadTGA: Only 32 or 24 bit images supported\non \"%s\",\n%i not supported", filename, (int) (filedata->pixel_size));

	columns	= LittleShort (filedata->width			);
	rows	= LittleShort (filedata->height			);
	numPixels = columns * rows;

	*pic = output = malloc (numPixels);
	input = (byte *) filedata + sizeof (TargaHeader) + filedata->id_length; // skip TARGA image comment

	if (filedata->image_type == 2) // Uncompressed, RGB images
	{
		for(row = rows - 1 ; row >= 0 ; row--)
		{
			pixbuf = output + row * columns;
			for (column = 0 ; column < columns ; column++)
			{
				byte
					red
				,	green
				,	blue
				,	alphabyte
					;
				switch (filedata->pixel_size)
				{
					case 24:
							blue		= input[ (rows - row) * columns + 0];
							green		= input[ (rows - row) * columns + 1];
							red			= input[ (rows - row) * columns + 2];
							*pixbuf++ = BestColor (red, green, blue, 0, 254);
							break;
					case 32:
							blue		= input[ (rows - row) * columns + 0];
							green		= input[ (rows - row) * columns + 1];
							red			= input[ (rows - row) * columns + 2];
							alphabyte	= input[ (rows - row) * columns + 3];
							*pixbuf++ = (alphabyte == 255) ? 255 : BestColor (red, green, blue, 0, 254);
							break;
				}
				input += (filedata->pixel_size / 8);
			}
		}
	}
}
I've just checked the TargaHeader struct and it's fine, it passes the image_type test with no problems, but when checking for the pixel_size it goes nuts, apparently reading two, five or eight bytes ahead, and returning 49 instead of 24.

These are the first 32 bytes of the TGA file:

Code: Select all

00 00 02 00   00 00 00 00   00 00 00 00   00 01 00 01
18 00 31 47   65 31 46 61   31 44 5B 33   4C 73 33 4E
This makes no sense for me, specially since the algorithm for loading PCX images is quite similar, and works just fine.

Posted: Thu Feb 24, 2011 2:32 am
by leileilol
Try ripping up Quake2's TGA loader rather than the questionably functional GLQuake one (what did glquake ever use tgas for lol)

Quake3's image loader is slicker and more streamlined by the way. I'd actually recommend using that so you also get JPEGs and BMPs in software (jpeg lib overhead, though...)

Posted: Thu Feb 24, 2011 2:38 am
by Sajt
The fields in the TGA file are packed tightly, not padded for data alignment (like C structs are). You'll have to load it byte by byte.

Posted: Thu Feb 24, 2011 4:54 am
by Spike

Code: Select all

#pragma pack(push,1)
struct my struct
{
char sillydef;
int mybadlylayedoutfield;
} foo;
#pragma pack(pop)
will work as expected. without the pragmas, the struct is larger than you might expect due to padding for alignment.
gcc+mvsc should both understand the pragma.

Posted: Mon Feb 28, 2011 7:42 pm
by mankrip
Thanks! It worked perfectly.

Posted: Tue Mar 01, 2011 2:02 am
by Sajt
If you're going for crossplatform stability you probably shouldn't do that. I think some processors don't allow access of unaligned memory at all... (they crash). Someone else might be able to tell you more about this. The safest thing is not to do that pragma thing, but to load the header byte by byte.

Posted: Tue Mar 01, 2011 2:21 am
by Spike
if you're using gcc on a packed structure, it'll do special accesses to ensure its all aligned and fine and dandy.
but yeah, its slower on such machines (slower on x86 too, just not so noticably).

Posted: Mon Apr 04, 2011 5:10 pm
by qbism
Thanks, this is great, now loading tga skies directly into 8-bit engine! Got type 10 (compressed) tga working also, need some clean-up and will post.

Posted: Tue Apr 05, 2011 3:33 am
by qbism
Modified from Makaqu 1.4alpha

Code: Select all

void LoadTGA_as8bit (char *filename, byte **pic, int *width, int *height)
{
	TargaHeader		*filedata;					//pcx_t	*pcx;
	byte			*input, *output;			//*out,

	int				row, column;				//int		x, y;
	int				columns, rows, numPixels;	//int		dataByte, runLength;
	byte			*pixbuf;					//byte	*pix;
	loadedfile_t	*fileinfo;

	*pic = NULL;

	fileinfo = COM_LoadTempFile (filename);
	if (!fileinfo)
		return;

	// parse the file
	filedata = (TargaHeader *) (fileinfo->data);

	if (filedata->image_type != 2 && filedata->image_type != 10)
		Sys_Error ("LoadTGA: Only type 2 and 10 targa RGB images supported\n");

	if (filedata->colormap_type != 0 || (filedata->pixel_size != 32 && filedata->pixel_size != 24))
		Sys_Error ("LoadTGA: Only 32 or 24 bit images supported (no colormaps)\non \"%s\"\n", filename);

   columns   = LittleShort (filedata->width);
   rows   = LittleShort (filedata->height);
	Con_Printf ("%i columns x %i rows\n", columns, rows);
	numPixels = columns * rows;
    *width = filedata->width;
   *height = filedata->height;

	*pic = output = Q_malloc (numPixels);
	input = (byte *) filedata + sizeof (TargaHeader) + filedata->id_length; // skip TARGA image comment

	if (filedata->image_type == 2) // Uncompressed, RGB images
	{
        byte red, green, blue,alphabyte;
        int k = 0;
		for(row = rows - 1 ; row >= 0 ; row--)
		{
			pixbuf = output + row * columns;
			for (column = 0 ; column < columns ; column++)
			{
				switch (filedata->pixel_size)
				{
					case 24:
							blue		= input[k++];
							green		= input[k++];
							red			= input[k++];
							*pixbuf++ = BestColor (red, green, blue, 0, 254);
							break;
					case 32:
							blue		= input[k++];
							green		= input[k++];
							red			= input[k++];
							alphabyte	= input[k++];
							*pixbuf++ =  BestColor (red, green, blue, 0, 254);
							break;
				}
			}
		}
	}

	else if (filedata->image_type == 10) // Runlength encoded RGB images
	{
        byte red, green, blue, alphabyte, packetHeader, packetSize, bestcol;
        int j, k=0;

		for (row = rows - 1 ; row >= 0 ; row--)
		{
			pixbuf = output + row * columns;
			for (column = 0 ; column < columns ; )
			{
				packetHeader = input[k++];
				packetSize = 1 + (packetHeader & 0x7f);
				if (packetHeader & 0x80) // run-length packet
				{
					switch (filedata->pixel_size)
					{
						case 24:
								blue		= input[k++];
								green		= input[k++];
								red			= input[k++];
								bestcol = BestColor (red, green, blue, 0, 254);
								break;
						case 32:
								blue		= input[k++];
								green		= input[k++];
								red			= input[k++];
								alphabyte	= input[k++];
								bestcol =  BestColor (red, green, blue, 0, 254);
								break;
					}
                    for(j = 0 ; j < packetSize ; j++)
					{
                        *pixbuf++ = bestcol;
						column++;
						if (column == columns) // run spans across rows
						{
							column = 0;
							if (row > 0)
								row--;
							else
								goto breakOut;
							pixbuf = output + row * columns;
						}
					}
				}
				else // non run-length packet
				{
					for(j = 0 ; j < packetSize ; j++)
					{
						switch (filedata->pixel_size)
						{
							case 24:
									blue		= input[k++];
									green		= input[k++];
									red			= input[k++];
									*pixbuf++   = BestColor (red, green, blue, 0, 254);
									break;
							case 32:
									blue		= input[k++];
									green		= input[k++];
									red			= input[k++];
									alphabyte	= input[k++];
									*pixbuf++   = BestColor (red, green, blue, 0, 254);
									break;
						}
						column++;
						if (column == columns) // pixel packet run spans across rows
						{
							column = 0;
							if (row > 0)
								row--;
							else
								goto breakOut;
							pixbuf = output + row * columns;
						}
					}
				}
			}
			breakOut:;
		}
	}

}


Posted: Wed Apr 06, 2011 12:11 am
by frag.machine
Where's the BestColor function ? I'm a bit curious about how you did it.

Posted: Wed Apr 06, 2011 12:19 am
by leileilol
You can steal BestColor from the qlumpy source. That's what I did.

Posted: Wed Apr 06, 2011 5:01 pm
by qbism
BestColor is in the revised load palette tute:
http://forums.inside3d.com/viewtopic.php?p=32285

Re: Crazy pointer arithmetic issues

Posted: Mon Dec 10, 2012 2:31 am
by mankrip
That #pragma isn't recognized when compiling the Dreamcast version of the engine, so I did this:

Code: Select all

void LoadTGA_as8bit (char *filename, byte **pic, int *width, int *height)
{
	TargaHeader		filedata;					//pcx_t	*pcx;
[...]
	loadedfile_t	*fileinfo;

[...]

	fileinfo = COM_LoadTempFile (filename);
	if (!fileinfo)
		return;

	// parse the file
	filedata.id_length			= fileinfo->data[0];
	filedata.colormap_type		= fileinfo->data[1];
	filedata.image_type			= fileinfo->data[2];
	filedata.colormap_index		= * (unsigned short *) (fileinfo->data + 3);
	filedata.colormap_length	= * (unsigned short *) (fileinfo->data + 5);
	filedata.colormap_size		= fileinfo->data[7];
	filedata.x_origin			= * (unsigned short *) (fileinfo->data + 8);
	filedata.y_origin			= * (unsigned short *) (fileinfo->data + 10);
	filedata.width				= * (unsigned short *) (fileinfo->data + 12);
	filedata.height				= * (unsigned short *) (fileinfo->data + 14);
	filedata.pixel_size			= fileinfo->data[16];
	filedata.attributes			= fileinfo->data[17];

[...]
}
This have worked for me (on Windows at least, I'm going to test it on the Dreamcast in some minutes), but I guess it may not work on machines using a different endianess. How do I take care of the endianess?

Re: Crazy pointer arithmetic issues

Posted: Sat Dec 22, 2012 5:25 am
by mankrip
mankrip wrote:How do I take care of the endianess?
Some googling gave me the answer.