From c630821f1dbd3bb6f6dc91197977c2b2a2e72246 Mon Sep 17 00:00:00 2001 From: teoxoy <28601907+teoxoy@users.noreply.github.com> Date: Fri, 6 Sep 2024 20:56:24 +0200 Subject: [PATCH] [wgpu-core] use `Fallible` for `Texture` --- tests/tests/resource_error.rs | 11 +-------- wgpu-core/src/command/clear.rs | 7 +----- wgpu-core/src/command/transfer.rs | 22 ++++------------- wgpu-core/src/device/global.rs | 39 +++++++++++++++---------------- wgpu-core/src/device/queue.rs | 13 ++--------- wgpu-core/src/hub.rs | 2 +- wgpu-core/src/present.rs | 10 ++++---- wgpu-core/src/resource.rs | 8 +++---- 8 files changed, 36 insertions(+), 76 deletions(-) diff --git a/tests/tests/resource_error.rs b/tests/tests/resource_error.rs index 0c58f7a13..d071053eb 100644 --- a/tests/tests/resource_error.rs +++ b/tests/tests/resource_error.rs @@ -54,21 +54,12 @@ static BAD_TEXTURE: GpuTestConfiguration = GpuTestConfiguration::new().run_sync( Some("dimension x is zero"), ); - let error = match ctx.adapter_info.backend.to_str() { - "vulkan" | "vk" => "textureid id(0,1,vk) is invalid", - "dx12" | "d3d12" => "textureid id(0,1,d3d12) is invalid", - "metal" | "mtl" => "textureid id(0,1,mtl) is invalid", - "opengl" | "gles" | "gl" => "textureid id(0,1,gl) is invalid", - "webgpu" => "textureid id(0,1,webgpu) is invalid", - b => b, - }; - fail( &ctx.device, || { let _ = texture.create_view(&wgpu::TextureViewDescriptor::default()); }, - Some(error), + Some("Texture with '' label is invalid"), ); valid(&ctx.device, || texture.destroy()); valid(&ctx.device, || texture.destroy()); diff --git a/wgpu-core/src/command/clear.rs b/wgpu-core/src/command/clear.rs index 0000a2be5..20b350f57 100644 --- a/wgpu-core/src/command/clear.rs +++ b/wgpu-core/src/command/clear.rs @@ -27,8 +27,6 @@ use wgt::{math::align_to, BufferAddress, BufferUsages, ImageSubresourceRange, Te pub enum ClearError { #[error("To use clear_texture the CLEAR_TEXTURE feature needs to be enabled")] MissingClearTextureFeature, - #[error("TextureId {0:?} is invalid")] - InvalidTextureId(TextureId), #[error(transparent)] DestroyedResource(#[from] DestroyedResourceError), #[error("{0} can not be cleared")] @@ -203,10 +201,7 @@ impl Global { return Err(ClearError::MissingClearTextureFeature); } - let dst_texture = hub - .textures - .get(dst) - .map_err(|_| ClearError::InvalidTextureId(dst))?; + let dst_texture = hub.textures.strict_get(dst).get()?; dst_texture.same_device_as(cmd_buf.as_ref())?; diff --git a/wgpu-core/src/command/transfer.rs b/wgpu-core/src/command/transfer.rs index 463e62b51..c90a8a84f 100644 --- a/wgpu-core/src/command/transfer.rs +++ b/wgpu-core/src/command/transfer.rs @@ -41,8 +41,6 @@ pub enum CopySide { #[derive(Clone, Debug, Error)] #[non_exhaustive] pub enum TransferError { - #[error("TextureId {0:?} is invalid")] - InvalidTextureId(TextureId), #[error("Source and destination cannot be the same buffer")] SameSourceDestinationBuffer, #[error(transparent)] @@ -740,10 +738,7 @@ impl Global { return Ok(()); } - let dst_texture = hub - .textures - .get(destination.texture) - .map_err(|_| TransferError::InvalidTextureId(destination.texture))?; + let dst_texture = hub.textures.strict_get(destination.texture).get()?; dst_texture.same_device_as(cmd_buf.as_ref())?; @@ -904,10 +899,7 @@ impl Global { return Ok(()); } - let src_texture = hub - .textures - .get(source.texture) - .map_err(|_| TransferError::InvalidTextureId(source.texture))?; + let src_texture = hub.textures.strict_get(source.texture).get()?; src_texture.same_device_as(cmd_buf.as_ref())?; @@ -1082,14 +1074,8 @@ impl Global { return Ok(()); } - let src_texture = hub - .textures - .get(source.texture) - .map_err(|_| TransferError::InvalidTextureId(source.texture))?; - let dst_texture = hub - .textures - .get(destination.texture) - .map_err(|_| TransferError::InvalidTextureId(source.texture))?; + let src_texture = hub.textures.strict_get(source.texture).get()?; + let dst_texture = hub.textures.strict_get(destination.texture).get()?; src_texture.same_device_as(cmd_buf.as_ref())?; dst_texture.same_device_as(cmd_buf.as_ref())?; diff --git a/wgpu-core/src/device/global.rs b/wgpu-core/src/device/global.rs index 0deebf200..f2f51a09c 100644 --- a/wgpu-core/src/device/global.rs +++ b/wgpu-core/src/device/global.rs @@ -196,11 +196,14 @@ impl Global { /// Assign `id_in` an error with the given `label`. /// /// See `create_buffer_error` for more context and explanation. - pub fn create_texture_error(&self, backend: wgt::Backend, id_in: Option) { - let hub = &self.hub; - let fid = hub.textures.prepare(backend, id_in); - - fid.assign_error(); + pub fn create_texture_error( + &self, + backend: wgt::Backend, + id_in: Option, + desc: &resource::TextureDescriptor, + ) { + let fid = self.hub.textures.prepare(backend, id_in); + fid.assign(Fallible::Invalid(Arc::new(desc.label.to_string()))); } #[cfg(feature = "replay")] @@ -323,7 +326,7 @@ impl Global { Err(error) => break 'error error, }; - let id = fid.assign(texture); + let id = fid.assign(Fallible::Valid(texture)); api_log!("Device::create_texture({desc:?}) -> {id:?}"); return (id, None); @@ -331,7 +334,7 @@ impl Global { log::error!("Device::create_texture error: {error}"); - let id = fid.assign_error(); + let id = fid.assign(Fallible::Invalid(Arc::new(desc.label.to_string()))); (id, Some(error)) } @@ -368,7 +371,7 @@ impl Global { Err(error) => break 'error error, }; - let id = fid.assign(texture); + let id = fid.assign(Fallible::Valid(texture)); api_log!("Device::create_texture({desc:?}) -> {id:?}"); return (id, None); @@ -376,7 +379,7 @@ impl Global { log::error!("Device::create_texture error: {error}"); - let id = fid.assign_error(); + let id = fid.assign(Fallible::Invalid(Arc::new(desc.label.to_string()))); (id, Some(error)) } @@ -420,10 +423,7 @@ impl Global { let hub = &self.hub; - let texture = hub - .textures - .get(texture_id) - .map_err(|_| resource::DestroyError::InvalidTextureId(texture_id))?; + let texture = hub.textures.strict_get(texture_id).get()?; #[cfg(feature = "trace")] if let Some(trace) = texture.device.trace.lock().as_mut() { @@ -439,9 +439,10 @@ impl Global { let hub = &self.hub; - if let Some(_texture) = hub.textures.unregister(texture_id) { - #[cfg(feature = "trace")] - if let Some(t) = _texture.device.trace.lock().as_mut() { + let _texture = hub.textures.strict_unregister(texture_id); + #[cfg(feature = "trace")] + if let Ok(texture) = _texture.get() { + if let Some(t) = texture.device.trace.lock().as_mut() { t.add(trace::Action::DestroyTexture(texture_id)); } } @@ -460,11 +461,9 @@ impl Global { let fid = hub.texture_views.prepare(texture_id.backend(), id_in); let error = 'error: { - let texture = match hub.textures.get(texture_id) { + let texture = match hub.textures.strict_get(texture_id).get() { Ok(texture) => texture, - Err(_) => { - break 'error resource::CreateTextureViewError::InvalidTextureId(texture_id) - } + Err(e) => break 'error e.into(), }; let device = &texture.device; diff --git a/wgpu-core/src/device/queue.rs b/wgpu-core/src/device/queue.rs index bde4003f9..6139a5f3b 100644 --- a/wgpu-core/src/device/queue.rs +++ b/wgpu-core/src/device/queue.rs @@ -619,10 +619,7 @@ impl Global { return Ok(()); } - let dst = hub - .textures - .get(destination.texture) - .map_err(|_| TransferError::InvalidTextureId(destination.texture))?; + let dst = hub.textures.strict_get(destination.texture).get()?; dst.same_device_as(queue.as_ref())?; @@ -713,12 +710,6 @@ impl Global { let snatch_guard = device.snatchable_lock.read(); - // Re-get `dst` immutably here, so that the mutable borrow of the - // `texture_guard.get` above ends in time for the `clear_texture` - // call above. Since we've held `texture_guard` the whole time, we know - // the texture hasn't gone away in the mean time, so we can unwrap. - let dst = hub.textures.get(destination.texture).unwrap(); - let dst_raw = dst.try_raw(&snatch_guard)?; let (block_width, block_height) = dst.desc.format.block_dimensions(); @@ -865,7 +856,7 @@ impl Global { let src_width = source.source.width(); let src_height = source.source.height(); - let dst = hub.textures.get(destination.texture).unwrap(); + let dst = hub.textures.strict_get(destination.texture).get()?; if !conv::is_valid_external_image_copy_dst_texture_format(dst.desc.format) { return Err( diff --git a/wgpu-core/src/hub.rs b/wgpu-core/src/hub.rs index 3fb2e7963..d7a56f370 100644 --- a/wgpu-core/src/hub.rs +++ b/wgpu-core/src/hub.rs @@ -177,7 +177,7 @@ pub struct Hub { pub(crate) query_sets: Registry>, pub(crate) buffers: Registry>, pub(crate) staging_buffers: Registry>, - pub(crate) textures: Registry>, + pub(crate) textures: Registry>, pub(crate) texture_views: Registry>, pub(crate) samplers: Registry>, } diff --git a/wgpu-core/src/present.rs b/wgpu-core/src/present.rs index 480772e27..95f0dae77 100644 --- a/wgpu-core/src/present.rs +++ b/wgpu-core/src/present.rs @@ -215,7 +215,7 @@ impl Global { .textures .insert_single(&texture, hal::TextureUses::UNINITIALIZED); - let id = fid.assign(texture); + let id = fid.assign(resource::Fallible::Valid(texture)); if present.acquired_texture.is_some() { return Err(SurfaceError::AlreadyAcquired); @@ -280,8 +280,8 @@ impl Global { // The texture ID got added to the device tracker by `submit()`, // and now we are moving it away. - let texture = hub.textures.unregister(texture_id); - if let Some(texture) = texture { + let texture = hub.textures.strict_unregister(texture_id).get(); + if let Ok(texture) = texture { device .trackers .lock() @@ -350,9 +350,9 @@ impl Global { // The texture ID got added to the device tracker by `submit()`, // and now we are moving it away. - let texture = hub.textures.unregister(texture_id); + let texture = hub.textures.strict_unregister(texture_id).get(); - if let Some(texture) = texture { + if let Ok(texture) = texture { device .trackers .lock() diff --git a/wgpu-core/src/resource.rs b/wgpu-core/src/resource.rs index 8e61ad11e..8d6ab7b4c 100644 --- a/wgpu-core/src/resource.rs +++ b/wgpu-core/src/resource.rs @@ -1266,7 +1266,7 @@ impl Global { let hub = &self.hub; - if let Ok(texture) = hub.textures.get(id) { + if let Ok(texture) = hub.textures.strict_get(id).get() { let snatch_guard = texture.device.snatchable_lock.read(); let hal_texture = texture.raw(&snatch_guard); let hal_texture = hal_texture @@ -1646,8 +1646,6 @@ impl TextureView { pub enum CreateTextureViewError { #[error(transparent)] Device(#[from] DeviceError), - #[error("TextureId {0:?} is invalid")] - InvalidTextureId(TextureId), #[error(transparent)] DestroyedResource(#[from] DestroyedResourceError), #[error("Not enough memory left to create texture view")] @@ -1690,6 +1688,8 @@ pub enum CreateTextureViewError { texture: wgt::TextureFormat, view: wgt::TextureFormat, }, + #[error(transparent)] + InvalidResource(#[from] InvalidResourceError), } #[derive(Clone, Debug, Error)] @@ -1862,8 +1862,6 @@ impl QuerySet { #[derive(Clone, Debug, Error)] #[non_exhaustive] pub enum DestroyError { - #[error("TextureId {0:?} is invalid")] - InvalidTextureId(TextureId), #[error("Resource is already destroyed")] AlreadyDestroyed, #[error(transparent)]