From d87225d908a2fca89856e5f37010b68d84229c32 Mon Sep 17 00:00:00 2001 From: Joshua Yanovski Date: Tue, 18 May 2021 15:30:25 -0700 Subject: [PATCH] Various improvements to chunk load latency. Firstly, most importantly, improves the heuristic used for deciding which chunks to mesh (which matters more even at low view distances with meshing being so expensive now, but has an even more obvious improvement at large view distances). Essentially, instead of always prioritizing whatever chunk was fetched earliest from the server, instead we prioritize chunks *closest* to the player first, then chunk order. This greatly improves the apparent latency for things like picking up a sprite, as well as cases where the player moves out of the loaded range but (due to slow loading from the server or a large VD range) there are many remaining chunks left to be meshed still within the VD but nowhere near the player. By properly priotizing chunks near the player, we minimize the time / likelihood of a player being on or very near an unmeshed chunk, and make high VDs and faster travel speeds more viable. We make a few other minor improvements as well: Avoid duplicate meshing of neighbors when first inserting chunks, if they are already in the todo list and the chunk being inserted was not directly modified. Also avoid remeshing neighbors if only a solid block's color changed, which could sometimes be useful for non-sprite modifications (for example flame-induced changes to non-destructible terrain color). --- voxygen/src/mesh/terrain.rs | 2 ++ voxygen/src/scene/terrain.rs | 67 ++++++++++++++++++++++++++---------- 2 files changed, 50 insertions(+), 19 deletions(-) diff --git a/voxygen/src/mesh/terrain.rs b/voxygen/src/mesh/terrain.rs index 8a32bc801b..7904be3a6e 100644 --- a/voxygen/src/mesh/terrain.rs +++ b/voxygen/src/mesh/terrain.rs @@ -464,6 +464,8 @@ impl<'a, V: RectRasterableVol + ReadVol + Debug + 'static> } } +/// NOTE: Make sure to reflect any changes to how meshing is performanced in +/// [scene::terrain::Terrain::skip_remesh]. fn should_draw_greedy( pos: Vec3, delta: Vec3, diff --git a/voxygen/src/scene/terrain.rs b/voxygen/src/scene/terrain.rs index bfd5333142..3713425bd5 100644 --- a/voxygen/src/scene/terrain.rs +++ b/voxygen/src/scene/terrain.rs @@ -186,6 +186,7 @@ fn mesh_worker + RectRasterableVol + ReadVol + Debug + ' &blocks_of_interest, )); mesh = Some(MeshWorkerResponseMesh { + // TODO: Take sprite bounds into account somehow? z_bounds: (bounds.min.z, bounds.max.z), opaque_mesh, fluid_mesh, @@ -644,14 +645,29 @@ impl Terrain { } /// Determine whether a given block change actually require remeshing. - fn skip_remesh(old_block: Block, new_block: Block) -> bool { - // Both blocks are unfilled and of the same kind (this includes - // sprites within the same fluid, for example). - !new_block.is_filled() && !old_block.is_filled() && new_block.kind() == old_block.kind() && - // Block glow and sunlight handling are the same (so we don't have to redo - // lighting). - new_block.get_glow() == old_block.get_glow() && - new_block.get_max_sunlight() == old_block.get_max_sunlight() + /// + /// Returns (skip_color, skip_lights) where + /// + /// skip_color means no textures were recolored (i.e. this was a sprite only + /// change). + /// + /// skip_lights means no remeshing or relighting was required + /// (i.e. the block opacity / lighting info / block kind didn't change). + fn skip_remesh(old_block: Block, new_block: Block) -> (bool, bool) { + let same_mesh = + // Both blocks are of the same opacity and same liquidity (since these are what we use + // to determine mesh boundaries). + new_block.is_liquid() == old_block.is_liquid() && + new_block.is_opaque() == old_block.is_opaque(); + let skip_lights = same_mesh && + // Block glow and sunlight handling are the same (so we don't have to redo + // lighting). + new_block.get_glow() == old_block.get_glow() && + new_block.get_max_sunlight() == old_block.get_max_sunlight(); + let skip_color = same_mesh && + // Both blocks are uncolored + !new_block.has_color() && !old_block.has_color(); + (skip_color, skip_lights) } /// Find the glow level (light from lamps) at the given world position. @@ -770,7 +786,9 @@ impl Terrain { for j in -1..2 { let pos = pos + Vec2::new(i, j); - if !self.chunks.contains_key(&pos) || modified { + if !(self.chunks.contains_key(&pos) || self.mesh_todo.contains_key(&pos)) + || modified + { let mut neighbours = true; for i in -1..2 { for j in -1..2 { @@ -806,7 +824,7 @@ impl Terrain { // stores the old state. let new_block = scene_data.state.get_block(pos); - let skip_remesh = if let Some(new_block) = new_block { + let (skip_color, skip_lights) = if let Some(new_block) = new_block { Self::skip_remesh(old_block, new_block) } else { // The block coordinates of a modified block should be in bounds, since they are @@ -819,9 +837,13 @@ impl Terrain { bug; please contact the developers if you see this error message!", pos ); - false + (false, false) }; + // Currently, we can only skip remeshing if both lights and + // colors don't need to be reworked. + let skip_remesh = skip_color && skip_lights; + // TODO: Be cleverer about this to avoid remeshing all neighbours. There are a // few things that can create an 'effect at a distance'. These are // as follows: @@ -831,10 +853,11 @@ impl Terrain { // removed (or added) thereby // changing the way that sunlight propagates into the cavity. // - // We can and should be cleverer about this, but it's non-trivial. For now, just - // conservatively assume that the lighting in all neighbouring - // chunks is invalidated. Thankfully, this doesn't need to happen often - // because block modification is unusual in Veloren. + // We can and should be cleverer about this, but it's non-trivial. For now, we + // don't remesh if only a block color changed or a sprite was + // altered in a way that doesn't affect its glow, but we make no + // attempt to do smarter cavity checking (to see if altering the + // block changed the sunlight neighbors could get). // let block_effect_radius = block.get_glow().unwrap_or(0).max(1); let block_effect_radius = crate::mesh::terrain::MAX_LIGHT_DIST; @@ -848,9 +871,9 @@ impl Terrain { let neighbour_pos = pos + Vec3::new(x, y, 0) * block_effect_radius; let neighbour_chunk_pos = scene_data.state.terrain().pos_key(neighbour_pos); - if skip_remesh && !(x == 0 && y == 0) { + if skip_lights && !(x == 0 && y == 0) { // We don't need to remesh neighboring chunks if this block change doesn't - // require remeshing. + // require relighting. continue; } @@ -900,18 +923,24 @@ impl Terrain { }; span!(guard, "Queue meshing from todo list"); + let mesh_focus_pos = focus_pos.map(|e| e.trunc()).xy().as_::(); for (todo, chunk) in self .mesh_todo .values_mut() .filter(|todo| !todo.is_worker_active) - .min_by_key(|todo| todo.started_tick) + .min_by_key(|todo| ((todo.pos.as_::() * TerrainChunk::RECT_SIZE.as_::()).distance_squared(mesh_focus_pos), todo.started_tick)) // Find a reference to the actual `TerrainChunk` we're meshing .and_then(|todo| { let pos = todo.pos; Some((todo, scene_data.state .terrain() .get_key_arc(pos) - .cloned()?)) + .cloned() + .or_else(|| { + warn!("Invariant violation: a chunk whose neighbors have not been fetched was found in the todo list, + which could halt meshing entirely."); + None + })?)) }) { if self.mesh_todos_active.load(Ordering::Relaxed) > meshing_cores {