Is this code efficient?

[N36fSL3A]

Well I was thinking about how I'd implement my 3D model format. I tried to make the model format very simple, and I didn't actually compile it, but I was wondering what you thing of this prototype code?

// VlE 3D Model Format outline
#include <iostream>
#include <string>
#include <fstream>

#define VERSION	1

namespace binary
{
	Uint32 Read32(std::ifstream &input)
	{
		Uint32 value;
		Uint8  bytes[4];
		
		input.read((char*)bytes, 4);
		
		// Reconstruct the bytes into a 32 bit integer
		value = (bytes[0] | (bytes[1] << 8) | (bytes[2] << 16) | (bytes[3] << 24))
		
		return value;
	}
}

enum a_humanStates
{
	hIDLE,
	hWALK,
	hWALK
	hCROUCH;
};

enum a_WeaponAnim
{
	wIDLE,
	wFIRE,
	wRELOAD,
	wBLOCK1,
	wBLOCK2,
	wBLOCK3,
	wMELEE1,
	wMELEE2,
	wMELEE3;
	;
};

struct Vertex
{
	float x, y, z;
};

Vertex -operator(Vertex a, Vertex b)
{
	float x, y, z;
	
	x = a.x - b.x;
	y = a.y - b.y;
	z = a.z - b.z;
	
	Vertex ret = {x, y, z};
	
	return ret;

}

Vertex +operator(Vertex a, Vertex b)
{
	float x, y, z;
	
	x = a.x + b.x;
	y = a.y + b.y;
	z = a.z + b.z;
	
	Vertex ret = {x, y, z};
	
	return ret;

}
Vertex -operator(Vertex a, Vertex b)
{
	float x, y, z;
	
	x = a.x - b.x;
	y = a.y - b.y;
	z = a.z - b.z;
	
	Vertex ret = {x, y, z};
	
	return ret;

}

Vertex *operator(Vertex a, Vertex b)
{
	float x, y, z;
	
	x = a.x * b.x;
	y = a.y * b.y;
	z = a.z * b.z;
	
	Vertex ret = {x, y, z};
	
	return ret;

}

Vertex /operator(Vertex a, Vertex b)
{
	float x, y, z;
	
	x = a.x / b.x;
	y = a.y / b.y;
	z = a.z / b.z;
	
	Vertex ret = {x, y, z};
	
	return ret;

}

struct Triangle
{
	Vertex points[3];
};

struct Mesh
{
	Triangles*   mesh; // Mesh. Each model has many, many meshes
	                   // if they use vertex animation
					   
	~Mesh()
	{
		delete[] mesh;
	}
};

struct Animation3D
{
	Uint32 startFrame;
	Uint32 endFrame;
};

class Model
{
	public:
		Model(char* location);
		void Draw();
		~Model();
		
		void Animate(float seconds);
		
	private:
		char* name[16];
	
		Mesh*        model;    // Dynamic array of meshes
		
		Uint32       numAnims; // The number of animations the model contains.
		Animation3D* anim;     // Aniamtion array
		
		Uint32 frames;
		
		Uint32 currentFrame;
		
		Uint32* numPoints;
		Uint32* numTriangles;
		
		bool   vertexAnim;
};

Model::Model(char* location)
{
	std::ifstream input(location);
	
	Uint32 identifier;// Magic number. Should equal 1,243,853
	Uint8  version;   // Version of the model
	
	currentFrame = 0;
	
	// If the file loaded correctly
	if(input)
	{
		identifier = binary::Read32(input);
		if(identifier != 1243853)
		{
			std::cout << "Failure loading model: Not valid format.\n";
			return;
		}
		
		input.read((char*)version, 1);
		if(version != VERSION)
		{
			std::cout << "Failure loading model: Not compatible version.\n";
		}
		
		input.read((char*)name, 16);
		
		input.read((char*)vertexAnim, 1);
		
		frames = binary::Read32(input); // Load the number of frames there are
		if(frames == 0)
		{
			std::cout << "Failure loading model: 0 Frames";
			input.close();
			return;
		}
		
		//numPoints    = new Uint32[frames];
		numTriangles = new Uint32[frames]; // We allocate an array of the number of triangles accordingly
		
		for(unsigned int f = 0; f < frames; f++)
		{
			//numPoints[f]    = binary::Read32(input);
			numTriangles[f] = binary::Read32(input); // Load the number of triangles for each frame
		}
		
		// Allocate an array of meshes
		model = new Mesh[frames];
		
		// Loop through the number of frames, allocate enough memory for the models.
		for(Uint32 m = 0; m < frames; m++)
		{
			model[m]->mesh = new Triangle[numTriangles[f]]; // First allocate triangle memory for the 
			                                                // each individual mesh

			/* -Remember, a mesh is nothing more than a bunch of triangles- */
			
			// Read the positions of the triangle points
			// in each individual mesh.
			for(Uint32 t = 0; t < numTriangles[f]; t++)
			{
				// Get each point of the mesh
				model[m]->mesh[t]->x = binary::Read32(input); // Read the x coord
				model[m]->mesh[t]->y = binary::Read32(input); // Read the y coord
				model[m]->mesh[t]->z = binary::Read32(input); // Read the z coord
			}
		}
		
		// Read the number, then allocate memory for animations
		Uint32 animations = binary::Read32(input);
	
		anim = new Animation3D[animations];
		
		// Loop through and read the start and and frames for
		// all values
		for(Uint32 a = 0; a < animations; a++)
		{
			// Load values
			anim[a].startFrame = binary::Read32(input);
			anim[a].endFrame   = binary::Read32(input);
		}
		
		std::cout << "Model successfully loaded.\n";
	}
}

Model::~Model()
{
	delete[] Frames;
	delete[] anim;
	delete[] numPoints;
	delete[] model;
};

As you can see it's a very simple format, based around the MD2 format.

[zb0S216C]

Lumpkin wrote:

"I was wondering what you thing of this prototype code?"

It's OK, but could be improved.

Lumpkin wrote:

"Is this code efficient?"

Compile the code and then profile it. We can't give an accurate prediction of program speed due to the optimisations that you or your compiler enables which may affect the final executable. There are better design choices that you could consider, however.

Also, what do you deem as "efficient?" Each project has its own performance goals that it needs to conform to, so what are yours?

_Wazzak

[N36fSL3A]

I'll be able to test in a sec, just reading through python documentation to finish up the exporter.

It's OK, but could be improved.

What could be improved?

What do you mean profile? How would I do that?

Should I get the time at the beginning of the operation and subtract it from the time at the end of the operation?

[htirwin]

What IDE and compiler do you use? With gcc, you can compile with the -pg flag, and then run the program, and then run gprof. gprof -b ./name-of-program.

In code blocks, you can just go to build options and add the -pg flag, then compile and run the program, then go into the plugins menu, and choose profile.

What profiling does for you, is it tells you how much time is spent in each function, and the percentages out of the total time that each function took.

There are other profilers. I think you need to pay for the visual studio profiler.

[9wqjE3v7]

Where is your error handling? If your model class constructor fails, the draw subroutine could return unexpected results. If not too many methods rely on the constructor, then using simple enumerated error codes as return values on other methods would be fine. If a larger project, use exceptions.

Also explicitly defining a destructor without a copy/= operator could result in multiple errors in terms of dynamic memory allocation (two pointers pointing to the same block of memory). I would strongly advise unique_ptr instead of using raw pointers unless you can really keep track of what is going in and out of the heap.

A little less significant, though you do not need to specify a return value for a constructor.

[N36fSL3A]

I know, I want to return from the constructor if messes up.

I'm actually going over the code now as we speak, I'm changing up some things so it fits into my program nicely.

Also, how would I set the pointer to NULL if it fails in the constructor?

[9wqjE3v7]

The simplest way is to use the 'nothrow' exception.

eg. int x = new (nothrow) int [y]

If the pointer fails to allocate memory, then the pointer is immediately set to NULL.

How is just using return going to aid error handling? The only way is to throw an exception.

[N36fSL3A]

I want to to exit the function early if there's an error.

[9wqjE3v7]

How are other methods (if any) which rely on the constructor are going to know that there was an error?

[N36fSL3A]

When I start it, I was hoping to do something like this:

Model* plyr = new Model("plyrmodel.vle");
if(plyr == NULL)
{
    // (The constructor could be able to set this to null if the file
    // failed to load...
    //(display error message and exit)
}

[9wqjE3v7]

That is a fine way to handle it, though I am talking about handling determining errors WITHIN the constructor. How would the draw function in your model class know that the constructor terminated abnormally? Whoever is using your classes should not have to handle all the errors themselves, since they do not know everything that couid go wrong with the constructor.

[S G H]

I suggest you to keep a boolean state which tells if it did properly initialize or not.
Handling errors from your Constructor isn't really versatile.

[9wqjE3v7]

^That is the example I wanted to give, have a bool as a private member set to true if all is well. Sorry for the misleading reply, I meant error determining, not checking. Though a constructor could have to handle errors to an extent if it depends on something else.

Do something like this:

foo::foo() : is_init(false) {
//do something | set is_init to true or false depending on occurrence
}

Sorry if the code comes out a little odd, I'm on my phone.

[N36fSL3A]

Alright. I think I finished handling all necessary functions. Now I just have a few more python-side things to do.

[N36fSL3A]

Why can't python have static type vars? Can someone tell me how to export intergers and floats to binary files with it?

[o1vk4iN6]

Vertex *operator(Vertex a, Vertex b)

Should use const references here and you can really cut down on line usage by creating a constructor. Not even sure you are using the operator correctly...

struct Vec3
{
    float x;
    float y;
    float z;

    Vec3(float x, float y, float z) : x(x), y(y), z(z) { }

    Vec3 operator - (const Vec3& a) { return Vec3(x-a.x, y-a.y, z-a.z); }
    Vec3 operator + (const Vec3& a) { return Vec3(x+a.x, y+a.y, z+a.z); }

};

Also would be better to call that a vector, a vertex doesn't really need to have operator overloads for stuff like addition...

struct Vertex
{
    Vec3 position;
    Vec3 normal;
};

[N36fSL3A]

Noticed that too. It's already changed in the current code. I'll update it tomorrow.

[TheIdeasMan]

@Lumpkin

I can see that you have your * operator which doesn't make any sense at all. What is the meaning of the result?

Normally you would have scalar product, dot product, & cross product, which are the 3 vector multiplication operations.

The / operator doesn't make sense either. You could have one that multiplies the vector by a scalar of 1 over a real number, but it is probably not worth it.

You will probably need Magnitude, Unit Vector (A vector length 1.0 aka normalised vector) functions as well, to enable you to do basic vector math.

EDIT:

The thing is, I thought you knew how to do 3d vector math - or was that your previous incarnation?

Just to check, can you tell us what the distance between (1,1,1) and (2,2,2) is?

However it is really good that you are even doing vector math at all, at your age.

[9wqjE3v7]

Now that TheIdeasMan has mentioned it, you shouldn't really need a vector subtraction method, since doing (3,3,3) - (2,2,2) is the same as doing (3,3,3) + (-2,-2,-2)

[LB]

It would then be convenient to have operator-() with no parameters to negate it.