malloc, the best way?

[likesoursugar]

Hi. I'm working with some kind of map file and I will stora data like, triangles, vertices and the coordinates for the vertices.
I have a problem tho.

        file->header.num_tris = 1;


	//memory allocation
	file->triangles = (struct s_triangle *)
		malloc (sizeof (struct s_triangle) * file->header.num_tris);

	file->triangles->vertex = (struct vec3 *)
		malloc (sizeof (struct vec3) * file->header.num_verts);


	for(int i = 0; i < file->header.num_tris; i++)
	{
		for(int j = 0; j < 3; j++)
		{
			file->triangles[i].vertex[j].x = 1;
			file->triangles[i].vertex[j].y = 2;
			file->triangles[i].vertex[j].z = 4;
		}
	}

so... if I change the file->header.num_tris value from 1 to 2 or higher the program will crash, Is there something I did wrong with the malloc?
If you need some more code for the structures, jsut tell me.
Would be awesome if you could help me with this problem :)

[Deco Da Man]

Tip: Use a debugger to find out which line is crashing the program.
(Put breakpoints on each line)

(I can't help, sorry :( )

[likesoursugar]

[QUOTE=Deco Da Man;20205770]Tip: Use a debugger to find out which line is crashing the program.
(Put breakpoints on each line)

(I can't help, sorry :( )[/QUOTE]

if I change file->triangles[i].vertex[j].x = 1;
to
file->triangles[i].vertex.x = 1;

and change the
structure

struct s_triangle
{
	vec3 * vertex;
};

to vec3 vertex;
It will run, but I need that array for vertex =/
I'm maby wrong now but yeah...

[editline]09:30AM[/editline]

ok found the problem, kinda, it's the triangles[i].. hmm

[Deco Da Man]

Why don't you do:

struct s_triangle {
    vec3 a, b, c;
}

?

[likesoursugar]

hmm, that would work, I will try that :) thanks

[editline]11:16AM[/editline]

or

struct s_triangle {
    vec3 vertice[3];
}

[Wyzard]

[QUOTE=likesoursugar;20205439]

	file->triangles->vertex = (struct vec3 *)
		malloc (sizeof (struct vec3) * file->header.num_verts);

[/QUOTE]

This doesn't look quite right. Since file->triangles is an array, you should be initializing something like file->triangles[i].vertex, not file->triangles->vertex.

file->triangles->vertex is the same as file->triangles[0].vertex. You're initializing the first triangle's vertex array, which is sufficient when there's only one triangle. Once you add a second triangle, though, you end up trying to store values into file->triangles[1].vertex, which is an invalid pointer since it hasn't been allocated yet. You need to use a loop a loop to allocate vertex arrays for [i]each[/i] triangle.

Is this C or C++, btw? If it's C, you don't need to explicitly cast the return value from malloc(). If it's C++, your triangle structure should be allocating its own vertex list in a constructor, and you should be using something like std::vector or at least new[] instead of malloc().

[likesoursugar]

[QUOTE]or at least new[] instead of malloc(). [/QUOTE]
yes I'm going that now

file->triangles = new s_triangle [file->header.num_tris];

that's working very well.
And i'm now doing something like:

struct s_triangle
{
	 vec3 vert[3];
};
//to set some values to the vertices
for(int i= 0; i < file->header.num_tris; i++)
	{
		for(int j= 0; j < 3; j++)
		{
			file->triangles[i].vert[j].x = 1.15;
			file->triangles[i].vert[j].y = -2.25;
			file->triangles[i].vert[j].z = 3.35;
		}
	}

and it's working :)

[Wyzard]

What's the point in initializing the vertex coordinates to arbitrary hard-coded values? You're (presumably) going to overwrite them later with real values, (otherwise your vertices would all be in the same place so there's nothing to draw,) so this step is unnecessary.

[likesoursugar]

[QUOTE=Wyzard;20211085]What's the point in initializing the vertex coordinates to arbitrary hard-coded values? You're (presumably) going to overwrite them later with real values, (otherwise your vertices would all be in the same place so there's nothing to draw,) so this step is unnecessary.[/QUOTE]

you're right, yes. it's not necessary, but I did this just to see that everything was ok.

[Wyzard]

Instead of storing a set of vertex coordinates for each triangle, you should consider storing the coordinates for [I]all[/I] the vertices in one big array, and then storing three vertex [I]numbers[/I] (indexes into the vertex array) in the triangle objects. That way you avoid storing each set of coordinates multiple times, since most vertices are part of more than one triangle.

This representation also translates easily into both [URL="http://www.songho.ca/opengl/gl_vbo.html"]OpenGL[/URL] and [URL="http://www.toymaker.info/Games/html/buffers.html"]Direct3D[/URL] vertex buffers, which are much more efficient than the [URL="http://www.devmaster.net/wiki/Immediate_mode"]immediate mode[/URL] rendering that you're probably using now.

[Deco Da Man]

I'm really familiar with the semantics of C/C++. Would this work?

struct s_triangle {
	vec3 vert[3];
}

// in code, later on

s_triangle triangles[NUM_TRIANGLES];

// loop through triangles here

vec3 tri_verts[NUM_TRIANGLES*3] = (vec3*)triangles

// use the tri_verts for stuff like OpenGL

I'm trying to learn myself :P

[likesoursugar]

[QUOTE=Wyzard;20242767]Instead of storing a set of vertex coordinates for each triangle, you should consider storing the coordinates for [I]all[/I] the vertices in one big array, and then storing three vertex [I]numbers[/I] (indexes into the vertex array) in the triangle objects. That way you avoid storing each set of coordinates multiple times, since most vertices are part of more than one triangle.

This representation also translates easily into both [URL="http://www.songho.ca/opengl/gl_vbo.html"]OpenGL[/URL] and [URL="http://www.toymaker.info/Games/html/buffers.html"]Direct3D[/URL] vertex buffers, which are much more efficient than the [URL="http://www.devmaster.net/wiki/Immediate_mode"]immediate mode[/URL] rendering that you're probably using now.[/QUOTE]

Yeah I thought about that :O will change my code abit and do some tests. I mean to create a real Vertex Meshes vertex mesh, I save power by doing this right? And it will be easier for me to create a map editor ( because sometimes I will need to move 2 vertices, but with mesh, just 1) I'm probably wrong.

[Wyzard]

[QUOTE=Deco Da Man;20246892]Would this work?[/QUOTE]
No, your initialization of tri_verts is syntactically invalid, and you're still storing vertex coordinates in each triangle. I meant something more like this:

#include <cstddef>  // for size_t

vec3 tri_verts[NUM_VERTICES];

struct s_triangle {
    size_t vert[3];  // Indexes into the tri_verts array
};

Or, better yet:

#include <vector>

std::vector<vec3> tri_verts;

class Triangle {
private:
    std::vector<vec3>::iterator vert[3];
public:
    // ...
};

[QUOTE=likesoursugar;20247561]And it will be easier for me to create a map editor ( because sometimes I will need to move 2 vertices, but with mesh, just 1)[/QUOTE]
Yes, it's easier to move a vertex in a mesh when you only have to change one set of coordinates in a vertex array, rather than looping through all your triangles and checking each one's vertex coordinates to find the ones that need to be modified.