Keskustelu: Ohjelmointikysymykset: C++: SDL-ongelma

paha laittaa ongelmaan suoraan viittaava otsikko, kun ei tiedä tarkasti mikä on vikana :(

ohjelma kaatuilee salaperäisesti kun esim. kuvapinta alustetaan
eli siis ongelma seuraavassa koodissa:

int main(int agrc, char *argv[]){

	if(SDL_Init(SDL_INIT_VIDEO|SDL_INIT_NOPARACHUTE)<0){
		fprintf(stderr, "SDL:n alustus ei onnistunut: %s\n", SDL_GetError());
		return 0;
	}

	SDL_Surface * naytto;

    naytto = SDL_SetVideoMode(screenW, screenH, bittisyvyys, SDL_HWSURFACE);

	imgHandler kuvitus;


	SDL_Surface *testi = kuvitus.getPicSurface(kUKKO);


	fprintf(stdout, "kuvien määrä ;): %s\n", kuvitus.getPicCount());


	SDL_FreeSurface(naytto);
	SDL_FreeSurface(testi);
	SDL_Delay(3000);
	SDL_Quit();
	return 0;
}

kaatuminen tosin tapahtuu vasta tuossa SDL_Quit() kohdassa
sitten vielä että tuo imgHandler toimii, koska ohjelma ei kaadu jos vain tuo kuvitus ja naytto ovat alustettuina.. ne siis toimivat vaikka ilmankin NOPARACHUTEa
mutta heti tuon fprintf:n ja testi-kuvapinnan kanssa ohjelma kaatuu viimeistään ohjelman lopussa..

Tää on aika yleinen virhe. Sun ei kuulu käsitellä näyttöä SDL_FreeSurfacella. Mutta jos välttämättä haluat, vaihda SDL_FreeSurface(naytto) ja SDL_FreeSurface(testi) -rivien paikkaa.

SDL_Quit() vapauttaa näyttöpinnan. Jos vapautat sen itse SDL_FreeSurfacella, ohjelma kaatuu SDL_Quit:ssa sen yrittäessä vapauttaa ohjelmalle varaamatonta muistia.

hmm.. otin sen näyttöpinnan vapautuksen pois, mutta se kaatuu vieläkin samas kohtaa..
edit:: tää ei päästä esim fprintf -funktiota läpi, eikä tuota SDL_Surface *testi; juttuakaan, aina kaatuu

edit 2::

#include "imgHandleri.h"

imgContainer imgContainer::setPicSurfaceCon(const char *nimi){
	kuva = SDL_LoadBMP(nimi);
	return *this;
}

imgHandler::imgHandler(){
	a;
	b;
	c;
	kuvat[kUKKO] = (a.setPicSurfaceCon(fUKKO));
	kuvat[kSEINA] = (b.setPicSurfaceCon(fSEINA));
	kuvat[kTAUSTA] = (c.setPicSurfaceCon(fTAUSTA));
	//fprintf(stdout, "kuvat ladattu %s\n");
}

SDL_Surface * imgHandler::getPicSurface(int a){
	//fprintf(stdout, "kuva etsitty %s\n");
	return ( ( kuvat.find(a)->second ).getPicSurfaceCon() );
}

int imgHandler::getPicCount(){
	return kuvat.size();
}

void imgHandler::destructImgHandler(){
	for(iter = kuvat.begin(); iter != kuvat.end(); iter++){
		SDL_FreeSurface( ((*iter).second).getPicSurfaceCon() );
		//fprintf(stdout, "kuva poistettu %s\n");
	}
}

ja vielä se container:

class imgContainer{
	private:
		SDL_Surface *kuva;
	public:
		imgContainer() : kuva(0) {}
		imgContainer setPicSurfaceCon(const char *nimi);
		//~imgContainer() { SDL_FreeSurface(kuva); }

		SDL_Surface * getPicSurfaceCon(){
			return kuva;
		}
};

Ainakin tuo imgContainer-luokka näyttäisi olevan päin honkia. Teepä niin viisaasti, että laitat jokaisen SDL_LoadBMP:n ja SDL_FreeSurfacen kaveriksi tulostuksen:

printf("%p ladattu\n", kuva); // %p niin kuin osoitin
printf("%p vapautettu\n", kuva);

Näet, että pintoja ehkä vapautellaan hieman siellä täällä, toisinaan sama monta kertaa ja jo imgHandler-luokan luonnissa.

Toinen pulma on itsestäänselvästi siinä, että tulostat int-tyyppistä tietoa %s-formaatilla.

Mielekäs toteutus tuolle imgContainerille olisi tämän tapainen:

#include <cassert>

class imgContainer {
private:
	// Apurakenne, joka sisältää varsinaiset tiedot
	struct imgContainerData {
		SDL_Surface *surf;
		int numrefs; // Moniko imgContainer käyttää tätä dataa?
		imgContainerData() : surf(0), numrefs(0) {}
		imgContainerData(const char *fn) : surf(SDL_LoadBMP(fn)), numrefs(0) {}
		~imgContainerData() {
			assert(numrefs == 0); // Eihän kukaan tosiaan käytä enää?
			if (surf) SDL_FreeSurface(surf);
		}
	};
	// Itse imgContainer sisältää vain viittauksen oikeaan dataan
	imgContainerData &c;
public:
	// Uutta luodessa täytyy luoda se oikeakin data
	imgContainer() : c(*new imgContainerData()) {
		c.numrefs++;
	}
	// Toinen luomistapa, ladataan kuva samalla
	imgContainer(const char *fn) : c(*new imgContainerData(fn)) {
		c.numrefs++;
	}
	// Kun imgContainer kopioidaan, uusi osoittaa samaan dataan
	imgContainer(imgContainer const& old) : c(old.c) {
		c.numrefs++; // Uusi käyttäjä datalle
	}
	// Tuhotessa tietenkin dataa käyttää taas yksi vähemmän
	~imgContainer() {
		c.numrefs--;
		// Jos tämä oli viimeinen käyttäjä, datankin voi tuhota
		if (c.numrefs == 0) {
			delete &c;
		}
	}
	// Osoittimen saa näin näppärästi ulos suoraan tyyppimuunnoksella
	operator SDL_Surface * () { return c.surf; }
};

Vielä pieni testiohjelma ja sen aiheuttamat luomiset ja tuhoamiset:

imgContainer lataa_kuva(const char *fn)
{
	imgContainer a(fn);
	return a;
}
void piirra(imgContainer kuva) { }

int main()
{
	imgContainer c = lataa_kuva("moi.bmp");
	piirra(c);
	imgContainer d = c;
	return 0;
}

imgContainerData("moi.bmp")
imgContainer("moi.bmp")
imgContainer(old)
~imgContainer()
imgContainer(old)
~imgContainer()
~imgContainer()
~imgContainerData()

eli siis, jos nyt tuota tutkittuani oikein ymmärsin, niin ohjelma kaatui sen testi -kuvapinnan kanssa siksi, että yritin sijoittaa sinne vapautetun kuvan ja sen jälkeen vielä uudelleen vapauttaa sen?

en tuota koodia nyt välttämättä lopulliseen toteutukseen suoraan kopioi, koska tätä projektiani teen siksi, että oppisin ohjelmoimaan paremmin...
no, kehittymistä odotellessa..

mutta kiitoksia taasen ;)

