Group materialIndex ignored when converting groups to faces in Geometry.fromBufferGeometry

[fallenoak]

It appears that Geometry's fromBufferGeometry() function ignores materialIndex in groups when converting groups to faces. Unless I'm mistaken, the fix should be as simple as extending addFace() in fromBufferGeometry() to support passing in the group's materialIndex and assigning it to the created Face3.

Happy to submit a PR if someone can confirm the current behavior isn't intended.

[WestLangley]

Nice find! I expect that was an oversight.

Aside: The method might not get a lot of use. Do you mind explaining your use case for converting from BufferGeometry to Geometry?

[fallenoak]

I'm afraid it's not a very compelling one-- I was hunting for a way to easily merge indexed BufferGeometry objects together, and was testing if I could merge them as regular Geometry objects as a first approach.

The merge worked fine, but all the materials were set to the first material in the MultiMaterial for the Mesh I subsequently created. (The geometries in question use MultiMaterial).

[WestLangley]

OK. Thanks for the reply... I wonder if there is a compelling rule for when it makes sense to merge geometries that require multiple materials?...

[jmstein]

Hey,

It looked like this was resolved (addFace() appears to take in materialIndex now), but it appears to only call it if "geometry.index !== null" (see Geometry.js:228 and Geometry.js:296). I'm having a tough time understanding why this should be the case (though I'm definitely fairly new to three.js).

From reading posts (including this one), it seems like fromBufferGeometry is not expected to get much use, which is surprising to me, perhaps I'm doing something weird. My use case is loading a model (via OBJLoader), which returns a BufferGeometry. Then I wish to make it deformable for cloth simulation and Geometry seems like the right object to use.

For now, I have this following workaround, though it feels like this should not be necessary:

extractGeometryFromBuffer = function(bufferGeo) {
  var geometry = new THREE.Geometry().fromBufferGeometry(bufferGeo);

  // This appears to not get copied over properly, not clear why
  for (let group of bufferGeo.groups) {
    let start = group.start / 3;
    let end = start + group.count / 3;
    for (let f = start; f < end; ++f) {
      geometry.faces[f].materialIndex = group.materialIndex;
    }
  }

  geometry.computeFaceNormals();
  geometry.mergeVertices();
  geometry.computeVertexNormals();

  return geometry;
};

Also, thanks WestLangley for your responses to the issues here and on SO, they've been really helpful a number of times!

[mrdoob]

My use case is loading a model (via OBJLoader), which returns a BufferGeometry. Then I wish to make it deformable for cloth simulation and Geometry seems like the right object to use.

Hmm, why does Geometry seem like the right object to use?

[jmstein]

I need to modify positions on the fly and know face information to compute collisions.

It's hard to use for iterating over faces to compute how a face has collided, since it doesn't have "face 4 has vertices 2, 34, and 35" type information like Geometry. I could have some other data structure and update the BufferGeometry, but that seems awkward. My understanding is that I need to update each vertex for each face with BufferGeometry. Geometry provides a much cleaner interface to keep it updated.

If the main concern is performance, the cloth simulation is by far the bottleneck, the scene renders at 60fps easily.

[jmstein]

Is this an unsupported / discouraged usage pattern?

[mrdoob]

It will eventually be yeah... Internally, Geometry gets converted to BufferGeometry before rendering. So if you create a BufferGeometry is less work for the engine to do and also less memory usage.