Skip to content

Commit

Permalink
[pr feedback] minor cleanup in zero_init_texture_after_discard, use…
Browse files Browse the repository at this point in the history 
… hygene
  • Loading branch information
@Wumpf
Wumpf committed Oct 25, 2021
1 parent 87dc9b0 commit 3047510
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 43 deletions.
24 changes: 11 additions & 13 deletions wgpu-core/src/command/memory_init.rs
View file
Original file line number Diff line number Diff line change
@@ -1,19 +1,17 @@
use std::collections::hash_map::Entry;
use std::ops::Range;
use std::vec::Drain;
use std::{collections::hash_map::Entry, ops::Range, vec::Drain};

use hal::CommandEncoder;

use crate::command::collect_zero_buffer_copies_for_clear_texture;
use crate::device::Device;
use crate::hub::Storage;
use crate::id;
use crate::id::TextureId;
use crate::init_tracker::*;
use crate::resource::{Buffer, Texture};
use crate::track::TrackerSet;
use crate::track::{ResourceTracker, TextureSelector, TextureState};
use crate::FastHashMap;
use crate::{
command::collect_zero_buffer_copies_for_clear_texture,
device::Device,
hub::Storage,
id::{self, TextureId},
init_tracker::*,
resource::{Buffer, Texture},
track::{ResourceTracker, TextureSelector, TextureState, TrackerSet},
FastHashMap,
};

use super::{BakedCommands, DestroyedBufferError, DestroyedTextureError};

