Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bevy_render: delegate buffer aligning to RenderResourceContext #842

Merged
merged 1 commit into from
Nov 11, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 16 additions & 52 deletions crates/bevy_render/src/render_graph/nodes/render_resources_node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,6 @@ use bevy_utils::HashMap;
use renderer::{AssetRenderResourceBindings, BufferId, RenderResourceType, RenderResources};
use std::{hash::Hash, marker::PhantomData, ops::DerefMut};

pub const BIND_BUFFER_ALIGNMENT: usize = 256;

fn get_aligned_dynamic_uniform_size(data_size: usize) -> usize {
BIND_BUFFER_ALIGNMENT * ((data_size as f32 / BIND_BUFFER_ALIGNMENT as f32).ceil() as usize)
}

#[derive(Debug)]
struct QueuedBufferWrite {
buffer: BufferId,
Expand All @@ -42,13 +36,9 @@ struct BufferArray<I> {
}

impl<I: Hash + Eq> BufferArray<I> {
pub fn new(item_size: usize, min_capacity: usize, align: bool) -> Self {
pub fn new(item_size: usize, min_capacity: usize) -> Self {
BufferArray {
item_size: if align {
get_aligned_dynamic_uniform_size(item_size)
} else {
item_size
},
item_size,
len: 0,
buffer_capacity: 0,
min_capacity,
Expand Down Expand Up @@ -158,13 +148,18 @@ where
T: renderer::RenderResources,
{
/// Initialize this UniformBufferArrays using information from a RenderResources value.
fn initialize(&mut self, render_resources: &T) {
fn initialize(
&mut self,
render_resources: &T,
render_resource_context: &dyn RenderResourceContext,
) {
if self.buffer_arrays.len() != render_resources.render_resources_len() {
let mut buffer_arrays = Vec::with_capacity(render_resources.render_resources_len());
for render_resource in render_resources.iter() {
if let Some(RenderResourceType::Buffer) = render_resource.resource_type() {
let size = render_resource.buffer_byte_len().unwrap();
buffer_arrays.push(Some(BufferArray::new(size, 10, true)));
let aligned_size = render_resource_context.get_aligned_uniform_size(size, true);
buffer_arrays.push(Some(BufferArray::new(aligned_size, 10)));
} else {
buffer_arrays.push(None);
}
Expand Down Expand Up @@ -247,8 +242,10 @@ where
Some(RenderResourceType::Buffer) => {
let size = render_resource.buffer_byte_len().unwrap();
let render_resource_name = uniforms.get_render_resource_name(i).unwrap();
let aligned_size =
render_resource_context.get_aligned_uniform_size(size, false);
let buffer_array = self.buffer_arrays[i].as_mut().unwrap();
let range = 0..size as u64;
let range = 0..aligned_size as u64;
let (target_buffer, target_offset) = if dynamic_uniforms {
let binding = buffer_array.get_binding(id).unwrap();
let dynamic_index = if let RenderResourceBinding::Buffer {
Expand All @@ -270,7 +267,7 @@ where
size: current_size, ..
}) = render_resource_context.get_buffer_info(buffer_id)
{
if size == current_size {
if aligned_size == current_size {
matching_buffer = Some(buffer_id);
} else {
render_resource_context.remove_buffer(buffer_id);
Expand All @@ -291,7 +288,7 @@ where
}

let buffer = render_resource_context.create_buffer(BufferInfo {
size,
size: aligned_size,
buffer_usage: BufferUsage::COPY_DST | usage,
..Default::default()
});
Expand Down Expand Up @@ -432,7 +429,7 @@ fn render_resources_node_system<T: RenderResources>(
uniform_buffer_arrays.begin_update();
// initialize uniform buffer arrays using the first RenderResources
if let Some((_, first, _, _)) = query.iter_mut().next() {
uniform_buffer_arrays.initialize(first);
uniform_buffer_arrays.initialize(first, render_resource_context);
}

for entity in query.removed::<T>() {
Expand Down Expand Up @@ -482,23 +479,6 @@ fn render_resources_node_system<T: RenderResources>(
state
.uniform_buffer_arrays
.copy_staging_buffer_to_final_buffers(&mut state.command_queue, staging_buffer);
} else {
// TODO: can we just remove this?
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems it can be safely removed, tested a lot of bevy examples without it and everything seems to work.

let mut staging_buffer: [u8; 0] = [];
for (entity, uniforms, draw, mut render_pipelines) in query.iter_mut() {
if !draw.is_visible {
continue;
}

state.uniform_buffer_arrays.write_uniform_buffers(
entity,
&uniforms,
state.dynamic_uniforms,
render_resource_context,
&mut render_pipelines.bindings,
&mut staging_buffer,
);
}
}
}

Expand Down Expand Up @@ -579,7 +559,7 @@ fn asset_render_resources_node_system<T: RenderResources + Asset>(
// initialize uniform buffer arrays using the first RenderResources
if let Some(first_handle) = modified_assets.get(0) {
let asset = assets.get(*first_handle).expect(EXPECT_ASSET_MESSAGE);
uniform_buffer_arrays.initialize(asset);
uniform_buffer_arrays.initialize(asset, render_resource_context);
}

for asset_handle in modified_assets.iter() {
Expand Down Expand Up @@ -620,22 +600,6 @@ fn asset_render_resources_node_system<T: RenderResources + Asset>(
state
.uniform_buffer_arrays
.copy_staging_buffer_to_final_buffers(&mut state.command_queue, staging_buffer);
} else {
let mut staging_buffer: [u8; 0] = [];
for asset_handle in modified_assets.iter() {
let asset = assets.get(*asset_handle).expect(EXPECT_ASSET_MESSAGE);
let mut render_resource_bindings =
asset_render_resource_bindings.get_or_insert_mut(&Handle::<T>::weak(*asset_handle));
// TODO: only setup buffer if we haven't seen this handle before
state.uniform_buffer_arrays.write_uniform_buffers(
*asset_handle,
&asset,
state.dynamic_uniforms,
render_resource_context,
&mut render_resource_bindings,
&mut staging_buffer,
);
}
}

for (asset_handle, draw, mut render_pipelines) in query.iter_mut() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,6 @@ pub struct TextureCopyNode {
pub texture_event_reader: EventReader<AssetEvent<Texture>>,
}

pub const ALIGNMENT: usize = 256;
fn get_aligned(data_size: f32) -> usize {
ALIGNMENT * ((data_size / ALIGNMENT as f32).ceil() as usize)
}

impl Node for TextureCopyNode {
fn update(
&mut self,
Expand All @@ -34,7 +29,9 @@ impl Node for TextureCopyNode {
if let Some(texture) = textures.get(handle) {
let texture_descriptor: TextureDescriptor = texture.into();
let width = texture.size.x() as usize;
let aligned_width = get_aligned(texture.size.x());
let aligned_width = render_context
.resources()
.get_aligned_texture_size(texture.size.x() as usize);
let format_size = texture.format.pixel_size();
let mut aligned_data =
vec![0; format_size * aligned_width * texture.size.y() as usize];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -140,4 +140,12 @@ impl RenderResourceContext for HeadlessRenderResourceContext {
) -> bool {
false
}

fn get_aligned_uniform_size(&self, size: usize, _dynamic: bool) -> usize {
size
}

fn get_aligned_texture_size(&self, size: usize) -> usize {
size
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,12 @@ impl SharedBuffers {
buffer_usage: BufferUsage,
) -> Option<RenderResourceBinding> {
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,
size: aligned_size,
buffer_usage: BufferUsage::COPY_SRC | BufferUsage::MAP_WRITE,
mapped_at_creation: true,
});
Expand All @@ -49,7 +52,7 @@ impl SharedBuffers {
self.render_resource_context.unmap_buffer(staging_buffer);

let destination_buffer = self.render_resource_context.create_buffer(BufferInfo {
size,
size: aligned_size,
buffer_usage: BufferUsage::COPY_DST | buffer_usage,
..Default::default()
});
Expand All @@ -68,7 +71,7 @@ impl SharedBuffers {
buffers.push(destination_buffer);
Some(RenderResourceBinding::Buffer {
buffer: destination_buffer,
range: 0..size as u64,
range: 0..aligned_size as u64,
dynamic_index: None,
})
} else {
Expand Down
3 changes: 2 additions & 1 deletion crates/bevy_render/src/renderer/render_resource_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ pub trait RenderResourceContext: Downcast + Send + Sync + 'static {
fn remove_texture(&self, texture: TextureId);
fn remove_sampler(&self, sampler: SamplerId);
fn get_buffer_info(&self, buffer: BufferId) -> Option<BufferInfo>;

fn get_aligned_uniform_size(&self, size: usize, dynamic: bool) -> usize;
fn get_aligned_texture_size(&self, data_size: usize) -> usize;
fn set_asset_resource_untyped(
&self,
handle: HandleUntyped,
Expand Down
15 changes: 15 additions & 0 deletions crates/bevy_wgpu/src/renderer/wgpu_render_resource_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ pub struct WgpuRenderResourceContext {
pub resources: WgpuResources,
}

pub const BIND_BUFFER_ALIGNMENT: usize = 256;
pub const TEXTURE_ALIGNMENT: usize = 256;

impl WgpuRenderResourceContext {
pub fn new(device: Arc<wgpu::Device>) -> Self {
WgpuRenderResourceContext {
Expand Down Expand Up @@ -553,4 +556,16 @@ impl RenderResourceContext for WgpuRenderResourceContext {
let buffer = buffers.get(&id).unwrap();
buffer.unmap();
}

fn get_aligned_texture_size(&self, size: usize) -> usize {
(size + TEXTURE_ALIGNMENT - 1) & !(TEXTURE_ALIGNMENT - 1)
}

fn get_aligned_uniform_size(&self, size: usize, dynamic: bool) -> usize {
if dynamic {
(size + BIND_BUFFER_ALIGNMENT - 1) & !(BIND_BUFFER_ALIGNMENT - 1)
} else {
size
}
}
}