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

Update FBXLoader #12004

Merged
merged 2 commits into from
Aug 22, 2017
Merged

Update FBXLoader #12004

merged 2 commits into from
Aug 22, 2017

Conversation

takahirox
Copy link
Collaborator

This PR updates FBXLoader

  • Seems like FBX can include unused materials which don't have any connections. Let's ignore them so far. This fixes FBXLoader failure #11973 loading failure issue
  • Switches default material from basic to standard. standard material is more 'standard' than basic material in Three.js now

@WestLangley
Copy link
Collaborator

Switches default material from basic to standard

Here's the thing... MeshStandardMaterial is designed to be used with an environment map so there is something to reflect. If there is no environment map, MeshPhongMaterial is likely better. If there is no indication of material shininess, then MeshLambertMaterial is preferable. All of these options assume lights in the scene.

But this is a judgement call... I would probably default to Lambert.

@takahirox
Copy link
Collaborator Author

The reason why I chose Standard is I saw a FBX file which includes materials the shader type of which are 'unknown' but look like PBR based materials (they have envMap, roughness, and others).

Of course, it'd be a minor case and I don't have any special preference.
If Lambert would be better, I'll change.

@mrdoob
Copy link
Owner

mrdoob commented Aug 22, 2017

Lets hope people use environment maps when using FBXLoader 😉

@mrdoob mrdoob merged commit 65dde62 into mrdoob:dev Aug 22, 2017
@mrdoob
Copy link
Owner

mrdoob commented Aug 22, 2017

Thanks!

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.

FBXLoader failure
3 participants