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

GLTFExporter Doc: Update for new options #13433

Merged
merged 4 commits into from
Feb 25, 2018

Conversation

takahirox
Copy link
Collaborator

This PR updates GLTFExporter doc for new options and fixes a minor stuff.

@@ -80,7 +80,9 @@ <h3>[method:null parse]( [param:Object input], [param:Function onCompleted], [pa
<li>truncateDrawRange - bool. Export just the attributes within the drawRange, if defined, instead of exporting the whole array. Default is true.</li>
<li>binary - bool. Export in binary (.glb) format, returning an ArrayBuffer. Default is false.</li>
<li>embedImages - bool. Export with images embedded into the glTF asset. Default is true.</li>
<li>animations - Array<[page:AnimationClip AnimationClip]>. List of animations to be included in the export.
<li>animations - Array<[page:AnimationClip AnimationClip]>. List of animations to be included in the export.</li>
<li>forceIndices - bool. Generate indices for non-index geometry and export with it. Default is false.</li>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not sure about the wording here, maybe "with them"? Or Export non-indexed geometry by creating temporary indices for it? /cc @mrdoob @donmccurdy

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks, Replaced "it" with "them" so far.

@mrdoob
Copy link
Owner

mrdoob commented Feb 25, 2018

I don't think I would document these. As they're just temporal workarounds for Facebook 3D posts...?

@takahirox
Copy link
Collaborator Author

Will we remove them once FB fixes the limitations?

@mrdoob
Copy link
Owner

mrdoob commented Feb 25, 2018

I hope so. At least the forceIndices one, as that's basically a hack.

@fernandojsg
Copy link
Collaborator

I believe forcePOT could stay, because some engines, specially on mobile maybe could still have that limitation.
And regarding non-indexed to indexed it's true that it won't be useful probably after facebook will implement it, but I would just keep it documented until we'll remove from the code, and once we'll remove from the code, remove also the documentation for the sake of consistency

@takahirox
Copy link
Collaborator Author

I wanna add note "These're temporal workarounds for FB" to Doc (or in the code) rather than non documenting.

@mrdoob
Copy link
Owner

mrdoob commented Feb 25, 2018

Okay, lets remove when they're not needed then.

@mrdoob mrdoob merged commit c6d275d into mrdoob:dev Feb 25, 2018
@mrdoob mrdoob added this to the r91 milestone Feb 25, 2018
@mrdoob
Copy link
Owner

mrdoob commented Feb 25, 2018

Thanks!

@takahirox takahirox deleted the GLTFExporterDocUpdate branch February 27, 2018 16:54
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.

3 participants