GSOC 26: multi-material audit, edge cases, and a windows path fix

src/webgl/loading.js

@@ -13,7 +13,7 @@
   try {
     const response = await fetch(url, { method: 'HEAD' });
     return response.ok;
   } catch (error) {
     return false;
   }
 }
@@ -157,7 +157,12 @@
 
   const slash = modelPath.lastIndexOf('/');
   const folder = slash >= 0 ? modelPath.slice(0, slash) : '';
-  const resolve = file => (folder ? `${folder}/${file}` : file);
+  // mtl files exported on windows can use backslashes, which mean nothing to a
+  // url, so swap them for the separator the fetch actually needs
+  const resolve = file => {
+    const path = file.replace(/\\/g, '/');
+    return folder ? `${folder}/${path}` : path;
+  };
 
   const jobs = [];
   for (const name in materials) {
@@ -629,7 +634,7 @@
         const parsedMaterials = await Promise.all(parsedMaterialPromises);
         const materials = Object.assign({}, ...parsedMaterials);
         return materials;
       } catch (error) {
         return {};
       }
     }
@@ -740,7 +745,7 @@
     // material per kept face, aligned with model.faces, for bucketing later
     const faceMaterials = [];
     let hasColoredVertices = false;
     let hasColorlessVertices = false;
     for (let line = 0; line < lines.length; ++line) {
       // Each line is a separate object (vertex, face, vertex normal, etc)
       // For each line, split it into tokens on whitespace. The first token

test/unit/assets/duplicate_usemtl.mtl

@@ -0,0 +1,5 @@
+newmtl a
+Kd 1 0 0
+
+newmtl b
+Kd 0 0 1

test/unit/assets/duplicate_usemtl.obj

@@ -0,0 +1,17 @@
+mtllib duplicate_usemtl.mtl
+v -1 -1 0
+v 1 -1 0
+v 1 1 0
+v -1 -1 2
+v 1 -1 2
+v 1 1 2
+v -1 -1 4
+v 1 -1 4
+v 1 1 4
+vn 0 0 1
+usemtl a
+f 1//1 2//1 3//1
+usemtl b
+f 4//1 5//1 6//1
+usemtl a
+f 7//1 8//1 9//1

test/unit/assets/multi_material_2.mtl

@@ -0,0 +1,5 @@
+newmtl red
+Kd 1 0 0
+
+newmtl blue
+Kd 0 0 1

test/unit/assets/multi_material_2.obj

@@ -0,0 +1,20 @@
+mtllib multi_material_2.mtl
+v -1 -1 0
+v 1 -1 0
+v 1 1 0
+v -1 1 0
+v -1 -1 2
+v 1 -1 2
+v 1 1 2
+v -1 1 2
+vt 0 0
+vt 1 0
+vt 1 1
+vt 0 1
+vn 0 0 1
+usemtl red
+f 1/1/1 2/2/1 3/3/1
+f 1/1/1 3/3/1 4/4/1
+usemtl blue
+f 5/1/1 6/2/1 7/3/1
+f 5/1/1 7/3/1 8/4/1

test/unit/assets/no_mtl.obj

@@ -0,0 +1,5 @@
+v -1 -1 0
+v 1 -1 0
+v 1 1 0
+vn 0 0 1
+f 1//1 2//1 3//1

test/unit/assets/no_normals.mtl

@@ -0,0 +1,5 @@
+newmtl one
+Kd 1 0 0
+
+newmtl two
+Kd 0 1 0

test/unit/assets/no_normals.obj

@@ -0,0 +1,11 @@
+mtllib no_normals.mtl
+v -1 -1 0
+v 1 -1 0
+v 1 1 0
+v -1 -1 2
+v 1 -1 2
+v 1 1 2
+usemtl one
+f 1 2 3
+usemtl two
+f 4 5 6

test/unit/assets/unknown_material.mtl

@@ -0,0 +1,2 @@
+newmtl known
+Kd 0 1 0

test/unit/assets/unknown_material.obj

@@ -0,0 +1,12 @@
+mtllib unknown_material.mtl
+v -1 -1 0
+v 1 -1 0
+v 1 1 0
+v -1 -1 2
+v 1 -1 2
+v 1 1 2
+vn 0 0 1
+usemtl known
+f 1//1 2//1 3//1
+usemtl ghost
+f 4//1 5//1 6//1

test/unit/assets/windows_path.mtl

@@ -0,0 +1,6 @@
+newmtl winA
+Kd 1 1 1
+map_Kd textures\cat.jpg
+
+newmtl winB
+Kd 0 1 1

test/unit/assets/windows_path.obj

@@ -0,0 +1,15 @@
+mtllib windows_path.mtl
+v -1 -1 0
+v 1 -1 0
+v 1 1 0
+v -1 -1 2
+v 1 -1 2
+v 1 1 2
+vt 0 0
+vt 1 0
+vt 1 1
+vn 0 0 1
+usemtl winA
+f 1/1/1 2/2/1 3/3/1
+usemtl winB
+f 4/1/1 5/2/1 6/3/1

test/unit/io/loadModel.js

@@ -307,4 +307,79 @@ suite('loadModel', function () {
       'Face count should match'
     );
   });
