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

Implement accurate margins between adjacent chunks #379

Merged
merged 6 commits into from
Apr 29, 2024

Conversation

patowen
Copy link
Collaborator

@patowen patowen commented Apr 21, 2024

Fixes #41 (Although ambient occlusion won't be 100% accurate due to ignored edge and corner margins)
Fixes #330 (Necessary to avoid noticeable z-fighting after this change)

This PR ensures that the chunk margins that affect which surfaces are extracted are accurate, ensuring that no surfaces between two solid voxels are rendered, which should result in better draw performance and a clearer view when no-clipping beneath the terrain.

After a chunk is ready to be added to the world (which could be from terrain generation, loading saves, or downloading chunks from a server), the margins of that chunk and its adjacent chunks are modified to be consistent with each other. Then, whenever a block update occurs, margins in adjacent chunks are updated to be consistent with that block update.

To help facilitate this feature, a few additional chunk-coordinate-related math functions have been added to make it easier to reason about how adjacent chunks interact with each other, especially given that different vertices have (necessarily) inconsistent orientations.

A small change has been made to the "extract.comp" shader to prevent the wrong chunk from rendering a particular surface.

@patowen
Copy link
Collaborator Author

patowen commented Apr 21, 2024

I don't believe the CI failure is related to this PR. I'm willing to look into it a bit more closely, but the PR should still be ready to review as-is.

@patowen patowen requested a review from Ralith April 21, 2024 05:53
@patowen
Copy link
Collaborator Author

patowen commented Apr 21, 2024

I ran some very crude benchmarks, and it seems like any performance costs for margin computation is likely insignificant enough that we don't need to put much effort into optimizing it for now. I added a few extra temporary metrics to add an additional crude benefit to measure the performance of the code in margins.rs:

--- a/client/src/graphics/voxels/mod.rs
+++ b/client/src/graphics/voxels/mod.rs
@@ -99,9 +99,14 @@ impl Voxels {
         for chunk in frame.drawn.drain(..) {
             self.states.peek_mut(chunk).refcount -= 1;
         }
+        let populate_chunks_started = Instant::now();
+        let mut num_chunks_populated = 0;
         while let Some(chunk) = self.worldgen.poll() {
+            num_chunks_populated += 1;
+            let populate_chunk_started = Instant::now();
             let chunk_id = ChunkId::new(chunk.node, chunk.chunk);
             sim.graph.populate_chunk(chunk_id, chunk.voxels, false);
+            metrics::histogram!("populate_chunk").record(populate_chunk_started.elapsed());

             // Now that the block is populated, we can apply any pending block updates the server
             // provided that the client couldn't apply.
@@ -112,6 +117,9 @@ impl Voxels {
                 }
             }
         }
+        if num_chunks_populated != 0 {
+            metrics::histogram!("populate_chunks").record(populate_chunks_started.elapsed());
+        }

         // Determine what to load/render
         let view = sim.view();

With a view distance of 90, where I load up the game with cargo run --bin client --release and run to the left a little bit on the bridge, I get the following on my computer.

On master branch:

key=populate_chunk percentile_25=300ns percentile_50=400ns percentile_75=500ns max=125.055µs
key=populate_chunks percentile_25=3.001µs percentile_50=4.503µs percentile_75=11.607µs max=285.439µs
key=frame.cpu percentile_25=7.069695ms percentile_50=7.553023ms percentile_75=8.146943ms max=17.154047ms
key=frame.cpu.voxels.draw percentile_25=26.111µs percentile_50=33.215µs percentile_75=41.407µs max=137.727µs
key=frame.cpu.voxels.graph_traversal percentile_25=1.458175ms percentile_50=1.560575ms percentile_75=1.758207ms max=3.268607ms
key=frame.cpu.voxels.node_scan percentile_25=2.646015ms percentile_50=2.887679ms percentile_75=3.219455ms max=11.255807ms
key=frame.gpu.draw percentile_25=1.196031ms percentile_50=1.407999ms percentile_75=1.759231ms max=3.491839ms
key=frame.gpu.after_draw percentile_25=43.711µs percentile_50=49.535µs percentile_75=92.479µs max=878.079µs

On margins branch:

