From f502b805b8420d4e3ca48d826f9e8d2a774f3be3 Mon Sep 17 00:00:00 2001 From: Sean Lilley Date: Sat, 26 Oct 2019 20:15:10 -0400 Subject: [PATCH 1/5] Handle face signature containing attributes that are missing --- lib/loadObj.js | 6 +++--- .../box-missing-attributes.mtl | 12 +++++++++++ .../box-missing-attributes.obj | 20 +++++++++++++++++++ specs/lib/loadObjSpec.js | 9 +++++++++ 4 files changed, 44 insertions(+), 3 deletions(-) create mode 100644 specs/data/box-missing-attributes/box-missing-attributes.mtl create mode 100644 specs/data/box-missing-attributes/box-missing-attributes.obj diff --git a/lib/loadObj.js b/lib/loadObj.js index 90e4969..392d479 100644 --- a/lib/loadObj.js +++ b/lib/loadObj.js @@ -189,7 +189,7 @@ function loadObj(objPath, options) { function createVertex(p, u, n) { // Positions - if (defined(p)) { + if (defined(p) && (globalPositions.length > 0)) { const px = globalPositions.get(p * 3); const py = globalPositions.get(p * 3 + 1); const pz = globalPositions.get(p * 3 + 2); @@ -199,7 +199,7 @@ function loadObj(objPath, options) { } // Normals - if (defined(n)) { + if (defined(n) && (globalNormals.length > 0)) { const nx = globalNormals.get(n * 3); const ny = globalNormals.get(n * 3 + 1); const nz = globalNormals.get(n * 3 + 2); @@ -209,7 +209,7 @@ function loadObj(objPath, options) { } // UVs - if (defined(u)) { + if (defined(u) && (globalUvs.length > 0)) { const ux = globalUvs.get(u * 2); const uy = globalUvs.get(u * 2 + 1); primitive.uvs.push(ux); diff --git a/specs/data/box-missing-attributes/box-missing-attributes.mtl b/specs/data/box-missing-attributes/box-missing-attributes.mtl new file mode 100644 index 0000000..abbc294 --- /dev/null +++ b/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 diff --git a/specs/data/box-missing-attributes/box-missing-attributes.obj b/specs/data/box-missing-attributes/box-missing-attributes.obj new file mode 100644 index 0000000..1df6678 --- /dev/null +++ b/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 diff --git a/specs/lib/loadObjSpec.js b/specs/lib/loadObjSpec.js index 7a84397..7cb3172 100644 --- a/specs/lib/loadObjSpec.js +++ b/specs/lib/loadObjSpec.js @@ -40,6 +40,7 @@ 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 objInvalidPath = 'invalid.obj'; function getMeshes(data) { @@ -455,6 +456,14 @@ describe('loadObj', () => { expect(primitives[3].indices.length).toBe(6); // 2 faces }); + it('does not add 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 file has invalid contents', async () => { let thrownError; try { From 07182a6d8e341c0ea7c371c2ff2ec886ba71bc05 Mon Sep 17 00:00:00 2001 From: Sean Lilley Date: Sat, 26 Oct 2019 20:57:03 -0400 Subject: [PATCH 2/5] Update CHANGES --- CHANGES.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGES.md b/CHANGES.md index df7736c..4173f79 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -4,6 +4,7 @@ Change Log ### 3.?.? - 2019-??-?? * Added back `inputUpAxis` and `outputUpAxis`. [#211](https://github.com/AnalyticalGraphicsInc/obj2gltf/pull/211) +* Fixed parsing faces that reference non-existing attributes. [#218](https://github.com/AnalyticalGraphicsInc/obj2gltf/pull/218) ### 3.0.4 - 2019-07-22 From 2a7f0e5c7b0400000ca4e95db58e6d465ea7dbf9 Mon Sep 17 00:00:00 2001 From: Sean Lilley Date: Sun, 27 Oct 2019 13:30:51 -0400 Subject: [PATCH 3/5] PR feedback --- specs/lib/loadObjSpec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/specs/lib/loadObjSpec.js b/specs/lib/loadObjSpec.js index 47d1570..a51611e 100644 --- a/specs/lib/loadObjSpec.js +++ b/specs/lib/loadObjSpec.js @@ -497,7 +497,7 @@ describe('loadObj', () => { } }); - it('does not add missing normals and uvs', async() => { + it('does not add missing normals and uvs', async () => { const data = await loadObj(objMissingAttributesPath, options); const primitive = getPrimitives(data)[0]; expect(primitive.positions.length).toBeGreaterThan(0); From e7bd6d6461c3a17b5b73a3471fc17d9da06b0115 Mon Sep 17 00:00:00 2001 From: Sean Lilley Date: Sun, 27 Oct 2019 15:00:35 -0400 Subject: [PATCH 4/5] Check for incomplete attributes --- lib/loadObj.js | 9 +++++ lib/readLines.js | 11 +++++- .../box-incomplete-normals.obj | 36 +++++++++++++++++++ .../box-incomplete-positions.obj | 34 ++++++++++++++++++ .../box-incomplete-uvs.obj | 22 ++++++++++++ specs/lib/loadObjSpec.js | 33 +++++++++++++++++ 6 files changed, 144 insertions(+), 1 deletion(-) create mode 100644 specs/data/box-incomplete-attributes/box-incomplete-normals.obj create mode 100644 specs/data/box-incomplete-attributes/box-incomplete-positions.obj create mode 100644 specs/data/box-incomplete-attributes/box-incomplete-uvs.obj diff --git a/lib/loadObj.js b/lib/loadObj.js index c46b13b..e893ab5 100644 --- a/lib/loadObj.js +++ b/lib/loadObj.js @@ -196,6 +196,9 @@ function loadObj(objPath, options) { function createVertex(p, u, n) { // Positions 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); @@ -206,6 +209,9 @@ function loadObj(objPath, options) { // Normals 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); @@ -216,6 +222,9 @@ function loadObj(objPath, options) { // UVs 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); diff --git a/lib/readLines.js b/lib/readLines.js index d4a4c1b..e740ef1 100644 --- a/lib/readLines.js +++ b/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); }); } diff --git a/specs/data/box-incomplete-attributes/box-incomplete-normals.obj b/specs/data/box-incomplete-attributes/box-incomplete-normals.obj new file mode 100644 index 0000000..328214c --- /dev/null +++ b/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 diff --git a/specs/data/box-incomplete-attributes/box-incomplete-positions.obj b/specs/data/box-incomplete-attributes/box-incomplete-positions.obj new file mode 100644 index 0000000..789bb2d --- /dev/null +++ b/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 diff --git a/specs/data/box-incomplete-attributes/box-incomplete-uvs.obj b/specs/data/box-incomplete-attributes/box-incomplete-uvs.obj new file mode 100644 index 0000000..fb87835 --- /dev/null +++ b/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 diff --git a/specs/lib/loadObjSpec.js b/specs/lib/loadObjSpec.js index a51611e..deef264 100644 --- a/specs/lib/loadObjSpec.js +++ b/specs/lib/loadObjSpec.js @@ -42,6 +42,9 @@ 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) { @@ -505,6 +508,36 @@ describe('loadObj', () => { 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 { From ff26d5d0aedc3496554b2c8bf017161dca05a50d Mon Sep 17 00:00:00 2001 From: Sean Lilley Date: Sun, 27 Oct 2019 15:26:40 -0400 Subject: [PATCH 5/5] eslint fixes --- lib/readLines.js | 2 +- specs/lib/loadObjSpec.js | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/readLines.js b/lib/readLines.js index e740ef1..a566fb5 100644 --- a/lib/readLines.js +++ b/lib/readLines.js @@ -23,7 +23,7 @@ function readLines(path, callback) { const lineReader = readline.createInterface({ input : stream }); - + const callbackWrapper = function(line) { try { callback(line); diff --git a/specs/lib/loadObjSpec.js b/specs/lib/loadObjSpec.js index deef264..6fdf609 100644 --- a/specs/lib/loadObjSpec.js +++ b/specs/lib/loadObjSpec.js @@ -42,9 +42,9 @@ 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 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) { @@ -500,7 +500,7 @@ describe('loadObj', () => { } }); - it('does not add missing normals and uvs', async () => { + it('ignores missing normals and uvs', async () => { const data = await loadObj(objMissingAttributesPath, options); const primitive = getPrimitives(data)[0]; expect(primitive.positions.length).toBeGreaterThan(0);