Shape indexing was broken when `TINYOBJ_FLAG_TRIANGULATE` flag was present.

[wendyn-projects]

I wanted to load a model I downloaded from this site. _If you want to try it on that model, note that some faces consist of like 16 vertices - necessitating larger value for TINYOBJ_MAX_FACES_PER_F_LINE._ The model is suppose to look something like this: . It works when I render whole vertex buffer but when I try to render individual shapes it looks really weird:

It turned out the number of faces for each shape is counted for each face command

if (commands[i].type == COMMAND_F) {
    face_count++;
}

However the number of face commands isn't the same as number of faces after we split some into triangles. Luckily the command also has some information about the face! So I figured I can just calculate how many triangles the face was split into and add that instead. Now I don't know what ½ of the variable names mean, but apparently commands[i].num_f_num_verts does the thing:

[syoyo]

👀 Let me give some time to review PR

[wendyn-projects]

Alright, I came up with minimal reproduction sample:

Expand to show the file

```obj v -1.0 -2.0 -1.0 v 1.0 -2.0 -1.0 v -1.0 -2.0 1.0 v 1.0 -2.0 1.0 v 1.0 0.0 -1.0 v -1.0 0.0 -1.0 v 1.0 0.0 1.0 v -1.0 0.0 1.0 v 1.0 2.0 -1.0 v -1.0 2.0 -1.0 v 1.0 2.0 1.0 v -1.0 2.0 1.0 g bottom f 2 1 3 4 g left_bottom f 1 6 8 3 g left_top f 6 10 12 8 g right_bottom f 2 5 7 4 g right_bottom f 5 9 11 7 g back f 3 4 11 12 g front f 1 2 9 10 g top_half1 f 9 11 10 g top_half2 f 12 11 10 ```

Expected result from tinyobj_shape_t **shapes:

bottom -       offset:  0, length: 2 faces
left_bottom -  offset:  2, length: 2 faces
left_top -     offset:  4, length: 2 faces
right_bottom - offset:  6, length: 2 faces
right_top -    offset:  8, length: 2 faces
back -         offset: 10, length: 2 faces
front -        offset: 12, length: 2 faces
tophalf1 -     offset: 14, length: 1 faces
tophalf2 -     offset: 15, length: 1 faces

(After triangulation there should be 16 faces in total)

Expand to show the file


Actual result from tinyobj_shape_t **shapes:

bottom -       offset:  0, length: 1 faces
left_bottom -  offset:  1, length: 1 faces
left_top -     offset:  2, length: 1 faces
right_bottom - offset:  3, length: 1 faces
right_top -    offset:  4, length: 1 faces
back -         offset:  5, length: 1 faces
front -        offset:  6, length: 1 faces
tophalf1 -     offset:  7, length: 1 faces
tophalf2 -     offset:  8, length: 1 faces

(but the number of face commands is counted instead)

Expand to show the file


[syoyo]

Awesome! Can I add it to regression test case?

[wendyn-projects]

Already ahead of you.

[wendyn-projects]

I ended up making a model with different combinations of triangles and quads to cover as many cases.

[syoyo]

@wendyn-projects Awesome! Thanks!

I still need to figure out what face_offset and length in tinyobj_shape_t mean. (It was never used in the example/test code)

  typedef struct {
    char *name; /* group name or object name. */
    unsigned int face_offset;
    unsigned int length;
  } tinyobj_shape_t;

Probably current tinyobj_shape_t is incomplete and it should be rewritten to align with the definition in C++ version: https://github.com/tinyobjloader/tinyobjloader/blob/cab4ad7254cbf7eaaafdb73d272f99e92f166df8/tiny_obj_loader.h#L358

[wendyn-projects]

I figured face_offset is suppose to point into tinyobj_attrib_t, it just breaks when you have a model with quads and use triangulation flag.

and if I look at mesh_t it stores: face indexes, vertex counts, material ids, smoothing, metadata which are all already already present in tinyobj_attrib_t (except smoothing and metadata).

should be rewritten to align with the definition in C++ version

What's missing is lines_t and points_t, but that would be just adding 2 more arrays into tinyobj_attrib_t and offsets/lengths into tinyobj_shape_t, no? Hence I don't think this PR would make it any more difficult.

I am just trying to fix a case where you have quads in a model and put up a triangulation flag. Also I found out this is an old issue.

that would be just adding 2 more arrays

I can give it a try if there is a chance of getting this added, what do you think?

[syoyo]

Good find > https://github.com/syoyo/tinyobjloader-c/issues/2#issuecomment-663948408

I think it'd be better first rewrite corresponding variable names to face_vertex_indices, face_vertex_counts, ... and face_offset should be renamed to face_vertex_count_offset or something(Array index to face_vertex_counts)