key=populate_chunk percentile_25=300ns percentile_50=400ns percentile_75=600ns max=81.727µs
key=populate_chunks percentile_25=3.401µs percentile_50=6.403µs percentile_75=29.903µs max=942.079µs
key=frame.cpu percentile_25=7.229439ms percentile_50=7.770111ms percentile_75=8.372223ms max=14.811135ms
key=frame.cpu.voxels.draw percentile_25=26.015µs percentile_50=32.703µs percentile_75=40.703µs max=1.399807ms
key=frame.cpu.voxels.graph_traversal percentile_25=1.446911ms percentile_50=1.553407ms percentile_75=1.715199ms max=2.990079ms
key=frame.cpu.voxels.node_scan percentile_25=2.811903ms percentile_50=3.110911ms percentile_75=3.510271ms max=9.019391ms
key=frame.gpu.draw percentile_25=188.543µs percentile_50=612.863µs percentile_75=738.303µs max=1.102847ms
key=frame.gpu.after_draw percentile_25=45.567µs percentile_50=57.695µs percentile_75=118.463µs max=1.138687ms

To make this easier to read, here's the same thing but with numbers reduced to 2 significant figures and master and margins next to each other for easier comparison:

branch  metric                 [  25%,   50%,   75%,   max]

master  populate_chunk         [300ns, 400ns, 500ns, 130µs]
margins populate_chunk         [300ns, 400ns, 600ns,  82µs]

master  populate_chunks        [3.0µs, 4.5µs,  12µs, 290µs]
margins populate_chunks        [3.4µs, 6.4µs,  30µs, 940µs]

master  cpu                    [7.1ms, 7.6ms, 8.1ms,  17ms]
margins cpu                    [7.2ms, 7.8ms, 8.4ms,  15ms]

master  voxels.draw            [ 26µs,  33µs, 41µs,  140µs]
margins voxels.draw            [ 26µs,  33µs,  41µs, 1.4ms]

master  voxels.graph_traversal [1.5ms, 1.6ms, 1.8ms, 3.3ms]
margins voxels.graph_traversal [1.4ms, 1.6ms, 1.7ms, 3.0ms]

master  voxels.node_scan       [2.6ms, 2.9ms, 3.2ms,  11ms]
margins voxels.node_scan       [2.8ms, 3.1ms, 3.8ms, 9.0ms]

master  gpu.draw               [1.2ms, 1.4ms, 1.8ms, 3.5ms]
margins gpu.draw               [190µs, 610µs, 740µs, 1.1ms]

master  gpu.after_draw         [ 44µs,  50µs,  92µs, 880µs]
margins gpu.after_draw         [ 46µs,  58µs, 120µs, 1.1ms]

I'm not sure how much faith can be put in these numbers, as the benchmarks I ran were very crude, likely also depending on difficult-to-reproduce factors like route taken, speed, etc.

As margins are things that ought to have been done, I think this level of benchmarking is probably fine for this PR, but when we start actually working on optimizing the game, it would definitely be worth reworking how we measure performance.

EDIT: I'm also not sure if we can appreciably increase render distance until #52 is completed. To figure out the extent of performance left on the table, one option would be to generate a certain range of terrain offline, send it all to the GPU (probably still separated by chunks), and see how fast it is. I'm not sure if we're at the stage yet to optimize this much, though.

@patowen
Copy link
Collaborator Author

patowen commented Apr 21, 2024

I looked into the CI failure, and I believe the root cause is rust-lang/jobserver-rs#87, I think that hopefully, once rust-lang/jobserver-rs#88 is deployed (which should be in version 0.1.31), this PR will be able to pass the CI checks. I believe the breakage was introduced in 0.1.29 (and continued onto 0.1.30 after 0.1.29 was yanked), and 0.1.29 was released on April 11th 2024, 10 days ago.

EDIT: 0.1.31 was released today (April 22nd, 10 hours ago as of this edit), and CI now succeeds!

common/src/dodeca.rs Outdated Show resolved Hide resolved
common/src/dodeca.rs Outdated Show resolved Hide resolved
common/src/dodeca.rs Outdated Show resolved Hide resolved
common/src/dodeca.rs Outdated Show resolved Hide resolved
common/src/voxel_math.rs Outdated Show resolved Hide resolved
common/src/margins.rs Show resolved Hide resolved
common/src/margins.rs Show resolved Hide resolved
common/src/node.rs Outdated Show resolved Hide resolved
common/src/node.rs Outdated Show resolved Hide resolved
common/src/margins.rs Outdated Show resolved Hide resolved
common/src/dodeca.rs Outdated Show resolved Hide resolved
common/src/dodeca.rs Show resolved Hide resolved
@patowen patowen merged commit a727c32 into Ralith:master Apr 29, 2024
6 checks passed
@patowen patowen deleted the margins branch April 29, 2024 23:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Surface extraction can produce extra surfaces with a risk of z-fighting Populate chunk margins
2 participants