Code and test fixes

lib/loadObj.js

@@ -479,7 +479,7 @@ function loadMaterials(mtlPaths, objPath, options) {
     return Promise.map(mtlPaths, function(mtlPath) {
         mtlPath = normalizeMtlPath(mtlPath, objDirectory);
         var shallowPath = path.join(objDirectory, path.basename(mtlPath));
-        if (options.secure && outsideDirectory(mtlPath, objPath)) {
+        if (options.secure && outsideDirectory(mtlPath, objDirectory)) {
             // Try looking for the .mtl in the same directory as the obj
             options.logger('The material file is outside of the obj directory and the secure flag is true. Attempting to read the material file from within the obj directory instead.');
             return loadMtl(shallowPath)
@@ -511,10 +511,8 @@ function loadMaterials(mtlPaths, objPath, options) {
         });
 }
 
-function loadImages(imagePaths, objPath, options) {
-    var images = {};
+function loadImagePath(imagePath, objPath, options) {
     var objDirectory = path.dirname(objPath);
-    return Promise.map(imagePaths, function(imagePath) {
     var shallowPath = path.join(objDirectory, path.basename(imagePath));
     if (options.secure && outsideDirectory(imagePath, objDirectory)) {
         // Try looking for the image in the same directory as the obj
@@ -524,7 +522,7 @@ function loadImages(imagePaths, objPath, options) {
                 options.logger(error.message);
                 options.logger('Could not read image file at ' + shallowPath + '. This image will be ignored');
             });
-        } else {
+    }
     return loadImage(imagePath, options)
         .catch(function(error) {
             // Try looking for the image in the same directory as the obj
@@ -536,9 +534,17 @@ function loadImages(imagePaths, objPath, options) {
             options.logger(error.message);
             options.logger('Could not read image file at ' + shallowPath + '. This image will be ignored.');
         });
-        }
+}
+
+function loadImages(imagePaths, objPath, options) {
+    var images = {};
+    return Promise.map(imagePaths, function(imagePath) {
+        return loadImagePath(imagePath, objPath, options)
+            .then(function(image) {
+                images[imagePath] = image;
+            });
     }, {concurrency : 10})
-        .then(function(images) {
+        .then(function() {
             return images;
         });
 }

specs/data/box-complex-material-alpha/box-complex-material-alpha.mtl

@@ -0,0 +1,20 @@
+# Blender MTL File: 'None'
+# Material Count: 1
+
+newmtl Material
+Ns 96.078431
+Ka 0.200000 0.200000 0.200000
+Kd 0.640000 0.640000 0.640000
+Ks 0.500000 0.500000 0.500000
+Ke 0.100000 0.100000 0.100000
+Ni 1.000000
+d 0.900000
+Tr 0.100000
+map_Ka ambient.gif
+map_Ke emission.jpg
+map_Kd diffuse.png
+map_Ks specular.jpeg
+map_Ns shininess.png
+map_Bump bump.png
+map_d alpha.png
+illum 2

specs/data/box-complex-material-alpha/box-complex-material-alpha.obj

@@ -0,0 +1,46 @@
+# Blender v2.78 (sub 0) OBJ File: ''
+# www.blender.org
+mtllib box-complex-material-alpha.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/data/box-complex-material/box-complex-material.mtl

@@ -16,5 +16,4 @@ map_Kd diffuse.png
 map_Ks specular.jpeg
 map_Ns shininess.png
 map_Bump bump.png
-map_d alpha.png
 illum 2

specs/data/box-objects-groups-materials/box-objects-groups-materials.obj

@@ -36,7 +36,7 @@ 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
-g CubeBlue_CubeBlue_Blue
+g Blue
 usemtl Blue
 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
@@ -79,7 +79,7 @@ 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
-g CubeGreen_CubeGreen_Green
+g Green
 usemtl Green
 f 9/21/7 10/22/7 12/23/7 11/24/7
 f 11/25/8 12/26/8 16/27/8 15/28/8
@@ -122,7 +122,7 @@ 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
-g CubeRed_CubeRed_Red
+g Red
 usemtl Red
 f 17/41/13 18/42/13 20/43/13 19/44/13
 f 19/45/14 20/46/14 24/47/14 23/48/14

specs/data/box/box.mtl

@@ -3,10 +3,10 @@
 
 newmtl Material
 Ns 96.078431
-Ka 0.000000 0.000000 0.000000
+Ka 0.100000 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
+Ke 0.000000 0.000000 0.100000
 Ni 1.000000
 d 1.000000
 illum 2

specs/lib/createGltfSpec.js

@@ -17,7 +17,6 @@ var diffuseTextureUrl = 'specs/data/box-textured/cesium.png';
 var transparentDiffuseTextureUrl = 'specs/data/box-complex-material/diffuse.png';
 
 var defaultOptions = obj2gltf.defaults;
-var defined = Cesium.defined;
 var checkTransparencyOptions = clone(defaultOptions);
 checkTransparencyOptions.checkTransparency = true;
 
@@ -25,7 +24,6 @@ describe('createGltf', function() {
     var boxObjData;
     var duplicateBoxObjData;
     var groupObjData;
-    var mixedAttributesObjData;
     var diffuseTexture;
     var transparentDiffuseTexture;
 
@@ -50,12 +48,7 @@ describe('createGltf', function() {
             loadImage(transparentDiffuseTextureUrl, checkTransparencyOptions)
                 .then(function(image) {
                    transparentDiffuseTexture = image;
-                }),
-            loadObj(mixedAttributesObjUrl, defaultOptions)
-                .then(function(data) {
-                    mixedAttributesObjData = data;
                 })
-
         ]).then(done);
     });
 
@@ -266,8 +259,8 @@ describe('createGltf', function() {
         boxObjData.nodes[1].meshes[0].primitives[0].normals.length = 0;
 
         var gltf = createGltf(boxObjData, defaultOptions);
-        var kmc1 = gltf.materials.Material.extensions.KHR_materials_common;
-        var kmc2 = gltf.materials.Material_constant.extensions.KHR_materials_common;
+        var kmc1 = gltf.materials['Material'].extensions.KHR_materials_common;
+        var kmc2 = gltf.materials['Material-2'].extensions.KHR_materials_common;
 
         expect(kmc1.technique).toBe('PHONG');
         expect(kmc2.technique).toBe('CONSTANT');
@@ -279,8 +272,8 @@ describe('createGltf', function() {
         boxObjData.nodes[0].meshes[0].primitives[0].normals.length = 0;
 
         var gltf = createGltf(boxObjData, defaultOptions);
-        var kmc1 = gltf.materials.Material.extensions.KHR_materials_common;
-        var kmc2 = gltf.materials.Material_shaded.extensions.KHR_materials_common;
+        var kmc1 = gltf.materials['Material'].extensions.KHR_materials_common;
+        var kmc2 = gltf.materials['Material-2'].extensions.KHR_materials_common;
 
         expect(kmc1.technique).toBe('CONSTANT');
         expect(kmc2.technique).toBe('PHONG');
@@ -317,62 +310,6 @@ describe('createGltf', function() {
         expect(attributes.TEXCOORD_0).toBeUndefined();
     });
 
-    function getDiffuse(material) {
-        return material.extensions.KHR_materials_common.values.diffuse;
-    }
-
-    fit('splits incompatible materials', function() {
-        var gltf = createGltf(mixedAttributesObjData, defaultOptions);
-        var materials = gltf.materials;
-        var materialNames = Object.keys(materials).sort();
-
-        // Expect three copies of each material for
-        // * positions/normals/uvs
-        // * positions/normals
-        // * positions/uvs
-        expect(materialNames).toEqual([
-            'Material',
-            'Material-2',
-            'Material-3',
-            'Missing',
-            'Missing-2',
-            'Missing-3',
-            'default',
-            'default-2',
-            'default-3'
-        ]);
-        console.log(materials['Material']);
-        console.log(materials['Material-2']);
-        console.log(materials['Material-3']);
-
-
-        expect(getDiffuse(materials['Material'])).toBe('texture_cesium');
-        expect(getDiffuse(materials['Material-2'])).toEqual('texture_cesium');
-        //expect(getDiffuse(materials['Material-3'])).toBe('texture_cesium');
-        // expect(getDiffuse(materials['Missing'])).toEqual([0.0, 0.0, 0.0, 1.0]);
-        // expect(getDiffuse(materials['Missing-2'])).toEqual([0.0, 0.0, 0.0, 1.0]);
-        // expect(getDiffuse(materials['Missing-3'])).toEqual([0.0, 0.0, 0.0, 1.0]);
-        // expect(getDiffuse(materials['default'])).toEqual([0.0, 0.0, 0.0, 1.0]);
-        // expect(getDiffuse(materials['default-2'])).toEqual([0.0, 0.0, 0.0, 1.0]);
-        // expect(getDiffuse(materials['default-3'])).toEqual([0.0, 0.0, 0.0, 1.0]);
-
-        // // Test that primitives without uvs reference materials without textures
-        // for (var meshName in meshes) {
-        //     if (meshes.hasOwnProperty(meshName)) {
-        //         var mesh = meshes[meshName];
-        //         var primitives = mesh.primitives;
-        //         var primitivesLength = primitives.length;
-        //         for (var i = 0; i < primitivesLength; ++i) {
-        //             var primitive = primitives[i];
-        //             var material = materials[primitive.material];
-        //             if (!defined(primitive.attributes.TEXCOORD_0)) {
-        //                 expect(material.extensions.KHR_materials_common.diffuse).toEqual([ 0.5, 0.5, 0.5, 1 ]);
-        //             }
-        //         }
-        //     }
-        // }
-    });
-
     function expandObjData(objData, duplicatesLength) {
         var primitive = objData.nodes[0].meshes[0].primitives[0];
         var indices = primitive.indices;

specs/lib/loadMtlSpec.js

@@ -2,7 +2,7 @@
 var path = require('path');
 var loadMtl = require('../../lib/loadMtl');
 
-var complexMaterialUrl = 'specs/data/box-complex-material/box-complex-material.mtl';
+var complexMaterialAlphaUrl = 'specs/data/box-complex-material-alpha/box-complex-material-alpha.mtl';
 var diffuseAmbientSameMaterialUrl = 'specs/data/box-diffuse-ambient-same/box-diffuse-ambient-same.mtl';
 var multipleMaterialsUrl = 'specs/data/box-multiple-materials/box-multiple-materials.mtl';
 var texturedWithOptionsMaterialUrl = 'specs/data/box-texture-options/box-texture-options.mtl';
@@ -14,7 +14,7 @@ function getImagePath(objPath, relativePath) {
 
 describe('loadMtl', function() {
     it('loads complex material', function(done) {
-        expect(loadMtl(complexMaterialUrl)
+        expect(loadMtl(complexMaterialAlphaUrl)
             .then(function(materials) {
                 var material = materials.Material;
                 expect(material).toBeDefined();
@@ -24,13 +24,13 @@ describe('loadMtl', function() {
                 expect(material.specularColor).toEqual([0.5, 0.5, 0.5, 1.0]);
                 expect(material.specularShininess).toEqual(96.078431);
                 expect(material.alpha).toEqual(0.9);
-                expect(material.ambientTexture).toEqual(getImagePath(complexMaterialUrl, 'ambient.gif'));
-                expect(material.emissionTexture).toEqual(getImagePath(complexMaterialUrl, 'emission.jpg'));
-                expect(material.diffuseTexture).toEqual(getImagePath(complexMaterialUrl, 'diffuse.png'));
-                expect(material.specularTexture).toEqual(getImagePath(complexMaterialUrl, 'specular.jpeg'));
-                expect(material.specularShininessMap).toEqual(getImagePath(complexMaterialUrl, 'shininess.png'));
-                expect(material.normalMap).toEqual(getImagePath(complexMaterialUrl, 'bump.png'));
-                expect(material.alphaMap).toEqual(getImagePath(complexMaterialUrl, 'alpha.png'));
+                expect(material.ambientTexture).toEqual(getImagePath(complexMaterialAlphaUrl, 'ambient.gif'));
+                expect(material.emissionTexture).toEqual(getImagePath(complexMaterialAlphaUrl, 'emission.jpg'));
+                expect(material.diffuseTexture).toEqual(getImagePath(complexMaterialAlphaUrl, 'diffuse.png'));
+                expect(material.specularTexture).toEqual(getImagePath(complexMaterialAlphaUrl, 'specular.jpeg'));
+                expect(material.specularShininessMap).toEqual(getImagePath(complexMaterialAlphaUrl, 'shininess.png'));
+                expect(material.normalMap).toEqual(getImagePath(complexMaterialAlphaUrl, 'bump.png'));
+                expect(material.alphaMap).toEqual(getImagePath(complexMaterialAlphaUrl, 'alpha.png'));
             }), done).toResolve();
     });
 
@@ -45,29 +45,27 @@ describe('loadMtl', function() {
     });
 
     it('loads mtl with textures having options', function(done) {
-        options.metallicRoughness = true;
         expect(loadMtl(texturedWithOptionsMaterialUrl)
             .then(function(materials) {
-                expect(materials.length).toBe(1);
-                var material = materials[0];
-                var pbr = material.pbrMetallicRoughness;
-                expect(pbr.baseColorTexture).toBeDefined();
-                expect(pbr.metallicRoughnessTexture).toBeDefined();
-                expect(pbr.baseColorFactor).toEqual([1.0, 1.0, 1.0, 0.9]);
-                expect(pbr.metallicFactor).toBe(1.0);
-                expect(pbr.roughnessFactor).toBe(1.0);
-                expect(material.name).toBe('Material');
-                expect(material.emissiveTexture).toBeDefined();
-                expect(material.normalTexture).toBeDefined();
-                expect(material.occlusionTexture).toBeDefined();
-                expect(material.emissiveFactor).toEqual([1.0, 1.0, 1.0]);
-                expect(material.alphaMode).toBe('BLEND');
-                expect(material.doubleSided).toBe(true);
+                var material = materials.Material;
+                expect(material).toBeDefined();
+                expect(material.ambientColor).toEqual([0.2, 0.2, 0.2, 1.0]);
+                expect(material.emissionColor).toEqual([0.1, 0.1, 0.1, 1.0]);
+                expect(material.diffuseColor).toEqual([0.64, 0.64, 0.64, 1.0]);
+                expect(material.specularColor).toEqual([0.5, 0.5, 0.5, 1.0]);
+                expect(material.specularShininess).toEqual(96.078431);
+                expect(material.alpha).toEqual(0.9);
+                expect(material.ambientTexture).toEqual(getImagePath(texturedWithOptionsMaterialUrl, 'ambient.gif'));
+                expect(material.emissionTexture).toEqual(getImagePath(texturedWithOptionsMaterialUrl, 'emission.jpg'));
+                expect(material.diffuseTexture).toEqual(getImagePath(texturedWithOptionsMaterialUrl, 'diffuse.png'));
+                expect(material.specularTexture).toEqual(getImagePath(texturedWithOptionsMaterialUrl, 'specular.jpeg'));
+                expect(material.specularShininessMap).toEqual(getImagePath(texturedWithOptionsMaterialUrl, 'shininess.png'));
+                expect(material.normalMap).toEqual(getImagePath(texturedWithOptionsMaterialUrl, 'bump.png'));
             }), done).toResolve();
     });
 
     it('ambient texture is ignored if it is the same as the diffuse texture', function(done) {
-        expect(loadMtl(diffuseAmbientSameMaterialUrl, options)
+        expect(loadMtl(diffuseAmbientSameMaterialUrl)
             .then(function(materials) {
                 expect(Object.keys(materials).length).toBe(1);
                 var material = materials['Material'];

specs/lib/loadObjSpec.js

@@ -122,8 +122,8 @@ describe('loadObj', function() {
         expect(loadObj(objUnnormalizedUrl, defaultOptions)
             .then(function(data) {
                 var scratchNormal = new Cesium.Cartesian3();
-                var mesh = getMeshes(data)[0];
-                var normals = mesh.normals;
+                var primitive = getPrimitives(data)[0];
+                var normals = primitive.normals;
                 var normalsLength = normals.length / 3;
                 for (var i = 0; i < normalsLength; ++i) {
                     var normalX = normals.get(i * 3);
@@ -365,7 +365,11 @@ describe('loadObj', function() {
     });
 
     it('loads obj with missing mtllib', function(done) {
-        expect(loadObj(objMissingMtllibUrl, defaultOptions)
+        var options = clone(defaultOptions);
+        var spy = jasmine.createSpy('logger');
+        options.logger = spy;
+
+        expect(loadObj(objMissingMtllibUrl, options)
             .then(function(data) {
                 expect(data.materials).toEqual({});
                 expect(spy.calls.argsFor(0)[0].indexOf('ENOENT') >= 0).toBe(true);
@@ -378,7 +382,7 @@ describe('loadObj', function() {
     it('loads obj with missing usemtl', function(done) {
         expect(loadObj(objMissingUsemtlUrl, defaultOptions)
             .then(function(data) {
-                expect(data.materials.length).toBe(1);
+                expect(Object.keys(data.materials).length).toBe(1);
                 expect(data.nodes[0].meshes[0].primitives[0].material).toBe('Material');
             }), done).toResolve();
     });
@@ -394,6 +398,8 @@ describe('loadObj', function() {
 
     it('does not load resources outside of the obj directory when secure is true', function(done) {
         var options = clone(defaultOptions);
+        var spy = jasmine.createSpy('logger');
+        options.logger = spy;
         options.secure = true;
 
         expect(loadObj(objExternalResourcesUrl, options)
@@ -409,20 +415,23 @@ describe('loadObj', function() {
     });
 
     it('loads resources from root directory when the .mtl path does not exist', function(done) {
-        expect(loadObj(objResourcesInRootUrl, options)
+        expect(loadObj(objResourcesInRootUrl, defaultOptions)
             .then(function(data) {
-                expect(data.materials['Material'].diffuseTexture.source).toBeDefined();
-                expect(diffuseTexture.source).toBeDefined();
+                var material = data.materials['Material'];
+                var image = data.images[material.diffuseTexture];
+                expect(image.source).toBeDefined();
             }), done).toResolve();
     });
 
     it('loads resources from root directory when the .mtl path is outside of the obj directory and secure is true', function(done) {
+        var options = clone(defaultOptions);
         options.secure = true;
         expect(loadObj(objExternalResourcesInRootUrl, options)
             .then(function(data) {
-                var materials = data.materials;
-                expect(Object.keys(materials).length).toBe(2);
-                expect(materials['MaterialTextured'].diffuseTexture.source).toBeDefined();
+                expect(Object.keys(data.materials).length).toBe(2);
+                var material = data.materials['MaterialTextured'];
+                var image = data.images[material.diffuseTexture];
+                expect(image.source).toBeDefined();
             }), done).toResolve();
     });
 
@@ -436,7 +445,11 @@ describe('loadObj', function() {
     });
 
     it('loads obj with missing texture', function(done) {
-        expect(loadObj(objMissingTextureUrl, defaultOptions)
+        var options = clone(defaultOptions);
+        var spy = jasmine.createSpy('logger');
+        options.logger = spy;
+
+        expect(loadObj(objMissingTextureUrl, options)
             .then(function(data) {
                 var imagePath = getImagePath(objMissingTextureUrl, 'cesium.png');
                 expect(data.images[imagePath]).toBeUndefined();