+
+  suite('multi-material edge cases', function () {
+    test('a two-material OBJ splits into one part per material', async function () {
+      const model = await mockP5Prototype.loadModel(
+        '/test/unit/assets/multi_material_2.obj'
+      );
+      assert.equal(model.parts.length, 2);
+      assert.deepEqual(model.parts[0].partState.fill, [1, 0, 0, 1]);
+      assert.deepEqual(model.parts[1].partState.fill, [0, 0, 1, 1]);
+      // every face lands in exactly one part
+      const total = model.parts.reduce((sum, p) => sum + p.faces.length, 0);
+      assert.equal(total, model.faces.length);
+    });
+
+    test('usemtl naming a material the mtl does not define still loads', async function () {
+      const model = await mockP5Prototype.loadModel(
+        '/test/unit/assets/unknown_material.obj'
+      );
+      // the unknown group falls back to an empty material instead of throwing
+      assert.equal(model.parts.length, 2);
+      assert.deepEqual(model.parts[0].partState.fill, [0, 1, 0, 1]);
+      assert.isNull(model.parts[1].partState.fill);
+      const total = model.parts.reduce((sum, p) => sum + p.faces.length, 0);
+      assert.equal(total, model.faces.length);
+    });
+
+    test('a material used in two separate groups collects into one part', async function () {
+      const model = await mockP5Prototype.loadModel(
+        '/test/unit/assets/duplicate_usemtl.obj'
+      );
+      // grouping is by material name, so the two 'a' groups share a part
+      assert.equal(model.parts.length, 2);
+      assert.deepEqual(
+        model.parts.map(p => p.faces.length).sort(),
+        [1, 2]
+      );
+      const total = model.parts.reduce((sum, p) => sum + p.faces.length, 0);
+      assert.equal(total, model.faces.length);
+    });
+
+    test('an OBJ with no vn lines gets normals computed per part', async function () {
+      const model = await mockP5Prototype.loadModel(
+        '/test/unit/assets/no_normals.obj'
+      );
+      assert.equal(model.parts.length, 2);
+      for (const part of model.parts) {
+        assert.equal(part.vertexNormals.length, part.vertices.length);
+      }
+    });
+
+    test('an OBJ with no mtllib loads as plain geometry', async function () {
+      const model = await mockP5Prototype.loadModel(
+        '/test/unit/assets/no_mtl.obj'
+      );
+      assert.isAbove(model.vertices.length, 0);
+      assert.isAbove(model.faces.length, 0);
+    });
+
+    test('windows-style texture paths are resolved with forward slashes', async function () {
+      const requested = [];
+      mockP5Prototype.loadImage = async url => {
+        requested.push(url);
+        return { width: 1, height: 1 };
+      };
+      try {
+        await mockP5Prototype.loadModel('/test/unit/assets/windows_path.obj');
+        // the mtl writes `textures\cat.jpg`, which no server would resolve
+        assert.equal(requested.length, 1);
+        assert.notInclude(requested[0], '\\');
+        assert.include(requested[0], 'textures/cat.jpg');
+      } finally {
+        delete mockP5Prototype.loadImage;
+      }
+    });
+  });
 });

test/unit/webgl/p5.Geometry.js

@@ -507,5 +507,51 @@ suite('p5.Geometry', function () {
         fillSpy.mockRestore();
       }
     );
+
+    test('drawing a multi-material model leaves the caller material intact',
+      function() {
+        const renderer = myp5.createCanvas(50, 50, myp5.WEBGL);
+        const geom = myp5.buildGeometry(() => {
+          myp5.texture(myp5.createGraphics(10, 10));
+          myp5.box(8);
+          myp5.specularMaterial(255, 0, 0);
+          myp5.shininess(99);
+          myp5.sphere(8);
+        });
+
+        // whatever the sketch set before the call has to survive it, otherwise
+        // the next shape silently picks up the model's material
+        const callerTex = myp5.createGraphics(10, 10);
+        myp5.background(255);
+        myp5.texture(callerTex);
+        myp5.shininess(7);
+        myp5.model(geom);
+
+        expect(renderer.states._tex).toBe(callerTex);
+        expect(renderer.states._useShininess).toEqual(7);
+      }
+    );
+
+    test('a multi-material model can be drawn inside buildGeometry',
+      function() {
+        myp5.createCanvas(50, 50, myp5.WEBGL);
+        const inner = myp5.buildGeometry(() => {
+          myp5.texture(myp5.createGraphics(10, 10));
+          myp5.box(8);
+          myp5.texture(myp5.createGraphics(10, 10));
+          myp5.sphere(8);
+        });
+        expect(inner.parts.length).toEqual(2);
+
+        const outer = myp5.buildGeometry(() => {
+          myp5.model(inner);
+        });
+        // the geometry comes through whole. the materials flatten to whatever
+        // the callback had set, since the builder follows renderer state rather
+        // than the parts of a model handed to it
+        expect(outer.vertices.length).toEqual(inner.vertices.length);
+        expect(outer.faces.length).toEqual(inner.faces.length);
+      }
+    );
   });
 });