-
-
Notifications
You must be signed in to change notification settings - Fork 35.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
BatchedMesh: Add support for Instanced rendering with sorting, frustum culling #28462
Conversation
📦 Bundle sizeFull ESM build, minified and gzipped.
🌳 Bundle size after tree-shakingMinimal build including a renderer, camera, empty scene, and dependencies.
|
This should be ready now based on the changes discussed here: #28456 (comment). I've updated this demo to use the new API to test the recent changes: external demo However this is now a breaking change since users must now explicitly add an instance after adding a geometry to the BatchedMesh: const geometryId = batchedMesh.addGeometry( geometry );
const instanceId = batchedMesh.addInstanced( geometryId );
batchedMesh.setMatrixAt( instanceId, ... ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please consider my comments suggestions only, and sorry for the extra runaround. This looks excellent! :)
boundingSphere.union( _sphere ); | ||
|
||
} | ||
|
||
} | ||
|
||
addInstance( geometryId ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Optional: Perhaps we take a Matrix4 as the second argument, and save users a step when they're creating static geometry?
// update the geometry | ||
this.setGeometryAt( geometryId, geometry ); | ||
|
||
return geometryId; | ||
|
||
} | ||
|
||
setGeometryAt( id, geometry ) { | ||
setGeometryAt( geometryId, geometry ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As a future addition, does this distinction make sense?
setGeometryAt( geometryId, geometry )
overwrites a geometry (up to the allocated vertex count), and affects all instances of that geometrysetInstanceAt( instanceId, geometryId )
changes the geometry instantiated by a specific instance ID
I think these are good suggestions - but I'd say lets consider them in the future. Unless there's something fundamental that doesn't feel right with the current BatchedMesh model (like it will require a breaking change in the future) I'd prefer to avoid changing the API further in this PR. |
Agreed, both could be non-breaking changes in a future PR. |
Merging for the next release! |
Redo for #28404. Waiting on docs discussions from #28456.
Related issue: #25059, #28103
Description
This PR makes some changes and additions to the BatchedMesh class to support rendering multiple copies of the same geometry without redundant memory usage. This just uses the regular
multiDraw
functions so we retain support for object sorting and frustum culling. Sorting is done by sorting the draw ranges to upload as well as a uint id buffer that indexes into the matrix texture. AbatchId
geometry attribute is no longer needed.You can see a live demo from my prototype here. With three unique geometries and rendering 20,000 items the geometry memory usage goes down from ~192MB to ~23KB.
This is a proposed API and to demonstrate how this can be achieved. Once an API is settled on I can update documentation & any loader changes needed.
Usage
API Description
addGeometry( geometry )
Inserts new geometry and associated draw range data into the mesh in addition to a new draw instance so it will render. The id of the new draw instance is returned.
addInstance( id )
Takes the id of an item that has already been inserted into the mesh either via
addGeometry
oraddInstance
and adds a new draw instance using the same geometry.setGeometryAt( id, geometry )
Sets the geometry associated with the given instance id. All of the instances that are rendering with the same referenced geometry will be changed, as well.
deleteInstance( id )
Removes the instance from being drawn. Just like the previous
deleteGeometry
, though, this doesn't free up space in the buffer geometry or remove instance objects due to how ids work, yet, so this should remain undocumented. Theoptimize
function would need to be implemented.Things to Note
Because we use instance ids to add new instances that use the same geometry - once all instances that reference a specific geometry have been deleted, it will not be possible to add new instances of the geometry even though the data is still in the batched mesh. The future
optimize
function would remove the geometry if it's no longer referenced by any draw instances.Another API option might be to enabling managing of geometry vs instances separately but this makes usage a bit more verbose and less clear if you don't care to use instances. I waiver between which I prefer: