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

Associated MTL files should be optional #30

Closed
hammypants opened this issue Oct 22, 2020 · 6 comments
Closed

Associated MTL files should be optional #30

hammypants opened this issue Oct 22, 2020 · 6 comments

Comments

@hammypants
Copy link

To bring parity with tinyobjloader, Blender, etc.

@Twinklebear
Copy link
Owner

Twinklebear commented Oct 22, 2020

You're right, they should be. Which loading path are you using (from file or memory)? I must have got a bug in there that's making them required

@hammypants
Copy link
Author

Hi! From file.

I note that tinyobjloader will work if it can't find an associated mtl file (so the metadata in the obj says it exists), but it reports it chooses a default material instead of erroring.

I also note that I only post this via the understanding this is aiming to be a close-match to the way that lib functions. Feel free to disregard if there's a better way/this is not in line with another goal-- I just got surprised by a break.

@Twinklebear
Copy link
Owner

Cool, I'll take a look when I get some time (likely a week or so from now). Falling back to a default material if the MTL file isn't found is also the behavior I'd like, it's pretty common to not have one if you're just viewing some meshes for testing

@Twinklebear
Copy link
Owner

A bit longer than a few weeks, but let me know what you think of 8754343 . I'm not a fan of failing silently if we fail to load something, so what I've done instead of generating a default is to split the LoadResult up to separate the MTL load result from the OBJ file. This way the MTL file can fail to load and report the error on its own, without blowing up the entire OBJ file load. The application can then see the MTL file failed to load and generate an appropriate default material.

@Twinklebear
Copy link
Owner

Now in 3.0.0

@hammypants
Copy link
Author

A bit late on my response also. :) This works for me; I like your approach more than the way the C++ lib works here-- more control + more obvious. Thank you for taking the time to get to this.

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

No branches or pull requests

2 participants