BufferGeometryUtils: Add mergeBufferGeometries() helper.

[donmccurdy]

Fixes #9468, #6188, and partially addresses #11944.

Alternative to #8162, #9529, #12795, and #12804.

I'd like to have this utility for GLTFLoader, so that primitives can be loaded as BufferGeometry groups. Adds a warning to BufferGeometry.prototype.merge() when it is called without an offset, as I believe the current behavior is probably not what many are expecting.

I think this approach is comparatively complete, covering indexed and non-indexed geometry, preserving morph targets, and creating geometry groups. Users who do not want the groups may simply call .clearGroups() on the result. Although, perhaps a better name would be groupBufferGeometries( geometries )?

[WestLangley]

This is very nice!

I believe it does not make sense to merge geometries that require different materials, as the merged geometry must be rendered with separate draw calls anyway.

If I understand correctly, it seems you are ignoring the existing groups in each geometry, while simultaneously creating new groups by assigning each merged geometry a different material index. (Please correct me if I am wrong.)

I'm thinking a reasonable argument can be made that it makes sense to ignore groups altogether when merging. But I don't have a strong opinion on this one way or the other.

[donmccurdy]

I believe it does not make sense to merge geometries that require different materials, as the merged geometry must be rendered with separate draw calls anyway.

For the multi-materials case I'm thinking of #11944 in GLTFLoader, and just trying to have a simpler single-mesh output when glTF specifies one mesh with multiple materials. But optimizing performance seems like the more important use case for merging geometry, so happy to treat that as the primary goal here, not multiple materials.

If I understand correctly, it seems you are ignoring the existing groups in each geometry, while simultaneously creating new groups by assigning each merged geometry a different material index.

Correct, this code isn't checking for existing groups.

I'm thinking a reasonable argument can be made that it makes sense to ignore groups altogether when merging. But I don't have a strong opinion on this one way or the other.

Groups could still be added after calling this method, yes. It would also be fairly easy to change the signature to: mergeBufferGeometries( geometries, useGroups = false ), creating groups and retaining existing ones only if useGroups=true. I'm fine with either.

[WestLangley]

I'm fine with however you want to handle this. :)

[donmccurdy]

Ok thanks — removed the creation of groups. I think that part could be generalized to a mergeMeshes() method later, where dealing with multiple materials would feel more appropriate.

With that change, this doesn't fully address #11944, but still covers everything else. I'll adjust GLTFLoader accordingly soon.

[donmccurdy]

Removed the GLTFLoader part of this PR for now, since that would need to deal with multiple materials.

[mrdoob]

Do you mind resolving the conflicts? 😇

[donmccurdy]

Done ✅

[mrdoob]

Thanks!

[makc]

too bad you guys lost matrices args along the way. the use case I was hoping to use this for was to group a bunch of separate objects together, but it looks like now I have to explicitly apply their transformations to geometries before merging.

[donmccurdy]

too bad you guys lost matrices args along the way. the use case I was hoping to use this for was to group a bunch of separate objects together, but it looks like now I have to explicitly apply their transformations to geometries before merging.

There are several use cases for merging geometries, and not all involve merging transformations. E.g. my use case had more to do with multi-material meshes. This utility tries to remain generic and handles only the geometry portion of that, but you're welcome to use this to create a mergeMeshes( [ mesh1, mesh2, ... ] ) helper or similar that applies transformations.

[looeee]

The matrix transformation is applied at the Object3D level anyway, it wouldn't make sense to apply it to BufferGeometry when merging since geometries don't have a matrix.

But a mergeMeshes utility that does this would be a nice addition.

[takahirox]

BufferGeometry doesn't have .userData property. Perhaps only GLTFLoader can set from glTF primitive.extras. (Any other loaders/modules can also set?) So, is this merging for GLTFLoader special?

[donmccurdy]

Oh, good point. Yes apparently it is glTF-specific. Probably deserves a comment, or maybe we can revisit whether GLTFLoader should do this..

[takahirox]

I don't think it's a good idea to pollute BufferGeometry with Three.js unofficial property. For example, it can be lost in serialization. I prefer officially adding .userData property to BufferGeometry.

@mrdoob What do you think of adding .userData property to BufferGeometry?