edited a bit...

Huomaa, että kun funktion palautusarvo on luokka, palautettava luokan kopio tuhotaan funktion päätyttyä. Tällöin siis tuossa imgContainer-luokassasi oleva kuvapinta tulee vapautettua jo heti palautuksen jälkeen, joten siinä, johon palautusarvon sijoitat, onkin kelvoton osoitin. Samoin luokka luodaan ja tuhotaan jokaisen funktiokutsun yhteydessä, jos parametrina annetaan koko olio eikä esimerkiksi viittausta tai osoitinta. Jos lisäät esimerkkiini tulostukset main-funktioon eri kutsujen väleihin, kutsuttavien funktioiden sisään sekä luokan luojaan ja tuhoajaan, näet havainnollisesti, missä kohti uusia olioita luodaan ja tuhotaan. Kannattaa kokeilla.

tarkistan nyt vielä että ymmärsin tuon yhden kohdan oikein:

imgContainer() : c(*new imgContainerData()) {
    c.numrefs++;
}

eikö tämä siis välitä sille imgContainerData &c -jäsenmuuttujalle tuon uuden luodun struktuurin sisällön?

Ei. Viittaus on pohjimmiltaan suunnilleen sama asia kuin osoitin, se vain toimii hieman eri syntaksilla. Seuraavat koodit ovat käytännössä ekvivalentteja:

X * c = new X;
c->jasen = 1;
delete c;

X &c = *new X;
c.jasen = 1;
delete &c;

Ehkä on selkeämpää, jos käytät tuon viittauksen sijaan sittenkin osoitinta. :) Viittauksen hyvä puoli on kuitenkin se, ettei sen arvoa voi muuttaa matkalla, vaan se viittaa aina samaan paikkaan.

