Merge pull request #161 from AnalyticalGraphicsInc/keep-mixed-attributes

Keep mixed attributes

CHANGES.md

@@ -1,6 +1,10 @@
 Change Log
 ==========
 
+### 2.3.2 ????-??-??
+
+* Improved parsing of faces with mismatching attributes. [#161](https://github.com/AnalyticalGraphicsInc/obj2gltf/pull/161)
+
 ### 2.3.1 2018-10-16
 
 * Improved parsing models with concave or n-sided faces. [#157](https://github.com/AnalyticalGraphicsInc/obj2gltf/pull/157)

lib/loadObj.js

@@ -122,25 +122,44 @@ function loadObj(objPath, options) {
         vertexCount = 0;
     }
 
-    function useMaterial(name) {
+    function reusePrimitive(callback) {
-        var material = getName(name);
-        activeMaterial = material;
-
-        // Look to see if this material has already been used by a primitive in the mesh
         var primitives = mesh.primitives;
         var primitivesLength = primitives.length;
         for (var i = 0; i < primitivesLength; ++i) {
-            if (primitives[i].material === material) {
+            if (primitives[i].material === activeMaterial) {
-                primitive = primitives[i];
+                if (!defined(callback) || callback(primitives[i])) {
-                clearVertexCache();
+                    primitive = primitives[i];
-                vertexCount = primitive.positions.length / 3;
+                    clearVertexCache();
-                return;
+                    vertexCount = primitive.positions.length / 3;
+                    return;
+                }
             }
         }
-        // Add a new primitive with this material
         addPrimitive();
     }
 
+    function useMaterial(name) {
+        activeMaterial = getName(name);
+        reusePrimitive();
+    }
+
+    function faceAndPrimitiveMatch(uvs, normals, primitive) {
+        var faceHasUvs = uvs[0].length > 0;
+        var faceHasNormals = normals[0].length > 0;
+        var primitiveHasUvs = primitive.uvs.length > 0;
+        var primitiveHasNormals = primitive.normals.length > 0;
+        return primitiveHasUvs === faceHasUvs && primitiveHasNormals === faceHasNormals;
+    }
+
+    function checkPrimitive(uvs, normals) {
+        var firstFace = primitive.indices.length === 0;
+        if (!firstFace && !faceAndPrimitiveMatch(uvs, normals, primitive)) {
+            reusePrimitive(function(primitive) {
+                return faceAndPrimitiveMatch(uvs, normals, primitive);
+            });
+        }
+    }
+
     function getOffset(a, attributeData, components) {
         var i = parseInt(a);
         if (i < 0) {
@@ -261,16 +280,8 @@ function loadObj(objPath, options) {
     function addFace(vertices, positions, uvs, normals) {
         var i;
         var isWindingCorrect;
-        var firstFace = primitive.indices.length === 0;
-        var faceHasUvs = uvs[0].length > 0;
-        var faceHasNormals = normals[0].length > 0;
-        var primitiveHasUvs = primitive.uvs.length > 0;
-        var primitiveHasNormals = primitive.normals.length > 0;
 
-        if (!firstFace && (faceHasUvs !== primitiveHasUvs || faceHasNormals !== primitiveHasNormals)) {
+        checkPrimitive(uvs, normals);
-            // Discard faces that don't use the same attributes
-            return;
-        }
 
         if (vertices.length === 3) {
             isWindingCorrect = checkWindingCorrect(positions[0], positions[1], positions[2], normals[0]);

specs/lib/loadObjSpec.js

@@ -494,11 +494,15 @@ describe('loadObj', function() {
             }), done).toResolve();
     });
 
-    it('discards faces that don\'t use the same attributes as other faces in the primitive', function(done) {
+    it('separates faces that don\'t use the same attributes as other faces in the primitive', function(done) {
         expect(loadObj(objMixedAttributesPath, options)
             .then(function(data) {
-                var primitive = getPrimitives(data)[0];
+                var primitives = getPrimitives(data);
-                expect(primitive.indices.length).toBe(18); // 3 faces removed
+                expect(primitives.length).toBe(4);
+                expect(primitives[0].indices.length).toBe(18); // 6 faces
+                expect(primitives[1].indices.length).toBe(6); // 2 faces
+                expect(primitives[2].indices.length).toBe(6); // 2 faces
+                expect(primitives[3].indices.length).toBe(6); // 2 faces
             }), done).toResolve();
     });