DGL • Thema anzeigen - C++: Problem

C++: Problem

Moderator: DGL-Team

Seite 2 von 2

[ 21 Beiträge ]

Gehe zu Seite Vorherige 1, 2

Vorheriges Thema | Nächstes Thema

Autor

Nachricht

dj3hut1

Betreff des Beitrags:

Verfasst: Do Mär 23, 2006 08:23

DGL Member

Registriert: Di Dez 27, 2005 12:44
Beiträge: 393
Wohnort: Berlin
Programmiersprache: Java, C++, Groovy

Hallo,

was mir noch aufgefallen ist : warum benutzt du eigentlich zum Speicher reservieren und freigeben malloc und free?
Soweit ich weiss sollte man in C++ dazu new und delete benutzen, allerdings müsstest du dann auch deine Methode LoadTga nochmal umschreiben.

Nebenbei musst du auch alles was du an Speicher reservierst wieder freigeben, egal ob nun mit malloc oder new.

In deinem Destruktor von der World-Klasse fehlt theoretisch noch vor free(matrix) ein Anweisung a la :

Code:

for(int i = 0; i < this->fieldsX; i++) free(this->matrix[i])

Viele Grüße
dj3hut1

Nach oben

SchodMC

Betreff des Beitrags:

Verfasst: Do Mär 23, 2006 08:45

DGL Member

Registriert: Fr Jul 12, 2002 07:15
Beiträge: 916
Wohnort: Dietzhölztal / Hessen
Programmiersprache: C/C++, Obj-C

dj3hut1 hat nicht unrecht.

Aber mir ist da noch etwas ganz anderes aufgefallen:

1.) Nur eine Kleinigkeit, aber hast das einen Grund das Du immer this verwendest? Innerhalb deiner Klasse wird immer die Klassenvariable bevorzugt. this-> zu verwenden ist also unnötig.

2.) Der viel wichtigere Grund: Du greifst auf uninitialisierte Objekte zu. Und zwar an folgender Stelle:

Code:

 
   this->matrix = (Area**) malloc(this->fieldsX*sizeof(Area*)); 
 
   tmpos.X = 0; 
   tmpos.Y = 0; 
   for(int i = 0; i < this->fieldsX; i++) 
   { 
      tmpos.Y = 0; 
 

Hier initialisierst Du den ersten Teil des mehr dimmensionalen Arrays. Soweit so gut. Es wird zwar für C++ empfohlen wenn möglich von malloc/free auf new/delete umzusteigen, aber das ist in diesem Fall egal. Kritischer ist folgender Bereich:

Code:

 
      this->matrix[i] = (Area*) malloc(this->fieldsY*sizeof(Area)); 
      for(int j = 0; j < this->fieldsY; j++) 
      { 
          this->matrix[i][j].setID(idgiver); 
         idgiver++; 
         this->matrix[i][j].setSize(Fieldsize); 
         this->matrix[i][j].Setup(tmpos, GRASLAND); 
         tmpos.Y += this->fieldsize; 
      } 
      tmpos.X += this->fieldsize; 
  } 
 

Du reservierst von Hand Speicher für dein Objekt. Das hat den Nachteil, dass weder der Konstruktor aufgerufen noch die vtable initialisiert wird. Funktioniert zwar in diesem Fall, ist aber extrem unsauber! Wenn Du z.B. einen Nachfahren von Area definierst, werden mit dem von Dir erzeugten Code nie die überschriebenen Funktionen des Nachfahren aufgerufen.

Warum nimmst Du nicht einen vector für Dein array? Das würde dann z.B. so aussehen:

im Header:

Code:

 
    #include <vector>
 
    using namespace std;
    ....
    typedef vector<Area *> arealist_t; // Macht das ganze ein wenig einfacher
    vector<arealist_t> matrix;
 
  public:
    vector<arealist_t> &Matrix() { return matrix }; // Liefer nur die Referenz, macht also keine Kopie vom vector!
 

und im World-Constructor:

