-
Notifications
You must be signed in to change notification settings - Fork 311
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
Upgrade project for gltf-pipeline #7
Conversation
ab273c1
to
4052ef1
Compare
Should we do this soon? Is this often needed for OBJ models? |
@lasalvavida can you please test this? |
Here's a few test models: https://github.com/SaschaWillems/Vulkan/tree/master/models If any didn't work before this PR; it is OK if they don't work now; we just don't want to break ones that previously worked. |
@pjcozzi There are two obj models in that repo, chinesedragon and hammardillo and both convert correctly with these changes. @lilleyse I would advise perhaps just adding gltf-pipeline as a git dependency for now if that's possible so these changes can get merged and then updating once gltf-pipeline is in npm. |
Updated |
Thanks for the review, @lasalvavida. Please merge when ready. |
Read obj with streams and handle large buffers
This is also ready. |
@lasalvavida you should have commit access to this now to review and merge when ready. |
A made a bunch of structural changes so that more functionality is handled by the gltf-pipeline. The diff is a little crazy mostly due to indenting now that some callback stages are removed.
Changes:
modelMaterialCommon.js
. This is now handled in thegltf-pipeline
.convert.js
, which is the starting point for code that wants to useobj2gltf
as a libraryTODO