From ade628d3679f9f87b8512d1c42b7e7f554acc9b9 Mon Sep 17 00:00:00 2001 From: Nicolas Silva Date: Fri, 5 Jan 2024 16:15:38 +0100 Subject: [PATCH] Propagate errors when openning/closing a command encoder --- wgpu-core/src/command/clear.rs | 7 ++-- wgpu-core/src/command/compute.rs | 21 ++++++----- wgpu-core/src/command/mod.rs | 59 +++++++++++++++++++------------ wgpu-core/src/command/query.rs | 7 ++-- wgpu-core/src/command/render.rs | 20 +++++------ wgpu-core/src/command/transfer.rs | 33 ++++++++++------- 6 files changed, 89 insertions(+), 58 deletions(-) diff --git a/wgpu-core/src/command/clear.rs b/wgpu-core/src/command/clear.rs index 3a6ef14e37..1a4b4cdeb1 100644 --- a/wgpu-core/src/command/clear.rs +++ b/wgpu-core/src/command/clear.rs @@ -5,6 +5,7 @@ use crate::device::trace::Command as TraceCommand; use crate::{ api_log, command::CommandBuffer, + device::DeviceError, get_lowest_common_denom, global::Global, hal_api::HalApi, @@ -66,6 +67,8 @@ whereas subesource range specified start {subresource_base_array_layer} and coun subresource_base_array_layer: u32, subresource_array_layer_count: Option, }, + #[error(transparent)] + Device(#[from] DeviceError), } impl Global { @@ -149,7 +152,7 @@ impl Global { // actual hal barrier & operation let dst_barrier = dst_pending.map(|pending| pending.into_hal(&dst_buffer, &snatch_guard)); - let cmd_buf_raw = cmd_buf_data.encoder.open(); + let cmd_buf_raw = cmd_buf_data.encoder.open()?; unsafe { cmd_buf_raw.transition_buffers(dst_barrier.into_iter()); cmd_buf_raw.clear_buffer(dst_raw, offset..end); @@ -228,7 +231,7 @@ impl Global { if !device.is_valid() { return Err(ClearError::InvalidDevice(cmd_buf.device.as_info().id())); } - let (encoder, tracker) = cmd_buf_data.open_encoder_and_tracker(); + let (encoder, tracker) = cmd_buf_data.open_encoder_and_tracker()?; clear_texture( &dst_texture, diff --git a/wgpu-core/src/command/compute.rs b/wgpu-core/src/command/compute.rs index 5be83a6b14..0533f29c3b 100644 --- a/wgpu-core/src/command/compute.rs +++ b/wgpu-core/src/command/compute.rs @@ -1,3 +1,4 @@ +use crate::device::DeviceError; use crate::resource::Resource; use crate::snatch::SnatchGuard; use crate::{ @@ -186,6 +187,8 @@ pub enum DispatchError { /// Error encountered when performing a compute pass. #[derive(Clone, Debug, Error)] pub enum ComputePassErrorInner { + #[error(transparent)] + Device(#[from] DeviceError), #[error(transparent)] Encoder(#[from] CommandEncoderError), #[error("Bind group at index {0:?} is invalid")] @@ -366,17 +369,17 @@ impl Global { timestamp_writes: Option<&ComputePassTimestampWrites>, ) -> Result<(), ComputePassError> { profiling::scope!("CommandEncoder::run_compute_pass"); - let init_scope = PassErrorScope::Pass(encoder_id); + let pass_scope = PassErrorScope::Pass(encoder_id); let hub = A::hub(self); - let cmd_buf = CommandBuffer::get_encoder(hub, encoder_id).map_pass_err(init_scope)?; + let cmd_buf = CommandBuffer::get_encoder(hub, encoder_id).map_pass_err(pass_scope)?; let device = &cmd_buf.device; if !device.is_valid() { return Err(ComputePassErrorInner::InvalidDevice( cmd_buf.device.as_info().id(), )) - .map_pass_err(init_scope); + .map_pass_err(pass_scope); } let mut cmd_buf_data = cmd_buf.data.lock(); @@ -399,10 +402,10 @@ impl Global { // We automatically keep extending command buffers over time, and because // we want to insert a command buffer _before_ what we're about to record, // we need to make sure to close the previous one. - encoder.close(); + encoder.close().map_pass_err(pass_scope)?; // will be reset to true if recording is done without errors *status = CommandEncoderStatus::Error; - let raw = encoder.open(); + let raw = encoder.open().map_pass_err(pass_scope)?; let bind_group_guard = hub.bind_groups.read(); let pipeline_guard = hub.compute_pipelines.read(); @@ -426,7 +429,7 @@ impl Global { .query_sets .add_single(&*query_set_guard, tw.query_set) .ok_or(ComputePassErrorInner::InvalidQuerySet(tw.query_set)) - .map_pass_err(init_scope)?; + .map_pass_err(pass_scope)?; // Unlike in render passes we can't delay resetting the query sets since // there is no auxillary pass. @@ -862,12 +865,12 @@ impl Global { *status = CommandEncoderStatus::Recording; // Stop the current command buffer. - encoder.close(); + encoder.close().map_pass_err(pass_scope)?; // Create a new command buffer, which we will insert _before_ the body of the compute pass. // // Use that buffer to insert barriers and clear discarded images. - let transit = encoder.open(); + let transit = encoder.open().map_pass_err(pass_scope)?; fixup_discarded_surfaces( pending_discard_init_fixups.into_iter(), transit, @@ -881,7 +884,7 @@ impl Global { &snatch_guard, ); // Close the command buffer, and swap it with the previous. - encoder.close_and_swap(); + encoder.close_and_swap().map_pass_err(pass_scope)?; Ok(()) } diff --git a/wgpu-core/src/command/mod.rs b/wgpu-core/src/command/mod.rs index dabd899908..cc1c5fcea5 100644 --- a/wgpu-core/src/command/mod.rs +++ b/wgpu-core/src/command/mod.rs @@ -18,7 +18,7 @@ pub use self::{ use self::memory_init::CommandBufferTextureMemoryActions; -use crate::device::Device; +use crate::device::{Device, DeviceError}; use crate::error::{ErrorFormatter, PrettyError}; use crate::hub::Hub; use crate::id::CommandBufferId; @@ -58,20 +58,24 @@ pub(crate) struct CommandEncoder { //TODO: handle errors better impl CommandEncoder { /// Closes the live encoder - fn close_and_swap(&mut self) { + fn close_and_swap(&mut self) -> Result<(), DeviceError> { if self.is_open { self.is_open = false; - let new = unsafe { self.raw.end_encoding().unwrap() }; + let new = unsafe { self.raw.end_encoding()? }; self.list.insert(self.list.len() - 1, new); } + + Ok(()) } - fn close(&mut self) { + fn close(&mut self) -> Result<(), DeviceError> { if self.is_open { self.is_open = false; - let cmd_buf = unsafe { self.raw.end_encoding().unwrap() }; + let cmd_buf = unsafe { self.raw.end_encoding()? }; self.list.push(cmd_buf); } + + Ok(()) } fn discard(&mut self) { @@ -81,18 +85,21 @@ impl CommandEncoder { } } - fn open(&mut self) -> &mut A::CommandEncoder { + fn open(&mut self) -> Result<&mut A::CommandEncoder, DeviceError> { if !self.is_open { self.is_open = true; let label = self.label.as_deref(); - unsafe { self.raw.begin_encoding(label).unwrap() }; + unsafe { self.raw.begin_encoding(label)? }; } - &mut self.raw + + Ok(&mut self.raw) } - fn open_pass(&mut self, label: Option<&str>) { + fn open_pass(&mut self, label: Option<&str>) -> Result<(), DeviceError> { self.is_open = true; - unsafe { self.raw.begin_encoding(label).unwrap() }; + unsafe { self.raw.begin_encoding(label)? }; + + Ok(()) } } @@ -119,10 +126,13 @@ pub struct CommandBufferMutable { } impl CommandBufferMutable { - pub(crate) fn open_encoder_and_tracker(&mut self) -> (&mut A::CommandEncoder, &mut Tracker) { - let encoder = self.encoder.open(); + pub(crate) fn open_encoder_and_tracker( + &mut self, + ) -> Result<(&mut A::CommandEncoder, &mut Tracker), DeviceError> { + let encoder = self.encoder.open()?; let tracker = &mut self.trackers; - (encoder, tracker) + + Ok((encoder, tracker)) } } @@ -401,6 +411,8 @@ pub enum CommandEncoderError { Invalid, #[error("Command encoder must be active")] NotRecording, + #[error(transparent)] + Device(#[from] DeviceError), } impl Global { @@ -419,12 +431,15 @@ impl Global { let cmd_buf_data = cmd_buf_data.as_mut().unwrap(); match cmd_buf_data.status { CommandEncoderStatus::Recording => { - cmd_buf_data.encoder.close(); - cmd_buf_data.status = CommandEncoderStatus::Finished; - //Note: if we want to stop tracking the swapchain texture view, - // this is the place to do it. - log::trace!("Command buffer {:?}", encoder_id); - None + if let Err(e) = cmd_buf_data.encoder.close() { + Some(e.into()) + } else { + cmd_buf_data.status = CommandEncoderStatus::Finished; + //Note: if we want to stop tracking the swapchain texture view, + // this is the place to do it. + log::trace!("Command buffer {:?}", encoder_id); + None + } } CommandEncoderStatus::Finished => Some(CommandEncoderError::NotRecording), CommandEncoderStatus::Error => { @@ -457,7 +472,7 @@ impl Global { list.push(TraceCommand::PushDebugGroup(label.to_string())); } - let cmd_buf_raw = cmd_buf_data.encoder.open(); + let cmd_buf_raw = cmd_buf_data.encoder.open()?; if !self .instance .flags @@ -494,7 +509,7 @@ impl Global { .flags .contains(wgt::InstanceFlags::DISCARD_HAL_LABELS) { - let cmd_buf_raw = cmd_buf_data.encoder.open(); + let cmd_buf_raw = cmd_buf_data.encoder.open()?; unsafe { cmd_buf_raw.insert_debug_marker(label); } @@ -520,7 +535,7 @@ impl Global { list.push(TraceCommand::PopDebugGroup); } - let cmd_buf_raw = cmd_buf_data.encoder.open(); + let cmd_buf_raw = cmd_buf_data.encoder.open()?; if !self .instance .flags diff --git a/wgpu-core/src/command/query.rs b/wgpu-core/src/command/query.rs index daf5ed47c8..8c1f4b8f12 100644 --- a/wgpu-core/src/command/query.rs +++ b/wgpu-core/src/command/query.rs @@ -4,6 +4,7 @@ use hal::CommandEncoder as _; use crate::device::trace::Command as TraceCommand; use crate::{ command::{CommandBuffer, CommandEncoderError}, + device::DeviceError, global::Global, hal_api::HalApi, id::{self, Id, TypedId}, @@ -104,6 +105,8 @@ impl From for SimplifiedQueryType { #[derive(Clone, Debug, Error)] #[non_exhaustive] pub enum QueryError { + #[error(transparent)] + Device(#[from] DeviceError), #[error(transparent)] Encoder(#[from] CommandEncoderError), #[error("Error encountered while trying to use queries")] @@ -367,7 +370,7 @@ impl Global { let encoder = &mut cmd_buf_data.encoder; let tracker = &mut cmd_buf_data.trackers; - let raw_encoder = encoder.open(); + let raw_encoder = encoder.open()?; let query_set_guard = hub.query_sets.read(); let query_set = tracker @@ -409,7 +412,7 @@ impl Global { let encoder = &mut cmd_buf_data.encoder; let tracker = &mut cmd_buf_data.trackers; let buffer_memory_init_actions = &mut cmd_buf_data.buffer_memory_init_actions; - let raw_encoder = encoder.open(); + let raw_encoder = encoder.open()?; if destination_offset % wgt::QUERY_RESOLVE_BUFFER_ALIGNMENT != 0 { return Err(QueryError::Resolve(ResolveError::BufferOffsetAlignment)); diff --git a/wgpu-core/src/command/render.rs b/wgpu-core/src/command/render.rs index d962a8135f..700a8afbd4 100644 --- a/wgpu-core/src/command/render.rs +++ b/wgpu-core/src/command/render.rs @@ -1312,11 +1312,11 @@ impl Global { .contains(wgt::InstanceFlags::DISCARD_HAL_LABELS); let label = hal_label(base.label, self.instance.flags); - let init_scope = PassErrorScope::Pass(encoder_id); + let pass_scope = PassErrorScope::Pass(encoder_id); let hub = A::hub(self); - let cmd_buf = CommandBuffer::get_encoder(hub, encoder_id).map_pass_err(init_scope)?; + let cmd_buf = CommandBuffer::get_encoder(hub, encoder_id).map_pass_err(pass_scope)?; let device = &cmd_buf.device; let snatch_guard = device.snatchable_lock.read(); @@ -1336,7 +1336,7 @@ impl Global { } if !device.is_valid() { - return Err(DeviceError::Lost).map_pass_err(init_scope); + return Err(DeviceError::Lost).map_pass_err(pass_scope); } let encoder = &mut cmd_buf_data.encoder; @@ -1349,10 +1349,10 @@ impl Global { // We automatically keep extending command buffers over time, and because // we want to insert a command buffer _before_ what we're about to record, // we need to make sure to close the previous one. - encoder.close(); + encoder.close().map_pass_err(pass_scope)?; // We will reset this to `Recording` if we succeed, acts as a fail-safe. *status = CommandEncoderStatus::Error; - encoder.open_pass(label); + encoder.open_pass(label).map_pass_err(pass_scope)?; let bundle_guard = hub.render_bundles.read(); let bind_group_guard = hub.bind_groups.read(); @@ -1383,7 +1383,7 @@ impl Global { &*texture_guard, &*query_set_guard, ) - .map_pass_err(init_scope)?; + .map_pass_err(pass_scope)?; tracker.set_size( Some(&*buffer_guard), @@ -2364,9 +2364,9 @@ impl Global { log::trace!("Merging renderpass into cmd_buf {:?}", encoder_id); let (trackers, pending_discard_init_fixups) = - info.finish(raw).map_pass_err(init_scope)?; + info.finish(raw).map_pass_err(pass_scope)?; - encoder.close(); + encoder.close().map_pass_err(pass_scope)?; (trackers, pending_discard_init_fixups) }; @@ -2381,7 +2381,7 @@ impl Global { let tracker = &mut cmd_buf_data.trackers; { - let transit = encoder.open(); + let transit = encoder.open().map_pass_err(pass_scope)?; fixup_discarded_surfaces( pending_discard_init_fixups.into_iter(), @@ -2409,7 +2409,7 @@ impl Global { } *status = CommandEncoderStatus::Recording; - encoder.close_and_swap(); + encoder.close_and_swap().map_pass_err(pass_scope)?; Ok(()) } diff --git a/wgpu-core/src/command/transfer.rs b/wgpu-core/src/command/transfer.rs index 9f8f8dfc34..6bde17c646 100644 --- a/wgpu-core/src/command/transfer.rs +++ b/wgpu-core/src/command/transfer.rs @@ -4,7 +4,7 @@ use crate::{ api_log, command::{clear_texture, CommandBuffer, CommandEncoderError}, conv, - device::{Device, MissingDownlevelFlags}, + device::{Device, DeviceError, MissingDownlevelFlags}, error::{ErrorFormatter, PrettyError}, global::Global, hal_api::HalApi, @@ -25,7 +25,7 @@ use wgt::{BufferAddress, BufferUsages, Extent3d, TextureUsages}; use std::{iter, sync::Arc}; -use super::{memory_init::CommandBufferTextureMemoryActions, CommandEncoder}; +use super::{memory_init::CommandBufferTextureMemoryActions, ClearError, CommandEncoder}; pub type ImageCopyBuffer = wgt::ImageCopyBuffer; pub type ImageCopyTexture = wgt::ImageCopyTexture; @@ -135,7 +135,7 @@ pub enum TransferError { dst_format: wgt::TextureFormat, }, #[error(transparent)] - MemoryInitFailure(#[from] super::ClearError), + MemoryInitFailure(#[from] ClearError), #[error("Cannot encode this copy because of a missing downelevel flag")] MissingDownlevelFlags(#[from] MissingDownlevelFlags), #[error("Source texture sample count must be 1, got {sample_count}")] @@ -186,6 +186,12 @@ pub enum CopyError { Transfer(#[from] TransferError), } +impl From for CopyError { + fn from(err: DeviceError) -> Self { + CopyError::Encoder(CommandEncoderError::Device(err)) + } +} + pub(crate) fn extract_texture_selector( copy_texture: &ImageCopyTexture, copy_size: &Extent3d, @@ -447,7 +453,7 @@ fn handle_texture_init( copy_texture: &ImageCopyTexture, copy_size: &Extent3d, texture: &Arc>, -) { +) -> Result<(), ClearError> { let init_action = TextureInitTrackerAction { texture: texture.clone(), range: TextureInitRange { @@ -463,7 +469,7 @@ 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 = encoder.open(); + let cmd_buf_raw = encoder.open()?; for init in immediate_inits { clear_texture( &init.texture, @@ -475,10 +481,11 @@ fn handle_texture_init( &mut trackers.textures, &device.alignments, device.zero_buffer.as_ref().unwrap(), - ) - .unwrap(); + )?; } } + + Ok(()) } /// Prepare a transfer's source texture. @@ -503,7 +510,7 @@ fn handle_src_texture_init( source, copy_size, texture, - ); + )?; Ok(()) } @@ -543,7 +550,7 @@ fn handle_dst_texture_init( destination, copy_size, texture, - ); + )?; Ok(()) } @@ -707,7 +714,7 @@ impl Global { dst_offset: destination_offset, size: wgt::BufferSize::new(size).unwrap(), }; - let cmd_buf_raw = cmd_buf_data.encoder.open(); + let cmd_buf_raw = cmd_buf_data.encoder.open()?; unsafe { cmd_buf_raw.transition_buffers(src_barrier.into_iter().chain(dst_barrier)); cmd_buf_raw.copy_buffer_to_buffer(src_raw, dst_raw, iter::once(region)); @@ -867,7 +874,7 @@ impl Global { } }); - let cmd_buf_raw = encoder.open(); + let cmd_buf_raw = encoder.open()?; unsafe { cmd_buf_raw.transition_textures(dst_barrier.into_iter()); cmd_buf_raw.transition_buffers(src_barrier.into_iter()); @@ -1035,7 +1042,7 @@ impl Global { size: hal_copy_size, } }); - let cmd_buf_raw = encoder.open(); + let cmd_buf_raw = encoder.open()?; unsafe { cmd_buf_raw.transition_buffers(dst_barrier.into_iter()); cmd_buf_raw.transition_textures(src_barrier.into_iter()); @@ -1207,7 +1214,7 @@ impl Global { size: hal_copy_size, } }); - let cmd_buf_raw = cmd_buf_data.encoder.open(); + let cmd_buf_raw = cmd_buf_data.encoder.open()?; unsafe { cmd_buf_raw.transition_textures(barriers.into_iter()); cmd_buf_raw.copy_texture_to_texture(