Merge branch 'imbris/fix-mesh-panic' into 'master'

Fix #746 and misc cleanup

Closes #746

See merge request veloren/veloren!1380

common/src/util/mod.rs

@@ -1,5 +1,6 @@
 mod color;
 mod dir;
+mod option;
 
 pub const GIT_VERSION: &str = include_str!(concat!(env!("OUT_DIR"), "/githash"));
 
@@ -10,6 +11,7 @@ lazy_static::lazy_static! {
 
 pub use color::*;
 pub use dir::*;
+pub use option::*;
 
 #[cfg(feature = "tracy")] pub use tracy_client;
 

common/src/util/option.rs

@@ -0,0 +1,10 @@
+pub fn either_with<T, F>(opt1: Option<T>, opt2: Option<T>, f: F) -> Option<T>
+where
+    F: FnOnce(T, T) -> T,
+{
+    match (opt1, opt2) {
+        (Some(v1), Some(v2)) => Some(f(v1, v2)),
+        (Some(v), None) | (None, Some(v)) => Some(v),
+        (None, None) => None,
+    }
+}

voxygen/src/mesh/terrain.rs

@@ -7,11 +7,13 @@ use crate::{
 };
 use common::{
     span,
-    terrain::{Block, BlockKind},
+    terrain::Block,
+    util::either_with,
     vol::{ReadVol, RectRasterableVol, Vox},
     volumes::vol_grid_2d::{CachedVolGrid2d, VolGrid2d},
 };
 use std::{collections::VecDeque, fmt::Debug};
+use tracing::error;
 use vek::*;
 
 type TerrainVertex = <TerrainPipeline as render::Pipeline>::Vertex;
@@ -27,19 +29,6 @@ enum FaceKind {
     Fluid,
 }
 
-trait Blendable {
-    fn is_blended(&self) -> bool;
-}
-
-impl Blendable for BlockKind {
-    #[allow(clippy::match_single_binding)] // TODO: Pending review in #587
-    fn is_blended(&self) -> bool {
-        match self {
-            _ => false,
-        }
-    }
-}
-
 const SUNLIGHT: u8 = 24;
 const _MAX_LIGHT_DIST: i32 = SUNLIGHT as i32;
 
@@ -256,12 +245,9 @@ impl<'a, V: RectRasterableVol<Vox = Block> + ReadVol + Debug>
         // Calculate chunk lighting
         let mut light = calc_light(range, self, lit_blocks);
 
-        let mut lowest_opaque = range.size().d;
+        let mut opaque_limits = None::<Limits>;
-        let mut highest_opaque = 0;
+        let mut fluid_limits = None::<Limits>;
-        let mut lowest_fluid = range.size().d;
+        let mut air_limits = None::<Limits>;
-        let mut highest_fluid = 0;
-        let mut lowest_air = range.size().d;
-        let mut highest_air = 0;
         let flat_get = {
             span!(_guard, "copy to flat array");
             let (w, h, d) = range.size().into_tuple();
@@ -279,15 +265,18 @@ impl<'a, V: RectRasterableVol<Vox = Block> + ReadVol + Debug>
                                 .map(|b| *b)
                                 .unwrap_or(Block::empty());
                             if block.is_opaque() {
-                                lowest_opaque = lowest_opaque.min(z);
+                                opaque_limits = opaque_limits
-                                highest_opaque = highest_opaque.max(z);
+                                    .map(|l| l.including(z))
+                                    .or_else(|| Some(Limits::from_value(z)));
                             } else if block.is_fluid() {
-                                lowest_fluid = lowest_fluid.min(z);
+                                fluid_limits = fluid_limits
-                                highest_fluid = highest_fluid.max(z);
+                                    .map(|l| l.including(z))
+                                    .or_else(|| Some(Limits::from_value(z)));
                             } else {
                                 // Assume air
-                                lowest_air = lowest_air.min(z);
+                                air_limits = air_limits
-                                highest_air = highest_air.max(z);
+                                    .map(|l| l.including(z))
+                                    .or_else(|| Some(Limits::from_value(z)));
                             };
                             flat[i] = block;
                             i += 1;
@@ -307,35 +296,29 @@ impl<'a, V: RectRasterableVol<Vox = Block> + ReadVol + Debug>
             }
         };
 
-        // TODO: figure out why this has to be -2 instead of -1
         // Constrain iterated area