Code:

 
   // Das fällt weg: this->matrix = (Area**) malloc(this->fieldsX*sizeof(Area*));
   tmpos.X = 0; 
   tmpos.Y = 0; 
 
   arealist_t tmplist; // Arealist = vector<Area *>
 
   for(int i = 0; i < this->fieldsX; i++) 
   { 
      tmpos.Y = 0; 
 
      // Fällt auch weg.
      //this->matrix[i] = (Area*) malloc(this->fieldsY*sizeof(Area)); 
 
      for(int j = 0; j < this->fieldsY; j++) 
      { 
          Area *tmp = new Area; // Erstellt das Objekt mit new -> Konstruktor & Co werden verarbeitet.
 
          tmp->setID(idgiver); 
          idgiver++; 
          tmp->setSize(Fieldsize); 
          tmp->Setup(tmpos, GRASLAND); 
          tmpos.Y += this->fieldsize; 
 
          tmplist.push_back(tmp); // Fügt das Objekt zum temp-vector hinzu...
      }
      tmpos.X += this->fieldsize; 
      
      // Nun den temporären vector zum matrix vector kopieren und
      // in gleich wieder leeren, damit er im nächsten Schleifendurchlauf
      // wieder verwendet werden kann.
      matrix.push_back(tmplist);
      tmplist.clear();
   } 
}
 

Freigegeben werden die objekte am einfachsten über iterratoren:

Code:

 
World::~World() 
{ 
   // free(matrix); 
   for (vector<arealist_t>::iterator i = matrix.begin(); i != matrix.end(); ++i)
      for (arealist_t::iterator j = i->begin(); j != i->end(); ++j)
         delete *j; // Das eigentliche object freigeben.
 
   // Der folgende Code muss nicht sein, da dies beim Freigeben des
   // World-Objektes sowieso gemacht wird.
   matrix.clear();
}
 

Der Zugriff erfolgt wie gewohnt:

Code:

 
void World::Draw() 
{ 
   for(int i = 0; i < this->fieldsX; i++) 
   { 
      for(int j = 0; j < this->fieldsY; j++) 
      { 
         matrix[i][j]->Draw(); 
      } 
   } 
}
 

Das mag zwar auf den ersten Blick kompliziert aussehen, ist aber sauberer, zumal Deine Area-Objekte korrekt initialisiert werden. Der Vector ist zudem nicht langsam oder so, sondern sehr performant (vector ist ein Template und viel wird dadurch als inline verwendet). Eine kleine Verzögerung mag es bei dem von mir hier geschreibenen Code geben, da mit matrix.push_back(tmplist) die Liste der Pointer einmal kopiert wird, aber das kann man vernachlässigen.

Ist nur so eine Idee. Ich finde es auf jeden Fall lohnenswert. Vectoren machen das handling von dymamischen Arrays um vieles einfacher!

_________________
Und was würdest Du tun, wenn Du wüsstest, dass morgen Dein letzter Tag auf dieser Erde ist?

Nach oben

Hondo

Betreff des Beitrags:

Verfasst: Do Mär 23, 2006 18:06

DGL Member

Registriert: Mi Mär 22, 2006 17:39
Beiträge: 7

Mahlzeit!

Eins nach dem anderen:

OpenGL ist initialisiert an dieser Stelle

Code:

 
#include "game.h"
 
int main(int argc, char *argv[])
{
    Core *core = new Core(1024,768);
    core->SetFramesPerSecond(70.0f,false);
    while(!core->Done())
    {
        core->Poll();
        switch(core->Gamestate())
        {
        case START:
            core->Start();
            break;          
        case INTRO:
            core->Intro();
            break;
        case MENU:
            core->Menu();
            break;
        case NEWGAME:
            core->NewGame();
            break;
        case INGAME:
            core->InGame();
            break;
        }
        SDL_GL_SwapBuffers();
    }
    delete core;
    return 0;
}
 

ist die Hauptschleife. core->Gamestate wird initialisiert mit START (=1), der Code der dortigen Funktion:

Code:

 
void Core::Start()
{
    if ( SDL_Init(SDL_INIT_AUDIO|SDL_INIT_VIDEO) < 0 )
    {
        printf("Konnte SDL nicht initialisieren: %s\n", SDL_GetError());
        exit(1);
    }
 
    this->screen = SDL_SetVideoMode(this->SolutionX, this->SolutionY, 0, SDL_OPENGL);
    if(!screen)
    {
        printf ("Konnte kein Framebuffer erstellen: %s\n", SDL_GetError());
    }
    SDL_GL_SetAttribute( SDL_GL_RED_SIZE, 5 );
    SDL_GL_SetAttribute( SDL_GL_GREEN_SIZE, 5 );
    SDL_GL_SetAttribute( SDL_GL_BLUE_SIZE, 5 );
    SDL_GL_SetAttribute( SDL_GL_DEPTH_SIZE, 16 );
    SDL_GL_SetAttribute( SDL_GL_DOUBLEBUFFER, 1 );
    glClearColor(0.0f, 0.0f, 0.0f, 1.0f);
 
    glViewport(0, 0, this->screen->w, this->screen->h);
    glMatrixMode(GL_PROJECTION);
    glLoadIdentity();
    glOrtho(0, this->screen->w, 0, this->screen->h, -1.0f, 1.0f);
    glMatrixMode(GL_MODELVIEW);
    glLoadIdentity();
    
    this->tmptime = SDL_GetTicks();
    this->framecounter = 0;
    this->gamestate = NEWGAME;
}
 

