Documenting the main data structures

[leino]

Hello!

This is a nice library, but I had a bit of trouble using it because I didn't quite understand what was contained in the main data structures.

In my opinion, that is the most important thing to document.

For instance: the members in 'tinyobj_attrib_t' struct could use some comments.
In particular, the members: 'num_faces', 'num_face_num_verts', 'faces' and 'face_num_verts'.
What are their relationships, and how they are affected by the TINYOBJ_FLAG_TRIANGULATE flag?

[syoyo]

This C89 version is currently experimental, so less document at this time.

API is mostly similar with C++ version, thus you'll find enough information about it there: https://github.com/syoyo/tinyobjloader
(You can send a documentation PR based on this!)

[raysan5]

I decided to use tinyobjloader-c in raylib to replace my old obj-loading implementation and I also had some trouble trying to understand attrib.num_faces and attrib.num_face_num_verts.

Loading a simple triangulated cube for reference I got:

attrib.num_vertices: 8
attrib.num_normals: 6
attrib.num_texcoords: 24
attrib.num_faces: 36
attrib.num_face_num_verts: 12

Vertex, texcoords and normals look ok to me but I have the feeling that attrib.num_faces and attrib.num_face_num_verts are swapped, is that possible?

[syoyo]

is that possible?

Possible.

Still, If you find an issue, please file an issue with reproducable code and .obj file.

[bbblitz]

Vertex, texcoords and normals look ok to me but I have the feeling that attrib.num_faces and attrib.num_face_num_verts are swapped, is that possible?

is that possible?

Possible.

Still, If you find an issue, please file an issue with reproducable code and .obj file.

It seems @andystanton suspects so too!

tinyobjloader-c/test/tinyobj_api_tests.c

Line 102 in a56a006

// TODO: should these two values be swapped?

I just ran into this myself too.

[mj-saunders]

I am also having some confusion determining what some of the main struct names actually mean.
If perhaps we can get some consensus, I don't mind making the edit and sorting a PR.

I'm going to do a little more comparison against the c++ version to see how their variables are named.

[mj-saunders]

The following are some descriptions as far as I can decipher.

---

Comparing against c++ version, I believe (c | c++):

face_num_verts == num_face_vertices

---

TINYOBJ_MAX_FACES_PER_F_LINE
I believe this is actually defining the max number of vertices per face, defined by one file line.
If I correctly understand the file format, there can only be one face per F line.
But the "triangulate" flag will cause the code to convert polygons to triangle fans.
(Crude example): they would convert a quad defined as "F 1 2 3 4" - to two triangles, as though they were defined as "F 1 2 3"; and "F 1 3 4".

tinyobj_vertex_index_t
This struct holds three indices.
One for each of three arrays holding vertices, vertex texture coordinates and vertex normals.

---

parseLine() Function

num_f
The number of vertices that get parsed from the line (3 or more; triangle vs poly).
It might be more clear to think of this variable as number of indices, or according to the file format description, "face elements".

f_num_verts
The number of vertices for a given face (always 3 when triangulated).
This becomes face_num_verts in attrib struct.

num_f_num_verts
Number of faces defined by the indices of one parsed line. Being an array allows it to handle converting polygons to a triangle fan.

---

tinyobj_parse_obj() Function

attrib->num_face_num_verts
I'm not sure if this should be num_f or num_f_num_verts.
Given the naming, I would assume the latter. But also given the naming, I'm not sure what it is for.

---

So, yes the code needs a patch. A couple of lines should read as follows.

1378: attrib->num_faces = (unsigned int)num_faces;
1381: attrib->num_face_num_verts = (unsigned int)num_f; // or num_f_num_verts ?

I think the other issues are purely with regard to variable names being more clear.

[syoyo]

@mj-saunders Awesome! Let me give some time to review your description.

[mj-saunders]

@syoyo No problem. Hopefully what I've written is actually correct ;)

[mj-saunders]

Also, I have not confirmed this, but I have a feeling that face_count in section 5 of tinyobj_parse_obj() (Construct shape information) will not match up to the correct array index for triangulated meshes.

It only counts F "commands" (lines in the obj file beginning with f), whereas triangulation will have caused extra faces to be added to the final array.

Please correct me if I have just misunderstood something.

[syoyo]

Firstly, current naming of variables are confusing #16 so I am planning to rename it.

I'd like to follow USD's naming: https://graphics.pixar.com/usd/docs/api/class_usd_geom_mesh.html

• FaceVertexIndices(faces): 1D linearlized array of vertex indices. e.g. [0, 1, 2, 0, 2, 3]
• FaceVertexCounts(face_num_verts): The number of vertices in a face. e.g. [3, 3]
• NumFaces(num_f): The number of faces. This equals to len(FaceVertexCounts)

Then things will be much cleaner(and need to rewrie the description)