Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Huge speed-up for loading meshes with loadObj function. #51

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

ferreram
Copy link

Added hash function for Vertex / TexturableVertex. Fix errors in TexturableVertex struct (uv init with glm::vec3 & tangent data not checked for equality operator). Simplify default constructor of Vertex / TexturableVertex & equality operator of those & Face struct.

…nction for Vertex / TexturableVertex. Fix errors in TexturableVertex struct (uv init with glm::vec3 & tangent data not checked for equality operator). Simplify default constructor of Vertex / TexturableVertex & equality operator of those & Face struct.
@patrykcieslak
Copy link
Owner

Hello @ferreram ! I appreciate your contribution but sadly something is not working well :(
When I tried to merge it to the master and run some simulations, the meshes are messed up completely (see screenshot).
I export meshes using Blender as triangulated and with normals and UV coordinates.
obj_loading_errors

@ferreram
Copy link
Author

Hi @patrykcieslak. Sorry for the inconvenience, I forgot a minus one to access vector elements from the IDs of vertices / normals / uvs in the obj files. It should properly work now.

Please, note that the speed-up is mostly significant on big models.

My results from running a few tests:

  • model 1: 46895 vertices and 46895 normals -> before: 0.056 secs / after: 0.069 secs
  • model 2: 150561 vertices and 150561 normals -> before: 3.1 secs / after: 0.51 secs
  • model 3: 451371 vertices and 601556 normals -> before: 4536.23 secs / after: 1.5 secs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants