From b69281573f0053deda0d2196bf2d0500f4c213d3 Mon Sep 17 00:00:00 2001 From: Sean Lilley Date: Fri, 27 Mar 2020 09:41:37 -0400 Subject: [PATCH 1/4] Remove triangle winding order check --- lib/loadObj.js | 32 +------------------------------- 1 file changed, 1 insertion(+), 31 deletions(-) diff --git a/lib/loadObj.js b/lib/loadObj.js index bb64340..16e53c8 100644 --- a/lib/loadObj.js +++ b/lib/loadObj.js @@ -256,18 +256,6 @@ function loadObj(objPath, options) { return Cartesian3.fromElements(px, py, pz, result); } - function getNormal(index, result) { - const nx = globalNormals.get(index * 3); - const ny = globalNormals.get(index * 3 + 1); - const nz = globalNormals.get(index * 3 + 2); - return Cartesian3.fromElements(nx, ny, nz, result); - } - - const scratch1 = new Cartesian3(); - const scratch2 = new Cartesian3(); - const scratch3 = new Cartesian3(); - const scratch4 = new Cartesian3(); - const scratch5 = new Cartesian3(); const scratchCenter = new Cartesian3(); const scratchAxis1 = new Cartesian3(); const scratchAxis2 = new Cartesian3(); @@ -276,23 +264,6 @@ function loadObj(objPath, options) { const scratchVertexIndices = []; const scratchPoints = []; - function checkWindingCorrect(positionIndex1, positionIndex2, positionIndex3, normalIndex) { - if (!defined(normalIndex)) { - // If no face normal, we have to assume the winding is correct. - return true; - } - const normal = getNormal(normalIndex, scratchNormal); - const A = getPosition(positionIndex1, scratch1); - const B = getPosition(positionIndex2, scratch2); - const C = getPosition(positionIndex3, scratch3); - - const BA = Cartesian3.subtract(B, A, scratch4); - const CA = Cartesian3.subtract(C, A, scratch5); - const cross = Cartesian3.cross(BA, CA, scratch3); - - return (Cartesian3.dot(normal, cross) >= 0); - } - function addTriangle(index1, index2, index3, correctWinding) { if (correctWinding) { primitive.indices.push(index1); @@ -314,11 +285,10 @@ function loadObj(objPath, options) { checkPrimitive(uvs, faceNormals); if (vertices.length === 3) { - const isWindingCorrect = 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]); - addTriangle(index1, index2, index3, isWindingCorrect); + addTriangle(index1, index2, index3, true); } else { // Triangulate if the face is not a triangle const points = scratchPoints; const vertexIndices = scratchVertexIndices; From fb491952fc0d71f0b5c07b37dc2c908a3ee3bf6b Mon Sep 17 00:00:00 2001 From: Sean Lilley Date: Thu, 2 Apr 2020 11:43:05 -0400 Subject: [PATCH 2/4] Update CHANGES --- CHANGES.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGES.md b/CHANGES.md index 72d2e54..ed245ec 100644 --- a/CHANGES.md +++ b/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) +* Remove triangle winding order sanitization. [#236](https://github.com/CesiumGS/obj2gltf/pull/236) ### 3.1.1 - 2021-06-22 From 5fc3ff8d6e634912f2d2e32c7c29f53d75da1c48 Mon Sep 17 00:00:00 2001 From: Sean Lilley Date: Sun, 1 Aug 2021 19:47:25 -0400 Subject: [PATCH 3/4] Add triangle winding order sanitization option --- CHANGES.md | 2 +- README.md | 3 +++ bin/obj2gltf.js | 18 ++++++++++++------ lib/loadObj.js | 36 +++++++++++++++++++++++++++++++++--- lib/obj2gltf.js | 10 +++++++++- 5 files changed, 58 insertions(+), 11 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index ed245ec..a1584ef 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -4,7 +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) -* Remove triangle winding order sanitization. [#236](https://github.com/CesiumGS/obj2gltf/pull/236) +* 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 diff --git a/README.md b/README.md index fd9b0c9..c2f20ec 100644 --- a/README.md +++ b/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 diff --git a/bin/obj2gltf.js b/bin/obj2gltf.js index 1ca3f42..b8ec374 100644 --- a/bin/obj2gltf.js +++ b/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'); diff --git a/lib/loadObj.js b/lib/loadObj.js index 16e53c8..66c636d 100644 --- a/lib/loadObj.js +++ b/lib/loadObj.js @@ -256,6 +256,18 @@ function loadObj(objPath, options) { return Cartesian3.fromElements(px, py, pz, result); } + function getNormal(index, result) { + const nx = globalNormals.get(index * 3); + const ny = globalNormals.get(index * 3 + 1); + const nz = globalNormals.get(index * 3 + 2); + return Cartesian3.fromElements(nx, ny, nz, result); + } + + const scratch1 = new Cartesian3(); + const scratch2 = new Cartesian3(); + const scratch3 = new Cartesian3(); + const scratch4 = new Cartesian3(); + const scratch5 = new Cartesian3(); const scratchCenter = new Cartesian3(); const scratchAxis1 = new Cartesian3(); const scratchAxis2 = new Cartesian3(); @@ -264,6 +276,23 @@ function loadObj(objPath, options) { const scratchVertexIndices = []; const scratchPoints = []; + function checkWindingCorrect(positionIndex1, positionIndex2, positionIndex3, normalIndex) { + if (!defined(normalIndex)) { + // If no face normal, we have to assume the winding is correct. + return true; + } + const normal = getNormal(normalIndex, scratchNormal); + const A = getPosition(positionIndex1, scratch1); + const B = getPosition(positionIndex2, scratch2); + const C = getPosition(positionIndex3, scratch3); + + const BA = Cartesian3.subtract(B, A, scratch4); + const CA = Cartesian3.subtract(C, A, scratch5); + const cross = Cartesian3.cross(BA, CA, scratch3); + + return (Cartesian3.dot(normal, cross) >= 0); + } + function addTriangle(index1, index2, index3, correctWinding) { if (correctWinding) { primitive.indices.push(index1); @@ -276,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); @@ -285,10 +314,11 @@ function loadObj(objPath, options) { checkPrimitive(uvs, faceNormals); if (vertices.length === 3) { + 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]); - addTriangle(index1, index2, index3, true); + addTriangle(index1, index2, index3, isWindingCorrect); } else { // Triangulate if the face is not a triangle const points = scratchPoints; const vertexIndices = scratchVertexIndices; @@ -381,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; diff --git a/lib/obj2gltf.js b/lib/obj2gltf.js index cdb57e8..dd537e2 100644 --- a/lib/obj2gltf.js +++ b/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 }; /** From 49df1a06557d3c16dda4f85064acc967e4faacdf Mon Sep 17 00:00:00 2001 From: Sean Lilley Date: Sun, 1 Aug 2021 20:11:28 -0400 Subject: [PATCH 4/4] Added test --- .../box-incorrect-winding-order.mtl | 10 ++++++ .../box-incorrect-winding-order.obj | 32 +++++++++++++++++++ specs/lib/loadObjSpec.js | 24 ++++++++++++++ 3 files changed, 66 insertions(+) create mode 100644 specs/data/box-incorrect-winding-order/box-incorrect-winding-order.mtl create mode 100644 specs/data/box-incorrect-winding-order/box-incorrect-winding-order.obj diff --git a/specs/data/box-incorrect-winding-order/box-incorrect-winding-order.mtl b/specs/data/box-incorrect-winding-order/box-incorrect-winding-order.mtl new file mode 100644 index 0000000..70d3ba1 --- /dev/null +++ b/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 diff --git a/specs/data/box-incorrect-winding-order/box-incorrect-winding-order.obj b/specs/data/box-incorrect-winding-order/box-incorrect-winding-order.obj new file mode 100644 index 0000000..adad911 --- /dev/null +++ b/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 diff --git a/specs/lib/loadObjSpec.js b/specs/lib/loadObjSpec.js index 3a88628..0db5d32 100644 --- a/specs/lib/loadObjSpec.js +++ b/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 {