-        let z_start = if (lowest_air > lowest_opaque && lowest_air <= lowest_fluid)
+        let (z_start, z_end) = match (air_limits, fluid_limits, opaque_limits) {
-            || (lowest_air > lowest_fluid && lowest_air <= lowest_opaque)
+            (Some(air), Some(fluid), Some(opaque)) => air.three_way_intersection(fluid, opaque),
-        {
+            (Some(air), Some(fluid), None) => air.intersection(fluid),
-            lowest_air - 2
+            (Some(air), None, Some(opaque)) => air.intersection(opaque),
-        } else if lowest_fluid > lowest_opaque && lowest_fluid <= lowest_air {
+            (None, Some(fluid), Some(opaque)) => fluid.intersection(opaque),
-            lowest_fluid - 2
+            // No interfaces (Note: if there are multiple fluid types this could change)
-        } else if lowest_fluid > lowest_air && lowest_fluid <= lowest_opaque {
+            (Some(_), None, None) | (None, Some(_), None) | (None, None, Some(_)) => None,
-            lowest_fluid - 1
+            (None, None, None) => {
-        } else {
+                error!("Impossible unless given an input AABB that has a height of zero");
-            lowest_opaque - 1
+                None
+            },
         }
-        .max(0);
+        .map_or((0, 0), |limits| {
-        let z_end = if (highest_air < highest_opaque && highest_air >= highest_fluid)
+            let (start, end) = limits.into_tuple();
-            || (highest_air < highest_fluid && highest_air >= highest_opaque)
+            let start = start.max(0);
-        {
+            let end = end.min(range.size().d - 1).max(start);
-            highest_air + 1
+            (start, end)
-        } else if highest_fluid < highest_opaque && highest_fluid >= highest_air {
+        });
-            highest_fluid + 1
-        } else if highest_fluid < highest_air && highest_fluid >= highest_opaque {
-            highest_fluid
-        } else {
-            highest_opaque
-        }
-        .min(range.size().d - 1);
 
         let max_size =
             guillotiere::Size::new(i32::from(max_texture_size.x), i32::from(max_texture_size.y));
+        assert!(z_end >= z_start);
         let greedy_size = Vec3::new(range.size().w - 2, range.size().h - 2, z_end - z_start + 1);
         // NOTE: Terrain sizes are limited to 32 x 32 x 16384 (to fit in 24 bits: 5 + 5
         // + 14). FIXME: Make this function fallible, since the terrain
@@ -452,3 +435,49 @@ fn should_draw_greedy(
         ))
     }
 }
+
+/// 1D Aabr
+#[derive(Copy, Clone, Debug)]
+struct Limits {
+    min: i32,
+    max: i32,
+}
+
+impl Limits {
+    fn from_value(v: i32) -> Self { Self { min: v, max: v } }
+
+    fn including(mut self, v: i32) -> Self {
+        if v < self.min {
+            self.min = v
+        } else if v > self.max {
+            self.max = v
+        }
+        self
+    }
+
+    fn union(self, other: Self) -> Self {
+        Self {
+            min: self.min.min(other.min),
+            max: self.max.max(other.max),
+        }
+    }
+
+    // Find limits that include the overlap of the two
+    fn intersection(self, other: Self) -> Option<Self> {
+        // Expands intersection by 1 since that fits our use-case
+        // (we need to get blocks on either side of the interface)
+        let min = self.min.max(other.min) - 1;
+        let max = self.max.min(other.max) + 1;
+
+        (min < max).then_some(Self { min, max })
+    }
+
+    // Find limits that include any areas of overlap between two of the three
+    fn three_way_intersection(self, two: Self, three: Self) -> Option<Self> {
+        let intersection = self.intersection(two);
+        let intersection = either_with(self.intersection(three), intersection, Limits::union);
+        either_with(two.intersection(three), intersection, Limits::union)
+    }
+
+    fn into_tuple(self) -> (i32, i32) { (self.min, self.max) }
+}

voxygen/src/scene/terrain.rs

@@ -385,7 +385,7 @@ impl<V: RectRasterableVol> Terrain<V> {
     fn make_atlas(
         renderer: &mut Renderer,
     ) -> Result<(AtlasAllocator, Texture<ColLightFmt>), RenderError> {
-        span!(_guard, "male_atlas", "Terrain::make_atlas");
+        span!(_guard, "make_atlas", "Terrain::make_atlas");
         let max_texture_size = renderer.max_texture_size();
         let atlas_size =
             guillotiere::Size::new(i32::from(max_texture_size), i32::from(max_texture_size));
@@ -436,6 +436,7 @@ impl<V: RectRasterableVol> Terrain<V> {
             // Temporarily remember dead chunks for shadowing purposes.
             self.shadow_chunks.push((pos, chunk));
         }
+
         if let Some(_todo) = self.mesh_todo.remove(&pos) {
             //Do nothing on todo mesh removal.
         }
@@ -453,6 +454,22 @@ impl<V: RectRasterableVol> Terrain<V> {
         view_mat: Mat4<f32>,
         proj_mat: Mat4<f32>,
     ) -> (Aabb<f32>, Vec<math::Vec3<f32>>, math::Aabr<f32>) {
+        // Remove any models for chunks that have been recently removed.
+        // Note: Does this before adding to todo list just in case removed chunks were
+        // replaced with new chunks (although this would probably be recorded as
+        // modified chunks)
+        for &pos in &scene_data.state.terrain_changes().removed_chunks {
+            self.remove_chunk(pos);
+            // Remove neighbors from meshing todo
+            for i in -1..2 {
+                for j in -1..2 {
+                    if i != 0 || j != 0 {
+                        self.mesh_todo.remove(&(pos + Vec2::new(i, j)));
+                    }
+                }
+            }
+        }
+
         span!(_guard, "maintain", "Terrain::maintain");
         let current_tick = scene_data.tick;
         let current_time = scene_data.state.get_time();
@@ -553,11 +570,6 @@ impl<V: RectRasterableVol> Terrain<V> {
         }
         drop(guard);
 
-        // Remove any models for chunks that have been recently removed.
-        for &pos in &scene_data.state.terrain_changes().removed_chunks {
-            self.remove_chunk(pos);
-        }
-
         // Limit ourselves to u16::MAX even if larger textures are supported.
         let max_texture_size = renderer.max_texture_size();