Skip to content
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

Fix to splats frustum culling #5804

Merged
merged 1 commit into from Nov 8, 2023
Merged

Fix to splats frustum culling #5804

merged 1 commit into from Nov 8, 2023

Conversation

mvaligursky
Copy link
Contributor

@mvaligursky mvaligursky commented Nov 8, 2023

  • Instancing used to turn off frustum culling of the mesh instance. The API now has additional parameter to control this. If the case a user sets up a customAabb on the render / model component, they can enable culling for the mesh instancing. Note that there is no culling per instance, but a single custom aabb is used to cull all instances.
  • this is used by gaussian splat rendering to skip rendering when splat is off-screen

Changed API:

// note the new cull parameter, which defaults to false which was the original behaviour
MeshInstance.setInstancing(vertexBuffer, cull = false)

@mvaligursky mvaligursky self-assigned this Nov 8, 2023
@mvaligursky mvaligursky added bug area: graphics Graphics related issue labels Nov 8, 2023
@LeXXik
Copy link
Contributor

LeXXik commented Nov 8, 2023

Ah, that was a thing. I had a screen space shader for a polyline that used instanced geometry. And when the culling was on while geometry went outside the frustum, the skybox went black among other issues. I disabled culling as a temp solution, but should revisit it after this is released. Curious if adding a frustum culling per instance would be hard?

@mvaligursky
Copy link
Contributor Author

You can do this in the meantime:

meshInstance.setInstancing(vertexBuffer);
meshInstance.cull = true;

Per instance culling is doable, but:

  • I'd need to assume instancing VB contains matrices. Currently (officially) it does, but I want to extend it to other things. And people already using it to store other things in those matrices. Hardware instancing improvements #4878
  • I'd need to have an internal VB to copy data to
  • I'd need to cull per camera, so added cost, even though the user can often do a single cull with larger bounds for example

So I suspect we might not add per instancing culling, and leave users to do this if needed, and update their index buffer dynamically with the results.

@mvaligursky mvaligursky merged commit 960b2e5 into main Nov 8, 2023
7 checks passed
@mvaligursky mvaligursky deleted the mv-splat-culling branch November 8, 2023 14:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: graphics Graphics related issue bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants