diff --git a/wgpu-core/src/command/clear.rs b/wgpu-core/src/command/clear.rs index b22c01f5e9..a2679ba566 100644 --- a/wgpu-core/src/command/clear.rs +++ b/wgpu-core/src/command/clear.rs @@ -91,8 +91,8 @@ impl Global { let cmd_buf = hub .command_buffers .get(command_encoder_id.into_command_buffer_id()); - let mut cmd_buf_data = cmd_buf.try_get()?; - cmd_buf_data.check_recording()?; + let mut cmd_buf_data = cmd_buf.data.lock(); + let cmd_buf_data = cmd_buf_data.record()?; #[cfg(feature = "trace")] if let Some(ref mut list) = cmd_buf_data.commands { @@ -174,8 +174,8 @@ impl Global { let cmd_buf = hub .command_buffers .get(command_encoder_id.into_command_buffer_id()); - let mut cmd_buf_data = cmd_buf.try_get()?; - cmd_buf_data.check_recording()?; + let mut cmd_buf_data = cmd_buf.data.lock(); + let cmd_buf_data = cmd_buf_data.record()?; #[cfg(feature = "trace")] if let Some(ref mut list) = cmd_buf_data.commands { diff --git a/wgpu-core/src/command/compute.rs b/wgpu-core/src/command/compute.rs index 89681b8e27..0f236533f4 100644 --- a/wgpu-core/src/command/compute.rs +++ b/wgpu-core/src/command/compute.rs @@ -8,8 +8,8 @@ use crate::{ end_pipeline_statistics_query, memory_init::{fixup_discarded_surfaces, SurfacesInDiscardState}, validate_and_begin_pipeline_statistics_query, ArcPassTimestampWrites, BasePass, - BindGroupStateChange, CommandBuffer, CommandEncoderError, CommandEncoderStatus, MapPassErr, - PassErrorScope, PassTimestampWrites, QueryUseError, StateChange, + BindGroupStateChange, CommandBuffer, CommandEncoderError, MapPassErr, PassErrorScope, + PassTimestampWrites, QueryUseError, StateChange, }, device::{Device, DeviceError, MissingDownlevelFlags, MissingFeatures}, global::Global, @@ -30,8 +30,7 @@ use wgt::{BufferAddress, DynamicOffset}; use super::{bind::BinderError, memory_init::CommandBufferTextureMemoryActions}; use crate::ray_tracing::TlasAction; -use std::sync::Arc; -use std::{fmt, mem::size_of, str}; +use std::{fmt, mem::size_of, str, sync::Arc}; pub struct ComputePass { /// All pass data & records is stored here. @@ -282,7 +281,9 @@ impl Global { /// If creation fails, an invalid pass is returned. /// Any operation on an invalid pass will return an error. /// - /// If successful, puts the encoder into the [`CommandEncoderStatus::Locked`] state. + /// If successful, puts the encoder into the [`Locked`] state. + /// + /// [`Locked`]: crate::command::CommandEncoderStatus::Locked pub fn command_encoder_create_compute_pass( &self, encoder_id: id::CommandEncoderId, @@ -299,11 +300,7 @@ impl Global { let cmd_buf = hub.command_buffers.get(encoder_id.into_command_buffer_id()); - match cmd_buf - .try_get() - .map_err(|e| e.into()) - .and_then(|mut cmd_buf_data| cmd_buf_data.lock_encoder()) - { + match cmd_buf.data.lock().lock_encoder() { Ok(_) => {} Err(e) => return make_err(e, arc_desc), }; @@ -340,7 +337,8 @@ impl Global { .hub .command_buffers .get(encoder_id.into_command_buffer_id()); - let mut cmd_buf_data = cmd_buf.try_get().map_pass_err(pass_scope)?; + let mut cmd_buf_data = cmd_buf.data.lock(); + let cmd_buf_data = cmd_buf_data.get_inner().map_pass_err(pass_scope)?; if let Some(ref mut list) = cmd_buf_data.commands { list.push(crate::device::trace::Command::RunComputePass { @@ -408,19 +406,16 @@ impl Global { let device = &cmd_buf.device; device.check_is_valid().map_pass_err(pass_scope)?; - let mut cmd_buf_data = cmd_buf.try_get().map_pass_err(pass_scope)?; - cmd_buf_data.unlock_encoder().map_pass_err(pass_scope)?; - let cmd_buf_data = &mut *cmd_buf_data; + let mut cmd_buf_data = cmd_buf.data.lock(); + let mut cmd_buf_data_guard = cmd_buf_data.unlock_encoder().map_pass_err(pass_scope)?; + let cmd_buf_data = &mut *cmd_buf_data_guard; let encoder = &mut cmd_buf_data.encoder; - let status = &mut cmd_buf_data.status; // 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(&cmd_buf.device).map_pass_err(pass_scope)?; - // will be reset to true if recording is done without errors - *status = CommandEncoderStatus::Error; let raw_encoder = encoder.open(&cmd_buf.device).map_pass_err(pass_scope)?; let mut state = State { @@ -590,10 +585,6 @@ impl Global { state.raw_encoder.end_compute_pass(); } - // We've successfully recorded the compute pass, bring the - // command buffer out of the error state. - *status = CommandEncoderStatus::Recording; - let State { snatch_guard, tracker, @@ -626,6 +617,7 @@ impl Global { encoder .close_and_swap(&cmd_buf.device) .map_pass_err(pass_scope)?; + cmd_buf_data_guard.mark_successful(); Ok(()) } diff --git a/wgpu-core/src/command/mod.rs b/wgpu-core/src/command/mod.rs index de0f86bce4..cbdcae9bed 100644 --- a/wgpu-core/src/command/mod.rs +++ b/wgpu-core/src/command/mod.rs @@ -13,6 +13,7 @@ mod render_command; mod timestamp_writes; mod transfer; +use std::mem::{self, ManuallyDrop}; use std::sync::Arc; pub(crate) use self::clear::clear_texture; @@ -47,7 +48,6 @@ use crate::device::trace::Command as TraceCommand; const PUSH_CONSTANT_CLEAR_ARRAY: &[u32] = &[0_u32; 64]; /// The current state of a [`CommandBuffer`]. -#[derive(Debug)] pub(crate) enum CommandEncoderStatus { /// Ready to record commands. An encoder's initial state. /// @@ -60,7 +60,7 @@ pub(crate) enum CommandEncoderStatus { /// /// [`command_encoder_clear_buffer`]: Global::command_encoder_clear_buffer /// [`compute_pass_end`]: Global::compute_pass_end - Recording, + Recording(CommandBufferMutable), /// Locked by a render or compute pass. /// @@ -68,9 +68,9 @@ pub(crate) enum CommandEncoderStatus { /// and exited when the pass is ended. /// /// As long as the command encoder is locked, any command building operation on it will fail - /// and put the encoder into the [`CommandEncoderStatus::Error`] state. + /// and put the encoder into the [`Self::Error`] state. /// See - Locked, + Locked(CommandBufferMutable), /// Command recording is complete, and the buffer is ready for submission. /// @@ -79,21 +79,147 @@ pub(crate) enum CommandEncoderStatus { /// /// [`Global::queue_submit`] drops command buffers unless they are /// in this state. - Finished, + Finished(CommandBufferMutable), /// An error occurred while recording a compute or render pass. /// /// When a `CommandEncoder` is left in this state, we have also /// returned an error result from the function that encountered /// the problem. Future attempts to use the encoder (for example, - /// calls to [`CommandBufferMutable::check_recording`]) will also return - /// errors. - /// - /// Calling [`Global::command_encoder_finish`] in this state - /// discards the command buffer under construction. + /// calls to [`Self::record`]) will also return errors. Error, } +impl CommandEncoderStatus { + /// Checks that the encoder is in the [`Self::Recording`] state. + pub(crate) fn record(&mut self) -> Result<&mut CommandBufferMutable, CommandEncoderError> { + match self { + Self::Recording(inner) => Ok(inner), + Self::Locked(_) => { + *self = Self::Error; + Err(CommandEncoderError::Locked) + } + Self::Finished(_) => Err(CommandEncoderError::NotRecording), + Self::Error => Err(CommandEncoderError::Invalid), + } + } + + #[cfg(feature = "trace")] + fn get_inner(&mut self) -> Result<&mut CommandBufferMutable, CommandEncoderError> { + match self { + Self::Locked(inner) | Self::Finished(inner) | Self::Recording(inner) => Ok(inner), + Self::Error => Err(CommandEncoderError::Invalid), + } + } + + /// Locks the encoder by putting it in the [`Self::Locked`] state. + /// + /// Call [`Self::unlock_encoder`] to put the [`CommandBuffer`] back into the [`Self::Recording`] state. + fn lock_encoder(&mut self) -> Result<(), CommandEncoderError> { + match mem::replace(self, Self::Error) { + Self::Recording(inner) => { + *self = Self::Locked(inner); + Ok(()) + } + Self::Finished(inner) => { + *self = Self::Finished(inner); + Err(CommandEncoderError::NotRecording) + } + Self::Locked(_) => Err(CommandEncoderError::Locked), + Self::Error => Err(CommandEncoderError::Invalid), + } + } + + /// Unlocks the [`CommandBuffer`] and puts it back into the [`Self::Recording`] state. + /// + /// This function is the unlocking counterpart to [`Self::lock_encoder`]. + /// + /// It is only valid to call this function if the encoder is in the [`Self::Locked`] state. + fn unlock_encoder(&mut self) -> Result, CommandEncoderError> { + match mem::replace(self, Self::Error) { + Self::Locked(inner) => { + *self = Self::Recording(inner); + Ok(RecordingGuard { inner: self }) + } + Self::Finished(inner) => { + *self = Self::Finished(inner); + Err(CommandEncoderError::NotRecording) + } + Self::Recording(_) => Err(CommandEncoderError::Invalid), + Self::Error => Err(CommandEncoderError::Invalid), + } + } + + fn finish(&mut self, device: &Device) -> Result<(), CommandEncoderError> { + match mem::replace(self, Self::Error) { + Self::Recording(mut inner) => { + if let Err(e) = inner.encoder.close(device) { + Err(e.into()) + } else { + *self = Self::Finished(inner); + // Note: if we want to stop tracking the swapchain texture view, + // this is the place to do it. + Ok(()) + } + } + Self::Finished(inner) => { + *self = Self::Finished(inner); + Err(CommandEncoderError::NotRecording) + } + Self::Locked(_) => Err(CommandEncoderError::Locked), + Self::Error => Err(CommandEncoderError::Invalid), + } + } +} + +/// A guard to enforce error reporting, for a [`CommandBuffer`] in the [`Recording`] state. +/// +/// An [`RecordingGuard`] holds a mutable reference to a [`CommandEncoderStatus`] that +/// has been verified to be in the [`Recording`] state. The [`RecordingGuard`] dereferences +/// mutably to the [`CommandBufferMutable`] that the status holds. +/// +/// Dropping an [`RecordingGuard`] sets the [`CommandBuffer`]'s state to +/// [`CommandEncoderStatus::Error`]. If your use of the guard was +/// successful, call its [`mark_successful`] method to dispose of it. +/// +/// [`Recording`]: CommandEncoderStatus::Recording +/// [`mark_successful`]: Self::mark_successful +pub(crate) struct RecordingGuard<'a> { + inner: &'a mut CommandEncoderStatus, +} + +impl<'a> RecordingGuard<'a> { + pub(crate) fn mark_successful(self) { + mem::forget(self) + } +} + +impl<'a> Drop for RecordingGuard<'a> { + fn drop(&mut self) { + *self.inner = CommandEncoderStatus::Error; + } +} + +impl<'a> std::ops::Deref for RecordingGuard<'a> { + type Target = CommandBufferMutable; + + fn deref(&self) -> &Self::Target { + match &*self.inner { + CommandEncoderStatus::Recording(command_buffer_mutable) => command_buffer_mutable, + _ => unreachable!(), + } + } +} + +impl<'a> std::ops::DerefMut for RecordingGuard<'a> { + fn deref_mut(&mut self) -> &mut Self::Target { + match self.inner { + CommandEncoderStatus::Recording(command_buffer_mutable) => command_buffer_mutable, + _ => unreachable!(), + } + } +} + /// A raw [`CommandEncoder`][rce], and the raw [`CommandBuffer`][rcb]s built from it. /// /// Each wgpu-core [`CommandBuffer`] owns an instance of this type, which is @@ -122,7 +248,7 @@ pub(crate) struct CommandEncoder { /// /// [`CommandEncoder`]: hal::Api::CommandEncoder /// [`CommandAllocator`]: crate::command::CommandAllocator - pub(crate) raw: Box, + pub(crate) raw: ManuallyDrop>, /// All the raw command buffers for our owning [`CommandBuffer`], in /// submission order. @@ -137,6 +263,8 @@ pub(crate) struct CommandEncoder { /// [`wgpu_hal::CommandEncoder`]: hal::CommandEncoder pub(crate) list: Vec>, + pub(crate) device: Arc, + /// True if `raw` is in the "recording" state. /// /// See the documentation for [`wgpu_hal::CommandEncoder`] for @@ -206,16 +334,6 @@ impl CommandEncoder { Ok(()) } - /// Discard the command buffer under construction, if any. - /// - /// The underlying hal encoder is closed, if it was recording. - pub(crate) fn discard(&mut self) { - if self.is_open { - self.is_open = false; - unsafe { self.raw.discard_encoding() }; - } - } - /// Begin recording a new command buffer, if we haven't already. /// /// The underlying hal encoder is put in the "recording" state. @@ -245,6 +363,20 @@ impl CommandEncoder { } } +impl Drop for CommandEncoder { + fn drop(&mut self) { + if self.is_open { + unsafe { self.raw.discard_encoding() }; + } + unsafe { + self.raw.reset_all(mem::take(&mut self.list)); + } + // SAFETY: We are in the Drop impl and we don't use self.raw anymore after this point. + let raw = unsafe { ManuallyDrop::take(&mut self.raw) }; + self.device.command_allocator.release_encoder(raw); + } +} + /// Look at the documentation for [`CommandBufferMutable`] for an explanation of /// the fields in this struct. This is the "built" counterpart to that type. pub(crate) struct BakedCommands { @@ -262,9 +394,6 @@ pub struct CommandBufferMutable { /// [`wgpu_hal::Api::CommandBuffer`]: hal::Api::CommandBuffer pub(crate) encoder: CommandEncoder, - /// The current state of this command buffer's encoder. - status: CommandEncoderStatus, - /// All the resources that the commands recorded so far have referred to. pub(crate) trackers: Tracker, @@ -297,86 +426,6 @@ impl CommandBufferMutable { Ok((encoder, tracker)) } - fn lock_encoder_impl(&mut self, lock: bool) -> Result<(), CommandEncoderError> { - match self.status { - CommandEncoderStatus::Recording => { - if lock { - self.status = CommandEncoderStatus::Locked; - } - Ok(()) - } - CommandEncoderStatus::Locked => { - // Any operation on a locked encoder is required to put it into the invalid/error state. - // See https://www.w3.org/TR/webgpu/#encoder-state-locked - self.encoder.discard(); - self.status = CommandEncoderStatus::Error; - Err(CommandEncoderError::Locked) - } - CommandEncoderStatus::Finished => Err(CommandEncoderError::NotRecording), - CommandEncoderStatus::Error => Err(CommandEncoderError::Invalid), - } - } - - /// Checks that the encoder is in the [`CommandEncoderStatus::Recording`] state. - fn check_recording(&mut self) -> Result<(), CommandEncoderError> { - self.lock_encoder_impl(false) - } - - /// Locks the encoder by putting it in the [`CommandEncoderStatus::Locked`] state. - /// - /// Call [`CommandBufferMutable::unlock_encoder`] to put the [`CommandBuffer`] back into the [`CommandEncoderStatus::Recording`] state. - fn lock_encoder(&mut self) -> Result<(), CommandEncoderError> { - self.lock_encoder_impl(true) - } - - /// Unlocks the [`CommandBuffer`] and puts it back into the [`CommandEncoderStatus::Recording`] state. - /// - /// This function is the counterpart to [`CommandBufferMutable::lock_encoder`]. - /// It is only valid to call this function if the encoder is in the [`CommandEncoderStatus::Locked`] state. - fn unlock_encoder(&mut self) -> Result<(), CommandEncoderError> { - match self.status { - CommandEncoderStatus::Recording => Err(CommandEncoderError::Invalid), - CommandEncoderStatus::Locked => { - self.status = CommandEncoderStatus::Recording; - Ok(()) - } - CommandEncoderStatus::Finished => Err(CommandEncoderError::Invalid), - CommandEncoderStatus::Error => Err(CommandEncoderError::Invalid), - } - } - - pub fn check_finished(&self) -> Result<(), CommandEncoderError> { - match self.status { - CommandEncoderStatus::Finished => Ok(()), - _ => Err(CommandEncoderError::Invalid), - } - } - - pub(crate) fn finish(&mut self, device: &Device) -> Result<(), CommandEncoderError> { - match self.status { - CommandEncoderStatus::Recording => { - if let Err(e) = self.encoder.close(device) { - Err(e.into()) - } else { - self.status = CommandEncoderStatus::Finished; - // Note: if we want to stop tracking the swapchain texture view, - // this is the place to do it. - Ok(()) - } - } - CommandEncoderStatus::Locked => { - self.encoder.discard(); - self.status = CommandEncoderStatus::Error; - Err(CommandEncoderError::Locked) - } - CommandEncoderStatus::Finished => Err(CommandEncoderError::NotRecording), - CommandEncoderStatus::Error => { - self.encoder.discard(); - Err(CommandEncoderError::Invalid) - } - } - } - pub(crate) fn into_baked_commands(self) -> BakedCommands { BakedCommands { encoder: self.encoder, @@ -385,13 +434,6 @@ impl CommandBufferMutable { texture_memory_actions: self.texture_memory_actions, } } - - pub(crate) fn destroy(mut self) { - self.encoder.discard(); - unsafe { - self.encoder.raw.reset_all(self.encoder.list); - } - } } /// A buffer of commands to be submitted to the GPU for execution. @@ -424,15 +466,12 @@ pub struct CommandBuffer { /// When this is submitted, dropped, or destroyed, its contents are /// extracted into a [`BakedCommands`] by /// [`CommandBufferMutable::into_baked_commands`]. - pub(crate) data: Mutex>, + pub(crate) data: Mutex, } impl Drop for CommandBuffer { fn drop(&mut self) { resource_log!("Drop {}", self.error_ident()); - if let Some(data) = self.data.lock().take() { - data.destroy(); - } } } @@ -448,14 +487,14 @@ impl CommandBuffer { label: label.to_string(), data: Mutex::new( rank::COMMAND_BUFFER_DATA, - Some(CommandBufferMutable { + CommandEncoderStatus::Recording(CommandBufferMutable { encoder: CommandEncoder { - raw: encoder, - is_open: false, + raw: ManuallyDrop::new(encoder), list: Vec::new(), + device: device.clone(), + is_open: false, hal_label: label.to_hal(device.instance_flags).map(str::to_owned), }, - status: CommandEncoderStatus::Recording, trackers: Tracker::new(), buffer_memory_init_actions: Default::default(), texture_memory_actions: Default::default(), @@ -478,7 +517,7 @@ impl CommandBuffer { device: device.clone(), support_clear_texture: device.features.contains(wgt::Features::CLEAR_TEXTURE), label: label.to_string(), - data: Mutex::new(rank::COMMAND_BUFFER_DATA, None), + data: Mutex::new(rank::COMMAND_BUFFER_DATA, CommandEncoderStatus::Error), } } @@ -560,19 +599,14 @@ impl CommandBuffer { } impl CommandBuffer { - pub fn try_get<'a>( - &'a self, - ) -> Result, InvalidResourceError> { - let g = self.data.lock(); - crate::lock::MutexGuard::try_map(g, |data| data.as_mut()) - .map_err(|_| InvalidResourceError(self.error_ident())) - } - - pub fn try_take<'a>(&'a self) -> Result { - self.data - .lock() - .take() - .ok_or_else(|| InvalidResourceError(self.error_ident())) + pub fn take_finished<'a>(&'a self) -> Result { + let status = mem::replace(&mut *self.data.lock(), CommandEncoderStatus::Error); + match status { + CommandEncoderStatus::Finished(command_buffer_mutable) => Ok(command_buffer_mutable), + CommandEncoderStatus::Recording(_) + | CommandEncoderStatus::Locked(_) + | CommandEncoderStatus::Error => Err(InvalidResourceError(self.error_ident())), + } } } @@ -672,11 +706,7 @@ impl Global { let cmd_buf = hub.command_buffers.get(encoder_id.into_command_buffer_id()); - let error = match cmd_buf - .try_get() - .map_err(|e| e.into()) - .and_then(|mut cmd_buf_data| cmd_buf_data.finish(&cmd_buf.device)) - { + let error = match cmd_buf.data.lock().finish(&cmd_buf.device) { Ok(_) => None, Err(e) => Some(e), }; @@ -695,8 +725,8 @@ impl Global { let hub = &self.hub; let cmd_buf = hub.command_buffers.get(encoder_id.into_command_buffer_id()); - let mut cmd_buf_data = cmd_buf.try_get()?; - cmd_buf_data.check_recording()?; + let mut cmd_buf_data = cmd_buf.data.lock(); + let cmd_buf_data = cmd_buf_data.record()?; #[cfg(feature = "trace")] if let Some(ref mut list) = cmd_buf_data.commands { @@ -727,8 +757,8 @@ impl Global { let hub = &self.hub; let cmd_buf = hub.command_buffers.get(encoder_id.into_command_buffer_id()); - let mut cmd_buf_data = cmd_buf.try_get()?; - cmd_buf_data.check_recording()?; + let mut cmd_buf_data = cmd_buf.data.lock(); + let cmd_buf_data = cmd_buf_data.record()?; #[cfg(feature = "trace")] if let Some(ref mut list) = cmd_buf_data.commands { @@ -758,8 +788,8 @@ impl Global { let hub = &self.hub; let cmd_buf = hub.command_buffers.get(encoder_id.into_command_buffer_id()); - let mut cmd_buf_data = cmd_buf.try_get()?; - cmd_buf_data.check_recording()?; + let mut cmd_buf_data = cmd_buf.data.lock(); + let cmd_buf_data = cmd_buf_data.record()?; #[cfg(feature = "trace")] if let Some(ref mut list) = cmd_buf_data.commands { diff --git a/wgpu-core/src/command/query.rs b/wgpu-core/src/command/query.rs index e99144663b..becf7ce3bb 100644 --- a/wgpu-core/src/command/query.rs +++ b/wgpu-core/src/command/query.rs @@ -321,8 +321,8 @@ impl Global { let cmd_buf = hub .command_buffers .get(command_encoder_id.into_command_buffer_id()); - let mut cmd_buf_data = cmd_buf.try_get()?; - cmd_buf_data.check_recording()?; + let mut cmd_buf_data = cmd_buf.data.lock(); + let cmd_buf_data = cmd_buf_data.record()?; cmd_buf .device @@ -361,8 +361,8 @@ impl Global { let cmd_buf = hub .command_buffers .get(command_encoder_id.into_command_buffer_id()); - let mut cmd_buf_data = cmd_buf.try_get()?; - cmd_buf_data.check_recording()?; + let mut cmd_buf_data = cmd_buf.data.lock(); + let cmd_buf_data = cmd_buf_data.record()?; #[cfg(feature = "trace")] if let Some(ref mut list) = cmd_buf_data.commands { diff --git a/wgpu-core/src/command/ray_tracing.rs b/wgpu-core/src/command/ray_tracing.rs index e527761c4b..421c727ae7 100644 --- a/wgpu-core/src/command/ray_tracing.rs +++ b/wgpu-core/src/command/ray_tracing.rs @@ -126,7 +126,7 @@ impl Global { #[cfg(feature = "trace")] let trace_tlas: Vec = tlas_iter.collect(); #[cfg(feature = "trace")] - if let Some(ref mut list) = cmd_buf.data.lock().as_mut().unwrap().commands { + if let Some(ref mut list) = cmd_buf.data.lock().get_inner()?.commands { list.push( crate::device::trace::Command::BuildAccelerationStructuresUnsafeTlas { blas: trace_blas.clone(), @@ -170,7 +170,7 @@ impl Global { let mut scratch_buffer_blas_size = 0; let mut blas_storage = Vec::new(); let mut cmd_buf_data = cmd_buf.data.lock(); - let cmd_buf_data = cmd_buf_data.as_mut().unwrap(); + let cmd_buf_data = cmd_buf_data.record()?; iter_blas( blas_iter, @@ -435,7 +435,7 @@ impl Global { .collect(); #[cfg(feature = "trace")] - if let Some(ref mut list) = cmd_buf.data.lock().as_mut().unwrap().commands { + if let Some(ref mut list) = cmd_buf.data.lock().get_inner()?.commands { list.push(crate::device::trace::Command::BuildAccelerationStructures { blas: trace_blas.clone(), tlas: trace_tlas.clone(), @@ -486,7 +486,7 @@ impl Global { let mut scratch_buffer_blas_size = 0; let mut blas_storage = Vec::new(); let mut cmd_buf_data = cmd_buf.data.lock(); - let cmd_buf_data = cmd_buf_data.as_mut().unwrap(); + let cmd_buf_data = cmd_buf_data.record()?; iter_blas( blas_iter, diff --git a/wgpu-core/src/command/render.rs b/wgpu-core/src/command/render.rs index 7211edc712..c35998b807 100644 --- a/wgpu-core/src/command/render.rs +++ b/wgpu-core/src/command/render.rs @@ -14,8 +14,8 @@ use crate::{ end_occlusion_query, end_pipeline_statistics_query, memory_init::{fixup_discarded_surfaces, SurfacesInDiscardState}, ArcPassTimestampWrites, BasePass, BindGroupStateChange, CommandBuffer, CommandEncoderError, - CommandEncoderStatus, DrawError, ExecutionError, MapPassErr, PassErrorScope, - PassTimestampWrites, QueryUseError, RenderCommandError, StateChange, + DrawError, ExecutionError, MapPassErr, PassErrorScope, PassTimestampWrites, QueryUseError, + RenderCommandError, StateChange, }, device::{ AttachmentData, Device, DeviceError, MissingDownlevelFlags, MissingFeatures, @@ -45,8 +45,7 @@ use serde::Deserialize; #[cfg(feature = "serde")] use serde::Serialize; -use std::sync::Arc; -use std::{borrow::Cow, fmt, iter, mem::size_of, num::NonZeroU32, ops::Range, str}; +use std::{borrow::Cow, fmt, iter, mem::size_of, num::NonZeroU32, ops::Range, str, sync::Arc}; use super::render_command::ArcRenderCommand; use super::{ @@ -1321,7 +1320,9 @@ impl Global { /// If creation fails, an invalid pass is returned. /// Any operation on an invalid pass will return an error. /// - /// If successful, puts the encoder into the [`CommandEncoderStatus::Locked`] state. + /// If successful, puts the encoder into the [`Locked`] state. + /// + /// [`Locked`]: crate::command::CommandEncoderStatus::Locked pub fn command_encoder_create_render_pass( &self, encoder_id: id::CommandEncoderId, @@ -1422,11 +1423,7 @@ impl Global { let cmd_buf = hub.command_buffers.get(encoder_id.into_command_buffer_id()); - match cmd_buf - .try_get() - .map_err(|e| e.into()) - .and_then(|mut cmd_buf_data| cmd_buf_data.lock_encoder()) - { + match cmd_buf.data.lock().lock_encoder() { Ok(_) => {} Err(e) => return make_err(e, arc_desc), }; @@ -1457,7 +1454,8 @@ impl Global { .hub .command_buffers .get(encoder_id.into_command_buffer_id()); - let mut cmd_buf_data = cmd_buf.try_get().map_pass_err(pass_scope)?; + let mut cmd_buf_data = cmd_buf.data.lock(); + let cmd_buf_data = cmd_buf_data.get_inner().map_pass_err(pass_scope)?; if let Some(ref mut list) = cmd_buf_data.commands { list.push(crate::device::trace::Command::RunRenderPass { @@ -1532,9 +1530,9 @@ impl Global { base.label.as_deref().unwrap_or("") ); - let mut cmd_buf_data = cmd_buf.try_get().map_pass_err(pass_scope)?; - cmd_buf_data.unlock_encoder().map_pass_err(pass_scope)?; - let cmd_buf_data = &mut *cmd_buf_data; + let mut cmd_buf_data = cmd_buf.data.lock(); + let mut cmd_buf_data_guard = cmd_buf_data.unlock_encoder().map_pass_err(pass_scope)?; + let cmd_buf_data = &mut *cmd_buf_data_guard; let device = &cmd_buf.device; let snatch_guard = &device.snatchable_lock.read(); @@ -1545,7 +1543,6 @@ impl Global { device.check_is_valid().map_pass_err(pass_scope)?; let encoder = &mut cmd_buf_data.encoder; - let status = &mut cmd_buf_data.status; let tracker = &mut cmd_buf_data.trackers; let buffer_memory_init_actions = &mut cmd_buf_data.buffer_memory_init_actions; let texture_memory_actions = &mut cmd_buf_data.texture_memory_actions; @@ -1555,8 +1552,6 @@ impl Global { // 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(&cmd_buf.device).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(hal_label, &cmd_buf.device) .map_pass_err(pass_scope)?; @@ -1867,7 +1862,6 @@ impl Global { }; let encoder = &mut cmd_buf_data.encoder; - let status = &mut cmd_buf_data.status; let tracker = &mut cmd_buf_data.trackers; { @@ -1886,10 +1880,10 @@ impl Global { CommandBuffer::insert_barriers_from_scope(transit, tracker, &scope, snatch_guard); } - *status = CommandEncoderStatus::Recording; encoder .close_and_swap(&cmd_buf.device) .map_pass_err(pass_scope)?; + cmd_buf_data_guard.mark_successful(); Ok(()) } diff --git a/wgpu-core/src/command/transfer.rs b/wgpu-core/src/command/transfer.rs index acfaa8c52c..e1eeece5d3 100644 --- a/wgpu-core/src/command/transfer.rs +++ b/wgpu-core/src/command/transfer.rs @@ -549,8 +549,8 @@ impl Global { let cmd_buf = hub .command_buffers .get(command_encoder_id.into_command_buffer_id()); - let mut cmd_buf_data = cmd_buf.try_get()?; - cmd_buf_data.check_recording()?; + let mut cmd_buf_data = cmd_buf.data.lock(); + let cmd_buf_data = cmd_buf_data.record()?; let device = &cmd_buf.device; device.check_is_valid()?; @@ -707,8 +707,8 @@ impl Global { let cmd_buf = hub .command_buffers .get(command_encoder_id.into_command_buffer_id()); - let mut cmd_buf_data = cmd_buf.try_get()?; - cmd_buf_data.check_recording()?; + let mut cmd_buf_data = cmd_buf.data.lock(); + let cmd_buf_data = cmd_buf_data.record()?; let device = &cmd_buf.device; device.check_is_valid()?; @@ -746,7 +746,7 @@ impl Global { // have an easier time inserting "immediate-inits" that may be required // by prior discards in rare cases. handle_dst_texture_init( - &mut cmd_buf_data, + cmd_buf_data, device, destination, copy_size, @@ -860,14 +860,14 @@ impl Global { let cmd_buf = hub .command_buffers .get(command_encoder_id.into_command_buffer_id()); - let mut cmd_buf_data = cmd_buf.try_get()?; - cmd_buf_data.check_recording()?; + let mut cmd_buf_data = cmd_buf.data.lock(); + let cmd_buf_data = cmd_buf_data.record()?; let device = &cmd_buf.device; device.check_is_valid()?; #[cfg(feature = "trace")] - if let Some(ref mut list) = cmd_buf_data.commands { + if let Some(list) = cmd_buf_data.commands.as_mut() { list.push(TraceCommand::CopyTextureToBuffer { src: *source, dst: *destination, @@ -895,7 +895,7 @@ impl Global { // have an easier time inserting "immediate-inits" that may be required // by prior discards in rare cases. handle_src_texture_init( - &mut cmd_buf_data, + cmd_buf_data, device, source, copy_size, @@ -1027,8 +1027,8 @@ impl Global { let cmd_buf = hub .command_buffers .get(command_encoder_id.into_command_buffer_id()); - let mut cmd_buf_data = cmd_buf.try_get()?; - cmd_buf_data.check_recording()?; + let mut cmd_buf_data = cmd_buf.data.lock(); + let cmd_buf_data = cmd_buf_data.record()?; let device = &cmd_buf.device; device.check_is_valid()?; @@ -1092,7 +1092,7 @@ impl Global { // have an easier time inserting "immediate-inits" that may be required // by prior discards in rare cases. handle_src_texture_init( - &mut cmd_buf_data, + cmd_buf_data, device, source, copy_size, @@ -1100,7 +1100,7 @@ impl Global { &snatch_guard, )?; handle_dst_texture_init( - &mut cmd_buf_data, + cmd_buf_data, device, destination, copy_size, diff --git a/wgpu-core/src/device/life.rs b/wgpu-core/src/device/life.rs index f1c6d5bfbd..83fe377d81 100644 --- a/wgpu-core/src/device/life.rs +++ b/wgpu-core/src/device/life.rs @@ -320,7 +320,6 @@ impl LifetimeTracker { pub fn triage_submissions( &mut self, last_done: SubmissionIndex, - command_allocator: &crate::command::CommandAllocator, ) -> SmallVec<[SubmittedWorkDoneClosure; 1]> { profiling::scope!("triage_submissions"); @@ -336,8 +335,10 @@ impl LifetimeTracker { for a in self.active.drain(..done_count) { self.ready_to_map.extend(a.mapped); for encoder in a.encoders { - let raw = unsafe { encoder.land() }; - command_allocator.release_encoder(raw); + // This involves actually decrementing the ref count of all command buffer + // resources, so can be _very_ expensive. + profiling::scope!("drop command buffer trackers"); + drop(encoder); } drop(a.temp_resources); work_done_closures.extend(a.work_done_closures); diff --git a/wgpu-core/src/device/queue.rs b/wgpu-core/src/device/queue.rs index f0b18649df..3fbfeeb38c 100644 --- a/wgpu-core/src/device/queue.rs +++ b/wgpu-core/src/device/queue.rs @@ -31,7 +31,8 @@ use smallvec::SmallVec; use crate::resource::{Blas, DestroyedAccelerationStructure, Tlas}; use crate::scratch::ScratchBuffer; use std::{ - iter, mem, + iter, + mem::{self, ManuallyDrop}, ptr::NonNull, sync::{atomic::Ordering, Arc}, }; @@ -113,8 +114,7 @@ impl Queue { bool, ) { let mut life_tracker = self.lock_life(); - let submission_closures = - life_tracker.triage_submissions(submission_index, &self.device.command_allocator); + let submission_closures = life_tracker.triage_submissions(submission_index); let mapping_closures = life_tracker.handle_mapping(snatch_guard); @@ -274,27 +274,6 @@ pub(crate) struct EncoderInFlight { pub(crate) pending_tlas_s: FastHashMap>, } -impl EncoderInFlight { - /// Free all of our command buffers. - /// - /// Return the command encoder, fully reset and ready to be - /// reused. - pub(crate) unsafe fn land(mut self) -> Box { - unsafe { self.inner.raw.reset_all(self.inner.list) }; - { - // This involves actually decrementing the ref count of all command buffer - // resources, so can be _very_ expensive. - profiling::scope!("drop command buffer trackers"); - drop(self.trackers); - drop(self.pending_buffers); - drop(self.pending_textures); - drop(self.pending_blas_s); - drop(self.pending_tlas_s); - } - self.inner.raw - } -} - /// A private command encoder for writes made directly on the device /// or queue. /// @@ -393,7 +372,7 @@ impl PendingWrites { fn pre_submit( &mut self, command_allocator: &CommandAllocator, - device: &Device, + device: &Arc, queue: &Queue, ) -> Result, DeviceError> { if self.is_recording { @@ -412,8 +391,9 @@ impl PendingWrites { let encoder = EncoderInFlight { inner: crate::command::CommandEncoder { - raw: mem::replace(&mut self.command_encoder, new_encoder), + raw: ManuallyDrop::new(mem::replace(&mut self.command_encoder, new_encoder)), list: vec![cmd_buf], + device: device.clone(), is_open: false, hal_label: None, }, @@ -1134,7 +1114,7 @@ impl Queue { // Note that we are required to invalidate all command buffers in both the success and failure paths. // This is why we `continue` and don't early return via `?`. #[allow(unused_mut)] - let mut cmd_buf_data = command_buffer.try_take(); + let mut cmd_buf_data = command_buffer.take_finished(); #[cfg(feature = "trace")] if let Some(ref mut trace) = *self.device.trace.lock() { @@ -1147,9 +1127,6 @@ impl Queue { } if first_error.is_some() { - if let Ok(cmd_buf_data) = cmd_buf_data { - cmd_buf_data.destroy(); - } continue; } @@ -1165,7 +1142,6 @@ impl Queue { ); if let Err(err) = res { first_error.get_or_insert(err); - cmd_buf_data.destroy(); continue; } cmd_buf_data.into_baked_commands() @@ -1558,7 +1534,6 @@ fn validate_command_buffer( used_surface_textures: &mut track::TextureUsageScope, ) -> Result<(), QueueSubmitError> { command_buffer.same_device_as(queue)?; - cmd_buf_data.check_finished()?; { profiling::scope!("check resource state"); diff --git a/wgpu-core/src/device/resource.rs b/wgpu-core/src/device/resource.rs index f5a34b84bf..45dac57920 100644 --- a/wgpu-core/src/device/resource.rs +++ b/wgpu-core/src/device/resource.rs @@ -3535,9 +3535,7 @@ impl Device { .map_err(|e| self.handle_hal_error(e))?; drop(fence); if let Some(queue) = self.get_queue() { - let closures = queue - .lock_life() - .triage_submissions(submission_index, &self.command_allocator); + let closures = queue.lock_life().triage_submissions(submission_index); assert!( closures.is_empty(), "wait_for_submit is not expected to work with closures" diff --git a/wgpu-core/src/lock/observing.rs b/wgpu-core/src/lock/observing.rs index afda1ad574..c530a56539 100644 --- a/wgpu-core/src/lock/observing.rs +++ b/wgpu-core/src/lock/observing.rs @@ -78,15 +78,6 @@ impl Mutex { } } -impl<'a, T> MutexGuard<'a, T> { - pub fn try_map(s: Self, f: F) -> Result, ()> - where - F: FnOnce(&mut T) -> Option<&mut U>, - { - parking_lot::MutexGuard::try_map(s.inner, f).map_err(|_| ()) - } -} - impl<'a, T> std::ops::Deref for MutexGuard<'a, T> { type Target = T; diff --git a/wgpu-core/src/lock/vanilla.rs b/wgpu-core/src/lock/vanilla.rs index 51e472b118..9a35b6d9d8 100644 --- a/wgpu-core/src/lock/vanilla.rs +++ b/wgpu-core/src/lock/vanilla.rs @@ -30,15 +30,6 @@ impl Mutex { } } -impl<'a, T> MutexGuard<'a, T> { - pub fn try_map(s: Self, f: F) -> Result, ()> - where - F: FnOnce(&mut T) -> Option<&mut U>, - { - parking_lot::MutexGuard::try_map(s.0, f).map_err(|_| ()) - } -} - impl<'a, T> std::ops::Deref for MutexGuard<'a, T> { type Target = T; diff --git a/wgpu-core/src/resource.rs b/wgpu-core/src/resource.rs index b72abf4b11..82d82ac871 100644 --- a/wgpu-core/src/resource.rs +++ b/wgpu-core/src/resource.rs @@ -1368,9 +1368,10 @@ impl Global { let hub = &self.hub; let cmd_buf = hub.command_buffers.get(id.into_command_buffer_id()); - let cmd_buf_data = cmd_buf.try_get(); + let mut cmd_buf_data = cmd_buf.data.lock(); + let cmd_buf_data = cmd_buf_data.record(); - if let Ok(mut cmd_buf_data) = cmd_buf_data { + if let Ok(cmd_buf_data) = cmd_buf_data { let cmd_buf_raw = cmd_buf_data .encoder .open(&cmd_buf.device)