Ok.

Tällee offtopickkina tuli mieleen, että kuinka monta vuotta pitää ohjelmoida, ennen kuin on yhtä hyvä kuin sinä? :D

Näennäisestä samankaltaisuudesta huolimatta pointtereilla ja referensseillä on pari hyvin tärkeää eroa. Osoitin voi olla null, ja osoitinta voidaan muuttaa osoittamaan jonnekin muualle. Sen sijaan, kun viite on alustettu, se toimii eräänlaisena aliaksena sille viittauksen kohteelle, eikä viitettä voi muuttaa viittaamaan toiseen kohteeseen. Lisäksi viitteen voi aina olettaa viittaavan johonkin - kyllä sen tietenkin nulliksikin voi alustaa, mutta se sotii kaikkia järkeviä käytäntöjä vastaan ja on silkkaa vaikeuksien keräämistä.

Peukalosääntö on, että C++:ssa kannattaa käyttää viitteitä aina kun voi ja osoittimia silloin kun täytyy, esimerkiksi kun kohde voi jollain tavalla puuttua tai vaihtua. Tuossa container-esimerkissä sen jäsenmuuttujan kannattaa siis olla osoitin eikä viite.

Tuosta korjaillusta containerista alkaa jo jotain tolkkua saadakin, mutta siinä pitäisi vielä muistaa C++:n klassinen kolmen sääntö: Jos luokka on sillä tavalla ei-triviaali, että sille täytyy erikseen kirjoittaa destruktori, kopioiva konstruktori tai sijoitusoperaattori, on syytä kirjoittaa nuo kaikki kolme. Tai sitten estää sijoituksen ja kopioivan konstruktorin käyttö kokonaan. Tällä tavoin resurssien hallinta hoituu yleensä vähimmillä vaivoilla ja yllätyksillä.

Koodin ja koodin lukijoiden kannalta tuo esimerkkitapaus kannattaisi myös suunnitella niin, ettei mitään ImageContaineria edes käytetä. Pelkkä ImageData-luokka tms. yhdessä shared_ptr:n tms. kanssa on järkevämpi ja selkeämpää. Mutta eihän näitä kaikkia voikaan ihan heti tietää ja osata.

C++:sta on niin paljon kirjallisuutta, ettei ihan kaikkia mokia tarvitse ehtiä tehdä ihan itse. Vaikka koodaamaan oppii koodaamalla, niin kyllä muukin perehtyminen tukee oppimista.

koo kirjoitti:

Koodin ja koodin lukijoiden kannalta tuo esimerkkitapaus kannattaisi myös suunnitella niin, ettei mitään ImageContaineria edes käytetä. Pelkkä ImageData-luokka tms. yhdessä shared_ptr:n tms. kanssa on järkevämpi ja selkeämpää. Mutta eihän näitä kaikkia voikaan ihan heti tietää ja osata.

Aluksihan tuossa oli imgHandler -luokka, minne oli tarkoitus laittaa map tauluun kaikki SDL_Surfacet. imgContainer tuli siitä, kun mietin miten ne kuvapinnat voisi helposti vapauttaa, ja eikös ~map kutsu taulukoitujen luokkien tuhoajia..
Tosin tälleen jälkikäteen katsottuna olisi tavallinen for silmukka ollut parempi..
mutta täytyy katella miten ton kuvasysteemin lopulta viimestelen

Resurssienhallinta on C++:ssa se kova juttu, sitä varten esimerkiksi destruktorisysteemi toimii niin kuin toimii. Esim. Javassa hommaan on otettu erilainen lähestymistapa ja siksi käytettävät rakenteet ovatkin siellä erilaisia.

Jos luokka mallintaa jonkun resurssin, jota ei periaatteessa voi kopioida, vastaavien olioiden on yleensä järkevää olla ei-kopioitavia.

Jos noita olioita kuitenkin pitää luoda ja tuhota dynaamisesti ja niihin voi olla useampia viittauksia, ei tuollaisen "container-luokan" tekeminen ole yleensä paras ratkaisu. Yksi hyvä malli on se, että oliolla on vain yksi omistaja esimerkiksi auto_ptr:n avulla. Muille käyttäjille annetaan sitten viite tähän olioon. Toinen hyvä malli on käyttää shared_ptr:ia, jolloin olion omistajuus on jaettu. Molemmissa tapauksissa olio tuhoutuu automaattisesti, kun viimeinen tai ainoa omistaja häipyy.

