-
-
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
WebGPURenderer: BatchMesh support for Instanced rendering with sorting, frustum culling #28753
WebGPURenderer: BatchMesh support for Instanced rendering with sorting, frustum culling #28753
Conversation
@RenaudRohlinger I push a commit releated to sort in webgpu, could confirm if your issue is related with this? Seems that some objects are being ignored in webgpu but I haven't checked the reason yet. |
Thanks @sunag! Unfortunately we cannot use the instanceIndex that's the whole issue of the current implement of the BatchMesh fallback. It needs to be related to the current draw index instead. |
Otherwise for the WebGPURenderer instead of running this every frame: const uniforms = properties.get( material ).currentProgram.getUniforms();
for ( let i = 0; i < drawCount; i ++ ) {
uniforms.setValue( _gl, '_gl_DrawID', i );
renderer.render( starts[ i ] / bytesPerElement, counts[ i ] );
} Could there be any alternative like storing the drawId into an attribute or a DataTexture for example? @gkjohnson I think it might be necessary as I also worried about the renderBundle case that will not be able to run this code (except maybe with indirectDrawCall in WebGPU). I worry the previous implementation of BatchMesh was a more appropriate version for WebGPU. Edit: Found out that Sunag solved the issue as it seems like with WebGPU we can specify the firstInstanceID in the draw command (last argument of drawIndexed) which makes the fallback super clean. |
Wow with WebGPU you can manually specify the first Instance ID in drawIndexed? This is actually super cool!! Amazing! |
@sunag Tell me when you're done with your review, I will clean up the code and merge the PR if it's ok with you. 😊 PS: By the way I don't think it would make sense to take the time to support the WebGL fallback only for firefox. By the time the WebGPURenderer become more popular firefox will probably already support WebGPU. |
I agree, I thought the same thing. Maybe we could have a Great work, I think that was all. |
…g, frustum culling (#28753) * WebGPURenderer: Full BatchMesh Support in both backend * cleanup * fix circular deps * webgpu wip * cleanup webgup * webgpu * cleanup * cleanup * cleanup * add the ability to specify nodeType in TextureNode and CubeTextureNode * drawIndex * cleanup * fix uint override to indirectTexture in BatchNode * update batch mesh example * fix glslnodebuilder * cleanup and fix glsl * more cleanup * cleanup and feedbacks * minor cleanup --------- Co-authored-by: sunag <[email protected]>
Related issue: #28462
Description
This PR aligns the BatchNode with the new BatchMesh API.
I had to implement several features to achieve this:
DataTexture
in theGLSLNodeBuilder
.GLSLNodeBuilder
(currently only for vertex).TextureNode
.TextureNode
to override thenodeType
.gl_drawID
in WebGL).This contribution is funded by Utsubo