BufferGeometry: toIndexed

Since i already posted the half snippet, this patches THREE.BufferGeometry with a function to create a indexed one from a non-indexed. A new BufferGeometry is returned.

A indexed geometry reduces the data of all attributes and will generally perform better, also computeVertexNormals can be called only on indexed geometries to compute smooth normals.

11 Likes

I’ve made an update to this, all attributes are now considered, if vertices don’t share all attributes they remain individual. Also skinning and morph attributes.

I use it in a converter process for my assets since some modelling tools or exporters will generate non-indexed geometries once they use disconnected UV nets, what is mostly the case for sliced up UVs.

The old simple approach is still available since it doesn’t add much code, if you call geometry.toIndexed( false ); it will only merge by position. Reason can be a raw model, or if you want to compute vertex normals regardless of disconnected UVs.

Let me know if you face any issues, since it should be fine with any scenario now i’d make a pull request if all cases are covered.

5 Likes

That is a nice feature. (I haven’t inspected the implementation (yet).)

The reason for that, I figure, is that it requries first indexing faces by which vertices they are connected to. This is much easier to do, and can be done unambiguously, with indexed geometries. To achieve the same result with unindexed geometries, one will in practice have to do about all that is needed to convert to an indexed geometry.

You are using spatial hashing with configurable precision, which should be OK. :+1:
The meaning of the precision parameter is basically the number of digits that must match. OK

Line 35-92 can be greatly simplified by employing a nested loop.

The else clauses in line 256-277 can be omitted.

Each vertex needs all normals of the faces it’s connected to to compute a weighted normal. So if there are UV seams it ignores it’s neighbours which would cause a visible seam later with lights. In some cases this is a desired effect like at steep angles.

It’s for performance reasons, since there are only 4 cases that’s fine.

A bounding sphere is required for frustum culling.

Did you test? I would expect that a modern JS engine would be happy to get a short and intuitive nested for loop expression to work with for optimization. But on the other hand, you tell it beforehand which few values itemSize can be, and that means it can possibly be optimized statically.

A bounding sphere is required for frustum culling.

Yes, but it will be generated on demand by the renderer. It makes sense to copy the source sphere to spare an extra computeBoundingSphere, but there should be no rush with making a new one if none was present. That only adds code lines. (If I am not mistaken.)

You shouldn’t hope it optimizes it somehow for you, anything with clear cases you can tell the compiler in advance, such as initialization values avoids a lot steps.

There shouldn’t be even the case none is present, optimally the bounding sphere and box should be delivered with the asset, it isn’t really for free computing those.

This isn’t really optimal being JIT for any more advanced use cases. As mentioned optimally it would already come with the asset, i even precompute one per hierarchy level for better culling on object level, any system where it’s needed like collissions or directly in game mechanics you would have to check and perform this first.

Thank you for your insight. Would it also help the engine to do this?

if (! ([1,2,3,4].includes(itemSize) ) {
    console.error("Invalid itemSize %d.", itemSize);
    return null;
}

Now the subsequent nested loop can in principle be unrolled on the valid itemSize cases statically.

OK. I agree that it is wise to deliver them with the asset. (I am not used to thinking in assets, as I like to generate geometry on the fly.) But what if your user is not done moving the vertices? Then the automatic computeBoundingSphere is just an annoying extra wait. To me it looks a bit like you are “forcing” your way onto the user, since the else clauses are for specifically those cases where no bounding sphere is present yet, which may have been a user choice, (e.g. because that happens later in their workflow.) But it’s no big deal, of course. Easy to change if any of the conservative voices among the three.js maintainers asks.

I hope your PR gets merged, as this would be a nice addition to three.js. :raised_hands:

@Fyrestar have you made a PR for this?

I hope your PR gets merged, as this would be a nice addition to three.js. :raised_hands:

@Fyrestar have you made a PR for this?

Not to take take away from @Fyrestar’s efforts but there’s already a mergeVertices function in BufferGeometryUtils that I added earlier this year which should handle all these cases, as well. I’m sure there’s something to be gained by sharing code between the implementations, though!

6 Likes

I got here by Google search. It might be worth noting in the docs, that this function kind of does the “toIndexed operation”. (It might also be worth mentioning in the docs-page of BufferGeometry, as a note/comment, either in general or in the description of “toUnindexed”)

As of now the docs itself make it hard to find that this functionality exists

4 Likes

yes great I really need that , thx alot

I was about to repackage @Fyrestar 's function in npm, to make it easier tu use, then I found this comment. This really deserve a comment on the BufferGeometry class, as I’m not the first that didn’t notice that mergeVertices would do it.

Yes. MergeVertices makes indexes.

Got here from a google search. I think mergeVertices should be mentioned in the doc

I forgot this. read the doc found nothing then google searched and found this post and i was surprised that not only did i know about this but have commented on this abt a year ago. please do mention sth about this on the doc

From the docs:

https://threejs.org/docs/?q=buffergeom#examples/en/utils/BufferGeometryUtils.mergeVertices

There are a number of terms for this: indexing, merging vertices, welding, etc. If there’s more information you’d like to see in the documentation, please consider adding it in a pull request. :slight_smile:

I suppose this should be the correct way to implement this now
By using BufferGeometryUtils.mergeVertices from

import * as BufferGeometryUtils from "three/examples/jsm/utils/BufferGeometryUtils.js";

as mentioned above:

https://threejs.org/docs/?q=buffergeom#examples/en/utils/BufferGeometryUtils.mergeVertices