diff --git a/CHANGES.md b/CHANGES.md index 4134d51..d9e077e 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,6 +1,10 @@ Change Log ========== +### 3.0.2 2019-03-?? + +* Fixed a crash when saving separate resources that would exceed the Node buffer size limit. [#173](https://github.com/AnalyticalGraphicsInc/obj2gltf/pull/173) + ### 3.0.1 2019-03-08 * Fixed handling of materials that don't have names. [#173](https://github.com/AnalyticalGraphicsInc/obj2gltf/pull/173) diff --git a/lib/createGltf.js b/lib/createGltf.js index 1c6c26a..c314ac5 100644 --- a/lib/createGltf.js +++ b/lib/createGltf.js @@ -1,4 +1,5 @@ 'use strict'; +const BUFFER_MAX_BYTE_LENGTH = require('buffer').constants.MAX_LENGTH; const Cesium = require('cesium'); const getBufferPadded = require('./getBufferPadded'); const getDefaultMaterial = require('./loadMtl').getDefaultMaterial; @@ -95,7 +96,7 @@ function createGltf(objData, options) { }); } - addBuffers(gltf, bufferState, name); + addBuffers(gltf, bufferState, name, options.separate); if (options.specularGlossiness) { gltf.extensionsUsed.push('KHR_materials_pbrSpecularGlossiness'); @@ -110,7 +111,7 @@ function createGltf(objData, options) { return gltf; } -function addBufferView(gltf, buffers, accessors, byteStride, target) { +function addCombinedBufferView(gltf, buffers, accessors, byteStride, target) { const length = buffers.length; if (length === 0) { return; @@ -135,11 +136,11 @@ function addBufferView(gltf, buffers, accessors, byteStride, target) { }); } -function addBuffers(gltf, bufferState, name) { - addBufferView(gltf, bufferState.positionBuffers, bufferState.positionAccessors, 12, WebGLConstants.ARRAY_BUFFER); - addBufferView(gltf, bufferState.normalBuffers, bufferState.normalAccessors, 12, WebGLConstants.ARRAY_BUFFER); - addBufferView(gltf, bufferState.uvBuffers, bufferState.uvAccessors, 8, WebGLConstants.ARRAY_BUFFER); - addBufferView(gltf, bufferState.indexBuffers, bufferState.indexAccessors, undefined, WebGLConstants.ELEMENT_ARRAY_BUFFER); +function addCombinedBuffers(gltf, bufferState, name) { + addCombinedBufferView(gltf, bufferState.positionBuffers, bufferState.positionAccessors, 12, WebGLConstants.ARRAY_BUFFER); + addCombinedBufferView(gltf, bufferState.normalBuffers, bufferState.normalAccessors, 12, WebGLConstants.ARRAY_BUFFER); + addCombinedBufferView(gltf, bufferState.uvBuffers, bufferState.uvAccessors, 8, WebGLConstants.ARRAY_BUFFER); + addCombinedBufferView(gltf, bufferState.indexBuffers, bufferState.indexAccessors, undefined, WebGLConstants.ELEMENT_ARRAY_BUFFER); let buffers = []; buffers = buffers.concat(bufferState.positionBuffers, bufferState.normalBuffers, bufferState.uvBuffers, bufferState.indexBuffers); @@ -156,6 +157,62 @@ function addBuffers(gltf, bufferState, name) { }); } +function addSeparateBufferView(gltf, buffer, accessor, byteStride, target, name) { + const bufferIndex = gltf.buffers.length; + const bufferViewIndex = gltf.bufferViews.length; + + gltf.buffers.push({ + name : name + '_' + bufferIndex, + byteLength : buffer.length, + extras : { + _obj2gltf : { + source : buffer + } + } + }); + + gltf.bufferViews.push({ + buffer : bufferIndex, + byteLength : buffer.length, + byteOffset : 0, + byteStride : byteStride, + target : target + }); + + gltf.accessors[accessor].bufferView = bufferViewIndex; + gltf.accessors[accessor].byteOffset = 0; +} + +function addSeparateBufferViews(gltf, buffers, accessors, byteStride, target, name) { + const length = buffers.length; + for (let i = 0; i < length; ++i) { + addSeparateBufferView(gltf, buffers[i], accessors[i], byteStride, target, name); + } +} + +function addSeparateBuffers(gltf, bufferState, name) { + addSeparateBufferViews(gltf, bufferState.positionBuffers, bufferState.positionAccessors, 12, WebGLConstants.ARRAY_BUFFER, name); + addSeparateBufferViews(gltf, bufferState.normalBuffers, bufferState.normalAccessors, 12, WebGLConstants.ARRAY_BUFFER, name); + addSeparateBufferViews(gltf, bufferState.uvBuffers, bufferState.uvAccessors, 8, WebGLConstants.ARRAY_BUFFER, name); + addSeparateBufferViews(gltf, bufferState.indexBuffers, bufferState.indexAccessors, undefined, WebGLConstants.ELEMENT_ARRAY_BUFFER, name); +} + +function addBuffers(gltf, bufferState, name, separate) { + const buffers = bufferState.positionBuffers.concat(bufferState.normalBuffers, bufferState.uvBuffers, bufferState.indexBuffers); + const buffersLength = buffers.length; + let buffersByteLength = 0; + for (let i = 0; i < buffersLength; ++i) { + buffersByteLength += buffers[i].length; + } + + if (separate && (buffersByteLength > createGltf._getBufferMaxByteLength())) { + // Don't combine buffers if the combined buffer will exceed the Node limit. + addSeparateBuffers(gltf, bufferState, name); + } else { + addCombinedBuffers(gltf, bufferState, name); + } +} + function addTexture(gltf, texture) { const imageName = texture.name; const textureName = texture.name; @@ -481,3 +538,8 @@ function addNode(gltf, name, meshIndex, parentIndex) { return nodeIndex; } + +// Exposed for testing +createGltf._getBufferMaxByteLength = function() { + return BUFFER_MAX_BYTE_LENGTH; +}; diff --git a/lib/writeGltf.js b/lib/writeGltf.js index 1ce57ab..000b7e4 100644 --- a/lib/writeGltf.js +++ b/lib/writeGltf.js @@ -35,7 +35,7 @@ function writeGltf(gltf, options) { } if (separate) { - promises.push(writeSeparateBuffer(gltf, options)); + promises.push(writeSeparateBuffers(gltf, options)); } else if (!binary) { writeEmbeddedBuffer(gltf); } @@ -103,8 +103,11 @@ function encodeTextures(gltf) { } function deleteExtras(gltf) { - const buffer = gltf.buffers[0]; - delete buffer.extras; + const buffers = gltf.buffers; + const buffersLength = buffers.length; + for (let i = 0; i < buffersLength; ++i) { + delete buffers[i].extras; + } const images = gltf.images; const imagesLength = images.length; @@ -123,12 +126,14 @@ function removeEmpty(json) { }); } -function writeSeparateBuffer(gltf, options) { - const buffer = gltf.buffers[0]; - const source = buffer.extras._obj2gltf.source; - const bufferUri = buffer.name + '.bin'; - buffer.uri = bufferUri; - return options.writer(bufferUri, source); +function writeSeparateBuffers(gltf, options) { + const buffers = gltf.buffers; + return Promise.map(buffers, function(buffer) { + const source = buffer.extras._obj2gltf.source; + const bufferUri = buffer.name + '.bin'; + buffer.uri = bufferUri; + return options.writer(bufferUri, source); + }, {concurrency : 10}); } function writeSeparateTextures(gltf, options) { diff --git a/specs/lib/createGltfSpec.js b/specs/lib/createGltfSpec.js index 3c26384..831b1c8 100644 --- a/specs/lib/createGltfSpec.js +++ b/specs/lib/createGltfSpec.js @@ -60,6 +60,29 @@ describe('createGltf', () => { expect(indexAccessor.count).toBe(36); }); + it('does not combine buffers when that buffer would exceed the Node buffer size limit', () => { + spyOn(createGltf, '_getBufferMaxByteLength').and.returnValue(0); + const clonedOptions = clone(options, true); + clonedOptions.separate = true; + + const gltf = createGltf(boxObjData, clonedOptions); + expect(gltf.accessors.length).toBe(4); + expect(gltf.buffers.length).toBe(4); + expect(gltf.bufferViews.length).toBe(4); + + const length = gltf.buffers.length; + for (let i = 0; i < length; ++i) { + const accessor = gltf.accessors[i]; + const bufferView = gltf.bufferViews[i]; + const buffer = gltf.buffers[i]; + expect(accessor.bufferView).toBe(i); + expect(accessor.byteOffset).toBe(0); + expect(bufferView.buffer).toBe(i); + expect(bufferView.byteOffset).toBe(0); + expect(bufferView.byteLength).toBe(buffer.byteLength); + } + }); + it('multiple nodes, meshes, and primitives', () => { const gltf = createGltf(groupObjData, options); diff --git a/specs/lib/obj2gltfSpec.js b/specs/lib/obj2gltfSpec.js index 0fd7f7a..a30d57a 100644 --- a/specs/lib/obj2gltfSpec.js +++ b/specs/lib/obj2gltfSpec.js @@ -3,6 +3,7 @@ const { DeveloperError } = require('cesium'); const fsExtra = require('fs-extra'); const path = require('path'); const Promise = require('bluebird'); +const createGltf = require('../../lib/createGltf'); const obj2gltf = require('../../lib/obj2gltf'); const texturedObjPath = 'specs/data/box-textured/box-textured.obj'; @@ -43,6 +44,17 @@ describe('obj2gltf', () => { expect(fsExtra.outputFile.calls.count()).toBe(2); // Saves out .png and .bin }); + it('convert obj to gltf with separate resources when buffer exceeds Node limit', async () => { + spyOn(createGltf, '_getBufferMaxByteLength').and.returnValue(0); + const options = { + separate : true, + separateTextures : true, + outputDirectory : outputDirectory + }; + await obj2gltf(texturedObjPath, options); + expect(fsExtra.outputFile.calls.count()).toBe(5); // Saves out .png and four .bin for positions, normals, uvs, and indices + }); + it('converts obj to glb with separate resources', async () => { const options = { separate : true, @@ -126,7 +138,7 @@ describe('obj2gltf', () => { } }; await obj2gltf(texturedObjPath, options); - expect(filePaths).toEqual(['box-textured.bin', 'cesium.png']); + expect(filePaths).toEqual(['cesium.png', 'box-textured.bin']); expect(fileContents[0]).toBeDefined(); expect(fileContents[1]).toBeDefined(); });