Adding BufferGeometryUtils.mergeVertices

[yomboprime]

This adds BufferGeometryUtils.mergeVertices and modifies the webgl_physics_volume example which used the Geometry one, removing completely the dependency of the example to Geometry.

Works with usual attributes, I've not implemented interleaved ones.

[Mugen87]

What happens when you attempt to merge vertices but the corresponding uv-coordinates or vertex normals are actually different? Wouldn't you corrupt the data in this case?

[donmccurdy]

Strong +1 for adding a BufferGeometryUtils.mergeVertices function for BufferGeometry, although I think @gkjohnson's PR #14116 covers some needs (morph targets, configurable tolerance) that this PR does not. Since that PR has stalled for now, merging just this function — after cherry-picking any code we want from the other PR — would work.

[yomboprime]

What happens when you attempt to merge vertices but the corresponding uv-coordinates or vertex normals are actually different? Wouldn't you corrupt the data in this case?

Yes, that is what mapIndices() solves in the example. There is no alternative, you are right. The function is not useful with UVs.

[gkjohnson]

@donmccurdy I'd like to get that PR merged, too. It looks like the milestone keeps getting pushed back but it's unclear why. Maybe @mrdoob can shed some light? I'm happy to make any needed changes to get it merged. If BufferGeometryUtils is the right place for it we can do that to.

[yomboprime]

Strong +1 for adding a BufferGeometryUtils.mergeVertices function for BufferGeometry, although I think @gkjohnson's PR #14116 covers some needs (morph targets, configurable tolerance) that this PR does not. Since that PR has stalled for now, merging just this function — after cherry-picking any code we want from the other PR — would work.

I forgot about morphTargets completely.

I didn't see that PR. I can leave my PR on hold and leave the team decide.

[donmccurdy]

It looks like the milestone keeps getting pushed back but it's unclear why.

I think the milestone changing with each upcoming release just means we're tentatively willing to include it with the upcoming release (once review and discussion has resolved), but it doesn't block the release. In other words, "awaiting further review." I'll comment there.

[takahirox]

nit: If there is an else statement, it should be on the same line as the closing bracket of the block, like } else {.

https://github.com/mrdoob/three.js/wiki/Mr.doob's-Code-Style%E2%84%A2#if-statement

[mrdoob]

@yomboprime Should I merge #14116 instead? Anything that PR is missing?

[yomboprime]

@yomboprime Should I merge #14116 instead? Anything that PR is missing?

Yes, this one only adds the points and lines support, which is generally irrelevant.

[mrdoob]

I see. Sorry for the duplicated efforts.

Feel free to add the points and lines support once #14116 is merged thought!