-
-
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
InstancedMesh: Proposal to support sorting, frustum culling #27170
Comments
We can technically do this with InstancedMesh, by sorting the instance attributes. Not sure if I like that solution — either we break user's indices, or require some indirection between the user-facing index and the index on GPU. But for the sake of discussion... how would that differ from using WEBGL_multi_draw?
Ouch, that's a pretty high cost... 😕
A similar alternative ... could we allow BatchedMesh to reuse a geometry without duplication? One thing I like about the THREE.BatchedMesh name and API is that we aren't committed to a specific method of implementation. |
This would require sorting all per-instanced attribute data including color, matrices, and (in the long term) others which would be more cost prohibitive. A fully featured instanced mesh implementation would afford any material attribute to be defined per-instance - all of which would have to be rearranged. And in order to sort we'd need to double-store all sortable data in addition to breaking the GPU index. For simpler cases this might work but it's not ideal. The multi draw range approach is much cleaner.
BatchedMesh will only work with the extension in the next release, as well, meaning FF will not be supported. Firefox is the only platform that doesn't support the extension so I'm hoping there's some pressure that can be applied to get it added.
Some of the implementation could be reused in a common class but overall I think trying to make instancing and batching work in a single API will be more convoluted than it's worth. |
I guess another alternative is to support both code paths - the current implementation that doesn't support sorting or frustum culling and then a multi draw one that does. Until FF supports the extension, that is. |
If it can help this is how I handle multidraw fallback for firefox. Then in the BatchedMesh class a simple bufferattribute function renderMultiDraw( starts, counts, drawCount ) {
const extension = extensions.get( 'WEBGL_multi_draw' );
if ( extension === null ) {
for ( let i = 0; i < starts.length; i ++ ) {
this.render( starts[ i ] / bytesPerElement, counts[ i ] );
info.update( counts[ i ], mode, 1 );
}
}
extension.multiDrawElementsWEBGL( mode, counts, 0, type, starts, 0, drawCount );
const sum = counts.reduce( ( partialSum, a ) => partialSum + a, 0 );
info.update( sum, mode, 1 );
}
function renderInstancesMultiDraw( starts, counts, instanceCounts, primCount ) {
const extension = extensions.get( 'WEBGL_multi_draw' );
if ( extension === null ) {
for ( let i = 0; i < starts.length; i ++ ) {
this.renderInstances( starts[ i ] / bytesPerElement, counts[ i ], instanceCounts[ i ] );
info.update( counts[ i ], mode, instanceCounts[ i ] );
}
} else {
extension.multiDrawElementsInstancedWEBGL( mode, counts, 0, type, starts, 0, instanceCounts, 0, primCount );
const sum = counts.reduce( ( b, a ) => b + a, 0 );
const sumInstances = instanceCounts.reduce( ( b, a ) => b + a, 0 );
info.update( sum, mode, sumInstances );
}
} |
Looks like this removes a lot of the benefit of current instancing (makes a new drawcall for each mesh) but it would still do away with a lot of the overhead from updating shader uniforms compared to the naive approach of a bunch of meshes. This seems like a good middle-ground to me while keeping the code simple. If @mrdoob, @Mugen87, and / or @donmccurdy agree maybe this can be added to InstancedMesh
BatchedMesh is already using a batch id attribute - I expect that with sorting and filtering of individual meshes the gl_DrawID value wouldn't be correct, anyway (I don't know how it could be). I may be missing something but for some reason I can't get the extensions working in the shader to check, though 🤔 It's unclear what the behavior of InstanceID will be with multi draw, as well. |
@gkjohnson I'm happy with whatever you prefer here -- thank you! |
If the goal is to sort instances by transparency... |
The goal was to enable quickly and transparently sorting instances. If we just add it to the existing implementation then a bunch of data has to be re arranged in the vertex buffers and the user indices would no longer line up, either. It looks like I misunderstood the capabilities of multidraw's instanced draw function, though. It looks like it's not possible to specify subranges of the InstancedBufferAttributes to draw meaning my plan for sorting similar to BatchedMesh won't work as easily. Instead In order to get the kind of sorting and filtering I'm suggesting I think we'd have to change the structure of InstancedMesh to use a DataTexture to store transform matrices and a separate one for colors then we can use a sorted instanced buffer with indices into those data textures to adjust which objects we draw and in what order. This is more aligned with how BatchedMesh works, as well. The downside is that we have to use texture slots for different material properties like colors which becomes prohibitive at some point. TextureArrays could help with this but still. The Node material system may help make this less complicated. I'm not exactly sure what we should do here.. |
For another data point - it looks as thought Babylon is performing sorting on the CPU by rearranging the matrices in a buffer. The matrix data is double-stored, though. Frustum culling could be implemented in a similar way. |
That demo runs super slow here (MacBook M2 Pro)... 300ms per frame 🤔 |
Here are some recordings:
|
Okay stunning... I just updated Chrome to the latest version and now runs extremely slowly on my M1 Macbook, as well... So this is just a browser regression... |
I have an M2 14" to test on, but what jumps out at me is the use of |
It turns out this was related to using the https://jsfiddle.net/g9q4cswr/3/ flat varying float vBatchId; // bad
//
varying float vBatchIed; // good I've also updated the demo to remove the "flat" modifier and now performance is better again, so you can see the proof of concept for rendering an arbitrary number of material properties in a single draw call: https://gkjohnson.github.io/batched-material-properties-demo/
At least it didn't turn out to be something as critical as a Float texture that caused the problem 😬 |
Solved the issue here too yep! |
Is using a flat varying integer instead of a flat varying float fixes the issue though? I never experienced that issue with flat int AFAIK. |
The issue still happens with |
Saw this recently in private which I wonder if related due to Chrome update using Metal by default:
|
Hello, I have written a class that extends https://discourse.threejs.org/t/instancedmesh2-easy-handling-and-frustum-culling/58622 I would like to help if possible. |
Description
With the current InstancedMesh implementation it's not possible to easily or quickly sort individual instances to improve opaque overdraw for performance or transparency sorting.
Here are a couple posts referring to the transparency sorting issues with InstancedMesh: ref1, ref2
Solution
Similar to how the
WebGL_multi_draw
extension is used for BatchedMesh, there are "instanced" variants of the multidraw functions that allow for sorting the order in which instanced draws are performed while retaining a single draw call.If desired we could switch InstancedMesh over to use the multidraw extension, as well, and provide support for sorting objects within a single InstancedMesh. See #27168 (adds sorting objects for BatchedMesh) for a reference on how this might be implemented for InstancedMesh.
The drawbacks are still that
WebGL_multi_draw
is not supported in Firefox so fully switching over to multidraw would remove InstancedMesh support for FF.Alternatives
Additional context
cc @WestLangley @donmccurdy
The text was updated successfully, but these errors were encountered: