mrdoob / three.js

JavaScript 3D Library.
https://threejs.org/
MIT License
101.92k stars 35.32k forks source link

BatchedMesh: `bufferSubData` causes lag #27980

Open marwie opened 6 months ago

marwie commented 6 months ago

Description

Hi, I'm currently stress testing with 20 instances and ~300.000 vertices each. We have LODs generated and I reserve the vertex space of LOD 0 for each instance so the float32 buffer ends up being ~ 9.000.000

When LODs switch and setGeometry is called i noticed quite a few big lags which seem to be coming from the call to bufferSubData with the whole buffer.

My question is if this is currently expected or if I might be doing something wrong here.

image

image

Solution

I would hope this could be optimized by providing the ranges array and the batched mesh could keep track of which ranges have changed. Could this be a possible improvement?

Alternatives

Clamp the max size of a BatchedMesh (on my side)

Additional context

https://mastodon.gamedev.place/@marwi/112145459503913620

marwie commented 6 months ago

I added calls to addUpdateRange in 516 and 540

dstAttribute.addUpdateRange( vertexStart * itemSize, vertexCount * itemSize ); and dstIndex.addUpdateRange( indexStart, reservedRange.indexCount );

Before: https://github.com/mrdoob/three.js/assets/5083203/219448b2-2e10-4ad9-87d9-8d708055f8b4

After: https://github.com/mrdoob/three.js/assets/5083203/808310c9-f93e-488b-a87d-412d7cc09df6

donmccurdy commented 6 months ago

I'm a little off topic perhaps, but note that you could switch between LODs generated by meshoptimizer without changing the vertex buffer at all. Update the index, set draw range, nothing else needs to change:

https://jsfiddle.net/donmccurdy/84acgydp/

marwie commented 6 months ago

That's very interesting @donmccurdy thanks for sharing

gkjohnson commented 6 months ago

I'm a little off topic perhaps, but note that you could switch between LODs generated by meshoptimizer without changing the vertex buffer at all. Update the index, set draw range, nothing else needs to change:

Yeah there's currently no way to update only the geometry index (or any specific attribute) when setting data in the BatchedMesh geometry - but perhaps something like an "options" object can be added to setGeometryAt to limit which attributes should be updated if meaningful performance benefits can be shown:

// both lods share all attributes except for index buffer
lod0 = ...;
lod1 = ...;

// initialize the geometry
const id = batchedMesh.addGeometry( lod0 );

// swap LoD by only updating the index buffer
batchdMesh.setGeometryAt( id, lod1, { attributes: [ 'index' ] } );
gkjohnson commented 2 months ago

This will have been fixed in #27981 with the use of updateRanges. You still can't swap the geometry used by an instance as @donmccurdy suggested - something like the following would be fairly straightforward, though:

batchedMesh.setInstancedGeometry( instanceId, geometryId );

The implementation would be fairly simple:

setInstancedGeometry( instanceId, geometryId ) {

  // check to make sure ids are in valid ranges

  this._drawInfo[ instanceId ].geometryIndex = geometryId;

}