InstancedBufferGeometry.maxInstancedCount => .instanceCount

[WestLangley]

Fixes #18990.

I'll update the migration docs if this is merged.

[Mugen87]

Don't you think it's better to name the property just count so it's aligned to InstancedMesh?

[WestLangley]

@Mugen87 OK, if you want.

But now that I think about it, I am wondering if this PR is even a good idea...

Having a count property would lead users to think they have to set it -- and they don't. It works similar to drawRange.count, which also defaults to Infinity. The renderer will not try to render more instances than the buffer will allow. The users do not have to set count at all.

Maybe leave the API the same?

[Mugen87]

I think the thing is that you want to align the API towards drawRange whereas I want to align it to InstancedMesh :)

When creating an instance of InstancedMesh, count is a mandatory parameter. If not assigned, the respective property will be undefined and the creation of instanceMatrix will fail. I think it would be great if the instancing API (InstancedMesh and InstancedBufferGeometry ) have similar semantics. However, I also understand the benefits to make it consistent to how drawRange works. Especially since maxInstancedCount is no mandatory parameter. Um, I'm not sure about all this...

@mrdoob What is your preference here? 😊

[WestLangley]

With InstancedMesh, count is required because the constructor allocates the instanceMatrix buffer.

With InstancedBufferGeometry, the user allocates the buffers, because there can be any number of them. maxInstancedCount, which is optional, serves the same purpose as drawRange in that case.

[Mugen87]

Yes, that's important to highlight.

[WestLangley]

I think this PR is good-to-go.

[WestLangley]

@mrdoob tl;dr

Currently, .maxInstancedCount is used to specify the number of instances to render, and there is no check preventing the renderer from trying to render more instances than the buffer will allow.

This PR renames the property to .instanceCount, and also adds a check preventing the mentioned rendering error -- just as we do with .drawRange.

[mrdoob]

Thanks!