Bitmap loader

[Tc++H]

Ich schriebe gerade eine Bmp Loader für ein Spiel.

Quellcode:

Headerdatei:

//////////////////////////////////////////////////////////////////////////
#include <cstdio>
#include <cassert>

//////////////////////////////////////////////////////////////////////////

//////////////////////////////////////////////////////////////////////////
//////////////////////////////////////////////////////////////////////////
//
// Speicherleichen vermeiden, TIFE KOPIEN!!!!
// eventuell Imgae class erstellen
//////////////////////////////////////////////////////////////////////////
//////////////////////////////////////////////////////////////////////////

#pragma  pack(1)
struct ZpBMPFileHeader
{
	char			m_cType[2];		// 2 bytes
	unsigned int	m_wFileSize;	// 4 bytes
	short int		m_nReserved1;	// 2 bytes
	short int		m_nReserved2;	// 2 bytes
	unsigned int	m_wOffsetBits;	// 4 bytes;
};
#pragma pack()

#pragma  pack(1)
struct ZpBMPInfoHeader
{
	unsigned int	m_wSizeOfHeader;	// 4 bytes - 40
	unsigned int	m_wWidth;			// 4 bytes
	unsigned int	m_wHeight;			// 4 bytes
	short int		m_nPlanes;			// 2 bytes
	short int		m_nBitCount;		// 2 bytes
	unsigned int	m_wCompression;		// 4 bytes
	unsigned int	m_wSizeOfImage;		// 4 bytes
	unsigned int	m_wXPelsPerMeter;	// 4 bytes
	unsigned int	m_wYPelsPerMeter;	// 4 bytes
	unsigned int	m_wColorUsed;		// 4 bytes;
	unsigned int	m_wColorImportant;	// 4 bytes
};
#pragma pack()

struct ZpImage
{
	unsigned int	m_wWidth;
	unsigned int	m_wHeight;
	unsigned int*	m_pARGB;
};

class ZpBmpLoader
{
private:
	ZpBMPFileHeader	m_BmpFileHeader;
	ZpBMPInfoHeader	m_BmpInfoHeader;
	ZpImage			m_Image;

public:
	ZpBmpLoader(void);
	ZpBmpLoader(const ZpBmpLoader	*Bitmap);
	ZpBmpLoader(const char			*pFileName);

	~ZpBmpLoader(void);

	bool LoadBmpFile(const char *pFileName);
	void Release(void);

	void print(void)
	{
		printf("%x\n", m_Image.m_pARGB);
		printf("%d\n", m_Image.m_wWidth);
	}

	ZpBmpLoader &operator=(const ZpBmpLoader	*Bitmap);
};

Quellcodedatei:

//////////////////////////////////////////////////////////////////////////
#include "ZpBmpLoader.h"

//////////////////////////////////////////////////////////////////////////
ZpBmpLoader::ZpBmpLoader(void)
{
}

ZpBmpLoader::ZpBmpLoader(const ZpBmpLoader *Bitmap)
{
	m_Image.m_wWidth	= Bitmap->m_Image.m_wWidth;
	m_Image.m_wHeight	= Bitmap->m_Image.m_wHeight;
	m_Image.m_pARGB		= new(unsigned int[m_Image.m_wWidth * m_Image.m_wHeight]);

	//*m_Image.m_pARGB = *Bitmap->m_Image.m_pARGB;
	// oder

	for(unsigned int h = 0; h < m_Image.m_wHeight; h ++)
	{
		for(unsigned int w = 0; w < m_Image.m_wWidth; w ++)
		{
			m_Image.m_pARGB[w + h * m_Image.m_wWidth] = Bitmap->m_Image.m_pARGB[w + h * Bitmap->m_Image.m_wWidth];
		}
	}

	m_BmpFileHeader = Bitmap->m_BmpFileHeader;
	m_BmpInfoHeader = Bitmap->m_BmpInfoHeader;
}

