Merge pull request #185 from AnalyticalGraphicsInc/separate-fix

Separate resources no longer hit Node buffer size limit

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)

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) {
+function addCombinedBuffers(gltf, bufferState, name) {
-    addBufferView(gltf, bufferState.positionBuffers, bufferState.positionAccessors, 12, WebGLConstants.ARRAY_BUFFER);
+    addCombinedBufferView(gltf, bufferState.positionBuffers, bufferState.positionAccessors, 12, WebGLConstants.ARRAY_BUFFER);
-    addBufferView(gltf, bufferState.normalBuffers, bufferState.normalAccessors, 12, WebGLConstants.ARRAY_BUFFER);
+    addCombinedBufferView(gltf, bufferState.normalBuffers, bufferState.normalAccessors, 12, WebGLConstants.ARRAY_BUFFER);
-    addBufferView(gltf, bufferState.uvBuffers, bufferState.uvAccessors, 8, WebGLConstants.ARRAY_BUFFER);
+    addCombinedBufferView(gltf, bufferState.uvBuffers, bufferState.uvAccessors, 8, WebGLConstants.ARRAY_BUFFER);
-    addBufferView(gltf, bufferState.indexBuffers, bufferState.indexAccessors, undefined, WebGLConstants.ELEMENT_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;
+};

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];
+    const buffers = gltf.buffers;
-    delete buffer.extras;
+    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) {
+function writeSeparateBuffers(gltf, options) {
-    const buffer = gltf.buffers[0];
+    const buffers = gltf.buffers;
-    const source = buffer.extras._obj2gltf.source;
+    return Promise.map(buffers, function(buffer) {
-    const bufferUri = buffer.name + '.bin';
+        const source = buffer.extras._obj2gltf.source;
-    buffer.uri = bufferUri;
+        const bufferUri = buffer.name + '.bin';
-    return options.writer(bufferUri, source);
+        buffer.uri = bufferUri;
+        return options.writer(bufferUri, source);
+    }, {concurrency : 10});
 }
 
 function writeSeparateTextures(gltf, options) {

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);
 

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();
     });