Batchmesh: Fix cases where calling optimize can result in inconsistent state

[gkjohnson]

Related issue: #29527

Description

I found some issues while working on a batched mesh implementation for 3d tiles (NASA-AMMOS/3DTilesRendererJS#800) that takes advantage of the new functions from #29527, #29577 and saw that calling optimize could result in geometry displaying incorrectly.

• Store the next insertion points for geometry as dedicated variables rather than using reservedRanges since there is now no guarantee of the ranges being presorted.
• Handle the case in expanding the geometry size in which the index buffer array can change array type (ie Uint16 before expanding to Uint32 afterward).
• Presort the order of the geometry ranges in the optimize function before shifting data since there's no guarantee of order.
• Update some comments and spelling.

Once I've gotten the 3d tiles BatchedMesh plugin completely working and merged I'll plan to submit a PR with some cleanup for BatchedMesh, as well.

[github-actions]

📦 Bundle size

Full ESM build, minified and gzipped.

	Before	After	Diff
WebGL	690.29 171.01	690.67 171.1	+379 B +92 B
WebGPU	816.05 219.94	816.43 220.08	+379 B +132 B
WebGPU Nodes	815.56 219.81	815.94 219.95	+379 B +132 B

🌳 Bundle size after tree-shaking

Minimal build including a renderer, camera, empty scene, and dependencies.

	Before	After	Diff
WebGL	463.31 111.89	463.31 111.89	+0 B +0 B
WebGPU	538.61 145.41	538.61 145.41	+0 B +0 B
WebGPU Nodes	494.73 135.28	494.73 135.28	+0 B +0 B

[mrdoob]

What do you think about moving BatchedMesh to addons/objects/? 🤔

[gkjohnson]

What do you think about moving BatchedMesh to addons/objects/? 🤔

Is there a reason? It was requested to be moved to core, previously 😅

I think it's nice to have something that exposes this capability built into the project but if there's something that needs to change in order to accommodate that I'm happy to discuss what that looks like.

[Makio64]

What do you think about moving BatchedMesh to addons/objects/? 🤔

I would say it belongs to core cause it might be the most powerful class since InstanceMesh to reduce drawcall and improve performance.

I used it extensively this last 2 months in game production and therefore I might have report many bugs about it.
This said, it was mostly edge cases custom vertexNode, edge devices/browser (ios beta safari webpu flag enabled) or api improvement request but issues are getting resolve fast everytime.

Overall me and my teams are more than happy with BatchedMesh for the performance and versatility it offers and I can't visualize new medium/large threejs project without it.

There is room for improvement and stabilization but its going in the right direction + as @RenaudRohlinger mention here its gonna gain even more performance on WebGPU soon.

[mrdoob]

I just feel it's getting kind of big and it's for pretty advanced users that wouldn't mind adding an extra import on their projects 🤔

[mrdoob]

@Makio64 You make it sound as if I'm suggesting removing it from the project completely 😅

I'm just suggesting moving that file to addons. The renderers would remain intact.

[Makio64]

@Makio64 You make it sound as if I'm suggesting removing it from the project completely 😅

I'm just suggesting moving that file to addons. The renderers would remain intact.

@mrdoob woops, sorry! I understand the point of the size, I think BatchedMesh can be simplified / factorized as I suggested here : #29591 (comment) . I'll PR it this weekend

[Mugen87]

I was also wondering if we could implement a simpler basic version of BatchedMesh in the core (like the one from the initial PRs) and then maybe a more optimized version with a larger API in addons that derives from BatchedMesh. Ideally, they share the same core functionality.

[gkjohnson]

I just feel it's getting kind of big and it's for pretty advanced users

I'm mostly trying to understand the line you're feeling for when something belongs in core and when something belongs in the examples folder and what the concerns are more exactly so I can suggest other options for mitigating them. Regarding size - this class will be treeshaken when building. I'd also like to avoid thrash with dependent projects. Moving the class (again) is a breaking change and means a major release for any dependent libraries.

From my perspective this is where I'd wanted BatchedMesh to be in terms of features when I started working on it around almost a year ago (!!). In any other engine this would be considered a core feature and, in fact, is as simple as a toggle. But I understand there are concerns about complexity.

In the past I've suggested adding BatchedGeometry, which would be responsible for managing sub geometries and geometry ids, while BatchedMesh would be responsible for managing instance ids which would allow for cleaning up and simplifying the main class a bit but I haven't heard other thoughts on this.

I was also wondering if we could implement a simpler basic version of BatchedMesh in the core ... then maybe a more optimized version with a larger API in addons that derives from BatchedMesh.

This is something I could support.

But I'd prefer not to block this PR consider it's bug fixes on this discussion. Can we make a separate issue to discuss, instead?