From 3898c205552b23828799a8f363bc982aa08af8e3 Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Tue, 9 Nov 2021 21:34:08 +0100 Subject: [PATCH 01/27] 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 --- wgpu-core/src/command/clear.rs | 5 +---- wgpu/src/lib.rs | 14 ++++++++++++-- wgpu/tests/clear_texture.rs | 4 +++- 3 files changed, 16 insertions(+), 7 deletions(-) diff --git a/wgpu-core/src/command/clear.rs b/wgpu-core/src/command/clear.rs index d36569edfe..5117cb180a 100644 --- a/wgpu-core/src/command/clear.rs +++ b/wgpu-core/src/command/clear.rs @@ -41,7 +41,7 @@ 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, Option), #[error("texture lacks the aspects that were specified in the image subresource range. Texture with format {texture_format:?}, specified was {subresource_range_aspects:?}")] MissingTextureAspect { @@ -246,9 +246,6 @@ impl Global { .inner .as_raw() .ok_or(ClearError::InvalidTexture(dst))?; - if !dst_texture.desc.usage.contains(TextureUsages::COPY_DST) { - return Err(ClearError::MissingCopyDstUsageFlag(None, Some(dst))); - } // actual hal barrier & operation let dst_barrier = dst_pending.map(|pending| pending.into_hal(dst_texture)); diff --git a/wgpu/src/lib.rs b/wgpu/src/lib.rs index e99f87bda8..7102bb29c8 100644 --- a/wgpu/src/lib.rs +++ b/wgpu/src/lib.rs @@ -2383,12 +2383,16 @@ impl CommandEncoder { /// Clears texture to zero. /// - /// Where possible it may be significantly more efficient to perform clears via render passes! + /// Note that unlike with clear_buffer, `COPY_DST` usage is not required. + /// + /// # Implementation notes + /// + /// - implemented either via buffer copies, render/depth target clear + /// - behaves like texture zero init, but is performed immediately (clearing is *not* delayed via marking it as uninitialized) /// /// # Panics /// /// - `CLEAR_COMMANDS` extension not enabled - /// - Texture does not have `COPY_DST` usage. /// - Range is out of bounds pub fn clear_texture(&mut self, texture: &Texture, subresource_range: &ImageSubresourceRange) { Context::command_encoder_clear_texture( @@ -2401,6 +2405,12 @@ impl CommandEncoder { /// Clears buffer to zero. /// + /// # Implementation notes + /// + /// - implemented via backend specific function which may be emulated with buffer copies + /// - behaves like delayed buffer zero init (i.e. lazy zero init for buffers that weren't mapped at creation), + /// but is performed immediately (clearing is *not* delayed via marking it as uninitialized) + /// /// # Panics /// /// - Buffer does not have `COPY_DST` usage. diff --git a/wgpu/tests/clear_texture.rs b/wgpu/tests/clear_texture.rs index 8f56e3b00e..6c16c10566 100644 --- a/wgpu/tests/clear_texture.rs +++ b/wgpu/tests/clear_texture.rs @@ -131,7 +131,9 @@ fn single_texture_clear_test( sample_count: 1, // multisampling is not supported for clear dimension, format, - usage: wgpu::TextureUsages::COPY_DST, + // Forces internally the required usages to be able to clear it. + // This is not visible on the API level. + usage: wgpu::TextureUsages::TEXTURE_BINDING, }); let mut encoder = ctx .device From 185814448ec6552ac88d1bc8a79a097df2ef5150 Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Sun, 14 Nov 2021 15:24:00 +0100 Subject: [PATCH 02/27] clear_texture via renderpasses wip --- wgpu-core/src/command/clear.rs | 144 +++++++++++++++++++++++++-------- wgpu-core/src/device/mod.rs | 14 +++- wgpu-core/src/present.rs | 2 + wgpu-core/src/resource.rs | 75 ++++++++++++++++- wgpu/tests/clear_texture.rs | 5 -- 5 files changed, 196 insertions(+), 44 deletions(-) diff --git a/wgpu-core/src/command/clear.rs b/wgpu-core/src/command/clear.rs index 5117cb180a..fd3e628ec1 100644 --- a/wgpu-core/src/command/clear.rs +++ b/wgpu-core/src/command/clear.rs @@ -14,9 +14,7 @@ use crate::{ 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)] @@ -48,10 +46,6 @@ 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 { @@ -66,6 +60,12 @@ whereas subesource range specified start {subresource_base_array_layer} and coun subresource_base_array_layer: u32, subresource_array_layer_count: Option, }, + #[error("failed to create view for clearing texture {texture:?} at mip level {mip_level}, layer {layer}")] + FailedToCreateTextureViewForClear { + texture: TextureId, + mip_level: u32, + layer: u32, + }, } impl Global { @@ -163,7 +163,7 @@ impl Global { let cmd_buf = CommandBuffer::get_encoder_mut(&mut *cmd_buf_guard, command_encoder_id) .map_err(|_| ClearError::InvalidCommandEncoder(command_encoder_id))?; let (_, mut token) = hub.buffers.read(&mut token); // skip token - let (texture_guard, _) = hub.textures.read(&mut token); + let (mut texture_guard, _) = hub.textures.write(&mut token); // todo: take only write access if needed #[cfg(feature = "trace")] if let Some(ref mut list) = cmd_buf.commands { @@ -191,14 +191,6 @@ impl Global { }); }; - // 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,6 +220,23 @@ impl Global { }); } + let clear_usage = if dst_texture + .hal_usage + .contains(hal::TextureUses::DEPTH_STENCIL_WRITE) + { + hal::TextureUses::DEPTH_STENCIL_WRITE + } else if dst_texture + .hal_usage + .contains(hal::TextureUses::COLOR_TARGET) + { + hal::TextureUses::COLOR_TARGET + } else if dst_texture.hal_usage.contains(hal::TextureUses::COPY_DST) { + hal::TextureUses::COPY_DST + } else { + panic!("Every texture must at least have one of hal::TextureUses::DEPTH_STENCIL_WRITE/COLOR_TARGET/COPY_DST. + Otherwise zero initialization is not possible (which in turn is mandated by wgpu specification)"); + }; + // query from tracker with usage (and check usage) let (dst_texture, dst_pending) = cmd_buf .trackers @@ -239,7 +248,7 @@ impl Global { levels: subresource_range.base_mip_level..subresource_level_end, layers: subresource_range.base_array_layer..subresource_layer_end, }, - hal::TextureUses::COPY_DST, + clear_usage, ) .map_err(ClearError::InvalidTexture)?; let dst_raw = dst_texture @@ -247,27 +256,92 @@ impl Global { .as_raw() .ok_or(ClearError::InvalidTexture(dst))?; - // 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 mut zero_buffer_copy_regions = Vec::new(); - collect_zero_buffer_copies_for_clear_texture( - &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); + // actual hal barrier & operation + { + let dst_barrier = dst_pending.map(|pending| pending.into_hal(dst_texture)); + unsafe { + encoder.transition_textures(dst_barrier); + } + } + + let device = &device_guard[cmd_buf.device_id.value]; + if clear_usage == hal::TextureUses::COPY_DST { + let mut zero_buffer_copy_regions = Vec::new(); + collect_zero_buffer_copies_for_clear_texture( + &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, + ); if !zero_buffer_copy_regions.is_empty() { - encoder.copy_buffer_to_texture( - &device.zero_buffer, - dst_raw, - zero_buffer_copy_regions.into_iter(), - ); + unsafe { + encoder.copy_buffer_to_texture( + &device.zero_buffer, + dst_raw, + zero_buffer_copy_regions.into_iter(), + ); + } + } + } else { + let dst_texture = texture_guard + .get_mut(dst) + .map_err(|_| ClearError::InvalidTexture(dst))?; + let extent_base = dst_texture.desc.size; + let sample_count = dst_texture.desc.sample_count; + let is_3d_texture = dst_texture.desc.dimension == wgt::TextureDimension::D3; + + for mip_level in subresource_range.base_mip_level..subresource_level_end { + let extent = extent_base.mip_level_size(mip_level, is_3d_texture); + for layer in subresource_range.base_array_layer..subresource_layer_end { + let target = hal::Attachment { + view: dst_texture + .get_or_create_clear_view(&device.raw, mip_level, layer) + .map_err(|_| ClearError::FailedToCreateTextureViewForClear { + texture: dst, + mip_level, + layer, + })?, + usage: clear_usage, + }; + + if clear_usage == hal::TextureUses::DEPTH_STENCIL_WRITE { + unsafe { + encoder.begin_render_pass(&hal::RenderPassDescriptor { + label: Some("clear_texture clear pass"), + extent, + sample_count, + color_attachments: &[], + depth_stencil_attachment: Some(hal::DepthStencilAttachment { + target, + depth_ops: hal::AttachmentOps::STORE, + stencil_ops: hal::AttachmentOps::STORE, + clear_value: (0.0, 0), + }), + }); + } + } else { + unsafe { + encoder.begin_render_pass(&hal::RenderPassDescriptor { + label: Some("clear_texture clear pass"), + extent, + sample_count, + color_attachments: &[hal::ColorAttachment { + target, + resolve_target: None, + ops: hal::AttachmentOps::STORE, + clear_value: wgt::Color::TRANSPARENT, + }], + depth_stencil_attachment: None, + }); + } + }; + unsafe { + encoder.end_render_pass(); + } + } } } Ok(()) diff --git a/wgpu-core/src/device/mod.rs b/wgpu-core/src/device/mod.rs index ea74421f0f..c45ebdb17d 100644 --- a/wgpu-core/src/device/mod.rs +++ b/wgpu-core/src/device/mod.rs @@ -625,6 +625,7 @@ impl Device { layers: 0..desc.array_layer_count(), }, life_guard: LifeGuard::new(desc.label.borrow_or_default()), + clear_views: ArrayVec::new(), } } @@ -634,9 +635,16 @@ impl Device { adapter: &crate::instance::Adapter, desc: &resource::TextureDescriptor, ) -> Result, resource::CreateTextureError> { - // Enforce COPY_DST, otherwise we wouldn't be able to initialize the texture. - let hal_usage = - conv::map_texture_usage(desc.usage, desc.format.into()) | hal::TextureUses::COPY_DST; + // Enforce having COPY_DST/DEPTH_STENCIL_WRIT/COLOR_TARGET otherwise we wouldn't be able to initialize the texture. + let format_desc = desc.format.describe(); + let hal_usage = conv::map_texture_usage(desc.usage, desc.format.into()) + | if format_desc.sample_type == wgt::TextureSampleType::Depth { + hal::TextureUses::DEPTH_STENCIL_WRITE + } else if desc.sample_count > 1 { + hal::TextureUses::COLOR_TARGET + } else { + hal::TextureUses::COPY_DST + }; let hal_desc = hal::TextureDescriptor { label: desc.label.borrow_option(), diff --git a/wgpu-core/src/present.rs b/wgpu-core/src/present.rs index dabd9bd642..e0f20fef09 100644 --- a/wgpu-core/src/present.rs +++ b/wgpu-core/src/present.rs @@ -22,6 +22,7 @@ use crate::{ LifeGuard, Stored, }; +use arrayvec::ArrayVec; use hal::{Queue as _, Surface as _}; use thiserror::Error; use wgt::SurfaceStatus as Status; @@ -158,6 +159,7 @@ impl Global { levels: 0..1, }, life_guard: LifeGuard::new(""), + clear_views: ArrayVec::new(), }; let ref_count = texture.life_guard.add_ref(); diff --git a/wgpu-core/src/resource.rs b/wgpu-core/src/resource.rs index 270202df53..b5f4b8cd0a 100644 --- a/wgpu-core/src/resource.rs +++ b/wgpu-core/src/resource.rs @@ -8,9 +8,17 @@ use crate::{ Label, LifeGuard, RefCount, Stored, }; +use arrayvec::ArrayVec; +use hal::Device; +use smallvec::SmallVec; use thiserror::Error; -use std::{borrow::Borrow, num::NonZeroU8, ops::Range, ptr::NonNull}; +use std::{ + borrow::Borrow, + num::{NonZeroU32, NonZeroU8}, + ops::Range, + ptr::NonNull, +}; #[repr(C)] #[derive(Debug)] @@ -188,6 +196,71 @@ pub struct Texture { pub(crate) initialization_status: TextureInitTracker, pub(crate) full_range: TextureSelector, pub(crate) life_guard: LifeGuard, + pub(crate) clear_views: + ArrayVec; 1]>, { hal::MAX_MIP_LEVELS as usize }>, +} + +impl Texture { + pub(crate) fn get_or_create_clear_view( + &mut self, + device: &A::Device, + mip_level: u32, + layer: u32, + ) -> Result<&A::TextureView, DeviceError> { + if self.clear_views.len() != self.desc.mip_level_count as usize { + self.clear_views.extend( + std::iter::repeat_with(|| SmallVec::new()).take(self.desc.mip_level_count as usize), + ); + } + + let layers = &mut self.clear_views[mip_level as usize]; + + if layers.len() != self.desc.size.depth_or_array_layers as usize { + layers.extend( + std::iter::repeat_with(|| None).take(self.desc.size.depth_or_array_layers as usize), + ); + } + + let view = &mut layers[layer as usize]; + + if view.is_none() { + let raw_texture = match self.inner.as_raw() { + Some(raw) => raw, + None => panic!("can't clear surface texture"), + }; + + *view = Some(unsafe { + device.create_texture_view( + raw_texture, + &hal::TextureViewDescriptor { + label: Some("clear texture view"), + format: self.desc.format, + dimension: match self.desc.dimension { + wgt::TextureDimension::D1 => wgt::TextureViewDimension::D1, + wgt::TextureDimension::D2 => wgt::TextureViewDimension::D2, + wgt::TextureDimension::D3 => wgt::TextureViewDimension::D3, + }, + usage: if self.desc.format.describe().sample_type + == wgt::TextureSampleType::Depth + { + hal::TextureUses::DEPTH_STENCIL_WRITE + } else { + hal::TextureUses::COLOR_TARGET + }, + range: wgt::ImageSubresourceRange { + aspect: wgt::TextureAspect::All, + base_mip_level: mip_level, + mip_level_count: NonZeroU32::new(1), + base_array_layer: layer, + array_layer_count: NonZeroU32::new(1), + }, + }, + )? + }); + } + + Ok(view.as_mut().unwrap()) + } } impl Global { diff --git a/wgpu/tests/clear_texture.rs b/wgpu/tests/clear_texture.rs index 6c16c10566..c56f3e75e4 100644 --- a/wgpu/tests/clear_texture.rs +++ b/wgpu/tests/clear_texture.rs @@ -112,11 +112,6 @@ fn single_texture_clear_test( size: wgpu::Extent3d, dimension: wgpu::TextureDimension, ) { - // clear_texture not supported for depth textures. - if format.describe().sample_type == wgpu::TextureSampleType::Depth { - return; - } - println!("clearing texture with {:?}", format); let texture = ctx.device.create_texture(&wgpu::TextureDescriptor { From 118bb2b877025a8aae2ad953855bee83d23caf78 Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Sun, 14 Nov 2021 19:55:40 +0100 Subject: [PATCH 03/27] 3D depth textures are no longer allowed, volumes are always cleared via CPY_DST --- wgpu-core/src/command/clear.rs | 26 ++++++++++++++++++-------- wgpu-core/src/device/mod.rs | 10 +++++++++- wgpu-core/src/resource.rs | 4 +++- wgpu/tests/clear_texture.rs | 27 ++++++++++++++++----------- 4 files changed, 46 insertions(+), 21 deletions(-) diff --git a/wgpu-core/src/command/clear.rs b/wgpu-core/src/command/clear.rs index fd3e628ec1..69949e53e7 100644 --- a/wgpu-core/src/command/clear.rs +++ b/wgpu-core/src/command/clear.rs @@ -220,14 +220,17 @@ impl Global { }); } - let clear_usage = if dst_texture - .hal_usage - .contains(hal::TextureUses::DEPTH_STENCIL_WRITE) + let is_3d = dst_texture.desc.dimension == wgt::TextureDimension::D3; + let clear_usage = if !is_3d + && dst_texture + .hal_usage + .contains(hal::TextureUses::DEPTH_STENCIL_WRITE) { hal::TextureUses::DEPTH_STENCIL_WRITE - } else if dst_texture - .hal_usage - .contains(hal::TextureUses::COLOR_TARGET) + } else if !is_3d + && dst_texture + .hal_usage + .contains(hal::TextureUses::COLOR_TARGET) { hal::TextureUses::COLOR_TARGET } else if dst_texture.hal_usage.contains(hal::TextureUses::COPY_DST) { @@ -289,9 +292,14 @@ impl Global { let dst_texture = texture_guard .get_mut(dst) .map_err(|_| ClearError::InvalidTexture(dst))?; - let extent_base = dst_texture.desc.size; - let sample_count = dst_texture.desc.sample_count; let is_3d_texture = dst_texture.desc.dimension == wgt::TextureDimension::D3; + let extent_base = wgt::Extent3d { + width: dst_texture.desc.size.width, + height: dst_texture.desc.size.height, + depth_or_array_layers: 1, // TODO: What about 3d textures? Only one slice a time, sure but how to select it? + }; + + let sample_count = dst_texture.desc.sample_count; for mip_level in subresource_range.base_mip_level..subresource_level_end { let extent = extent_base.mip_level_size(mip_level, is_3d_texture); @@ -320,6 +328,7 @@ impl Global { stencil_ops: hal::AttachmentOps::STORE, clear_value: (0.0, 0), }), + multiview: None, }); } } else { @@ -335,6 +344,7 @@ impl Global { clear_value: wgt::Color::TRANSPARENT, }], depth_stencil_attachment: None, + multiview: None, }); } }; diff --git a/wgpu-core/src/device/mod.rs b/wgpu-core/src/device/mod.rs index c45ebdb17d..d1eba54433 100644 --- a/wgpu-core/src/device/mod.rs +++ b/wgpu-core/src/device/mod.rs @@ -635,8 +635,16 @@ impl Device { adapter: &crate::instance::Adapter, desc: &resource::TextureDescriptor, ) -> Result, resource::CreateTextureError> { - // Enforce having COPY_DST/DEPTH_STENCIL_WRIT/COLOR_TARGET otherwise we wouldn't be able to initialize the texture. let format_desc = desc.format.describe(); + + // Depth volume textures can't be written to - depth forbids COPY_DST and volume textures can't be rendered to - therefore they aren't allowed. + if format_desc.sample_type == wgt::TextureSampleType::Depth + && desc.dimension == wgt::TextureDimension::D3 + { + return Err(resource::CreateTextureError::CannotCreateDepthVolumeTexture(desc.format)); + } + + // Enforce having COPY_DST/DEPTH_STENCIL_WRIT/COLOR_TARGET otherwise we wouldn't be able to initialize the texture. let hal_usage = conv::map_texture_usage(desc.usage, desc.format.into()) | if format_desc.sample_type == wgt::TextureSampleType::Depth { hal::TextureUses::DEPTH_STENCIL_WRITE diff --git a/wgpu-core/src/resource.rs b/wgpu-core/src/resource.rs index b5f4b8cd0a..8f5fb45673 100644 --- a/wgpu-core/src/resource.rs +++ b/wgpu-core/src/resource.rs @@ -238,7 +238,7 @@ impl Texture { dimension: match self.desc.dimension { wgt::TextureDimension::D1 => wgt::TextureViewDimension::D1, wgt::TextureDimension::D2 => wgt::TextureViewDimension::D2, - wgt::TextureDimension::D3 => wgt::TextureViewDimension::D3, + wgt::TextureDimension::D3 => panic!("Can't create clear view for D3 textures. Use clear via copy instead."), }, usage: if self.desc.format.describe().sample_type == wgt::TextureSampleType::Depth @@ -328,6 +328,8 @@ pub enum TextureDimensionError { pub enum CreateTextureError { #[error(transparent)] Device(#[from] DeviceError), + #[error("Depth Texture format {0:?} can't be used for volume textures")] + CannotCreateDepthVolumeTexture(wgt::TextureFormat), #[error("Textures cannot have empty usage flags")] EmptyUsage, #[error(transparent)] diff --git a/wgpu/tests/clear_texture.rs b/wgpu/tests/clear_texture.rs index c56f3e75e4..9639e592b0 100644 --- a/wgpu/tests/clear_texture.rs +++ b/wgpu/tests/clear_texture.rs @@ -112,7 +112,10 @@ fn single_texture_clear_test( size: wgpu::Extent3d, dimension: wgpu::TextureDimension, ) { - println!("clearing texture with {:?}", format); + println!( + "clearing texture with {:?}, dimension {:?}, size {:?}", + format, dimension, size + ); let texture = ctx.device.create_texture(&wgpu::TextureDescriptor { label: Some(&format!("texture {:?}", format)), @@ -186,16 +189,18 @@ fn clear_texture_tests(ctx: &TestingContext, formats: &[wgpu::TextureFormat], su wgpu::TextureDimension::D2, ); // volume texture - single_texture_clear_test( - ctx, - format, - wgpu::Extent3d { - width: 16, - height: 16, - depth_or_array_layers: 16, - }, - wgpu::TextureDimension::D3, - ); + if format.describe().sample_type != wgt::TextureSampleType::Depth { + single_texture_clear_test( + ctx, + format, + wgpu::Extent3d { + width: 16, + height: 16, + depth_or_array_layers: 16, + }, + wgpu::TextureDimension::D3, + ); + } } } From 09b940ddb3834de087224f640ec879310c0db7d9 Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Sun, 14 Nov 2021 19:56:42 +0100 Subject: [PATCH 04/27] cleanup texture's clear_views --- wgpu-core/src/device/life.rs | 24 ++++++++++++++++++------ wgpu-core/src/device/mod.rs | 12 ++++++++++-- wgpu-core/src/device/queue.rs | 8 ++++++-- wgpu-core/src/hub.rs | 11 +++++++++++ 4 files changed, 45 insertions(+), 10 deletions(-) diff --git a/wgpu-core/src/device/life.rs b/wgpu-core/src/device/life.rs index a12e202c0e..b03dacd7e1 100644 --- a/wgpu-core/src/device/life.rs +++ b/wgpu-core/src/device/life.rs @@ -256,7 +256,10 @@ impl LifetimeTracker { for res in temp_resources { match res { TempResource::Buffer(raw) => last_resources.buffers.push(raw), - TempResource::Texture(raw) => last_resources.textures.push(raw), + TempResource::Texture(raw, views) => { + last_resources.textures.push(raw); + last_resources.texture_views.extend(views); + } } } @@ -336,7 +339,10 @@ impl LifetimeTracker { .map_or(&mut self.free_resources, |a| &mut a.last_resources); match temp_resource { TempResource::Buffer(raw) => resources.buffers.push(raw), - TempResource::Texture(raw) => resources.textures.push(raw), + TempResource::Texture(raw, views) => { + resources.texture_views.extend(views); + resources.textures.push(raw); + } } } @@ -455,12 +461,18 @@ impl LifetimeTracker { resource::TextureInner::Native { raw: Some(raw) } => raw, _ => continue, }; - self.active + let non_referenced_resources = self + .active .iter_mut() .find(|a| a.index == submit_index) - .map_or(&mut self.free_resources, |a| &mut a.last_resources) - .textures - .push(raw); + .map_or(&mut self.free_resources, |a| &mut a.last_resources); + + non_referenced_resources.textures.push(raw); + non_referenced_resources.texture_views.extend( + res.clear_views + .into_iter() + .flat_map(|layers| layers.into_iter().filter_map(|v| v)), + ); } } } diff --git a/wgpu-core/src/device/mod.rs b/wgpu-core/src/device/mod.rs index d1eba54433..b5ae33fc8d 100644 --- a/wgpu-core/src/device/mod.rs +++ b/wgpu-core/src/device/mod.rs @@ -3396,15 +3396,23 @@ impl Global { trace.lock().add(trace::Action::FreeTexture(texture_id)); } + let last_submit_index = texture.life_guard.life_count(); + match texture.inner { resource::TextureInner::Native { ref mut raw } => { let raw = raw.take().ok_or(resource::DestroyError::AlreadyDestroyed)?; - let temp = queue::TempResource::Texture(raw); + let temp = queue::TempResource::Texture( + raw, + texture + .clear_views + .drain(..) + .flat_map(|layers| layers.into_iter().filter_map(|v| v)) + .collect(), + ); if device.pending_writes.dst_textures.contains(&texture_id) { device.pending_writes.temp_resources.push(temp); } else { - let last_submit_index = texture.life_guard.life_count(); drop(texture_guard); device .lock_life(&mut token) diff --git a/wgpu-core/src/device/queue.rs b/wgpu-core/src/device/queue.rs index 2cfe984f30..ced599a8ec 100644 --- a/wgpu-core/src/device/queue.rs +++ b/wgpu-core/src/device/queue.rs @@ -17,6 +17,7 @@ use crate::{ use hal::{CommandEncoder as _, Device as _, Queue as _}; use parking_lot::Mutex; +use smallvec::SmallVec; use std::{iter, mem, num::NonZeroU32, ptr}; use thiserror::Error; @@ -63,7 +64,7 @@ impl StagingData { #[derive(Debug)] pub enum TempResource { Buffer(A::Buffer), - Texture(A::Texture), + Texture(A::Texture, SmallVec<[A::TextureView; 1]>), } /// A queue execution for a particular command encoder. @@ -116,7 +117,10 @@ impl PendingWrites { TempResource::Buffer(buffer) => unsafe { device.destroy_buffer(buffer); }, - TempResource::Texture(texture) => unsafe { + TempResource::Texture(texture, views) => unsafe { + for view in views.into_iter() { + device.destroy_texture_view(view); + } device.destroy_texture(texture); }, } diff --git a/wgpu-core/src/hub.rs b/wgpu-core/src/hub.rs index 8ec2823aed..3fb313b150 100644 --- a/wgpu-core/src/hub.rs +++ b/wgpu-core/src/hub.rs @@ -644,6 +644,17 @@ impl Hub { for element in self.textures.data.write().map.drain(..) { if let Element::Occupied(texture, _) = element { let device = &devices[texture.device_id.value]; + + for layer_clear_views in texture.clear_views { + for view in layer_clear_views { + if let Some(view) = view { + unsafe { + device.raw.destroy_texture_view(view); + } + } + } + } + if let TextureInner::Native { raw: Some(raw) } = texture.inner { unsafe { device.raw.destroy_texture(raw); From 2007f1170d8780ab049b35d9f321af67bc918c1e Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Sun, 21 Nov 2021 11:46:30 +0100 Subject: [PATCH 05/27] rename CLEAR_COMMANDS to CLEAR_TEXTURE --- deno_webgpu/src/lib.rs | 6 +++--- wgpu-core/src/command/clear.rs | 8 ++++---- wgpu-core/src/command/mod.rs | 4 ++-- wgpu-hal/src/dx12/adapter.rs | 2 +- wgpu-hal/src/gles/adapter.rs | 2 +- wgpu-hal/src/metal/adapter.rs | 2 +- wgpu-hal/src/vulkan/adapter.rs | 2 +- wgpu-types/src/lib.rs | 4 ++-- wgpu/src/lib.rs | 10 ++-------- wgpu/tests/clear_texture.rs | 11 +++++------ 10 files changed, 22 insertions(+), 29 deletions(-) diff --git a/deno_webgpu/src/lib.rs b/deno_webgpu/src/lib.rs index db5ac1a8cf..fa193e5704 100644 --- a/deno_webgpu/src/lib.rs +++ b/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 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( diff --git a/wgpu-core/src/command/clear.rs b/wgpu-core/src/command/clear.rs index 69949e53e7..4861eb1927 100644 --- a/wgpu-core/src/command/clear.rs +++ b/wgpu-core/src/command/clear.rs @@ -19,8 +19,8 @@ use wgt::{BufferAddress, BufferSize, BufferUsages, ImageSubresourceRange, Textur /// 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")] @@ -173,8 +173,8 @@ impl Global { }); } - 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 diff --git a/wgpu-core/src/command/mod.rs b/wgpu-core/src/command/mod.rs index aa45ec9ab7..e210fb3c6b 100644 --- a/wgpu-core/src/command/mod.rs +++ b/wgpu-core/src/command/mod.rs @@ -101,7 +101,7 @@ pub struct CommandBuffer { buffer_memory_init_actions: Vec, texture_memory_actions: CommandBufferTextureMemoryActions, limits: wgt::Limits, - support_clear_buffer_texture: bool, + support_clear_texture: bool, #[cfg(feature = "trace")] pub(crate) commands: Option>, } @@ -129,7 +129,7 @@ impl CommandBuffer { buffer_memory_init_actions: Default::default(), texture_memory_actions: Default::default(), limits, - support_clear_buffer_texture: features.contains(wgt::Features::CLEAR_COMMANDS), + support_clear_texture: features.contains(wgt::Features::CLEAR_TEXTURE), #[cfg(feature = "trace")] commands: if enable_tracing { Some(Vec::new()) diff --git a/wgpu-hal/src/dx12/adapter.rs b/wgpu-hal/src/dx12/adapter.rs index 69f63b9350..2234d15717 100644 --- a/wgpu-hal/src/dx12/adapter.rs +++ b/wgpu-hal/src/dx12/adapter.rs @@ -186,7 +186,7 @@ impl super::Adapter { | wgt::Features::TEXTURE_ADAPTER_SPECIFIC_FORMAT_FEATURES | wgt::Features::TIMESTAMP_QUERY | wgt::Features::TEXTURE_COMPRESSION_BC - | wgt::Features::CLEAR_COMMANDS + | wgt::Features::CLEAR_TEXTURE | wgt::Features::TEXTURE_FORMAT_16BIT_NORM; //TODO: in order to expose this, we need to run a compute shader // that extract the necessary statistics out of the D3D12 result. diff --git a/wgpu-hal/src/gles/adapter.rs b/wgpu-hal/src/gles/adapter.rs index 0f14c0c334..9c882c8c41 100644 --- a/wgpu-hal/src/gles/adapter.rs +++ b/wgpu-hal/src/gles/adapter.rs @@ -286,7 +286,7 @@ impl super::Adapter { let mut features = wgt::Features::empty() | wgt::Features::TEXTURE_ADAPTER_SPECIFIC_FORMAT_FEATURES - | wgt::Features::CLEAR_COMMANDS; + | wgt::Features::CLEAR_TEXTURE; features.set( wgt::Features::ADDRESS_MODE_CLAMP_TO_BORDER, extensions.contains("GL_EXT_texture_border_clamp"), diff --git a/wgpu-hal/src/metal/adapter.rs b/wgpu-hal/src/metal/adapter.rs index a794ec12d0..287430a7fe 100644 --- a/wgpu-hal/src/metal/adapter.rs +++ b/wgpu-hal/src/metal/adapter.rs @@ -920,7 +920,7 @@ impl super::PrivateCapabilities { | F::TEXTURE_ADAPTER_SPECIFIC_FORMAT_FEATURES | F::PUSH_CONSTANTS | F::POLYGON_MODE_LINE - | F::CLEAR_COMMANDS + | F::CLEAR_TEXTURE | F::TEXTURE_FORMAT_16BIT_NORM; features.set(F::DEPTH_CLIP_CONTROL, self.supports_depth_clip_control); diff --git a/wgpu-hal/src/vulkan/adapter.rs b/wgpu-hal/src/vulkan/adapter.rs index 8c98f62df6..c0ee41c8f8 100644 --- a/wgpu-hal/src/vulkan/adapter.rs +++ b/wgpu-hal/src/vulkan/adapter.rs @@ -332,7 +332,7 @@ impl PhysicalDeviceFeatures { | F::TIMESTAMP_QUERY | F::PIPELINE_STATISTICS_QUERY | F::TEXTURE_ADAPTER_SPECIFIC_FORMAT_FEATURES - | F::CLEAR_COMMANDS; + | F::CLEAR_TEXTURE; let mut dl_flags = Df::all(); dl_flags.set(Df::CUBE_ARRAY_TEXTURES, self.core.image_cube_array != 0); diff --git a/wgpu-types/src/lib.rs b/wgpu-types/src/lib.rs index 980e81c929..e356c1669a 100644 --- a/wgpu-types/src/lib.rs +++ b/wgpu-types/src/lib.rs @@ -505,13 +505,13 @@ bitflags::bitflags! { /// /// This is a native-only feature. const VERTEX_WRITABLE_STORAGE = 1 << 36; - /// Enables clear to zero for buffers & textures. + /// Enables clear to zero for textures. /// /// Supported platforms: /// - All /// /// This is a native only feature. - const CLEAR_COMMANDS = 1 << 37; + const CLEAR_TEXTURE = 1 << 37; /// Enables creating shader modules from SPIR-V binary data (unsafe). /// /// SPIR-V data is not parsed or interpreted in any way; you can use diff --git a/wgpu/src/lib.rs b/wgpu/src/lib.rs index 7102bb29c8..a0eb7abe9d 100644 --- a/wgpu/src/lib.rs +++ b/wgpu/src/lib.rs @@ -2387,12 +2387,12 @@ impl CommandEncoder { /// /// # Implementation notes /// - /// - implemented either via buffer copies, render/depth target clear + /// - implemented either via buffer copies and render/depth target clear, path depends on texture usages /// - behaves like texture zero init, but is performed immediately (clearing is *not* delayed via marking it as uninitialized) /// /// # Panics /// - /// - `CLEAR_COMMANDS` extension not enabled + /// - `CLEAR_TEXTURE` extension not enabled /// - Range is out of bounds pub fn clear_texture(&mut self, texture: &Texture, subresource_range: &ImageSubresourceRange) { Context::command_encoder_clear_texture( @@ -2405,12 +2405,6 @@ impl CommandEncoder { /// Clears buffer to zero. /// - /// # Implementation notes - /// - /// - implemented via backend specific function which may be emulated with buffer copies - /// - behaves like delayed buffer zero init (i.e. lazy zero init for buffers that weren't mapped at creation), - /// but is performed immediately (clearing is *not* delayed via marking it as uninitialized) - /// /// # Panics /// /// - Buffer does not have `COPY_DST` usage. diff --git a/wgpu/tests/clear_texture.rs b/wgpu/tests/clear_texture.rs index 9639e592b0..2e1d50beb0 100644 --- a/wgpu/tests/clear_texture.rs +++ b/wgpu/tests/clear_texture.rs @@ -207,7 +207,7 @@ fn clear_texture_tests(ctx: &TestingContext, formats: &[wgpu::TextureFormat], su #[test] fn clear_texture_2d_uncompressed() { initialize_test( - TestParameters::default().features(wgpu::Features::CLEAR_COMMANDS), + TestParameters::default().features(wgpu::Features::CLEAR_TEXTURE), |ctx| { clear_texture_tests(&ctx, TEXTURE_FORMATS_UNCOMPRESSED, true); }, @@ -218,7 +218,7 @@ fn clear_texture_2d_uncompressed() { fn clear_texture_2d_bc() { initialize_test( TestParameters::default() - .features(wgpu::Features::CLEAR_COMMANDS | wgpu::Features::TEXTURE_COMPRESSION_BC), + .features(wgpu::Features::CLEAR_TEXTURE | wgpu::Features::TEXTURE_COMPRESSION_BC), |ctx| { clear_texture_tests(&ctx, TEXTURE_FORMATS_BC, false); }, @@ -228,9 +228,8 @@ fn clear_texture_2d_bc() { #[test] fn clear_texture_2d_astc() { initialize_test( - TestParameters::default().features( - wgpu::Features::CLEAR_COMMANDS | wgpu::Features::TEXTURE_COMPRESSION_ASTC_LDR, - ), + TestParameters::default() + .features(wgpu::Features::CLEAR_TEXTURE | wgpu::Features::TEXTURE_COMPRESSION_ASTC_LDR), |ctx| { clear_texture_tests(&ctx, TEXTURE_FORMATS_ASTC, false); }, @@ -241,7 +240,7 @@ fn clear_texture_2d_astc() { fn clear_texture_2d_etc2() { initialize_test( TestParameters::default() - .features(wgpu::Features::CLEAR_COMMANDS | wgpu::Features::TEXTURE_COMPRESSION_ETC2), + .features(wgpu::Features::CLEAR_TEXTURE | wgpu::Features::TEXTURE_COMPRESSION_ETC2), |ctx| { clear_texture_tests(&ctx, TEXTURE_FORMATS_ETC2, false); }, From f233a812a04a1447e17e2729c72a103f1d114035 Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Sun, 21 Nov 2021 12:30:17 +0100 Subject: [PATCH 06/27] separate clear_texture into reusable & more descriptive parts --- wgpu-core/src/command/clear.rs | 323 +++++++++++++++++++-------------- 1 file changed, 183 insertions(+), 140 deletions(-) diff --git a/wgpu-core/src/command/clear.rs b/wgpu-core/src/command/clear.rs index 4861eb1927..7898de4f40 100644 --- a/wgpu-core/src/command/clear.rs +++ b/wgpu-core/src/command/clear.rs @@ -5,10 +5,12 @@ 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}, + hub::{Global, GlobalIdentityHandlerFactory, HalApi, Resource, Storage, Token}, + id::{self, BufferId, CommandEncoderId, DeviceId, TextureId}, init_tracker::MemoryInitKind, + resource::Texture, track::TextureSelector, }; @@ -60,12 +62,8 @@ whereas subesource range specified start {subresource_base_array_layer} and coun subresource_base_array_layer: u32, subresource_array_layer_count: Option, }, - #[error("failed to create view for clearing texture {texture:?} at mip level {mip_level}, layer {layer}")] - FailedToCreateTextureViewForClear { - texture: TextureId, - mip_level: u32, - layer: u32, - }, + #[error("failed to create view for clearing texture at mip level {mip_level}, layer {layer}")] + FailedToCreateTextureViewForClear { mip_level: u32, layer: u32 }, } impl Global { @@ -163,7 +161,7 @@ impl Global { let cmd_buf = CommandBuffer::get_encoder_mut(&mut *cmd_buf_guard, command_encoder_id) .map_err(|_| ClearError::InvalidCommandEncoder(command_encoder_id))?; let (_, mut token) = hub.buffers.read(&mut token); // skip token - let (mut texture_guard, _) = hub.textures.write(&mut token); // todo: take only write access if needed + let (mut texture_guard, _) = hub.textures.write(&mut token); #[cfg(feature = "trace")] if let Some(ref mut list) = cmd_buf.commands { @@ -178,7 +176,7 @@ impl Global { } let dst_texture = texture_guard - .get(dst) + .get_mut(dst) // todo: take only write access if needed .map_err(|_| ClearError::InvalidTexture(dst))?; // Check if subresource aspects are valid. @@ -220,141 +218,117 @@ impl Global { }); } - let is_3d = dst_texture.desc.dimension == wgt::TextureDimension::D3; - let clear_usage = if !is_3d - && dst_texture - .hal_usage - .contains(hal::TextureUses::DEPTH_STENCIL_WRITE) - { - hal::TextureUses::DEPTH_STENCIL_WRITE - } else if !is_3d - && dst_texture - .hal_usage - .contains(hal::TextureUses::COLOR_TARGET) - { - hal::TextureUses::COLOR_TARGET - } else if dst_texture.hal_usage.contains(hal::TextureUses::COPY_DST) { - hal::TextureUses::COPY_DST - } else { - panic!("Every texture must at least have one of hal::TextureUses::DEPTH_STENCIL_WRITE/COLOR_TARGET/COPY_DST. - Otherwise zero initialization is not possible (which in turn is mandated by wgpu specification)"); - }; - - // 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, - }, - clear_usage, - ) - .map_err(ClearError::InvalidTexture)?; - let dst_raw = dst_texture - .inner - .as_raw() - .ok_or(ClearError::InvalidTexture(dst))?; - - let encoder = cmd_buf.encoder.open(); + clear_texture( + dst_texture, + cmd_buf, + dst, + subresource_range.base_mip_level..subresource_level_end, + subresource_range.base_array_layer..subresource_layer_end, + &*device_guard, + ) + } +} - // actual hal barrier & operation - { - let dst_barrier = dst_pending.map(|pending| pending.into_hal(dst_texture)); - unsafe { - encoder.transition_textures(dst_barrier); - } +fn clear_texture( + dst_texture: &mut Texture, + cmd_buf: &mut CommandBuffer, + dst: TextureId, + mip_range: Range, + layer_range: Range, + device_guard: &Storage, DeviceId>, +) -> Result<(), ClearError> { + // Determine which way of clearing should be used. + let is_3d_texture = dst_texture.desc.dimension == wgt::TextureDimension::D3; + let clear_usage = if !is_3d_texture + && dst_texture + .hal_usage + .contains(hal::TextureUses::DEPTH_STENCIL_WRITE) + { + hal::TextureUses::DEPTH_STENCIL_WRITE + } else if !is_3d_texture + && dst_texture + .hal_usage + .contains(hal::TextureUses::COLOR_TARGET) + { + hal::TextureUses::COLOR_TARGET + } else if dst_texture.hal_usage.contains(hal::TextureUses::COPY_DST) { + hal::TextureUses::COPY_DST + } else { + panic!("Every texture must at least have one of hal::TextureUses::DEPTH_STENCIL_WRITE/COLOR_TARGET/COPY_DST. + Otherwise zero initialization is not possible (which in turn is mandated by wgpu specification)"); + }; + + // Issue adequate barrier. + let dst_pending = cmd_buf.trackers.textures.change_replace( + id::Valid(dst), + dst_texture.life_guard().ref_count.as_ref().unwrap(), + TextureSelector { + levels: mip_range.clone(), + layers: layer_range.clone(), + }, + clear_usage, + ); + let dst_raw = dst_texture + .inner + .as_raw() + .ok_or(ClearError::InvalidTexture(dst))?; + let encoder = cmd_buf.encoder.open(); + { + let dst_barrier = dst_pending.map(|pending| pending.into_hal(dst_texture)); + unsafe { + encoder.transition_textures(dst_barrier); } + } + let device = &device_guard[cmd_buf.device_id.value]; + + if clear_usage == hal::TextureUses::COPY_DST { + clear_texture_via_buffer_copies( + &dst_texture.desc, + device, + mip_range, + layer_range, + encoder, + dst_raw, + ); + } else { + clear_texture_via_render_passes( + dst_texture, + is_3d_texture, + mip_range, + layer_range, + device, + clear_usage, + encoder, + )?; + } - let device = &device_guard[cmd_buf.device_id.value]; - if clear_usage == hal::TextureUses::COPY_DST { - let mut zero_buffer_copy_regions = Vec::new(); - collect_zero_buffer_copies_for_clear_texture( - &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, - ); - if !zero_buffer_copy_regions.is_empty() { - unsafe { - encoder.copy_buffer_to_texture( - &device.zero_buffer, - dst_raw, - zero_buffer_copy_regions.into_iter(), - ); - } - } - } else { - let dst_texture = texture_guard - .get_mut(dst) - .map_err(|_| ClearError::InvalidTexture(dst))?; - let is_3d_texture = dst_texture.desc.dimension == wgt::TextureDimension::D3; - let extent_base = wgt::Extent3d { - width: dst_texture.desc.size.width, - height: dst_texture.desc.size.height, - depth_or_array_layers: 1, // TODO: What about 3d textures? Only one slice a time, sure but how to select it? - }; + Ok(()) +} - let sample_count = dst_texture.desc.sample_count; - - for mip_level in subresource_range.base_mip_level..subresource_level_end { - let extent = extent_base.mip_level_size(mip_level, is_3d_texture); - for layer in subresource_range.base_array_layer..subresource_layer_end { - let target = hal::Attachment { - view: dst_texture - .get_or_create_clear_view(&device.raw, mip_level, layer) - .map_err(|_| ClearError::FailedToCreateTextureViewForClear { - texture: dst, - mip_level, - layer, - })?, - usage: clear_usage, - }; - - if clear_usage == hal::TextureUses::DEPTH_STENCIL_WRITE { - unsafe { - encoder.begin_render_pass(&hal::RenderPassDescriptor { - label: Some("clear_texture clear pass"), - extent, - sample_count, - color_attachments: &[], - depth_stencil_attachment: Some(hal::DepthStencilAttachment { - target, - depth_ops: hal::AttachmentOps::STORE, - stencil_ops: hal::AttachmentOps::STORE, - clear_value: (0.0, 0), - }), - multiview: None, - }); - } - } else { - unsafe { - encoder.begin_render_pass(&hal::RenderPassDescriptor { - label: Some("clear_texture clear pass"), - extent, - sample_count, - color_attachments: &[hal::ColorAttachment { - target, - resolve_target: None, - ops: hal::AttachmentOps::STORE, - clear_value: wgt::Color::TRANSPARENT, - }], - depth_stencil_attachment: None, - multiview: None, - }); - } - }; - unsafe { - encoder.end_render_pass(); - } - } - } +fn clear_texture_via_buffer_copies( + texture_desc: &wgt::TextureDescriptor<()>, + device: &Device, + mip_range: Range, + layer_range: Range, + encoder: &mut A::CommandEncoder, + dst_raw: &A::Texture, +) { + let mut zero_buffer_copy_regions = Vec::new(); + collect_zero_buffer_copies_for_clear_texture( + &texture_desc, + device.alignments.buffer_copy_pitch.get() as u32, + mip_range, + layer_range, + &mut zero_buffer_copy_regions, + ); + if !zero_buffer_copy_regions.is_empty() { + unsafe { + encoder.copy_buffer_to_texture( + &device.zero_buffer, + dst_raw, + zero_buffer_copy_regions.into_iter(), + ); } - Ok(()) } } @@ -431,3 +405,72 @@ pub(crate) fn collect_zero_buffer_copies_for_clear_texture( } } } + +fn clear_texture_via_render_passes( + dst_texture: &mut Texture, + is_3d_texture: bool, + mip_range: Range, + layer_range: Range, + device: &Device, + clear_usage: hal::TextureUses, + 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, // TODO: What about 3d textures? Only one slice a time, sure but how to select it? + }; + let sample_count = dst_texture.desc.sample_count; + for mip_level in mip_range { + let extent = extent_base.mip_level_size(mip_level, is_3d_texture); + for layer in layer_range.clone() { + let target = hal::Attachment { + view: dst_texture + .get_or_create_clear_view(&device.raw, mip_level, layer) + .map_err(|_| ClearError::FailedToCreateTextureViewForClear { + mip_level, + layer, + })?, + usage: clear_usage, + }; + + if clear_usage == hal::TextureUses::DEPTH_STENCIL_WRITE { + unsafe { + encoder.begin_render_pass(&hal::RenderPassDescriptor { + label: Some("clear_texture clear pass"), + extent, + sample_count, + color_attachments: &[], + depth_stencil_attachment: Some(hal::DepthStencilAttachment { + target, + depth_ops: hal::AttachmentOps::STORE, + stencil_ops: hal::AttachmentOps::STORE, + clear_value: (0.0, 0), + }), + multiview: None, + }); + } + } else { + unsafe { + encoder.begin_render_pass(&hal::RenderPassDescriptor { + label: Some("clear_texture clear pass"), + extent, + sample_count, + color_attachments: &[hal::ColorAttachment { + target, + resolve_target: None, + ops: hal::AttachmentOps::STORE, + clear_value: wgt::Color::TRANSPARENT, + }], + depth_stencil_attachment: None, + multiview: None, + }); + } + }; + unsafe { + encoder.end_render_pass(); + } + } + } + Ok(()) +} From 16fe26c36b7f1e68dcc3b52153b93c845bd853c3 Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Sun, 21 Nov 2021 20:14:38 +0100 Subject: [PATCH 07/27] texture clear views are now created ahead of time --- wgpu-core/src/command/clear.rs | 112 +++++++++++++++------------------ wgpu-core/src/device/life.rs | 13 ++-- wgpu-core/src/device/mod.rs | 80 +++++++++++++++++++---- wgpu-core/src/hub.rs | 21 +++---- wgpu-core/src/present.rs | 3 +- wgpu-core/src/resource.rs | 92 +++++++-------------------- 6 files changed, 162 insertions(+), 159 deletions(-) diff --git a/wgpu-core/src/command/clear.rs b/wgpu-core/src/command/clear.rs index 7898de4f40..df5bd37e02 100644 --- a/wgpu-core/src/command/clear.rs +++ b/wgpu-core/src/command/clear.rs @@ -7,10 +7,10 @@ use crate::{ command::CommandBuffer, device::Device, get_lowest_common_denom, - hub::{Global, GlobalIdentityHandlerFactory, HalApi, Resource, Storage, Token}, + hub::{Global, GlobalIdentityHandlerFactory, HalApi, Resource, Token}, id::{self, BufferId, CommandEncoderId, DeviceId, TextureId}, init_tracker::MemoryInitKind, - resource::Texture, + resource::{Texture, TextureClearMode}, track::TextureSelector, }; @@ -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`")] @@ -224,84 +226,80 @@ impl Global { dst, subresource_range.base_mip_level..subresource_level_end, subresource_range.base_array_layer..subresource_layer_end, - &*device_guard, + &device_guard[cmd_buf.device_id.value], ) } } fn clear_texture( - dst_texture: &mut Texture, + dst_texture: &Texture, cmd_buf: &mut CommandBuffer, dst: TextureId, mip_range: Range, layer_range: Range, - device_guard: &Storage, DeviceId>, + device: &Device, ) -> Result<(), ClearError> { - // Determine which way of clearing should be used. - let is_3d_texture = dst_texture.desc.dimension == wgt::TextureDimension::D3; - let clear_usage = if !is_3d_texture - && dst_texture - .hal_usage - .contains(hal::TextureUses::DEPTH_STENCIL_WRITE) - { - hal::TextureUses::DEPTH_STENCIL_WRITE - } else if !is_3d_texture - && dst_texture - .hal_usage - .contains(hal::TextureUses::COLOR_TARGET) - { - hal::TextureUses::COLOR_TARGET - } else if dst_texture.hal_usage.contains(hal::TextureUses::COPY_DST) { - hal::TextureUses::COPY_DST - } else { - panic!("Every texture must at least have one of hal::TextureUses::DEPTH_STENCIL_WRITE/COLOR_TARGET/COPY_DST. - Otherwise zero initialization is not possible (which in turn is mandated by wgpu specification)"); - }; - - // Issue adequate barrier. - let dst_pending = cmd_buf.trackers.textures.change_replace( - id::Valid(dst), - dst_texture.life_guard().ref_count.as_ref().unwrap(), - TextureSelector { - levels: mip_range.clone(), - layers: layer_range.clone(), - }, - clear_usage, - ); + let encoder = cmd_buf.encoder.open(); let dst_raw = dst_texture .inner .as_raw() .ok_or(ClearError::InvalidTexture(dst))?; - let encoder = cmd_buf.encoder.open(); - { - let dst_barrier = dst_pending.map(|pending| pending.into_hal(dst_texture)); - unsafe { - encoder.transition_textures(dst_barrier); + + // Issue the right barrier. + let clear_usage = match dst_texture.clear_mode { + TextureClearMode::BufferCopy => hal::TextureUses::COPY_DST, + TextureClearMode::RenderPass(_) => { + if dst_texture + .hal_usage + .contains(hal::TextureUses::DEPTH_STENCIL_WRITE) + { + hal::TextureUses::DEPTH_STENCIL_WRITE + } else { + hal::TextureUses::COLOR_TARGET + } } + TextureClearMode::None => { + return Err(ClearError::NoValidTextureClearMode(dst)); + } + }; + let dst_barrier = cmd_buf + .trackers + .textures + .change_replace( + id::Valid(dst), + dst_texture.life_guard().ref_count.as_ref().unwrap(), + TextureSelector { + levels: mip_range.clone(), + layers: layer_range.clone(), + }, + clear_usage, + ) + .map(|pending| pending.into_hal(dst_texture)); + unsafe { + encoder.transition_textures(dst_barrier); } - let device = &device_guard[cmd_buf.device_id.value]; - if clear_usage == hal::TextureUses::COPY_DST { - clear_texture_via_buffer_copies( + // Record actual clearing + match dst_texture.clear_mode { + TextureClearMode::BufferCopy => clear_texture_via_buffer_copies( &dst_texture.desc, device, mip_range, layer_range, encoder, dst_raw, - ); - } else { - clear_texture_via_render_passes( + ), + TextureClearMode::RenderPass(_) => clear_texture_via_render_passes( dst_texture, - is_3d_texture, mip_range, layer_range, - device, clear_usage, encoder, - )?; + )?, + TextureClearMode::None => { + return Err(ClearError::NoValidTextureClearMode(dst)); + } } - Ok(()) } @@ -407,11 +405,9 @@ pub(crate) fn collect_zero_buffer_copies_for_clear_texture( } fn clear_texture_via_render_passes( - dst_texture: &mut Texture, - is_3d_texture: bool, + dst_texture: &Texture, mip_range: Range, layer_range: Range, - device: &Device, clear_usage: hal::TextureUses, encoder: &mut A::CommandEncoder, ) -> Result<(), ClearError> { @@ -421,16 +417,12 @@ fn clear_texture_via_render_passes( depth_or_array_layers: 1, // TODO: What about 3d textures? Only one slice a time, sure but how to select it? }; let sample_count = dst_texture.desc.sample_count; + let is_3d_texture = dst_texture.desc.dimension == wgt::TextureDimension::D3; for mip_level in mip_range { let extent = extent_base.mip_level_size(mip_level, is_3d_texture); for layer in layer_range.clone() { let target = hal::Attachment { - view: dst_texture - .get_or_create_clear_view(&device.raw, mip_level, layer) - .map_err(|_| ClearError::FailedToCreateTextureViewForClear { - mip_level, - layer, - })?, + view: dst_texture.get_clear_view(mip_level, layer), usage: clear_usage, }; diff --git a/wgpu-core/src/device/life.rs b/wgpu-core/src/device/life.rs index b03dacd7e1..96da3deae6 100644 --- a/wgpu-core/src/device/life.rs +++ b/wgpu-core/src/device/life.rs @@ -468,11 +468,14 @@ impl LifetimeTracker { .map_or(&mut self.free_resources, |a| &mut a.last_resources); non_referenced_resources.textures.push(raw); - non_referenced_resources.texture_views.extend( - res.clear_views - .into_iter() - .flat_map(|layers| layers.into_iter().filter_map(|v| v)), - ); + match res.clear_mode { + resource::TextureClearMode::RenderPass(clear_views) => { + non_referenced_resources + .texture_views + .extend(clear_views.into_iter()); + } + _ => {} + } } } } diff --git a/wgpu-core/src/device/mod.rs b/wgpu-core/src/device/mod.rs index b5ae33fc8d..137b9bc4e0 100644 --- a/wgpu-core/src/device/mod.rs +++ b/wgpu-core/src/device/mod.rs @@ -600,6 +600,7 @@ impl Device { self_id: id::DeviceId, desc: &resource::TextureDescriptor, format_features: wgt::TextureFormatFeatures, + clear_mode: resource::TextureClearMode, ) -> resource::Texture { debug_assert_eq!(self_id.backend(), A::VARIANT); @@ -625,7 +626,7 @@ impl Device { layers: 0..desc.array_layer_count(), }, life_guard: LifeGuard::new(desc.label.borrow_or_default()), - clear_views: ArrayVec::new(), + clear_mode, } } @@ -693,13 +694,60 @@ impl Device { return Err(resource::CreateTextureError::InvalidMipLevelCount(mips)); } - let raw = unsafe { + let raw_texture = unsafe { self.raw .create_texture(&hal_desc) .map_err(DeviceError::from)? }; - let mut texture = self.create_texture_from_hal(raw, self_id, desc, format_features); + let clear_mode = if hal_usage.contains(hal::TextureUses::DEPTH_STENCIL_WRITE) + || hal_usage.contains(hal::TextureUses::COLOR_TARGET) + { + let usage = if desc.format.describe().sample_type == wgt::TextureSampleType::Depth { + hal::TextureUses::DEPTH_STENCIL_WRITE + } else { + hal::TextureUses::COLOR_TARGET + }; + let dimension = match desc.dimension { + wgt::TextureDimension::D1 => wgt::TextureViewDimension::D1, + wgt::TextureDimension::D2 => wgt::TextureViewDimension::D2, + wgt::TextureDimension::D3 => wgt::TextureViewDimension::D2, // Todo? Is this even specified? + }; + + let mut clear_views = SmallVec::new(); + for layer in 0..desc.size.depth_or_array_layers { + for mip_level in 0..desc.mip_level_count { + unsafe { + clear_views.push( + self.raw + .create_texture_view( + &raw_texture, + &hal::TextureViewDescriptor { + label: Some("clear texture view"), + format: desc.format, + dimension, + usage, + range: wgt::ImageSubresourceRange { + aspect: wgt::TextureAspect::All, + base_mip_level: mip_level, + mip_level_count: NonZeroU32::new(1), + base_array_layer: layer, + array_layer_count: NonZeroU32::new(1), + }, + }, + ) + .map_err(DeviceError::from)?, + ) + } + } + } + resource::TextureClearMode::RenderPass(clear_views) + } else { + resource::TextureClearMode::BufferCopy + }; + + let mut texture = + self.create_texture_from_hal(raw_texture, self_id, desc, format_features, clear_mode); texture.hal_usage = hal_usage; Ok(texture) } @@ -3339,8 +3387,13 @@ impl Global { Err(error) => break error, }; - let mut texture = - device.create_texture_from_hal(hal_texture, device_id, desc, format_features); + let mut texture = device.create_texture_from_hal( + hal_texture, + device_id, + desc, + format_features, + resource::TextureClearMode::None, + ); if desc.usage.contains(wgt::TextureUsages::COPY_DST) { texture.hal_usage |= hal::TextureUses::COPY_DST; } @@ -3401,14 +3454,15 @@ impl Global { match texture.inner { resource::TextureInner::Native { ref mut raw } => { let raw = raw.take().ok_or(resource::DestroyError::AlreadyDestroyed)?; - let temp = queue::TempResource::Texture( - raw, - texture - .clear_views - .drain(..) - .flat_map(|layers| layers.into_iter().filter_map(|v| v)) - .collect(), - ); + let clear_views = match std::mem::replace( + &mut texture.clear_mode, + resource::TextureClearMode::None, + ) { + resource::TextureClearMode::BufferCopy => SmallVec::new(), + resource::TextureClearMode::RenderPass(clear_views) => clear_views, + resource::TextureClearMode::None => SmallVec::new(), + }; + let temp = queue::TempResource::Texture(raw, clear_views); if device.pending_writes.dst_textures.contains(&texture_id) { device.pending_writes.temp_resources.push(temp); diff --git a/wgpu-core/src/hub.rs b/wgpu-core/src/hub.rs index 3fb313b150..d742cf6966 100644 --- a/wgpu-core/src/hub.rs +++ b/wgpu-core/src/hub.rs @@ -5,7 +5,7 @@ use crate::{ id, instance::{Adapter, HalSurface, Instance, Surface}, pipeline::{ComputePipeline, RenderPipeline, ShaderModule}, - resource::{Buffer, QuerySet, Sampler, Texture, TextureView}, + resource::{Buffer, QuerySet, Sampler, Texture, TextureClearMode, TextureView}, Epoch, Index, }; @@ -644,21 +644,20 @@ impl Hub { for element in self.textures.data.write().map.drain(..) { if let Element::Occupied(texture, _) = element { let device = &devices[texture.device_id.value]; - - for layer_clear_views in texture.clear_views { - for view in layer_clear_views { - if let Some(view) = view { + if let TextureInner::Native { raw: Some(raw) } = texture.inner { + unsafe { + device.raw.destroy_texture(raw); + } + } + match texture.clear_mode { + TextureClearMode::RenderPass(clear_views) => { + for view in clear_views { unsafe { device.raw.destroy_texture_view(view); } } } - } - - if let TextureInner::Native { raw: Some(raw) } = texture.inner { - unsafe { - device.raw.destroy_texture(raw); - } + _ => {} } } } diff --git a/wgpu-core/src/present.rs b/wgpu-core/src/present.rs index e0f20fef09..c13b8923cb 100644 --- a/wgpu-core/src/present.rs +++ b/wgpu-core/src/present.rs @@ -22,7 +22,6 @@ use crate::{ LifeGuard, Stored, }; -use arrayvec::ArrayVec; use hal::{Queue as _, Surface as _}; use thiserror::Error; use wgt::SurfaceStatus as Status; @@ -159,7 +158,7 @@ impl Global { levels: 0..1, }, life_guard: LifeGuard::new(""), - clear_views: ArrayVec::new(), + clear_mode: resource::TextureClearMode::None, // TODO: Shouldn't there be a view? }; let ref_count = texture.life_guard.add_ref(); diff --git a/wgpu-core/src/resource.rs b/wgpu-core/src/resource.rs index 8f5fb45673..5d92c71989 100644 --- a/wgpu-core/src/resource.rs +++ b/wgpu-core/src/resource.rs @@ -8,17 +8,10 @@ use crate::{ Label, LifeGuard, RefCount, Stored, }; -use arrayvec::ArrayVec; -use hal::Device; use smallvec::SmallVec; use thiserror::Error; -use std::{ - borrow::Borrow, - num::{NonZeroU32, NonZeroU8}, - ops::Range, - ptr::NonNull, -}; +use std::{borrow::Borrow, num::NonZeroU8, ops::Range, ptr::NonNull}; #[repr(C)] #[derive(Debug)] @@ -186,6 +179,16 @@ impl TextureInner { } } +#[derive(Debug)] +pub enum TextureClearMode { + BufferCopy, + // View for clear via RenderPass for every subsurface + RenderPass(SmallVec<[A::TextureView; 1]>), + // Texture can't be cleared, attempting to do so will cause panic. + // (either because it is impossible for the type of texture or it is being destroyed) + None, +} + #[derive(Debug)] pub struct Texture { pub(crate) inner: TextureInner, @@ -196,70 +199,23 @@ pub struct Texture { pub(crate) initialization_status: TextureInitTracker, pub(crate) full_range: TextureSelector, pub(crate) life_guard: LifeGuard, - pub(crate) clear_views: - ArrayVec; 1]>, { hal::MAX_MIP_LEVELS as usize }>, + pub(crate) clear_mode: TextureClearMode, } impl Texture { - pub(crate) fn get_or_create_clear_view( - &mut self, - device: &A::Device, - mip_level: u32, - layer: u32, - ) -> Result<&A::TextureView, DeviceError> { - if self.clear_views.len() != self.desc.mip_level_count as usize { - self.clear_views.extend( - std::iter::repeat_with(|| SmallVec::new()).take(self.desc.mip_level_count as usize), - ); - } - - let layers = &mut self.clear_views[mip_level as usize]; - - if layers.len() != self.desc.size.depth_or_array_layers as usize { - layers.extend( - std::iter::repeat_with(|| None).take(self.desc.size.depth_or_array_layers as usize), - ); + pub(crate) fn get_clear_view(&self, mip_level: u32, depth_or_layer: u32) -> &A::TextureView { + match &self.clear_mode { + TextureClearMode::BufferCopy => { + panic!("Given texture is cleared with buffer copies, not render passes") + } + TextureClearMode::None => { + panic!("Given texture can't be cleared") + } + TextureClearMode::RenderPass(clear_views) => { + let index = mip_level + depth_or_layer * self.desc.mip_level_count; + &clear_views[index as usize] + } } - - let view = &mut layers[layer as usize]; - - if view.is_none() { - let raw_texture = match self.inner.as_raw() { - Some(raw) => raw, - None => panic!("can't clear surface texture"), - }; - - *view = Some(unsafe { - device.create_texture_view( - raw_texture, - &hal::TextureViewDescriptor { - label: Some("clear texture view"), - format: self.desc.format, - dimension: match self.desc.dimension { - wgt::TextureDimension::D1 => wgt::TextureViewDimension::D1, - wgt::TextureDimension::D2 => wgt::TextureViewDimension::D2, - wgt::TextureDimension::D3 => panic!("Can't create clear view for D3 textures. Use clear via copy instead."), - }, - usage: if self.desc.format.describe().sample_type - == wgt::TextureSampleType::Depth - { - hal::TextureUses::DEPTH_STENCIL_WRITE - } else { - hal::TextureUses::COLOR_TARGET - }, - range: wgt::ImageSubresourceRange { - aspect: wgt::TextureAspect::All, - base_mip_level: mip_level, - mip_level_count: NonZeroU32::new(1), - base_array_layer: layer, - array_layer_count: NonZeroU32::new(1), - }, - }, - )? - }); - } - - Ok(view.as_mut().unwrap()) } } From e3e530fb886c6e16dc190f1d23de5a02a5765b7e Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Sun, 21 Nov 2021 20:28:17 +0100 Subject: [PATCH 08/27] discarded surface fixup goes through new clear_texture method now --- wgpu-core/src/command/clear.rs | 31 ++++++++++----------- wgpu-core/src/command/memory_init.rs | 41 ++++++---------------------- 2 files changed, 24 insertions(+), 48 deletions(-) diff --git a/wgpu-core/src/command/clear.rs b/wgpu-core/src/command/clear.rs index df5bd37e02..2df5000b4a 100644 --- a/wgpu-core/src/command/clear.rs +++ b/wgpu-core/src/command/clear.rs @@ -11,7 +11,7 @@ use crate::{ id::{self, BufferId, CommandEncoderId, DeviceId, TextureId}, init_tracker::MemoryInitKind, resource::{Texture, TextureClearMode}, - track::TextureSelector, + track::{ResourceTracker, TextureSelector, TextureState}, }; use hal::CommandEncoder as _; @@ -221,29 +221,30 @@ impl Global { } clear_texture( - dst_texture, - cmd_buf, dst, + dst_texture, subresource_range.base_mip_level..subresource_level_end, subresource_range.base_array_layer..subresource_layer_end, + cmd_buf.encoder.open(), + &mut cmd_buf.trackers.textures, &device_guard[cmd_buf.device_id.value], ) } } -fn clear_texture( +pub(crate) fn clear_texture( + dst_texture_id: TextureId, dst_texture: &Texture, - cmd_buf: &mut CommandBuffer, - dst: TextureId, mip_range: Range, layer_range: Range, + encoder: &mut A::CommandEncoder, + texture_tracker: &mut ResourceTracker, device: &Device, ) -> Result<(), ClearError> { - let encoder = cmd_buf.encoder.open(); let dst_raw = dst_texture .inner .as_raw() - .ok_or(ClearError::InvalidTexture(dst))?; + .ok_or(ClearError::InvalidTexture(dst_texture_id))?; // Issue the right barrier. let clear_usage = match dst_texture.clear_mode { @@ -259,14 +260,12 @@ fn clear_texture( } } TextureClearMode::None => { - return Err(ClearError::NoValidTextureClearMode(dst)); + return Err(ClearError::NoValidTextureClearMode(dst_texture_id)); } }; - let dst_barrier = cmd_buf - .trackers - .textures + let dst_barrier = texture_tracker .change_replace( - id::Valid(dst), + id::Valid(dst_texture_id), dst_texture.life_guard().ref_count.as_ref().unwrap(), TextureSelector { levels: mip_range.clone(), @@ -297,13 +296,13 @@ fn clear_texture( encoder, )?, TextureClearMode::None => { - return Err(ClearError::NoValidTextureClearMode(dst)); + return Err(ClearError::NoValidTextureClearMode(dst_texture_id)); } } Ok(()) } -fn clear_texture_via_buffer_copies( +fn clear_texture_via_buffer_copies( texture_desc: &wgt::TextureDescriptor<()>, device: &Device, mip_range: Range, @@ -404,7 +403,7 @@ pub(crate) fn collect_zero_buffer_copies_for_clear_texture( } } -fn clear_texture_via_render_passes( +fn clear_texture_via_render_passes( dst_texture: &Texture, mip_range: Range, layer_range: Range, diff --git a/wgpu-core/src/command/memory_init.rs b/wgpu-core/src/command/memory_init.rs index 8167d7cdd9..bad04493e5 100644 --- a/wgpu-core/src/command/memory_init.rs +++ b/wgpu-core/src/command/memory_init.rs @@ -13,7 +13,7 @@ use crate::{ FastHashMap, }; -use super::{BakedCommands, DestroyedBufferError, DestroyedTextureError}; +use super::{clear::clear_texture, BakedCommands, DestroyedBufferError, DestroyedTextureError}; /// Surface that was discarded by `StoreOp::Discard` of a preceding renderpass. /// Any read access to this surface needs to be preceded by a texture initialization. @@ -131,43 +131,20 @@ pub(crate) fn fixup_discarded_surfaces< texture_tracker: &mut ResourceTracker, device: &Device, ) { - let mut zero_buffer_copy_regions = Vec::new(); for init in inits { let mip_range = init.mip_level..(init.mip_level + 1); let layer_range = init.layer..(init.layer + 1); - let (texture, pending) = texture_tracker - .use_replace( - &*texture_guard, - init.texture, - TextureSelector { - levels: mip_range.clone(), - layers: layer_range.clone(), - }, - hal::TextureUses::COPY_DST, - ) - .unwrap(); - - collect_zero_buffer_copies_for_clear_texture( - &texture.desc, - device.alignments.buffer_copy_pitch.get() as u32, + clear_texture( + init.texture, + texture_guard.get(init.texture).unwrap(), mip_range, layer_range, - &mut zero_buffer_copy_regions, - ); - - let barriers = pending.map(|pending| pending.into_hal(texture)); - let raw_texture = texture.inner.as_raw().unwrap(); - - unsafe { - // TODO: Should first gather all barriers, do a single transition_textures call, and then send off copy_buffer_to_texture commands. - encoder.transition_textures(barriers); - encoder.copy_buffer_to_texture( - &device.zero_buffer, - raw_texture, - zero_buffer_copy_regions.drain(..), - ); - } + encoder, + texture_tracker, + device, + ) + .unwrap(); } } From a760ddd312566f010fff32f0277efad74eb2d23b Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Sun, 12 Dec 2021 18:19:45 +0100 Subject: [PATCH 09/27] onq ueue texture initialization now goes threw clear_texture pending inits need to store Stored textures now though, causing more ref count bumping --- wgpu-core/src/command/clear.rs | 19 +++-- wgpu-core/src/command/memory_init.rs | 102 +++++++++----------------- wgpu-core/src/command/render.rs | 14 ++-- wgpu-core/src/command/transfer.rs | 19 ++++- wgpu-core/src/device/mod.rs | 7 +- wgpu-core/src/init_tracker/texture.rs | 6 +- 6 files changed, 74 insertions(+), 93 deletions(-) diff --git a/wgpu-core/src/command/clear.rs b/wgpu-core/src/command/clear.rs index 2df5000b4a..44abf8f62a 100644 --- a/wgpu-core/src/command/clear.rs +++ b/wgpu-core/src/command/clear.rs @@ -12,6 +12,7 @@ use crate::{ init_tracker::MemoryInitKind, resource::{Texture, TextureClearMode}, track::{ResourceTracker, TextureSelector, TextureState}, + Stored, }; use hal::CommandEncoder as _; @@ -221,7 +222,10 @@ impl Global { } clear_texture( - dst, + &Stored { + value: id::Valid(dst), + ref_count: dst_texture.life_guard().ref_count.as_ref().unwrap().clone(), + }, dst_texture, subresource_range.base_mip_level..subresource_level_end, subresource_range.base_array_layer..subresource_layer_end, @@ -233,7 +237,7 @@ impl Global { } pub(crate) fn clear_texture( - dst_texture_id: TextureId, + dst_texture_id: &Stored, dst_texture: &Texture, mip_range: Range, layer_range: Range, @@ -244,7 +248,7 @@ pub(crate) fn clear_texture( let dst_raw = dst_texture .inner .as_raw() - .ok_or(ClearError::InvalidTexture(dst_texture_id))?; + .ok_or(ClearError::InvalidTexture(dst_texture_id.value.0))?; // Issue the right barrier. let clear_usage = match dst_texture.clear_mode { @@ -260,13 +264,14 @@ pub(crate) fn clear_texture( } } TextureClearMode::None => { - return Err(ClearError::NoValidTextureClearMode(dst_texture_id)); + return Err(ClearError::NoValidTextureClearMode(dst_texture_id.value.0)); } }; + let dst_barrier = texture_tracker .change_replace( - id::Valid(dst_texture_id), - dst_texture.life_guard().ref_count.as_ref().unwrap(), + dst_texture_id.value, + &dst_texture_id.ref_count, TextureSelector { levels: mip_range.clone(), layers: layer_range.clone(), @@ -296,7 +301,7 @@ pub(crate) fn clear_texture( encoder, )?, TextureClearMode::None => { - return Err(ClearError::NoValidTextureClearMode(dst_texture_id)); + return Err(ClearError::NoValidTextureClearMode(dst_texture_id.value.0)); } } Ok(()) diff --git a/wgpu-core/src/command/memory_init.rs b/wgpu-core/src/command/memory_init.rs index bad04493e5..94ed47cda0 100644 --- a/wgpu-core/src/command/memory_init.rs +++ b/wgpu-core/src/command/memory_init.rs @@ -3,14 +3,13 @@ use std::{collections::hash_map::Entry, ops::Range, vec::Drain}; use hal::CommandEncoder; 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, + track::{ResourceTracker, TextureState, TrackerSet}, + FastHashMap, Stored, }; use super::{clear::clear_texture, BakedCommands, DestroyedBufferError, DestroyedTextureError}; @@ -19,7 +18,7 @@ use super::{clear::clear_texture, BakedCommands, DestroyedBufferError, Destroyed /// Any read access to this surface needs to be preceded by a texture initialization. #[derive(Clone)] pub(crate) struct TextureSurfaceDiscard { - pub texture: TextureId, + pub texture: Stored, pub mip_level: u32, pub layer: u32, } @@ -54,6 +53,7 @@ impl CommandBufferTextureMemoryActions { texture_guard: &Storage, TextureId>, ) -> SurfacesInDiscardState { let mut immediately_necessary_clears = SurfacesInDiscardState::new(); + let texture_id = action.id.value.0; // Note that within a command buffer we may stack arbitrary memory init actions on the same texture // Since we react to them in sequence, they are going to be dropped again at queue submit @@ -61,7 +61,7 @@ impl CommandBufferTextureMemoryActions { // We don't need to add MemoryInitKind::NeedsInitializedMemory to init_actions if a surface is part of the discard list. // But that would mean splitting up the action which is more than we'd win here. self.init_actions - .extend(match texture_guard.get(action.id) { + .extend(match texture_guard.get(texture_id) { Ok(texture) => texture.initialization_status.check_action(action), Err(_) => return immediately_necessary_clears, // texture no longer exists }); @@ -70,7 +70,7 @@ impl CommandBufferTextureMemoryActions { // (i.e. most of the time self.discards is empty!) let init_actions = &mut self.init_actions; self.discards.retain(|discarded_surface| { - if discarded_surface.texture == action.id + if discarded_surface.texture.value.0 == texture_id && action.range.layer_range.contains(&discarded_surface.layer) && action .range @@ -82,7 +82,7 @@ impl CommandBufferTextureMemoryActions { // Mark surface as implicitly initialized (this is relevant because it might have been uninitialized prior to discarding init_actions.push(TextureInitTrackerAction { - id: discarded_surface.texture, + id: discarded_surface.texture.clone(), range: TextureInitRange { mip_range: discarded_surface.mip_level ..(discarded_surface.mip_level + 1), @@ -103,7 +103,7 @@ impl CommandBufferTextureMemoryActions { // Shortcut for register_init_action when it is known that the action is an implicit init, not requiring any immediate resource init. pub(crate) fn register_implicit_init( &mut self, - id: TextureId, + id: Stored, range: TextureInitRange, texture_guard: &Storage, TextureId>, ) { @@ -136,8 +136,8 @@ pub(crate) fn fixup_discarded_surfaces< let layer_range = init.layer..(init.layer + 1); clear_texture( - init.texture, - texture_guard.get(init.texture).unwrap(), + &init.texture, + texture_guard.get(init.texture.value.0).unwrap(), mip_range, layer_range, encoder, @@ -243,8 +243,8 @@ impl BakedCommands { let mut ranges: Vec = Vec::new(); for texture_use in self.texture_memory_actions.drain_init_actions() { let texture = texture_guard - .get_mut(texture_use.id) - .map_err(|_| DestroyedTextureError(texture_use.id))?; + .get_mut(texture_use.id.value.0) + .map_err(|_| DestroyedTextureError(texture_use.id.value.0))?; let use_range = texture_use.range; let affected_mip_trackers = texture @@ -262,73 +262,37 @@ impl BakedCommands { } } MemoryInitKind::NeedsInitializedMemory => { - ranges.clear(); for (mip_level, mip_tracker) in affected_mip_trackers { - for layer_range in mip_tracker.drain(use_range.layer_range.clone()) { - ranges.push(TextureInitRange { - mip_range: mip_level as u32..(mip_level as u32 + 1), + ranges.extend(mip_tracker.drain(use_range.layer_range.clone()).map( + |layer_range| TextureInitRange { + mip_range: (mip_level as u32)..(mip_level as u32 + 1), layer_range, - }) - } - } - - let raw_texture = texture - .inner - .as_raw() - .ok_or(DestroyedTextureError(texture_use.id))?; - - let mut texture_barriers = Vec::new(); - let mut zero_buffer_copy_regions = Vec::new(); - for range in &ranges { - // Don't do use_replace since the texture may already no longer have a ref_count. - // However, we *know* that it is currently in use, so the tracker must already know about it. - texture_barriers.extend( - device_tracker - .textures - .change_replace_tracked( - id::Valid(texture_use.id), - TextureSelector { - levels: range.mip_range.clone(), - layers: range.layer_range.clone(), - }, - hal::TextureUses::COPY_DST, - ) - .map(|pending| pending.into_hal(texture)), - ); - - collect_zero_buffer_copies_for_clear_texture( - &texture.desc, - device.alignments.buffer_copy_pitch.get() as u32, - range.mip_range.clone(), - range.layer_range.clone(), - &mut zero_buffer_copy_regions, - ); - } - - if !zero_buffer_copy_regions.is_empty() { - debug_assert!(texture.hal_usage.contains(hal::TextureUses::COPY_DST), - "Texture needs to have the COPY_DST flag. Otherwise we can't ensure initialized memory!"); - unsafe { - // TODO: Could safe on transition_textures calls by bundling barriers from *all* textures. - // (a bbit more tricky because a naive approach would have to borrow same texture several times then) - self.encoder - .transition_textures(texture_barriers.into_iter()); - self.encoder.copy_buffer_to_texture( - &device.zero_buffer, - raw_texture, - zero_buffer_copy_regions.into_iter(), - ); - } + }, + )); } } } + + // TODO: Could we attempt some range collapsing here? + for range in ranges.drain(..) { + clear_texture( + &texture_use.id, + &*texture, + range.mip_range, + range.layer_range, + &mut self.encoder, + &mut device_tracker.textures, + device, + ) + .unwrap(); + } } // Now that all buffers/textures have the proper init state for before cmdbuf start, we discard init states for textures it left discarded after its execution. for surface_discard in self.texture_memory_actions.discards.iter() { let texture = texture_guard - .get_mut(surface_discard.texture) - .map_err(|_| DestroyedTextureError(surface_discard.texture))?; + .get_mut(surface_discard.texture.value.0) + .map_err(|_| DestroyedTextureError(surface_discard.texture.value.0))?; texture .initialization_status .discard(surface_discard.mip_level, surface_discard.layer); diff --git a/wgpu-core/src/command/render.rs b/wgpu-core/src/command/render.rs index 10ec405ee2..8e8b33d617 100644 --- a/wgpu-core/src/command/render.rs +++ b/wgpu-core/src/command/render.rs @@ -568,7 +568,7 @@ impl<'a, A: HalApi> RenderPassInfo<'a, A> { if channel.load_op == LoadOp::Load { pending_discard_init_fixups.extend(texture_memory_actions.register_init_action( &TextureInitTrackerAction { - id: view.parent_id.value.0, + id: view.parent_id.clone(), range: TextureInitRange::from(view.selector.clone()), // Note that this is needed even if the target is discarded, kind: MemoryInitKind::NeedsInitializedMemory, @@ -578,7 +578,7 @@ impl<'a, A: HalApi> RenderPassInfo<'a, A> { } else if channel.store_op == StoreOp::Store { // Clear + Store texture_memory_actions.register_implicit_init( - view.parent_id.value.0, + view.parent_id.clone(), TextureInitRange::from(view.selector.clone()), texture_guard, ); @@ -587,7 +587,7 @@ impl<'a, A: HalApi> RenderPassInfo<'a, A> { // the discard happens at the *end* of a pass // but recording the discard right away be alright since the texture can't be used during the pass anyways texture_memory_actions.discard(TextureSurfaceDiscard { - texture: view.parent_id.value.0, + texture: view.parent_id.clone(), mip_level: view.selector.levels.start, layer: view.selector.layers.start, }); @@ -729,7 +729,7 @@ impl<'a, A: HalApi> RenderPassInfo<'a, A> { pending_discard_init_fixups.extend( cmd_buf.texture_memory_actions.register_init_action( &TextureInitTrackerAction { - id: view.parent_id.value.0, + id: view.parent_id.clone(), range: TextureInitRange::from(view.selector.clone()), kind: MemoryInitKind::NeedsInitializedMemory, }, @@ -745,7 +745,7 @@ impl<'a, A: HalApi> RenderPassInfo<'a, A> { if at.depth.store_op != at.stencil.store_op { if !need_init_beforehand { cmd_buf.texture_memory_actions.register_implicit_init( - view.parent_id.value.0, + view.parent_id.clone(), TextureInitRange::from(view.selector.clone()), texture_guard, ); @@ -761,7 +761,7 @@ impl<'a, A: HalApi> RenderPassInfo<'a, A> { } else if at.depth.store_op == StoreOp::Discard { // Both are discarded using the regular path. discarded_surfaces.push(TextureSurfaceDiscard { - texture: view.parent_id.value.0, + texture: view.parent_id.clone(), mip_level: view.selector.levels.start, layer: view.selector.layers.start, }); @@ -838,7 +838,7 @@ impl<'a, A: HalApi> RenderPassInfo<'a, A> { } cmd_buf.texture_memory_actions.register_implicit_init( - resolve_view.parent_id.value.0, + resolve_view.parent_id.clone(), TextureInitRange::from(resolve_view.selector.clone()), texture_guard, ); diff --git a/wgpu-core/src/command/transfer.rs b/wgpu-core/src/command/transfer.rs index 7a533656cb..3f38c1fe54 100644 --- a/wgpu-core/src/command/transfer.rs +++ b/wgpu-core/src/command/transfer.rs @@ -9,10 +9,11 @@ use crate::{ device::Device, error::{ErrorFormatter, PrettyError}, hub::{Global, GlobalIdentityHandlerFactory, HalApi, Storage, Token}, - id::{BufferId, CommandEncoderId, TextureId}, + id::{self, BufferId, CommandEncoderId, TextureId}, init_tracker::{MemoryInitKind, TextureInitRange, TextureInitTrackerAction}, resource::{Texture, TextureErrorDimension}, track::TextureSelector, + Stored, }; use hal::CommandEncoder as _; @@ -388,7 +389,10 @@ fn get_copy_dst_texture_init_requirement( MemoryInitKind::NeedsInitializedMemory }; TextureInitTrackerAction { - id: copy_texture.texture, + id: Stored { + value: id::Valid(copy_texture.texture), + ref_count: texture.life_guard.ref_count.as_ref().unwrap().clone(), // For copies the texture object isn't discarded yet! + }, range: TextureInitRange { mip_range: copy_texture.mip_level..copy_texture.mip_level + 1, layer_range: copy_texture.origin.z @@ -406,9 +410,18 @@ fn handle_src_texture_init( copy_size: &Extent3d, texture_guard: &Storage, TextureId>, ) { + let id = id::Valid(source.texture); let immediate_src_init = cmd_buf.texture_memory_actions.register_init_action( &TextureInitTrackerAction { - id: source.texture, + id: Stored { + value: id, + ref_count: texture_guard[id] + .life_guard + .ref_count + .as_ref() + .unwrap() + .clone(), // For transfers the texture object isn't discarded yet! + }, range: TextureInitRange { mip_range: src_base.mip_level..src_base.mip_level + 1, layer_range: src_base.origin.z diff --git a/wgpu-core/src/device/mod.rs b/wgpu-core/src/device/mod.rs index 137b9bc4e0..4ca846473b 100644 --- a/wgpu-core/src/device/mod.rs +++ b/wgpu-core/src/device/mod.rs @@ -1519,11 +1519,10 @@ impl Device { ) -> Result<(), binding_model::CreateBindGroupError> { // Careful here: the texture may no longer have its own ref count, // if it was deleted by the user. - let parent_id = view.parent_id.value; - let texture = &texture_guard[parent_id]; + let texture = &texture_guard[view.parent_id.value]; used.textures .change_extend( - parent_id, + view.parent_id.value, &view.parent_id.ref_count, view.selector.clone(), internal_use, @@ -1532,7 +1531,7 @@ impl Device { check_texture_usage(texture.desc.usage, pub_usage)?; used_texture_ranges.push(TextureInitTrackerAction { - id: parent_id.0, + id: view.parent_id.clone(), range: TextureInitRange { mip_range: view.desc.range.mip_range(&texture.desc), layer_range: view.desc.range.layer_range(&texture.desc), diff --git a/wgpu-core/src/init_tracker/texture.rs b/wgpu-core/src/init_tracker/texture.rs index e8603a9922..26e313020f 100644 --- a/wgpu-core/src/init_tracker/texture.rs +++ b/wgpu-core/src/init_tracker/texture.rs @@ -1,5 +1,5 @@ use super::{InitTracker, MemoryInitKind}; -use crate::{id::TextureId, track::TextureSelector}; +use crate::{id::TextureId, track::TextureSelector, Stored}; use arrayvec::ArrayVec; use std::ops::Range; @@ -20,7 +20,7 @@ impl From for TextureInitRange { #[derive(Debug, Clone)] pub(crate) struct TextureInitTrackerAction { - pub(crate) id: TextureId, + pub(crate) id: Stored, pub(crate) range: TextureInitRange, pub(crate) kind: MemoryInitKind, } @@ -69,7 +69,7 @@ impl TextureInitTracker { if mip_range_start < mip_range_end && layer_range_start < layer_range_end { Some(TextureInitTrackerAction { - id: action.id, + id: action.id.clone(), range: TextureInitRange { mip_range: mip_range_start as u32..mip_range_end as u32, layer_range: layer_range_start..layer_range_end, From ad5ed2cd6988076cfac073ffb65876e2fca13064 Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Sun, 12 Dec 2021 19:44:57 +0100 Subject: [PATCH 10/27] texture init on queue_write_texture now also goes through new clear_texture --- wgpu-core/src/command/clear.rs | 34 +++++++-- wgpu-core/src/command/mod.rs | 3 +- wgpu-core/src/device/queue.rs | 132 ++++++++++++++------------------- 3 files changed, 86 insertions(+), 83 deletions(-) diff --git a/wgpu-core/src/command/clear.rs b/wgpu-core/src/command/clear.rs index 44abf8f62a..57e8c70073 100644 --- a/wgpu-core/src/command/clear.rs +++ b/wgpu-core/src/command/clear.rs @@ -244,6 +244,28 @@ pub(crate) fn clear_texture( encoder: &mut A::CommandEncoder, texture_tracker: &mut ResourceTracker, device: &Device, +) -> Result<(), ClearError> { + clear_texture_no_device( + dst_texture_id, + dst_texture, + mip_range, + layer_range, + encoder, + texture_tracker, + &device.alignments, + &device.zero_buffer, + ) +} + +pub(crate) fn clear_texture_no_device( + dst_texture_id: &Stored, + dst_texture: &Texture, + mip_range: Range, + layer_range: Range, + encoder: &mut A::CommandEncoder, + texture_tracker: &mut ResourceTracker, + alignments: &hal::Alignments, + zero_buffer: &A::Buffer, ) -> Result<(), ClearError> { let dst_raw = dst_texture .inner @@ -285,9 +307,10 @@ pub(crate) fn clear_texture( // Record actual clearing match dst_texture.clear_mode { - TextureClearMode::BufferCopy => clear_texture_via_buffer_copies( + TextureClearMode::BufferCopy => clear_texture_via_buffer_copies::( &dst_texture.desc, - device, + alignments, + zero_buffer, mip_range, layer_range, encoder, @@ -309,7 +332,8 @@ pub(crate) fn clear_texture( fn clear_texture_via_buffer_copies( texture_desc: &wgt::TextureDescriptor<()>, - device: &Device, + alignments: &hal::Alignments, + zero_buffer: &A::Buffer, mip_range: Range, layer_range: Range, encoder: &mut A::CommandEncoder, @@ -318,7 +342,7 @@ fn clear_texture_via_buffer_copies( let mut zero_buffer_copy_regions = Vec::new(); collect_zero_buffer_copies_for_clear_texture( &texture_desc, - device.alignments.buffer_copy_pitch.get() as u32, + alignments.buffer_copy_pitch.get() as u32, mip_range, layer_range, &mut zero_buffer_copy_regions, @@ -326,7 +350,7 @@ fn clear_texture_via_buffer_copies( if !zero_buffer_copy_regions.is_empty() { unsafe { encoder.copy_buffer_to_texture( - &device.zero_buffer, + zero_buffer, dst_raw, zero_buffer_copy_regions.into_iter(), ); diff --git a/wgpu-core/src/command/mod.rs b/wgpu-core/src/command/mod.rs index e210fb3c6b..89bdd77dd5 100644 --- a/wgpu-core/src/command/mod.rs +++ b/wgpu-core/src/command/mod.rs @@ -9,8 +9,9 @@ mod render; mod transfer; pub use self::bundle::*; +pub(crate) use self::clear::clear_texture_no_device; pub(crate) use self::clear::collect_zero_buffer_copies_for_clear_texture; -pub use self::clear::ClearError; +pub(crate) use self::clear::ClearError; pub use self::compute::*; pub use self::draw::*; use self::memory_init::CommandBufferTextureMemoryActions; diff --git a/wgpu-core/src/device/queue.rs b/wgpu-core/src/device/queue.rs index ced599a8ec..92427fec25 100644 --- a/wgpu-core/src/device/queue.rs +++ b/wgpu-core/src/device/queue.rs @@ -4,7 +4,7 @@ use crate::{ align_to, command::{ extract_texture_selector, validate_linear_texture_data, validate_texture_copy_range, - CommandBuffer, CopySide, ImageCopyTexture, TransferError, + ClearError, CommandBuffer, CopySide, ImageCopyTexture, TransferError, }, conv, device::{DeviceError, WaitIdleError}, @@ -12,7 +12,7 @@ use crate::{ hub::{Global, GlobalIdentityHandlerFactory, HalApi, Token}, id, resource::{BufferAccessError, BufferMapState, TextureInner}, - track, FastHashSet, + track, FastHashSet, Stored, }; use hal::{CommandEncoder as _, Device as _, Queue as _}; @@ -216,6 +216,8 @@ pub enum QueueWriteError { Queue(#[from] DeviceError), #[error(transparent)] Transfer(#[from] TransferError), + #[error(transparent)] + Clear(#[from] ClearError), } #[derive(Clone, Debug, Error)] @@ -377,7 +379,7 @@ impl Global { return Ok(()); } - let (texture_guard, _) = hub.textures.read(&mut token); + let (mut texture_guard, _) = hub.textures.write(&mut token); // For clear we need write access to the texture. TODO: Can we acquire write lock later? let (selector, dst_base, texture_format) = extract_texture_selector(destination, size, &*texture_guard)?; let format_desc = texture_format.describe(); @@ -422,7 +424,51 @@ impl Global { let stage_size = stage_bytes_per_row as u64 * block_rows_in_copy as u64; let stage = device.prepare_stage(stage_size)?; + let dst = texture_guard.get_mut(destination.texture).unwrap(); + if !dst.desc.usage.contains(wgt::TextureUsages::COPY_DST) { + return Err( + TransferError::MissingCopyDstUsageFlag(None, Some(destination.texture)).into(), + ); + } + let mut trackers = device.trackers.lock(); + let encoder = device.pending_writes.activate(); + + // If the copy does not fully cover the layers, we need to initialize to zero *first* as we don't keep track of partial texture layer inits. + // Strictly speaking we only need to clear the areas of a layer untouched, but this would get increasingly messy. + + let init_layer_range = + destination.origin.z..destination.origin.z + size.depth_or_array_layers; + if dst.initialization_status.mips[destination.mip_level as usize] + .check(init_layer_range.clone()) + .is_some() + { + if size.width != dst.desc.size.width || size.height != dst.desc.size.height { + for layer_range in dst.initialization_status.mips[destination.mip_level as usize] + .drain(init_layer_range) + .collect::>>() + { + crate::command::clear_texture_no_device( + &Stored { + value: id::Valid(destination.texture), + ref_count: dst.life_guard.ref_count.as_ref().unwrap().clone(), + }, + &*dst, + destination.mip_level..(destination.mip_level + 1), + layer_range, + encoder, + &mut trackers.textures, + &device.alignments, + &device.zero_buffer, + ) + .map_err(QueueWriteError::from)?; + } + } else { + dst.initialization_status.mips[destination.mip_level as usize] + .drain(init_layer_range); + } + } + let (dst, transition) = trackers .textures .use_replace( @@ -433,11 +479,6 @@ impl Global { ) .unwrap(); - if !dst.desc.usage.contains(wgt::TextureUsages::COPY_DST) { - return Err( - TransferError::MissingCopyDstUsageFlag(None, Some(destination.texture)).into(), - ); - } let (hal_copy_size, array_layer_count) = validate_texture_copy_range(destination, &dst.desc, CopySide::Destination, size)?; dst.life_guard.use_at(device.active_submission_index + 1); @@ -512,78 +553,15 @@ impl Global { usage: hal::BufferUses::MAP_WRITE..hal::BufferUses::COPY_SRC, }; - let encoder = device.pending_writes.activate(); + let dst_raw = dst + .inner + .as_raw() + .ok_or(TransferError::InvalidTexture(destination.texture))?; + unsafe { encoder.transition_textures(transition.map(|pending| pending.into_hal(dst))); encoder.transition_buffers(iter::once(barrier)); - } - - // If the copy does not fully cover the layers, we need to initialize to zero *first* as we don't keep track of partial texture layer inits. - // Strictly speaking we only need to clear the areas of a layer untouched, but this would get increasingly messy. - - let init_layer_range = - destination.origin.z..destination.origin.z + size.depth_or_array_layers; - if dst.initialization_status.mips[destination.mip_level as usize] - .check(init_layer_range.clone()) - .is_some() - { - // For clear we need write access to the texture! - drop(texture_guard); - let (mut texture_guard, _) = hub.textures.write(&mut token); - let dst = texture_guard.get_mut(destination.texture).unwrap(); - let dst_raw = dst - .inner - .as_raw() - .ok_or(TransferError::InvalidTexture(destination.texture))?; - - let layers_to_initialize = dst.initialization_status.mips - [destination.mip_level as usize] - .drain(init_layer_range); - - let mut zero_buffer_copy_regions = Vec::new(); - if size.width != dst.desc.size.width || size.height != dst.desc.size.height { - for layer in layers_to_initialize { - crate::command::collect_zero_buffer_copies_for_clear_texture( - &dst.desc, - device.alignments.buffer_copy_pitch.get() as u32, - destination.mip_level..(destination.mip_level + 1), - layer, - &mut zero_buffer_copy_regions, - ); - } - } - unsafe { - if !zero_buffer_copy_regions.is_empty() { - encoder.copy_buffer_to_texture( - &device.zero_buffer, - dst_raw, - zero_buffer_copy_regions.iter().cloned(), - ); - encoder.transition_textures(zero_buffer_copy_regions.iter().map(|copy| { - hal::TextureBarrier { - texture: dst_raw, - range: wgt::ImageSubresourceRange { - aspect: wgt::TextureAspect::All, - base_mip_level: copy.texture_base.mip_level, - mip_level_count: NonZeroU32::new(1), - base_array_layer: copy.texture_base.array_layer, - array_layer_count: NonZeroU32::new(1), - }, - usage: hal::TextureUses::COPY_DST..hal::TextureUses::COPY_DST, - } - })); - } - encoder.copy_buffer_to_texture(&stage.buffer, dst_raw, regions); - } - } else { - let dst_raw = dst - .inner - .as_raw() - .ok_or(TransferError::InvalidTexture(destination.texture))?; - - unsafe { - encoder.copy_buffer_to_texture(&stage.buffer, dst_raw, regions); - } + encoder.copy_buffer_to_texture(&stage.buffer, dst_raw, regions); } device.pending_writes.consume(stage); From 35a520fbdb0e3c4f2f9e0e31e7053aff8a43427c Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Sun, 19 Dec 2021 13:52:22 +0100 Subject: [PATCH 11/27] transfer functions on commandbuffer use now new texture init route --- wgpu-core/src/command/mod.rs | 1 - wgpu-core/src/command/transfer.rs | 202 +++++++++++++----------------- wgpu-core/src/device/queue.rs | 2 +- 3 files changed, 90 insertions(+), 115 deletions(-) diff --git a/wgpu-core/src/command/mod.rs b/wgpu-core/src/command/mod.rs index 89bdd77dd5..52d4a59ac8 100644 --- a/wgpu-core/src/command/mod.rs +++ b/wgpu-core/src/command/mod.rs @@ -10,7 +10,6 @@ mod transfer; pub use self::bundle::*; pub(crate) use self::clear::clear_texture_no_device; -pub(crate) use self::clear::collect_zero_buffer_copies_for_clear_texture; pub(crate) use self::clear::ClearError; pub use self::compute::*; pub use self::draw::*; diff --git a/wgpu-core/src/command/transfer.rs b/wgpu-core/src/command/transfer.rs index 3f38c1fe54..154424ef48 100644 --- a/wgpu-core/src/command/transfer.rs +++ b/wgpu-core/src/command/transfer.rs @@ -1,10 +1,7 @@ #[cfg(feature = "trace")] use crate::device::trace::Command as TraceCommand; use crate::{ - command::{ - collect_zero_buffer_copies_for_clear_texture, memory_init::fixup_discarded_surfaces, - CommandBuffer, CommandEncoderError, - }, + command::{memory_init::fixup_discarded_surfaces, CommandBuffer, CommandEncoderError}, conv, device::Device, error::{ErrorFormatter, PrettyError}, @@ -105,6 +102,8 @@ pub enum TransferError { src_format: wgt::TextureFormat, dst_format: wgt::TextureFormat, }, + #[error(transparent)] + MemoryInitFailure(#[from] super::ClearError), } impl PrettyError for TransferError { @@ -375,72 +374,103 @@ pub(crate) fn validate_texture_copy_range( Ok((copy_extent, array_layer_count)) } -fn get_copy_dst_texture_init_requirement( - texture: &Texture, - copy_texture: &wgt::ImageCopyTexture, +fn handle_texture_init( + init_kind: MemoryInitKind, + cmd_buf: &mut CommandBuffer, + device: &Device, + copy_texture: &ImageCopyTexture, copy_size: &Extent3d, -) -> TextureInitTrackerAction { - // Attention: If we don't write full texture subresources, we need to a full clear first since we don't track subrects. - let dst_init_kind = if copy_size.width == texture.desc.size.width - && copy_size.height == texture.desc.size.height - { - MemoryInitKind::ImplicitlyInitialized - } else { - MemoryInitKind::NeedsInitializedMemory - }; - TextureInitTrackerAction { + texture_guard: &Storage, id::Id>>, + texture: &Texture, +) { + let init_action = TextureInitTrackerAction { id: Stored { value: id::Valid(copy_texture.texture), - ref_count: texture.life_guard.ref_count.as_ref().unwrap().clone(), // For copies the texture object isn't discarded yet! + // For copies the texture object isn't discarded yet (somebody just passed it in!), so it should be safe to access the ref_count + ref_count: texture.life_guard.ref_count.as_ref().unwrap().clone(), }, range: TextureInitRange { mip_range: copy_texture.mip_level..copy_texture.mip_level + 1, layer_range: copy_texture.origin.z ..(copy_texture.origin.z + copy_size.depth_or_array_layers), }, - kind: dst_init_kind, + kind: init_kind, + }; + + // Register the init action. + let immediate_inits = cmd_buf + .texture_memory_actions + .register_init_action(&{ init_action }, texture_guard); + + // In rare cases we may need to insert an init operation immediately onto the command buffer. + if !immediate_inits.is_empty() { + let cmd_buf_raw = cmd_buf.encoder.open(); + fixup_discarded_surfaces( + immediate_inits.into_iter(), + cmd_buf_raw, + texture_guard, + &mut cmd_buf.trackers.textures, + device, + ); } } +// Ensures the source texture of a transfer is in the right initialization state and records the state for after the transfer operation. fn handle_src_texture_init( cmd_buf: &mut CommandBuffer, device: &Device, source: &ImageCopyTexture, - src_base: &hal::TextureCopyBase, copy_size: &Extent3d, texture_guard: &Storage, TextureId>, -) { - let id = id::Valid(source.texture); - let immediate_src_init = cmd_buf.texture_memory_actions.register_init_action( - &TextureInitTrackerAction { - id: Stored { - value: id, - ref_count: texture_guard[id] - .life_guard - .ref_count - .as_ref() - .unwrap() - .clone(), // For transfers the texture object isn't discarded yet! - }, - range: TextureInitRange { - mip_range: src_base.mip_level..src_base.mip_level + 1, - layer_range: src_base.origin.z - ..(src_base.origin.z + copy_size.depth_or_array_layers), - }, - kind: MemoryInitKind::NeedsInitializedMemory, - }, +) -> Result<(), TransferError> { + let texture = texture_guard + .get(source.texture) + .map_err(|_| TransferError::InvalidTexture(source.texture))?; + + handle_texture_init( + MemoryInitKind::NeedsInitializedMemory, + cmd_buf, + device, + source, + copy_size, texture_guard, + texture, ); - if !immediate_src_init.is_empty() { - let cmd_buf_raw = cmd_buf.encoder.open(); - fixup_discarded_surfaces( - immediate_src_init.into_iter(), - cmd_buf_raw, - texture_guard, - &mut cmd_buf.trackers.textures, - device, - ); - } + Ok(()) +} + +// Ensures the destination texture of a transfer is in the right initialization state and records the state for after the transfer operation. +fn handle_dst_texture_init( + cmd_buf: &mut CommandBuffer, + device: &Device, + destination: &ImageCopyTexture, + copy_size: &Extent3d, + texture_guard: &Storage, TextureId>, +) -> Result<(), TransferError> { + let texture = texture_guard + .get(destination.texture) + .map_err(|_| TransferError::InvalidTexture(destination.texture))?; + + // Attention: If we don't write full texture subresources, we need to a full clear first since we don't track subrects. + // This means that in rare cases even a *destination* texture of a transfer may need an immediate texture init. + let dst_init_kind = if copy_size.width == texture.desc.size.width + && copy_size.height == texture.desc.size.height + { + MemoryInitKind::ImplicitlyInitialized + } else { + MemoryInitKind::NeedsInitializedMemory + }; + + handle_texture_init( + dst_init_kind, + cmd_buf, + device, + destination, + copy_size, + texture_guard, + texture, + ); + Ok(()) } impl Global { @@ -611,6 +641,9 @@ impl Global { let (dst_range, dst_base, _) = extract_texture_selector(destination, copy_size, &*texture_guard)?; + // Handle texture init *before* dealing with barrier transitions so we have an easier time inserting "immediate-inits" that may be required by prior discards in rare cases. + handle_dst_texture_init(cmd_buf, device, destination, copy_size, &texture_guard)?; + let (src_buffer, src_pending) = cmd_buf .trackers .buffers @@ -676,19 +709,6 @@ impl Global { source.layout.offset..(source.layout.offset + required_buffer_bytes_in_copy), MemoryInitKind::NeedsInitializedMemory, )); - let mut dst_zero_buffer_copy_regions = Vec::new(); - for immediate_init in cmd_buf.texture_memory_actions.register_init_action( - &get_copy_dst_texture_init_requirement(dst_texture, destination, copy_size), - &texture_guard, - ) { - collect_zero_buffer_copies_for_clear_texture( - &dst_texture.desc, - device.alignments.buffer_copy_pitch.get() as u32, - immediate_init.mip_level..(immediate_init.mip_level + 1), - immediate_init.layer..(immediate_init.layer + 1), - &mut dst_zero_buffer_copy_regions, - ); - } let regions = (0..array_layer_count).map(|rel_array_layer| { let mut texture_base = dst_base.clone(); @@ -701,17 +721,10 @@ impl Global { size: hal_copy_size, } }); + let cmd_buf_raw = cmd_buf.encoder.open(); unsafe { cmd_buf_raw.transition_textures(dst_barriers); - // potential dst buffer init (for previously discarded dst_texture + partial copy) - if !dst_zero_buffer_copy_regions.is_empty() { - cmd_buf_raw.copy_buffer_to_texture( - &device.zero_buffer, - dst_raw, - dst_zero_buffer_copy_regions.into_iter(), - ); - } cmd_buf_raw.transition_buffers(src_barriers); cmd_buf_raw.copy_buffer_to_texture(src_raw, dst_raw, regions); } @@ -755,15 +768,8 @@ impl Global { let (src_range, src_base, _) = extract_texture_selector(source, copy_size, &*texture_guard)?; - // Handle src texture init *before* dealing with barrier transitions so we have an easier time inserting "immediate-inits" that may be required by prior discards in rare cases. - handle_src_texture_init( - cmd_buf, - device, - source, - &src_base, - copy_size, - &texture_guard, - ); + // Handle texture init *before* dealing with barrier transitions so we have an easier time inserting "immediate-inits" that may be required by prior discards in rare cases. + handle_src_texture_init(cmd_buf, device, source, copy_size, &texture_guard)?; let (src_texture, src_pending) = cmd_buf .trackers @@ -900,15 +906,9 @@ impl Global { return Err(TransferError::MismatchedAspects.into()); } - // Handle src texture init *before* dealing with barrier transitions so we have an easier time inserting "immediate-inits" that may be required by prior discards in rare cases. - handle_src_texture_init( - cmd_buf, - device, - source, - &src_tex_base, - copy_size, - &texture_guard, - ); + // Handle texture init *before* dealing with barrier transitions so we have an easier time inserting "immediate-inits" that may be required by prior discards in rare cases. + handle_src_texture_init(cmd_buf, device, source, copy_size, &texture_guard)?; + handle_dst_texture_init(cmd_buf, device, destination, copy_size, &texture_guard)?; let (src_texture, src_pending) = cmd_buf .trackers @@ -977,20 +977,6 @@ impl Global { copy_size, )?; - let mut dst_zero_buffer_copy_regions = Vec::new(); - for immediate_init in cmd_buf.texture_memory_actions.register_init_action( - &get_copy_dst_texture_init_requirement(dst_texture, destination, copy_size), - &texture_guard, - ) { - collect_zero_buffer_copies_for_clear_texture( - &dst_texture.desc, - device.alignments.buffer_copy_pitch.get() as u32, - immediate_init.mip_level..(immediate_init.mip_level + 1), - immediate_init.layer..(immediate_init.layer + 1), - &mut dst_zero_buffer_copy_regions, - ); - } - let hal_copy_size = hal::CopyExtent { width: src_copy_size.width.min(dst_copy_size.width), height: src_copy_size.height.min(dst_copy_size.height), @@ -1010,16 +996,6 @@ impl Global { let cmd_buf_raw = cmd_buf.encoder.open(); unsafe { cmd_buf_raw.transition_textures(barriers.into_iter()); - - // potential dst buffer init (for previously discarded dst_texture + partial copy) - if !dst_zero_buffer_copy_regions.is_empty() { - cmd_buf_raw.copy_buffer_to_texture( - &device.zero_buffer, - dst_raw, - dst_zero_buffer_copy_regions.into_iter(), - ); - } - cmd_buf_raw.copy_texture_to_texture( src_raw, hal::TextureUses::COPY_SRC, diff --git a/wgpu-core/src/device/queue.rs b/wgpu-core/src/device/queue.rs index 92427fec25..c58fa696c1 100644 --- a/wgpu-core/src/device/queue.rs +++ b/wgpu-core/src/device/queue.rs @@ -217,7 +217,7 @@ pub enum QueueWriteError { #[error(transparent)] Transfer(#[from] TransferError), #[error(transparent)] - Clear(#[from] ClearError), + MemoryInitFailure(#[from] ClearError), } #[derive(Clone, Debug, Error)] From d59a38b54bc501cabd7b38749dbcaecd3a7f1415 Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Sun, 19 Dec 2021 13:56:45 +0100 Subject: [PATCH 12/27] merge collect_zero_buffer_copies_for_clear_texture into clear_texture_via_buffer_copies --- wgpu-core/src/command/clear.rs | 157 ++++++++++++++++----------------- 1 file changed, 75 insertions(+), 82 deletions(-) diff --git a/wgpu-core/src/command/clear.rs b/wgpu-core/src/command/clear.rs index 57e8c70073..f970567123 100644 --- a/wgpu-core/src/command/clear.rs +++ b/wgpu-core/src/command/clear.rs @@ -333,102 +333,95 @@ pub(crate) fn clear_texture_no_device( fn clear_texture_via_buffer_copies( texture_desc: &wgt::TextureDescriptor<()>, alignments: &hal::Alignments, - zero_buffer: &A::Buffer, + zero_buffer: &A::Buffer, // Buffer of size device::ZERO_BUFFER_SIZE mip_range: Range, layer_range: Range, encoder: &mut A::CommandEncoder, dst_raw: &A::Texture, ) { - let mut zero_buffer_copy_regions = Vec::new(); - collect_zero_buffer_copies_for_clear_texture( - &texture_desc, - alignments.buffer_copy_pitch.get() as u32, - mip_range, - layer_range, - &mut zero_buffer_copy_regions, - ); - if !zero_buffer_copy_regions.is_empty() { - unsafe { - encoder.copy_buffer_to_texture( - zero_buffer, - dst_raw, - zero_buffer_copy_regions.into_iter(), + // Gather list of zero_buffer copies to clear a texture. + let zero_buffer_copy_regions = { + let texture_desc: &wgt::TextureDescriptor<()> = &texture_desc; + let buffer_copy_pitch = alignments.buffer_copy_pitch.get() as u32; + let format_desc = texture_desc.format.describe(); + let mut zero_buffer_copy_regions = Vec::new(); + + let bytes_per_row_alignment = + get_lowest_common_denom(buffer_copy_pitch, format_desc.block_size as u32); + + for mip_level in 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); + mip_size.height = align_to(mip_size.height, format_desc.block_dimensions.1 as u32); + + let bytes_per_row = align_to( + mip_size.width / format_desc.block_dimensions.0 as u32 + * format_desc.block_size as u32, + bytes_per_row_alignment, ); - } - } -} -pub(crate) fn collect_zero_buffer_copies_for_clear_texture( - texture_desc: &wgt::TextureDescriptor<()>, - buffer_copy_pitch: u32, - mip_range: Range, - layer_range: Range, - out_copy_regions: &mut Vec, // TODO: Something better than Vec -) { - let format_desc = texture_desc.format.describe(); + let max_rows_per_copy = crate::device::ZERO_BUFFER_SIZE as u32 / bytes_per_row; + // round down to a multiple of rows needed by the texture format + 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); - let bytes_per_row_alignment = - get_lowest_common_denom(buffer_copy_pitch, format_desc.block_size as u32); + let z_range = 0..(if texture_desc.dimension == wgt::TextureDimension::D3 { + mip_size.depth_or_array_layers + } else { + 1 + }); - for mip_level in 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); - mip_size.height = align_to(mip_size.height, format_desc.block_dimensions.1 as u32); - - let bytes_per_row = align_to( - mip_size.width / format_desc.block_dimensions.0 as u32 * format_desc.block_size as u32, - bytes_per_row_alignment, - ); - - let max_rows_per_copy = crate::device::ZERO_BUFFER_SIZE as u32 / bytes_per_row; - // round down to a multiple of rows needed by the texture format - 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); - - 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() { - // 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 { - buffer_layout: wgt::ImageDataLayout { - offset: 0, - bytes_per_row: NonZeroU32::new(bytes_per_row), - rows_per_image: None, - }, - texture_base: hal::TextureCopyBase { - mip_level, - array_layer, - origin: wgt::Origin3d { - x: 0, // Always full rows - y: mip_size.height - num_rows_left, - z, + for array_layer in 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); + + zero_buffer_copy_regions.push(hal::BufferTextureCopy { + buffer_layout: wgt::ImageDataLayout { + offset: 0, + bytes_per_row: NonZeroU32::new(bytes_per_row), + rows_per_image: None, }, - aspect: hal::FormatAspects::all(), - }, - size: hal::CopyExtent { - width: mip_size.width, // full row - height: num_rows, - depth: 1, // Only single slice of volume texture at a time right now - }, - }); + texture_base: hal::TextureCopyBase { + mip_level, + array_layer, + origin: wgt::Origin3d { + x: 0, // Always full rows + y: mip_size.height - num_rows_left, + z, + }, + aspect: hal::FormatAspects::all(), + }, + size: hal::CopyExtent { + width: mip_size.width, // full row + height: num_rows, + depth: 1, // Only single slice of volume texture at a time right now + }, + }); - num_rows_left -= num_rows; + num_rows_left -= num_rows; + } } } } + + zero_buffer_copy_regions + }; + + if !zero_buffer_copy_regions.is_empty() { + unsafe { + encoder.copy_buffer_to_texture( + zero_buffer, + dst_raw, + zero_buffer_copy_regions.into_iter(), + ); + } } } From 44b5e927dc349fb5a952799e75d4985bae0c9989 Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Sun, 19 Dec 2021 14:08:41 +0100 Subject: [PATCH 13/27] clear functions now take TextureInitRange --- wgpu-core/src/command/clear.rs | 183 ++++++++++++--------------- wgpu-core/src/command/memory_init.rs | 14 +- wgpu-core/src/command/transfer.rs | 25 ++-- wgpu-core/src/device/queue.rs | 7 +- 4 files changed, 110 insertions(+), 119 deletions(-) diff --git a/wgpu-core/src/command/clear.rs b/wgpu-core/src/command/clear.rs index f970567123..be9a4f112d 100644 --- a/wgpu-core/src/command/clear.rs +++ b/wgpu-core/src/command/clear.rs @@ -9,7 +9,7 @@ use crate::{ get_lowest_common_denom, hub::{Global, GlobalIdentityHandlerFactory, HalApi, Resource, Token}, id::{self, BufferId, CommandEncoderId, DeviceId, TextureId}, - init_tracker::MemoryInitKind, + init_tracker::{MemoryInitKind, TextureInitRange}, resource::{Texture, TextureClearMode}, track::{ResourceTracker, TextureSelector, TextureState}, Stored, @@ -227,8 +227,10 @@ impl Global { ref_count: dst_texture.life_guard().ref_count.as_ref().unwrap().clone(), }, dst_texture, - subresource_range.base_mip_level..subresource_level_end, - subresource_range.base_array_layer..subresource_layer_end, + 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], @@ -239,8 +241,7 @@ impl Global { pub(crate) fn clear_texture( dst_texture_id: &Stored, dst_texture: &Texture, - mip_range: Range, - layer_range: Range, + range: TextureInitRange, encoder: &mut A::CommandEncoder, texture_tracker: &mut ResourceTracker, device: &Device, @@ -248,8 +249,7 @@ pub(crate) fn clear_texture( clear_texture_no_device( dst_texture_id, dst_texture, - mip_range, - layer_range, + range, encoder, texture_tracker, &device.alignments, @@ -260,8 +260,7 @@ pub(crate) fn clear_texture( pub(crate) fn clear_texture_no_device( dst_texture_id: &Stored, dst_texture: &Texture, - mip_range: Range, - layer_range: Range, + range: TextureInitRange, encoder: &mut A::CommandEncoder, texture_tracker: &mut ResourceTracker, alignments: &hal::Alignments, @@ -295,8 +294,8 @@ pub(crate) fn clear_texture_no_device( dst_texture_id.value, &dst_texture_id.ref_count, TextureSelector { - levels: mip_range.clone(), - layers: layer_range.clone(), + levels: range.mip_range.clone(), + layers: range.layer_range.clone(), }, clear_usage, ) @@ -311,18 +310,13 @@ pub(crate) fn clear_texture_no_device( &dst_texture.desc, alignments, zero_buffer, - mip_range, - layer_range, + range, encoder, dst_raw, ), - TextureClearMode::RenderPass(_) => clear_texture_via_render_passes( - dst_texture, - mip_range, - layer_range, - clear_usage, - encoder, - )?, + TextureClearMode::RenderPass(_) => { + clear_texture_via_render_passes(dst_texture, range, clear_usage, encoder)? + } TextureClearMode::None => { return Err(ClearError::NoValidTextureClearMode(dst_texture_id.value.0)); } @@ -334,101 +328,88 @@ fn clear_texture_via_buffer_copies( texture_desc: &wgt::TextureDescriptor<()>, alignments: &hal::Alignments, zero_buffer: &A::Buffer, // Buffer of size device::ZERO_BUFFER_SIZE - mip_range: Range, - layer_range: Range, + range: TextureInitRange, encoder: &mut A::CommandEncoder, dst_raw: &A::Texture, ) { - // Gather list of zero_buffer copies to clear a texture. - let zero_buffer_copy_regions = { - let texture_desc: &wgt::TextureDescriptor<()> = &texture_desc; - let buffer_copy_pitch = alignments.buffer_copy_pitch.get() as u32; - let format_desc = texture_desc.format.describe(); - let mut zero_buffer_copy_regions = Vec::new(); - - let bytes_per_row_alignment = - get_lowest_common_denom(buffer_copy_pitch, format_desc.block_size as u32); - - for mip_level in 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); - mip_size.height = align_to(mip_size.height, format_desc.block_dimensions.1 as u32); - - let bytes_per_row = align_to( - mip_size.width / format_desc.block_dimensions.0 as u32 - * format_desc.block_size as u32, - bytes_per_row_alignment, - ); - - let max_rows_per_copy = crate::device::ZERO_BUFFER_SIZE as u32 / bytes_per_row; - // round down to a multiple of rows needed by the texture format - 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 {:?}", + // Gather list of zero_buffer copies and issue a single command then to perform them + let mut zero_buffer_copy_regions = Vec::new(); + let texture_desc: &wgt::TextureDescriptor<()> = &texture_desc; + 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 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); + mip_size.height = align_to(mip_size.height, format_desc.block_dimensions.1 as u32); + + let bytes_per_row = align_to( + mip_size.width / format_desc.block_dimensions.0 as u32 * format_desc.block_size as u32, + bytes_per_row_alignment, + ); + + let max_rows_per_copy = crate::device::ZERO_BUFFER_SIZE as u32 / bytes_per_row; + // round down to a multiple of rows needed by the texture format + 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); - 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() { - // 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); - - zero_buffer_copy_regions.push(hal::BufferTextureCopy { - buffer_layout: wgt::ImageDataLayout { - offset: 0, - bytes_per_row: NonZeroU32::new(bytes_per_row), - rows_per_image: None, + let z_range = 0..(if texture_desc.dimension == wgt::TextureDimension::D3 { + mip_size.depth_or_array_layers + } else { + 1 + }); + + 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); + + zero_buffer_copy_regions.push(hal::BufferTextureCopy { + buffer_layout: wgt::ImageDataLayout { + offset: 0, + bytes_per_row: NonZeroU32::new(bytes_per_row), + rows_per_image: None, + }, + texture_base: hal::TextureCopyBase { + mip_level, + array_layer, + origin: wgt::Origin3d { + x: 0, // Always full rows + y: mip_size.height - num_rows_left, + z, }, - texture_base: hal::TextureCopyBase { - mip_level, - array_layer, - origin: wgt::Origin3d { - x: 0, // Always full rows - y: mip_size.height - num_rows_left, - z, - }, - aspect: hal::FormatAspects::all(), - }, - size: hal::CopyExtent { - width: mip_size.width, // full row - height: num_rows, - depth: 1, // Only single slice of volume texture at a time right now - }, - }); + aspect: hal::FormatAspects::all(), + }, + size: hal::CopyExtent { + width: mip_size.width, // full row + height: num_rows, + depth: 1, // Only single slice of volume texture at a time right now + }, + }); - num_rows_left -= num_rows; - } + num_rows_left -= num_rows; } } } + } - zero_buffer_copy_regions - }; - - if !zero_buffer_copy_regions.is_empty() { - unsafe { - encoder.copy_buffer_to_texture( - zero_buffer, - dst_raw, - zero_buffer_copy_regions.into_iter(), - ); - } + unsafe { + encoder.copy_buffer_to_texture(zero_buffer, dst_raw, zero_buffer_copy_regions.into_iter()); } } fn clear_texture_via_render_passes( dst_texture: &Texture, - mip_range: Range, - layer_range: Range, + range: TextureInitRange, clear_usage: hal::TextureUses, encoder: &mut A::CommandEncoder, ) -> Result<(), ClearError> { @@ -439,9 +420,9 @@ fn clear_texture_via_render_passes( }; let sample_count = dst_texture.desc.sample_count; let is_3d_texture = dst_texture.desc.dimension == wgt::TextureDimension::D3; - for mip_level in mip_range { + for mip_level in range.mip_range { let extent = extent_base.mip_level_size(mip_level, is_3d_texture); - for layer in layer_range.clone() { + for layer in range.layer_range.clone() { let target = hal::Attachment { view: dst_texture.get_clear_view(mip_level, layer), usage: clear_usage, diff --git a/wgpu-core/src/command/memory_init.rs b/wgpu-core/src/command/memory_init.rs index 94ed47cda0..7c7ea8361d 100644 --- a/wgpu-core/src/command/memory_init.rs +++ b/wgpu-core/src/command/memory_init.rs @@ -119,7 +119,7 @@ impl CommandBufferTextureMemoryActions { } } -// Utility function that takes discarded surfaces from register_init_action and initializes them on the spot. +// Utility function that takes discarded surfaces from (several calls to) register_init_action and initializes them on the spot. // Takes care of barriers as well! pub(crate) fn fixup_discarded_surfaces< A: hal::Api, @@ -132,14 +132,13 @@ pub(crate) fn fixup_discarded_surfaces< device: &Device, ) { for init in inits { - let mip_range = init.mip_level..(init.mip_level + 1); - let layer_range = init.layer..(init.layer + 1); - clear_texture( &init.texture, texture_guard.get(init.texture.value.0).unwrap(), - mip_range, - layer_range, + TextureInitRange { + mip_range: init.mip_level..(init.mip_level + 1), + layer_range: init.layer..(init.layer + 1), + }, encoder, texture_tracker, device, @@ -278,8 +277,7 @@ impl BakedCommands { clear_texture( &texture_use.id, &*texture, - range.mip_range, - range.layer_range, + range, &mut self.encoder, &mut device_tracker.textures, device, diff --git a/wgpu-core/src/command/transfer.rs b/wgpu-core/src/command/transfer.rs index 154424ef48..f29fc88b9c 100644 --- a/wgpu-core/src/command/transfer.rs +++ b/wgpu-core/src/command/transfer.rs @@ -1,7 +1,7 @@ #[cfg(feature = "trace")] use crate::device::trace::Command as TraceCommand; use crate::{ - command::{memory_init::fixup_discarded_surfaces, CommandBuffer, CommandEncoderError}, + command::{CommandBuffer, CommandEncoderError}, conv, device::Device, error::{ErrorFormatter, PrettyError}, @@ -19,6 +19,8 @@ use wgt::{BufferAddress, BufferUsages, Extent3d, TextureUsages}; use std::iter; +use super::clear::clear_texture; + pub type ImageCopyBuffer = wgt::ImageCopyBuffer; pub type ImageCopyTexture = wgt::ImageCopyTexture; @@ -405,13 +407,20 @@ fn handle_texture_init( // In rare cases we may need to insert an init operation immediately onto the command buffer. if !immediate_inits.is_empty() { let cmd_buf_raw = cmd_buf.encoder.open(); - fixup_discarded_surfaces( - immediate_inits.into_iter(), - cmd_buf_raw, - texture_guard, - &mut cmd_buf.trackers.textures, - device, - ); + for init in immediate_inits { + clear_texture( + &init.texture, + texture, + TextureInitRange { + mip_range: init.mip_level..(init.mip_level + 1), + layer_range: init.layer..(init.layer + 1), + }, + cmd_buf_raw, + &mut cmd_buf.trackers.textures, + device, + ) + .unwrap(); + } } } diff --git a/wgpu-core/src/device/queue.rs b/wgpu-core/src/device/queue.rs index c58fa696c1..93c133608e 100644 --- a/wgpu-core/src/device/queue.rs +++ b/wgpu-core/src/device/queue.rs @@ -11,6 +11,7 @@ use crate::{ get_lowest_common_denom, hub::{Global, GlobalIdentityHandlerFactory, HalApi, Token}, id, + init_tracker::TextureInitRange, resource::{BufferAccessError, BufferMapState, TextureInner}, track, FastHashSet, Stored, }; @@ -454,8 +455,10 @@ impl Global { ref_count: dst.life_guard.ref_count.as_ref().unwrap().clone(), }, &*dst, - destination.mip_level..(destination.mip_level + 1), - layer_range, + TextureInitRange { + mip_range: destination.mip_level..(destination.mip_level + 1), + layer_range, + }, encoder, &mut trackers.textures, &device.alignments, From 60a5bba14ae961d14d790847e7fcdb817bebf00a Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Sun, 19 Dec 2021 14:19:22 +0100 Subject: [PATCH 14/27] Fix clippy lints --- wgpu-core/src/command/clear.rs | 1 - wgpu-core/src/device/life.rs | 12 +++++------- wgpu-core/src/hub.rs | 11 ++++------- wgpu-core/src/resource.rs | 4 ++-- 4 files changed, 11 insertions(+), 17 deletions(-) diff --git a/wgpu-core/src/command/clear.rs b/wgpu-core/src/command/clear.rs index be9a4f112d..5dbd60913a 100644 --- a/wgpu-core/src/command/clear.rs +++ b/wgpu-core/src/command/clear.rs @@ -334,7 +334,6 @@ fn clear_texture_via_buffer_copies( ) { // Gather list of zero_buffer copies and issue a single command then to perform them let mut zero_buffer_copy_regions = Vec::new(); - let texture_desc: &wgt::TextureDescriptor<()> = &texture_desc; let buffer_copy_pitch = alignments.buffer_copy_pitch.get() as u32; let format_desc = texture_desc.format.describe(); diff --git a/wgpu-core/src/device/life.rs b/wgpu-core/src/device/life.rs index 96da3deae6..04a8e1fced 100644 --- a/wgpu-core/src/device/life.rs +++ b/wgpu-core/src/device/life.rs @@ -468,13 +468,11 @@ impl LifetimeTracker { .map_or(&mut self.free_resources, |a| &mut a.last_resources); non_referenced_resources.textures.push(raw); - match res.clear_mode { - resource::TextureClearMode::RenderPass(clear_views) => { - non_referenced_resources - .texture_views - .extend(clear_views.into_iter()); - } - _ => {} + if let resource::TextureClearMode::RenderPass(clear_views) = res.clear_mode + { + non_referenced_resources + .texture_views + .extend(clear_views.into_iter()); } } } diff --git a/wgpu-core/src/hub.rs b/wgpu-core/src/hub.rs index d742cf6966..3c7e682212 100644 --- a/wgpu-core/src/hub.rs +++ b/wgpu-core/src/hub.rs @@ -649,15 +649,12 @@ impl Hub { device.raw.destroy_texture(raw); } } - match texture.clear_mode { - TextureClearMode::RenderPass(clear_views) => { - for view in clear_views { - unsafe { - device.raw.destroy_texture_view(view); - } + if let TextureClearMode::RenderPass(clear_views) = texture.clear_mode { + for view in clear_views { + unsafe { + device.raw.destroy_texture_view(view); } } - _ => {} } } } diff --git a/wgpu-core/src/resource.rs b/wgpu-core/src/resource.rs index 5d92c71989..a132bf0565 100644 --- a/wgpu-core/src/resource.rs +++ b/wgpu-core/src/resource.rs @@ -204,14 +204,14 @@ pub struct Texture { impl Texture { pub(crate) fn get_clear_view(&self, mip_level: u32, depth_or_layer: u32) -> &A::TextureView { - match &self.clear_mode { + match self.clear_mode { TextureClearMode::BufferCopy => { panic!("Given texture is cleared with buffer copies, not render passes") } TextureClearMode::None => { panic!("Given texture can't be cleared") } - TextureClearMode::RenderPass(clear_views) => { + TextureClearMode::RenderPass(ref clear_views) => { let index = mip_level + depth_or_layer * self.desc.mip_level_count; &clear_views[index as usize] } From 2b31b6bba80b38c5cbe34700bc113d5280c39f04 Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Sun, 19 Dec 2021 14:42:10 +0100 Subject: [PATCH 15/27] command_encoder_clear_texture no longer takes write lock on texture --- wgpu-core/src/command/clear.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/wgpu-core/src/command/clear.rs b/wgpu-core/src/command/clear.rs index 5dbd60913a..d10e57e8f1 100644 --- a/wgpu-core/src/command/clear.rs +++ b/wgpu-core/src/command/clear.rs @@ -164,7 +164,7 @@ impl Global { let cmd_buf = CommandBuffer::get_encoder_mut(&mut *cmd_buf_guard, command_encoder_id) .map_err(|_| ClearError::InvalidCommandEncoder(command_encoder_id))?; let (_, mut token) = hub.buffers.read(&mut token); // skip token - let (mut texture_guard, _) = hub.textures.write(&mut token); + let (texture_guard, _) = hub.textures.read(&mut token); #[cfg(feature = "trace")] if let Some(ref mut list) = cmd_buf.commands { @@ -179,7 +179,7 @@ impl Global { } let dst_texture = texture_guard - .get_mut(dst) // todo: take only write access if needed + .get(dst) .map_err(|_| ClearError::InvalidTexture(dst))?; // Check if subresource aspects are valid. From 5761e98677acdb64b94e9fc237c47d0352ec47a0 Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Sun, 19 Dec 2021 17:19:59 +0100 Subject: [PATCH 16/27] TextureClearMode encodes now is_color --- wgpu-core/src/command/clear.rs | 52 ++++++++++++++++------------------ wgpu-core/src/device/life.rs | 3 +- wgpu-core/src/device/mod.rs | 18 +++++++----- wgpu-core/src/hub.rs | 2 +- wgpu-core/src/resource.rs | 7 +++-- 5 files changed, 44 insertions(+), 38 deletions(-) diff --git a/wgpu-core/src/command/clear.rs b/wgpu-core/src/command/clear.rs index d10e57e8f1..d306390d4d 100644 --- a/wgpu-core/src/command/clear.rs +++ b/wgpu-core/src/command/clear.rs @@ -274,16 +274,10 @@ pub(crate) fn clear_texture_no_device( // Issue the right barrier. let clear_usage = match dst_texture.clear_mode { TextureClearMode::BufferCopy => hal::TextureUses::COPY_DST, - TextureClearMode::RenderPass(_) => { - if dst_texture - .hal_usage - .contains(hal::TextureUses::DEPTH_STENCIL_WRITE) - { - hal::TextureUses::DEPTH_STENCIL_WRITE - } else { - hal::TextureUses::COLOR_TARGET - } - } + 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.value.0)); } @@ -314,8 +308,8 @@ pub(crate) fn clear_texture_no_device( encoder, dst_raw, ), - TextureClearMode::RenderPass(_) => { - clear_texture_via_render_passes(dst_texture, range, clear_usage, encoder)? + TextureClearMode::RenderPass { is_color, .. } => { + clear_texture_via_render_passes(dst_texture, range, is_color, encoder)? } TextureClearMode::None => { return Err(ClearError::NoValidTextureClearMode(dst_texture_id.value.0)); @@ -409,7 +403,7 @@ fn clear_texture_via_buffer_copies( fn clear_texture_via_render_passes( dst_texture: &Texture, range: TextureInitRange, - clear_usage: hal::TextureUses, + is_color: bool, encoder: &mut A::CommandEncoder, ) -> Result<(), ClearError> { let extent_base = wgt::Extent3d { @@ -424,22 +418,26 @@ fn clear_texture_via_render_passes( for layer in range.layer_range.clone() { let target = hal::Attachment { view: dst_texture.get_clear_view(mip_level, layer), - usage: clear_usage, + usage: if is_color { + hal::TextureUses::COLOR_TARGET + } else { + hal::TextureUses::DEPTH_STENCIL_WRITE + }, }; - if clear_usage == hal::TextureUses::DEPTH_STENCIL_WRITE { + if is_color { unsafe { encoder.begin_render_pass(&hal::RenderPassDescriptor { label: Some("clear_texture clear pass"), extent, sample_count, - color_attachments: &[], - depth_stencil_attachment: Some(hal::DepthStencilAttachment { + color_attachments: &[hal::ColorAttachment { target, - depth_ops: hal::AttachmentOps::STORE, - stencil_ops: hal::AttachmentOps::STORE, - clear_value: (0.0, 0), - }), + resolve_target: None, + ops: hal::AttachmentOps::STORE, + clear_value: wgt::Color::TRANSPARENT, + }], + depth_stencil_attachment: None, multiview: None, }); } @@ -449,13 +447,13 @@ fn clear_texture_via_render_passes( label: Some("clear_texture clear pass"), extent, sample_count, - color_attachments: &[hal::ColorAttachment { + color_attachments: &[], + depth_stencil_attachment: Some(hal::DepthStencilAttachment { target, - resolve_target: None, - ops: hal::AttachmentOps::STORE, - clear_value: wgt::Color::TRANSPARENT, - }], - depth_stencil_attachment: None, + depth_ops: hal::AttachmentOps::STORE, + stencil_ops: hal::AttachmentOps::STORE, + clear_value: (0.0, 0), + }), multiview: None, }); } diff --git a/wgpu-core/src/device/life.rs b/wgpu-core/src/device/life.rs index 04a8e1fced..91756ec776 100644 --- a/wgpu-core/src/device/life.rs +++ b/wgpu-core/src/device/life.rs @@ -468,7 +468,8 @@ impl LifetimeTracker { .map_or(&mut self.free_resources, |a| &mut a.last_resources); non_referenced_resources.textures.push(raw); - if let resource::TextureClearMode::RenderPass(clear_views) = res.clear_mode + if let resource::TextureClearMode::RenderPass { clear_views, .. } = + res.clear_mode { non_referenced_resources .texture_views diff --git a/wgpu-core/src/device/mod.rs b/wgpu-core/src/device/mod.rs index 4ca846473b..12637f4255 100644 --- a/wgpu-core/src/device/mod.rs +++ b/wgpu-core/src/device/mod.rs @@ -703,11 +703,12 @@ impl Device { let clear_mode = if hal_usage.contains(hal::TextureUses::DEPTH_STENCIL_WRITE) || hal_usage.contains(hal::TextureUses::COLOR_TARGET) { - let usage = if desc.format.describe().sample_type == wgt::TextureSampleType::Depth { - hal::TextureUses::DEPTH_STENCIL_WRITE - } else { - hal::TextureUses::COLOR_TARGET - }; + let (is_color, usage) = + if desc.format.describe().sample_type == wgt::TextureSampleType::Depth { + (false, hal::TextureUses::DEPTH_STENCIL_WRITE) + } else { + (true, hal::TextureUses::COLOR_TARGET) + }; let dimension = match desc.dimension { wgt::TextureDimension::D1 => wgt::TextureViewDimension::D1, wgt::TextureDimension::D2 => wgt::TextureViewDimension::D2, @@ -741,7 +742,10 @@ impl Device { } } } - resource::TextureClearMode::RenderPass(clear_views) + resource::TextureClearMode::RenderPass { + clear_views, + is_color, + } } else { resource::TextureClearMode::BufferCopy }; @@ -3458,7 +3462,7 @@ impl Global { resource::TextureClearMode::None, ) { resource::TextureClearMode::BufferCopy => SmallVec::new(), - resource::TextureClearMode::RenderPass(clear_views) => clear_views, + resource::TextureClearMode::RenderPass { clear_views, .. } => clear_views, resource::TextureClearMode::None => SmallVec::new(), }; let temp = queue::TempResource::Texture(raw, clear_views); diff --git a/wgpu-core/src/hub.rs b/wgpu-core/src/hub.rs index 3c7e682212..2f78ab5faf 100644 --- a/wgpu-core/src/hub.rs +++ b/wgpu-core/src/hub.rs @@ -649,7 +649,7 @@ impl Hub { device.raw.destroy_texture(raw); } } - if let TextureClearMode::RenderPass(clear_views) = texture.clear_mode { + if let TextureClearMode::RenderPass { clear_views, .. } = texture.clear_mode { for view in clear_views { unsafe { device.raw.destroy_texture_view(view); diff --git a/wgpu-core/src/resource.rs b/wgpu-core/src/resource.rs index a132bf0565..04eceee462 100644 --- a/wgpu-core/src/resource.rs +++ b/wgpu-core/src/resource.rs @@ -183,7 +183,10 @@ impl TextureInner { pub enum TextureClearMode { BufferCopy, // View for clear via RenderPass for every subsurface - RenderPass(SmallVec<[A::TextureView; 1]>), + RenderPass { + clear_views: SmallVec<[A::TextureView; 1]>, + is_color: bool, + }, // Texture can't be cleared, attempting to do so will cause panic. // (either because it is impossible for the type of texture or it is being destroyed) None, @@ -211,7 +214,7 @@ impl Texture { TextureClearMode::None => { panic!("Given texture can't be cleared") } - TextureClearMode::RenderPass(ref clear_views) => { + TextureClearMode::RenderPass{ ref clear_views, .. } => { let index = mip_level + depth_or_layer * self.desc.mip_level_count; &clear_views[index as usize] } From 6732a699d0efe57b72f9f88249f0da09e895aa24 Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Sun, 19 Dec 2021 17:39:28 +0100 Subject: [PATCH 17/27] code cleanup, mostly about `use` --- wgpu-core/src/command/clear.rs | 28 ++++++++++++---------------- wgpu-core/src/command/mod.rs | 11 +++-------- wgpu-core/src/command/transfer.rs | 6 +++--- 3 files changed, 18 insertions(+), 27 deletions(-) diff --git a/wgpu-core/src/command/clear.rs b/wgpu-core/src/command/clear.rs index d306390d4d..897dbdeef7 100644 --- a/wgpu-core/src/command/clear.rs +++ b/wgpu-core/src/command/clear.rs @@ -8,7 +8,7 @@ use crate::{ device::Device, get_lowest_common_denom, hub::{Global, GlobalIdentityHandlerFactory, HalApi, Resource, Token}, - id::{self, BufferId, CommandEncoderId, DeviceId, TextureId}, + id::{BufferId, CommandEncoderId, DeviceId, TextureId, Valid}, init_tracker::{MemoryInitKind, TextureInitRange}, resource::{Texture, TextureClearMode}, track::{ResourceTracker, TextureSelector, TextureState}, @@ -65,8 +65,6 @@ whereas subesource range specified start {subresource_base_array_layer} and coun subresource_base_array_layer: u32, subresource_array_layer_count: Option, }, - #[error("failed to create view for clearing texture at mip level {mip_level}, layer {layer}")] - FailedToCreateTextureViewForClear { mip_level: u32, layer: u32 }, } impl Global { @@ -223,7 +221,7 @@ impl Global { clear_texture( &Stored { - value: id::Valid(dst), + value: Valid(dst), ref_count: dst_texture.life_guard().ref_count.as_ref().unwrap().clone(), }, dst_texture, @@ -350,7 +348,7 @@ fn clear_texture_via_buffer_copies( 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 @@ -416,15 +414,7 @@ fn clear_texture_via_render_passes( for mip_level in range.mip_range { let extent = extent_base.mip_level_size(mip_level, is_3d_texture); for layer in range.layer_range.clone() { - let target = hal::Attachment { - view: dst_texture.get_clear_view(mip_level, layer), - usage: if is_color { - hal::TextureUses::COLOR_TARGET - } else { - hal::TextureUses::DEPTH_STENCIL_WRITE - }, - }; - + let view = dst_texture.get_clear_view(mip_level, layer); if is_color { unsafe { encoder.begin_render_pass(&hal::RenderPassDescriptor { @@ -432,7 +422,10 @@ fn clear_texture_via_render_passes( extent, sample_count, color_attachments: &[hal::ColorAttachment { - target, + target: hal::Attachment { + view, + usage: hal::TextureUses::COLOR_TARGET, + }, resolve_target: None, ops: hal::AttachmentOps::STORE, clear_value: wgt::Color::TRANSPARENT, @@ -449,7 +442,10 @@ fn clear_texture_via_render_passes( sample_count, color_attachments: &[], depth_stencil_attachment: Some(hal::DepthStencilAttachment { - target, + target: hal::Attachment { + view, + usage: hal::TextureUses::DEPTH_STENCIL_WRITE, + }, depth_ops: hal::AttachmentOps::STORE, stencil_ops: hal::AttachmentOps::STORE, clear_value: (0.0, 0), diff --git a/wgpu-core/src/command/mod.rs b/wgpu-core/src/command/mod.rs index 52d4a59ac8..a2b33801e9 100644 --- a/wgpu-core/src/command/mod.rs +++ b/wgpu-core/src/command/mod.rs @@ -8,15 +8,10 @@ mod query; mod render; mod transfer; -pub use self::bundle::*; -pub(crate) use self::clear::clear_texture_no_device; -pub(crate) use self::clear::ClearError; -pub use self::compute::*; -pub use self::draw::*; +pub(crate) use self::clear::{clear_texture_no_device, ClearError}; +pub use self::{bundle::*, compute::*, draw::*, query::*, render::*, transfer::*}; + use self::memory_init::CommandBufferTextureMemoryActions; -pub use self::query::*; -pub use self::render::*; -pub use self::transfer::*; use crate::error::{ErrorFormatter, PrettyError}; use crate::init_tracker::BufferInitTrackerAction; diff --git a/wgpu-core/src/command/transfer.rs b/wgpu-core/src/command/transfer.rs index f29fc88b9c..3c5337e2a0 100644 --- a/wgpu-core/src/command/transfer.rs +++ b/wgpu-core/src/command/transfer.rs @@ -6,7 +6,7 @@ use crate::{ device::Device, error::{ErrorFormatter, PrettyError}, hub::{Global, GlobalIdentityHandlerFactory, HalApi, Storage, Token}, - id::{self, BufferId, CommandEncoderId, TextureId}, + id::{BufferId, CommandEncoderId, Id, TextureId, Valid}, init_tracker::{MemoryInitKind, TextureInitRange, TextureInitTrackerAction}, resource::{Texture, TextureErrorDimension}, track::TextureSelector, @@ -382,12 +382,12 @@ fn handle_texture_init( device: &Device, copy_texture: &ImageCopyTexture, copy_size: &Extent3d, - texture_guard: &Storage, id::Id>>, + texture_guard: &Storage, Id>>, texture: &Texture, ) { let init_action = TextureInitTrackerAction { id: Stored { - value: id::Valid(copy_texture.texture), + value: Valid(copy_texture.texture), // For copies the texture object isn't discarded yet (somebody just passed it in!), so it should be safe to access the ref_count ref_count: texture.life_guard.ref_count.as_ref().unwrap().clone(), }, From ac436b5daf1180a6a1a483b713011a031b7336ae Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Sun, 26 Dec 2021 15:15:35 +0100 Subject: [PATCH 18/27] Handle volume textures in clear_texture_via_render_passes properly --- wgpu-core/src/command/clear.rs | 79 +++++++++++++++++----------------- wgpu-core/src/device/mod.rs | 4 +- 2 files changed, 41 insertions(+), 42 deletions(-) diff --git a/wgpu-core/src/command/clear.rs b/wgpu-core/src/command/clear.rs index 897dbdeef7..1c0f8aa6d6 100644 --- a/wgpu-core/src/command/clear.rs +++ b/wgpu-core/src/command/clear.rs @@ -407,54 +407,53 @@ fn clear_texture_via_render_passes( let extent_base = wgt::Extent3d { width: dst_texture.desc.size.width, height: dst_texture.desc.size.height, - depth_or_array_layers: 1, // TODO: What about 3d textures? Only one slice a time, sure but how to select it? + depth_or_array_layers: 1, // Only one layer or slice is cleared at a time. + }; + let layer_or_depth_range = match dst_texture.desc.dimension { + wgt::TextureDimension::D1 => range.layer_range, + wgt::TextureDimension::D2 => range.layer_range, + wgt::TextureDimension::D3 => 0..dst_texture.desc.size.depth_or_array_layers, }; 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); - for layer in range.layer_range.clone() { - let view = dst_texture.get_clear_view(mip_level, layer); - if is_color { - unsafe { - encoder.begin_render_pass(&hal::RenderPassDescriptor { - label: Some("clear_texture clear pass"), - extent, - sample_count, - color_attachments: &[hal::ColorAttachment { - target: hal::Attachment { - view, - usage: hal::TextureUses::COLOR_TARGET, - }, - resolve_target: None, - ops: hal::AttachmentOps::STORE, - clear_value: wgt::Color::TRANSPARENT, - }], - depth_stencil_attachment: None, - multiview: None, - }); - } + for depth_or_layer in layer_or_depth_range.clone() { + 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 { - unsafe { - encoder.begin_render_pass(&hal::RenderPassDescriptor { - label: Some("clear_texture clear pass"), - extent, - sample_count, - color_attachments: &[], - depth_stencil_attachment: Some(hal::DepthStencilAttachment { - target: hal::Attachment { - view, - usage: hal::TextureUses::DEPTH_STENCIL_WRITE, - }, - depth_ops: hal::AttachmentOps::STORE, - stencil_ops: hal::AttachmentOps::STORE, - clear_value: (0.0, 0), - }), - multiview: None, - }); - } + ( + &[][..], + 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(); } } diff --git a/wgpu-core/src/device/mod.rs b/wgpu-core/src/device/mod.rs index 12637f4255..75f7b0e21f 100644 --- a/wgpu-core/src/device/mod.rs +++ b/wgpu-core/src/device/mod.rs @@ -716,7 +716,7 @@ impl Device { }; let mut clear_views = SmallVec::new(); - for layer in 0..desc.size.depth_or_array_layers { + for slice_or_layer in 0..desc.size.depth_or_array_layers { for mip_level in 0..desc.mip_level_count { unsafe { clear_views.push( @@ -732,7 +732,7 @@ impl Device { aspect: wgt::TextureAspect::All, base_mip_level: mip_level, mip_level_count: NonZeroU32::new(1), - base_array_layer: layer, + base_array_layer: slice_or_layer, array_layer_count: NonZeroU32::new(1), }, }, From 11babe85a5474911c72ea5bfb637c8b4c53eeb35 Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Sun, 26 Dec 2021 15:53:33 +0100 Subject: [PATCH 19/27] texture clear no longer requires id::Stored --- wgpu-core/src/command/clear.rs | 44 ++++++++++++++------------- wgpu-core/src/command/memory_init.rs | 27 ++++++++-------- wgpu-core/src/command/render.rs | 14 ++++----- wgpu-core/src/command/transfer.rs | 9 ++---- wgpu-core/src/device/mod.rs | 2 +- wgpu-core/src/device/queue.rs | 7 ++--- wgpu-core/src/init_tracker/texture.rs | 4 +-- 7 files changed, 50 insertions(+), 57 deletions(-) diff --git a/wgpu-core/src/command/clear.rs b/wgpu-core/src/command/clear.rs index 1c0f8aa6d6..a6112ccd90 100644 --- a/wgpu-core/src/command/clear.rs +++ b/wgpu-core/src/command/clear.rs @@ -12,7 +12,6 @@ use crate::{ init_tracker::{MemoryInitKind, TextureInitRange}, resource::{Texture, TextureClearMode}, track::{ResourceTracker, TextureSelector, TextureState}, - Stored, }; use hal::CommandEncoder as _; @@ -220,10 +219,7 @@ impl Global { } clear_texture( - &Stored { - value: Valid(dst), - ref_count: dst_texture.life_guard().ref_count.as_ref().unwrap().clone(), - }, + Valid(dst), dst_texture, TextureInitRange { mip_range: subresource_range.base_mip_level..subresource_level_end, @@ -237,7 +233,7 @@ impl Global { } pub(crate) fn clear_texture( - dst_texture_id: &Stored, + dst_texture_id: Valid, dst_texture: &Texture, range: TextureInitRange, encoder: &mut A::CommandEncoder, @@ -256,7 +252,7 @@ pub(crate) fn clear_texture( } pub(crate) fn clear_texture_no_device( - dst_texture_id: &Stored, + dst_texture_id: Valid, dst_texture: &Texture, range: TextureInitRange, encoder: &mut A::CommandEncoder, @@ -267,7 +263,7 @@ pub(crate) fn clear_texture_no_device( let dst_raw = dst_texture .inner .as_raw() - .ok_or(ClearError::InvalidTexture(dst_texture_id.value.0))?; + .ok_or(ClearError::InvalidTexture(dst_texture_id.0))?; // Issue the right barrier. let clear_usage = match dst_texture.clear_mode { @@ -277,21 +273,27 @@ pub(crate) fn clear_texture_no_device( } => hal::TextureUses::DEPTH_STENCIL_WRITE, TextureClearMode::RenderPass { is_color: true, .. } => hal::TextureUses::COLOR_TARGET, TextureClearMode::None => { - return Err(ClearError::NoValidTextureClearMode(dst_texture_id.value.0)); + return Err(ClearError::NoValidTextureClearMode(dst_texture_id.0)); } }; - let dst_barrier = texture_tracker - .change_replace( - dst_texture_id.value, - &dst_texture_id.ref_count, - TextureSelector { - levels: range.mip_range.clone(), - layers: range.layer_range.clone(), - }, - clear_usage, - ) - .map(|pending| pending.into_hal(dst_texture)); + let selector = TextureSelector { + levels: range.mip_range.clone(), + layers: range.layer_range.clone(), + }; + + // 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); } @@ -310,7 +312,7 @@ pub(crate) fn clear_texture_no_device( clear_texture_via_render_passes(dst_texture, range, is_color, encoder)? } TextureClearMode::None => { - return Err(ClearError::NoValidTextureClearMode(dst_texture_id.value.0)); + return Err(ClearError::NoValidTextureClearMode(dst_texture_id.0)); } } Ok(()) diff --git a/wgpu-core/src/command/memory_init.rs b/wgpu-core/src/command/memory_init.rs index 7c7ea8361d..4fa21203f1 100644 --- a/wgpu-core/src/command/memory_init.rs +++ b/wgpu-core/src/command/memory_init.rs @@ -9,7 +9,7 @@ use crate::{ init_tracker::*, resource::{Buffer, Texture}, track::{ResourceTracker, TextureState, TrackerSet}, - FastHashMap, Stored, + FastHashMap, }; use super::{clear::clear_texture, BakedCommands, DestroyedBufferError, DestroyedTextureError}; @@ -18,7 +18,7 @@ use super::{clear::clear_texture, BakedCommands, DestroyedBufferError, Destroyed /// Any read access to this surface needs to be preceded by a texture initialization. #[derive(Clone)] pub(crate) struct TextureSurfaceDiscard { - pub texture: Stored, + pub texture: TextureId, pub mip_level: u32, pub layer: u32, } @@ -53,7 +53,6 @@ impl CommandBufferTextureMemoryActions { texture_guard: &Storage, TextureId>, ) -> SurfacesInDiscardState { let mut immediately_necessary_clears = SurfacesInDiscardState::new(); - let texture_id = action.id.value.0; // Note that within a command buffer we may stack arbitrary memory init actions on the same texture // Since we react to them in sequence, they are going to be dropped again at queue submit @@ -61,7 +60,7 @@ impl CommandBufferTextureMemoryActions { // We don't need to add MemoryInitKind::NeedsInitializedMemory to init_actions if a surface is part of the discard list. // But that would mean splitting up the action which is more than we'd win here. self.init_actions - .extend(match texture_guard.get(texture_id) { + .extend(match texture_guard.get(action.id) { Ok(texture) => texture.initialization_status.check_action(action), Err(_) => return immediately_necessary_clears, // texture no longer exists }); @@ -70,7 +69,7 @@ impl CommandBufferTextureMemoryActions { // (i.e. most of the time self.discards is empty!) let init_actions = &mut self.init_actions; self.discards.retain(|discarded_surface| { - if discarded_surface.texture.value.0 == texture_id + if discarded_surface.texture == action.id && action.range.layer_range.contains(&discarded_surface.layer) && action .range @@ -103,13 +102,13 @@ impl CommandBufferTextureMemoryActions { // Shortcut for register_init_action when it is known that the action is an implicit init, not requiring any immediate resource init. pub(crate) fn register_implicit_init( &mut self, - id: Stored, + id: id::Valid, range: TextureInitRange, texture_guard: &Storage, TextureId>, ) { let must_be_empty = self.register_init_action( &TextureInitTrackerAction { - id, + id: id.0, range, kind: MemoryInitKind::ImplicitlyInitialized, }, @@ -133,8 +132,8 @@ pub(crate) fn fixup_discarded_surfaces< ) { for init in inits { clear_texture( - &init.texture, - texture_guard.get(init.texture.value.0).unwrap(), + id::Valid(init.texture), + texture_guard.get(init.texture).unwrap(), TextureInitRange { mip_range: init.mip_level..(init.mip_level + 1), layer_range: init.layer..(init.layer + 1), @@ -242,8 +241,8 @@ impl BakedCommands { let mut ranges: Vec = Vec::new(); for texture_use in self.texture_memory_actions.drain_init_actions() { let texture = texture_guard - .get_mut(texture_use.id.value.0) - .map_err(|_| DestroyedTextureError(texture_use.id.value.0))?; + .get_mut(texture_use.id) + .map_err(|_| DestroyedTextureError(texture_use.id))?; let use_range = texture_use.range; let affected_mip_trackers = texture @@ -275,7 +274,7 @@ impl BakedCommands { // TODO: Could we attempt some range collapsing here? for range in ranges.drain(..) { clear_texture( - &texture_use.id, + id::Valid(texture_use.id), &*texture, range, &mut self.encoder, @@ -289,8 +288,8 @@ impl BakedCommands { // Now that all buffers/textures have the proper init state for before cmdbuf start, we discard init states for textures it left discarded after its execution. for surface_discard in self.texture_memory_actions.discards.iter() { let texture = texture_guard - .get_mut(surface_discard.texture.value.0) - .map_err(|_| DestroyedTextureError(surface_discard.texture.value.0))?; + .get_mut(surface_discard.texture) + .map_err(|_| DestroyedTextureError(surface_discard.texture))?; texture .initialization_status .discard(surface_discard.mip_level, surface_discard.layer); diff --git a/wgpu-core/src/command/render.rs b/wgpu-core/src/command/render.rs index 8e8b33d617..5940461f87 100644 --- a/wgpu-core/src/command/render.rs +++ b/wgpu-core/src/command/render.rs @@ -568,7 +568,7 @@ impl<'a, A: HalApi> RenderPassInfo<'a, A> { if channel.load_op == LoadOp::Load { pending_discard_init_fixups.extend(texture_memory_actions.register_init_action( &TextureInitTrackerAction { - id: view.parent_id.clone(), + id: view.parent_id.value.0, range: TextureInitRange::from(view.selector.clone()), // Note that this is needed even if the target is discarded, kind: MemoryInitKind::NeedsInitializedMemory, @@ -578,7 +578,7 @@ impl<'a, A: HalApi> RenderPassInfo<'a, A> { } else if channel.store_op == StoreOp::Store { // Clear + Store texture_memory_actions.register_implicit_init( - view.parent_id.clone(), + view.parent_id.value, TextureInitRange::from(view.selector.clone()), texture_guard, ); @@ -587,7 +587,7 @@ impl<'a, A: HalApi> RenderPassInfo<'a, A> { // the discard happens at the *end* of a pass // but recording the discard right away be alright since the texture can't be used during the pass anyways texture_memory_actions.discard(TextureSurfaceDiscard { - texture: view.parent_id.clone(), + texture: view.parent_id.value.0, mip_level: view.selector.levels.start, layer: view.selector.layers.start, }); @@ -729,7 +729,7 @@ impl<'a, A: HalApi> RenderPassInfo<'a, A> { pending_discard_init_fixups.extend( cmd_buf.texture_memory_actions.register_init_action( &TextureInitTrackerAction { - id: view.parent_id.clone(), + id: view.parent_id.value.0, range: TextureInitRange::from(view.selector.clone()), kind: MemoryInitKind::NeedsInitializedMemory, }, @@ -745,7 +745,7 @@ impl<'a, A: HalApi> RenderPassInfo<'a, A> { if at.depth.store_op != at.stencil.store_op { if !need_init_beforehand { cmd_buf.texture_memory_actions.register_implicit_init( - view.parent_id.clone(), + view.parent_id.value, TextureInitRange::from(view.selector.clone()), texture_guard, ); @@ -761,7 +761,7 @@ impl<'a, A: HalApi> RenderPassInfo<'a, A> { } else if at.depth.store_op == StoreOp::Discard { // Both are discarded using the regular path. discarded_surfaces.push(TextureSurfaceDiscard { - texture: view.parent_id.clone(), + texture: view.parent_id.value.0, mip_level: view.selector.levels.start, layer: view.selector.layers.start, }); @@ -838,7 +838,7 @@ impl<'a, A: HalApi> RenderPassInfo<'a, A> { } cmd_buf.texture_memory_actions.register_implicit_init( - resolve_view.parent_id.clone(), + resolve_view.parent_id.value, TextureInitRange::from(resolve_view.selector.clone()), texture_guard, ); diff --git a/wgpu-core/src/command/transfer.rs b/wgpu-core/src/command/transfer.rs index 3c5337e2a0..5392b5e253 100644 --- a/wgpu-core/src/command/transfer.rs +++ b/wgpu-core/src/command/transfer.rs @@ -10,7 +10,6 @@ use crate::{ init_tracker::{MemoryInitKind, TextureInitRange, TextureInitTrackerAction}, resource::{Texture, TextureErrorDimension}, track::TextureSelector, - Stored, }; use hal::CommandEncoder as _; @@ -386,11 +385,7 @@ fn handle_texture_init( texture: &Texture, ) { let init_action = TextureInitTrackerAction { - id: Stored { - value: Valid(copy_texture.texture), - // For copies the texture object isn't discarded yet (somebody just passed it in!), so it should be safe to access the ref_count - ref_count: texture.life_guard.ref_count.as_ref().unwrap().clone(), - }, + id: copy_texture.texture, range: TextureInitRange { mip_range: copy_texture.mip_level..copy_texture.mip_level + 1, layer_range: copy_texture.origin.z @@ -409,7 +404,7 @@ fn handle_texture_init( let cmd_buf_raw = cmd_buf.encoder.open(); for init in immediate_inits { clear_texture( - &init.texture, + Valid(init.texture), texture, TextureInitRange { mip_range: init.mip_level..(init.mip_level + 1), diff --git a/wgpu-core/src/device/mod.rs b/wgpu-core/src/device/mod.rs index 75f7b0e21f..1a83f86e9e 100644 --- a/wgpu-core/src/device/mod.rs +++ b/wgpu-core/src/device/mod.rs @@ -1535,7 +1535,7 @@ impl Device { check_texture_usage(texture.desc.usage, pub_usage)?; used_texture_ranges.push(TextureInitTrackerAction { - id: view.parent_id.clone(), + id: view.parent_id.value.0, range: TextureInitRange { mip_range: view.desc.range.mip_range(&texture.desc), layer_range: view.desc.range.layer_range(&texture.desc), diff --git a/wgpu-core/src/device/queue.rs b/wgpu-core/src/device/queue.rs index 93c133608e..0efe809ebd 100644 --- a/wgpu-core/src/device/queue.rs +++ b/wgpu-core/src/device/queue.rs @@ -13,7 +13,7 @@ use crate::{ id, init_tracker::TextureInitRange, resource::{BufferAccessError, BufferMapState, TextureInner}, - track, FastHashSet, Stored, + track, FastHashSet, }; use hal::{CommandEncoder as _, Device as _, Queue as _}; @@ -450,10 +450,7 @@ impl Global { .collect::>>() { crate::command::clear_texture_no_device( - &Stored { - value: id::Valid(destination.texture), - ref_count: dst.life_guard.ref_count.as_ref().unwrap().clone(), - }, + id::Valid(destination.texture), &*dst, TextureInitRange { mip_range: destination.mip_level..(destination.mip_level + 1), diff --git a/wgpu-core/src/init_tracker/texture.rs b/wgpu-core/src/init_tracker/texture.rs index 26e313020f..6f6f8f2d36 100644 --- a/wgpu-core/src/init_tracker/texture.rs +++ b/wgpu-core/src/init_tracker/texture.rs @@ -1,5 +1,5 @@ use super::{InitTracker, MemoryInitKind}; -use crate::{id::TextureId, track::TextureSelector, Stored}; +use crate::{id::TextureId, track::TextureSelector}; use arrayvec::ArrayVec; use std::ops::Range; @@ -20,7 +20,7 @@ impl From for TextureInitRange { #[derive(Debug, Clone)] pub(crate) struct TextureInitTrackerAction { - pub(crate) id: Stored, + pub(crate) id: TextureId, pub(crate) range: TextureInitRange, pub(crate) kind: MemoryInitKind, } From 6d8b98642bc513265f74e691cdb8909b08675c1f Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Sun, 26 Dec 2021 21:52:41 +0100 Subject: [PATCH 20/27] init tracking fixes for volumes and init on partial subresource writes --- wgpu-core/src/command/clear.rs | 14 ++++++++------ wgpu-core/src/command/transfer.rs | 17 +++++++++++------ wgpu-core/src/device/mod.rs | 9 +++++++-- wgpu-core/src/device/queue.rs | 11 +++++++---- wgpu-core/src/init_tracker/mod.rs | 2 +- wgpu-core/src/init_tracker/texture.rs | 16 +++++++++++++++- wgpu-core/src/resource.rs | 14 +++++++++++--- 7 files changed, 60 insertions(+), 23 deletions(-) diff --git a/wgpu-core/src/command/clear.rs b/wgpu-core/src/command/clear.rs index a6112ccd90..42d742ab5c 100644 --- a/wgpu-core/src/command/clear.rs +++ b/wgpu-core/src/command/clear.rs @@ -411,16 +411,18 @@ fn clear_texture_via_render_passes( height: dst_texture.desc.size.height, depth_or_array_layers: 1, // Only one layer or slice is cleared at a time. }; - let layer_or_depth_range = match dst_texture.desc.dimension { - wgt::TextureDimension::D1 => range.layer_range, - wgt::TextureDimension::D2 => range.layer_range, - wgt::TextureDimension::D3 => 0..dst_texture.desc.size.depth_or_array_layers, - }; + 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); - for depth_or_layer in layer_or_depth_range.clone() { + 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 { diff --git a/wgpu-core/src/command/transfer.rs b/wgpu-core/src/command/transfer.rs index 5392b5e253..9e19828375 100644 --- a/wgpu-core/src/command/transfer.rs +++ b/wgpu-core/src/command/transfer.rs @@ -7,7 +7,10 @@ use crate::{ error::{ErrorFormatter, PrettyError}, hub::{Global, GlobalIdentityHandlerFactory, HalApi, Storage, Token}, id::{BufferId, CommandEncoderId, Id, TextureId, Valid}, - init_tracker::{MemoryInitKind, TextureInitRange, TextureInitTrackerAction}, + init_tracker::{ + has_copy_partial_init_tracker_coverage, MemoryInitKind, TextureInitRange, + TextureInitTrackerAction, + }, resource::{Texture, TextureErrorDimension}, track::TextureSelector, }; @@ -457,12 +460,14 @@ fn handle_dst_texture_init( // Attention: If we don't write full texture subresources, we need to a full clear first since we don't track subrects. // This means that in rare cases even a *destination* texture of a transfer may need an immediate texture init. - let dst_init_kind = if copy_size.width == texture.desc.size.width - && copy_size.height == texture.desc.size.height - { - MemoryInitKind::ImplicitlyInitialized - } else { + let dst_init_kind = if has_copy_partial_init_tracker_coverage( + copy_size, + destination.mip_level, + &texture.desc, + ) { MemoryInitKind::NeedsInitializedMemory + } else { + MemoryInitKind::ImplicitlyInitialized }; handle_texture_init( diff --git a/wgpu-core/src/device/mod.rs b/wgpu-core/src/device/mod.rs index 1a83f86e9e..902846c8cd 100644 --- a/wgpu-core/src/device/mod.rs +++ b/wgpu-core/src/device/mod.rs @@ -712,12 +712,17 @@ impl Device { let dimension = match desc.dimension { wgt::TextureDimension::D1 => wgt::TextureViewDimension::D1, wgt::TextureDimension::D2 => wgt::TextureViewDimension::D2, - wgt::TextureDimension::D3 => wgt::TextureViewDimension::D2, // Todo? Is this even specified? + wgt::TextureDimension::D3 => wgt::TextureViewDimension::D3, }; let mut clear_views = SmallVec::new(); - for slice_or_layer in 0..desc.size.depth_or_array_layers { for mip_level in 0..desc.mip_level_count { + let num_slices_or_layers = if desc.dimension == wgt::TextureDimension::D3 { + (desc.size.depth_or_array_layers >> mip_level).max(1) + } else { + desc.size.depth_or_array_layers + }; + for slice_or_layer in 0..num_slices_or_layers { unsafe { clear_views.push( self.raw diff --git a/wgpu-core/src/device/queue.rs b/wgpu-core/src/device/queue.rs index 0efe809ebd..ff3d640321 100644 --- a/wgpu-core/src/device/queue.rs +++ b/wgpu-core/src/device/queue.rs @@ -11,7 +11,7 @@ use crate::{ get_lowest_common_denom, hub::{Global, GlobalIdentityHandlerFactory, HalApi, Token}, id, - init_tracker::TextureInitRange, + init_tracker::{has_copy_partial_init_tracker_coverage, TextureInitRange}, resource::{BufferAccessError, BufferMapState, TextureInner}, track, FastHashSet, }; @@ -438,13 +438,16 @@ impl Global { // If the copy does not fully cover the layers, we need to initialize to zero *first* as we don't keep track of partial texture layer inits. // Strictly speaking we only need to clear the areas of a layer untouched, but this would get increasingly messy. - let init_layer_range = - destination.origin.z..destination.origin.z + size.depth_or_array_layers; + let init_layer_range = if dst.desc.dimension == wgt::TextureDimension::D3 { + 0..1 // volume textures don't have a layer range as array volumes aren't supported + } else { + destination.origin.z..destination.origin.z + size.depth_or_array_layers + }; if dst.initialization_status.mips[destination.mip_level as usize] .check(init_layer_range.clone()) .is_some() { - if size.width != dst.desc.size.width || size.height != dst.desc.size.height { + if has_copy_partial_init_tracker_coverage(size, destination.mip_level, &dst.desc) { for layer_range in dst.initialization_status.mips[destination.mip_level as usize] .drain(init_layer_range) .collect::>>() diff --git a/wgpu-core/src/init_tracker/mod.rs b/wgpu-core/src/init_tracker/mod.rs index ac25ea7e24..097b3c829e 100644 --- a/wgpu-core/src/init_tracker/mod.rs +++ b/wgpu-core/src/init_tracker/mod.rs @@ -20,7 +20,7 @@ mod buffer; mod texture; pub(crate) use buffer::{BufferInitTracker, BufferInitTrackerAction}; -pub(crate) use texture::{TextureInitRange, TextureInitTracker, TextureInitTrackerAction}; +pub(crate) use texture::{TextureInitRange, TextureInitTracker, TextureInitTrackerAction, has_copy_partial_init_tracker_coverage}; #[derive(Debug, Clone, Copy)] pub(crate) enum MemoryInitKind { diff --git a/wgpu-core/src/init_tracker/texture.rs b/wgpu-core/src/init_tracker/texture.rs index 6f6f8f2d36..ac744247dc 100644 --- a/wgpu-core/src/init_tracker/texture.rs +++ b/wgpu-core/src/init_tracker/texture.rs @@ -6,7 +6,21 @@ use std::ops::Range; #[derive(Debug, Clone)] pub(crate) struct TextureInitRange { pub(crate) mip_range: Range, - pub(crate) layer_range: Range, + pub(crate) layer_range: Range, // Strictly array layers. We do *not* track volume slices separately. +} + +// Returns true if a copy operation doesn't fully cover the texture init tracking granularity. +// I.e. if this function returns true for a pending copy operation, the target texture needs to be ensured to be initialized first! +pub(crate) fn has_copy_partial_init_tracker_coverage( + copy_size: &wgt::Extent3d, + mip_level: u32, + desc: &wgt::TextureDescriptor<()>, +) -> bool { + let target_size = desc.mip_level_size(mip_level).unwrap(); + copy_size.width != target_size.width + || copy_size.height != target_size.height + || (desc.dimension == wgt::TextureDimension::D3 + && copy_size.depth_or_array_layers != target_size.depth_or_array_layers) } impl From for TextureInitRange { diff --git a/wgpu-core/src/resource.rs b/wgpu-core/src/resource.rs index 04eceee462..6c3085f17d 100644 --- a/wgpu-core/src/resource.rs +++ b/wgpu-core/src/resource.rs @@ -182,7 +182,7 @@ impl TextureInner { #[derive(Debug)] pub enum TextureClearMode { BufferCopy, - // View for clear via RenderPass for every subsurface + // View for clear via RenderPass for every subsurface (mip/layer/slice) RenderPass { clear_views: SmallVec<[A::TextureView; 1]>, is_color: bool, @@ -214,8 +214,16 @@ impl Texture { TextureClearMode::None => { panic!("Given texture can't be cleared") } - TextureClearMode::RenderPass{ ref clear_views, .. } => { - let index = mip_level + depth_or_layer * self.desc.mip_level_count; + TextureClearMode::RenderPass { + ref clear_views, .. + } => { + let index = if self.desc.dimension == wgt::TextureDimension::D3 { + (0..mip_level).fold(0, |acc, mip| { + acc + (self.desc.size.depth_or_array_layers >> mip).max(1) + }) + } else { + mip_level * self.desc.size.depth_or_array_layers + } + depth_or_layer; &clear_views[index as usize] } } From ad959632ea352b6191125b0df080550e19a9e697 Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Sun, 26 Dec 2021 21:54:09 +0100 Subject: [PATCH 21/27] texture creation enforces COPY_DST only if absolutely necessary --- wgpu-core/src/conv.rs | 4 +++ wgpu-core/src/device/mod.rs | 70 ++++++++++++++++++++++--------------- 2 files changed, 46 insertions(+), 28 deletions(-) diff --git a/wgpu-core/src/conv.rs b/wgpu-core/src/conv.rs index 930a532976..b6632b5903 100644 --- a/wgpu-core/src/conv.rs +++ b/wgpu-core/src/conv.rs @@ -70,6 +70,10 @@ pub fn map_texture_usage( hal::TextureUses::COPY_SRC, usage.contains(wgt::TextureUsages::COPY_SRC), ); + u.set( + hal::TextureUses::COPY_DST, + usage.contains(wgt::TextureUsages::COPY_DST), + ); u.set( hal::TextureUses::RESOURCE, usage.contains(wgt::TextureUsages::TEXTURE_BINDING), diff --git a/wgpu-core/src/device/mod.rs b/wgpu-core/src/device/mod.rs index 902846c8cd..891f79705d 100644 --- a/wgpu-core/src/device/mod.rs +++ b/wgpu-core/src/device/mod.rs @@ -597,6 +597,7 @@ impl Device { fn create_texture_from_hal( &self, hal_texture: A::Texture, + hal_usage: hal::TextureUses, self_id: id::DeviceId, desc: &resource::TextureDescriptor, format_features: wgt::TextureFormatFeatures, @@ -604,8 +605,6 @@ impl Device { ) -> resource::Texture { debug_assert_eq!(self_id.backend(), A::VARIANT); - let hal_usage = conv::map_texture_usage(desc.usage, desc.format.into()); - resource::Texture { inner: resource::TextureInner::Native { raw: Some(hal_texture), @@ -645,27 +644,6 @@ impl Device { return Err(resource::CreateTextureError::CannotCreateDepthVolumeTexture(desc.format)); } - // Enforce having COPY_DST/DEPTH_STENCIL_WRIT/COLOR_TARGET otherwise we wouldn't be able to initialize the texture. - let hal_usage = conv::map_texture_usage(desc.usage, desc.format.into()) - | if format_desc.sample_type == wgt::TextureSampleType::Depth { - hal::TextureUses::DEPTH_STENCIL_WRITE - } else if desc.sample_count > 1 { - hal::TextureUses::COLOR_TARGET - } else { - hal::TextureUses::COPY_DST - }; - - let hal_desc = hal::TextureDescriptor { - label: desc.label.borrow_option(), - size: desc.size, - mip_level_count: desc.mip_level_count, - sample_count: desc.sample_count, - dimension: desc.dimension, - format: desc.format, - usage: hal_usage, - memory_flags: hal::MemoryFlags::empty(), - }; - let format_features = self .describe_format_features(adapter, desc.format) .map_err(|error| resource::CreateTextureError::MissingFeatures(desc.format, error))?; @@ -694,14 +672,43 @@ impl Device { return Err(resource::CreateTextureError::InvalidMipLevelCount(mips)); } + // Enforce having COPY_DST/DEPTH_STENCIL_WRIT/COLOR_TARGET otherwise we wouldn't be able to initialize the texture. + let hal_usage = conv::map_texture_usage(desc.usage, desc.format.into()) + | if format_desc.sample_type == wgt::TextureSampleType::Depth { + hal::TextureUses::DEPTH_STENCIL_WRITE + } else if desc.usage.contains(wgt::TextureUsages::COPY_DST) { + hal::TextureUses::COPY_DST // (set already) + } else { + // Use COPY_DST only if we can't use COLOR_TARGET + if format_features + .allowed_usages + .contains(wgt::TextureUsages::RENDER_ATTACHMENT) + { + hal::TextureUses::COLOR_TARGET + } else { + hal::TextureUses::COPY_DST + } + }; + + let hal_desc = hal::TextureDescriptor { + label: desc.label.borrow_option(), + size: desc.size, + mip_level_count: desc.mip_level_count, + sample_count: desc.sample_count, + dimension: desc.dimension, + format: desc.format, + usage: hal_usage, + memory_flags: hal::MemoryFlags::empty(), + }; + let raw_texture = unsafe { self.raw .create_texture(&hal_desc) .map_err(DeviceError::from)? }; - let clear_mode = if hal_usage.contains(hal::TextureUses::DEPTH_STENCIL_WRITE) - || hal_usage.contains(hal::TextureUses::COLOR_TARGET) + let clear_mode = if hal_usage + .intersects(hal::TextureUses::DEPTH_STENCIL_WRITE | hal::TextureUses::COLOR_TARGET) { let (is_color, usage) = if desc.format.describe().sample_type == wgt::TextureSampleType::Depth { @@ -716,7 +723,7 @@ impl Device { }; let mut clear_views = SmallVec::new(); - for mip_level in 0..desc.mip_level_count { + for mip_level in 0..desc.mip_level_count { let num_slices_or_layers = if desc.dimension == wgt::TextureDimension::D3 { (desc.size.depth_or_array_layers >> mip_level).max(1) } else { @@ -755,8 +762,14 @@ impl Device { resource::TextureClearMode::BufferCopy }; - let mut texture = - self.create_texture_from_hal(raw_texture, self_id, desc, format_features, clear_mode); + let mut texture = self.create_texture_from_hal( + raw_texture, + hal_usage, + self_id, + desc, + format_features, + clear_mode, + ); texture.hal_usage = hal_usage; Ok(texture) } @@ -3397,6 +3410,7 @@ impl Global { let mut texture = device.create_texture_from_hal( hal_texture, + conv::map_texture_usage(desc.usage, desc.format.into()), device_id, desc, format_features, From 614725fc3b96867c5bf3a26bc902e77d2d371e94 Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Sun, 26 Dec 2021 21:56:33 +0100 Subject: [PATCH 22/27] unrolled functional chain, reduce unsafe scope size --- wgpu-core/src/command/memory_init.rs | 8 +++--- wgpu-core/src/device/mod.rs | 39 ++++++++++++---------------- 2 files changed, 21 insertions(+), 26 deletions(-) diff --git a/wgpu-core/src/command/memory_init.rs b/wgpu-core/src/command/memory_init.rs index 4fa21203f1..939931bf77 100644 --- a/wgpu-core/src/command/memory_init.rs +++ b/wgpu-core/src/command/memory_init.rs @@ -261,12 +261,12 @@ impl BakedCommands { } MemoryInitKind::NeedsInitializedMemory => { for (mip_level, mip_tracker) in affected_mip_trackers { - ranges.extend(mip_tracker.drain(use_range.layer_range.clone()).map( - |layer_range| TextureInitRange { + for layer_range in mip_tracker.drain(use_range.layer_range.clone()) { + ranges.push(TextureInitRange { mip_range: (mip_level as u32)..(mip_level as u32 + 1), layer_range, - }, - )); + }); + } } } } diff --git a/wgpu-core/src/device/mod.rs b/wgpu-core/src/device/mod.rs index 891f79705d..cf320609bb 100644 --- a/wgpu-core/src/device/mod.rs +++ b/wgpu-core/src/device/mod.rs @@ -730,28 +730,23 @@ impl Device { desc.size.depth_or_array_layers }; for slice_or_layer in 0..num_slices_or_layers { - unsafe { - clear_views.push( - self.raw - .create_texture_view( - &raw_texture, - &hal::TextureViewDescriptor { - label: Some("clear texture view"), - format: desc.format, - dimension, - usage, - range: wgt::ImageSubresourceRange { - aspect: wgt::TextureAspect::All, - base_mip_level: mip_level, - mip_level_count: NonZeroU32::new(1), - base_array_layer: slice_or_layer, - array_layer_count: NonZeroU32::new(1), - }, - }, - ) - .map_err(DeviceError::from)?, - ) - } + let desc = hal::TextureViewDescriptor { + label: Some("clear texture view"), + format: desc.format, + dimension, + usage, + range: wgt::ImageSubresourceRange { + aspect: wgt::TextureAspect::All, + base_mip_level: mip_level, + mip_level_count: NonZeroU32::new(1), + base_array_layer: slice_or_layer, + array_layer_count: NonZeroU32::new(1), + }, + }; + clear_views.push( + unsafe { self.raw.create_texture_view(&raw_texture, &desc) } + .map_err(DeviceError::from)?, + ); } } resource::TextureClearMode::RenderPass { From dc24f94037a4bfbee3220df85754a21c362a6a07 Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Sun, 26 Dec 2021 22:01:48 +0100 Subject: [PATCH 23/27] fix clippy lints --- wgpu-core/src/command/memory_init.rs | 2 +- wgpu-core/src/init_tracker/texture.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/wgpu-core/src/command/memory_init.rs b/wgpu-core/src/command/memory_init.rs index 939931bf77..ea0b303609 100644 --- a/wgpu-core/src/command/memory_init.rs +++ b/wgpu-core/src/command/memory_init.rs @@ -81,7 +81,7 @@ impl CommandBufferTextureMemoryActions { // Mark surface as implicitly initialized (this is relevant because it might have been uninitialized prior to discarding init_actions.push(TextureInitTrackerAction { - id: discarded_surface.texture.clone(), + id: discarded_surface.texture, range: TextureInitRange { mip_range: discarded_surface.mip_level ..(discarded_surface.mip_level + 1), diff --git a/wgpu-core/src/init_tracker/texture.rs b/wgpu-core/src/init_tracker/texture.rs index ac744247dc..a2913ea3d4 100644 --- a/wgpu-core/src/init_tracker/texture.rs +++ b/wgpu-core/src/init_tracker/texture.rs @@ -83,7 +83,7 @@ impl TextureInitTracker { if mip_range_start < mip_range_end && layer_range_start < layer_range_end { Some(TextureInitTrackerAction { - id: action.id.clone(), + id: action.id, range: TextureInitRange { mip_range: mip_range_start as u32..mip_range_end as u32, layer_range: layer_range_start..layer_range_end, From 73d706fee73830f64d5f0b9558985be1dcff35f2 Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Tue, 28 Dec 2021 15:54:00 +0100 Subject: [PATCH 24/27] clear_texture test no longer creates 1D textures see #2323 --- wgpu/tests/clear_texture.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/wgpu/tests/clear_texture.rs b/wgpu/tests/clear_texture.rs index 2e1d50beb0..8794c2d85d 100644 --- a/wgpu/tests/clear_texture.rs +++ b/wgpu/tests/clear_texture.rs @@ -36,10 +36,13 @@ static TEXTURE_FORMATS_UNCOMPRESSED: &[wgpu::TextureFormat] = &[ wgpu::TextureFormat::Rgba32Uint, wgpu::TextureFormat::Rgba32Sint, wgpu::TextureFormat::Rgba32Float, + wgpu::TextureFormat::Rgb9e5Ufloat, +]; + +static TEXTURE_FORMATS_DEPTH: &[wgpu::TextureFormat] = &[ wgpu::TextureFormat::Depth32Float, wgpu::TextureFormat::Depth24Plus, wgpu::TextureFormat::Depth24PlusStencil8, - wgpu::TextureFormat::Rgb9e5Ufloat, ]; // needs TEXTURE_COMPRESSION_BC @@ -210,6 +213,7 @@ fn clear_texture_2d_uncompressed() { TestParameters::default().features(wgpu::Features::CLEAR_TEXTURE), |ctx| { clear_texture_tests(&ctx, TEXTURE_FORMATS_UNCOMPRESSED, true); + clear_texture_tests(&ctx, TEXTURE_FORMATS_DEPTH, false); }, ) } From 087145b652227218b54ff2e4406bfa916d3dd6c8 Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Tue, 28 Dec 2021 15:57:43 +0100 Subject: [PATCH 25/27] 3D textures are no longer cleared as render target since this isn't supported on Metal --- wgpu-core/src/device/mod.rs | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/wgpu-core/src/device/mod.rs b/wgpu-core/src/device/mod.rs index cf320609bb..5e5500dd77 100644 --- a/wgpu-core/src/device/mod.rs +++ b/wgpu-core/src/device/mod.rs @@ -683,6 +683,7 @@ impl Device { if format_features .allowed_usages .contains(wgt::TextureUsages::RENDER_ATTACHMENT) + && desc.dimension != wgt::TextureDimension::D3 // Render targets into 3D textures are not { hal::TextureUses::COLOR_TARGET } else { @@ -719,17 +720,12 @@ impl Device { let dimension = match desc.dimension { wgt::TextureDimension::D1 => wgt::TextureViewDimension::D1, wgt::TextureDimension::D2 => wgt::TextureViewDimension::D2, - wgt::TextureDimension::D3 => wgt::TextureViewDimension::D3, + wgt::TextureDimension::D3 => unreachable!(), }; let mut clear_views = SmallVec::new(); for mip_level in 0..desc.mip_level_count { - let num_slices_or_layers = if desc.dimension == wgt::TextureDimension::D3 { - (desc.size.depth_or_array_layers >> mip_level).max(1) - } else { - desc.size.depth_or_array_layers - }; - for slice_or_layer in 0..num_slices_or_layers { + for array_layer in 0..desc.size.depth_or_array_layers { let desc = hal::TextureViewDescriptor { label: Some("clear texture view"), format: desc.format, @@ -739,7 +735,7 @@ impl Device { aspect: wgt::TextureAspect::All, base_mip_level: mip_level, mip_level_count: NonZeroU32::new(1), - base_array_layer: slice_or_layer, + base_array_layer: array_layer, array_layer_count: NonZeroU32::new(1), }, }; From 4b74c4f7dbee0310f0a0abea981ab62f7c9be2d7 Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Tue, 28 Dec 2021 13:07:25 +0100 Subject: [PATCH 26/27] fix deno building issue, fix formatting --- wgpu-core/src/command/mod.rs | 6 ++++-- wgpu-core/src/device/mod.rs | 3 ++- wgpu-core/src/init_tracker/mod.rs | 5 ++++- 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/wgpu-core/src/command/mod.rs b/wgpu-core/src/command/mod.rs index a2b33801e9..e288acfd29 100644 --- a/wgpu-core/src/command/mod.rs +++ b/wgpu-core/src/command/mod.rs @@ -8,8 +8,10 @@ mod query; mod render; mod transfer; -pub(crate) use self::clear::{clear_texture_no_device, ClearError}; -pub use self::{bundle::*, compute::*, draw::*, query::*, render::*, transfer::*}; +pub(crate) use self::clear::clear_texture_no_device; +pub use self::{ + bundle::*, clear::ClearError, compute::*, draw::*, query::*, render::*, transfer::*, +}; use self::memory_init::CommandBufferTextureMemoryActions; diff --git a/wgpu-core/src/device/mod.rs b/wgpu-core/src/device/mod.rs index 5e5500dd77..097eec3f2e 100644 --- a/wgpu-core/src/device/mod.rs +++ b/wgpu-core/src/device/mod.rs @@ -683,7 +683,8 @@ impl Device { if format_features .allowed_usages .contains(wgt::TextureUsages::RENDER_ATTACHMENT) - && desc.dimension != wgt::TextureDimension::D3 // Render targets into 3D textures are not + && desc.dimension != wgt::TextureDimension::D3 + // Render targets into 3D textures are not { hal::TextureUses::COLOR_TARGET } else { diff --git a/wgpu-core/src/init_tracker/mod.rs b/wgpu-core/src/init_tracker/mod.rs index 097b3c829e..b5c6160244 100644 --- a/wgpu-core/src/init_tracker/mod.rs +++ b/wgpu-core/src/init_tracker/mod.rs @@ -20,7 +20,10 @@ mod buffer; mod texture; pub(crate) use buffer::{BufferInitTracker, BufferInitTrackerAction}; -pub(crate) use texture::{TextureInitRange, TextureInitTracker, TextureInitTrackerAction, has_copy_partial_init_tracker_coverage}; +pub(crate) use texture::{ + has_copy_partial_init_tracker_coverage, TextureInitRange, TextureInitTracker, + TextureInitTrackerAction, +}; #[derive(Debug, Clone, Copy)] pub(crate) enum MemoryInitKind { From f80a13d24aa95f7fa966a0bac1ecc5709d8f9bd2 Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Tue, 28 Dec 2021 18:12:28 +0100 Subject: [PATCH 27/27] TextureInner::Surface can now be zero initialized --- wgpu-core/src/device/mod.rs | 24 +++++++++++++-------- wgpu-core/src/present.rs | 42 ++++++++++++++++++++++++++++++++++++- wgpu/examples/boids/main.rs | 4 +++- 3 files changed, 59 insertions(+), 11 deletions(-) diff --git a/wgpu-core/src/device/mod.rs b/wgpu-core/src/device/mod.rs index 097eec3f2e..f2f86fbcda 100644 --- a/wgpu-core/src/device/mod.rs +++ b/wgpu-core/src/device/mod.rs @@ -3465,17 +3465,16 @@ impl Global { let last_submit_index = texture.life_guard.life_count(); + let clear_views = + match std::mem::replace(&mut texture.clear_mode, resource::TextureClearMode::None) { + resource::TextureClearMode::BufferCopy => SmallVec::new(), + resource::TextureClearMode::RenderPass { clear_views, .. } => clear_views, + resource::TextureClearMode::None => SmallVec::new(), + }; + match texture.inner { resource::TextureInner::Native { ref mut raw } => { let raw = raw.take().ok_or(resource::DestroyError::AlreadyDestroyed)?; - let clear_views = match std::mem::replace( - &mut texture.clear_mode, - resource::TextureClearMode::None, - ) { - resource::TextureClearMode::BufferCopy => SmallVec::new(), - resource::TextureClearMode::RenderPass { clear_views, .. } => clear_views, - resource::TextureClearMode::None => SmallVec::new(), - }; let temp = queue::TempResource::Texture(raw, clear_views); if device.pending_writes.dst_textures.contains(&texture_id) { @@ -3487,7 +3486,14 @@ impl Global { .schedule_resource_destruction(temp, last_submit_index); } } - resource::TextureInner::Surface { .. } => {} //TODO + resource::TextureInner::Surface { .. } => { + for clear_view in clear_views { + unsafe { + device.raw.destroy_texture_view(clear_view); + } + } + // TODO? + } } Ok(()) diff --git a/wgpu-core/src/present.rs b/wgpu-core/src/present.rs index c13b8923cb..7421377ed7 100644 --- a/wgpu-core/src/present.rs +++ b/wgpu-core/src/present.rs @@ -9,6 +9,8 @@ When this texture is presented, we remove it from the device tracker as well as extract it from the hub. !*/ +use std::{borrow::Borrow, num::NonZeroU32}; + #[cfg(feature = "trace")] use crate::device::trace::Action; use crate::{ @@ -125,6 +127,31 @@ impl Global { let suf = A::get_surface_mut(surface); let (texture_id, status) = match unsafe { suf.raw.acquire_texture(FRAME_TIMEOUT_MS) } { Ok(Some(ast)) => { + let clear_view_desc = hal::TextureViewDescriptor { + label: Some("clear texture view"), + format: config.format, + dimension: wgt::TextureViewDimension::D2, + usage: hal::TextureUses::COLOR_TARGET, + range: wgt::ImageSubresourceRange { + aspect: wgt::TextureAspect::All, + base_mip_level: 0, + mip_level_count: NonZeroU32::new(1), + base_array_layer: 0, + array_layer_count: NonZeroU32::new(1), + }, + }; + let mut clear_views = smallvec::SmallVec::new(); + clear_views.push( + unsafe { + hal::Device::create_texture_view( + &device.raw, + &ast.texture.borrow(), + &clear_view_desc, + ) + } + .map_err(DeviceError::from)?, + ); + let present = surface.presentation.as_mut().unwrap(); let texture = resource::Texture { inner: resource::TextureInner::Surface { @@ -158,7 +185,10 @@ impl Global { levels: 0..1, }, life_guard: LifeGuard::new(""), - clear_mode: resource::TextureClearMode::None, // TODO: Shouldn't there be a view? + clear_mode: resource::TextureClearMode::RenderPass { + clear_views, + is_color: true, + }, }; let ref_count = texture.life_guard.add_ref(); @@ -240,6 +270,16 @@ impl Global { let (texture, _) = hub.textures.unregister(texture_id.value.0, &mut token); if let Some(texture) = texture { + if let resource::TextureClearMode::RenderPass { clear_views, .. } = + texture.clear_mode + { + for clear_view in clear_views { + unsafe { + hal::Device::destroy_texture_view(&device.raw, clear_view); + } + } + } + let suf = A::get_surface_mut(surface); match texture.inner { resource::TextureInner::Surface { diff --git a/wgpu/examples/boids/main.rs b/wgpu/examples/boids/main.rs index 919ba37074..043d4bc3f9 100644 --- a/wgpu/examples/boids/main.rs +++ b/wgpu/examples/boids/main.rs @@ -279,7 +279,9 @@ impl framework::Example for Example { view, resolve_target: None, ops: wgpu::Operations { - load: wgpu::LoadOp::Clear(wgpu::Color::BLACK), + // Not clearing here in order to test wgpu's zero texture initialization on a surface texture. + // Users should avoid loading uninitialized memory since this can cause additional overhead. + load: wgpu::LoadOp::Load, store: true, }, }];