Anything that uses thinInstanceSetBuffer first and then a subsequent targetted update to said buffers either with thinInstanceSetMatrixAt or thinInstanceSetAttributeAt will break. The quick fix is to use false option when setting up buffers.
For matrices, the proper fix is to rebuild the underlying Float32Array and then call thinInstanceSetBuffer again. You an do so via example below:
// get existing
let allMatrices = sphere.thinInstanceGetWorldMatrices();
matricesData = new Float32Array(16 * allMatrices.length);
// rebuild
allMatrices.forEach((x,i)=>x.copyToArray(matricesData, i * 16))
// change desired instance
matrix = BABYLON.Matrix.Translation(1, 0, 3);
matrix.copyToArray(matricesData, 1 * 16);
// set buffer again
sphere.thinInstanceSetBuffer("matrix", matricesData, 16);
I have no straightforward fix for attributes, best I can think of is to store each thin instance attribute params and rebuild the Float32Array everytime you have to change properties for a single instance. Which is a big-deal if you have hundred/thousands of thin instances and you just need to update 1.
If you are using thinInstanceSetBuffer in your code, you should pass false for the staticBuffer flag, and that should be the only change you need to apply. Could you provide a PG where this change is not enough to fix the problem?
Yes, the false flag work but based on my understanding, there is a perf hit ? which was the reason why the buffers were set to static in 6.38.0? I’m trying to preserve perf while reworking my code to be memory friendly. thinInstanceGetWorldMatrices is a clean fix and still keeps static buffers, but there is no equivalent for attributes? Smthg like thinInstanceGetAttribute? To keep static buffers and still allow for user to update selective thin instances, thats what I was going for, I guess…
It is not a clean fix, because calling thinInstanceSetBuffer will recreate the vertex buffer (that’s why it works). If you modify a matrix from time to time, then it’s ok and probably the best solution because it will recreate the vertex buffer in a static mode, but if you call thinInstanceSetBuffer several times per second / each frame, then it may be slower than setting the buffer to “non static” and calling thinInstanceBufferUpdated (to be tested against real scenarios to be sure).
You can manage the attribute buffers yourself, see:
I don’t think thats the correct use case for thin instances. But yes, agreed, setting the buffer non-static for such scenarios would prolly be best.
Yes, I’m aware. I’m going to use a pseudo code example as below to illustrate my dilemma.
var blah = new Float32Array(3000 * 4);
//populate blah with some loop and data, flush data, reclaim memory
mesh.thinInstanceSetBuffer("color", blah, 4);
//oops, time to change the 2565th thin instance color
//pre 6.38.0, nice simple elegant one-liner
mesh.thinInstanceSetAttributeAt("color", 2565, [1,0,0,0]);
// post 6.38.0, buffers are static
create new blah
//populate blah again >> oops, previous data has been flushed and not kept in code.
//how to update my 2565th thin instance? Aha! data exists in mesh, grab data from mesh...
//hmm, no method exists to get attribute data from mesh! :sob:
afaik, there is no way to fetch buffer data from mesh for attributes? But a method exists to pull matrix data for all thin instances via thinInstanceGetWorldMatrices. Correct me if I’m wrong.
Before the change to the default value of staticBuffer flag, your code was using staticBuffer=false (because it was the default), that’s why thinInstanceSetAttributeAt works. To do the same thing now, you simply have to do mesh.thinInstanceSetBuffer("color", blah, 4, false);. It’s strictly the same thing (it’s the slow path in both cases) and mesh.thinInstanceSetAttributeAt will work again.
Now, if you want the buffer to be static but still be able to update an instance from time to time, you will have to update the blah buffer and call mesh.thinInstanceSetBuffer("color", blah, 4);. This will recreate the vertex buffer, which could be slow, but if it’s done infrequently it’s ok.
Yes, exactly! That’s what I want to do now but I no longer have a reference to the blah buffer (In the pseudo code example above, I should have been more explicit that the call to change a specific instance’s color happens in another func, say, upon user interaction). I need to rebuild the blah buffer from the data that is alr in the mesh! Something like:
let allColor = mesh.thinInstanceGetAttribute("color");
// rebuild blah, change desired instance
<insert codes here>
// and then set buffer again
mesh.thinInstanceSetBuffer("color", blah, 4);
But we don’t have such method yet, right ? Pls correct me if I’m wrong.
Yes, its done infrequently. It can’t be slower than a known fps drop on a client’s device.
The entire reason for this thread stems from the fact that a known code setting MAY introduce a significant perf hit on a device which was independently verified by another user in the community (which its awesome, btw!). Yes, setting the buffers with the false option is a workaround but is that what you want your user to experience knowing that it MAY lead to a perf drop on their device?
Apologies, I for one simply cannot endorse that. Prior to 6.38.0, I was ignorant as I too did not encounter said issue in my testing. Now I’m looking for a better solution, pls help me help my users.
I think the solutions discussed simply trade perf for mem, which I guess was inevitable. I’m marking this thread as solved. Thanks for all the help.
fwiw, this is my scope of impact wrt to the breaking change:
Seedborn uses 50+ thin instance buffers across 30 classes and ~35 master meshes, as of writing. Bulk of it are used for matrices, VATs, texture control, UV control etc. During architecture conception back when thin instances was first introduced (pre-webgpu era), I reasoned that it had potential for scalable flexible visuals that was not user interaction intensive, aka, simcity/civ type of games. In conjunction with custom materials (now material plugins), the final result met the bar after a lot of testing. A clean Seedborn start consumes ~1GB mem. With thin instance heavy scenes, the mem hit was in the acceptable range of handheld devices. With the change to static buffers, I am no longer sure this is the correct approach, float32arrays are not the most efficient data structures/mem store. I’d have to trim out CSM or scale down or rework…
Short of figuring out why dynamic buffers are consuming so much more resources, I’d find it hard to recommend thin instances as the approach towards massively scalable visuals for AAA apps as of now. If someone has better ideas, feel free to start another thread for discussion. Hope it helps.
It will automatically recreate a buffer if it is static and you tried to update it (the buffer will be recreated as static).
However, it could lead to sub-optimal performance if you update the buffers too often, so it’s disabled by default and you have to set this flag to enable the feature:
This would be great. Not having to keep a reference to the float array certainly helps.
erm, should this be a global flag? How about making it mesh specific, akin to how thinInstanceEnablePicking alr works? keeps all thin instance settings specific to targetted mesh in one place instead of across the board.
Testing the PR with your PG works. Tried to abuse and it also works (https://playground.babylonjs.com/#8NVQRY#12), which kinda doesn’t make sense cos the dynamic buffer seems irrelevant now with this hack, meh…
