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

Reintroduce clear_texture Metal/Vulkan/DX12 #1905

Merged
merged 13 commits into from
Sep 7, 2021
Merged
4 changes: 2 additions & 2 deletions player/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,11 +80,11 @@ impl GlobalPlay for wgc::hub::Global<IdentityPassThroughFactory> {
trace::Command::ClearBuffer { dst, offset, size } => self
.command_encoder_clear_buffer::<A>(encoder, dst, offset, size)
.unwrap(),
trace::Command::ClearImage {
trace::Command::ClearTexture {
dst,
subresource_range,
} => self
.command_encoder_clear_image::<A>(encoder, dst, &subresource_range)
.command_encoder_clear_texture::<A>(encoder, dst, &subresource_range)
.unwrap(),
trace::Command::WriteTimestamp {
query_set_id,
Expand Down
2 changes: 1 addition & 1 deletion player/tests/data/all.ron
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
tests: [
"bind-group.ron",
"buffer-copy.ron",
"clear-buffer-image.ron",
"clear-buffer-texture.ron",
"buffer-zero-init.ron",
"pipeline-statistics-query.ron",
"quad.ron",
Expand Down
36 changes: 0 additions & 36 deletions player/tests/data/buffer-clear.ron

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,17 @@
name: "Quad",
buffer: (index: 0, epoch: 1),
offset: 0,
data: File("clear-image.bin", 16384),
data: File("clear-texture.bin", 16384),
),
(
name: "buffer clear",
buffer: (index: 1, epoch: 1),
offset: 0,
data: Raw([
0x00, 0x00, 0x80, 0xBF,
0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x80, 0x3F,
]),
)
],
Expand All @@ -35,6 +35,25 @@
bits: 27,
),
)),
// First fill the texture to ensure it wasn't just zero initialized or "happened" to be zero.
WriteTexture(
to: (
texture: Id(0, 1, Empty),
mip_level: 0,
array_layer: 0,
),
data: "quad.bin",
layout: (
offset: 0,
bytes_per_row: Some(256),
rows_per_image: None,
),
size: (
width: 64,
height: 64,
depth_or_array_layers: 1,
),
),
CreateBuffer(
Id(0, 1, Empty),
(
Expand All @@ -46,6 +65,7 @@
mapped_at_creation: false,
),
),

