Merge branch 'master' into absolute-paths

CHANGES.md

@@ -1,10 +1,12 @@
 Change Log
 ==========
 
-### 3.?.? - 2019-??-??
+### 3.?.? ????-??-??
 
 * Added back `inputUpAxis` and `outputUpAxis`. [#211](https://github.com/AnalyticalGraphicsInc/obj2gltf/pull/211)
 * Fixed handling of mtl and texture absolute paths. [#219](https://github.com/AnalyticalGraphicsInc/obj2gltf/pull/219)
+* Fixed specular image not being decoded when referenced by other textures. [#217](https://github.com/AnalyticalGraphicsInc/obj2gltf/pull/217)
+* Fixed parsing faces that reference non-existing attributes. [#218](https://github.com/AnalyticalGraphicsInc/obj2gltf/pull/218)
 
 ### 3.0.4 - 2019-07-22
 

lib/createGltf.js

@@ -237,11 +237,10 @@ function addTexture(gltf, texture) {
 
 function getTexture(gltf, texture) {
     let textureIndex;
-    const name = texture.name;
-    const textures = gltf.textures;
-    const length = textures.length;
+    const images = gltf.images;
+    const length = images.length;
     for (let i = 0; i < length; ++i) {
-        if (textures[i].name === name) {
+        if (images[i].extras._obj2gltf === texture) {
             textureIndex = i;
             break;
         }

lib/loadMtl.js

@@ -8,6 +8,7 @@ const readLines = require('./readLines');
 const Texture = require('./Texture');
 
 const CesiumMath = Cesium.Math;
+const clone = Cesium.clone;
 const combine = Cesium.combine;
 const defaultValue = Cesium.defaultValue;
 const defined = Cesium.defined;
@@ -178,13 +179,30 @@ function loadMtl(mtlPath, options) {
             material.ambientTexture = undefined;
         }
 
-        loadMaterialTexture(material, 'diffuseTexture', diffuseAlphaTextureOptions, mtlDirectory, texturePromiseMap, texturePromises, options);
-        loadMaterialTexture(material, 'ambientTexture', ambientTextureOptions, mtlDirectory, texturePromiseMap, texturePromises, options);
-        loadMaterialTexture(material, 'emissiveTexture', emissiveTextureOptions, mtlDirectory, texturePromiseMap, texturePromises, options);
-        loadMaterialTexture(material, 'specularTexture', specularTextureOptions, mtlDirectory, texturePromiseMap, texturePromises, options);
-        loadMaterialTexture(material, 'specularShininessTexture', specularShinessTextureOptions, mtlDirectory, texturePromiseMap, texturePromises, options);
-        loadMaterialTexture(material, 'normalTexture', normalTextureOptions, mtlDirectory, texturePromiseMap, texturePromises, options);
-        loadMaterialTexture(material, 'alphaTexture', alphaTextureOptions, mtlDirectory, texturePromiseMap, texturePromises, options);
+        const textureNames = ['diffuseTexture', 'ambientTexture', 'emissiveTexture', 'specularTexture', 'specularShininessTexture', 'normalTexture', 'alphaTexture'];
+        const textureOptions = [diffuseAlphaTextureOptions, ambientTextureOptions, emissiveTextureOptions, specularTextureOptions, specularShinessTextureOptions, normalTextureOptions, alphaTextureOptions];
+
+        const sharedOptions = {};
+        textureNames.forEach(function(name, index) {
+            const texturePath = material[name];
+            const originalOptions = textureOptions[index];
+            if (defined(texturePath) && defined(originalOptions)) {
+                if (!defined(sharedOptions[texturePath])) {
+                    sharedOptions[texturePath] = clone(originalOptions);
+                }
+                const options = sharedOptions[texturePath];
+                options.checkTransparency = options.checkTransparency || originalOptions.checkTransparency;
+                options.decode = options.decode || originalOptions.decode;
+                options.keepSource = options.keepSource || !originalOptions.decode || !originalOptions.checkTransparency;
+            }
+        });
+
+        textureNames.forEach(function(name) {
+            const texturePath = material[name];
+            if (defined(texturePath)) {
+                loadMaterialTexture(material, name, sharedOptions[texturePath], mtlDirectory, texturePromiseMap, texturePromises, options);
+            }
+        });
     }
 
     return readLines(mtlPath, parseLine)

lib/loadObj.js

@@ -195,7 +195,10 @@ function loadObj(objPath, options) {
 
     function createVertex(p, u, n) {
         // Positions
-        if (defined(p)) {
+        if (defined(p) && (globalPositions.length > 0)) {
+            if (p * 3 >= globalPositions.length) {
+                throw new RuntimeError(`Position index ${p} is out of bounds`);
+            }
             const px = globalPositions.get(p * 3);
             const py = globalPositions.get(p * 3 + 1);
             const pz = globalPositions.get(p * 3 + 2);
@@ -205,7 +208,10 @@ function loadObj(objPath, options) {
         }
 
         // Normals
-        if (defined(n)) {
+        if (defined(n) && (globalNormals.length > 0)) {
+            if (n * 3 >= globalNormals.length) {
+                throw new RuntimeError(`Normal index ${n} is out of bounds`);
+            }
             const nx = globalNormals.get(n * 3);
             const ny = globalNormals.get(n * 3 + 1);
             const nz = globalNormals.get(n * 3 + 2);
@@ -215,7 +221,10 @@ function loadObj(objPath, options) {
         }
 
         // UVs
-        if (defined(u)) {
+        if (defined(u) && (globalUvs.length > 0)) {
+            if (u * 2 >= globalUvs.length) {
+                throw new RuntimeError(`UV index ${u} is out of bounds`);
+            }
             const ux = globalUvs.get(u * 2);
             const uy = globalUvs.get(u * 2 + 1);
             primitive.uvs.push(ux);

lib/loadTexture.js

@@ -19,6 +19,7 @@ module.exports = loadTexture;
  * @param {Object} [options] An object with the following properties:
  * @param {Boolean} [options.checkTransparency=false] Do a more exhaustive check for texture transparency by looking at the alpha channel of each pixel.
  * @param {Boolean} [options.decode=false] Whether to decode the texture.
+ * @param {Boolean} [options.keepSource=false] Whether to keep the source image contents in memory.
  * @returns {Promise} A promise resolving to a Texture object.
  *
  * @private
@@ -27,6 +28,7 @@ function loadTexture(texturePath, options) {
     options = defaultValue(options, {});
     options.checkTransparency = defaultValue(options.checkTransparency, false);
     options.decode = defaultValue(options.decode, false);
+    options.keepSource = defaultValue(options.keepSource, false);
 
     return fsExtra.readFile(texturePath)
         .then(function(source) {
@@ -46,7 +48,10 @@ function loadTexture(texturePath, options) {
             }
 
             if (defined(decodePromise)) {
-                return decodePromise.thenReturn(texture);
+                return decodePromise
+                    .then(function() {
+                        return texture;
+                    });
             }
 
             return texture;
@@ -109,8 +114,10 @@ function decodePng(texture, options) {
                     texture.pixels = decodedResults.data;
                     texture.width = decodedResults.width;
                     texture.height = decodedResults.height;
+                    if (!options.keepSource) {
                         texture.source = undefined; // Unload resources
                     }
+                }
             });
     }
 }
@@ -122,6 +129,8 @@ function decodeJpeg(texture, options) {
         texture.pixels = decodedResults.data;
         texture.width = decodedResults.width;
         texture.height = decodedResults.height;
+        if (!options.keepSource) {
             texture.source = undefined; // Unload resources
         }
     }
+}

lib/readLines.js

@@ -23,6 +23,15 @@ function readLines(path, callback) {
         const lineReader = readline.createInterface({
             input : stream
         });
-        lineReader.on('line', callback);
+
+        const callbackWrapper = function(line) {
+            try {
+                callback(line);
+            } catch (error) {
+                reject(error);
+            }
+        };
+
+        lineReader.on('line', callbackWrapper);
     });
 }

package.json

@@ -38,13 +38,13 @@
     "cloc": "^2.5.0",
     "coveralls": "^3.0.4",
     "eslint": "^6.0.0",
-    "eslint-config-cesium": "^7.0.0",
+    "eslint-config-cesium": "^8.0.0",
     "gulp": "^4.0.2",
     "jasmine": "^3.4.0",
     "jasmine-spec-reporter": "^4.2.1",
     "jsdoc": "^3.6.2",
     "nyc": "^14.1.1",
-    "open": "^6.3.0",
+    "open": "^7.0.0",
     "requirejs": "^2.3.6"
   },
   "scripts": {

specs/data/box-incomplete-attributes/box-incomplete-normals.obj

@@ -0,0 +1,36 @@
+o Cube
+v -1.000000 -1.000000 1.000000
+v -1.000000 1.000000 1.000000
+v -1.000000 -1.000000 -1.000000
+v -1.000000 1.000000 -1.000000
+v 1.000000 -1.000000 1.000000
+v 1.000000 1.000000 1.000000
+v 1.000000 -1.000000 -1.000000
+v 1.000000 1.000000 -1.000000
+vt 0.0000 0.0000
+vt 1.0000 0.0000
+vt 1.0000 1.0000
+vt 0.0000 1.0000
+vt 0.0000 0.0000
+vt 1.0000 0.0000
+vt 1.0000 1.0000
+vt 0.0000 1.0000
+vt 0.0000 0.0000
+vt 1.0000 0.0000
+vt 1.0000 1.0000
+vt 0.0000 1.0000
+vt 0.0000 0.0000
+vt 1.0000 0.0000
+vt 1.0000 1.0000
+vt 0.0000 1.0000
+vt 1.0000 0.0000
+vt 1.0000 1.0000
+vt 0.0000 0.0000
+vt 0.0000 1.0000
+vn 0.000000 0.000000 1.000000
+f 1/1/1 2/2/1 4/3/1 3/4/1
+f 3/5/2 4/6/2 8/7/2 7/8/2
+f 7/9/3 8/10/3 6/11/3 5/12/3
+f 5/13/4 6/14/4 2/15/4 1/16/4
+f 3/5/5 7/17/5 5/18/5 1/16/5
+f 8/19/6 4/6/6 2/15/6 6/20/6

specs/data/box-incomplete-attributes/box-incomplete-positions.obj

@@ -0,0 +1,34 @@
+o Cube
+v -1.000000 -1.000000 1.000000
+vt 0.0000 0.0000
+vt 1.0000 0.0000
+vt 1.0000 1.0000
+vt 0.0000 1.0000
+vt 0.0000 0.0000
+vt 1.0000 0.0000
+vt 1.0000 1.0000
+vt 0.0000 1.0000
+vt 0.0000 0.0000
+vt 1.0000 0.0000
+vt 1.0000 1.0000
+vt 0.0000 1.0000
+vt 0.0000 0.0000
+vt 1.0000 0.0000
+vt 1.0000 1.0000
+vt 0.0000 1.0000
+vt 1.0000 0.0000
+vt 1.0000 1.0000
+vt 0.0000 0.0000
+vt 0.0000 1.0000
+vn -1.0000 0.0000 0.0000
+vn 0.0000 0.0000 -1.0000
+vn 1.0000 0.0000 0.0000
+vn 0.0000 0.0000 1.0000
+vn 0.0000 -1.0000 0.0000
+vn 0.0000 1.0000 0.0000
+f 1/1/1 2/2/1 4/3/1 3/4/1
+f 3/5/2 4/6/2 8/7/2 7/8/2
+f 7/9/3 8/10/3 6/11/3 5/12/3
+f 5/13/4 6/14/4 2/15/4 1/16/4
+f 3/5/5 7/17/5 5/18/5 1/16/5
+f 8/19/6 4/6/6 2/15/6 6/20/6

specs/data/box-incomplete-attributes/box-incomplete-uvs.obj

@@ -0,0 +1,22 @@
+o Cube
+v -1.000000 -1.000000 1.000000
+v -1.000000 1.000000 1.000000
+v -1.000000 -1.000000 -1.000000
+v -1.000000 1.000000 -1.000000
+v 1.000000 -1.000000 1.000000
+v 1.000000 1.000000 1.000000
+v 1.000000 -1.000000 -1.000000
+v 1.000000 1.000000 -1.000000
+vt 0.0000 0.0000
+vn -1.0000 0.0000 0.0000
+vn 0.0000 0.0000 -1.0000
+vn 1.0000 0.0000 0.0000
+vn 0.0000 0.0000 1.0000
+vn 0.0000 -1.0000 0.0000
+vn 0.0000 1.0000 0.0000
+f 1/1/1 2/2/1 4/3/1 3/4/1
+f 3/5/2 4/6/2 8/7/2 7/8/2
+f 7/9/3 8/10/3 6/11/3 5/12/3
+f 5/13/4 6/14/4 2/15/4 1/16/4
+f 3/5/5 7/17/5 5/18/5 1/16/5
+f 8/19/6 4/6/6 2/15/6 6/20/6

specs/data/box-missing-attributes/box-missing-attributes.mtl

@@ -0,0 +1,12 @@
+# Blender MTL File: 'box.blend'
+# Material Count: 1
+
+newmtl Material
+Ns 96.078431
+Ka 0.000000 0.000000 0.000000
+Kd 0.640000 0.640000 0.640000
+Ks 0.500000 0.500000 0.500000
+Ke 0.000000 0.000000 0.000000
+Ni 1.000000
+d 1.000000
+illum 2

specs/data/box-missing-attributes/box-missing-attributes.obj

@@ -0,0 +1,20 @@
+# Blender v2.78 (sub 0) OBJ File: 'box.blend'
+# www.blender.org
+mtllib box-missing-attributes.mtl
+o Cube
+v -1.000000 -1.000000 1.000000
+v -1.000000 1.000000 1.000000
+v -1.000000 -1.000000 -1.000000
+v -1.000000 1.000000 -1.000000
+v 1.000000 -1.000000 1.000000
+v 1.000000 1.000000 1.000000
+v 1.000000 -1.000000 -1.000000
+v 1.000000 1.000000 -1.000000
+usemtl Material
+s off
+f 1/1/1 2/2/1 4/3/1 3/4/1
+f 3/5/2 4/6/2 8/7/2 7/8/2
+f 7/9/3 8/10/3 6/11/3 5/12/3
+f 5/13/4 6/14/4 2/15/4 1/16/4
+f 3/5/5 7/17/5 5/18/5 1/16/5
+f 8/19/6 4/6/6 2/15/6 6/20/6

specs/data/box-shared-textures-2/box-shared-textures-2.mtl

@@ -0,0 +1,14 @@
+# Blender MTL File: 'box.blend'
+# Material Count: 1
+
+newmtl Material
+Ns 96.078431
+Ka 0.000000 0.000000 0.000000
+Kd 0.640000 0.640000 0.640000
+Ks 0.500000 0.500000 0.500000
+Ke 0.000000 0.000000 0.000000
+Ni 1.000000
+d 1.000000
+illum 2
+map_Kd cesium.png
+map_Ke cesium.png

specs/data/box-shared-textures-2/box-shared-textures-2.obj

@@ -1,6 +1,6 @@
 # Blender v2.78 (sub 0) OBJ File: 'box.blend'
 # www.blender.org
-mtllib box-diffuse-ambient-same.mtl
+mtllib box-shared-textures-2.mtl
 o Cube
 v -1.000000 -1.000000 1.000000
 v -1.000000 1.000000 1.000000

specs/data/box-shared-textures/box-shared-textures.mtl

@@ -12,3 +12,4 @@ d 1.000000
 illum 2
 map_Kd cesium.png
 map_Ka cesium.png
+map_Ks cesium.png

specs/data/box-shared-textures/box-shared-textures.obj

@@ -0,0 +1,46 @@
+# Blender v2.78 (sub 0) OBJ File: 'box.blend'
+# www.blender.org
+mtllib box-shared-textures.mtl
+o Cube
+v -1.000000 -1.000000 1.000000
+v -1.000000 1.000000 1.000000
+v -1.000000 -1.000000 -1.000000
+v -1.000000 1.000000 -1.000000
+v 1.000000 -1.000000 1.000000
+v 1.000000 1.000000 1.000000
+v 1.000000 -1.000000 -1.000000
+v 1.000000 1.000000 -1.000000
+vt 0.0000 0.0000
+vt 1.0000 0.0000
+vt 1.0000 1.0000
+vt 0.0000 1.0000
+vt 0.0000 0.0000
+vt 1.0000 0.0000
+vt 1.0000 1.0000
+vt 0.0000 1.0000
+vt 0.0000 0.0000
+vt 1.0000 0.0000
+vt 1.0000 1.0000
+vt 0.0000 1.0000
+vt 0.0000 0.0000
+vt 1.0000 0.0000
+vt 1.0000 1.0000
+vt 0.0000 1.0000
+vt 1.0000 0.0000
+vt 1.0000 1.0000
+vt 0.0000 0.0000
+vt 0.0000 1.0000
+vn -1.0000 0.0000 0.0000
+vn 0.0000 0.0000 -1.0000
+vn 1.0000 0.0000 0.0000
+vn 0.0000 0.0000 1.0000
+vn 0.0000 -1.0000 0.0000
+vn 0.0000 1.0000 0.0000
+usemtl Material
+s off
+f 1/1/1 2/2/1 4/3/1 3/4/1
+f 3/5/2 4/6/2 8/7/2 7/8/2
+f 7/9/3 8/10/3 6/11/3 5/12/3
+f 5/13/4 6/14/4 2/15/4 1/16/4
+f 3/5/5 7/17/5 5/18/5 1/16/5
+f 8/19/6 4/6/6 2/15/6 6/20/6

specs/lib/loadMtlSpec.js

@@ -15,7 +15,8 @@ const externalMaterialPath = 'specs/data/box-external-resources/box-external-res
 const resourcesInRootMaterialPath = 'specs/data/box-resources-in-root/box-resources-in-root.mtl';
 const externalInRootMaterialPath = 'specs/data/box-external-resources-in-root/box-external-resources-in-root.mtl';
 const transparentMaterialPath = 'specs/data/box-transparent/box-transparent.mtl';
-const diffuseAmbientSameMaterialPath = 'specs/data/box-diffuse-ambient-same/box-diffuse-ambient-same.mtl';
+const sharedTexturesMaterialPath = 'specs/data/box-shared-textures/box-shared-textures.mtl';
+const sharedTexturesMaterial2Path = 'specs/data/box-shared-textures-2/box-shared-textures-2.mtl';
 
 const diffuseTexturePath = 'specs/data/box-textured/cesium.png';
 const transparentDiffuseTexturePath = 'specs/data/box-complex-material/diffuse.png';
@@ -204,7 +205,7 @@ describe('loadMtl', () => {
     });
 
     it('ambient texture is ignored if it is the same as the diffuse texture', async () => {
-        const materials = await loadMtl(diffuseAmbientSameMaterialPath, options);
+        const materials = await loadMtl(sharedTexturesMaterialPath, options);
         expect(materials.length).toBe(1);
         const material = materials[0];
         const pbr = material.pbrMetallicRoughness;
@@ -212,6 +213,27 @@ describe('loadMtl', () => {
         expect(pbr.occlusionTexture).toBeUndefined();
     });
 
+    it('texture referenced by specular is decoded', async () => {
+        const materials = await loadMtl(sharedTexturesMaterialPath, options);
+        expect(materials.length).toBe(1);
+        const material = materials[0];
+        const pbr = material.pbrMetallicRoughness;
+        expect(pbr.baseColorTexture.pixels).toBeDefined();
+        expect(pbr.baseColorTexture.source).toBeDefined();
+        expect(pbr.metallicRoughnessTexture.pixels).toBeDefined();
+        expect(pbr.metallicRoughnessTexture.source).toBeUndefined();
+    });
+
+    it('texture referenced by diffuse and emissive is not decoded', async () => {
+        const materials = await loadMtl(sharedTexturesMaterial2Path, options);
+        expect(materials.length).toBe(1);
+        const material = materials[0];
+        const pbr = material.pbrMetallicRoughness;
+        expect(pbr.baseColorTexture).toBe(material.emissiveTexture);
+        expect(pbr.baseColorTexture.pixels).toBeUndefined();
+        expect(pbr.baseColorTexture.source).toBeDefined();
+    });
+
     describe('metallicRoughness', () => {
         it('creates default material', () => {
             const material = loadMtl._createMaterial(undefined, options);

specs/lib/loadObjSpec.js

@@ -42,6 +42,10 @@ const objInvalidContentsPath = 'specs/data/box/box.mtl';
 const objConcavePath = 'specs/data/concave/concave.obj';
 const objUnnormalizedPath = 'specs/data/box-unnormalized/box-unnormalized.obj';
 const objMixedAttributesPath = 'specs/data/box-mixed-attributes/box-mixed-attributes.obj';
+const objMissingAttributesPath = 'specs/data/box-missing-attributes/box-missing-attributes.obj';
+const objIncompletePositionsPath = 'specs/data/box-incomplete-attributes/box-incomplete-positions.obj';
+const objIncompleteNormalsPath = 'specs/data/box-incomplete-attributes/box-incomplete-normals.obj';
+const objIncompleteUvsPath = 'specs/data/box-incomplete-attributes/box-incomplete-uvs.obj';
 const objInvalidPath = 'invalid.obj';
 
 function getMeshes(data) {
@@ -499,6 +503,44 @@ describe('loadObj', () => {
         }
     });
 
+    it('ignores missing normals and uvs', async () => {
+        const data = await loadObj(objMissingAttributesPath, options);
+        const primitive = getPrimitives(data)[0];
+        expect(primitive.positions.length).toBeGreaterThan(0);
+        expect(primitive.normals.length).toBe(0);
+        expect(primitive.uvs.length).toBe(0);
+    });
+
+    it('throws when position index is out of bounds', async () => {
+        let thrownError;
+        try {
+            await loadObj(objIncompletePositionsPath, options);
+        } catch (e) {
+            thrownError = e;
+        }
+        expect(thrownError).toEqual(new RuntimeError('Position index 1 is out of bounds'));
+    });
+
+    it('throws when normal index is out of bounds', async () => {
+        let thrownError;
+        try {
+            await loadObj(objIncompleteNormalsPath, options);
+        } catch (e) {
+            thrownError = e;
+        }
+        expect(thrownError).toEqual(new RuntimeError('Normal index 1 is out of bounds'));
+    });
+
+    it('throws when uv index is out of bounds', async () => {
+        let thrownError;
+        try {
+            await loadObj(objIncompleteUvsPath, options);
+        } catch (e) {
+            thrownError = e;
+        }
+        expect(thrownError).toEqual(new RuntimeError('UV index 1 is out of bounds'));
+    });
+
     it('throws when file has invalid contents', async () => {
         let thrownError;
         try {