In core->NewGame() wird Gamestate auf INGAME gesetzt, weiter nichts (wird also quasi noch übersprungen), in Ingame() passiert dann das:

Code:

 
void Core::InGame()
{
    glClearColor(0.0f, 0.0f, 0.0f, 1.0f);
    glClear(GL_COLOR_BUFFER_BIT | GL_DEPTH_BUFFER_BIT);
 
    this->StartFrame();
    this->world->Draw();
 
    this->keys = SDL_GetKeyState(NULL);
    if(keys[SDLK_ESCAPE])
    {
        this->done = true;
    }
    this->EndFrame();
}
 

StartFrame und EndFrame sind die Framebremse und Framecounter. Also OGl ist initialisiert, wie gesagt mit glColor3f sieht alles korrekt aus.

malloc und free benutze ich aus gewohnheit.
den this-Zeiger benutze ich aus reiner Faulheit, weil mir VisualC++ dann alle Funktionen der Klasse anzeigt ;)
beides ist ja nicht falsch, und werde ich dementsprechend auch so beibehalten.

Die for-Schleife mit dem free(matrix[i]) -> richtig, hatte ich vergessen, ich hab seit nem halben Jahr nur Perl gemacht... danke für den Hinweis.

@SchodMC:

jo der konstruktor wird nicht aufgerufen, deswegen werden alle Initialisierungen in Area::Setup() durchgeführt. Also sollte es nicht notwendig sein, den Konstruktor der Klassen afzurufen, oder liege ich da falsch?

Das mit der vtable war mir so nicht bewusst. Warum wird diese dort nicht initialisiert? Geschieht das durch den Aufruf des Konstruktors? Eigentlich sollte es doch pro Klasse eine solche Tabelle geben, und in C++ soweit ich weiß auch nur wenn es wirklich virtuelle Methoden gibt. Also sollte das doch am Anfang des Programms automatisch geschehen, oder? Ich hab mich mit dieser vtable so noch nie genauer beschäftigt, Informationen sind hier sehr willkommen, aber grade nicht wirklich das Problem.

Da ich mich mit vectoren in C++ auch noch nie Beschäftigt habe und bis jetzt ganz gut ohne ausgekommen bin, möchte ich die Implementierung lieber so lassen, wie sie ist. Im Moment will ich mich mit OpenGL beschäftigen, nicht neue C++ Sprachkonzepte lernen. Trotzdem danke für den Vorschlag, ich hab mir den Code mal angesehen, und werd mich bei Gelegenheit mal genauer damit befassen.

Also, was ist mit der vtable, wie kann ich die Initialisierung hier trotzdem stattfinden lassen, b.z.w ist das überhaupt nötig, denn bis jetzt ist nichts vorgesehen, wo ich Vererbung überhaupt bräuchte. Virtuelle Methoden gibt es demzufolge auch nicht.

Also: Nochmal danke an alle, aber fällt noch jemanden was ein, was an der Texturierung falsch laufen könnte? Nochmal: Colorierung mir glColor3f funzt einwandfrei. Könnte es Fehler bei der Erstellung des Bildes gegeben haben? Hilft es vielleicht, wenn ich mal nen VC++ 6.0 Arbeitsbereich mit dem ganzen Code zur Verfügung stell?

Danke nochmal und grüße...

Nach oben

Lyr

Betreff des Beitrags:

Verfasst: Do Mär 23, 2006 23:02

DGL Member

Registriert: So Sep 26, 2004 05:57
Beiträge: 190
Wohnort: Linz

Ich zweifle nicht daran das OpenGL überhaupt initialisiert wird, ich bezweifle nur dass es zum Zeitpunkt von Area::Setup schon initialisiert ist. Wo hast du denn das
Core::world = new World( ... );
drinnen? Denn wenn nach glGenTextures( 1, &texID ); die texID immer noch 0 ist (oder halt nicht verändert wird egal wie er initialisiert ist), dann ist das die einzige Möglichkeit denn 0 ist ein reservierter Wert der niemals zurückgegeben wird.

