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

Write gltf changes #95

Merged
merged 1 commit into from
Jun 20, 2016
Merged

Write gltf changes #95

merged 1 commit into from
Jun 20, 2016

Conversation

lilleyse
Copy link
Contributor

A few random fixes and changes related to writing gltfs.

@@ -30,7 +30,7 @@ function updateBinaryObject(gltf, pipelineExtras, name, state) {
state.currentBinaryView++;
}

var objectSource = object.extras._pipeline.source;
var objectSource = new Buffer(object.extras._pipeline.source);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Buffer.concat below didn't like concatting a shader string with a buffer, so I just always wrap the source in a Buffer.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.2%) to 93.93% when pulling e5c8eea on write-gltf-changes into c86e629 on master.

}
mkdirp.sync(path.dirname(outputPath));
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change includes all the deleted specs. I thought it was weird that the pipeline won't export a gltf file unless createDirectory is true, especially since there are cases where you don't want the file to be put in an output folder.

@pjcozzi
Copy link
Contributor

pjcozzi commented Jun 18, 2016

@lasalvavida do you want to review this?

@lasalvavida
Copy link
Contributor

These changes look good to me.

@lasalvavida lasalvavida merged commit 509f37f into master Jun 20, 2016
@lasalvavida lasalvavida deleted the write-gltf-changes branch June 20, 2016 13:51
JoshuaStorm pushed a commit that referenced this pull request Jun 24, 2016
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.

4 participants