ZpBmpLoader::ZpBmpLoader(const char *pFileName)
{
	LoadBmpFile(pFileName);
}

ZpBmpLoader::~ZpBmpLoader(void)
{
	Release();
}

bool ZpBmpLoader::LoadBmpFile(const char *pFileName)
{
	FILE* pFile = fopen(pFileName, "rb");

	if(!pFile)
		return(false);

	fread(&m_BmpFileHeader, 1, sizeof(ZpBMPFileHeader), pFile);

	if((m_BmpFileHeader.m_cType[0] != 'B') && (m_BmpFileHeader.m_cType[1] != 'M'))
		return(false);

	fread(&m_BmpInfoHeader, 1, sizeof(ZpBMPInfoHeader), pFile);

	m_Image.m_wWidth  = m_BmpInfoHeader.m_wWidth;
	m_Image.m_wHeight = m_BmpInfoHeader.m_wHeight;

	fseek(pFile, m_BmpFileHeader.m_wOffsetBits, SEEK_SET);

	m_Image.m_pARGB = new(unsigned int[m_Image.m_wWidth * m_Image.m_wHeight]);
	assert(!(m_Image.m_pARGB == NULL) && "Error allocating memory for image");

	int iNumPaddedBytes = (m_Image.m_wWidth * 3) % 4;

	unsigned char r, g, b;
	unsigned char skip[4];

	for(unsigned int h = 0; h < m_Image.m_wHeight; h ++)
	{
		for(unsigned int w = 0; w < m_Image.m_wWidth; w ++)
		{
			fread(&b, 1, 1, pFile);
			fread(&g, 1, 1, pFile);
			fread(&r, 1, 1, pFile);

			m_Image.m_pARGB[w + h * m_Image.m_wWidth] = (r << 16 | g << 8 | b);
		}

		if(iNumPaddedBytes != 0)
		{
			fread(skip, 1, 4 - iNumPaddedBytes, pFile);
		}
	}

	fclose(pFile);
	return(true);
}

void ZpBmpLoader::Release(void)
{
	if(m_Image.m_pARGB != NULL)
		delete[](m_Image.m_pARGB);
	printf("release");
}

ZpBmpLoader &ZpBmpLoader::operator=(const ZpBmpLoader *Bitmap)
{
	if(this == Bitmap)
		return(*this);
	if(m_Image.m_pARGB)
		Release();
	m_Image.m_wWidth	= Bitmap->m_Image.m_wWidth;
	m_Image.m_wHeight	= Bitmap->m_Image.m_wHeight;
	m_Image.m_pARGB		= new(unsigned int[m_Image.m_wWidth * m_Image.m_wHeight]);

	//*m_Image.m_pARGB = *Bitmap->m_Image.m_pARGB;
	// oder

	for(unsigned int h = 0; h < m_Image.m_wHeight; h ++)
	{
		for(unsigned int w = 0; w < m_Image.m_wWidth; w ++)
		{
			m_Image.m_pARGB[w + h * m_Image.m_wWidth] = Bitmap->m_Image.m_pARGB[w + h * Bitmap->m_Image.m_wWidth];
		}
	}

	m_BmpFileHeader = Bitmap->m_BmpFileHeader;
	m_BmpInfoHeader = Bitmap->m_BmpInfoHeader;

	return(*this);
}

Der Bmp loader erzeugt leider nur Flache kopien, wenn ich ihm eine andere Instanz
übergebe, Warum???

Mein zweites Problem ist die Speicherverwaltung: Wenn ich eine Image löschen will, die gelöscht ist, gibt es einen Fehler und es kommt zum Absturz. Warum reichen folgende Zeilen zur Abfrage nicht aus?

void ZpBmpLoader::Release(void)
{
	if(m_Image.m_pARGB != NULL)
		delete[](m_Image.m_pARGB);
	printf("release");
}

[elise]

wo ist der kopierkonstruktor?

[Tc++H]

Also der ZpBmpLoader class nen

