Skip to content

Commit

Permalink
Makes mergeNodes only look at transformable attributes to determine i…
Browse files Browse the repository at this point in the history 
…f it can merge or not. Also adds an optimizeDrawCalls option, that will not merge accessors in order to make a more efficient model.
  • Loading branch information
Tom Fili committed Aug 25, 2017
1 parent 9aa18c9 commit ab1509f
Show file tree
Hide file tree
Showing 7 changed files with 207 additions and 16 deletions.
8 changes: 3 additions & 5 deletions lib/MergeDuplicateProperties.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ var getAccessorByteStride = require('./getAccessorByteStride');
var numberOfComponentsForType = require('./numberOfComponentsForType');

var defined = Cesium.defined;
var defaultValue = Cesium.defaultValue;

module.exports = MergeDuplicateProperties;

Expand All @@ -25,12 +24,11 @@ function MergeDuplicateProperties() {}
* Merges all duplicate elements in the glTF asset in top-down order so merged references resolve down the hierarchy.
*
* @param {Object} gltf A javascript object containing a glTF asset.
* @param {Boolean} skipMergeAccessors Option to skip running mergeAccessors.
* @param {Boolean} optimizeDrawCalls Option to optimize draw calls over size.
* @returns {Object} The glTF asset with merged duplicate elements.
*/
MergeDuplicateProperties.mergeAll = function(gltf, skipMergeAccessors) {
skipMergeAccessors = defaultValue(skipMergeAccessors, true);
if (!skipMergeAccessors) {
MergeDuplicateProperties.mergeAll = function(gltf, optimizeDrawCalls) {
if (!optimizeDrawCalls) {
MergeDuplicateProperties.mergeAccessors(gltf);
}
MergeDuplicateProperties.mergeShaders(gltf);
Expand Down
6 changes: 4 additions & 2 deletions lib/Pipeline.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ Pipeline.processJSON = function(gltf, options) {
* @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 {Boolean} [options.optimizeDrawCalls=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 @@ -116,19 +117,20 @@ Pipeline.processJSONWithExtras = function(gltfWithExtras, options) {
RemoveUnusedProperties.removeAll(gltfWithExtras);

var mergeVertices = defaultValue(options.mergeVertices, false);
var optimizeDrawCalls = defaultValue(options.optimizeDrawCalls, false);
if (!shouldPreserve) {
if (mergeVertices) {
mergeDuplicateVertices(gltfWithExtras);
}
MergeDuplicateProperties.mergeAll(gltfWithExtras, true);
MergeDuplicateProperties.mergeAll(gltfWithExtras, optimizeDrawCalls);
RemoveUnusedProperties.removeAll(gltfWithExtras);
removeDuplicatePrimitives(gltfWithExtras);
combinePrimitives(gltfWithExtras);
convertDagToTree(gltfWithExtras);
combineNodes(gltfWithExtras);
combineMeshes(gltfWithExtras);
combinePrimitives(gltfWithExtras);
MergeDuplicateProperties.mergeAll(gltfWithExtras);
MergeDuplicateProperties.mergeAll(gltfWithExtras, optimizeDrawCalls);
removeDuplicatePrimitives(gltfWithExtras);
RemoveUnusedProperties.removeAll(gltfWithExtras);
optimizeForVertexCache(gltfWithExtras);
Expand Down
15 changes: 9 additions & 6 deletions lib/PrimitiveHelpers.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ module.exports = {
getPrimitivesByMaterialMode : getPrimitivesByMaterialMode,
getPrimitiveConflicts : getPrimitiveConflicts,
primitiveEquals : primitiveEquals,
primitivesShareAttributeAccessor : primitivesShareAttributeAccessor,
primitivesShareAttributeAccessor: primitivesShareAttributeAccessor,
primitivesHaveOverlappingIndexAccessors : primitivesHaveOverlappingIndexAccessors,
transformPrimitives : transformPrimitives
};
Expand All @@ -26,16 +26,19 @@ module.exports = {
*
* @param {Object} primitive The first primitive to compare.
* @param {Object} comparePrimitive The second primitive to compare.
* @param {String[]} [attributesToCheck] An array of attributes to check for sharing. Defaults to checking all attributes.
* @returns {Boolean} True if primitives share an attribute, false otherwise.
*/
function primitivesShareAttributeAccessor(primitive, comparePrimitive) {
function primitivesShareAttributeAccessor(primitive, comparePrimitive, attributesToCheck) {
var attributes = primitive.attributes;
var compareAttributes = comparePrimitive.attributes;
for (var attribute in attributes) {
if (attributes.hasOwnProperty(attribute)) {
if (compareAttributes.hasOwnProperty(attribute)) {
if (attributes[attribute] === compareAttributes[attribute]) {
return true;
if (!defined(attributesToCheck) || (attributesToCheck.indexOf(attribute) !== -1)) {
if (attributes.hasOwnProperty(attribute)) {
if (compareAttributes.hasOwnProperty(attribute)) {
if (attributes[attribute] === compareAttributes[attribute]) {
return true;
}
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion lib/combineNodes.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,7 @@ function meshHasConflict(gltf, mesh, compareMesh) {
var primitive = primitives[i];
for (var j = 0; j < comparePrimitivesLength; j++) {
var comparePrimitive = comparePrimitives[j];
if (PrimitiveHelpers.primitivesShareAttributeAccessor(primitive, comparePrimitive)) {
if (PrimitiveHelpers.primitivesShareAttributeAccessor(primitive, comparePrimitive, ['POSITION', 'NORMAL'])) {
if (PrimitiveHelpers.primitivesHaveOverlappingIndexAccessors(gltf, primitive, comparePrimitive)) {
return true;
}
Expand Down
7 changes: 6 additions & 1 deletion lib/parseArguments.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,10 @@ function parseArguments(args) {
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'
},
'optimizeDrawCalls': {
describe: 'Optimize model for performance. This may make the resulting file size larger.',
type: 'boolean'
},
'removeNormals': {
alias: 'r',
describe: 'Strips off existing normals, allowing them to be regenerated.',
Expand Down Expand Up @@ -266,6 +270,7 @@ function parseArguments(args) {
quantize: argv.q,
removeNormals: argv.r,
textureCompressionOptions: textureCompressionOptions,
mergeVertices: argv.mergeVertices
mergeVertices: argv.mergeVertices,
optimizeDrawCalls: argv.optimizeDrawCalls
};
}
32 changes: 32 additions & 0 deletions specs/lib/MergeDuplicatePropertiesSpec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -352,4 +352,36 @@ describe('MergeDuplicateProperties', function() {
expect(meshTwoPrimitives[1].material).toBe('materialOne');
});
});

it('mergeAccessors is called if optimizeDrawCalls is false', function() {
var spyAccessors = spyOn(MergeDuplicateProperties, 'mergeAccessors');
var spyShaders = spyOn(MergeDuplicateProperties, 'mergeShaders');
var spyPrograms = spyOn(MergeDuplicateProperties, 'mergePrograms');
var spyTechniques = spyOn(MergeDuplicateProperties, 'mergeTechniques');
var spyMaterials = spyOn(MergeDuplicateProperties, 'mergeMaterials');

MergeDuplicateProperties.mergeAll({}, false);

expect(spyAccessors).toHaveBeenCalled();
expect(spyShaders).toHaveBeenCalled();
expect(spyPrograms).toHaveBeenCalled();
expect(spyTechniques).toHaveBeenCalled();
expect(spyMaterials).toHaveBeenCalled();
});

it('mergeAccessors isn\'t called if optimizeDrawCalls is true', function() {
var spyAccessors = spyOn(MergeDuplicateProperties, 'mergeAccessors');
var spyShaders = spyOn(MergeDuplicateProperties, 'mergeShaders');
var spyPrograms = spyOn(MergeDuplicateProperties, 'mergePrograms');
var spyTechniques = spyOn(MergeDuplicateProperties, 'mergeTechniques');
var spyMaterials = spyOn(MergeDuplicateProperties, 'mergeMaterials');

MergeDuplicateProperties.mergeAll({}, true);

expect(spyAccessors).not.toHaveBeenCalled();
expect(spyShaders).toHaveBeenCalled();
expect(spyPrograms).toHaveBeenCalled();
expect(spyTechniques).toHaveBeenCalled();
expect(spyMaterials).toHaveBeenCalled();
});
});
153 changes: 152 additions & 1 deletion specs/lib/combineNodesSpec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -435,4 +435,155 @@ describe('combineNodes', function() {
expect(nodes.nodeB).toBeDefined();
expect(nodes.nodeC).toBeDefined();
});
});

it('nodes with meshes with overlapping accessors that aren\'t transformable will still combine', function() {
var positions = new Float32Array([1.0, 0.0, 0.0, 0.0, 1.0, 0.0, 0.0, 0.0, 1.0]);
var normals = new Float32Array([1.0, 0.0, 0.0, 0.0, 1.0, 0.0, 0.0, 0.0, 1.0]);
var batchIds = new Uint16Array([0, 0, 0]);
var indices = new Uint16Array([0, 1]);
var overlappedIndices = new Uint16Array([1, 2]);
var positionsBuffer = Buffer.from(positions.buffer);
var normalsBuffer = Buffer.from(normals.buffer);
var batchIdsBuffer = Buffer.from(batchIds.buffer);
var attributesBuffer = Buffer.concat([positionsBuffer, normalsBuffer, batchIdsBuffer]);
var indicesBuffer = Buffer.from(indices.buffer);
var overlappedIndicesBuffer = Buffer.from(overlappedIndices.buffer);
var allIndicesBuffer = Buffer.concat([indicesBuffer, overlappedIndicesBuffer]);
var buffer = Buffer.concat([attributesBuffer, allIndicesBuffer]);
var gltf = {
accessors : {
positionAccessor : {
bufferView : 'attributesBufferView',
byteOffset : 0,
byteStride : 0,
componentType : WebGLConstants.FLOAT,
count : positions.length / 3,
type : "VEC3"
},
normalAccessor : {
bufferView : 'attributesBufferView',
byteOffset : normalsBuffer.length,
byteStride : 0,
componentType : WebGLConstants.FLOAT,
count : normals.length / 3,
type : "VEC3"
},
positionAccessor2 : {
bufferView : 'attributesBufferView',
byteOffset : 0,
byteStride : 0,
componentType : WebGLConstants.FLOAT,
count : positions.length / 3,
type : "VEC3"
},
normalAccessor2 : {
bufferView : 'attributesBufferView',
byteOffset : normalsBuffer.length,
byteStride : 0,
componentType : WebGLConstants.FLOAT,
count : normals.length / 3,
type : "VEC3"
},
batchIdAccessor : {
bufferView : 'attributesBufferView',
byteOffset : positionsBuffer.length + normalsBuffer.length,
byteStride : 0,
componentType : WebGLConstants.UNSIGNED_SHORT,
count : batchIdsBuffer.length,
type : "SCALAR"
},
indicesAccessor : {
bufferView : 'indicesBufferView',
byteOffset : 0,
byteStride : 0,
componentType : WebGLConstants.UNSIGNED_SHORT,
count : indices.length,
type : "SCALAR"
},
overlappedIndicesAccessor : {
bufferView : 'indicesBufferView',
byteOffset : indicesBuffer.length,
byteStride : 0,
componentType : WebGLConstants.UNSIGNED_SHORT,
count : overlappedIndices.length,
type : "SCALAR"
}
},
bufferViews : {
attributesBufferView : {
buffer : 'buffer',
byteLength : attributesBuffer.length,
byteOffset : 0,
target : WebGLConstants.ARRAY_BUFFER
},
indicesBufferView : {
buffer : 'buffer',
byteLength : allIndicesBuffer.length,
byteOffset : attributesBuffer.length,
target : WebGLConstants.ELEMENT_ARRAY_BUFFER
}
},
buffers : {
buffer : {
type : "arraybuffer",
byteLength : buffer.length,
extras : {
_pipeline : {
source : buffer
}
}
}
},
meshes : {
meshA : {
primitives : [
{
attributes : {
NORMAL : 'normalAccessor',
POSITION : 'positionAccessor',
_BATCHID: 'batchIdAccessor'
},
indices : 'indicesAccessor'
}
]
},
meshB : {
primitives : [
{
attributes : {
NORMAL : 'normalAccessor2',
POSITION : 'positionAccessor2',
_BATCHID: 'batchIdAccessor'
},
indices : 'overlappedIndicesAccessor'
}
]
}
},
nodes : {
nodeA : {
children : ['nodeB', 'nodeC']
},
nodeB : {
meshes : ['meshA'],
matrix : [1.0, 0.0, 0.0, 0.0,
0.0, 1.0, 0.0, 0.0,
0.0, 0.0, 1.0, 0.0,
1.0, 2.0, 3.0, 1.0]
},
nodeC: {
meshes : ['meshB']
}
},
scenes : {
scene : {
nodes : ['nodeA']
}
}
};
combineNodes(gltf);
var nodes = gltf.nodes;
expect(nodes.nodeB).toBeUndefined();
expect(nodes.nodeC).toBeUndefined();
});
});

0 comments on commit ab1509f

Please sign in to comment.