Do texture init via clear passes when possible (#2307)

* CLEAR_COMMANDS extension is now more of a window into wgpu zero-init
this has mostly implications on the constraints, but also allows a more leaky documentation which makes sense for this non-standard function as there is no other place to look it up

* clear_texture via renderpasses wip

* 3D depth textures are no longer allowed, volumes are always cleared via CPY_DST

* cleanup texture's clear_views

* rename CLEAR_COMMANDS to CLEAR_TEXTURE

* separate clear_texture into reusable & more descriptive parts

* texture clear views are now created ahead of time

* discarded surface fixup goes through new clear_texture method now

* onq ueue texture initialization now goes threw clear_texture
pending inits need to store Stored textures now though, causing more ref count bumping

* texture init on queue_write_texture now also goes through new clear_texture

* transfer functions on commandbuffer use now new texture init route

* merge collect_zero_buffer_copies_for_clear_texture into clear_texture_via_buffer_copies

* clear functions now take TextureInitRange

* Fix clippy lints

* command_encoder_clear_texture no longer takes write lock on texture

* TextureClearMode encodes now is_color

* code cleanup, mostly about `use`

* Handle volume textures in clear_texture_via_render_passes properly

* texture clear no longer requires id::Stored

* init tracking fixes for volumes and init on partial subresource writes

* texture creation enforces COPY_DST only if absolutely necessary

* unrolled functional chain, reduce unsafe scope size

* fix clippy lints

* clear_texture test no longer creates 1D textures

see #2323

* 3D textures are no longer cleared as render target since this isn't supported on Metal

* fix deno building issue, fix formatting

* TextureInner::Surface can now be zero initialized

deno_webgpu/src/lib.rs

@@ -202,8 +202,8 @@ fn deserialize_features(features: &wgpu_types::Features) -> Vec<&'static str> {
     if features.contains(wgpu_types::Features::VERTEX_WRITABLE_STORAGE) {
         return_features.push("vertex-writable-storage");
     }
-    if features.contains(wgpu_types::Features::CLEAR_COMMANDS) {
-        return_features.push("clear-commands");
+    if features.contains(wgpu_types::Features::CLEAR_TEXTURE) {
+        return_features.push("clear-texture");
     }
     if features.contains(wgpu_types::Features::SPIRV_SHADER_PASSTHROUGH) {
         return_features.push("spirv-shader-passthrough");
@@ -417,7 +417,7 @@ impl From<GpuRequiredFeatures> for wgpu_types::Features {
             required_features.0.contains("vertex-writable-storage"),
         );
         features.set(
-            wgpu_types::Features::CLEAR_COMMANDS,
+            wgpu_types::Features::CLEAR_TEXTURE,
             required_features.0.contains("clear-commands"),
         );
         features.set(

wgpu-core/src/command/clear.rs

@@ -5,24 +5,24 @@ use crate::device::trace::Command as TraceCommand;
 use crate::{
     align_to,
     command::CommandBuffer,
+    device::Device,
     get_lowest_common_denom,
-    hub::{Global, GlobalIdentityHandlerFactory, HalApi, Token},
-    id::{BufferId, CommandEncoderId, DeviceId, TextureId},
-    init_tracker::MemoryInitKind,
-    track::TextureSelector,
+    hub::{Global, GlobalIdentityHandlerFactory, HalApi, Resource, Token},
+    id::{BufferId, CommandEncoderId, DeviceId, TextureId, Valid},
+    init_tracker::{MemoryInitKind, TextureInitRange},
+    resource::{Texture, TextureClearMode},
+    track::{ResourceTracker, TextureSelector, TextureState},
 };
 
 use hal::CommandEncoder as _;
 use thiserror::Error;
-use wgt::{
-    BufferAddress, BufferSize, BufferUsages, ImageSubresourceRange, TextureAspect, TextureUsages,
-};
+use wgt::{BufferAddress, BufferSize, BufferUsages, ImageSubresourceRange, TextureAspect};
 
 /// Error encountered while attempting a clear.
 #[derive(Clone, Debug, Error)]
 pub enum ClearError {
-    #[error("to use clear_buffer/texture the CLEAR_COMMANDS feature needs to be enabled")]
-    MissingClearCommandsFeature,
+    #[error("to use clear_texture the CLEAR_TEXTURE feature needs to be enabled")]
+    MissingClearTextureFeature,
     #[error("command encoder {0:?} is invalid")]
     InvalidCommandEncoder(CommandEncoderId),
     #[error("device {0:?} is invalid")]
@@ -31,6 +31,8 @@ pub enum ClearError {
     InvalidBuffer(BufferId),
     #[error("texture {0:?} is invalid or destroyed")]
     InvalidTexture(TextureId),
+    #[error("texture {0:?} can not be cleared")]
+    NoValidTextureClearMode(TextureId),
     #[error("buffer clear size {0:?} is not a multiple of `COPY_BUFFER_ALIGNMENT`")]
     UnalignedFillSize(BufferSize),
     #[error("buffer offset {0:?} is not a multiple of `COPY_BUFFER_ALIGNMENT`")]
@@ -41,17 +43,13 @@ pub enum ClearError {
         end_offset: BufferAddress,
         buffer_size: BufferAddress,
     },
-    #[error("destination buffer/texture is missing the `COPY_DST` usage flag")]
+    #[error("destination buffer is missing the `COPY_DST` usage flag")]
     MissingCopyDstUsageFlag(Option<BufferId>, Option<TextureId>),
     #[error("texture lacks the aspects that were specified in the image subresource range. Texture with format {texture_format:?}, specified was {subresource_range_aspects:?}")]
     MissingTextureAspect {
         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 {
@@ -173,8 +171,8 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
             });
         }
 
-        if !cmd_buf.support_clear_buffer_texture {
-            return Err(ClearError::MissingClearCommandsFeature);
+        if !cmd_buf.support_clear_texture {
+            return Err(ClearError::MissingClearTextureFeature);
         }
 
         let dst_texture = texture_guard
@@ -191,14 +189,6 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
             });
         };
 
-        // 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(),
@@ -228,68 +218,123 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
             });
         }
 
