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

Allow using VBOs for meshes all the way down to 4 vertices #14366

Merged
merged 1 commit into from Feb 12, 2024

Conversation

paradust7
Copy link
Contributor

@wsor4035 wsor4035 added Maintenance Tasks to keep the codebase and related parts in order, including architectural improvements Code quality labels Feb 11, 2024
@sfan5 sfan5 added Performance @ Client rendering and removed Maintenance Tasks to keep the codebase and related parts in order, including architectural improvements Code quality labels Feb 12, 2024
@appgurueu
Copy link
Contributor

I found the following claim:

No. In fact, VBOs used naively can be slower than client arrays (i.e. just re-streaming the geometry from CPU memory every frame).

Hence wasn't sure whether setting this as low as 4 would be a good idea. Won't this mean each particle gets its own VBO? That seemed excessive.

To test this hypothesis, I compiled a release build, and tested using the code from #14155. The results were roughly the same on this PR and master, so this seems to introduce no blatant performance issue there (on my hardware at least). If there was a performance issue, it'd probably be dwarfed by the many draw calls anyways.

Copy link
Member

@sfan5 sfan5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems to work fine

@lhofhansl
Copy link
Contributor

With my NVidia card I find no difference. Scene with viewing_range = 1000, client_mesh_chunk = 1. With or without this PR I get 6 FPS.

As comparison with client_mesh_chunk = 8 I get 60 FPS (now limited by my display)

But it didn't make things slower, so good, I guess.

@appgurueu appgurueu changed the title Allow VBO to be used for meshes all the way down to 4 vertices Allow using VBOs for meshes all the way down to 4 vertices Feb 12, 2024
@appgurueu appgurueu merged commit e2ccd14 into minetest:master Feb 12, 2024
13 checks passed
@paradust7
Copy link
Contributor Author

@lhofhansl

Raising client_mesh_chunk resolves the memory bandwidth issue because it makes the VBO's larger, so that they pass irrlicht's default test of 500. It may also be resolving other hidden performance issues as well. Are there any downsides to a higher client_mesh_chunk?

I generated another perf report after changing just the VBO limit, and it shows that rendering is now spending most of its time in state changes (25% just in setRenderStates3DMode, of which 20% is in OnSetConstants). I'm going to be looking into this as well. I'll also test client_mesh_chunk=8 to see what changes.

I am doing this performance tuning to improve the framerate specifically for VR, not necessarily high viewing range. For the VR experience to be really smooth, it needs to render twice (one for each eye) at 144 fps (so roughly the equivalent of 288 fps). (There are rendering techniques that combine the rendering of both eyes into one, but I'm trying to see how far I can get with separate passes)

@lhofhansl
Copy link
Contributor

@paradust7 I think a larger client_mesh_chunk mainly reduces the number of drawcalls.

As you do perf reports on MT, could you do for large and small client_mesh_chunk sizes?
Increasing this is the single most significant performance settings.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants