From b61793142edf743c63b221325e2ee012c5236f75 Mon Sep 17 00:00:00 2001 From: Imbris Date: Tue, 27 Apr 2021 19:46:17 -0400 Subject: [PATCH] Make models require a non-zero amount of vertices --- voxygen/src/render/model.rs | 11 ++++++--- voxygen/src/render/renderer.rs | 5 ++-- voxygen/src/scene/figure/cache.rs | 16 +++++-------- voxygen/src/scene/figure/mod.rs | 12 ++++++---- voxygen/src/scene/simple.rs | 6 ++--- voxygen/src/scene/terrain.rs | 39 ++++++++++++++++++------------- 6 files changed, 51 insertions(+), 38 deletions(-) diff --git a/voxygen/src/render/model.rs b/voxygen/src/render/model.rs index 4057981188..7ba2dbb401 100644 --- a/voxygen/src/render/model.rs +++ b/voxygen/src/render/model.rs @@ -29,10 +29,15 @@ pub struct Model { } impl Model { - pub fn new(device: &wgpu::Device, mesh: &Mesh) -> Self { - Self { - vbuf: Buffer::new(device, wgpu::BufferUsage::VERTEX, mesh.vertices()), + /// Returns None if the provided mesh is empty + pub fn new(device: &wgpu::Device, mesh: &Mesh) -> Option { + if mesh.vertices().is_empty() { + return None; } + + Some(Self { + vbuf: Buffer::new(device, wgpu::BufferUsage::VERTEX, mesh.vertices()), + }) } /// Create a model with a slice of a portion of this model to send to the diff --git a/voxygen/src/render/renderer.rs b/voxygen/src/render/renderer.rs index 2744203426..728fbb2ac6 100644 --- a/voxygen/src/render/renderer.rs +++ b/voxygen/src/render/renderer.rs @@ -980,9 +980,10 @@ impl Renderer { } /// Create a new model from the provided mesh. - pub fn create_model(&mut self, mesh: &Mesh) -> Result, RenderError> { + /// If the provided mesh is empty this returns None + pub fn create_model(&mut self, mesh: &Mesh) -> Option> { self.ensure_sufficient_index_length::(mesh.vertices().len()); - Ok(Model::new(&self.device, mesh)) + Model::new(&self.device, mesh) } /// Create a new dynamic model with the specified size. diff --git a/voxygen/src/scene/figure/cache.rs b/voxygen/src/scene/figure/cache.rs index 56560928b2..5380cf1cf8 100644 --- a/voxygen/src/scene/figure/cache.rs +++ b/voxygen/src/scene/figure/cache.rs @@ -370,16 +370,12 @@ where vertex_range, }) = Arc::get_mut(recv).take().and_then(|cell| cell.take()) { - // FIXME: We really need to stop hard failing on failure to upload - // to the GPU. - let model_entry = col_lights - .create_figure( - renderer, - col_light, - (opaque, bounds), - vertex_range, - ) - .expect("Failed to upload figure data to the GPU!"); + let model_entry = col_lights.create_figure( + renderer, + col_light, + (opaque, bounds), + vertex_range, + ); *model = FigureModelEntryFuture::Done(model_entry); // NOTE: Borrow checker isn't smart enough to figure this out. if let FigureModelEntryFuture::Done(model) = model { diff --git a/voxygen/src/scene/figure/mod.rs b/voxygen/src/scene/figure/mod.rs index 17793c570f..e4d919fe63 100644 --- a/voxygen/src/scene/figure/mod.rs +++ b/voxygen/src/scene/figure/mod.rs @@ -5200,13 +5200,15 @@ impl FigureColLights { /// NOTE: Panics if the vertex range bounds are not in range of the opaque /// model stored in the BoneMeshes parameter. This is part of the /// function contract. + /// + /// NOTE: Panics if the provided mesh is empty. FIXME: do something else pub fn create_figure( &mut self, renderer: &mut Renderer, (tex, tex_size): ColLightInfo, (opaque, bounds): (Mesh, math::Aabb), vertex_ranges: [Range; N], - ) -> Result, RenderError> { + ) -> FigureModelEntry { span!(_guard, "create_figure", "FigureColLights::create_figure"); let atlas = &mut self.atlas; let allocation = atlas @@ -5216,7 +5218,9 @@ impl FigureColLights { let col_lights = renderer.figure_bind_col_light(col_lights); let model_len = u32::try_from(opaque.vertices().len()) .expect("The model size for this figure does not fit in a u32!"); - let model = renderer.create_model(&opaque)?; + let model = renderer + .create_model(&opaque) + .expect("The model contains no vertices!"); vertex_ranges.iter().for_each(|range| { assert!( @@ -5228,13 +5232,13 @@ impl FigureColLights { ); }); - Ok(FigureModelEntry { + FigureModelEntry { _bounds: bounds, allocation, col_lights, lod_vertex_ranges: vertex_ranges, model: FigureModel { opaque: model }, - }) + } } #[allow(clippy::unnecessary_wraps)] diff --git a/voxygen/src/scene/simple.rs b/voxygen/src/scene/simple.rs index ff7155f5a6..54fd63bea4 100644 --- a/voxygen/src/scene/simple.rs +++ b/voxygen/src/scene/simple.rs @@ -140,9 +140,9 @@ impl Scene { // total size is bounded by 2^24 * 3 * 1.5 which is bounded by // 2^27, which fits in a u32. let range = 0..opaque_mesh.vertices().len() as u32; - let model = col_lights - .create_figure(renderer, greedy.finalize(), (opaque_mesh, bounds), [range]) - .unwrap(); + let model = + col_lights + .create_figure(renderer, greedy.finalize(), (opaque_mesh, bounds), [range]); let mut buf = [Default::default(); anim::MAX_BONE_COUNT]; state.update( renderer, diff --git a/voxygen/src/scene/terrain.rs b/voxygen/src/scene/terrain.rs index 0f92729792..d24cdb48bd 100644 --- a/voxygen/src/scene/terrain.rs +++ b/voxygen/src/scene/terrain.rs @@ -72,7 +72,7 @@ type LightMapFn = Arc) -> f32 + Send + Sync>; pub struct TerrainChunkData { // GPU data load_time: f32, - opaque_model: Model, + opaque_model: Option>, fluid_model: Option>, /// If this is `None`, this texture is not allocated in the current atlas, /// and therefore there is no need to free its allocation. @@ -1146,18 +1146,8 @@ impl Terrain { self.insert_chunk(response.pos, TerrainChunkData { load_time, - opaque_model: renderer - .create_model(&mesh.opaque_mesh) - .expect("Failed to upload chunk mesh to the GPU!"), - fluid_model: if mesh.fluid_mesh.vertices().len() > 0 { - Some( - renderer - .create_model(&mesh.fluid_mesh) - .expect("Failed to upload chunk mesh to the GPU!"), - ) - } else { - None - }, + opaque_model: renderer.create_model(&mesh.opaque_mesh), + fluid_model: renderer.create_model(&mesh.fluid_mesh), col_lights_alloc: Some(allocation.id), col_lights: Arc::clone(&self.col_lights), light_map: mesh.light_map, @@ -1422,7 +1412,13 @@ impl Terrain { chunk_iter .filter(|chunk| chunk.can_shadow_sun()) .chain(self.shadow_chunks.iter().map(|(_, chunk)| chunk)) - .for_each(|chunk| drawer.draw(&chunk.opaque_model, &chunk.locals)); + .filter_map(|chunk| { + chunk + .opaque_model + .as_ref() + .map(|model| (model, &chunk.locals)) + }) + .for_each(|(model, locals)| drawer.draw(model, locals)); } pub fn chunks_for_point_shadows( @@ -1452,7 +1448,12 @@ impl Terrain { // don't use `shadow_chunks` here. chunk_iter .filter(|chunk| chunk.can_shadow_point) - .map(|chunk| (&chunk.opaque_model, &chunk.locals)) + .filter_map(|chunk| { + chunk + .opaque_model + .as_ref() + .map(|model| (model, &chunk.locals)) + }) } pub fn render<'a>(&'a self, drawer: &mut FirstPassDrawer<'a>, focus_pos: Vec3) { @@ -1470,7 +1471,13 @@ impl Terrain { }) .take(self.chunks.len()) .filter(|chunk| chunk.visible.is_visible()) - .for_each(|chunk| drawer.draw(&chunk.opaque_model, &chunk.col_lights, &chunk.locals)); + .filter_map(|chunk| { + chunk + .opaque_model + .as_ref() + .map(|model| (model, &chunk.col_lights, &chunk.locals)) + }) + .for_each(|(model, col_lights, locals)| drawer.draw(model, col_lights, locals)); } pub fn render_translucent<'a>(