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

Add mergeVertices option and disable by default #276

Merged
merged 6 commits into from
May 9, 2017
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
8 changes: 7 additions & 1 deletion lib/Pipeline.js
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,8 @@ Pipeline.processJSON = function(gltf, options) {
* @param {Object} [options.compressTextureCoordinates=false] Flag to run compressTextureCoordinates stage.
* @param {Object} [options.kmcOptions=undefined] Options to pass to the generateModelMaterialsCommon stage, if undefined, stage is not run.
* @param {Object} [options.quantize] Flag to run quantizeAttributes stage.
* @param {Object} [options.preserve=false] Flag to turn optimization pipeline stages on/off to preserve the original glTF hierarchy.
* @param {Boolean} [options.mergeVertices=false] Flag to merge duplicate vertices, which can produce a smaller model size but greatly increases conversion time. This setting only applies when options.preserve is false.
* @param {Object|Object[]} [options.textureCompressionOptions=undefined] Options to pass to the compressTextures stage. If an array of options is given, the textures will be compressed in multiple formats. If undefined, stage is not run.
* @returns {Promise} A promise that resolves to the processed glTF asset.
*/
Expand Down Expand Up @@ -111,8 +113,12 @@ Pipeline.processJSONWithExtras = function(gltfWithExtras, options) {
if (smoothNormals || faceNormals) {
generateNormals(gltfWithExtras, options);
}

var mergeVertices = defaultValue(options.mergeVertices, false);
if (!shouldPreserve) {
mergeDuplicateVertices(gltfWithExtras);
if (mergeVertices) {
mergeDuplicateVertices(gltfWithExtras);
}
MergeDuplicateProperties.mergeAll(gltfWithExtras);
RemoveUnusedProperties.removeAll(gltfWithExtras);
removeDuplicatePrimitives(gltfWithExtras);
Expand Down
6 changes: 5 additions & 1 deletion lib/mergeDuplicateVertices.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,10 @@ module.exports = mergeDuplicateVertices;
* @see removeUnusedVertices
*/
function mergeDuplicateVertices(gltf) {
return mergeDuplicateVertices._implementation(gltf);
}

mergeDuplicateVertices._implementation = function (gltf) {
var meshes = gltf.meshes;
var indexAccessors = {};
for (var meshId in meshes) {
Expand All @@ -37,7 +41,7 @@ function mergeDuplicateVertices(gltf) {
mergeDuplicateVerticesFromMapping(gltf, indexAccessors);
removeUnusedVertices(gltf);
return gltf;
}
};

function mergeDuplicateVerticesFromMapping(gltf, indexAccessors) {
var accessors = gltf.accessors;
Expand Down
9 changes: 7 additions & 2 deletions lib/parseArguments.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,10 @@ function parseArguments(args) {
describe: 'Compress the testure coordinates of this glTF asset.',
type: 'boolean'
},
'mergeVertices': {
describe: 'Merges duplicate vertices, which can produce a smaller model size but greatly increases conversion time. This setting only applies when preserve is false.',
type: 'boolean'
},
'removeNormals': {
alias: 'r',
describe: 'Strips off existing normals, allowing them to be regenerated.',
Expand Down Expand Up @@ -261,6 +265,7 @@ function parseArguments(args) {
preserve: argv.p,
quantize: argv.q,
removeNormals: argv.r,
textureCompressionOptions: textureCompressionOptions
textureCompressionOptions: textureCompressionOptions,
mergeVertices: argv.mergeVertices
};
}
}
26 changes: 26 additions & 0 deletions specs/lib/PipelineSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,15 @@ var fsExtra = require('fs-extra');
var path = require('path');
var Promise = require('bluebird');

var mergeDuplicateVertices = require('../../lib/mergeDuplicateVertices');
var Pipeline = require('../../lib/Pipeline');
var readGltf = require('../../lib/readGltf');

var processFile = Pipeline.processFile;
var processFileToDisk = Pipeline.processFileToDisk;
var processJSON = Pipeline.processJSON;
var processJSONToDisk = Pipeline.processJSONToDisk;
var processJSONWithExtras = Pipeline.processJSONWithExtras;

var fsExtraReadFile = Promise.promisify(fsExtra.readFile);

Expand Down Expand Up @@ -159,4 +161,28 @@ describe('Pipeline', function() {
expect(initialUri).not.toEqual(finalUri);
}), done).toResolve();
});

it('processJSONWithExtras does not merge duplicate vertices by default', function (done) {
spyOn(mergeDuplicateVertices, '_implementation').and.callThrough();
var promise = readGltf(gltfPath)
.then(function (gltf) {
return processJSONWithExtras(gltf);
})
.then(function () {
expect(mergeDuplicateVertices._implementation).not.toHaveBeenCalled();
});
expect(promise, done).toResolve();
});

it('processJSONWithExtras can merge duplicate vertices.', function (done) {
spyOn(mergeDuplicateVertices, '_implementation').and.callThrough();
var promise = readGltf(gltfPath)
.then(function (gltf) {
return processJSONWithExtras(gltf, {mergeVertices: true});
})
.then(function (gltf) {
expect(mergeDuplicateVertices._implementation).toHaveBeenCalledWith(gltf);
});
expect(promise, done).toResolve();
});
});