CesiumGS / obj2gltf

Convert OBJ assets to glTF
Apache License 2.0
1.71k stars 307 forks source link

Remove faces that don't match the same attribute layout #153

Closed lilleyse closed 6 years ago

lilleyse commented 6 years ago

Fixes two related problems where gltfs were rendering incorrectly because the number of uv attributes did not match the number of position attributes.

The first case is when two primitives in the same mesh use different types of attributes.

usemtl primitive1material
f 77 78 79 
f 77 80 78 
usemtl primitive2material
f 3665/2676 3666/2677 3634/2678 
f 3711/2679 3709/2680 3710/2681

This is fixed by storing attribute data on the primitive rather than the mesh. The downside to this is the vertex cache gets reset more often, meaning possibly more duplicate vertices. This won't affect most models, but it could be worse for those that are constantly interleaving usemtl lines.

The second case is when faces within a primitive use different types of attributes.

f 1/1/1 2/2/1 4/3/1 3/4/1
f 3 4 8 7
f 7/9/3 8/10/3 6/11/3 5/12/3

Most of the faces look like the first and third, but there would occasionally be faces like the second that don't match the attribute layout. These faces don't really make sense because they are missing uv coordinates and won't be textured properly anyways. Those faces get discarded now though I am thinking they should instead be assigned (0,0) uv coordinates.

mramato commented 6 years ago

Do we need to do a 1.x branch version of this PR as well, or is it something that can't easily be backported?

lilleyse commented 6 years ago

Yes still need to do the backport, which shouldn't look too different.

mramato commented 6 years ago

Works as advertised. Please up CHANGES

mramato commented 6 years ago

@lilleyse no rush, but don't forget about this.

lilleyse commented 6 years ago

@mramato updated.

mramato commented 6 years ago

Thanks!