-        // query from tracker with usage (and check usage)
-        let (dst_texture, dst_pending) = cmd_buf
-            .trackers
-            .textures
-            .use_replace(
-                &*texture_guard,
-                dst,
-                TextureSelector {
-                    levels: subresource_range.base_mip_level..subresource_level_end,
-                    layers: subresource_range.base_array_layer..subresource_layer_end,
-                },
-                hal::TextureUses::COPY_DST,
-            )
-            .map_err(ClearError::InvalidTexture)?;
-        let dst_raw = dst_texture
-            .inner
-            .as_raw()
-            .ok_or(ClearError::InvalidTexture(dst))?;
-        if !dst_texture.desc.usage.contains(TextureUsages::COPY_DST) {
-            return Err(ClearError::MissingCopyDstUsageFlag(None, Some(dst)));
+        clear_texture(
+            Valid(dst),
+            dst_texture,
+            TextureInitRange {
+                mip_range: subresource_range.base_mip_level..subresource_level_end,
+                layer_range: subresource_range.base_array_layer..subresource_layer_end,
+            },
+            cmd_buf.encoder.open(),
+            &mut cmd_buf.trackers.textures,
+            &device_guard[cmd_buf.device_id.value],
+        )
+    }
+}
+
+pub(crate) fn clear_texture<A: hal::Api>(
+    dst_texture_id: Valid<TextureId>,
+    dst_texture: &Texture<A>,
+    range: TextureInitRange,
+    encoder: &mut A::CommandEncoder,
+    texture_tracker: &mut ResourceTracker<TextureState>,
+    device: &Device<A>,
+) -> Result<(), ClearError> {
+    clear_texture_no_device(
+        dst_texture_id,
+        dst_texture,
+        range,
+        encoder,
+        texture_tracker,
+        &device.alignments,
+        &device.zero_buffer,
+    )
+}
+
+pub(crate) fn clear_texture_no_device<A: hal::Api>(
+    dst_texture_id: Valid<TextureId>,
+    dst_texture: &Texture<A>,
+    range: TextureInitRange,
+    encoder: &mut A::CommandEncoder,
+    texture_tracker: &mut ResourceTracker<TextureState>,
+    alignments: &hal::Alignments,
+    zero_buffer: &A::Buffer,
+) -> Result<(), ClearError> {
+    let dst_raw = dst_texture
+        .inner
+        .as_raw()
+        .ok_or(ClearError::InvalidTexture(dst_texture_id.0))?;
+
+    // Issue the right barrier.
+    let clear_usage = match dst_texture.clear_mode {
+        TextureClearMode::BufferCopy => hal::TextureUses::COPY_DST,
+        TextureClearMode::RenderPass {
+            is_color: false, ..
+        } => hal::TextureUses::DEPTH_STENCIL_WRITE,
+        TextureClearMode::RenderPass { is_color: true, .. } => hal::TextureUses::COLOR_TARGET,
+        TextureClearMode::None => {
+            return Err(ClearError::NoValidTextureClearMode(dst_texture_id.0));
         }
+    };
 
-        // actual hal barrier & operation
-        let dst_barrier = dst_pending.map(|pending| pending.into_hal(dst_texture));
-        let encoder = cmd_buf.encoder.open();
-        let device = &device_guard[cmd_buf.device_id.value];
+    let selector = TextureSelector {
+        levels: range.mip_range.clone(),
+        layers: range.layer_range.clone(),
+    };
 
-        let mut zero_buffer_copy_regions = Vec::new();
-        collect_zero_buffer_copies_for_clear_texture(
+    // If we're in a texture-init usecase, we know that the texture is already tracked since whatever caused the init requirement,
+    // will have caused the usage tracker to be aware of the texture. Meaning, that it is safe to call call change_replace_tracked if the life_guard is already gone
+    // (i.e. the user no longer holds on to this texture).
+    // On the other hand, when coming via command_encoder_clear_texture, the life_guard is still there since in order to call it a texture object is needed.
+    //
+    // We could in theory distinguish these two scenarios in the internal clear_texture api in order to remove this check and call the cheaper change_replace_tracked whenever possible.
+    let dst_barrier = if let Some(ref_count) = dst_texture.life_guard().ref_count.as_ref() {
+        texture_tracker.change_replace(dst_texture_id, ref_count, selector, clear_usage)
+    } else {
+        texture_tracker.change_replace_tracked(dst_texture_id, selector, clear_usage)
+    }
+    .map(|pending| pending.into_hal(dst_texture));
+    unsafe {
+        encoder.transition_textures(dst_barrier);
+    }
+
+    // Record actual clearing
+    match dst_texture.clear_mode {
+        TextureClearMode::BufferCopy => clear_texture_via_buffer_copies::<A>(
             &dst_texture.desc,
-            device.alignments.buffer_copy_pitch.get() as u32,
-            subresource_range.base_mip_level..subresource_level_end,
-            subresource_range.base_array_layer..subresource_layer_end,
-            &mut zero_buffer_copy_regions,
-        );
-        unsafe {
-            encoder.transition_textures(dst_barrier);
-            if !zero_buffer_copy_regions.is_empty() {
-                encoder.copy_buffer_to_texture(
-                    &device.zero_buffer,
-                    dst_raw,
-                    zero_buffer_copy_regions.into_iter(),
-                );
-            }
+            alignments,
+            zero_buffer,
+            range,
+            encoder,
+            dst_raw,
+        ),
+        TextureClearMode::RenderPass { is_color, .. } => {
+            clear_texture_via_render_passes(dst_texture, range, is_color, encoder)?
+        }
+        TextureClearMode::None => {
+            return Err(ClearError::NoValidTextureClearMode(dst_texture_id.0));
         }
-        Ok(())
     }
+    Ok(())
 }
 
-pub(crate) fn collect_zero_buffer_copies_for_clear_texture(
+fn clear_texture_via_buffer_copies<A: hal::Api>(
     texture_desc: &wgt::TextureDescriptor<()>,
-    buffer_copy_pitch: u32,
-    mip_range: Range<u32>,
-    layer_range: Range<u32>,
-    out_copy_regions: &mut Vec<hal::BufferTextureCopy>, // TODO: Something better than Vec
+    alignments: &hal::Alignments,
+    zero_buffer: &A::Buffer, // Buffer of size device::ZERO_BUFFER_SIZE
+    range: TextureInitRange,
+    encoder: &mut A::CommandEncoder,
+    dst_raw: &A::Texture,
 ) {
+    // Gather list of zero_buffer copies and issue a single command then to perform them
+    let mut zero_buffer_copy_regions = Vec::new();
+    let buffer_copy_pitch = alignments.buffer_copy_pitch.get() as u32;
     let format_desc = texture_desc.format.describe();
 
     let bytes_per_row_alignment =
         get_lowest_common_denom(buffer_copy_pitch, format_desc.block_size as u32);
 
-    for mip_level in mip_range {
+    for mip_level in range.mip_range {
         let mut mip_size = texture_desc.mip_level_size(mip_level).unwrap();
         // Round to multiple of block size
         mip_size.width = align_to(mip_size.width, format_desc.block_dimensions.0 as u32);
@@ -305,23 +350,23 @@ pub(crate) fn collect_zero_buffer_copies_for_clear_texture(
         let max_rows_per_copy = max_rows_per_copy / format_desc.block_dimensions.1 as u32
             * format_desc.block_dimensions.1 as u32;
         assert!(max_rows_per_copy > 0, "Zero buffer size is too small to fill a single row of a texture with format {:?} and desc {:?}",
-                        texture_desc.format, texture_desc.size);
+                texture_desc.format, texture_desc.size);
 
         let z_range = 0..(if texture_desc.dimension == wgt::TextureDimension::D3 {
             mip_size.depth_or_array_layers
         } else {
             1
         });
 
-        for array_layer in layer_range.clone() {
+        for array_layer in range.layer_range.clone() {
             // TODO: Only doing one layer at a time for volume textures right now.
             for z in z_range.clone() {
                 // May need multiple copies for each subresource! However, we assume that we never need to split a row.
                 let mut num_rows_left = mip_size.height;
                 while num_rows_left > 0 {
                     let num_rows = num_rows_left.min(max_rows_per_copy);
 
-                    out_copy_regions.push(hal::BufferTextureCopy {
+                    zero_buffer_copy_regions.push(hal::BufferTextureCopy {
                         buffer_layout: wgt::ImageDataLayout {
                             offset: 0,
                             bytes_per_row: NonZeroU32::new(bytes_per_row),
@@ -349,4 +394,73 @@ pub(crate) fn collect_zero_buffer_copies_for_clear_texture(
             }
         }
     }
+
+    unsafe {
+        encoder.copy_buffer_to_texture(zero_buffer, dst_raw, zero_buffer_copy_regions.into_iter());
+    }
+}
+
+fn clear_texture_via_render_passes<A: hal::Api>(
+    dst_texture: &Texture<A>,
+    range: TextureInitRange,
+    is_color: bool,
+    encoder: &mut A::CommandEncoder,
+) -> Result<(), ClearError> {
+    let extent_base = wgt::Extent3d {
+        width: dst_texture.desc.size.width,
+        height: dst_texture.desc.size.height,
+        depth_or_array_layers: 1, // Only one layer or slice is cleared at a time.
+    };
+
+    let sample_count = dst_texture.desc.sample_count;
+    let is_3d_texture = dst_texture.desc.dimension == wgt::TextureDimension::D3;
+    for mip_level in range.mip_range {
+        let extent = extent_base.mip_level_size(mip_level, is_3d_texture);
+        let layer_or_depth_range = if dst_texture.desc.dimension == wgt::TextureDimension::D3 {
+            // TODO: We assume that we're allowed to do clear operations on volume texture slices, this is not properly specified.
+            0..extent.depth_or_array_layers
+        } else {
+            range.layer_range.clone()
+        };
+        for depth_or_layer in layer_or_depth_range {
+            let color_attachments_tmp;
+            let (color_attachments, depth_stencil_attachment) = if is_color {
+                color_attachments_tmp = [hal::ColorAttachment {
+                    target: hal::Attachment {
+                        view: dst_texture.get_clear_view(mip_level, depth_or_layer),
+                        usage: hal::TextureUses::COLOR_TARGET,
+                    },
+                    resolve_target: None,
+                    ops: hal::AttachmentOps::STORE,
+                    clear_value: wgt::Color::TRANSPARENT,
+                }];
+                (&color_attachments_tmp[..], None)
+            } else {
+                (
+                    &[][..],
+                    Some(hal::DepthStencilAttachment {
+                        target: hal::Attachment {
+                            view: dst_texture.get_clear_view(mip_level, depth_or_layer),
+                            usage: hal::TextureUses::DEPTH_STENCIL_WRITE,
+                        },
+                        depth_ops: hal::AttachmentOps::STORE,
+                        stencil_ops: hal::AttachmentOps::STORE,
+                        clear_value: (0.0, 0),
+                    }),
+                )
+            };
+            unsafe {
+                encoder.begin_render_pass(&hal::RenderPassDescriptor {
+                    label: Some("clear_texture clear pass"),
+                    extent,
+                    sample_count,
+                    color_attachments,
+                    depth_stencil_attachment,
+                    multiview: None,
+                });
+                encoder.end_render_pass();
+            }
+        }
+    }
+    Ok(())
 }