Aber generell wäre es natürlich hilfreich den gesamten Code zu haben.

[edit]
und wegen dem malloc würde ich mir keine Sorgen machen, dein Konstruktor und Destruktor ist ja leer. Einen Pointer auf die vtable gibt es auch erst wenn du mindestens 1 virtuelle Methode in einer Klasse (oder Basisklasse) hast, was bei dir nicht der Fall ist. Darüber ob es ein schöner Stil ist oder nicht lässt sich aber natürlich streiten.
Ach ja und von den Membervariablen wird bei malloc natürlich auch kein Konstruktor/Destruktor aufgerufen, dürfte hier allerdings auch kein Problem sein, soweit man das erkennen kann.

Nach oben

SchodMC

Betreff des Beitrags:

Verfasst: Fr Mär 24, 2006 08:01

DGL Member

Registriert: Fr Jul 12, 2002 07:15
Beiträge: 916
Wohnort: Dietzhölztal / Hessen
Programmiersprache: C/C++, Obj-C

@Hondo: Sicher benötigst Du diese in deinem Fall nicht, deswegen läuft es (abgesehen von dem OpenGL Problem) ja bisher. Und weder der Konstruktor noch der Destruktor wird in deinem Fall aufgerufen. Sicher, Du kannst argumentieren dass Du weder CTOR, DTOR noch vtable benötigst, aber es ist einfach mehr als nur extrem unsauber. Jetzt mag das gehen, aber wenn Du in z.B. in einem Jahr an Deinem Objekt Area was änderst (z.B. im DTOR noch etwas aufräumen), dann läuft das nicht und Du bist Stunden mit der Fehlersuche beschäftigt. Die vtable wird übrigens von CTOR erstellt. Dieser wird von new aufgerufen (und der DTOR von delete).

Meine empfählung wäre auf jeden Fall eines: IMMER!!!!! einen sauberen strukturierten code erstellen. NIEMALS irgendwelche dirty-hacks einbauen, solange das nicht wirklich zu 100% notwendig ist! Das macht das Arbeiten deutlich leichter und weniger fehleranfällig. Und Du weißt nie ob zufällig ein anderer Kompiler bei einem object noch sonst irgendwelche Initialisierungen machen muss die nur über den Konstruktor erledigt werden. Zugegeben, die Wahrscheinlichkeit ist gering aber Stroustrup hat sich bei der Art der Implementierung von Klassen in C schon was überlegt. Es gibt vieler sochler tricksereien in C/C++ - und auch viel C/C++ code der instabil läuft!

Was die vectoren angeht: glaub mir, ich hab mich erst auch dagegen gewehrt. Jetzt möchte ich nicht mehr ohne leben müssen. Die Klassen der STL sind komfortabel und schnell. Überleg Dir's. Aber wenn Du Dich daran gewöhnt hast, wirst Du mir Recht geben.

Was Dein OpenGL-Problem angeht: überprüft doch mal NACH dem Erstellen oder Setzen Deiner Textur den Wert von glError(). Dann müsste man dem Problem auf die Schliche kommen.

_________________
Und was würdest Du tun, wenn Du wüsstest, dass morgen Dein letzter Tag auf dieser Erde ist?

Nach oben

Hondo

Betreff des Beitrags:

Verfasst: Fr Mär 24, 2006 17:54

DGL Member

Registriert: Mi Mär 22, 2006 17:39
Beiträge: 7

Mahlzeit!

Lyr, du hattest vollkommen recht, ich hatte openGL erst in Core::Start initialisiert, die Texturen (also die world insgesamt) aber schon im Core Konstrutor erstellt.

Jetzt hab ich nen schönen bunten Bildschirm. Besten Dank!

Ich hab jetzt auch malloc/free auf new/delete umgestellt, is schon besser so, werd mir das dann doch mal angewöhnen...

nochmal danke für die schnelle Hilfe und grüße an alle

Nach oben

Seite 2 von 2

[ 21 Beiträge ]

Gehe zu Seite Vorherige 1, 2

Foren-Übersicht » Programmierung » OpenGL

Wer ist online?

Mitglieder in diesem Forum: 0 Mitglieder und 8 Gäste

Du darfst keine neuen Themen in diesem Forum erstellen.
Du darfst keine Antworten zu Themen in diesem Forum erstellen.
Du darfst deine Beiträge in diesem Forum nicht ändern.
Du darfst deine Beiträge in diesem Forum nicht löschen.
Du darfst keine Dateianhänge in diesem Forum erstellen.