From c937f7baa26a75dea1d1a44ce1370fbd3163874c Mon Sep 17 00:00:00 2001 From: Carter Anderson Date: Tue, 1 Dec 2020 19:11:04 -0800 Subject: [PATCH 1/2] optimize Text rendering / SharedBuffers --- crates/bevy_render/src/draw.rs | 16 +- .../render_graph/nodes/shared_buffers_node.rs | 5 +- .../render_resource/shared_buffers.rs | 148 +++++++++++------- crates/bevy_text/src/draw.rs | 15 +- crates/bevy_wgpu/src/lib.rs | 11 +- 5 files changed, 105 insertions(+), 90 deletions(-) diff --git a/crates/bevy_render/src/draw.rs b/crates/bevy_render/src/draw.rs index ea489e175e13b..155fbbdababe4 100644 --- a/crates/bevy_render/src/draw.rs +++ b/crates/bevy_render/src/draw.rs @@ -1,7 +1,7 @@ use crate::{ pipeline::{PipelineCompiler, PipelineDescriptor, PipelineLayout, PipelineSpecialization}, renderer::{ - BindGroup, BindGroupId, BufferId, BufferUsage, RenderResource, RenderResourceBinding, + BindGroup, BindGroupId, BufferId, RenderResource, RenderResourceBinding, RenderResourceBindings, RenderResourceContext, SharedBuffers, }, shader::Shader, @@ -125,7 +125,7 @@ pub struct DrawContext<'a> { pub shaders: ResMut<'a, Assets>, pub pipeline_compiler: ResMut<'a, PipelineCompiler>, pub render_resource_context: Res<'a, Box>, - pub shared_buffers: Res<'a, SharedBuffers>, + pub shared_buffers: ResMut<'a, SharedBuffers>, #[system_param(ignore)] pub current_pipeline: Option>, } @@ -135,19 +135,11 @@ pub struct FetchDrawContext; impl<'a> DrawContext<'a> { pub fn get_uniform_buffer( - &self, - render_resource: &T, - ) -> Result { - self.get_buffer(render_resource, BufferUsage::UNIFORM) - } - - pub fn get_buffer( - &self, + &mut self, render_resource: &T, - buffer_usage: BufferUsage, ) -> Result { self.shared_buffers - .get_buffer(render_resource, buffer_usage) + .get_uniform_buffer(&**self.render_resource_context, render_resource) .ok_or(DrawError::BufferAllocationFailure) } diff --git a/crates/bevy_render/src/render_graph/nodes/shared_buffers_node.rs b/crates/bevy_render/src/render_graph/nodes/shared_buffers_node.rs index 676df16fc5162..39144b22d744d 100644 --- a/crates/bevy_render/src/render_graph/nodes/shared_buffers_node.rs +++ b/crates/bevy_render/src/render_graph/nodes/shared_buffers_node.rs @@ -16,8 +16,7 @@ impl Node for SharedBuffersNode { _input: &ResourceSlots, _output: &mut ResourceSlots, ) { - let shared_buffers = resources.get::().unwrap(); - let mut command_queue = shared_buffers.reset_command_queue(); - command_queue.execute(render_context); + let mut shared_buffers = resources.get_mut::().unwrap(); + shared_buffers.apply(render_context); } } diff --git a/crates/bevy_render/src/renderer/render_resource/shared_buffers.rs b/crates/bevy_render/src/renderer/render_resource/shared_buffers.rs index 39dcb3c673ea5..7e8b05edb4792 100644 --- a/crates/bevy_render/src/renderer/render_resource/shared_buffers.rs +++ b/crates/bevy_render/src/renderer/render_resource/shared_buffers.rs @@ -1,77 +1,104 @@ use super::{BufferId, BufferInfo, RenderResource, RenderResourceBinding}; use crate::{ render_graph::CommandQueue, - renderer::{BufferUsage, RenderResourceContext}, + renderer::{BufferUsage, RenderContext, RenderResourceContext}, }; -use bevy_ecs::Res; -use parking_lot::RwLock; -use std::sync::Arc; +use bevy_ecs::{Res, ResMut}; -// TODO: Instead of allocating small "exact size" buffers each frame, this should use multiple large shared buffers and probably -// a long-living "cpu mapped" staging buffer. Im punting that for now because I don't know the best way to use wgpu's new async -// buffer mapping yet. pub struct SharedBuffers { - render_resource_context: Box, - buffers: Arc>>, - command_queue: Arc>, + staging_buffer: BufferId, + uniform_buffer: BufferId, + buffers_to_free: Vec, + buffer_size: usize, + initial_size: usize, + current_offset: usize, + command_queue: CommandQueue, } impl SharedBuffers { - pub fn new(render_resource_context: Box) -> Self { + pub fn new(initial_size: usize) -> Self { Self { - render_resource_context, - buffers: Default::default(), + staging_buffer: BufferId::new(), // non-existent buffer + uniform_buffer: BufferId::new(), // non-existent buffer + buffer_size: 0, + current_offset: 0, + initial_size, + buffers_to_free: Default::default(), command_queue: Default::default(), } } - pub fn get_buffer( - &self, + pub fn grow( + &mut self, + render_resource_context: &dyn RenderResourceContext, + required_space: usize, + ) { + let first_resize = self.buffer_size == 0; + while self.buffer_size < self.current_offset + required_space { + self.buffer_size = if self.buffer_size == 0 { + self.initial_size + } else { + self.buffer_size * 2 + }; + } + + self.current_offset = 0; + + // ignore the initial "dummy buffers" + if !first_resize { + render_resource_context.unmap_buffer(self.staging_buffer); + self.buffers_to_free.push(self.staging_buffer); + self.buffers_to_free.push(self.uniform_buffer); + } + + self.staging_buffer = render_resource_context.create_buffer(BufferInfo { + size: self.buffer_size, + buffer_usage: BufferUsage::MAP_WRITE | BufferUsage::COPY_SRC, + mapped_at_creation: true, + }); + self.uniform_buffer = render_resource_context.create_buffer(BufferInfo { + size: self.buffer_size, + buffer_usage: BufferUsage::COPY_DST | BufferUsage::UNIFORM, + mapped_at_creation: false, + }); + } + + pub fn get_uniform_buffer( + &mut self, + render_resource_context: &dyn RenderResourceContext, render_resource: &T, - buffer_usage: BufferUsage, ) -> Option { if let Some(size) = render_resource.buffer_byte_len() { - let aligned_size = self - .render_resource_context - .get_aligned_uniform_size(size, false); - // PERF: this buffer will be slow - let staging_buffer = self.render_resource_context.create_buffer(BufferInfo { - size: aligned_size, - buffer_usage: BufferUsage::COPY_SRC | BufferUsage::MAP_WRITE, - mapped_at_creation: true, - }); + // TODO: overlap alignment if/when possible + let aligned_size = render_resource_context.get_aligned_uniform_size(size, true); + let mut new_offset = self.current_offset + aligned_size; + if new_offset > self.buffer_size { + self.grow(render_resource_context, aligned_size); + new_offset = aligned_size; + } + + let range = self.current_offset as u64..new_offset as u64; - self.render_resource_context.write_mapped_buffer( - staging_buffer, - 0..size as u64, + render_resource_context.write_mapped_buffer( + self.staging_buffer, + range.clone(), &mut |data, _renderer| { render_resource.write_buffer_bytes(data); }, ); - self.render_resource_context.unmap_buffer(staging_buffer); - - let destination_buffer = self.render_resource_context.create_buffer(BufferInfo { - size: aligned_size, - buffer_usage: BufferUsage::COPY_DST | buffer_usage, - ..Default::default() - }); - - let mut command_queue = self.command_queue.write(); - command_queue.copy_buffer_to_buffer( - staging_buffer, - 0, - destination_buffer, - 0, - size as u64, + self.command_queue.copy_buffer_to_buffer( + self.staging_buffer, + self.current_offset as u64, + self.uniform_buffer, + self.current_offset as u64, + aligned_size as u64, ); - let mut buffers = self.buffers.write(); - buffers.push(staging_buffer); - buffers.push(destination_buffer); + self.current_offset = new_offset; Some(RenderResourceBinding::Buffer { - buffer: destination_buffer, - range: 0..aligned_size as u64, + buffer: self.uniform_buffer, + range, dynamic_index: None, }) } else { @@ -79,21 +106,24 @@ impl SharedBuffers { } } - // TODO: remove this when this actually uses shared buffers - pub fn free_buffers(&self) { - let mut buffers = self.buffers.write(); - for buffer in buffers.drain(..) { - self.render_resource_context.remove_buffer(buffer) + pub fn update(&mut self, render_resource_context: &dyn RenderResourceContext) { + self.current_offset = 0; + for buffer in self.buffers_to_free.drain(..) { + render_resource_context.remove_buffer(buffer) } + render_resource_context.map_buffer(self.staging_buffer); } - pub fn reset_command_queue(&self) -> CommandQueue { - let mut command_queue = self.command_queue.write(); - std::mem::take(&mut *command_queue) + pub fn apply(&mut self, render_context: &mut dyn RenderContext) { + render_context.resources().unmap_buffer(self.staging_buffer); + let mut command_queue = std::mem::take(&mut self.command_queue); + command_queue.execute(render_context); } } -// TODO: remove this when this actually uses shared buffers -pub fn free_shared_buffers_system(shared_buffers: Res) { - shared_buffers.free_buffers(); +pub fn shared_buffers_update_system( + mut shared_buffers: ResMut, + render_resource_context: Res>, +) { + shared_buffers.update(&**render_resource_context); } diff --git a/crates/bevy_text/src/draw.rs b/crates/bevy_text/src/draw.rs index d06212c7b5b1a..60473b0d60d29 100644 --- a/crates/bevy_text/src/draw.rs +++ b/crates/bevy_text/src/draw.rs @@ -5,10 +5,7 @@ use bevy_render::{ mesh, pipeline::{PipelineSpecialization, VertexBufferDescriptor}, prelude::Msaa, - renderer::{ - AssetRenderResourceBindings, BindGroup, BufferUsage, RenderResourceBindings, - RenderResourceId, - }, + renderer::{AssetRenderResourceBindings, BindGroup, RenderResourceBindings, RenderResourceId}, }; use bevy_sprite::TextureAtlasSprite; use glyph_brush_layout::{HorizontalAlign, VerticalAlign}; @@ -108,14 +105,8 @@ impl<'a> Drawable for DrawableText<'a> { let transform = Mat4::from_translation(self.position + tv.position.extend(0.)); - let transform_buffer = context - .shared_buffers - .get_buffer(&transform, BufferUsage::UNIFORM) - .unwrap(); - let sprite_buffer = context - .shared_buffers - .get_buffer(&sprite, BufferUsage::UNIFORM) - .unwrap(); + let transform_buffer = context.get_uniform_buffer(&transform).unwrap(); + let sprite_buffer = context.get_uniform_buffer(&sprite).unwrap(); let sprite_bind_group = BindGroup::build() .add_binding(0, transform_buffer) .add_binding(1, sprite_buffer) diff --git a/crates/bevy_wgpu/src/lib.rs b/crates/bevy_wgpu/src/lib.rs index 27c1b3e802575..f11ac6aaea49e 100644 --- a/crates/bevy_wgpu/src/lib.rs +++ b/crates/bevy_wgpu/src/lib.rs @@ -12,7 +12,7 @@ pub use wgpu_resources::*; use bevy_app::prelude::*; use bevy_ecs::{Resources, World}; -use bevy_render::renderer::{free_shared_buffers_system, RenderResourceContext, SharedBuffers}; +use bevy_render::renderer::{shared_buffers_update_system, RenderResourceContext, SharedBuffers}; use renderer::WgpuRenderResourceContext; #[derive(Default)] @@ -22,7 +22,10 @@ impl Plugin for WgpuPlugin { fn build(&self, app: &mut AppBuilder) { let render_system = get_wgpu_render_system(app.resources_mut()); app.add_system_to_stage(bevy_render::stage::RENDER, render_system) - .add_system_to_stage(bevy_render::stage::POST_RENDER, free_shared_buffers_system); + .add_system_to_stage( + bevy_render::stage::POST_RENDER, + shared_buffers_update_system, + ); } } @@ -32,8 +35,8 @@ pub fn get_wgpu_render_system(resources: &mut Resources) -> impl FnMut(&mut Worl .unwrap_or_else(WgpuOptions::default); let mut wgpu_renderer = future::block_on(WgpuRenderer::new(options)); let resource_context = WgpuRenderResourceContext::new(wgpu_renderer.device.clone()); - resources.insert::>(Box::new(resource_context.clone())); - resources.insert(SharedBuffers::new(Box::new(resource_context))); + resources.insert::>(Box::new(resource_context)); + resources.insert(SharedBuffers::new(4096)); move |world, resources| { wgpu_renderer.update(world, resources); } From 5856b5e5c72db0e474e0e5924796b54979625fc4 Mon Sep 17 00:00:00 2001 From: Carter Anderson Date: Tue, 1 Dec 2020 20:03:22 -0800 Subject: [PATCH 2/2] use option instead of non-existent buffers --- .../render_resource/shared_buffers.rs | 49 +++++++++++-------- 1 file changed, 28 insertions(+), 21 deletions(-) diff --git a/crates/bevy_render/src/renderer/render_resource/shared_buffers.rs b/crates/bevy_render/src/renderer/render_resource/shared_buffers.rs index 7e8b05edb4792..bf2e399e19938 100644 --- a/crates/bevy_render/src/renderer/render_resource/shared_buffers.rs +++ b/crates/bevy_render/src/renderer/render_resource/shared_buffers.rs @@ -6,8 +6,8 @@ use crate::{ use bevy_ecs::{Res, ResMut}; pub struct SharedBuffers { - staging_buffer: BufferId, - uniform_buffer: BufferId, + staging_buffer: Option, + uniform_buffer: Option, buffers_to_free: Vec, buffer_size: usize, initial_size: usize, @@ -18,8 +18,8 @@ pub struct SharedBuffers { impl SharedBuffers { pub fn new(initial_size: usize) -> Self { Self { - staging_buffer: BufferId::new(), // non-existent buffer - uniform_buffer: BufferId::new(), // non-existent buffer + staging_buffer: None, + uniform_buffer: None, buffer_size: 0, current_offset: 0, initial_size, @@ -33,7 +33,6 @@ impl SharedBuffers { render_resource_context: &dyn RenderResourceContext, required_space: usize, ) { - let first_resize = self.buffer_size == 0; while self.buffer_size < self.current_offset + required_space { self.buffer_size = if self.buffer_size == 0 { self.initial_size @@ -44,23 +43,25 @@ impl SharedBuffers { self.current_offset = 0; - // ignore the initial "dummy buffers" - if !first_resize { - render_resource_context.unmap_buffer(self.staging_buffer); - self.buffers_to_free.push(self.staging_buffer); - self.buffers_to_free.push(self.uniform_buffer); + if let Some(staging_buffer) = self.staging_buffer.take() { + render_resource_context.unmap_buffer(staging_buffer); + self.buffers_to_free.push(staging_buffer); } - self.staging_buffer = render_resource_context.create_buffer(BufferInfo { + if let Some(uniform_buffer) = self.uniform_buffer.take() { + self.buffers_to_free.push(uniform_buffer); + } + + self.staging_buffer = Some(render_resource_context.create_buffer(BufferInfo { size: self.buffer_size, buffer_usage: BufferUsage::MAP_WRITE | BufferUsage::COPY_SRC, mapped_at_creation: true, - }); - self.uniform_buffer = render_resource_context.create_buffer(BufferInfo { + })); + self.uniform_buffer = Some(render_resource_context.create_buffer(BufferInfo { size: self.buffer_size, buffer_usage: BufferUsage::COPY_DST | BufferUsage::UNIFORM, mapped_at_creation: false, - }); + })); } pub fn get_uniform_buffer( @@ -78,9 +79,10 @@ impl SharedBuffers { } let range = self.current_offset as u64..new_offset as u64; - + let staging_buffer = self.staging_buffer.unwrap(); + let uniform_buffer = self.uniform_buffer.unwrap(); render_resource_context.write_mapped_buffer( - self.staging_buffer, + staging_buffer, range.clone(), &mut |data, _renderer| { render_resource.write_buffer_bytes(data); @@ -88,16 +90,16 @@ impl SharedBuffers { ); self.command_queue.copy_buffer_to_buffer( - self.staging_buffer, + staging_buffer, self.current_offset as u64, - self.uniform_buffer, + uniform_buffer, self.current_offset as u64, aligned_size as u64, ); self.current_offset = new_offset; Some(RenderResourceBinding::Buffer { - buffer: self.uniform_buffer, + buffer: uniform_buffer, range, dynamic_index: None, }) @@ -111,11 +113,16 @@ impl SharedBuffers { for buffer in self.buffers_to_free.drain(..) { render_resource_context.remove_buffer(buffer) } - render_resource_context.map_buffer(self.staging_buffer); + + if let Some(staging_buffer) = self.staging_buffer { + render_resource_context.map_buffer(staging_buffer); + } } pub fn apply(&mut self, render_context: &mut dyn RenderContext) { - render_context.resources().unmap_buffer(self.staging_buffer); + if let Some(staging_buffer) = self.staging_buffer { + render_context.resources().unmap_buffer(staging_buffer); + } let mut command_queue = std::mem::take(&mut self.command_queue); command_queue.execute(render_context); }