Yksi hyvä puoli noissa molemmissa tavoissa on se, että koodin lukijoiden on melko helppo ymmärtää, miten oliota käytetään siinä mielessä, että onko kyseessä viitteen tai osoittimen tapainen viittaus ja mitä se merkitsee. Ellei luokkien nimeämisestä ja rajapinnoissa ole huolellinen, voi koodin lukijoille tulla ärsyttäviä yllätyksiä:

A a;
a.data = 0;
// myöhemmin
A b = a;
b.data = 42;
// jossakin
if (a.data == b.data) {
// mitenkäs tänne jouduttiin, missäs a:n dataa on muutettu...

// MUTTA
shared_ptr<A> a(new A);
a->data = 0;
// myöhemmin
shared_ptr<A> b = a;
b->data = 42;
// sitten
if (a->data == b->data) {
// joo, no näinhän pointteriden kanssa voikin sattua, ei yllätä

Unohdin aivan, että noita pitäisi voida sijoittaakin. Silloin viittaus ei tosiaan tule kysymykseenkään, sen kanssa sijoitusoperaatio ei toimisi aivan odotusten mukaan.

Itse asiassa SDL_Surface sisältää jäsenen refcount. Dokumentaatio aiheesta on heikkoa (kuten SDL:n tapauksessa yleensäkin), mutta lähdekoodin perusteella tuota voi käyttää näin:

class imgContainer {
	SDL_Surface *surf;
public:
	imgContainer() : surf(0) { }
	imgContainer(const char *fn) {
		surf = SDL_LoadBMP(fn);
		// surf->refcount == 1
	}
	imgContainer(imgContainer const& other) {
		surf = other.surf;
		if (surf) {
			surf->refcount++;
		}
	}
	~imgContainer() {
		if (surf) {
			// surf->refcount pienenee itsestään.
			// Jos se menee nollaan, pinta vapautuu
			SDL_FreeSurface(surf);
		}
	}
	imgContainer& operator = (imgContainer const& other) {
		// Vapautetaan vanha pinta (ts. refcount pienenee)
		if (surf) {
			SDL_FreeSurface(surf);
		}
		// Otetaan uusi käyttöön ja nostetaan refcountia
		surf = other.surf;
		if (surf) {
			surf->refcount++;
		}
	}
	opreator SDL_Surface * () {
		return surf;
	}
};

Jos luokkia kaipaat, järkevää on saman tien kehittää kuville luokka, joka huolehtii myös piirtämisestä ja muista operaatioista. Kehottaisin myös noudattamaan annettua neuvoa: välitä mahdollisuuksien mukaan aina vain viittauksia kuviin, jottei tapahdu turhaa luonti-, kopiointi- ja tuhoamistyötä.

Jos kuitenkin haluat välttämättä vain tuollaisen pelkistetyn osoitinsuojan, niin mainittu shared_ptr kelpaa vallan mainiosti. C++0x-standardin julkaisemista ja kääntäjätukea odotellessa tuon voi kaivaa boost-kirjastosta. Ihan harjoituksen kannalta voi tietysti olla mukavaa kehittää myös omia luokkia, aina siinä jotain oppii. Sitten jossain vaiheessa voi ottaa käyttöön valmiit tekeleet, jotka ovat usein parempia tai ainakin säästävät omalta vaivalta.

Eräässä omassa toteutuksessani päädyin säilyttämään hallinnointijärjestelmän mapissa kuvien osoittimia, jolloin ne on helppo lopuksi silmukassa vapauttaa sieltä. Ennenaikaista vapauttamista varten tein erillisen funktion, joka poistaa osoittimen mapistakin. Varsinaisen ohjelman sisällä kuvia käsitellään siis osoitinten avulla ja vain kuvajärjestelmän omilla funktioilla (joskin eräs funktio mahdollistaa pikselidatan suoran muokkaamisen, joten kaikki on mahdollista).

P.S. Minä en ole ohjelmoinut kuin kuutisen vuotta, ja kuten näkyy, esimerkiksi koo on hiukkasen edellä. :) Mutta eipä tuo pelkistä vuosista ole kiinni, kuten tuolla yleisessä keskustelussa(si) tuli juuri todistettua. ;)

Kiitoksia taasen hyvistä neuvoista/ideoista.
On muuten aika jännä huomata miten lähestymistavat muuttuvat ohjelmointikokemuksen karttuessa.. :D
Mutta ehkä minäkin tästä vielä jonain päivänä etenen kuhan vaan harjottelen(minkä lukiolta ehdin..) :)