Texturen laden so i.O.?

[Ganon]

Hi.

Ich habe mir eine Klasse erstellt, mit der ich Texturen für OpenGL laden kann. Dies sollte Plattformunabhängig geschehen.

Jetzt wollte ich euch mal fragen ob man das so stehen lassen kann, oder ob man das so nicht machen sollte? Bisher kann man nur Bitmaps laden. Und man kann pro Objekt 3 Texturen laden.

Bisher funktioniert es. Aber ich hätte gerne Gewissheit. ;)

Würdet ihr euch das mal angucken?

Danke.


// Klasse für die Texturen
class KTexturen
{
private:
GLuint Textur[3];

public:
KTexturen(); // Konstruktor
~KTexturen(); // Destruktor
bool LadeTextur(char *Dateiname, int Tex);
GLuint HoleTextur(int Tex);
};

KTexturen::KTexturen()
{}

KTexturen::~KTexturen()
{}

bool KTexturen::LadeTextur(char *Dateiname, int Tex)
{
FILE *datei;
unsigned long groesse; // Dateigröße in Byte
unsigned long i; // Zählervariable
unsigned short int flaeche; // Anzahl der Flächen (muss 1 sein)
unsigned short int bpp; // Anzahl der Bit pro Pixel (muss 24 sein)
char temp; // temporärer Farbspeicher bgr->rgb Wandlung
unsigned long groesseX; // Bild-Auflösung X
unsigned long groesseY; // Bild-Auflösung Y
char *daten; // das Bild

// prüfen ob die Datei vorhanden ist
if ((datei = fopen(Dateiname, "rb")) == NULL)
{
cout << "Datei nicht gefunden: " << Dateiname << endl;
return false;
};

// Bitmap-Header nach der Bildauflösung durchsuchen
fseek(datei, 18, SEEK_CUR);

// Wenn Prozessor kein x86 ist, dann Bild-Header umstellen auf Big-Endian
#ifdef _M_IX86
#else
char *umkehr, swap;
#endif

// Lesen der Breite
if ((i = fread(&groesseX, 4, 1, datei)) != 1)
{
cout << "Fehler beim Lesen der Breite von " << Dateiname << endl;
return false;
};

#ifdef _M_IX86
#else
umkehr = (char *) &(groesseX);
swap = umkehr[0]; umkehr[0] = umkehr[3]; umkehr[3] = swap;
swap = umkehr[1]; umkehr[1] = umkehr[2]; umkehr[2] = swap;
#endif

cout << "Breite von " << Dateiname << ": " << groesseX << endl;

// Lesen der Höhe
if ((i = fread(&groesseY, 4, 1, datei)) != 1)
{
cout << "Fehler beim Lesen der Höhe von : " << Dateiname << endl;
return false;
};

#ifdef _M_IX86
#else
umkehr = (char *) &(groesseY);
swap = umkehr[0]; umkehr[0] = umkehr[3]; umkehr[3] = swap;
swap = umkehr[1]; umkehr[1] = umkehr[2]; umkehr[2] = swap;
#endif

cout << "Hoehe von " << Dateiname << ": " << groesseY << endl;

// Berrechnen der Dateigröße (24bit mal 3 byte pro Pixel)
groesse = groesseX * groesseY * 3;

// Lesen der Flächen
if ((fread(&flaeche, 2, 1, datei)) != 1)
{
cout << "Fehler beim Lesen der Fläche von " << Dateiname << endl;
return false;
};

#ifdef _M_IX86
#else
umkehr = (char *) &(flaeche);
swap = umkehr[0]; umkehr[0] = umkehr[1]; umkehr[1] = swap;
#endif

if (flaeche != 1)
{
cout << "Flächen von " << Dateiname << " ist nicht 1, sondern " << flaeche << endl;
return false;
};

// Lesen der Farbtiefe
if ((i = fread(&bpp, 2, 1, datei)) != 1)
{
cout << "Fehler beim Lesen der Farbtiefe von " << Dateiname << endl;
return false;
};

#ifdef _M_IX86
#else
umkehr = (char *) &(bpp);
swap = umkehr[0]; umkehr[0] = umkehr[1]; umkehr[1] = swap;
#endif

if (bpp != 24)
{
cout << "Farbtiefe von " << Dateiname << " ist nicht 24bit, sondern " << bpp << endl;
return false;
};

// Lesen des restlichen Bitmap-Headers
fseek(datei, 24, SEEK_CUR);

// Lesen der Daten
daten = (char *) malloc(groesse);

if (daten == NULL)
{
cout << "Fehler beim Ansteuern von Speicher fuer das Bild" << endl;
return false;
};

if ((i = fread(daten, groesse, 1, datei)) != 1)
{
cout << "Fehler beim Lesen der Daten von " << Dateiname << endl;
return false;
};

// Alle Farben umkehren (bgr -> rgb)
for (i=0;i<groesse;i+=3)
{
temp = daten[i];
daten[i] = daten[i+2];
daten[i+2] = temp;
};

// Dateizeiger schliessen
fclose(datei);

// Erzeugen von Texturspeicher
glGenTextures(1,&KTexturen::Textur[Tex]);

// Mip-Mappen der Textur
glBindTexture(GL_TEXTURE_2D,KTexturen::Textur[Tex]);

glTexParameteri(GL_TEXTURE_2D,GL_TEXTURE_MAG_FILTER,GL_LINEAR);
glTexParameteri(GL_TEXTURE_2D,GL_TEXTURE_MIN_FILTER,GL_LINEAR_MIPMAP_NEAREST);

gluBuild2DMipmaps(GL_TEXTURE_2D, 3, groesseX, groesseY, GL_RGB, GL_UNSIGNED_BYTE, daten);

// Fertig :-)))) *jippi*
return true;
}

