Skip to content

Commit

Permalink
allow buffers to be mapped on creation,
Browse files Browse the repository at this point in the history
add resource destruction callback
Fixes #592
  • Loading branch information
Wumpf committed Dec 18, 2022
1 parent a78b324 commit e7cbf61
Show file tree
Hide file tree
Showing 10 changed files with 89 additions and 27 deletions.
3 changes: 3 additions & 0 deletions crates/re_renderer/src/mesh.rs
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,7 @@ impl GpuMesh {
label: data.label.clone().push_str(" - vertices"),
size: vertex_buffer_combined_size,
usage: wgpu::BufferUsages::VERTEX | wgpu::BufferUsages::COPY_DST,
bypass_reuse_and_map_on_creation: false,
},
);
let mut staging_buffer = queue.write_buffer_with(
Expand All @@ -193,6 +194,7 @@ impl GpuMesh {
label: data.label.clone().push_str(" - indices"),
size: index_buffer_size,
usage: wgpu::BufferUsages::INDEX | wgpu::BufferUsages::COPY_DST,
bypass_reuse_and_map_on_creation: false,
},
);
let mut staging_buffer = queue.write_buffer_with(
Expand Down Expand Up @@ -220,6 +222,7 @@ impl GpuMesh {
label: data.label.clone().push_str(" - material uniforms"),
size: combined_buffers_size,
usage: wgpu::BufferUsages::COPY_DST | wgpu::BufferUsages::UNIFORM,
bypass_reuse_and_map_on_creation: false,
},
);

Expand Down
1 change: 1 addition & 0 deletions crates/re_renderer/src/renderer/lines.rs
Original file line number Diff line number Diff line change
Expand Up @@ -490,6 +490,7 @@ impl LineDrawData {
label: "lines batch uniform buffers".into(),
size: combined_buffers_size,
usage: wgpu::BufferUsages::UNIFORM | wgpu::BufferUsages::COPY_DST,
bypass_reuse_and_map_on_creation: false,
},
);

Expand Down
1 change: 1 addition & 0 deletions crates/re_renderer/src/renderer/mesh_renderer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@ impl MeshDrawData {
label: "MeshDrawData instance buffer".into(),
size: instance_buffer_size,
usage: wgpu::BufferUsages::VERTEX | wgpu::BufferUsages::COPY_DST,
bypass_reuse_and_map_on_creation: false,
},
);

Expand Down
1 change: 1 addition & 0 deletions crates/re_renderer/src/renderer/point_cloud.rs
Original file line number Diff line number Diff line change
Expand Up @@ -329,6 +329,7 @@ impl PointCloudDrawData {
label: "point batch uniform buffers".into(),
size: combined_buffers_size,
usage: wgpu::BufferUsages::UNIFORM | wgpu::BufferUsages::COPY_DST,
bypass_reuse_and_map_on_creation: false,
},
);

Expand Down
1 change: 1 addition & 0 deletions crates/re_renderer/src/renderer/rectangles.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,7 @@ impl RectangleDrawData {
label: "rectangle uniform buffers".into(),
size: combined_buffers_size,
usage: wgpu::BufferUsages::COPY_DST | wgpu::BufferUsages::UNIFORM,
bypass_reuse_and_map_on_creation: false,
},
);

Expand Down
1 change: 1 addition & 0 deletions crates/re_renderer/src/view_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,7 @@ impl ViewBuilder {
label: format!("{:?} - frame uniform buffer", config.name).into(),
size: std::mem::size_of::<FrameUniformBuffer>() as _,
usage: wgpu::BufferUsages::UNIFORM | wgpu::BufferUsages::COPY_DST,
bypass_reuse_and_map_on_creation: false,
},
);

Expand Down
6 changes: 5 additions & 1 deletion crates/re_renderer/src/wgpu_resources/bind_group_pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,10 @@ impl SizedResourceDesc for BindGroupDesc {
// We could guess something like a pointer per descriptor, but let's not pretend we know!
0
}

fn reusable(&self) -> bool {
true
}
}

/// Resource pool for bind groups.
Expand Down Expand Up @@ -185,7 +189,7 @@ impl GpuBindGroupPool {
_buffers: &mut GpuBufferPool,
_samplers: &mut GpuSamplerPool,
) {
self.pool.frame_maintenance(frame_index);
self.pool.frame_maintenance(frame_index, |_res| {});
// TODO(andreas): Update usage counter on dependent resources.
}

Expand Down
26 changes: 23 additions & 3 deletions crates/re_renderer/src/wgpu_resources/buffer_pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ slotmap::new_key_type! { pub struct GpuBufferHandle; }
/// Once all strong handles are dropped, the bind group will be marked for reclamation in the following frame.
pub type GpuBufferHandleStrong = std::sync::Arc<GpuBufferHandle>;