CreateBuffer(
Id(1, 1, Empty),
(
Expand All @@ -57,8 +77,18 @@
mapped_at_creation: false,
),
),
// Make sure there is something in the buffer, otherwise it might be just zero init!
WriteBuffer(
id: Id(1, 1, Empty),
data: "data1.bin",
range: (
start: 0,
end: 16,
),
queued: true,
),
Submit(1, [
ClearImage(
ClearTexture(
dst: Id(0, 1, Empty),
subresource_range: ImageSubresourceRange(
aspect: All,
Expand Down Expand Up @@ -88,6 +118,7 @@
depth_or_array_layers: 1,
),
),
// Partial clear to proove
ClearBuffer(
dst: Id(1, 1, Empty),
offset: 4,
Expand Down
File renamed without changes.
49 changes: 23 additions & 26 deletions wgpu-core/src/command/clear.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,10 @@ pub enum ClearError {
texture_format: wgt::TextureFormat,
subresource_range_aspects: TextureAspect,
},
#[error("Depth/Stencil formats are not supported for clearing")]
DepthStencilFormatNotSupported,
#[error("Multisampled textures are not supported for clearing")]
MultisampledTextureUnsupported,
#[error("image subresource level range is outside of the texture's level range. texture range is {texture_level_range:?}, \
whereas subesource range specified start {subresource_base_mip_level} and count {subresource_mip_level_count:?}")]
InvalidTextureLevelRange {
Expand All @@ -68,7 +72,7 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
offset: BufferAddress,
size: Option<BufferSize>,
) -> Result<(), ClearError> {
profiling::scope!("CommandEncoder::fill_buffer");
profiling::scope!("CommandEncoder::clear_buffer");

let hub = A::hub(self);
let mut token = Token::root();
Expand All @@ -82,7 +86,7 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
list.push(TraceCommand::ClearBuffer { dst, offset, size });
}

if !cmd_buf.support_fill_buffer_texture {
if !cmd_buf.support_clear_buffer_texture {
return Err(ClearError::MissingClearCommandsFeature);
}

Expand Down Expand Up @@ -122,7 +126,7 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
None => dst_buffer.size,
};
if offset == end {
log::trace!("Ignoring fill_buffer of size 0");
log::trace!("Ignoring clear_buffer of size 0");
return Ok(());
}

Expand All @@ -139,18 +143,18 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
let cmd_buf_raw = cmd_buf.encoder.open();
unsafe {
cmd_buf_raw.transition_buffers(dst_barrier);
cmd_buf_raw.fill_buffer(dst_raw, offset..end, 0);
cmd_buf_raw.clear_buffer(dst_raw, offset..end);
}
Ok(())
}

pub fn command_encoder_clear_image<A: HalApi>(
pub fn command_encoder_clear_texture<A: HalApi>(
&self,
command_encoder_id: CommandEncoderId,
dst: TextureId,
subresource_range: &ImageSubresourceRange,
) -> Result<(), ClearError> {
profiling::scope!("CommandEncoder::clear_image");
profiling::scope!("CommandEncoder::clear_texture");

let hub = A::hub(self);
let mut token = Token::root();
Expand All @@ -162,13 +166,13 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {

#[cfg(feature = "trace")]
if let Some(ref mut list) = cmd_buf.commands {
list.push(TraceCommand::ClearImage {
list.push(TraceCommand::ClearTexture {
dst,
subresource_range: subresource_range.clone(),
});
}

if !cmd_buf.support_fill_buffer_texture {
if !cmd_buf.support_clear_buffer_texture {
return Err(ClearError::MissingClearCommandsFeature);
}

Expand All @@ -185,6 +189,15 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
subresource_range_aspects: subresource_range.aspect,
});
};

// Check if texture is supported for clearing
if dst_texture.desc.format.describe().sample_type == wgt::TextureSampleType::Depth {
return Err(ClearError::DepthStencilFormatNotSupported);
}
if dst_texture.desc.sample_count > 1 {
return Err(ClearError::MultisampledTextureUnsupported);
}

// Check if subresource level range is valid
let subresource_level_end = match subresource_range.mip_level_count {
Some(count) => subresource_range.base_mip_level + count.get(),
Expand Down Expand Up @@ -228,7 +241,7 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
hal::TextureUses::COPY_DST,
)
.map_err(ClearError::InvalidTexture)?;
let _dst_raw = dst_texture
let dst_raw = dst_texture
.inner
.as_raw()
.ok_or(ClearError::InvalidTexture(dst))?;
Expand All @@ -241,23 +254,7 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
let cmd_buf_raw = cmd_buf.encoder.open();
unsafe {
cmd_buf_raw.transition_textures(dst_barrier);
/*TODO: image clears
cmd_buf_raw.clear_image(
dst_raw,
hal::image::Layout::TransferDstOptimal,
hal::command::ClearValue {
color: hal::command::ClearColor {
float32: conv::map_color_f32(&wgt::Color::TRANSPARENT),
},
},
std::iter::once(hal::image::SubresourceRange {
aspects,
level_start: subresource_range.base_mip_level as u8,
level_count: subresource_range.mip_level_count.map(|c| c.get() as u8),
layer_start: subresource_range.base_array_layer as u16,
layer_count: subresource_range.array_layer_count.map(|c| c.get() as u16),
}),
);*/
cmd_buf_raw.clear_texture(dst_raw, subresource_range);
}
Ok(())
}
Expand Down
6 changes: 3 additions & 3 deletions wgpu-core/src/command/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ impl<A: hal::Api> BakedCommands<A> {
assert!(range.end % 4 == 0, "Buffer {:?} has an uninitialized range with an end not aligned to 4 (end was {})", raw_buf, range.end);

unsafe {
self.encoder.fill_buffer(raw_buf, range.clone(), 0);
self.encoder.clear_buffer(raw_buf, range.clone());
}
}
}
Expand All @@ -160,7 +160,7 @@ pub struct CommandBuffer<A: hal::Api> {
pub(crate) trackers: TrackerSet,
buffer_memory_init_actions: Vec<BufferInitTrackerAction>,
limits: wgt::Limits,
support_fill_buffer_texture: bool,
support_clear_buffer_texture: bool,
#[cfg(feature = "trace")]
pub(crate) commands: Option<Vec<crate::device::trace::Command>>,
}
Expand All @@ -187,7 +187,7 @@ impl<A: HalApi> CommandBuffer<A> {
trackers: TrackerSet::new(A::VARIANT),
buffer_memory_init_actions: Default::default(),
limits,
support_fill_buffer_texture: features.contains(wgt::Features::CLEAR_COMMANDS),
support_clear_buffer_texture: features.contains(wgt::Features::CLEAR_COMMANDS),
#[cfg(feature = "trace")]
commands: if enable_tracing {
Some(Vec::new())
Expand Down
4 changes: 2 additions & 2 deletions wgpu-core/src/device/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ fn map_buffer<A: hal::Api>(

// Zero out uninitialized parts of the mapping. (Spec dictates all resources behave as if they were initialized with zero)
//
// If this is a read mapping, ideally we would use a `fill_buffer` command before reading the data from GPU (i.e. `invalidate_range`).
// If this is a read mapping, ideally we would use a `clear_buffer` command before reading the data from GPU (i.e. `invalidate_range`).
// However, this would require us to kick off and wait for a command buffer or piggy back on an existing one (the later is likely the only worthwhile option).
// As reading uninitialized memory isn't a particular important path to support,
// we instead just initialize the memory here and make sure it is GPU visible, so this happens at max only once for every buffer region.
Expand Down Expand Up @@ -498,7 +498,7 @@ impl<A: HalApi> Device<A> {
}
} else {
// We are required to zero out (initialize) all memory.
// This is done on demand using fill_buffer which requires write transfer usage!
// This is done on demand using clear_buffer which requires write transfer usage!
usage |= hal::BufferUses::COPY_DST;
}

Expand Down
2 changes: 1 addition & 1 deletion wgpu-core/src/device/trace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ pub enum Command {
offset: wgt::BufferAddress,
size: Option<wgt::BufferSize>,
},
ClearImage {
ClearTexture {
dst: id::TextureId,
subresource_range: wgt::ImageSubresourceRange,
},
Expand Down
3 changes: 2 additions & 1 deletion wgpu-hal/src/dx12/adapter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,8 @@ impl super::Adapter {
| wgt::Features::POLYGON_MODE_POINT
| wgt::Features::VERTEX_WRITABLE_STORAGE
| wgt::Features::TIMESTAMP_QUERY
| wgt::Features::TEXTURE_COMPRESSION_BC;
| wgt::Features::TEXTURE_COMPRESSION_BC
| wgt::Features::CLEAR_COMMANDS;
//TODO: in order to expose this, we need to run a compute shader
// that extract the necessary statistics out of the D3D12 result.
// Alternatively, we could allocate a buffer for the query set,
Expand Down
Loading