Skip to content

Commit

Permalink
Fixing texture leaking due to clear view
Browse files Browse the repository at this point in the history
  • Loading branch information
gents83 committed Sep 19, 2023
1 parent 88dc35c commit 8ca3cb0
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 23 deletions.
34 changes: 18 additions & 16 deletions tests/tests/mem_leaks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,8 @@ fn draw_test_with_reports(
let report = global_report.hub_report();
assert_eq!(report.shader_modules.num_allocated, 1);
assert_eq!(report.shader_modules.num_kept_from_user, 0);
assert_eq!(report.textures.num_allocated, 0);
assert_eq!(report.texture_views.num_allocated, 0);

let texture = ctx.device.create_texture_with_data(
&ctx.queue,
Expand Down Expand Up @@ -219,22 +221,22 @@ fn draw_test_with_reports(

let global_report = ctx.instance.generate_report();
let report = global_report.hub_report();
assert_eq!(report.command_buffers.num_allocated, 1);
assert_eq!(report.command_buffers.num_kept_from_user, 1);
assert_eq!(report.render_pipelines.num_allocated, 1);
assert_eq!(report.render_pipelines.num_kept_from_user, 0);
assert_eq!(report.pipeline_layouts.num_allocated, 1);
assert_eq!(report.pipeline_layouts.num_kept_from_user, 0);
assert_eq!(report.bind_group_layouts.num_allocated, 1);
assert_eq!(report.bind_group_layouts.num_kept_from_user, 0);
assert_eq!(report.bind_groups.num_allocated, 1);
assert_eq!(report.bind_groups.num_kept_from_user, 0);
assert_eq!(report.texture_views.num_allocated, 2);
assert_eq!(report.buffers.num_kept_from_user, 0);
assert_eq!(report.texture_views.num_kept_from_user, 0);
assert_eq!(report.textures.num_kept_from_user, 0);
assert_eq!(report.command_buffers.num_allocated, 1);
assert_eq!(report.render_pipelines.num_allocated, 1);
assert_eq!(report.pipeline_layouts.num_allocated, 1);
assert_eq!(report.bind_group_layouts.num_allocated, 1);
assert_eq!(report.bind_groups.num_allocated, 1);
assert_eq!(report.buffers.num_allocated, 1);
assert_eq!(report.buffers.num_kept_from_user, 0);
assert_eq!(report.texture_views.num_allocated, 2);
assert_eq!(report.textures.num_allocated, 1);
assert_eq!(report.textures.num_kept_from_user, 0);

ctx.queue.submit(Some(encoder.finish()));

Expand All @@ -251,9 +253,9 @@ fn draw_test_with_reports(
assert_eq!(report.bind_groups.num_allocated, 0);
assert_eq!(report.bind_group_layouts.num_allocated, 0);
assert_eq!(report.pipeline_layouts.num_allocated, 0);
//surface is still there
assert_eq!(report.texture_views.num_allocated, 1);
assert_eq!(report.textures.num_allocated, 1);
assert_eq!(report.texture_views.num_allocated, 0);
assert_eq!(report.textures.num_allocated, 0);
assert_eq!(report.buffers.num_allocated, 0);

drop(ctx.queue);
drop(ctx.device);
Expand All @@ -263,13 +265,13 @@ fn draw_test_with_reports(
let report = global_report.hub_report();

assert_eq!(report.queues.num_kept_from_user, 0);
assert_eq!(report.queues.num_allocated, 0);
//Still one texture alive because surface is not dropped till the end
assert_eq!(report.textures.num_allocated, 1);
//that is keeping still the device alive
assert_eq!(report.devices.num_allocated, 1);
assert_eq!(report.textures.num_kept_from_user, 0);
assert_eq!(report.devices.num_kept_from_user, 0);
assert_eq!(report.queues.num_allocated, 0);
assert_eq!(report.buffers.num_allocated, 0);
assert_eq!(report.textures.num_allocated, 0);
assert_eq!(report.texture_views.num_allocated, 0);
assert_eq!(report.devices.num_allocated, 0);
}

#[test]
Expand Down
4 changes: 3 additions & 1 deletion wgpu-core/src/device/global.rs
Original file line number Diff line number Diff line change
Expand Up @@ -735,7 +735,9 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
resource::TextureClearMode::None,
) {
resource::TextureClearMode::BufferCopy => SmallVec::new(),
resource::TextureClearMode::RenderPass { clear_views, .. } => clear_views,
resource::TextureClearMode::RenderPass {
mut clear_views, ..
} => clear_views.drain(..).collect(),
resource::TextureClearMode::Surface { mut clear_view } => {
if let Some(view) = clear_view.take() {
unsafe {
Expand Down
16 changes: 13 additions & 3 deletions wgpu-core/src/device/life.rs
Original file line number Diff line number Diff line change
Expand Up @@ -328,6 +328,15 @@ impl<A: HalApi> LifetimeTracker<A> {

pub fn cleanup(&mut self) {
profiling::scope!("LifetimeTracker::cleanup");
self.free_resources.textures.iter().for_each(|(_, t)| {
if let &mut resource::TextureClearMode::RenderPass {
ref mut clear_views,
..
} = &mut *t.clear_mode.write()
{
clear_views.clear();
}
});
self.free_resources.clear();
}

Expand Down Expand Up @@ -547,9 +556,10 @@ impl<A: HalApi> LifetimeTracker<A> {
.find(|a| a.index == submit_index)
.map_or(&mut self.free_resources, |a| &mut a.last_resources);

if let &resource::TextureClearMode::RenderPass {
ref clear_views, ..
} = &*texture.clear_mode.read()
if let &mut resource::TextureClearMode::RenderPass {
ref mut clear_views,
..
} = &mut *texture.clear_mode.write()
{
clear_views.into_iter().for_each(|v| {
non_referenced_resources
Expand Down
8 changes: 5 additions & 3 deletions wgpu-core/src/device/queue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1059,8 +1059,8 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
dst.info
.use_at(device.active_submission_index.load(Ordering::Relaxed) + 1);

let dst_raw = dst
.inner
let dst_inner = dst.inner();
let dst_raw = dst_inner
.as_ref()
.unwrap()
.as_raw()
Expand All @@ -1083,7 +1083,9 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
.textures
.set_single(&dst, selector, hal::TextureUses::COPY_DST)
.ok_or(TransferError::InvalidTexture(destination.texture))?;
encoder.transition_textures(transitions.map(|pending| pending.into_hal(&dst)));
encoder.transition_textures(
transitions.map(|pending| pending.into_hal(dst_inner.as_ref().unwrap())),
);
encoder.copy_external_image_to_texture(
source,
dst_raw,
Expand Down

0 comments on commit 8ca3cb0

Please sign in to comment.