search

thisistherk / fast_obj

Fast C OBJ parser
MIT License
632 stars 48 forks source link

Proposal: move material texture maps into fastObjMesh #36

Closed laurelkeys closed 5 months ago

laurelkeys commented 2 years ago

This PR replaces the fastObjTexture maps in fastObjMaterial with unsigned int indices (which index into a newly added fastObjMesh::textures array).

With this, all texture maps used in an OBJ file are grouped together and directly accessible from a fastObjMesh, making it really straight-forward to load them (i.e. without having to traverse the fastObjMesh::materials array checking all textures maps for each material, and where materials often contain duplicate texture maps between themselves).

It also makes the sizeof(fastObjMaterial) smaller, and concentrates/minimizes string allocations, but a downside is that read_map will now do string comparisons.

laurelkeys commented 2 years ago

P.S.: I've bumped the version to 1.3 since this would lead to a breaking change.

thisistherk commented 2 years ago

Thanks! Let me have a think about this - I think it's probably a better way of doing things, but slightly worried by the API change, mostly because I've never written any proper docs, so communicating the change to people is a pain!

kodokaii commented 1 year ago

I 100% approve of this change!

thisistherk commented 1 year ago

Woah, sorry, thanks for the prod. Completely forgot to get back to this! I'll try and make some time in the next couple of weeks to get this in - could do with a 1.3 release anyway for recent changes, and I can explain the diffs in the release notes.

laurelkeys commented 10 months ago

Fixed conflicts with https://github.com/thisistherk/fast_obj/pull/45

thisistherk commented 5 months ago

Thank you! Sorry this took so long!