From 44b3e24e32baa9ac12c581193bdab0f3f28cda8c Mon Sep 17 00:00:00 2001 From: Carter Anderson Date: Mon, 2 Nov 2020 13:15:07 -0800 Subject: [PATCH] fix mesh allocation bug and public mesh api improvements (#768) --- crates/bevy_gltf/src/loader.rs | 13 +- crates/bevy_render/src/mesh/mesh.rs | 246 ++++++++++-------- .../src/pipeline/pipeline_compiler.rs | 8 +- .../src/pipeline/render_pipelines.rs | 17 +- crates/bevy_text/src/draw.rs | 2 +- crates/bevy_ui/src/widget/text.rs | 12 +- examples/asset/asset_loading.rs | 2 +- 7 files changed, 155 insertions(+), 145 deletions(-) diff --git a/crates/bevy_gltf/src/loader.rs b/crates/bevy_gltf/src/loader.rs index 1e5e2251d5..5406ee6e83 100644 --- a/crates/bevy_gltf/src/loader.rs +++ b/crates/bevy_gltf/src/loader.rs @@ -20,7 +20,7 @@ use gltf::{ Primitive, }; use image::{GenericImageView, ImageFormat}; -use std::{borrow::Cow, path::Path}; +use std::path::Path; use thiserror::Error; /// An error that occurs when loading a GLTF file @@ -90,28 +90,25 @@ async fn load_gltf<'a, 'b>( .read_positions() .map(|v| VertexAttributeValues::Float3(v.collect())) { - mesh.attributes - .insert(Cow::Borrowed(Mesh::ATTRIBUTE_POSITION), vertex_attribute); + mesh.set_attribute(Mesh::ATTRIBUTE_POSITION, vertex_attribute); } if let Some(vertex_attribute) = reader .read_normals() .map(|v| VertexAttributeValues::Float3(v.collect())) { - mesh.attributes - .insert(Cow::Borrowed(Mesh::ATTRIBUTE_NORMAL), vertex_attribute); + mesh.set_attribute(Mesh::ATTRIBUTE_NORMAL, vertex_attribute); } if let Some(vertex_attribute) = reader .read_tex_coords(0) .map(|v| VertexAttributeValues::Float2(v.into_f32().collect())) { - mesh.attributes - .insert(Cow::Borrowed(Mesh::ATTRIBUTE_UV_0), vertex_attribute); + mesh.set_attribute(Mesh::ATTRIBUTE_UV_0, vertex_attribute); } if let Some(indices) = reader.read_indices() { - mesh.indices = Some(Indices::U32(indices.into_u32().collect())); + mesh.set_indices(Some(Indices::U32(indices.into_u32().collect()))); }; load_context.set_labeled_asset(&primitive_label, LoadedAsset::new(mesh)); diff --git a/crates/bevy_render/src/mesh/mesh.rs b/crates/bevy_render/src/mesh/mesh.rs index 6dfbea9bd8..4f6a36d40f 100644 --- a/crates/bevy_render/src/mesh/mesh.rs +++ b/crates/bevy_render/src/mesh/mesh.rs @@ -1,11 +1,11 @@ use crate::{ - pipeline::{PrimitiveTopology, RenderPipelines, VertexFormat}, + pipeline::{IndexFormat, PrimitiveTopology, RenderPipelines, VertexFormat}, renderer::{BufferInfo, BufferUsage, RenderResourceContext, RenderResourceId}, }; use bevy_app::prelude::{EventReader, Events}; use bevy_asset::{AssetEvent, Assets, Handle}; use bevy_core::AsBytes; -use bevy_ecs::{Local, Query, Res, ResMut}; +use bevy_ecs::{Local, Query, Res}; use bevy_math::*; use bevy_type_registry::TypeUuid; use std::borrow::Cow; @@ -89,18 +89,24 @@ pub enum Indices { U16(Vec), U32(Vec), } -// TODO: allow values to be unloaded after been submitting to the GPU to conserve memory -pub type VertexAttributesHashMap = HashMap, VertexAttributeValues>; +impl From<&Indices> for IndexFormat { + fn from(indices: &Indices) -> Self { + match indices { + Indices::U16(_) => IndexFormat::Uint16, + Indices::U32(_) => IndexFormat::Uint32, + } + } +} + +// TODO: allow values to be unloaded after been submitting to the GPU to conserve memory #[derive(Debug, TypeUuid)] #[uuid = "8ecbac0f-f545-4473-ad43-e1f4243af51e"] pub struct Mesh { - pub primitive_topology: PrimitiveTopology, + primitive_topology: PrimitiveTopology, /// `bevy_utils::HashMap` with all defined vertex attributes (Positions, Normals, ...) for this mesh. Attribute name maps to attribute values. - pub attributes: VertexAttributesHashMap, - pub indices: Option, - /// The layout of the attributes in the GPU buffer without `shader_location`. `None` will indicate that no data has been uploaded to the GPU yet. - pub attribute_buffer_descriptor_reference: Option, + attributes: HashMap, VertexAttributeValues>, + indices: Option, } impl Mesh { @@ -113,16 +119,107 @@ impl Mesh { primitive_topology, attributes: Default::default(), indices: None, - attribute_buffer_descriptor_reference: Default::default(), } } + pub fn primitive_topology(&self) -> PrimitiveTopology { + self.primitive_topology + } + + pub fn set_attribute( + &mut self, + name: impl Into>, + values: VertexAttributeValues, + ) { + self.attributes.insert(name.into(), values); + } + + pub fn attribute( + &mut self, + name: impl Into>, + ) -> Option<&VertexAttributeValues> { + self.attributes.get(&name.into()) + } + + pub fn set_indices(&mut self, indices: Option) { + self.indices = indices; + } + + pub fn indices(&self) -> Option<&Indices> { + self.indices.as_ref() + } + pub fn get_index_buffer_bytes(&self) -> Option> { self.indices.as_ref().map(|indices| match &indices { Indices::U16(indices) => indices.as_slice().as_bytes().to_vec(), Indices::U32(indices) => indices.as_slice().as_bytes().to_vec(), }) } + + pub fn get_vertex_buffer_descriptor(&self) -> VertexBufferDescriptor { + let mut attributes = Vec::new(); + let mut accumulated_offset = 0; + for (attribute_name, attribute_values) in self.attributes.iter() { + let vertex_format = VertexFormat::from(attribute_values); + attributes.push(VertexAttributeDescriptor { + name: attribute_name.clone(), + offset: accumulated_offset, + format: vertex_format, + shader_location: 0, + }); + accumulated_offset += vertex_format.get_size(); + } + + VertexBufferDescriptor { + name: Default::default(), + stride: accumulated_offset, + step_mode: InputStepMode::Vertex, + attributes, + } + } + + fn count_vertices(&self) -> usize { + let mut vertex_count: Option = None; + for (attribute_name, attribute_data) in self.attributes.iter() { + let attribute_len = attribute_data.len(); + if let Some(previous_vertex_count) = vertex_count { + assert_eq!(previous_vertex_count, attribute_len, + "Attribute {} has a different vertex count ({}) than other attributes ({}) in this mesh.", attribute_name, attribute_len, previous_vertex_count); + } + vertex_count = Some(attribute_len); + } + + vertex_count.unwrap_or(0) + } + + pub fn get_vertex_buffer_data(&self) -> Vec { + let mut vertex_size = 0; + for attribute_values in self.attributes.values() { + let vertex_format = VertexFormat::from(attribute_values); + vertex_size += vertex_format.get_size() as usize; + } + + let vertex_count = self.count_vertices(); + let mut attributes_interleaved_buffer = vec![0; vertex_count * vertex_size]; + // bundle into interleaved buffers + let mut attribute_offset = 0; + for attribute_values in self.attributes.values() { + let vertex_format = VertexFormat::from(attribute_values); + let attribute_size = vertex_format.get_size() as usize; + let attributes_bytes = attribute_values.get_bytes(); + for (vertex_index, attribute_bytes) in + attributes_bytes.chunks_exact(attribute_size).enumerate() + { + let offset = vertex_index * vertex_size + attribute_offset; + attributes_interleaved_buffer[offset..offset + attribute_size] + .copy_from_slice(attribute_bytes); + } + + attribute_offset += attribute_size; + } + + attributes_interleaved_buffer + } } /// Generation for some primitive shape meshes. @@ -131,7 +228,6 @@ pub mod shape { use crate::pipeline::PrimitiveTopology; use bevy_math::*; use hexasphere::Hexasphere; - use std::borrow::Cow; /// A cube. #[derive(Debug)] @@ -201,13 +297,10 @@ pub mod shape { ]); let mut mesh = Mesh::new(PrimitiveTopology::TriangleList); - mesh.attributes - .insert(Cow::Borrowed(Mesh::ATTRIBUTE_POSITION), positions.into()); - mesh.attributes - .insert(Cow::Borrowed(Mesh::ATTRIBUTE_NORMAL), normals.into()); - mesh.attributes - .insert(Cow::Borrowed(Mesh::ATTRIBUTE_UV_0), uvs.into()); - mesh.indices = Some(indices); + mesh.set_attribute(Mesh::ATTRIBUTE_POSITION, positions.into()); + mesh.set_attribute(Mesh::ATTRIBUTE_NORMAL, normals.into()); + mesh.set_attribute(Mesh::ATTRIBUTE_UV_0, uvs.into()); + mesh.set_indices(Some(indices)); mesh } } @@ -300,13 +393,10 @@ pub mod shape { } let mut mesh = Mesh::new(PrimitiveTopology::TriangleList); - mesh.indices = Some(indices); - mesh.attributes - .insert(Cow::Borrowed(Mesh::ATTRIBUTE_POSITION), positions.into()); - mesh.attributes - .insert(Cow::Borrowed(Mesh::ATTRIBUTE_NORMAL), normals.into()); - mesh.attributes - .insert(Cow::Borrowed(Mesh::ATTRIBUTE_UV_0), uvs.into()); + mesh.set_indices(Some(indices)); + mesh.set_attribute(Mesh::ATTRIBUTE_POSITION, positions.into()); + mesh.set_attribute(Mesh::ATTRIBUTE_NORMAL, normals.into()); + mesh.set_attribute(Mesh::ATTRIBUTE_UV_0, uvs.into()); mesh } } @@ -341,13 +431,10 @@ pub mod shape { } let mut mesh = Mesh::new(PrimitiveTopology::TriangleList); - mesh.indices = Some(indices); - mesh.attributes - .insert(Cow::Borrowed(Mesh::ATTRIBUTE_POSITION), positions.into()); - mesh.attributes - .insert(Cow::Borrowed(Mesh::ATTRIBUTE_NORMAL), normals.into()); - mesh.attributes - .insert(Cow::Borrowed(Mesh::ATTRIBUTE_UV_0), uvs.into()); + mesh.set_indices(Some(indices)); + mesh.set_attribute(Mesh::ATTRIBUTE_POSITION, positions.into()); + mesh.set_attribute(Mesh::ATTRIBUTE_NORMAL, normals.into()); + mesh.set_attribute(Mesh::ATTRIBUTE_UV_0, uvs.into()); mesh } } @@ -415,13 +502,10 @@ pub mod shape { let indices = Indices::U32(indices); let mut mesh = Mesh::new(PrimitiveTopology::TriangleList); - mesh.indices = Some(indices); - mesh.attributes - .insert(Cow::Borrowed(Mesh::ATTRIBUTE_POSITION), points.into()); - mesh.attributes - .insert(Cow::Borrowed(Mesh::ATTRIBUTE_NORMAL), normals.into()); - mesh.attributes - .insert(Cow::Borrowed(Mesh::ATTRIBUTE_UV_0), uvs.into()); + mesh.set_indices(Some(indices)); + mesh.set_attribute(Mesh::ATTRIBUTE_POSITION, points.into()); + mesh.set_attribute(Mesh::ATTRIBUTE_NORMAL, normals.into()); + mesh.set_attribute(Mesh::ATTRIBUTE_UV_0, uvs.into()); mesh } } @@ -456,7 +540,7 @@ pub struct MeshResourceProviderState { pub fn mesh_resource_provider_system( mut state: Local, render_resource_context: Res>, - mut meshes: ResMut>, + meshes: Res>, mesh_events: Res>>, mut query: Query<(&Handle, &mut RenderPipelines)>, ) { @@ -482,7 +566,7 @@ pub fn mesh_resource_provider_system( // update changed mesh data for changed_mesh_handle in changed_meshes.iter() { - if let Some(mesh) = meshes.get_mut(changed_mesh_handle) { + if let Some(mesh) = meshes.get(changed_mesh_handle) { // TODO: check for individual buffer changes in non-interleaved mode let index_buffer = render_resource_context.create_buffer_with_data( BufferInfo { @@ -498,12 +582,8 @@ pub fn mesh_resource_provider_system( INDEX_BUFFER_ASSET_INDEX, ); - // Vertex buffer - let vertex_count = attributes_count_vertices(&mesh.attributes).unwrap(); - let interleaved_buffer = - attributes_to_vertex_buffer_data(&mesh.attributes, vertex_count); + let interleaved_buffer = mesh.get_vertex_buffer_data(); - mesh.attribute_buffer_descriptor_reference = Some(interleaved_buffer.1); render_resource_context.set_asset_resource( changed_mesh_handle, RenderResourceId::Buffer(render_resource_context.create_buffer_with_data( @@ -511,7 +591,7 @@ pub fn mesh_resource_provider_system( buffer_usage: BufferUsage::VERTEX, ..Default::default() }, - &interleaved_buffer.0, + &interleaved_buffer, )), VERTEX_ATTRIBUTE_BUFFER_ID, ); @@ -525,7 +605,7 @@ pub fn mesh_resource_provider_system( buffer_usage: BufferUsage::VERTEX, ..Default::default() }, - &vec![0; (vertex_count * VertexFormat::Float4.get_size() as u32) as usize], + &vec![0; mesh.count_vertices() * VertexFormat::Float4.get_size() as usize], )), VERTEX_FALLBACK_BUFFER_ID, ); @@ -533,11 +613,17 @@ pub fn mesh_resource_provider_system( } // handover buffers to pipeline - // TODO: remove this once batches are pipeline specific and deprecate assigned_meshes draw target for (handle, mut render_pipelines) in query.iter_mut() { if let Some(mesh) = meshes.get(handle) { for render_pipeline in render_pipelines.pipelines.iter_mut() { render_pipeline.specialization.primitive_topology = mesh.primitive_topology; + // TODO: don't allocate a new vertex buffer descriptor for every entity + render_pipeline.specialization.vertex_buffer_descriptor = + mesh.get_vertex_buffer_descriptor(); + render_pipeline.specialization.index_format = mesh + .indices() + .map(|i| i.into()) + .unwrap_or(IndexFormat::Uint32); } if let Some(RenderResourceId::Buffer(index_buffer_resource)) = @@ -566,65 +652,3 @@ pub fn mesh_resource_provider_system( } } } - -pub fn attributes_count_vertices(attributes: &VertexAttributesHashMap) -> Option { - let mut vertex_count: Option = None; - for (attribute_name, attribute_data) in attributes { - let attribute_len = attribute_data.len(); - if let Some(previous_vertex_count) = vertex_count { - assert_eq!(previous_vertex_count, attribute_len as u32, - "Attribute {} has a different vertex count ({}) than other attributes ({}) in this mesh.", attribute_name, attribute_len, previous_vertex_count); - } - vertex_count = Some(attribute_len as u32); - } - vertex_count -} -pub fn attributes_to_vertex_buffer_data( - attributes: &VertexAttributesHashMap, - vertex_count: u32, -) -> (Vec, VertexBufferDescriptor) { - // get existing attribute data as bytes and generate attribute descriptor - let mut attributes_gpu_ready = Vec::<(VertexAttributeDescriptor, &[u8])>::default(); - let mut accumulated_offset = 0; - let mut attributes_sorted: Vec<_> = attributes.iter().collect(); - attributes_sorted.sort_by(|a, b| a.0.cmp(b.0)); - for attribute_data in attributes_sorted { - // TODO: allow for custom converter here - let vertex_format = VertexFormat::from(attribute_data.1); - attributes_gpu_ready.push(( - // this serves as a reference and is not supposed to be used directly. - VertexAttributeDescriptor { - name: attribute_data.0.clone(), - offset: accumulated_offset, - format: vertex_format, - shader_location: 0, - }, - attribute_data.1.get_bytes(), - )); - accumulated_offset += vertex_format.get_size(); - } - let mut attributes_interleaved_buffer = Vec::::default(); - - // bundle into interleaved buffers - for vertex_index in 0..vertex_count { - let vertex_index = vertex_index as usize; - for (attribute_descriptor, attributes_bytes) in &attributes_gpu_ready { - let stride = attribute_descriptor.format.get_size() as usize; - // insert one element - attributes_interleaved_buffer - .extend(&attributes_bytes[vertex_index * stride..vertex_index * stride + stride]); - } - } - - let vertex_buffer_descriptor_reference = VertexBufferDescriptor { - name: Default::default(), - stride: accumulated_offset, - step_mode: InputStepMode::Vertex, - attributes: attributes_gpu_ready.iter().map(|x| x.0.clone()).collect(), - }; - - ( - attributes_interleaved_buffer, - vertex_buffer_descriptor_reference, - ) -} diff --git a/crates/bevy_render/src/pipeline/pipeline_compiler.rs b/crates/bevy_render/src/pipeline/pipeline_compiler.rs index 6c1378711a..f89af48b93 100644 --- a/crates/bevy_render/src/pipeline/pipeline_compiler.rs +++ b/crates/bevy_render/src/pipeline/pipeline_compiler.rs @@ -20,7 +20,7 @@ pub struct PipelineSpecialization { pub primitive_topology: PrimitiveTopology, pub dynamic_bindings: Vec, pub index_format: IndexFormat, - pub mesh_attribute_layout: VertexBufferDescriptor, + pub vertex_buffer_descriptor: VertexBufferDescriptor, pub sample_count: u32, } @@ -28,11 +28,11 @@ impl Default for PipelineSpecialization { fn default() -> Self { Self { sample_count: 1, + index_format: IndexFormat::Uint32, shader_specialization: Default::default(), primitive_topology: Default::default(), dynamic_bindings: Default::default(), - index_format: IndexFormat::Uint32, - mesh_attribute_layout: Default::default(), + vertex_buffer_descriptor: Default::default(), } } } @@ -172,7 +172,7 @@ impl PipelineCompiler { // create a vertex layout that provides all attributes from either the specialized vertex buffers or a zero buffer let mut pipeline_layout = specialized_descriptor.layout.as_mut().unwrap(); // the vertex buffer descriptor of the mesh - let mesh_vertex_buffer_descriptor = pipeline_specialization.mesh_attribute_layout.clone(); + let mesh_vertex_buffer_descriptor = &pipeline_specialization.vertex_buffer_descriptor; // the vertex buffer descriptor that will be used for this pipeline let mut compiled_vertex_buffer_descriptor = VertexBufferDescriptor { diff --git a/crates/bevy_render/src/pipeline/render_pipelines.rs b/crates/bevy_render/src/pipeline/render_pipelines.rs index 5cae2587da..875c6d94bc 100644 --- a/crates/bevy_render/src/pipeline/render_pipelines.rs +++ b/crates/bevy_render/src/pipeline/render_pipelines.rs @@ -1,4 +1,4 @@ -use super::{IndexFormat, PipelineDescriptor, PipelineSpecialization}; +use super::{PipelineDescriptor, PipelineSpecialization}; use crate::{ draw::{Draw, DrawContext}, mesh::{Indices, Mesh}, @@ -91,21 +91,16 @@ pub fn draw_render_pipelines_system( continue; }; - let (index_range, index_format) = match mesh.indices.as_ref() { - Some(Indices::U32(indices)) => (Some(0..indices.len() as u32), IndexFormat::Uint32), - Some(Indices::U16(indices)) => (Some(0..indices.len() as u32), IndexFormat::Uint16), - None => (None, IndexFormat::Uint32), + let index_range = match mesh.indices() { + Some(Indices::U32(indices)) => Some(0..indices.len() as u32), + Some(Indices::U16(indices)) => Some(0..indices.len() as u32), + None => None, }; let render_pipelines = &mut *render_pipelines; for pipeline in render_pipelines.pipelines.iter_mut() { pipeline.specialization.sample_count = msaa.samples; - pipeline.specialization.index_format = index_format; - pipeline.specialization.mesh_attribute_layout = mesh - .attribute_buffer_descriptor_reference - .as_ref() - .unwrap() - .clone(); + // TODO: move these to mesh.rs? } for render_pipeline in render_pipelines.pipelines.iter() { diff --git a/crates/bevy_text/src/draw.rs b/crates/bevy_text/src/draw.rs index 231fc4a8a2..4f08bd40d5 100644 --- a/crates/bevy_text/src/draw.rs +++ b/crates/bevy_text/src/draw.rs @@ -51,7 +51,7 @@ impl<'a> Drawable for DrawableText<'a> { &bevy_sprite::SPRITE_SHEET_PIPELINE_HANDLE, &PipelineSpecialization { sample_count: self.msaa.samples, - mesh_attribute_layout: self.font_quad_vertex_descriptor.clone(), + vertex_buffer_descriptor: self.font_quad_vertex_descriptor.clone(), ..Default::default() }, )?; diff --git a/crates/bevy_ui/src/widget/text.rs b/crates/bevy_ui/src/widget/text.rs index 71ee2180de..6b2f5c1133 100644 --- a/crates/bevy_ui/src/widget/text.rs +++ b/crates/bevy_ui/src/widget/text.rs @@ -100,14 +100,8 @@ pub fn draw_text_system( mut asset_render_resource_bindings: ResMut, mut query: Query<(&mut Draw, &Text, &Node, &GlobalTransform)>, ) { - let font_quad_vertex_descriptor = { - let font_quad = meshes.get(&QUAD_HANDLE).unwrap(); - font_quad - .attribute_buffer_descriptor_reference - .as_ref() - .unwrap() - .clone() - }; + let font_quad = meshes.get(&QUAD_HANDLE).unwrap(); + let vertex_buffer_descriptor = font_quad.get_vertex_buffer_descriptor(); for (mut draw, text, node, global_transform) in query.iter_mut() { if let Some(font) = fonts.get(&text.font) { @@ -123,7 +117,7 @@ pub fn draw_text_system( style: &text.style, text: &text.value, container_size: node.size, - font_quad_vertex_descriptor: &font_quad_vertex_descriptor, + font_quad_vertex_descriptor: &vertex_buffer_descriptor, }; drawable_text.draw(&mut draw, &mut draw_context).unwrap(); } diff --git a/examples/asset/asset_loading.rs b/examples/asset/asset_loading.rs index d10f572f29..e9dc792d53 100644 --- a/examples/asset/asset_loading.rs +++ b/examples/asset/asset_loading.rs @@ -23,7 +23,7 @@ fn setup( if let Some(sphere) = meshes.get(&sphere_handle) { // You might notice that this doesn't run! This is because assets load in parallel without blocking. // When an asset has loaded, it will appear in relevant Assets collection. - println!("{:?}", sphere.primitive_topology); + println!("{:?}", sphere.primitive_topology()); } else { println!("sphere hasn't loaded yet"); }