TSL: Fix instance buffer size in instance().

[ahaensler]

Bug Description

Setting mesh.count is currently broken in the webgl fallback. Instance matrices are not added properly.

Explanation
I started with one instance, built the material and wanted to add more. They won't show up.

InstancedMesh defines two counts.

• this.instanceMatrix.count: max instance count.
• this.count current instance count, may be lower than max.

The attached PR defines a buffer big enough for all instances.

[github-actions]

📦 Bundle size

Full ESM build, minified and gzipped.

	Before	After	Diff
WebGL	338.74 79.06	338.74 79.06	+0 B +0 B
WebGPU	568.78 157.23	568.78 157.25	+4 B +11 B
WebGPU Nodes	567.39 156.99	567.39 157	+4 B +11 B

🌳 Bundle size after tree-shaking

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

	Before	After	Diff
WebGL	470.52 113.83	470.52 113.83	+0 B +0 B
WebGPU	642.52 173.77	642.53 173.78	+4 B +16 B
WebGPU Nodes	597.17 163.07	597.17 163.08	+4 B +9 B

[Mugen87]

Setting mesh.count is currently broken in the webgl fallback. Instance matrices are not added properly.

The same rules apply like with InstancedMesh.count.

The expected behavior is that mesh.count defines the maximum instance count. So it is not supported to define let's say 10 instances and then change the number to 20.

Your are not expected to modify the count property to instanceMatrix.

[ahaensler]

Your are not expected to modify the count property to instanceMatrix.

I am not modifying instanceMatrix, but mesh.count. The linked documentation says:

You can change the number of instances at runtime to an integer value in the range [0, count].

That sounds reasonable, don't you think?

Let me give a scenario that should work according to documentation.

 const mesh = new InstancedMesh(geometry, material, count = 20)
 ...

 // I want to draw 10 instances initially.
 mesh.count = 10
 mesh.material = someMaterial
 // A shader build happens, but the generated shader only supports 10 instances.
 ...

 // Now I want to draw 20 instances with the same material.
 mesh.count = 20

The last assignment has no effect, because the shader thinks there are only 10 instance matrices, when in fact the buffer always has size 20.

[ahaensler]

That's the faulty shader code that my PR fixes. It should have length of 20.

uniform NodeBuffer_1064 {
    mat4 buffer1064[10];
};

[Mugen87]

Do you mind demonstrating with a fiddle how the current code breaks? Use https://jsfiddle.net/mnqr9oj0/ as a starter template.

[Mugen87]

I think the better fix is to overwrite setup() in InstancedMeshNode like so:

setup( builder ) {

    this.count = this.instancedMesh.count;

    super.setup( builder );

}

In this way, InstanceNode.count is kept in sync.

[ahaensler]

https://jsfiddle.net/fjqLr8nd/2/

A fiddle featuring the bug and comments.

I think the better fix is to overwrite setup() in InstancedMeshNode like so

Sounds good, I'll make the changes.

[Mugen87]

My suggested change does not fix the issue.

The problem is that you set the count property back to 10 before the first render. That is an invalid use case, afaict. When the instanced mesh is rendered for the first time, the count and buffer sizes have to match.

In other words: The size of the buffers is determined by InstancedNode.count, not the values of the instanced attributes.

[ahaensler]

The problem is that you set the count property back to 10 before the first render. That is an invalid use case

Well, this can also be provoked after the first render.
I have updated the fiddle to demonstrate the problem.

https://jsfiddle.net/xfmynado/1/

My suggested change does not fix the issue.

I have pushed a solution that works: Instead of

this.count = this.instancedMesh.count;

we can use

this.count = this.instancedMesh.instanceMatrix.count;

[Mugen87]

After reconsidering the issue, your first commit was actually the best solution. Do you mind reverting to it?

You are right, the buffer size should be evaluated from the buffer attribute count since otherwise you can't properly "reserve" size that you want to use at a later point. Rendering once with the larger size shouldn't be required.

Sorry for the back and forth.

[ahaensler]

Thank you.

Rendering once with the larger size shouldn't be required.

I had to change count frequently and rebuilding materials every time is really slow.

[sunag]

I think we should consider this issue in the larger scope, this still does not fix similar problems that we may have related to Sprite or SkinnedMesh instances when used mesh.count for example.

We can check in RenderObject if the number of instances is greater than the previous one, it considers rebuilding the shader, otherwise it just renders fewer instances or create an extra property for dynamically change the number of instances that would be drawn.

[Mugen87]

Um, I'm unsure with adding dynamic resizing since that could cause developers to don't care about resizing anymore which has negative impacts on performance. At least, I think it's a good standard if developers think about maximum sizes for their app and allocate the respective size in advance to avoid reallocations whenever possible.

The PR is any event an improvement and should be merged no matter how we move on.

[sunag]

I agree, misuse could happen if resizing were automatic.

I think that as TSL improves, InstancedMesh should fall into disuse in favor of a more global solution. In that sense, we could use something like geometry.instanceCount to define the maximum size. Since this property is now redundant.

The PR raises a broader issue, but we can certainly address it in later PRs.

[Mugen87]

InstancedMesh should fall into disuse in favor of a more global solution.

Definitely. InstancedMesh was the first attempt to support instancing in WebGLRenderer and in that scope it did a good job. However, it is not a scalable solution since you can't easily use instancing with other renderable 3D objects. The node material and WebGPURenderer noticeably improve this situation.

For the interested reader: In WebGPURenderer you don't need a separate Instanced* class for using instancing. You can just assign a value to the count property of a renderable object and the renderer will configure instanced rendering. Multiple demos are using this approach, below is a code example with skinned meshes.

https://github.com/mrdoob/three.js/blob/54ac263593c81b669ca9a089491ddd9e240427d2/examples/webgpu_skinning_instancing.html#L102C13-L102C20

[ahaensler]

That example looks simple enough. InstancedMesh provides little added value.

Here's some ideas on how InstancedMesh could be replaced:

1. Introduce an InstancedMatrixAttribute that provides convenience methods like .setMatrixAt() etc. by subclassing InstancedBufferAttribute. That's low-effort and gets the job done.

2. A more intuitive solution would treat instances like Object3D.children and allow setting position/scale/rotation for each instance conveniently from the CPU side. This sounds difficult and may need a Proxy for monitoring an array, for example mesh.instances. The array might hold InstancedObject3D.