GLuint KTexturen::HoleTextur(int Tex)
{
return KTexturen::Textur[Tex];
}

[Demirug]

Das folgenden ist nicht böse gemeint aber IMHO machst du hier eine ganze Reihe von Design-Fehlern auf einmal.

1. Du ersetzt das auf Handles aufbauenden System von OpenGL durch ein wiederum auf Handles bassierendes System. Da du dabei nicht einmal eine Trennung von OpenGL durchführst hast du dein Programm nur um eine zusätzliche Ebene berreichert welche dir aber absolut nichts bringt.

2. Deine Methode macht viel zu viel. Du lädst eine Datei ein, legst die Textur an (mit Mipmaps) und setzts auch noch Die Filter dafür. Das sind 3 Operationen die alle auch durchaus getrennt benötigt werden.

3. Platform Spezifischer Code hat in einer Methode der hören Schicht eigentlich nichts verloren. Solche Dinge wie Endian geschichten packt man in eine eigene Sourcecode Datei und ruft sie nur noch auf. Die Endian Funktion für X86 tut dann eben einfach nichts. Wenn man schon Plattform unabhänig sein möchte dann sollte man das so lösen das man wenn man eine neue CPU hinzufügt nicht an allen möglichen Stellen ändern muss.

Ich würde das ganze also erst mal zerlegen und etwas mehr Objekt Orientiert machen:

1. Anlegen einer Klasse Bitmap welche die Bitmapdaten hält.
2. Anlegen einer Klasse Texture welche eine Textur darstellt.
3. Anlegen einer Klasse BitmapLoader.

Der Bitmaploader bekommt eine Methode Load welche ein Bitmap Objekt zurück liefert.

Die Texture Klasse bekommt Methoden zum einstellen der Filter und einen Konstruktor welcher aus einem Bitmapmap Object die Texture erzeugt.

Damit man das ganze Zusammenspiel nicht jedesmal neu einfügen muss sollte man noch eine zusätzliche Klasse TextureLoader einfügen welche die 3 anderen Klassen entsprechend nutzt um eine Textur zu erzeugen.

[zeckensack]

Ich sehe nicht ganz den Sinn dahinter, dass du drei Texturen in ein Objekt schmeisst ?(
Idealerweise sollte es IMO ein GL-Textur-Objekt pro Klasseninstanz sein. Dann kann man auch gleich das Laden im Konstruktor machen ... aber das nur nebenbei.

Einen Fehler habe ich dann doch gefunden. Und zwar gibst du die Texturobjekte nicht frei. Wenn dein Programm immer mal wieder neue Texturen nachlädt, verstopfst du so nach und nach den Texturspeicher (und die Verwaltungs-Ressourcen des Treibers).

Massnahme #1

KTexturen::KTexturen()
{
Textur[0]=0;
Textur[1]=0;
Textur[2]=0;
}

KTexturen::~KTexturen()
{
glDeleteTextures(3,Textur);
}
Anmerkung dazu:
"0" wird dir niemals von glGenTextures zurückgeliefert (... es sei denn, diese Funktion kann nicht fehlerfrei ausgeführt werden). 0 ist daher ein adäquater Platzhalter für "ich habe noch keine Ressourcen angefordert".
glDeleteTextures auf ID 0 ist auch unproblematisch, solange du das Default-Objekt (was eben diese 0 belegt) nicht benutzt. Und das tust du nur, solange du nie glBindTexture mit einem via glGenTextures generierten Handle aufrufst. Das sollte man sowieso nicht tun, das ist eine reine Abwärtskompatibilitätsmassnahme für Software aus einer Ära, als OpenGL nur eine einzige Textur überhaupt verwalten konnte. Ist also IMO auch eine sichere Sache.

Massnahme #2:bool KTexturen::LadeTextur(char *Dateiname, int Tex)
{
<...>
// Dateizeiger schliessen
fclose(datei);

//ältere Textur - falls vorhanden - freigeben
glDeleteTextures(1,&KTexturen::Textur[Tex]);

// Erzeugen von Texturspeicher
glGenTextures(1,&KTexturen::Textur[Tex]);
<...>
}Hier gilt wieder das gleiche, die 0 ist sauber.


Wenn du ganz fürchterlich korrekt sein willst, setzt du stattdessen dieses:if (Textur[Tex]!=0) glDeleteTextures(1,&Textur[Tex]);
Entsprechend kannst du natürlich auch den Destruktor 'aufwerten' (dort aber dann drei ifs, logisch).

[Ganon]

Hi.

@Demirug

Natürlich mache ich Fehler. Ich bin ja auch kein Profi, sondern will das hier lernen.
Das Ganze ist auch nur ein Lern-Projekt und kein Doom3-Konkurrent. *ggg*

Trotzdem Danke, für die Info. Ich werde es später, mit der Umstellung auf SDL (jetzt GLUT) mit betrachten. :)

@zeckensack

Ich hatte mir da etwas gedacht, aber wenn ich es recht überlege, ist meine Idee nicht so praktisch. Ich denke eine Textur pro Objekt ist sauberer. Dein Argument mit dem Konstruktor finde ich nicht schlecht.

Danke für den Tipp mit dem Löschen der Texturen.

:)