/// Buffer creation descriptor.
#[derive(Clone, Hash, PartialEq, Eq, Debug)]
pub struct BufferDesc {
/// Debug label of a buffer. This will show up in graphics debuggers for easy identification.
Expand All @@ -24,12 +25,29 @@ pub struct BufferDesc {
/// Usages of a buffer. If the buffer is used in any way that isn't specified here, the operation
/// will panic.
pub usage: wgpu::BufferUsages,

/// Allows a buffer to be mapped immediately after they are allocated.
///
/// ⚠️ Forces creation of a new buffer that cannot be reclaimed later ⚠️
/// This is because all other mapping operations are asynchronous. We could still allow
/// re-use by implementing a mechanism similar to the re-use strategy [`crate::StagingBelt`] employs,
/// but as of writing this is the only user needing in need of creation mapped buffer in the first place.
///
/// It does not have to be [`wgpu::BufferUsages::MAP_READ`] or [`wgpu::BufferUsages::MAP_WRITE`],
/// all buffers are allowed to be mapped at creation.
///
/// If this is `true`, [`size`](#structfield.size) must be a multiple of [`wgpu::COPY_BUFFER_ALIGNMENT`].
pub bypass_reuse_and_map_on_creation: bool,
}

impl SizedResourceDesc for BufferDesc {
fn resource_size_in_bytes(&self) -> u64 {
self.size
}

fn reusable(&self) -> bool {
!self.bypass_reuse_and_map_on_creation
}
}

#[derive(Default)]
Expand All @@ -39,7 +57,8 @@ pub struct GpuBufferPool {

impl GpuBufferPool {
/// Returns a ref counted handle to a currently unused buffer.
/// Once ownership to the handle is given up, the buffer may be reclaimed in future frames.
/// Once ownership to the handle is given up, the buffer may be reclaimed in future frames,
/// unless re-use was bypassed by [`BufferDesc::bypass_reuse_and_map_on_creation`]
///
/// For more efficient allocation (faster, less fragmentation) you should sub-allocate buffers whenever possible
/// either manually or using a higher level allocator.
Expand All @@ -49,14 +68,15 @@ impl GpuBufferPool {
label: desc.label.get(),
size: desc.size,
usage: desc.usage,
mapped_at_creation: false,
mapped_at_creation: desc.bypass_reuse_and_map_on_creation,
})
})
}

/// Called by `RenderContext` every frame. Updates statistics and may free unused buffers.
pub fn frame_maintenance(&mut self, frame_index: u64) {
self.pool.frame_maintenance(frame_index);
self.pool
.frame_maintenance(frame_index, |res| res.destroy());
}

/// Takes strong buffer handle to ensure the user is still holding on to the buffer.
Expand Down
69 changes: 47 additions & 22 deletions crates/re_renderer/src/wgpu_resources/dynamic_resource_pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ use super::resource::PoolError;

pub trait SizedResourceDesc {
fn resource_size_in_bytes(&self) -> u64;

/// Returns true if the resource can be re-used in subsequent alloc calls after it was discarded.
fn reusable(&self) -> bool;
}

/// Generic resource pool for all resources that have varying contents beyond their description.
Expand Down Expand Up @@ -57,8 +60,8 @@ where
Desc: Clone + Eq + Hash + Debug + SizedResourceDesc,
{
pub fn alloc<F: FnOnce(&Desc) -> Res>(&mut self, desc: &Desc, creation_func: F) -> Arc<Handle> {
// First check if we can reclaim a resource we have around from a previous frame.
let handle =
let handle = if desc.reusable() {
// First check if we can reclaim a resource we have around from a previous frame.
if let Entry::Occupied(mut entry) = self.last_frame_deallocated.entry(desc.clone()) {
re_log::trace!(?desc, "Reclaimed previously discarded resource",);

Expand All @@ -69,15 +72,26 @@ where
handle
// Otherwise create a new resource
} else {
let resource = creation_func(desc);
self.total_resource_size_in_bytes += desc.resource_size_in_bytes();
Arc::new(self.resources.insert((desc.clone(), resource)))
};
self.create_resource(creation_func, desc)
}
} else {
self.create_resource(creation_func, desc)
};

self.alive_handles.insert(*handle, Arc::clone(&handle));
handle
}

fn create_resource<F: FnOnce(&Desc) -> Res>(
&mut self,
creation_func: F,
desc: &Desc,
) -> Arc<Handle> {
let resource = creation_func(desc);
self.total_resource_size_in_bytes += desc.resource_size_in_bytes();
Arc::new(self.resources.insert((desc.clone(), resource)))
}