Expand Down
54 changes: 24 additions & 30 deletions wgpu/tests/zero_init_texture_after_discard.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ fn discarding_color_target_resets_texture_init_state_check_visible_on_copy_after
let mut encoder = ctx
.device
.create_command_encoder(&wgpu::CommandEncoderDescriptor::default());
drop(encoder.begin_render_pass(&wgpu::RenderPassDescriptor {
encoder.begin_render_pass(&wgpu::RenderPassDescriptor {
label: Some("Color Discard"),
color_attachments: &[wgpu::RenderPassColorAttachment {
view: &texture.create_view(&wgpu::TextureViewDescriptor::default()),
Expand All @@ -23,7 +23,7 @@ fn discarding_color_target_resets_texture_init_state_check_visible_on_copy_after
},
}],
depth_stencil_attachment: None,
}));
});
ctx.queue.submit([encoder.finish()]);
}
{
Expand All @@ -47,7 +47,7 @@ fn discarding_color_target_resets_texture_init_state_check_visible_on_copy_in_sa
let mut encoder = ctx
.device
.create_command_encoder(&wgpu::CommandEncoderDescriptor::default());
drop(encoder.begin_render_pass(&wgpu::RenderPassDescriptor {
encoder.begin_render_pass(&wgpu::RenderPassDescriptor {
label: Some("Color Discard"),
color_attachments: &[wgpu::RenderPassColorAttachment {
view: &texture.create_view(&wgpu::TextureViewDescriptor::default()),
Expand All @@ -58,7 +58,7 @@ fn discarding_color_target_resets_texture_init_state_check_visible_on_copy_in_sa
},
}],
depth_stencil_attachment: None,
}));
});
copy_texture_to_buffer(&mut encoder, &texture, &readback_buffer);
ctx.queue.submit([encoder.finish()]);
}
Expand All @@ -79,7 +79,7 @@ fn discarding_depth_target_resets_texture_init_state_check_visible_on_copy_in_sa
let mut encoder = ctx
.device
.create_command_encoder(&wgpu::CommandEncoderDescriptor::default());
drop(encoder.begin_render_pass(&wgpu::RenderPassDescriptor {
encoder.begin_render_pass(&wgpu::RenderPassDescriptor {
label: Some("Depth Discard"),
color_attachments: &[],
depth_stencil_attachment: Some(wgpu::RenderPassDepthStencilAttachment {
Expand All @@ -93,7 +93,7 @@ fn discarding_depth_target_resets_texture_init_state_check_visible_on_copy_in_sa
store: false, // discard!
}),
}),
}));
});
copy_texture_to_buffer(&mut encoder, &texture, &readback_buffer);
ctx.queue.submit([encoder.finish()]);
}
Expand All @@ -109,12 +109,12 @@ fn discarding_either_depth_or_stencil_aspect() {
&ctx,
wgpu::TextureFormat::Depth24PlusStencil8,
);
// TODO: How do we test this other than "doesn't crash"? we can't copy the texture to/from buffers!
// TODO: How do we test this other than "doesn't crash"? We can't copy the texture to/from buffers, so we would need to do a copy in a shader
{
let mut encoder = ctx
.device
.create_command_encoder(&wgpu::CommandEncoderDescriptor::default());
drop(encoder.begin_render_pass(&wgpu::RenderPassDescriptor {
encoder.begin_render_pass(&wgpu::RenderPassDescriptor {
label: Some("Depth Discard, Stencil Load"),
color_attachments: &[],
depth_stencil_attachment: Some(wgpu::RenderPassDepthStencilAttachment {
Expand All @@ -128,14 +128,14 @@ fn discarding_either_depth_or_stencil_aspect() {
store: true,
}),
}),
}));
});
ctx.queue.submit([encoder.finish()]);
}
{
let mut encoder = ctx
.device
.create_command_encoder(&wgpu::CommandEncoderDescriptor::default());
drop(encoder.begin_render_pass(&wgpu::RenderPassDescriptor {
encoder.begin_render_pass(&wgpu::RenderPassDescriptor {
label: Some("Depth Load, Stencil Discard"),
color_attachments: &[],
depth_stencil_attachment: Some(wgpu::RenderPassDepthStencilAttachment {
Expand All @@ -149,7 +149,7 @@ fn discarding_either_depth_or_stencil_aspect() {
store: false, // discard!
}),
}),
}));
});
ctx.queue.submit([encoder.finish()]);
}
});
Expand All @@ -174,8 +174,7 @@ fn create_white_texture_and_readback_buffer(
let format_desc = format.describe();

// Size for tests is chosen so that we don't need to care about buffer alignments.
assert_eq!(format_desc.block_dimensions.0, 1);
assert_eq!(format_desc.block_dimensions.1, 1);
assert_eq!(format_desc.block_dimensions, (1, 1));
assert_eq!(format_desc.block_size as u32, BYTES_PER_PIXEL);
assert_eq!(
(TEXTURE_SIZE.width * format_desc.block_size as u32) % wgpu::COPY_BYTES_PER_ROW_ALIGNMENT,
Expand Down Expand Up @@ -209,14 +208,11 @@ fn create_white_texture_and_readback_buffer(
// This behavior is curious, but does not violate any spec - it is wgpu's job to pass this test no matter what a render target discard does.

// ... but that said, for depth/stencil textures we need to do a clear.
if format == wgpu::TextureFormat::Depth32Float
|| format == wgpu::TextureFormat::Depth24PlusStencil8
|| format == wgpu::TextureFormat::Depth24Plus
{
if format_desc.sample_type == wgpu::TextureSampleType::Depth {
let mut encoder = ctx
.device
.create_command_encoder(&wgpu::CommandEncoderDescriptor::default());
drop(encoder.begin_render_pass(&wgpu::RenderPassDescriptor {
encoder.begin_render_pass(&wgpu::RenderPassDescriptor {
label: Some("Depth/Stencil setup"),
color_attachments: &[],
depth_stencil_attachment: Some(wgpu::RenderPassDepthStencilAttachment {
Expand All @@ -230,7 +226,7 @@ fn create_white_texture_and_readback_buffer(
store: true,
}),
}),
}));
});
ctx.queue.submit([encoder.finish()]);
} else {
let data = vec![255; buffer_size as usize];
Expand Down Expand Up @@ -267,7 +263,7 @@ fn copy_texture_to_buffer(
wgpu::ImageCopyTexture {
texture,
mip_level: 0,
origin: wgpu::Origin3d { x: 0, y: 0, z: 0 },
origin: wgpu::Origin3d::ZERO,
aspect: wgpu::TextureAspect::All,
},
wgpu::ImageCopyBuffer {
Expand All @@ -279,16 +275,14 @@ fn copy_texture_to_buffer(
}

fn assert_buffer_is_zero(readback_buffer: &wgpu::Buffer, device: &wgpu::Device) {
{
let buffer_slice = readback_buffer.slice(..);
let _ = buffer_slice.map_async(wgpu::MapMode::Read);
device.poll(wgpu::Maintain::Wait);
let buffer_view = buffer_slice.get_mapped_range();
let buffer_slice = readback_buffer.slice(..);
let _ = buffer_slice.map_async(wgpu::MapMode::Read);
device.poll(wgpu::Maintain::Wait);
let buffer_view = buffer_slice.get_mapped_range();

assert!(
buffer_view.iter().all(|b| *b == 0),
"texture was not fully cleared"
);
}
assert!(
buffer_view.iter().all(|b| *b == 0),
"texture was not fully cleared"
);
readback_buffer.unmap();
}

0 comments on commit 3047510

Please sign in to comment.