Problem mit char* bei int-Zuweisung

[unskilled]

paar worte zum codes:
dein fehler ist in zeile 76 der *.cpp korrigiert...

Wasabi666 schrieb:

/*PPMReader.h*/

#ifndef __PPMREADER_H__ //alles, was mit unterstrich großbuchstabe und unterstrich unterstrich
#define __PPMREADER_H__ //beginnt, ist eigtl für den compiler reserviert...

#include <iostream> //alle drei
#include <fstream> //includes hier
#include <string> //sind nicht notwendig

class PPMReader
{
	public:
		PPMReader(char *filepath); //const char*
		~PPMReader();

		char *GetPixel(int x, int y); //const - außerdem solltest du den rückgabetypen evtl auch irgendwas RAII-mäßiges ändern (boost::tuple oder schnell selbst was bauen)

		void SetPixel(int x, int y, char *rgb); //const char*

		bool SaveFile(char *filename); //const char, const
		bool ReadFile(char *filepath); //const char*

	protected:
		int width, height, bitdepth, pixelbytes;
		char *image;
};

#endif //#ifndef include_guard

/*PPMReader.cpp*/
#include "PPMReader.h"

#include <fstream>
#include <string>

PPMReader::PPMReader(char *filepath)
{
	ReadFile(filepath);
}

PPMReader::~PPMReader()
{ //sehr sinnvoll + memory-leak
}

char *PPMReader::GetPixel(int x, int y)
{
	char *rgb = new char[3];
	
	rgb[0] = image[x * 3     + y * width * 3];
	rgb[1] = image[x * 3 + 1 + y * width * 3];
	rgb[2] = image[x * 3 + 2 + y * width * 3];
	
	return rgb; //gefährlich, da der user hier selbst ein delete aufrufen muss
}

void PPMReader::SetPixel(int x, int y, char *rgb)
{
	image[x * 3     + y * width * 3] = rgb[0];
	image[x * 3 + 1 + y * width * 3] = rgb[1];
	image[x * 3 + 2 + y * width * 3] = rgb[2];
}

bool PPMReader::SaveFile(char *filename)
{
	using namespace std;

	fstream out;
	out.open(filename, ios::out | ios::binary); //fstream hat auch nen schönen konstruktor, der das kann ^^
	
	if(out.good())
	{
		out << "P6\n";
		out << width << " " << height << "\n";
		out << bitdepth << "\n";
		out << image << "\n";
	}
	else
	{
		out.close(); //unnötig
		return false;
	}

	return true; //statt mit returnwerten zu arbeiten, würde ich hier eine exception wählen
} //sollte ja nicht sooo oft vorkommen, dass er keine datei erstellen kann xD

bool PPMReader::ReadFile(char *filepath)
{
	using namespace std;

	const int buffersize = 1024;
	char *buffer = new char[buffersize]; //warum nicht stack?

	fstream in;
	in.open(filepath, ios::in | ios::binary);

	if(in.good())
	{
		in.getline(buffer, buffersize);
		string test = buffer;

		if(test == "P6")
		{
			in.getline(buffer, buffersize);
			while(buffer[0] == '#') in.getline(buffer, buffersize);
			string size(buffer, buffer+in.gcount());//siehe http://www.cplusplus.com/reference/iostream/istream/gcount/

			for(int i = 0; i < size.size(); i++)
			{
				if(size[i] == ' ')
				{
					width = atoi(size.substr(0, i).c_str());
					height = atoi(size.substr(i + 1, size.size()).c_str());
					break;
				}
			}
//der rest der fkt sieht so grausig aus, dass ich keine lust mehr hab^^
			in.getline(buffer, buffersize);
			while(buffer[0] == '#') in.getline(buffer, buffersize);
			bitdepth = atoi(buffer);

			int linebytes, imagebytes;

			if(bitdepth == 255)	pixelbytes = 3;
			else if(bitdepth == 511) pixelbytes = 6;
			else
			{
				in.close();
				return false;
			}

			linebytes = pixelbytes * width;
			imagebytes = linebytes * height;

			image = new char[imagebytes];

			/* Vorher leeren, sonst Fehler... */
			strcpy(image, "");

			in.read(image, imagebytes);
		}
		else 
		{
			in.close();//nicht notwednig
			return false;
		}
	}
	else
	{
		in.close();//nicht notwendig
		return false;
	}

	return true;//auch hier halte ich returnwerte für den falschen weg...
}

/*main.cpp*/

#include "PPMReader.h"