pub fn get_resource(&self, handle: Handle) -> Result<&Res, PoolError> {
self.resources
.get(handle)
Expand All @@ -91,7 +105,7 @@ where
})
}

pub fn frame_maintenance(&mut self, frame_index: u64) {
pub fn frame_maintenance(&mut self, frame_index: u64, on_destroy_resource: impl Fn(&Res)) {
self.current_frame_index = frame_index;

// Throw out any resources that we haven't reclaimed last frame.
Expand All @@ -102,7 +116,9 @@ where
"Drained dangling resources from last frame",
);
for handle in handles {
self.resources.remove(*handle);
if let Some((_, res)) = self.resources.remove(*handle) {
on_destroy_resource(&res);
}
self.total_resource_size_in_bytes -= desc.resource_size_in_bytes();
}
}
Expand All @@ -114,14 +130,19 @@ where
// get temporarily get back down to 1 without dropping the last user available copy of the Arc<Handle>.
self.alive_handles.retain(|handle, strong_handle| {
if Arc::strong_count(strong_handle) == 1 {
let desc = &self.resources[handle].0;
match self.last_frame_deallocated.entry(desc.clone()) {
Entry::Occupied(mut e) => {
e.get_mut().push(Arc::clone(strong_handle));
}
Entry::Vacant(e) => {
e.insert(smallvec![Arc::clone(strong_handle)]);
let (desc, res) = &self.resources[handle];

if desc.reusable() {
match self.last_frame_deallocated.entry(desc.clone()) {
Entry::Occupied(mut e) => {
e.get_mut().push(Arc::clone(strong_handle));
}
Entry::Vacant(e) => {
e.insert(smallvec![Arc::clone(strong_handle)]);
}
}
} else {
on_destroy_resource(res);
}
false
} else {
Expand Down Expand Up @@ -169,6 +190,10 @@ mod tests {
fn resource_size_in_bytes(&self) -> u64 {
1
}

fn reusable(&self) -> bool {
true
}
}

#[derive(Debug)]
Expand Down Expand Up @@ -199,7 +224,7 @@ mod tests {
// Still, no resources were dropped.
{
let drop_counter_before = drop_counter.load(Ordering::Acquire);
pool.frame_maintenance(1);
pool.frame_maintenance(1, |_| {});

assert_eq!(drop_counter_before, drop_counter.load(Ordering::Acquire),);
}
Expand All @@ -212,8 +237,8 @@ mod tests {
// Doing frame maintenance twice will drop all resources
{
let drop_counter_before = drop_counter.load(Ordering::Acquire);
pool.frame_maintenance(2);
pool.frame_maintenance(3);
pool.frame_maintenance(2, |_| {});
pool.frame_maintenance(3, |_| {});
let drop_counter_now = drop_counter.load(Ordering::Acquire);
assert_eq!(
drop_counter_before + initial_resource_descs.len() * 2,
Expand All @@ -236,9 +261,9 @@ mod tests {
assert_ne!(handle0, handle1);
drop(handle1);

pool.frame_maintenance(4);
pool.frame_maintenance(4, |_| {});
assert_eq!(drop_counter_before, drop_counter.load(Ordering::Acquire),);
pool.frame_maintenance(5);
pool.frame_maintenance(5, |_| {});
assert_eq!(
drop_counter_before + 1,
drop_counter.load(Ordering::Acquire),
Expand Down Expand Up @@ -314,8 +339,8 @@ mod tests {
// Query with invalid handle
let inner_handle = *handle;
drop(handle);
pool.frame_maintenance(0);
pool.frame_maintenance(1);
pool.frame_maintenance(0, |_| {});
pool.frame_maintenance(1, |_| {});
assert!(matches!(
pool.get_resource(inner_handle),
Err(PoolError::ResourceNotAvailable)
Expand Down
7 changes: 6 additions & 1 deletion crates/re_renderer/src/wgpu_resources/texture_pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,10 @@ impl SizedResourceDesc for TextureDesc {

size_in_bytes
}

fn reusable(&self) -> bool {
true
}
}

impl TextureDesc {
Expand Down Expand Up @@ -104,7 +108,8 @@ impl GpuTexturePool {

/// Called by `RenderContext` every frame. Updates statistics and may free unused textures.
pub fn frame_maintenance(&mut self, frame_index: u64) {
self.pool.frame_maintenance(frame_index);
self.pool
.frame_maintenance(frame_index, |res| res.texture.destroy());
}

/// Takes strong texture handle to ensure the user is still holding on to the texture.
Expand Down

0 comments on commit e7cbf61

Please sign in to comment.