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

Export public API via index.js #157

Merged
merged 21 commits into from
Aug 24, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,16 @@ Change Log

### Next Release

* cacheOptimization no longer crashes on primitives without indices. [#154](https://github.com/AnalyticalGraphicsInc/gltf-pipeline/issues/154)

* `cacheOptimization` no longer crashes on primitives without indices. [#154](https://github.com/AnalyticalGraphicsInc/gltf-pipeline/issues/154)
* Public API is exposed via `index.js` [#153](https://github.com/AnalyticalGraphicsInc/gltf-pipeline/issues/153)
* Documentation has been added for all exposed functions.
* `OptimizationStats` is removed from `removeUnused` stages.
* `gltfPipeline.js` is now named `Pipeline.js`.
* `bakeAmbientOcclusion.js` now directly exports the `bakeAmbientOcclusion` function.
* `bakeAmbientOcclusion` now takes a glTF asset as its first parameter to match the function signature of other stages.
* All `removeUnused` stages have been consolidated to `RemoveUnusedProperties` to clean up the global scope.
* `readBufferComponentType` and `writeBufferComponentType` have been renamed to `readBufferComponent` and `writeBufferComponent` respectively.

### 0.1.0-alpha3 - 2016-07-25

* Converted the API to now use promises instead of callbacks. [#135](https://github.com/AnalyticalGraphicsInc/gltf-pipeline/pull/135)
Expand Down
15 changes: 12 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
[![Coverage Status](https://coveralls.io/repos/AnalyticalGraphicsInc/gltf-pipeline/badge.svg?branch=master)](https://coveralls.io/r/AnalyticalGraphicsInc/gltf-pipeline?branch=master)

<p align="center">
<a href="https://www.khronos.org/gltf"><img src="doc/gltf.png" /></a>
<a href="https://www.khronos.org/gltf"><img src="doc/gltf.png" onerror="this.src='gltf.png'"/></a>
</p>

Content pipeline tools for optimizing [glTF](https://www.khronos.org/gltf) assets by [Richard Lee](http://leerichard.net/) and the [Cesium team](http://cesiumjs.org/).
Expand Down Expand Up @@ -60,7 +60,7 @@ To run JSHint automatically when a file is saved, run the following and leave it
npm run jsHint-watch
```

### Running test coverage
### Running Test Coverage

Coverage uses [istanbul](https://github.com/gotwarlost/istanbul). Run:
```
Expand All @@ -70,6 +70,15 @@ For complete coverage details, open `coverage/lcov-report/index.html`.

The tests and coverage covers the Node.js module; it does not cover the command-line interface, which is tiny.

## Generating Documentation

To generate the documentation:
```
npm run jsdoc
```

The documentation will be placed in the `doc` folder.

### Debugging

* To debug the tests in Webstorm, open the Gulp tab, right click the `test` task, and click `Debug 'test'`.
Expand All @@ -82,5 +91,5 @@ Pull requests are appreciated! Please use the same [Contributor License Agreeme
---

<p align="center">
<a href="http://cesiumjs.org/"><img src="doc/cesium.png" /></a>
<a href="http://cesiumjs.org/"><img src="doc/cesium.png" onerror="this.src='cesium.png'"/></a>
</p>
10 changes: 7 additions & 3 deletions bin/gltf-pipeline.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@ var path = require('path');
var defaultValue = Cesium.defaultValue;
var defined = Cesium.defined;

var gltfPipeline = require('../lib/gltfPipeline');
var Pipeline = require('../lib/Pipeline');

var processFileToDisk = gltfPipeline.processFileToDisk;
var processFileToDisk = Pipeline.processFileToDisk;

if (process.argv.length < 3 || defined(argv.h) || defined(argv.help)) {
var help =
Expand Down Expand Up @@ -72,5 +72,9 @@ var options = {
optimizeForCesium : optimizeForCesium
};

console.time('optimize');
// Node automatically waits for all promises to terminate
processFileToDisk(gltfPath, outputPath, options);
processFileToDisk(gltfPath, outputPath, options)
.then(function() {
console.timeEnd('optimize');
});
2 changes: 1 addition & 1 deletion gulpfile.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ var environmentSeparator = process.platform === 'win32' ? ';' : ':';
var nodeBinaries = path.join(__dirname, 'node_modules', '.bin');
process.env.PATH += environmentSeparator + nodeBinaries;

var jsHintFiles = ['**/*.js', '!node_modules/**', '!coverage/**'];
var jsHintFiles = ['**/*.js', '!node_modules/**', '!coverage/**', '!doc/**'];
var specFiles = ['**/*.js', '!node_modules/**', '!coverage/**'];

gulp.task('jsHint', function () {
Expand Down
41 changes: 39 additions & 2 deletions index.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,43 @@
module.exports = {
gltfPipeline : require('./lib/gltfPipeline'),
AccessorReader : require('./lib/AccessorReader'),
Copy link
Contributor

Choose a reason for hiding this comment

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

In there a reason we just don't just module.exports = require('./lib'); here instead of requiring every object individually?

Copy link
Contributor

Choose a reason for hiding this comment

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

There are some files we don't export.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think manually maintaining this list could be error prone and easy to forget about. Some other options would be to have a private folder at the same level of lib to make the decision more conscious during development; or you could just export everything and mark private objects @private.

That being said, I'm okay if you go the manual route until it becomes a pain.

addCesiumRTC : require('./lib/addCesiumRTC'),
addDefaults : require('./lib/addDefaults'),
addExtensionsUsed : require('./lib/addExtensionsUsed'),
addPipelineExtras : require('./lib/addPipelineExtras'),
bakeAmbientOcclusion : require('./lib/bakeAmbientOcclusion'),
byteLengthForComponentType : require('./lib/byteLengthForComponentType'),
combineMeshes : require('./lib/combineMeshes'),
combineNodes : require('./lib/combineNodes'),
compressIntegerAccessors : require('./lib/compressIntegerAccessors'),
compressTextureCoordinates : require('./lib/compressTextureCoordinates'),
convertDagToTree : require('./lib/convertDagToTree'),
encodeImages : require('./lib/encodeImages'),
findAccessorMinMax : require('./lib/findAccessorMinMax'),
generateNormals : require('./lib/generateNormals'),
getAccessorByteStride : require('./lib/getAccessorByteStride'),
getBinaryGltf : require('./lib/getBinaryGltf'),
getUniqueId : require('./lib/getUniqueId'),
loadGltfUris : require('./lib/loadGltfUris'),
mergeBuffers : require('./lib/mergeBuffers'),
mergeDuplicateAccessors : require('./lib/mergeDuplicateAccessors'),
mergeDuplicateVertices : require('./lib/mergeDuplicateVertices'),
numberOfComponentsForType : require('./lib/numberOfComponentsForType'),
octEncodeNormals : require('./lib/octEncodeNormals'),
optimizeForVertexCache : require('./lib/optimizeForVertexCache'),
packArray : require('./lib/packArray'),
parseBinaryGltf : require('./lib/parseBinaryGltf'),
Pipeline : require('./lib/Pipeline'),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is this correct? Shouldn't the capitalization put this first?

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, I didn't know that was a common thing to do. If so I'll correct it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mramato, I can't find a point of reference for this. Which way should this go?

Copy link
Contributor

Choose a reason for hiding this comment

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

It doesn't really matter in this case. Since this won't be auto-sorted, whatever you guys want to do is fine.

quantizeAttributes : require('./lib/quantizeAttributes'),
readAccessor : require('./lib/readAccessor'),
readBufferComponent : require('./lib/readBufferComponent'),
readGltf : require('./lib/readGltf'),
removeDuplicatePrimitives : require('./lib/removeDuplicatePrimitives'),
removePipelineExtras : require('./lib/removePipelineExtras'),
RemoveUnusedProperties : require('./lib/RemoveUnusedProperties'),
removeUnusedVertices : require('./lib/removeUnusedVertices'),
uninterleaveAndPackBuffers : require('./lib/uninterleaveAndPackBuffers'),
writeAccessor : require('./lib/writeAccessor'),
writeBinaryGltf : require('./lib/writeBinaryGltf'),
writeBufferComponent : require('./lib/writeBufferComponent'),
writeGltf : require('./lib/writeGltf')
};
};
Copy link
Contributor

Choose a reason for hiding this comment

The 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 addPipelineExtras. Either we need to handle that in the functions themselves, or expect users to call things like addPipelineExtras, loadGltfUris, etc on their own.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah I think that's ok for now.

50 changes: 46 additions & 4 deletions lib/AccessorReader.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,24 @@ var defaultValue = Cesium.defaultValue;
var byteLengthForComponentType = require('./byteLengthForComponentType');
var getAccessorByteStride = require('./getAccessorByteStride');
var numberOfComponentsForType = require('./numberOfComponentsForType');
var readBufferComponentType = require('./readBufferComponentType');
var writeBufferComponentType = require('./writeBufferComponentType');
var readBufferComponent = require('./readBufferComponent');
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch, I used the JetBrains refactor tool and it must have missed those.

var writeBufferComponent = require('./writeBufferComponent');

module.exports = AccessorReader;

/**
* Reads an accessor incrementally. This is useful to keep overhead low when
* memory consumption is important.
*
* The glTF asset must be initialized for the pipeline.
*
* @param {Object} gltf A javascript object containing a glTF asset.
* @param {Object} accessor The accessor object from glTF to read.
* @constructor
*
* @see addPipelineExtras
* @see loadGltfUris
*/
function AccessorReader(gltf, accessor) {
var bufferViews = gltf.bufferViews;
var buffers = gltf.buffers;
Expand All @@ -30,13 +43,21 @@ function AccessorReader(gltf, accessor) {
this.source = buffer.extras._pipeline.source;
}

/**
* Read data at the current index into components.
* Data will be read into components starting at index 0.
* Components will be grown if the number of components for the accessor is greater than the array size.
*
* @param {Array} components The array to read the data into.
* @returns {Array} components with data written into it.
*/
AccessorReader.prototype.read = function(components) {
if (this.index >= this.count) {
return undefined;
}
var componentByteLength = byteLengthForComponentType(this.componentType);
for (var i = 0; i < this.numberOfComponents; i++) {
var data = readBufferComponentType(this.source,
var data = readBufferComponent(this.source,
this.componentType,
this.byteOffset + (this.byteStride * this.index) + (componentByteLength * i)
);
Expand All @@ -49,6 +70,16 @@ AccessorReader.prototype.read = function(components) {
return components;
};

/**
* Write data at the current index into the accessor data.
* Specifying a componentType different from the accessor's componentType can be done for a use case like attribute compression.
* The data is being read, compressed, and then written back as a different, smaller data type. This is done in place
* and then the gaps can be removed using uninterleaveAndPackBuffers.
*
* @param {Array} data The data to write into the accessor.
* @param {Number} [componentType=this.componentType] The component type to use for writing the data.
* @param {Number} [dataOffset=0] Read starting from a different position in the data array.
*/
AccessorReader.prototype.write = function(data, componentType, dataOffset) {
componentType = defaultValue(componentType, this.componentType);
dataOffset = defaultValue(dataOffset, 0);
Expand All @@ -58,22 +89,33 @@ AccessorReader.prototype.write = function(data, componentType, dataOffset) {
byteStride = byteLengthForComponentType(componentType) * this.numberOfComponents;
}
for (var i = 0; i < this.numberOfComponents; i++) {
writeBufferComponentType(this.source,
writeBufferComponent(this.source,
componentType,
data[i + dataOffset],
this.byteOffset + (byteStride * this.index) + (componentByteLength * i)
);
}
};

/**
* Get if the AccessorReader is at the end of the accessor data.
*
* @returns {Boolean} True if there is more data to read, false if not.
*/
AccessorReader.prototype.hasNext = function() {
return this.index < this.count;
};

/**
* Increment AccessorReader.index by one.
*/
AccessorReader.prototype.next = function() {
this.index++;
};

/**
* Set AccessorReader.index to zero.
*/
AccessorReader.prototype.reset = function() {
this.index = 0;
};
34 changes: 18 additions & 16 deletions lib/GeometryMath.js
Original file line number Diff line number Diff line change
@@ -1,16 +1,18 @@
'use strict';
var Cesium = require('cesium');

var Cartesian3 = Cesium.Cartesian3;
var defined = Cesium.defined;
var DeveloperError = Cesium.DeveloperError;

module.exports = {
sumCartesian3sBarycentric: sumCartesian3sBarycentric,
flattenTriangle: flattenTriangle,
pointLineDistanceParametric: pointLineDistanceParametric
};
module.exports = GeometryMath;

// Functions for working with geometry like triangles and lines
/**
* Functions for working with geometry, like triangles and lines.
* @constructor
* @private
*/
function GeometryMath() {}

var sumBarycentricScratch = new Cartesian3();
/**
Expand All @@ -23,7 +25,7 @@ var sumBarycentricScratch = new Cartesian3();
* @param {Cartesian3} result The object onto which to store the result.
* @returns {Cartesian3} The modified result parameter.
*/
function sumCartesian3sBarycentric(barycentric, vector0, vector1, vector2, result) {
GeometryMath.sumCartesian3sBarycentric = function(barycentric, vector0, vector1, vector2, result) {
if (!defined(barycentric)) {
throw new DeveloperError('barycentric is required');
}
Expand All @@ -50,7 +52,7 @@ function sumCartesian3sBarycentric(barycentric, vector0, vector1, vector2, resul
Cartesian3.multiplyByScalar(vector2, barycentric.z, sumBarycentricScratch);
Cartesian3.add(result, sumBarycentricScratch, result);
return result;
}
};

var pointOnLineScratch = new Cartesian3();
/**
Expand All @@ -62,7 +64,7 @@ var pointOnLineScratch = new Cartesian3();
* @param {Cartesian3} lineDirection A direction specifying the line's orientation.
* @returns {Number} The parametric position on the line closest to the given point.
*/
function pointLineDistanceParametric(point, linePosition, lineDirection) {
GeometryMath.pointLineDistanceParametric = function(point, linePosition, lineDirection) {
if (!defined(point)) {
throw new DeveloperError('point is required');
}
Expand All @@ -78,7 +80,7 @@ function pointLineDistanceParametric(point, linePosition, lineDirection) {
var p3 = point;
var length2 = Cartesian3.magnitudeSquared(lineDirection);
return ((p3.x - p1.x) * (p2.x - p1.x) + (p3.y - p1.y) * (p2.y - p1.y) + (p3.z - p1.z) * (p2.z - p1.z)) / length2;
}
};

var xAxis = new Cartesian3();
var zAxis = new Cartesian3();
Expand All @@ -90,7 +92,7 @@ var yAxis = new Cartesian3();
* @param {Cartesian2[]} results The objects onto which to store the results.
* @returns {Cartesian2[]} The triangle's vertex positions in its own plane.
*/
function flattenTriangle(positions, results) {
GeometryMath.flattenTriangle = function(positions, results) {
if (!defined(positions)) {
throw new DeveloperError('positions is required');
}
Expand All @@ -111,10 +113,10 @@ function flattenTriangle(positions, results) {
// Compute x/y coordinates by getting distance to normalized x and y axes in 3D
results[0].x = 0.0;
results[0].y = 0.0;
results[1].x = pointLineDistanceParametric(positions[1], positions[0], xAxis);
results[1].y = pointLineDistanceParametric(positions[1], positions[0], yAxis);
results[2].x = pointLineDistanceParametric(positions[2], positions[0], xAxis);
results[2].y = pointLineDistanceParametric(positions[2], positions[0], yAxis);
results[1].x = GeometryMath.pointLineDistanceParametric(positions[1], positions[0], xAxis);
results[1].y = GeometryMath.pointLineDistanceParametric(positions[1], positions[0], yAxis);
results[2].x = GeometryMath.pointLineDistanceParametric(positions[2], positions[0], xAxis);
results[2].y = GeometryMath.pointLineDistanceParametric(positions[2], positions[0], yAxis);

return results;
}
};
Loading