ZpBmpLoader(const ZpBmpLoader *bmp)

verpassen.

Wie soll der nun genau aussehen? Ähnlich wie der '=' operator?!

[elise]

referenz auf ein objekt, kein zeiger.

[Tc++H]

Soll ich beim '=' operator auch lieber Referenzen verwenden?

[elise]

class Foo
{
public:
Foo(const Foo&);
Foo& operator=(const Foo&);
};

erscheint mir sinnvoll.
referenz bietet sich der einfachheit der übergabe des objektes meiner meinung nach an. (sonst müsstest du ja immer die methode o.operator=( &deinobjekt) aufrufen)

[Tc++H]

Stimmt, dann muss man auch nicht mehr so viele Fehlerchecks in die Methoden implentieren.

Hast du noch ne Antwort auf das Release Problem?

[elise]

ich frage mich grad, wozu du den kopierkonstruktor eigentlich brauchst.
ich seh nur instanzen in deiner klasse, keine verweise, und kein new im konstruktor..

[Tc++H]

Warum, es wird doch m_Imgage.ARGB erneuert.
Soll ich die ganze Klasse erneuern???

[elise]

ein kopierkonstruktor (und ein selbsthergestellter zuweisungsoperator) und ein definierter destruktor sind dann nötig, wenn die klasse wirklich ein tiefe kopie braucht. das ist dann der fall, wenn das objekt über zeiger im konstruktor mit new ein objekt auf dem heap alloziert. da ein objekt für sich sorgen soll, muss es selber auch üblicherweise im destruktor dieses alloziierte stück wieder freigeben.

wenn du eine kopie erstellst, besteht bei der flachen kopie in diesem von mir beschriebenen obigen fall dann das problem, dass der zeiger auf das im konstruktor allozierte objekt kopiert wird, und damit zwei zeiger auf dasselbe objekt bestehen.
beim aufruf eines destruktors wird dann das objekt gelöscht, der zweite destruktoraufruf der kopie (zum beispiel) läßt das programm abstürzen, da nichts mehr auf dem heap reserviert ist.
es ist wie siamesische zwillinge mit einem herzen.

du musst in diesem fall, also wenn eine klasse über einen zeiger extra nochmal alloziert, für eine tiefe kopie sorgen.

wenn du in einer methode (nicht konstruktor) allozierst, und den zeiger nicht irgendwie weitergibst, erzeugst du speicherlöcher.

http://fara.cs.uni-potsdam.de/~kaufmann/?page=GenCppFaqs&faq=BigThree#Answ

[Tc++H]

Ja, dass ist schon klar.

[elise]

sorry, hab dein programm noch nicht ganz durchstiegen.
der vortrag war voreilig

[Tc++H]

Sind auch ne ganze menge Zeilen, auf jeden Fall will ich ne Image class erstellen (statt Membervariable), sodass ich auch TGAs, Gifs und Jpgs laden kann.

[elise]

mach erstmal das

ZpBmpLoader::ZpBmpLoader(void)
{
	m_Image.m_pARGB=NULL;
}

dein m_Image ist einfach nicht angesprungen.

aber mir ist das alles zu gefährlich, soll man ein könner draufschaun

[Tc++H]

Wozu soll das gut sein???

[elise]

alleine bei falscheingabe einer datei stürzt dein programm sonst schon ab, bei aufruf von release..

bool ZpBmpLoader::LoadBmpFile(const char *pFileName)
{
    FILE* pFile = fopen(pFileName, "rb");

    if(!pFile)
        return(false);

du fragst ja in release ab auf

if(m_Image.m_pARGB != NULL)

aber wo wird der zeiger m_pARGB genullt? überseh ich das?

[Tc++H]

Stimmt, soloche Sachen übersieht man schnell!!!

[elise]

und ebenso hier..

if((m_BmpFileHeader.m_cType[0] != 'B') && (m_BmpFileHeader.m_cType[1] != 'M'))
        return(false);