-
Notifications
You must be signed in to change notification settings - Fork 252
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
Export public API via index.js #157
Changes from 5 commits
5524a73
863be07
0f6298d
da16e7b
ef85de6
6b66d6f
0b22358
a880aab
8d43261
c58e5de
545fce1
c5bf895
4bf4745
c752a1a
0c9d382
5687191
d3c8e01
3395a4f
a64d119
9f292b5
404401b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,59 @@ | ||
module.exports = { | ||
gltfPipeline : require('./lib/gltfPipeline'), | ||
readGltf : require('./lib/readGltf'), | ||
writeBinaryGltf : require('./lib/writeBinaryGltf'), | ||
writeGltf : require('./lib/writeGltf') | ||
}; | ||
AccessorReader : require('./lib/AccessorReader.js'), | ||
addCesiumRTC : require('./lib/addCesiumRTC.js'), | ||
addDefaults : require('./lib/addDefaults.js'), | ||
addExtensionsUsed : require('./lib/addExtensionsUsed.js'), | ||
addPipelineExtras : require('./lib/addPipelineExtras.js'), | ||
bakeAmbientOcclusion : require('./lib/bakeAmbientOcclusion.js'), | ||
byteLengthForComponentType : require('./lib/byteLengthForComponentType.js'), | ||
cacheOptimization : require('./lib/cacheOptimization.js'), | ||
combineMeshes : require('./lib/combineMeshes.js'), | ||
combineNodes : require('./lib/combineNodes.js'), | ||
compressIntegerAccessors : require('./lib/compressIntegerAccessors.js'), | ||
compressTextureCoordinates : require('./lib/compressTextureCoordinates.js'), | ||
convertDagToTree : require('./lib/convertDagToTree.js'), | ||
encodeImages : require('./lib/encodeImages.js'), | ||
findAccessorMinMax : require('./lib/findAccessorMinMax.js'), | ||
generateNormals : require('./lib/generateNormals.js'), | ||
getAccessorByteStride : require('./lib/getAccessorByteStride.js'), | ||
getBinaryGltf : require('./lib/getBinaryGltf.js'), | ||
getUniqueId : require('./lib/getUniqueId.js'), | ||
gltfPipeline : require('./lib/gltfPipeline.js'), | ||
loadGltfUris : require('./lib/loadGltfUris.js'), | ||
mergeBuffers : require('./lib/mergeBuffers.js'), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I just noticed that there isn't doc for this one. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Never mind... |
||
mergeDuplicateAccessors : require('./lib/mergeDuplicateAccessors.js'), | ||
mergeDuplicateVertices : require('./lib/mergeDuplicateVertices.js'), | ||
NodeHelpers : require('./lib/NodeHelpers.js'), | ||
numberOfComponentsForType : require('./lib/numberOfComponentsForType.js'), | ||
octEncodeNormals : require('./lib/octEncodeNormals.js'), | ||
packArray : require('./lib/packArray.js'), | ||
parseBinaryGltf : require('./lib/parseBinaryGltf.js'), | ||
quantizeAttributes : require('./lib/quantizeAttributes.js'), | ||
readAccessor : require('./lib/readAccessor.js'), | ||
readBufferComponentType : require('./lib/readBufferComponentType.js'), | ||
readGltf : require('./lib/readGltf.js'), | ||
removeDuplicatePrimitives : require('./lib/removeDuplicatePrimitives.js'), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And doc for this. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Never mind... |
||
removePipelineExtras : require('./lib/removePipelineExtras.js'), | ||
removeUnused : require('./lib/removeUnused.js'), | ||
removeUnusedAccessors : require('./lib/removeUnusedAccessors.js'), | ||
removeUnusedBuffers : require('./lib/removeUnusedBuffers.js'), | ||
removeUnusedBufferViews : require('./lib/removeUnusedBufferViews.js'), | ||
removeUnusedCameras : require('./lib/removeUnusedCameras.js'), | ||
removeUnusedImages : require('./lib/removeUnusedImages.js'), | ||
removeUnusedMaterials : require('./lib/removeUnusedMaterials.js'), | ||
removeUnusedMeshes : require('./lib/removeUnusedMeshes.js'), | ||
removeUnusedNodes : require('./lib/removeUnusedNodes.js'), | ||
removeUnusedPrimitiveAttributes : require('./lib/removeUnusedPrimitiveAttributes.js'), | ||
removeUnusedPrograms : require('./lib/removeUnusedPrograms.js'), | ||
removeUnusedSamplers : require('./lib/removeUnusedSamplers.js'), | ||
removeUnusedShaders : require('./lib/removeUnusedShaders.js'), | ||
removeUnusedSkins : require('./lib/removeUnusedSkins.js'), | ||
removeUnusedTechniques : require('./lib/removeUnusedTechniques.js'), | ||
removeUnusedTextures : require('./lib/removeUnusedTextures.js'), | ||
removeUnusedVertices : require('./lib/removeUnusedVertices.js'), | ||
uninterleaveAndPackBuffers : require('./lib/uninterleaveAndPackBuffers.js'), | ||
writeAccessor : require('./lib/writeAccessor.js'), | ||
writeBinaryGltf : require('./lib/writeBinaryGltf.js'), | ||
writeBufferComponentType : require('./lib/writeBufferComponentType.js'), | ||
writeGltf : require('./lib/writeGltf.js') | ||
}; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The main problem with exposing a lot of these functions is that they may rely on previous steps, like There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would it be enough to just put the dependencies in the doc until we work out custom pipelines? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah I think that's ok for now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you sure you want to expose all of this? That is a lot of API surface area. We learned a very hard lesson in Cesium to only expose what is needed to keep the maintain cost way down.
Please only expose what is currently needed and make sure the public API has full reference doc that can be generated like we do for Cesium. Don't worry about the generated doc's styling or hosting right now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's still way too much API surface area here. @lilleyse and @lasalvavida can you please take a pass over this?
I can help if needed, but I would rather you guys get the experience working with APIs. At this point, I would only expose top-level stages and helpers that @likangning93 needs. Anything else is a high-cost loan for money we don't need.