Merge pull request #236 from CesiumGS/remove-triangle-winding-order-check

Remove winding order sanitization for triangles

CHANGES.md

@@ -4,6 +4,7 @@ Change Log
 ### 3.?.? - 2021-??-??
 
 * Removed `minFilter` and `magFilter` from generated samplers so that runtime engines can use their preferred texture filtering. [#240](https://github.com/CesiumGS/obj2gltf/pull/240)
+* Triangle winding order sanitization is longer done by default. Use the `--triangle-winding-order-sanitization` option. [#236](https://github.com/CesiumGS/obj2gltf/pull/236)
 
 ### 3.1.1 - 2021-06-22
 

README.md

@@ -112,6 +112,9 @@ As a convenience the PBR textures may be supplied directly to the command line.
 |`--baseColorTexture`|Path to the baseColor/diffuse texture that should override textures in the .mtl file.|No|
 |`--emissiveTexture`|Path to the emissive texture that should override textures in the .mtl file.|No|
 |`--alphaTexture`|Path to the alpha texture that should override textures in the .mtl file.|No|
+|`--input-up-axis`|Up axis of the obj.|No|
+|`--output-up-axis`|Up axis of the converted glTF.|No|
+|`--triangle-winding-order-sanitization`|Apply triangle winding order sanitization.|No|
 
 ## Build Instructions
 

bin/obj2gltf.js

@@ -91,6 +91,11 @@ const argv = yargs
             type : 'boolean',
             default : defaults.specularGlossiness
         },
+        unlit : {
+            describe : 'The glTF will be saved with the KHR_materials_unlit extension.',
+            type : 'boolean',
+            default : defaults.unlit
+        },
         metallicRoughnessOcclusionTexture : {
             describe : 'Path to the metallic-roughness-occlusion texture that should override textures in the .mtl file, where occlusion is stored in the red channel, roughness is stored in the green channel, and metallic is stored in the blue channel. The model will be saved with a pbrMetallicRoughness material. This is often convenient in workflows where the .mtl does not exist or is not set up to use PBR materials. Intended for models with a single material',
             type : 'string',
@@ -124,11 +129,6 @@ const argv = yargs
         alphaTexture : {
             describe : 'Path to the alpha texture that should override textures in the .mtl file.'
         },
-        unlit : {
-            describe : 'The glTF will be saved with the KHR_materials_unlit extension.',
-            type : 'boolean',
-            default : defaults.unlit
-        },
         inputUpAxis : {
             describe: 'Up axis of the obj.',
             choices: ['X', 'Y', 'Z'],
@@ -140,6 +140,11 @@ const argv = yargs
             choices: ['X', 'Y', 'Z'],
             type: 'string',
             default: 'Y'
+        },
+        triangleWindingOrderSanitization : {
+            describe: 'Apply triangle winding order sanitization.',
+            type: 'boolean',
+            default: defaults.triangleWindingOrderSanitization
         }
     }).parse(args);
 
@@ -187,7 +192,8 @@ const options = {
     overridingTextures : overridingTextures,
     outputDirectory : outputDirectory,
     inputUpAxis : argv.inputUpAxis,
-    outputUpAxis : argv.outputUpAxis
+    outputUpAxis : argv.outputUpAxis,
+    triangleWindingOrderSanitization: argv.triangleWindingOrderSanitization
 };
 
 console.time('Total');

lib/loadObj.js

@@ -305,7 +305,7 @@ function loadObj(objPath, options) {
         }
     }
 
-    function addFace(vertices, positions, uvs, normals) {
+    function addFace(vertices, positions, uvs, normals, triangleWindingOrderSanitization) {
         correctAttributeIndices(positions, globalPositions, 3);
         correctAttributeIndices(normals, globalNormals, 3);
         correctAttributeIndices(uvs, globalUvs, 2);
@@ -314,7 +314,7 @@ function loadObj(objPath, options) {
         checkPrimitive(uvs, faceNormals);
 
         if (vertices.length === 3) {
-            const isWindingCorrect = checkWindingCorrect(positions[0], positions[1], positions[2], normals[0]);
+            const isWindingCorrect = !triangleWindingOrderSanitization || checkWindingCorrect(positions[0], positions[1], positions[2], normals[0]);
             const index1 = addVertex(vertices[0], positions[0], uvs[0], normals[0]);
             const index2 = addVertex(vertices[1], positions[1], uvs[1], normals[1]);
             const index3 = addVertex(vertices[2], positions[2], uvs[2], normals[2]);
@@ -411,7 +411,7 @@ function loadObj(objPath, options) {
                     faceNormals.push(result[3]);
                 }
                 if (faceVertices.length > 2) {
-                    addFace(faceVertices, facePositions, faceUvs, faceNormals);
+                    addFace(faceVertices, facePositions, faceUvs, faceNormals, options.triangleWindingOrderSanitization);
                 }
 
                 faceVertices.length = 0;

lib/obj2gltf.js

@@ -36,6 +36,7 @@ module.exports = obj2gltf;
  * @param {String} [options.overridingTextures.alphaTexture] Path to the alpha texture.
  * @param {String} [options.inputUpAxis='Y'] Up axis of the obj. Choices are 'X', 'Y', and 'Z'.
  * @param {String} [options.outputUpAxis='Y'] Up axis of the converted glTF. Choices are 'X', 'Y', and 'Z'.
+ * @param {String} [options.triangleWindingOrderSanitization=false] Apply triangle winding order sanitization.
  * @param {Logger} [options.logger] A callback function for handling logged messages. Defaults to console.log.
  * @param {Writer} [options.writer] A callback function that writes files that are saved as separate resources.
  * @param {String} [options.outputDirectory] Output directory for writing separate resources when options.writer is not defined.
@@ -58,6 +59,7 @@ function obj2gltf(objPath, options) {
     options.writer = defaultValue(options.writer, getDefaultWriter(options.outputDirectory));
     options.inputUpAxis = defaultValue(options.inputUpAxis, defaults.inputUpAxis);
     options.outputUpAxis = defaultValue(options.outputUpAxis, defaults.outputUpAxis);
+    options.triangleWindingOrderSanitization = defaultValue(options.triangleWindingOrderSanitization, defaults.triangleWindingOrderSanitization);
 
     if (!defined(objPath)) {
         throw new DeveloperError('objPath is required');
@@ -180,7 +182,13 @@ obj2gltf.defaults = {
      * @type String
      * @default 'Y'
      */
-    outputUpAxis: 'Y'
+    outputUpAxis: 'Y',
+    /**
+     * Gets or sets whether triangle winding order sanitization will be applied.
+     * @type Boolean
+     * @default false
+     */
+    windingOrderSanitization : false
 };
 
 /**

specs/data/box-incorrect-winding-order/box-incorrect-winding-order.mtl

@@ -0,0 +1,10 @@
+# Blender MTL File: 'None'
+# Material Count: 1
+
+newmtl None
+Ns 0
+Ka 0.000000 0.000000 0.000000
+Kd 0.8 0.8 0.8
+Ks 0.8 0.8 0.8
+d 1
+illum 2

specs/data/box-incorrect-winding-order/box-incorrect-winding-order.obj

@@ -0,0 +1,32 @@
+# Blender v2.78 (sub 0) OBJ File: ''
+# www.blender.org
+mtllib box-incorrect-winding-order.mtl
+o Cube_Cube.001
+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
+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 None
+s off
+f 1//1 3//1 2//1
+f 4//2 7//2 3//2
+f 8//3 5//3 7//3
+f 6//4 1//4 5//4
+f 7//5 1//5 3//5
+f 4//6 6//6 8//6
+f 2//1 4//1 3//1
+f 4//2 8//2 7//2
+f 8//3 6//3 5//3
+f 6//4 2//4 1//4
+f 7//5 5//5 1//5
+f 4//6 2//6 6//6

specs/lib/loadObjSpec.js

@@ -46,6 +46,7 @@ const objMissingAttributesPath = 'specs/data/box-missing-attributes/box-missing-
 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 objIncorrectWindingOrderPath = 'specs/data/box-incorrect-winding-order/box-incorrect-winding-order.obj';
 const objInvalidPath = 'invalid.obj';
 
 function getMeshes(data) {
@@ -511,6 +512,29 @@ describe('loadObj', () => {
         expect(primitive.uvs.length).toBe(0);
     });
 
+    async function loadAndGetIndices(objPath, options) {
+        const data = await loadObj(objPath, options);
+        const primitive = getPrimitives(data)[0];
+        const indices = primitive.indices;
+        return new Uint16Array(indices.toUint16Buffer().buffer);
+    }
+
+    it('applies triangle winding order sanitization', async () => {
+        options.triangleWindingOrderSanitization = false;
+        const indicesIncorrect = await loadAndGetIndices(objIncorrectWindingOrderPath, options);
+
+        options.triangleWindingOrderSanitization = true;
+        const indicesCorrect = await loadAndGetIndices(objIncorrectWindingOrderPath, options);
+
+        expect(indicesIncorrect[0]).toBe(0);
+        expect(indicesIncorrect[2]).toBe(2);
+        expect(indicesIncorrect[1]).toBe(1);
+
+        expect(indicesCorrect[0]).toBe(0);
+        expect(indicesCorrect[2]).toBe(1);
+        expect(indicesCorrect[1]).toBe(2);
+    });
+
     it('throws when position index is out of bounds', async () => {
         let thrownError;
         try {