Instancing reset

[benaadams]

No description provided.

[benaadams]

Gah; that was poor. Yes should be =

[benaadams]

The bane of premature optimization...

[mrdoob]

I think I would separate it though...

state.enableAttribute( programAttribute );
state.setAttributeDivisor( programAttribute, data.meshPerAttribute, extension );

[benaadams]

Issue is enableAttribute wouldn't have the info; so would need to reset to 0 for every attribute as per #7225; or every enableAttribute would need to be always later followed by a setAttributeDivisor call including renderBufferImmediate, LensFlarePlugin and SpritePlugin

[benaadams]

Could maybe make a state.enableAttribute and state.enableAttributeWithDivisor (better name); that have differing param counts?

[mrdoob]

Could maybe make a state.enableAttribute and state.enableAttributeWithDivisor (better name); that have differing param counts?

That sounds good!

[benaadams]

Something more like that? (not tested)

[emnh]

Tested. Works.

[benaadams]

Is it an issue this is wrong in v72? Want a PR to master?

[mrdoob]

I think it's fine... r72 already had quite a bit of post-release fixes...

[benaadams]

OK, will leave master. Will likely cause a huge number of future merge conflicts if I tried - don't want you cursing me come r73 😜

[mrdoob]

I still consider all this stuff experimental and unstable API so it's kind of expected 😊

[benaadams]

As an aside: using instancing for 1x1 plane billboards will be likely slower than 4x duplication (position alone) as the gpu needs needs to process more data in the shader (position + offset); with larger models then instancing becomes more suitable as the CPU->GPU bandwidth becomes the bottleneck rather than the shader. I think the tradeoff starts happening around 100 vertices per model - though the api for updating is simpler for instancing - but that can be hidden by library.

[mrdoob]

Thanks guys!