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

Instanced block rendering #458

Open
6 of 13 tasks
kpreid opened this issue Jan 31, 2024 · 4 comments
Open
6 of 13 tasks

Instanced block rendering #458

kpreid opened this issue Jan 31, 2024 · 4 comments
Labels
area: graphics kind: incomplete A feature is partially implemented; the current state of the code is inconsistent kind: performance Ways to increase performance or efficiency without adding functionality

Comments

@kpreid
Copy link
Owner

kpreid commented Jan 31, 2024

As of b4d4270, there is just-barely-working support for rendering Space contents using instanced block meshes instead of chunk meshes. This already provides great performance improvement for rendering — at least on desktop — but there's a lot of pieces left to do.

  • Performance:
    • Investigate possible performance regression on web. (Not sure if it's real. If it is, it might be just the total number of WebGL state changes and draw calls increasing given the demo city's large number of distinct blocks — not sure what to do about that.)
    • Optimize instance list data structure management to reduce the repeated work each frame.
    • Tune the threshold at which instances are used.
  • Applicability:
    • Use AnimationHint to decide whether to use an instance. (Done in 5dcfa46.)
    • When a change in the world only affects instances, only update instances. (Done in 7a16b29, 434fc90)
    • Use instances in glTF export.
    • When a change in the world occurs, consider just adding/replacing one instance instead of remeshing the containing chunk, even if we wouldn't use instances from scratch. (Blocked on teaching glTF about instances.)
      • If a chunk experiences lots of changes and then quiesces, consider turning its instances into a chunk mesh.
    • Decide what to do about rendering transparent blocks — perhaps they should just continue to be chunked and rendered using chunk depth-sorting (as is the status quo).
  • Technical debt:
    • In the wgpu renderer, we're not picking the size of the instance buffer to correctly account for all these block instances. It's working anyway. Why? There must be some overallocation. (Fixed in 51d9197)
    • Add tests in the mesh crate for instance behavior. Right now all the existing tests continue to pass because the test MeshTypes disables instancing.
    • Tidy up sloppy API choices (types for block instance mesh and ChunkedSpaceMesh instance list).
    • Ensure all TODO(instancing) comments have been dealt with.
@kpreid kpreid added kind: incomplete A feature is partially implemented; the current state of the code is inconsistent kind: performance Ways to increase performance or efficiency without adding functionality area: graphics labels Jan 31, 2024
@kpreid
Copy link
Owner Author

kpreid commented Jan 31, 2024

In the wgpu renderer, we're not picking the size of the instance buffer to correctly account for all these block instances. It's working anyway. Why? There must be some overallocation.

This turned out to be overallocation because we allocate for all chunks and only push culled chunks, so it could fail given enough block instances. Fixed in 51d9197.

@kpreid
Copy link
Owner Author

kpreid commented Feb 1, 2024

a0c5474 fixes some of the mesh API and some of the inefficiency; it is now up to the user of ChunkedSpaceMesh to gather instances (so they can do it as part of the iteration they already must do), and none of the internal hash tables are exposed.

@kpreid
Copy link
Owner Author

kpreid commented Feb 4, 2024

Add tests in the mesh crate for instance behavior.

There are some tests now, added along with specific improvements. I'll call this done-enough-not-to-track even though there's more work ahead.

When a change in the world occurs, consider just adding/replacing one instance instead of remeshing the containing chunk.

This is partly supported via 7a16b29 — instances are never used for blocks that wouldn't be instanced, but instance-only changes should skip the mesh rebuild. However, it doesn't work 100% of the time, for reasons I have not yet diagnosed.

Also of note for performance: 7627d9e added InstanceMap specifically so that removals were possible, but the strategy in 7a16b29 is to sweep the entire chunk, so that we don't have to keep or update a complex todo data structure. So, one or the other should change.

@kpreid
Copy link
Owner Author

kpreid commented Feb 5, 2024

434fc90 makes instance-only updates work consistently.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: graphics kind: incomplete A feature is partially implemented; the current state of the code is inconsistent kind: performance Ways to increase performance or efficiency without adding functionality
Projects
None yet
Development

No branches or pull requests

1 participant