int main()
{
	PPMReader *ppm = new PPMReader("bild2-fleck.ppm"); //wieso nicht stack?
	
	char *pixel = new char[3]; //wieso nicht stack?
	pixel[0] = 0x00;
	pixel[1] = 0x00;
	pixel[2] = 0x00;
	
	ppm->SetPixel(1, 0, pixel);

	ppm->SaveFile("neu.ppm");

	system("PAUSE"); //ist an und für sich nen schlechter weg, die konsole offen zu halten
	return 0; //unnötig
}

btw:
std::vector<char> ist viel einfacher, als mit char* zu areiten... dann hättest du auch nicht ca. 20 memory-leaks in dem stückchen code... ^^

bb

PS: In C++ ist es eher unüblich, so ewig lange Fkt zu schreiben - wenn du mal hinguckst, könnte man da eigtl viele kleine draus machen und dann könnte man so gar durchsehen, wenn man den code nicht geschrieben hat bzw nen weilchen nicht angesehen hat... ^^

[drakon]

LordJaxom schrieb:

Auch der std::string-Konstruktor muss irgendwie die Größe der Daten erkennen, da Du selbst keine mitgibst.

*Kopf -> Tisch*

Sowas kommt davon, wenn man zu viele Sachen gleichzeitig macht.. -.-

[Wasabi666]

Ok, erstmal Kritik zur Kenntnis genommen
Also, wenn die Funktion so grausig aussieht, wenigstens sagen, was genau daran so böse is ^^, sonst kann ichs nicht verbessern...

Und was genau löst mein Problem? Sollte ich vielleicht out.write() verwenden?...

Achso und vector wollte ich mehr oder weniger nicht nehmen, wegen der Geschwindigkeit... Ein std::vector<char> ist um einiges langsamer als char*... korrigiert mich wenn ich falsch liege(aber bitte nicht zu heftig xD)

Und noch eine Frage zur GetPixel methode: wenn ich das irgendwie als int zurückgebe müsst ichs irgendwie wieder auseinanderklamüseln, könnt vllt schnell jemand aufschreiben, wie ichs schön lösen könnte?

und meinst du mit dem "warum stack?" warum ich die ganze zeit zeiger verwende?

puuh, das wars erstmal, :), klärt mich auf

[drakon]

Achso und vector wollte ich mehr oder weniger nicht nehmen, wegen der Geschwindigkeit... Ein std::vector<char> ist um einiges langsamer als char*... korrigiert mich wenn ich falsch liege(aber bitte nicht zu heftig xD)

Korrigiere: Stimmt nicht.
std::vector ist beinahe so schnell, wie jegliches Array gedöns. Im Debug Mode ist sie klar um einiges langsamer, dafür hat man aber ein Haufen checks usw. Aber im release ist sie in etwa so schnell, wie Arrays.

[Wasabi666]

ok, dann werd ichs damit auch versuchen, bloß kann ich irgendwie read, das direkt in den vector kopieren lassen oder muss ich das ganze jetz zeichenweise einlesen?... danke nochmal an alle

[Wasabi666]

ich habs jetzt erst in ein char* kopiert und dann den string damit gefüttert, mit gccount un so diesma ;),

ist das sehr unschön gelöst??

wie kann ich mein code jetz möglichst verbessern, dass selbst ihr zufrieden seid? um zu wissen was richtig un falsch is, muss mans mir schon sagen ^^

[unskilled]

ich hab keine ahnung, ob das funktioniert - würde mich eigtl sehr wundern, wenn ja^^ ich hatte ein wenig lange weile:

/*PPMReader.h*/

#ifndef H_PPMREADER_200909202000
#define H_PPMREADER_200909202000

#include <vector>
#include <utility>

struct color_tuple
{
private:
	unsigned int RGB[3];
public:
	unsigned int R() const {return RGB[0];}
	unsigned int G() const {return RGB[1];}
	unsigned int B() const {return RGB[2];}

	color_tuple(const char* RGB, unsigned char bytes_per_color);
	color_tuple(unsigned int R, unsigned int G, unsigned B);
};

class PPMReader
{
	private:
		std::vector<char> image;
		std::pair<unsigned int, unsigned int> size;

		unsigned char color_bytes;
	public:
		PPMReader(const char *filepath);

		color_tuple GetPixel(unsigned int x, unsigned int y) const;
		unsigned char GetBytesPerColor() const;

		void SetPixel(unsigned int x, unsigned int y, color_tuple color);

		void SaveFile(const char *filename) const;
		void ReadFile(const char *filepath);
};

#endif //#ifndef H_PPMREADER_200909202000

/*PPMReader.cpp*/
#include "PPMReader.h"

#include <fstream>
#include <string>
#include <sstream>

using std::ios;
using std::ifstream;
using std::ofstream;
using std::getline;
using std::pair;
using std::string;
using std::stringstream;

const char* header_tag = "P6";

#include <stdexcept>
typedef std::runtime_error      EXCEPTION_CANNOT_CREATE_FILE;
typedef std::runtime_error      EXCEPTION_CANNOT_OPEN_FILE;
typedef std::invalid_argument   EXCEPTION_WRONG_HEADER;
typedef std::runtime_error      EXCEPTION_NOT_IMPLEMENTED;

const char* read_file_exception_text = "Cannot open file for reading an image";
const char* save_file_exception_text = "Cannot create file for saving the image";
const char* wrong_header_text = "Expected other Header-Tag";
const char* not_implementet_colordepth_text = "Sorry, no support for this picture (to high maximum values for each color-value)";

//color_tuple
color_tuple::color_tuple(const char* rgb, unsigned char color_depth)
{
	const char *last;
	for(std::size_t i(0); i != 3; rgb = last, ++i)
	{
		last = rgb+color_depth;
		stringstream ss;
		ss.write(rgb, last-rgb);
		ss >> RGB[i];
	}
}

color_tuple::color_tuple(unsigned int R, unsigned int G, unsigned B)
{
	RGB[0] = R;
	RGB[1] = G;
	RGB[2] = B;
}

//helper
unsigned int pumuckls_pow(unsigned int b, unsigned int e) //^^
{
	unsigned int r = 1;

	while(e != 0)
	{ 
		if(e%2 != 0) 
			r *= b; 
		e /= 2; 
		b *= b; 
	}
	return r;
}

unsigned int get_max_from_depth(unsigned int depth)
{
	unsigned int R = 0;
	for(depth *= 8; depth != 0; --depth)
	{
		R += pumuckls_pow(2, depth-1);
	}

	return R;
}

void ignore_trash(ifstream &in)
{
	for(char delemiter; in.get(delemiter) && delemiter == '#'; )
	{
		std::string trash;
		getline(in, trash);
	}
	in.unget();
}

std::pair<int/*w*/, int/*h*/> read_size(const std::string &buffer)
{
	std::pair<int, int> R;

	stringstream ss;
	ss << buffer;
	ss >> R.first;
	ss >> R.second;

	return R;
}

unsigned char read_color_bytes(const std::string &buffer)
{
	unsigned int max;

	stringstream ss;
	ss << buffer;
	ss >> max;

	unsigned char bits(0);
	for(; max != 0; max >>= 1, ++bits)
		;
	unsigned char bytes = unsigned char(bits/8.f+.5f);
	return bytes != 3 ? bytes : 4;
}

//class
PPMReader::PPMReader(const char *filepath)
{
	ReadFile(filepath);
}

color_tuple PPMReader::GetPixel(unsigned int x, unsigned int y) const
{
	return color_tuple(
		&image[x*3*color_bytes + y*size.first*3*color_bytes],
		color_bytes
	);
}

unsigned char PPMReader::GetBytesPerColor() const
{
	return color_bytes;
}

void PPMReader::SetPixel(unsigned int x, unsigned int y, color_tuple color)
{
	//TODO: testen 
	//TODO: staticassert(sizeof(char) == 1), staticassert(sizeof(short) == 2), staticassert(sizeof(int) == 4)
	std::vector<char>::size_type
		at1 = x*3*color_bytes + y*size.second*3*color_bytes + 0*color_bytes,
		at2 = x*3*color_bytes + y*size.second*3*color_bytes + 1*color_bytes,
		at3 = x*3*color_bytes + y*size.second*3*color_bytes + 2*color_bytes;
	switch(color_bytes)
	{
	case 1:
		image[at1] = static_cast<char>(color.R());
		image[at2] = static_cast<char>(color.G());
		image[at3] = static_cast<char>(color.B());
		break;
	case 2:
		*reinterpret_cast<short*>(&image[at1]) = static_cast<short>(color.R());
		*reinterpret_cast<short*>(&image[at2]) = static_cast<short>(color.G());
		*reinterpret_cast<short*>(&image[at3]) = static_cast<short>(color.B());
		break;
	case 4:
		*reinterpret_cast<int*>(&image[at1]) = static_cast<int>(color.R());
		*reinterpret_cast<int*>(&image[at2]) = static_cast<int>(color.G());
		*reinterpret_cast<int*>(&image[at3]) = static_cast<int>(color.B());
		break;
	default:
		throw EXCEPTION_NOT_IMPLEMENTED(not_implementet_colordepth_text);
	}
}

void PPMReader::SaveFile(const char *filename) const
{
	ofstream out(filename, ios::out | ios::binary);

	if(out.bad())
		throw EXCEPTION_CANNOT_CREATE_FILE(save_file_exception_text);

	out << "P6\n"
		<< size.first << " " << size.second << "\n"
		<< get_max_from_depth(color_bytes) << "\n";

	out.write(&image[0], image.size());
}

void PPMReader::ReadFile(const char *filepath)
{
	ifstream in(filepath, ios::in | ios::binary);

	if(in.bad())
		throw EXCEPTION_CANNOT_OPEN_FILE(read_file_exception_text);

	string buffer;
	getline(in, buffer);

	if(buffer != header_tag)
		throw EXCEPTION_WRONG_HEADER(wrong_header_text);

	ignore_trash(in);
	getline(in, buffer);
	size = read_size(buffer);
	ignore_trash(in);
	getline(in, buffer);
	color_bytes = read_color_bytes(buffer);

	unsigned int length = color_bytes * size.first * size.second;
	image.resize(length);

	in.read(&image[0], length);
}

so in der richtung würd ichs jedenfalls lösen^^
gefallen tuts mir trotzdem nicht, aber vll hat jmd anderes ja lust, die fehler darin zu beseitigen und das iwie eleganter zu lösen

bb

[Wasabi666]

xD ich hab mich jetz an das angeben der größe beim string erstellen gehalten und habe anstatt vector<char> string genommen, was wenn ich mich nich irre sich doch sehr ähnlich ist, ansonsten geht meins jetzt auch...
versuch mich grad bei dir einzulesen, aber so ein paar sachen wie exception handling hab ich noch nich oft gemacht (c++ mach ich auch erst seit geraumen 2 monaten)...

was mich jetzt aber richtig interessiert ist:
WARUM GEHT FOLGENDES?: in.read(&image[0], groeße);?? das erscheint mich wegen eig unterschiedlicher datentypen zunächst sinnlos ^^ habs so jetzt für meins auch übernommen, da ich nich ünnötig erneut kopieren möchte, also danke!

[Wasabi666]

achso und das mit der pixelklasse werd ich auch noch einbaun, weil stimmt schon, dass man sonst vllt mal ein delete vergisst, danke

[unskilled]

wie, wegen des typs?
naja - ob string oder vector, nimmt sich fast nichts - ich hätt aber trotzdem vector genommen ^^

http://www.cplusplus.com/reference/

dort siehst du, was read für parameter entgegen nimmt, eine überladung ist in etwa so:

istream& read(char *first, size_t count)

in.read(&image[0], length);
ruft dieses auf:
image[0] = 1. element
&image[0] = adresse des 1. elements
length ist wohl selbsterklärend ;o)

bb

edit:

achso und das mit der pixelklasse werd ich auch noch einbaun, weil stimmt schon, dass man sonst vllt mal ein delete vergisst, danke

ich hoffe, dir fällt was eleganteres ein
ist das, was mir am allerwenigsten gefällt - aber ich hab erst am ende mal in die doku geguckt und gesehen, dass es so was gibt^^

[Wasabi666]

joa guck ich morgen mal in meiner info stunde bei ^^
also hauptsächlich war ich verwundert, weil vector doch eine art lineare liste ist und read() muss doch irgendwoher wissen, welche der variablen in dieser klasse auf das nächste element verweist, weil vector doch eig total anders is als char* und das kann read doch eig alles nich wissen ^^ deswegen versteh ichs nich ganz, warum das funktioniert

[unskilled]

"vector doch eine art lineare liste"
es ist ein array... (ne liste ist immer linear^^) es gibt 3 lineare container in der standard-lib: deque, list, vector

und vector ist nur ein wrapper für ein array - also von aufbau + funktionsweise in etwa so:

struct vector
{
  int *x;

  vector(int laenge)
  {
    x = new int[laenge];
  }

  ~vector()
  {
    delete x;
  }

  int& operator[] (int index) {return x[index];}
};

also kann man sicher sein, dass es ein stück zusammenhängender speicher ist (im gegensatz zu deque oder list)...

bb

[Wasabi666]

aaaaah dann macht das sinn, danke für die erklärung und schönen abend noch euch allen! (programmempfehlung: star wars 2 xD)