From def21843fcb80cd521c995529bc636918952b357 Mon Sep 17 00:00:00 2001 From: Rua Date: Sun, 29 Oct 2023 01:40:28 +0200 Subject: [PATCH] Partial validation of queue commands (#2373) * Move resource tracking/locking back from Queue to futures * Add partially validated `submit` command * Add partially validated `present` command * More safety docs * Small doc change * Add SemaphorePresentInfo * Put safety docs in the semaphore/fence modules instead * More fence and semaphore docs * Re-add missing imports * Remove state tracking from Fence * Remove state tracking from Semaphore --- examples/src/bin/gl-interop.rs | 28 +- vulkano/src/command_buffer/auto/mod.rs | 4 + vulkano/src/command_buffer/mod.rs | 271 +++++- vulkano/src/command_buffer/traits.rs | 67 +- vulkano/src/descriptor_set/mod.rs | 1 + vulkano/src/device/queue.rs | 978 ++++++-------------- vulkano/src/swapchain/acquire_present.rs | 293 ++++-- vulkano/src/sync/fence.rs | 677 +++----------- vulkano/src/sync/future/fence_signal.rs | 100 +- vulkano/src/sync/future/mod.rs | 334 ++++++- vulkano/src/sync/future/semaphore_signal.rs | 117 +-- vulkano/src/sync/semaphore.rs | 562 ++--------- 12 files changed, 1531 insertions(+), 1901 deletions(-) diff --git a/examples/src/bin/gl-interop.rs b/examples/src/bin/gl-interop.rs index 6e7d31e2..e035e34b 100644 --- a/examples/src/bin/gl-interop.rs +++ b/examples/src/bin/gl-interop.rs @@ -189,12 +189,16 @@ mod linux { .unwrap(), ); - let acquire_fd = acquire_sem - .export_fd(ExternalSemaphoreHandleType::OpaqueFd) - .unwrap(); - let release_fd = release_sem - .export_fd(ExternalSemaphoreHandleType::OpaqueFd) - .unwrap(); + let acquire_fd = unsafe { + acquire_sem + .export_fd(ExternalSemaphoreHandleType::OpaqueFd) + .unwrap() + }; + let release_fd = unsafe { + release_sem + .export_fd(ExternalSemaphoreHandleType::OpaqueFd) + .unwrap() + }; let barrier_clone = barrier.clone(); let barrier_2_clone = barrier_2.clone(); @@ -300,9 +304,9 @@ mod linux { Event::RedrawEventsCleared => { queue .with(|mut q| unsafe { - q.submit_unchecked( - [SubmitInfo { - signal_semaphores: vec![SemaphoreSubmitInfo::semaphore( + q.submit( + &[SubmitInfo { + signal_semaphores: vec![SemaphoreSubmitInfo::new( acquire_sem.clone(), )], ..Default::default() @@ -317,9 +321,9 @@ mod linux { queue .with(|mut q| unsafe { - q.submit_unchecked( - [SubmitInfo { - wait_semaphores: vec![SemaphoreSubmitInfo::semaphore( + q.submit( + &[SubmitInfo { + wait_semaphores: vec![SemaphoreSubmitInfo::new( release_sem.clone(), )], ..Default::default() diff --git a/vulkano/src/command_buffer/auto/mod.rs b/vulkano/src/command_buffer/auto/mod.rs index b43723fe..9558869b 100644 --- a/vulkano/src/command_buffer/auto/mod.rs +++ b/vulkano/src/command_buffer/auto/mod.rs @@ -131,6 +131,10 @@ unsafe impl PrimaryCommandBufferAbstract for PrimaryAutoCommandBuffer where A: CommandBufferAllocator, { + fn queue_family_index(&self) -> u32 { + self.inner.queue_family_index() + } + fn usage(&self) -> CommandBufferUsage { self.inner.usage() } diff --git a/vulkano/src/command_buffer/mod.rs b/vulkano/src/command_buffer/mod.rs index 15208744..dfb0239b 100644 --- a/vulkano/src/command_buffer/mod.rs +++ b/vulkano/src/command_buffer/mod.rs @@ -104,7 +104,7 @@ //! [`StandardCommandBufferAllocator`]: self::allocator::StandardCommandBufferAllocator //! [`CommandBufferAllocator`]: self::allocator::CommandBufferAllocator //! [inherit]: CommandBufferInheritanceInfo -//! [`build`]: CommandBufferBuilder::build +//! [`build`]: AutoCommandBufferBuilder::build //! [pipeline barriers]: CommandBufferBuilder::pipeline_barrier //! [`GpuFuture`]: crate::sync::GpuFuture @@ -723,7 +723,7 @@ pub struct SubmitInfo { /// The command buffers to execute. /// /// The default value is empty. - pub command_buffers: Vec>, + pub command_buffers: Vec, /// The semaphores to signal after the execution of this batch of command buffer operations /// has completed. @@ -746,10 +746,95 @@ impl Default for SubmitInfo { } } -/// Parameters for a semaphore signal or wait operation in a command buffer submission. +impl SubmitInfo { + pub(crate) fn validate(&self, device: &Device) -> Result<(), Box> { + let &Self { + ref wait_semaphores, + ref command_buffers, + ref signal_semaphores, + _ne: _, + } = self; + + for (index, semaphore_submit_info) in wait_semaphores.iter().enumerate() { + semaphore_submit_info + .validate(device) + .map_err(|err| err.add_context(format!("wait_semaphores[{}]", index)))?; + } + + for (index, command_buffer_submit_info) in command_buffers.iter().enumerate() { + command_buffer_submit_info + .validate(device) + .map_err(|err| err.add_context(format!("command_buffers[{}]", index)))?; + } + + for (index, semaphore_submit_info) in signal_semaphores.iter().enumerate() { + semaphore_submit_info + .validate(device) + .map_err(|err| err.add_context(format!("signal_semaphores[{}]", index)))?; + + let &SemaphoreSubmitInfo { + semaphore: _, + stages, + _ne: _, + } = semaphore_submit_info; + + if stages != PipelineStages::ALL_COMMANDS && !device.enabled_features().synchronization2 + { + return Err(Box::new(ValidationError { + context: format!("signal_semaphores[{}].stages", index).into(), + problem: "is not `PipelineStages::ALL_COMMANDS`".into(), + requires_one_of: RequiresOneOf(&[RequiresAllOf(&[Requires::Feature( + "synchronization2", + )])]), + vuids: &["VUID-vkQueueSubmit2-synchronization2-03866"], + })); + } + } + + Ok(()) + } +} + +/// Parameters for a command buffer in a queue submit operation. +#[derive(Clone, Debug)] +pub struct CommandBufferSubmitInfo { + /// The command buffer to execute. + /// + /// There is no default value. + pub command_buffer: Arc, + + pub _ne: crate::NonExhaustive, +} + +impl CommandBufferSubmitInfo { + /// Returns a `CommandBufferSubmitInfo` with the specified `command_buffer`. + #[inline] + pub fn new(command_buffer: Arc) -> Self { + Self { + command_buffer, + _ne: crate::NonExhaustive(()), + } + } + + pub(crate) fn validate(&self, device: &Device) -> Result<(), Box> { + let &Self { + ref command_buffer, + _ne: _, + } = self; + + // VUID? + assert_eq!(device, command_buffer.device().as_ref()); + + Ok(()) + } +} + +/// Parameters for a semaphore signal or wait operation in a queue submit operation. #[derive(Clone, Debug)] pub struct SemaphoreSubmitInfo { /// The semaphore to signal or wait for. + /// + /// There is no default value. pub semaphore: Arc, /// For a semaphore wait operation, specifies the pipeline stages in the second synchronization @@ -774,13 +859,191 @@ pub struct SemaphoreSubmitInfo { impl SemaphoreSubmitInfo { /// Returns a `SemaphoreSubmitInfo` with the specified `semaphore`. #[inline] - pub fn semaphore(semaphore: Arc) -> Self { + pub fn new(semaphore: Arc) -> Self { Self { semaphore, stages: PipelineStages::ALL_COMMANDS, _ne: crate::NonExhaustive(()), } } + + pub(crate) fn validate(&self, device: &Device) -> Result<(), Box> { + let &Self { + ref semaphore, + stages, + _ne: _, + } = self; + + // VUID? + assert_eq!(device, semaphore.device().as_ref()); + + stages.validate_device(device).map_err(|err| { + err.add_context("stages") + .set_vuids(&["VUID-VkSemaphoreSubmitInfo-stageMask-parameter"]) + })?; + + if !device.enabled_features().synchronization2 && stages.contains_flags2() { + return Err(Box::new(ValidationError { + context: "stages".into(), + problem: "contains flags from `VkPipelineStageFlagBits2`".into(), + requires_one_of: RequiresOneOf(&[RequiresAllOf(&[Requires::Feature( + "synchronization2", + )])]), + ..Default::default() + })); + } + + if !device.enabled_features().geometry_shader + && stages.intersects(PipelineStages::GEOMETRY_SHADER) + { + return Err(Box::new(ValidationError { + context: "stages".into(), + problem: "contains `PipelineStages::GEOMETRY_SHADER`".into(), + requires_one_of: RequiresOneOf(&[RequiresAllOf(&[Requires::Feature( + "geometry_shader", + )])]), + vuids: &["VUID-VkSemaphoreSubmitInfo-stageMask-03929"], + })); + } + + if !device.enabled_features().tessellation_shader + && stages.intersects( + PipelineStages::TESSELLATION_CONTROL_SHADER + | PipelineStages::TESSELLATION_EVALUATION_SHADER, + ) + { + return Err(Box::new(ValidationError { + context: "stages".into(), + problem: "contains `PipelineStages::TESSELLATION_CONTROL_SHADER` or \ + `PipelineStages::TESSELLATION_EVALUATION_SHADER`" + .into(), + requires_one_of: RequiresOneOf(&[RequiresAllOf(&[Requires::Feature( + "tessellation_shader", + )])]), + vuids: &["VUID-VkSemaphoreSubmitInfo-stageMask-03930"], + })); + } + + if !device.enabled_features().conditional_rendering + && stages.intersects(PipelineStages::CONDITIONAL_RENDERING) + { + return Err(Box::new(ValidationError { + context: "stages".into(), + problem: "contains `PipelineStages::CONDITIONAL_RENDERING`".into(), + requires_one_of: RequiresOneOf(&[RequiresAllOf(&[Requires::Feature( + "conditional_rendering", + )])]), + vuids: &["VUID-VkSemaphoreSubmitInfo-stageMask-03931"], + })); + } + + if !device.enabled_features().fragment_density_map + && stages.intersects(PipelineStages::FRAGMENT_DENSITY_PROCESS) + { + return Err(Box::new(ValidationError { + context: "stages".into(), + problem: "contains `PipelineStages::FRAGMENT_DENSITY_PROCESS`".into(), + requires_one_of: RequiresOneOf(&[RequiresAllOf(&[Requires::Feature( + "fragment_density_map", + )])]), + vuids: &["VUID-VkSemaphoreSubmitInfo-stageMask-03932"], + })); + } + + if !device.enabled_features().transform_feedback + && stages.intersects(PipelineStages::TRANSFORM_FEEDBACK) + { + return Err(Box::new(ValidationError { + context: "stages".into(), + problem: "contains `PipelineStages::TRANSFORM_FEEDBACK`".into(), + requires_one_of: RequiresOneOf(&[RequiresAllOf(&[Requires::Feature( + "transform_feedback", + )])]), + vuids: &["VUID-VkSemaphoreSubmitInfo-stageMask-03933"], + })); + } + + if !device.enabled_features().mesh_shader && stages.intersects(PipelineStages::MESH_SHADER) + { + return Err(Box::new(ValidationError { + context: "stages".into(), + problem: "contains `PipelineStages::MESH_SHADER`".into(), + requires_one_of: RequiresOneOf(&[RequiresAllOf(&[Requires::Feature( + "mesh_shader", + )])]), + vuids: &["VUID-VkSemaphoreSubmitInfo-stageMask-03934"], + })); + } + + if !device.enabled_features().task_shader && stages.intersects(PipelineStages::TASK_SHADER) + { + return Err(Box::new(ValidationError { + context: "stages".into(), + problem: "contains `PipelineStages::TASK_SHADER`".into(), + requires_one_of: RequiresOneOf(&[RequiresAllOf(&[Requires::Feature( + "task_shader", + )])]), + vuids: &["VUID-VkSemaphoreSubmitInfo-stageMask-03935"], + })); + } + + if !(device.enabled_features().attachment_fragment_shading_rate + || device.enabled_features().shading_rate_image) + && stages.intersects(PipelineStages::FRAGMENT_SHADING_RATE_ATTACHMENT) + { + return Err(Box::new(ValidationError { + context: "stages".into(), + problem: "contains `PipelineStages::FRAGMENT_SHADING_RATE_ATTACHMENT`".into(), + requires_one_of: RequiresOneOf(&[ + RequiresAllOf(&[Requires::Feature("attachment_fragment_shading_rate")]), + RequiresAllOf(&[Requires::Feature("shading_rate_image")]), + ]), + vuids: &["VUID-VkMemoryBarrier2-shadingRateImage-07316"], + })); + } + + if !device.enabled_features().subpass_shading + && stages.intersects(PipelineStages::SUBPASS_SHADING) + { + return Err(Box::new(ValidationError { + context: "stages".into(), + problem: "contains `PipelineStages::SUBPASS_SHADING`".into(), + requires_one_of: RequiresOneOf(&[RequiresAllOf(&[Requires::Feature( + "subpass_shading", + )])]), + vuids: &["VUID-VkSemaphoreSubmitInfo-stageMask-04957"], + })); + } + + if !device.enabled_features().invocation_mask + && stages.intersects(PipelineStages::INVOCATION_MASK) + { + return Err(Box::new(ValidationError { + context: "stages".into(), + problem: "contains `PipelineStages::INVOCATION_MASK`".into(), + requires_one_of: RequiresOneOf(&[RequiresAllOf(&[Requires::Feature( + "invocation_mask", + )])]), + vuids: &["VUID-VkSemaphoreSubmitInfo-stageMask-04995"], + })); + } + + if !(device.enabled_extensions().nv_ray_tracing + || device.enabled_features().ray_tracing_pipeline) + && stages.intersects(PipelineStages::RAY_TRACING_SHADER) + { + return Err(Box::new(ValidationError { + context: "stages".into(), + problem: "contains `PipelineStages::RAY_TRACING_SHADER`".into(), + requires_one_of: RequiresOneOf(&[RequiresAllOf(&[Requires::Feature( + "ray_tracing_pipeline", + )])]), + vuids: &["VUID-VkSemaphoreSubmitInfo-stageMask-07946"], + })); + } + + Ok(()) + } } #[derive(Debug, Default)] diff --git a/vulkano/src/command_buffer/traits.rs b/vulkano/src/command_buffer/traits.rs index 50fc98c3..4cb6c92b 100644 --- a/vulkano/src/command_buffer/traits.rs +++ b/vulkano/src/command_buffer/traits.rs @@ -9,7 +9,8 @@ use super::{ CommandBufferInheritanceInfo, CommandBufferResourcesUsage, CommandBufferState, - CommandBufferUsage, SecondaryCommandBufferResourcesUsage, SemaphoreSubmitInfo, SubmitInfo, + CommandBufferSubmitInfo, CommandBufferUsage, SecondaryCommandBufferResourcesUsage, + SemaphoreSubmitInfo, SubmitInfo, }; use crate::{ buffer::Buffer, @@ -17,7 +18,10 @@ use crate::{ image::{Image, ImageLayout}, swapchain::Swapchain, sync::{ - future::{now, AccessCheckError, AccessError, GpuFuture, NowFuture, SubmitAnyBuilder}, + future::{ + now, queue_submit, AccessCheckError, AccessError, GpuFuture, NowFuture, + SubmitAnyBuilder, + }, PipelineStages, }, DeviceSize, SafeDeref, Validated, ValidationError, VulkanError, VulkanObject, @@ -38,6 +42,9 @@ use std::{ pub unsafe trait PrimaryCommandBufferAbstract: VulkanObject + DeviceOwned + Send + Sync { + /// Returns the queue family index of this command buffer. + fn queue_family_index(&self) -> u32; + /// Returns the usage of this command buffer. fn usage(&self) -> CommandBufferUsage; @@ -143,6 +150,10 @@ where T: VulkanObject + SafeDeref + Send + Sync, T::Target: PrimaryCommandBufferAbstract, { + fn queue_family_index(&self) -> u32 { + (**self).queue_family_index() + } + fn usage(&self) -> CommandBufferUsage { (**self).usage() } @@ -237,7 +248,9 @@ where Ok(match self.previous.build_submission()? { SubmitAnyBuilder::Empty => SubmitAnyBuilder::CommandBuffer( SubmitInfo { - command_buffers: vec![self.command_buffer.clone()], + command_buffers: vec![CommandBufferSubmitInfo::new( + self.command_buffer.clone(), + )], ..Default::default() }, None, @@ -251,11 +264,13 @@ where SemaphoreSubmitInfo { // TODO: correct stages ; hard stages: PipelineStages::ALL_COMMANDS, - ..SemaphoreSubmitInfo::semaphore(semaphore) + ..SemaphoreSubmitInfo::new(semaphore) } }) .collect(), - command_buffers: vec![self.command_buffer.clone()], + command_buffers: vec![CommandBufferSubmitInfo::new( + self.command_buffer.clone(), + )], ..Default::default() }, None, @@ -265,7 +280,7 @@ where // FIXME: add pipeline barrier submit_info .command_buffers - .push(self.command_buffer.clone()); + .push(CommandBufferSubmitInfo::new(self.command_buffer.clone())); SubmitAnyBuilder::CommandBuffer(submit_info, fence) } SubmitAnyBuilder::QueuePresent(_) | SubmitAnyBuilder::BindSparse(_, _) => { @@ -305,9 +320,7 @@ where match self.build_submission_impl()? { SubmitAnyBuilder::Empty => {} SubmitAnyBuilder::CommandBuffer(submit_info, fence) => { - self.queue.with(|mut q| { - q.submit_with_future(submit_info, fence, &self.previous, &self.queue) - })?; + queue_submit(&self.queue, submit_info, fence, &self.previous).unwrap(); } _ => unreachable!(), }; @@ -319,7 +332,36 @@ where } unsafe fn signal_finished(&self) { - self.finished.store(true, Ordering::SeqCst); + if !self.finished.swap(true, Ordering::SeqCst) { + let resource_usage = self.command_buffer.resources_usage(); + + for usage in &resource_usage.buffers { + let mut state = usage.buffer.state(); + + for (range, range_usage) in usage.ranges.iter() { + if range_usage.mutable { + state.gpu_write_unlock(range.clone()); + } else { + state.gpu_read_unlock(range.clone()); + } + } + } + + for usage in &resource_usage.images { + let mut state = usage.image.state(); + + for (range, range_usage) in usage.ranges.iter() { + if range_usage.mutable { + state.gpu_write_unlock(range.clone()); + } else { + state.gpu_read_unlock(range.clone()); + } + } + } + + self.command_buffer.state().set_submit_finished(); + } + self.previous.signal_finished(); } @@ -445,7 +487,10 @@ where self.flush().unwrap(); // Block until the queue finished. self.queue.with(|mut q| q.wait_idle()).unwrap(); - unsafe { self.previous.signal_finished() }; + + unsafe { + self.signal_finished(); + } } } } diff --git a/vulkano/src/descriptor_set/mod.rs b/vulkano/src/descriptor_set/mod.rs index 7f2b0578..0077db4a 100644 --- a/vulkano/src/descriptor_set/mod.rs +++ b/vulkano/src/descriptor_set/mod.rs @@ -75,6 +75,7 @@ //! [`DescriptorSet`]. It is what you pass to the draw functions. //! //! [`DescriptorPool`]: pool::DescriptorPool +//! [`UnsafeDescriptorSet`]: sys::UnsafeDescriptorSet //! [`DescriptorSetAllocator`]: allocator::DescriptorSetAllocator //! [`StandardDescriptorSetAllocator`]: allocator::StandardDescriptorSetAllocator diff --git a/vulkano/src/device/queue.rs b/vulkano/src/device/queue.rs index ddb6f7a5..e6659028 100644 --- a/vulkano/src/device/queue.rs +++ b/vulkano/src/device/queue.rs @@ -9,36 +9,25 @@ use super::{Device, DeviceOwned, QueueCreateFlags}; use crate::{ - buffer::BufferState, - command_buffer::{ - CommandBufferResourcesUsage, CommandBufferState, CommandBufferUsage, SemaphoreSubmitInfo, - SubmitInfo, - }, - image::ImageState, + command_buffer::{CommandBufferSubmitInfo, SemaphoreSubmitInfo, SubmitInfo}, instance::{debug::DebugUtilsLabel, InstanceOwnedDebugWrapper}, macros::vulkan_bitflags, memory::{ BindSparseInfo, SparseBufferMemoryBind, SparseImageMemoryBind, SparseImageOpaqueMemoryBind, }, - swapchain::{PresentInfo, SwapchainPresentInfo}, - sync::{ - fence::{Fence, FenceState}, - future::{AccessCheckError, GpuFuture}, - semaphore::SemaphoreState, - }, + swapchain::{PresentInfo, SemaphorePresentInfo, SwapchainPresentInfo}, + sync::{fence::Fence, PipelineStages}, Requires, RequiresAllOf, RequiresOneOf, Validated, ValidationError, Version, VulkanError, VulkanObject, }; -use ahash::HashMap; use parking_lot::{Mutex, MutexGuard}; -use smallvec::{smallvec, SmallVec}; +use smallvec::SmallVec; use std::{ - collections::VecDeque, ffi::CString, hash::{Hash, Hasher}, - mem::{take, MaybeUninit}, + mem::MaybeUninit, ptr, - sync::{atomic::Ordering, Arc}, + sync::Arc, }; /// Represents a queue where commands can be submitted. @@ -126,7 +115,7 @@ impl Queue { pub fn with<'a, R>(self: &'a Arc, func: impl FnOnce(QueueGuard<'a>) -> R) -> R { func(QueueGuard { queue: self, - state: self.state.lock(), + _state: self.state.lock(), }) } } @@ -134,8 +123,10 @@ impl Queue { impl Drop for Queue { #[inline] fn drop(&mut self) { - let state = self.state.get_mut(); - let _ = state.wait_idle(&self.device, self.handle); + unsafe { + let fns = self.device.fns(); + let _ = (fns.v1_0.queue_wait_idle)(self.handle); + } } } @@ -176,14 +167,10 @@ impl Hash for Queue { pub struct QueueGuard<'a> { queue: &'a Arc, - state: MutexGuard<'a, QueueState>, + _state: MutexGuard<'a, QueueState>, } impl<'a> QueueGuard<'a> { - pub(crate) unsafe fn fence_signaled(&mut self, fence: &Fence) { - self.state.fence_signaled(fence) - } - /// Waits until all work on this queue has finished, then releases ownership of all resources /// that were in use by the queue. /// @@ -194,33 +181,19 @@ impl<'a> QueueGuard<'a> { /// program. #[inline] pub fn wait_idle(&mut self) -> Result<(), VulkanError> { - self.state.wait_idle(&self.queue.device, self.queue.handle) + unsafe { + let fns = self.queue.device.fns(); + (fns.v1_0.queue_wait_idle)(self.queue.handle) + .result() + .map_err(VulkanError::from) + } } #[cfg_attr(not(feature = "document_unchecked"), doc(hidden))] - pub(crate) unsafe fn bind_sparse_unchecked( + pub unsafe fn bind_sparse_unchecked( &mut self, - bind_infos: impl IntoIterator, - fence: Option>, - ) -> Result<(), VulkanError> { - let bind_infos: SmallVec<[_; 4]> = bind_infos.into_iter().collect(); - let mut states = States::from_bind_infos(&bind_infos); - - self.bind_sparse_unchecked_locked( - &bind_infos, - fence.as_ref().map(|fence| { - let state = fence.state(); - (fence, state) - }), - &mut states, - ) - } - - unsafe fn bind_sparse_unchecked_locked( - &mut self, - bind_infos: &SmallVec<[BindSparseInfo; 4]>, - fence: Option<(&Arc, MutexGuard<'_, FenceState>)>, - states: &mut States<'_>, + bind_infos: &[BindSparseInfo], + fence: Option<&Arc>, ) -> Result<(), VulkanError> { struct PerBindSparseInfo { wait_semaphores_vk: SmallVec<[ash::vk::Semaphore; 4]>, @@ -475,83 +448,140 @@ impl<'a> QueueGuard<'a> { bind_infos_vk.as_ptr(), fence .as_ref() - .map_or_else(Default::default, |(fence, _)| fence.handle()), + .map_or_else(Default::default, VulkanObject::handle), ) .result() - .map_err(VulkanError::from)?; + .map_err(VulkanError::from) + } - for bind_info in bind_infos { - let BindSparseInfo { - wait_semaphores, - buffer_binds: _, - image_opaque_binds: _, - image_binds: _, - signal_semaphores, + /// Queues swapchain images for presentation to the surface. + /// + /// # Safety + /// + /// For every semaphore in the `wait_semaphores` elements of `present_info`: + /// - The semaphore must be kept alive while the command is being executed. + /// - The semaphore must be already in the signaled state, or there must be a previously + /// submitted operation that will signal it. + /// - When the wait operation is executed, no other queue must be waiting on the same semaphore. + /// + /// For every element of `present_info.swapchain_infos`: + /// - `swapchain` must be kept alive while the command is being executed. + /// - `image_index` must be an index previously acquired from the swapchain, and the present + /// operation must happen-after the acquire operation. + /// - The swapchain image indicated by `swapchain` and `image_index` must be in the + /// [`ImageLayout::PresentSrc`] layout when the presentation operation is executed. + /// - The swapchain image indicated by `swapchain` and `image_index` must not be accessed + /// after this function is called, until it is acquired again. + /// - If `present_id` is `Some`, then it must be greater than any present ID previously used + /// for the same swapchain. + /// + /// [`ImageLayout::PresentSrc`]: crate::image::ImageLayout::PresentSrc + #[inline] + pub unsafe fn present( + &mut self, + present_info: &PresentInfo, + ) -> Result>, Validated> + { + self.validate_present(present_info)?; + + Ok(self.present_unchecked(present_info)?) + } + + fn validate_present(&self, present_info: &PresentInfo) -> Result<(), Box> { + let device = self.queue.device(); + + if !device.enabled_extensions().khr_swapchain { + return Err(Box::new(ValidationError { + requires_one_of: RequiresOneOf(&[RequiresAllOf(&[Requires::DeviceExtension( + "khr_swapchain", + )])]), + ..Default::default() + })); + } + + present_info + .validate(device) + .map_err(|err| err.add_context("present_info"))?; + + let &PresentInfo { + wait_semaphores: _, + ref swapchains, + _ne: _, + } = present_info; + + for (index, swapchain_info) in swapchains.iter().enumerate() { + let &SwapchainPresentInfo { + ref swapchain, + image_index: _, + present_id: _, + present_mode: _, + present_regions: _, _ne: _, - } = bind_info; + } = swapchain_info; - for semaphore in wait_semaphores { - let state = states.semaphores.get_mut(&semaphore.handle()).unwrap(); - state.add_queue_wait(self.queue); - } - - for semaphore in signal_semaphores { - let state = states.semaphores.get_mut(&semaphore.handle()).unwrap(); - state.add_queue_wait(self.queue); + if unsafe { + !device + .physical_device + .surface_support_unchecked(self.queue.queue_family_index, swapchain.surface()) + .unwrap_or_default() + } { + return Err(Box::new(ValidationError { + context: format!( + "present_info.swapchain_infos[{}].swapchain.surface()", + index + ) + .into(), + problem: "the queue family of this queue does not support presenting to \ + the surface" + .into(), + vuids: &["VUID-vkQueuePresentKHR-pSwapchains-01292"], + ..Default::default() + })); } } - let fence = fence.map(|(fence, mut state)| { - state.add_queue_signal(self.queue); - fence.clone() - }); - - self.state - .operations - .push_back((bind_infos.clone().into(), fence)); + // unsafe + // VUID-vkQueuePresentKHR-pWaitSemaphores-01294 + // VUID-vkQueuePresentKHR-pWaitSemaphores-03268 Ok(()) } #[cfg_attr(not(feature = "document_unchecked"), doc(hidden))] - #[inline] pub unsafe fn present_unchecked( - &mut self, - present_info: PresentInfo, - ) -> Result>, VulkanError> { - let mut states = States::from_present_info(&present_info); - self.present_unchecked_locked(&present_info, &mut states) - } - - unsafe fn present_unchecked_locked( &mut self, present_info: &PresentInfo, - states: &mut States<'_>, ) -> Result>, VulkanError> { let PresentInfo { wait_semaphores, - swapchain_infos, + swapchains, _ne: _, } = present_info; let wait_semaphores_vk: SmallVec<[_; 4]> = wait_semaphores .iter() - .map(|semaphore| semaphore.handle()) + .map(|semaphore_present_info| { + let &SemaphorePresentInfo { + ref semaphore, + _ne: _, + } = semaphore_present_info; + + semaphore.handle() + }) .collect(); - let mut swapchains_vk: SmallVec<[_; 4]> = SmallVec::with_capacity(swapchain_infos.len()); - let mut image_indices_vk: SmallVec<[_; 4]> = SmallVec::with_capacity(swapchain_infos.len()); - let mut present_ids_vk: SmallVec<[_; 4]> = SmallVec::with_capacity(swapchain_infos.len()); - let mut present_modes_vk: SmallVec<[_; 4]> = SmallVec::with_capacity(swapchain_infos.len()); - let mut present_regions_vk: SmallVec<[_; 4]> = - SmallVec::with_capacity(swapchain_infos.len()); - let mut rectangles_vk: SmallVec<[_; 4]> = SmallVec::with_capacity(swapchain_infos.len()); + let mut swapchains_vk: SmallVec<[_; 4]> = SmallVec::with_capacity(swapchains.len()); + let mut image_indices_vk: SmallVec<[_; 4]> = SmallVec::with_capacity(swapchains.len()); + let mut present_ids_vk: SmallVec<[_; 4]> = SmallVec::with_capacity(swapchains.len()); + let mut present_modes_vk: SmallVec<[_; 4]> = SmallVec::with_capacity(swapchains.len()); + let mut present_regions_vk: SmallVec<[_; 4]> = SmallVec::with_capacity(swapchains.len()); + let mut rectangles_vk: SmallVec<[_; 4]> = SmallVec::with_capacity(swapchains.len()); let mut has_present_ids = false; let mut has_present_modes = false; let mut has_present_regions = false; - for swapchain_info in swapchain_infos { + for swapchain_info in swapchains { let &SwapchainPresentInfo { ref swapchain, image_index, @@ -586,7 +616,7 @@ impl<'a> QueueGuard<'a> { } } - let mut results = vec![ash::vk::Result::SUCCESS; swapchain_infos.len()]; + let mut results = vec![ash::vk::Result::SUCCESS; swapchains.len()]; let mut info_vk = ash::vk::PresentInfoKHR { wait_semaphore_count: wait_semaphores_vk.len() as u32, p_wait_semaphores: wait_semaphores_vk.as_ptr(), @@ -660,26 +690,6 @@ impl<'a> QueueGuard<'a> { return Err(VulkanError::from(result)); } - for semaphore in wait_semaphores { - let state = states.semaphores.get_mut(&semaphore.handle()).unwrap(); - state.add_queue_wait(self.queue); - } - - self.state - .operations - .push_back((present_info.clone().into(), None)); - - // If a presentation results in a loss of full-screen exclusive mode, - // signal that to the relevant swapchain. - for (&result, swapchain_info) in results.iter().zip(&present_info.swapchain_infos) { - if result == ash::vk::Result::ERROR_FULL_SCREEN_EXCLUSIVE_MODE_LOST_EXT { - swapchain_info - .swapchain - .full_screen_exclusive_held() - .store(false, Ordering::SeqCst); - } - } - Ok(results.into_iter().map(|result| match result { ash::vk::Result::SUCCESS => Ok(false), ash::vk::Result::SUBOPTIMAL_KHR => Ok(true), @@ -687,195 +697,176 @@ impl<'a> QueueGuard<'a> { })) } - // Temporary function to keep futures working. - pub(crate) unsafe fn submit_with_future( + /// Submits command buffers to a queue to be executed. + /// + /// # Safety + /// + /// For every semaphore in the `wait_semaphores` elements of every `submit_infos` element: + /// - The semaphore must be kept alive while the command is being executed. + /// - The safety requirements for semaphores, as detailed in the + /// [`semaphore`](crate::sync::semaphore#Safety) module documentation, must be followed. + /// + /// For every command buffer in the `command_buffers` elements of every `submit_infos` element, + /// as well as any secondary command buffers recorded within it: + /// - The command buffer, and any resources it uses, must be kept alive while the command is + /// being executed. + /// - Any mutable resources used by the command buffer must be in the state (image layout, + /// query reset, event signal etc.) expected by the command buffer at the time it begins + /// executing, and must be synchronized appropriately. + /// - If the command buffer's `usage` is [`CommandBufferUsage::OneTimeSubmit`], then it must + /// not have been previously submitted. + /// - If the command buffer's `usage` is [`CommandBufferUsage::MultipleSubmit`], then it must + /// not be currently submitted and not yet completed. + /// - If a recorded command performs a queue family transfer acquire operation, then a + /// corresponding queue family transfer release operation with matching parameters must + /// have been previously submitted, and must happen-before it. + /// - If a recorded command references an [`Event`], then that `Event` must not be referenced + /// by a command that is currently executing on another queue. + /// + /// For every semaphore in the `signal_semaphores` elements of every `submit_infos` element: + /// - The semaphore must be kept alive while the command is being executed. + /// - The safety requirements for semaphores, as detailed in the + /// [`semaphore`](crate::sync::semaphore#Safety) module documentation, must be followed. + /// + /// If `fence` is `Some`: + /// - The fence must be kept alive while the command is being executed. + /// - The safety requirements for fences, as detailed in the + /// [`fence`](crate::sync::fence#Safety) module documentation, must be followed. + /// + /// [`CommandBufferUsage::OneTimeSubmit`]: crate::command_buffer::CommandBufferUsage::OneTimeSubmit + /// [`CommandBufferUsage::MultipleSubmit`]: crate::command_buffer::CommandBufferUsage::MultipleSubmit + /// [`Event`]: crate::sync::event::Event + #[inline] + pub unsafe fn submit( &mut self, - submit_info: SubmitInfo, - fence: Option>, - future: &dyn GpuFuture, - queue: &Queue, + submit_infos: &[SubmitInfo], + fence: Option<&Arc>, ) -> Result<(), Validated> { - let submit_infos: SmallVec<[_; 4]> = smallvec![submit_info]; - let mut states = States::from_submit_infos(&submit_infos); + self.validate_submit(submit_infos, fence)?; - for submit_info in &submit_infos { - for command_buffer in &submit_info.command_buffers { - let state = states - .command_buffers - .get(&command_buffer.handle()) - .unwrap(); + Ok(self.submit_unchecked(submit_infos, fence)?) + } - match command_buffer.usage() { - CommandBufferUsage::OneTimeSubmit => { - if state.has_been_submitted() { - return Err(Box::new(ValidationError { - problem: "a command buffer, or one of the secondary \ - command buffers it executes, was created with the \ - `CommandBufferUsage::OneTimeSubmit` usage, but \ - it has already been submitted in the past" - .into(), - vuids: &["VUID-vkQueueSubmit2-commandBuffer-03874"], - ..Default::default() - }) - .into()); - } - } - CommandBufferUsage::MultipleSubmit => { - if state.is_submit_pending() { - return Err(Box::new(ValidationError { - problem: "a command buffer, or one of the secondary \ - command buffers it executes, was not created with the \ - `CommandBufferUsage::SimultaneousUse` usage, but \ - it is already in use by the device" - .into(), - vuids: &["VUID-vkQueueSubmit2-commandBuffer-03875"], - ..Default::default() - }) - .into()); - } - } - CommandBufferUsage::SimultaneousUse => (), + fn validate_submit( + &self, + submit_infos: &[SubmitInfo], + fence: Option<&Arc>, + ) -> Result<(), Box> { + let device = self.queue.device(); + let queue_family_properties = &device.physical_device().queue_family_properties() + [self.queue.queue_family_index as usize]; + + if let Some(fence) = fence { + // VUID-vkQueueSubmit2-commonparent + assert_eq!(device, fence.device()); + } + + let supported_stages = PipelineStages::from(queue_family_properties.queue_flags); + + for (index, submit_info) in submit_infos.iter().enumerate() { + submit_info + .validate(device) + .map_err(|err| err.add_context(format!("submit_infos[{}]", index)))?; + + let &SubmitInfo { + ref wait_semaphores, + ref command_buffers, + ref signal_semaphores, + _ne: _, + } = submit_info; + + for (semaphore_index, semaphore_submit_info) in wait_semaphores.iter().enumerate() { + let &SemaphoreSubmitInfo { + semaphore: _, + stages, + _ne: _, + } = semaphore_submit_info; + + if !supported_stages.contains(stages) { + return Err(Box::new(ValidationError { + context: format!( + "submit_infos[{}].wait_semaphores[{}].stages", + index, semaphore_index + ) + .into(), + problem: "contains stages that are not supported by the queue family of \ + the queue" + .into(), + vuids: &["VUID-vkQueueSubmit2-stageMask-03870"], + ..Default::default() + })); } + } - let CommandBufferResourcesUsage { - buffers, - images, - buffer_indices: _, - image_indices: _, - } = command_buffer.resources_usage(); + for (command_buffer_index, command_buffer_submit_info) in + command_buffers.iter().enumerate() + { + let &CommandBufferSubmitInfo { + ref command_buffer, + _ne: _, + } = command_buffer_submit_info; - for usage in buffers { - let state = states.buffers.get_mut(&usage.buffer.handle()).unwrap(); - - for (range, range_usage) in usage.ranges.iter() { - match future.check_buffer_access( - &usage.buffer, - range.clone(), - range_usage.mutable, - queue, - ) { - Err(AccessCheckError::Denied(error)) => { - return Err(Box::new(ValidationError { - problem: format!( - "access to a resource has been denied \ - (resource use: {:?}, error: {})", - range_usage.first_use, error - ) - .into(), - ..Default::default() - }) - .into()); - } - Err(AccessCheckError::Unknown) => { - let result = if range_usage.mutable { - state.check_gpu_write(range.clone()) - } else { - state.check_gpu_read(range.clone()) - }; - - if let Err(error) = result { - return Err(Box::new(ValidationError { - problem: format!( - "access to a resource has been denied \ - (resource use: {:?}, error: {})", - range_usage.first_use, error - ) - .into(), - ..Default::default() - }) - .into()); - } - } - _ => (), - } - } + if command_buffer.queue_family_index() != self.queue.queue_family_index { + return Err(Box::new(ValidationError { + context: format!( + "submit_infos[{}].command_buffers[{}]\ + .command_buffer.queue_family_index()", + index, command_buffer_index + ) + .into(), + problem: "does not equal the queue family index of this queue".into(), + vuids: &["VUID-vkQueueSubmit2-commandBuffer-03878"], + ..Default::default() + })); } + } - for usage in images { - let state = states.images.get_mut(&usage.image.handle()).unwrap(); + for (semaphore_index, semaphore_submit_info) in signal_semaphores.iter().enumerate() { + let &SemaphoreSubmitInfo { + semaphore: _, + stages, + _ne: _, + } = semaphore_submit_info; - for (range, range_usage) in usage.ranges.iter() { - match future.check_image_access( - &usage.image, - range.clone(), - range_usage.mutable, - range_usage.expected_layout, - queue, - ) { - Err(AccessCheckError::Denied(error)) => { - return Err(Box::new(ValidationError { - problem: format!( - "access to a resource has been denied \ - (resource use: {:?}, error: {})", - range_usage.first_use, error - ) - .into(), - ..Default::default() - }) - .into()); - } - Err(AccessCheckError::Unknown) => { - let result = if range_usage.mutable { - state - .check_gpu_write(range.clone(), range_usage.expected_layout) - } else { - state.check_gpu_read(range.clone(), range_usage.expected_layout) - }; - - if let Err(error) = result { - return Err(Box::new(ValidationError { - problem: format!( - "access to a resource has been denied \ - (resource use: {:?}, error: {})", - range_usage.first_use, error - ) - .into(), - ..Default::default() - }) - .into()); - } - } - _ => (), - }; - } + if !supported_stages.contains(stages) { + return Err(Box::new(ValidationError { + context: format!( + "submit_infos[{}].signal_semaphores[{}].stages", + index, semaphore_index + ) + .into(), + problem: "contains stages that are not supported by the queue family of \ + the queue" + .into(), + vuids: &["VUID-vkQueueSubmit2-stageMask-03870"], + ..Default::default() + })); } } } - Ok(self.submit_unchecked_locked( - &submit_infos, - fence.as_ref().map(|fence| { - let state = fence.state(); - (fence, state) - }), - &mut states, - )?) + // unsafe + // VUID-vkQueueSubmit2-fence-04894 + // VUID-vkQueueSubmit2-fence-04895 + // VUID-vkQueueSubmit2-commandBuffer-03867 + // VUID-vkQueueSubmit2-semaphore-03868 + // VUID-vkQueueSubmit2-semaphore-03871 + // VUID-vkQueueSubmit2-semaphore-03872 + // VUID-vkQueueSubmit2-semaphore-03873 + // VUID-vkQueueSubmit2-commandBuffer-03874 + // VUID-vkQueueSubmit2-commandBuffer-03875 + // VUID-vkQueueSubmit2-commandBuffer-03876 + // VUID-vkQueueSubmit2-commandBuffer-03877 + // VUID-vkQueueSubmit2-commandBuffer-03879 + + Ok(()) } #[cfg_attr(not(feature = "document_unchecked"), doc(hidden))] pub unsafe fn submit_unchecked( &mut self, - submit_infos: impl IntoIterator, - fence: Option>, - ) -> Result<(), VulkanError> { - let submit_infos: SmallVec<[_; 4]> = submit_infos.into_iter().collect(); - let mut states = States::from_submit_infos(&submit_infos); - - self.submit_unchecked_locked( - &submit_infos, - fence.as_ref().map(|fence| { - let state = fence.state(); - (fence, state) - }), - &mut states, - ) - } - - unsafe fn submit_unchecked_locked( - &mut self, - submit_infos: &SmallVec<[SubmitInfo; 4]>, - fence: Option<(&Arc, MutexGuard<'_, FenceState>)>, - states: &mut States<'_>, + submit_infos: &[SubmitInfo], + fence: Option<&Arc>, ) -> Result<(), VulkanError> { if self.queue.device.enabled_features().synchronization2 { struct PerSubmitInfo { @@ -916,10 +907,17 @@ impl<'a> QueueGuard<'a> { let command_buffer_infos_vk = command_buffers .iter() - .map(|cb| ash::vk::CommandBufferSubmitInfo { - command_buffer: cb.handle(), - device_mask: 0, // TODO: - ..Default::default() + .map(|command_buffer_submit_info| { + let &CommandBufferSubmitInfo { + ref command_buffer, + _ne: _, + } = command_buffer_submit_info; + + ash::vk::CommandBufferSubmitInfo { + command_buffer: command_buffer.handle(), + device_mask: 0, // TODO: + ..Default::default() + } }) .collect(); @@ -991,7 +989,7 @@ impl<'a> QueueGuard<'a> { submit_info_vk.as_ptr(), fence .as_ref() - .map_or_else(Default::default, |(fence, _)| fence.handle()), + .map_or_else(Default::default, VulkanObject::handle), ) } else { debug_assert!(self.queue.device.enabled_extensions().khr_synchronization2); @@ -1001,11 +999,11 @@ impl<'a> QueueGuard<'a> { submit_info_vk.as_ptr(), fence .as_ref() - .map_or_else(Default::default, |(fence, _)| fence.handle()), + .map_or_else(Default::default, VulkanObject::handle), ) } .result() - .map_err(VulkanError::from)?; + .map_err(VulkanError::from) } else { struct PerSubmitInfo { wait_semaphores_vk: SmallVec<[ash::vk::Semaphore; 4]>, @@ -1038,8 +1036,17 @@ impl<'a> QueueGuard<'a> { }) .unzip(); - let command_buffers_vk = - command_buffers.iter().map(|cb| cb.handle()).collect(); + let command_buffers_vk = command_buffers + .iter() + .map(|command_buffer_submit_info| { + let &CommandBufferSubmitInfo { + ref command_buffer, + _ne: _, + } = command_buffer_submit_info; + + command_buffer.handle() + }) + .collect(); let signal_semaphores_vk = signal_semaphores .iter() @@ -1104,86 +1111,11 @@ impl<'a> QueueGuard<'a> { submit_info_vk.as_ptr(), fence .as_ref() - .map_or_else(Default::default, |(fence, _)| fence.handle()), + .map_or_else(Default::default, VulkanObject::handle), ) .result() - .map_err(VulkanError::from)?; + .map_err(VulkanError::from) } - - for submit_info in submit_infos { - let SubmitInfo { - wait_semaphores, - command_buffers, - signal_semaphores, - _ne: _, - } = submit_info; - - for semaphore_submit_info in wait_semaphores { - let state = states - .semaphores - .get_mut(&semaphore_submit_info.semaphore.handle()) - .unwrap(); - state.add_queue_wait(self.queue); - } - - for command_buffer in command_buffers { - let state = states - .command_buffers - .get_mut(&command_buffer.handle()) - .unwrap(); - state.add_queue_submit(); - - let CommandBufferResourcesUsage { - buffers, - images, - buffer_indices: _, - image_indices: _, - } = command_buffer.resources_usage(); - - for usage in buffers { - let state = states.buffers.get_mut(&usage.buffer.handle()).unwrap(); - - for (range, range_usage) in usage.ranges.iter() { - if range_usage.mutable { - state.gpu_write_lock(range.clone()); - } else { - state.gpu_read_lock(range.clone()); - } - } - } - - for usage in images { - let state = states.images.get_mut(&usage.image.handle()).unwrap(); - - for (range, range_usage) in usage.ranges.iter() { - if range_usage.mutable { - state.gpu_write_lock(range.clone(), range_usage.final_layout); - } else { - state.gpu_read_lock(range.clone()); - } - } - } - } - - for semaphore_submit_info in signal_semaphores { - let state = states - .semaphores - .get_mut(&semaphore_submit_info.semaphore.handle()) - .unwrap(); - state.add_queue_signal(self.queue); - } - } - - let fence = fence.map(|(fence, mut state)| { - state.add_queue_signal(self.queue); - fence.clone() - }); - - self.state - .operations - .push_back((submit_infos.clone().into(), fence)); - - Ok(()) } /// Opens a queue debug label region. @@ -1353,305 +1285,7 @@ impl<'a> QueueGuard<'a> { } #[derive(Debug, Default)] -struct QueueState { - operations: VecDeque<(QueueOperation, Option>)>, -} - -impl QueueState { - fn wait_idle(&mut self, device: &Device, handle: ash::vk::Queue) -> Result<(), VulkanError> { - unsafe { - let fns = device.fns(); - (fns.v1_0.queue_wait_idle)(handle) - .result() - .map_err(VulkanError::from)?; - - // Since we now know that the queue is finished with all work, - // we can safely release all resources. - for (operation, _) in take(&mut self.operations) { - operation.set_finished(); - } - - Ok(()) - } - } - - /// Called by `fence` when it finds that it is signaled. - fn fence_signaled(&mut self, fence: &Fence) { - // Find the most recent operation that uses `fence`. - let fence_index = self - .operations - .iter() - .enumerate() - .rev() - .find_map(|(index, (_, f))| { - f.as_ref().map_or(false, |f| **f == *fence).then_some(index) - }); - - if let Some(index) = fence_index { - // Remove all operations up to this index, and perform cleanup if needed. - for (operation, fence) in self.operations.drain(..index + 1) { - unsafe { - operation.set_finished(); - - if let Some(fence) = fence { - fence.state().set_signal_finished(); - } - } - } - } - } -} - -#[derive(Debug)] -enum QueueOperation { - BindSparse(SmallVec<[BindSparseInfo; 4]>), - Present(PresentInfo), - Submit(SmallVec<[SubmitInfo; 4]>), -} - -impl QueueOperation { - unsafe fn set_finished(self) { - match self { - QueueOperation::BindSparse(bind_infos) => { - for bind_info in bind_infos { - for semaphore in bind_info.wait_semaphores { - semaphore.state().set_wait_finished(); - } - - for semaphore in bind_info.signal_semaphores { - semaphore.state().set_signal_finished(); - } - } - - // TODO: Do we need to unlock buffers and images here? - } - QueueOperation::Present(present_info) => { - for semaphore in present_info.wait_semaphores { - semaphore.state().set_wait_finished(); - } - } - QueueOperation::Submit(submit_infos) => { - for submit_info in submit_infos { - for semaphore_submit_info in submit_info.wait_semaphores { - semaphore_submit_info.semaphore.state().set_wait_finished(); - } - - for semaphore_submit_info in submit_info.signal_semaphores { - semaphore_submit_info - .semaphore - .state() - .set_signal_finished(); - } - - for command_buffer in submit_info.command_buffers { - let resource_usage = command_buffer.resources_usage(); - - for usage in &resource_usage.buffers { - let mut state = usage.buffer.state(); - - for (range, range_usage) in usage.ranges.iter() { - if range_usage.mutable { - state.gpu_write_unlock(range.clone()); - } else { - state.gpu_read_unlock(range.clone()); - } - } - } - - for usage in &resource_usage.images { - let mut state = usage.image.state(); - - for (range, range_usage) in usage.ranges.iter() { - if range_usage.mutable { - state.gpu_write_unlock(range.clone()); - } else { - state.gpu_read_unlock(range.clone()); - } - } - } - - command_buffer.state().set_submit_finished(); - } - } - } - } - } -} - -impl From> for QueueOperation { - #[inline] - fn from(val: SmallVec<[BindSparseInfo; 4]>) -> Self { - Self::BindSparse(val) - } -} - -impl From for QueueOperation { - #[inline] - fn from(val: PresentInfo) -> Self { - Self::Present(val) - } -} - -impl From> for QueueOperation { - #[inline] - fn from(val: SmallVec<[SubmitInfo; 4]>) -> Self { - Self::Submit(val) - } -} - -// This struct exists to ensure that every object gets locked exactly once. -// Otherwise we get deadlocks. -#[derive(Debug)] -struct States<'a> { - buffers: HashMap>, - command_buffers: HashMap>, - images: HashMap>, - semaphores: HashMap>, -} - -impl<'a> States<'a> { - fn from_bind_infos(bind_infos: &'a [BindSparseInfo]) -> Self { - let mut buffers = HashMap::default(); - let mut images = HashMap::default(); - let mut semaphores = HashMap::default(); - - for bind_info in bind_infos { - let BindSparseInfo { - wait_semaphores, - buffer_binds, - image_opaque_binds, - image_binds, - signal_semaphores, - _ne: _, - } = bind_info; - - for semaphore in wait_semaphores { - semaphores - .entry(semaphore.handle()) - .or_insert_with(|| semaphore.state()); - } - - for (buffer, _) in buffer_binds { - let buffer = buffer.buffer(); - buffers - .entry(buffer.handle()) - .or_insert_with(|| buffer.state()); - } - - for (image, _) in image_opaque_binds { - images - .entry(image.handle()) - .or_insert_with(|| image.state()); - } - - for (image, _) in image_binds { - images - .entry(image.handle()) - .or_insert_with(|| image.state()); - } - - for semaphore in signal_semaphores { - semaphores - .entry(semaphore.handle()) - .or_insert_with(|| semaphore.state()); - } - } - - Self { - buffers, - command_buffers: HashMap::default(), - images, - semaphores, - } - } - - fn from_present_info(present_info: &'a PresentInfo) -> Self { - let mut semaphores = HashMap::default(); - - let PresentInfo { - wait_semaphores, - swapchain_infos: _, - _ne: _, - } = present_info; - - for semaphore in wait_semaphores { - semaphores - .entry(semaphore.handle()) - .or_insert_with(|| semaphore.state()); - } - - Self { - buffers: HashMap::default(), - command_buffers: HashMap::default(), - images: HashMap::default(), - semaphores, - } - } - - fn from_submit_infos(submit_infos: &'a [SubmitInfo]) -> Self { - let mut buffers = HashMap::default(); - let mut command_buffers = HashMap::default(); - let mut images = HashMap::default(); - let mut semaphores = HashMap::default(); - - for submit_info in submit_infos { - let SubmitInfo { - wait_semaphores, - command_buffers: info_command_buffers, - signal_semaphores, - _ne: _, - } = submit_info; - - for semaphore_submit_info in wait_semaphores { - let semaphore = &semaphore_submit_info.semaphore; - semaphores - .entry(semaphore.handle()) - .or_insert_with(|| semaphore.state()); - } - - for command_buffer in info_command_buffers { - command_buffers - .entry(command_buffer.handle()) - .or_insert_with(|| command_buffer.state()); - - let CommandBufferResourcesUsage { - buffers: buffers_usage, - images: images_usage, - buffer_indices: _, - image_indices: _, - } = command_buffer.resources_usage(); - - for usage in buffers_usage { - let buffer = &usage.buffer; - buffers - .entry(buffer.handle()) - .or_insert_with(|| buffer.state()); - } - - for usage in images_usage { - let image = &usage.image; - images - .entry(image.handle()) - .or_insert_with(|| image.state()); - } - } - - for semaphore_submit_info in signal_semaphores { - let semaphore = &semaphore_submit_info.semaphore; - semaphores - .entry(semaphore.handle()) - .or_insert_with(|| semaphore.state()); - } - } - - Self { - buffers, - command_buffers, - images, - semaphores, - } - } -} +struct QueueState {} /// Properties of a queue family in a physical device. #[derive(Clone, Debug)] @@ -1745,7 +1379,7 @@ mod tests { let (_device, queue) = gfx_dev_and_queue!(); queue - .with(|mut q| unsafe { q.submit_unchecked([Default::default()], None) }) + .with(|mut q| unsafe { q.submit(&[Default::default()], None) }) .unwrap(); } @@ -1758,7 +1392,7 @@ mod tests { assert!(!fence.is_signaled().unwrap()); queue - .with(|mut q| q.submit_unchecked([Default::default()], Some(fence.clone()))) + .with(|mut q| q.submit(&[Default::default()], Some(&fence))) .unwrap(); fence.wait(Some(Duration::from_secs(5))).unwrap(); diff --git a/vulkano/src/swapchain/acquire_present.rs b/vulkano/src/swapchain/acquire_present.rs index 093cdea6..6f605bea 100644 --- a/vulkano/src/swapchain/acquire_present.rs +++ b/vulkano/src/swapchain/acquire_present.rs @@ -14,10 +14,11 @@ use crate::{ image::{Image, ImageLayout}, sync::{ fence::Fence, - future::{AccessCheckError, AccessError, GpuFuture, SubmitAnyBuilder}, + future::{queue_present, AccessCheckError, AccessError, GpuFuture, SubmitAnyBuilder}, semaphore::Semaphore, }, - DeviceSize, Validated, ValidationError, VulkanError, VulkanObject, + DeviceSize, Requires, RequiresAllOf, RequiresOneOf, Validated, ValidationError, VulkanError, + VulkanObject, }; use smallvec::smallvec; use std::{ @@ -145,16 +146,6 @@ pub fn acquire_next_image( })? }; - unsafe { - let mut state = semaphore.state(); - state.swapchain_acquire(); - } - - unsafe { - let mut state = fence.state(); - state.import_swapchain_acquire(); - } - Ok(( image_index, is_suboptimal, @@ -216,16 +207,6 @@ pub unsafe fn acquire_next_image_raw( err => return Err(VulkanError::from(err).into()), }; - if let Some(semaphore) = semaphore { - let mut state = semaphore.state(); - state.swapchain_acquire(); - } - - if let Some(fence) = fence { - let mut state = fence.state(); - state.import_swapchain_acquire(); - } - Ok(AcquiredImage { image_index: out.assume_init(), is_suboptimal, @@ -431,12 +412,12 @@ pub struct PresentInfo { /// The semaphores to wait for before beginning the execution of the present operations. /// /// The default value is empty. - pub wait_semaphores: Vec>, + pub wait_semaphores: Vec, /// The present operations to perform. /// /// The default value is empty. - pub swapchain_infos: Vec, + pub swapchains: Vec, pub _ne: crate::NonExhaustive, } @@ -446,12 +427,79 @@ impl Default for PresentInfo { fn default() -> Self { Self { wait_semaphores: Vec::new(), - swapchain_infos: Vec::new(), + swapchains: Vec::new(), _ne: crate::NonExhaustive(()), } } } +impl PresentInfo { + pub(crate) fn validate(&self, device: &Device) -> Result<(), Box> { + let &Self { + ref wait_semaphores, + swapchains: ref swapchain_infos, + _ne: _, + } = self; + + for (index, semaphore_present_info) in wait_semaphores.iter().enumerate() { + semaphore_present_info + .validate(device) + .map_err(|err| err.add_context(format!("wait_semaphores[{}]", index)))?; + } + + assert!(!swapchain_infos.is_empty()); + + let has_present_mode = swapchain_infos + .first() + .map_or(false, |first| first.present_mode.is_some()); + + for (index, swapchain_info) in swapchain_infos.iter().enumerate() { + swapchain_info + .validate(device) + .map_err(|err| err.add_context(format!("swapchain_infos[{}]", index)))?; + + let &SwapchainPresentInfo { + swapchain: _, + image_index: _, + present_id: _, + present_mode, + present_regions: _, + _ne: _, + } = swapchain_info; + + if has_present_mode { + if present_mode.is_none() { + return Err(Box::new(ValidationError { + problem: format!( + "`swapchain_infos[0].present_mode` is `Some`, but \ + `swapchain_infos[{}].present_mode` is not also `Some`", + index + ) + .into(), + vuids: &["VUID-VkPresentInfoKHR-pSwapchains-09199"], + ..Default::default() + })); + } + } else { + if present_mode.is_some() { + return Err(Box::new(ValidationError { + problem: format!( + "`swapchain_infos[0].present_mode` is `None`, but \ + `swapchain_infos[{}].present_mode` is not also `None`", + index + ) + .into(), + vuids: &["VUID-VkPresentInfoKHR-pSwapchains-09199"], + ..Default::default() + })); + } + } + } + + Ok(()) + } +} + /// Parameters for a single present operation on a swapchain. #[derive(Clone, Debug)] pub struct SwapchainPresentInfo { @@ -530,6 +578,113 @@ impl SwapchainPresentInfo { _ne: crate::NonExhaustive(()), } } + + pub(crate) fn validate(&self, device: &Device) -> Result<(), Box> { + let &Self { + ref swapchain, + image_index, + present_id, + present_mode, + ref present_regions, + _ne: _, + } = self; + + // VUID-VkPresentInfoKHR-commonparent + assert_eq!(device, swapchain.device().as_ref()); + + if image_index >= swapchain.image_count() { + return Err(Box::new(ValidationError { + problem: "`image_index` is not less than `swapchain.image_count()`".into(), + vuids: &["VUID-VkPresentInfoKHR-pImageIndices-01430"], + ..Default::default() + })); + } + + if present_id.is_some() && !device.enabled_features().present_id { + return Err(Box::new(ValidationError { + context: "present_id".into(), + problem: "is `Some`".into(), + requires_one_of: RequiresOneOf(&[RequiresAllOf(&[Requires::Feature( + "present_id", + )])]), + vuids: &["VUID-VkPresentInfoKHR-pNext-06235"], + })); + } + + if let Some(present_mode) = present_mode { + if !swapchain.present_modes().contains(&present_mode) { + return Err(Box::new(ValidationError { + problem: "`swapchain.present_modes()` does not contain `present_mode`".into(), + vuids: &["VUID-VkSwapchainPresentModeInfoEXT-pPresentModes-07761"], + ..Default::default() + })); + } + } + + if !present_regions.is_empty() && !device.enabled_extensions().khr_incremental_present { + return Err(Box::new(ValidationError { + context: "present_regions".into(), + problem: "is not empty".into(), + requires_one_of: RequiresOneOf(&[RequiresAllOf(&[Requires::DeviceExtension( + "khr_incremental_present", + )])]), + ..Default::default() + })); + } + + for (index, rectangle_layer) in present_regions.iter().enumerate() { + let &RectangleLayer { + offset, + extent, + layer, + } = rectangle_layer; + + if offset[0] + extent[0] > swapchain.image_extent()[0] { + return Err(Box::new(ValidationError { + problem: format!( + "`present_regions[{0}].offset[0]` + `present_regions[{0}].extent[0]` is \ + greater than `swapchain.image_extent()[0]`", + index + ) + .into(), + vuids: &["VUID-VkRectLayerKHR-offset-04864"], + ..Default::default() + })); + } + + if offset[1] + extent[1] > swapchain.image_extent()[1] { + return Err(Box::new(ValidationError { + problem: format!( + "`present_regions[{0}].offset[1]` + `present_regions[{0}].extent[1]` is \ + greater than `swapchain.image_extent()[1]`", + index + ) + .into(), + vuids: &["VUID-VkRectLayerKHR-offset-04864"], + ..Default::default() + })); + } + + if layer >= swapchain.image_array_layers() { + return Err(Box::new(ValidationError { + problem: format!( + "`present_regions[{0}].layer` is greater than \ + `swapchain.image_array_layers()`", + index + ) + .into(), + vuids: &["VUID-VkRectLayerKHR-layer-01262"], + ..Default::default() + })); + } + } + + // unsafe + // VUID-VkPresentInfoKHR-pImageIndices-01430 + // VUID-VkPresentIdKHR-presentIds-04999 + + Ok(()) + } } /// Represents a rectangular region on an image layer. @@ -572,6 +727,40 @@ impl From<&RectangleLayer> for ash::vk::RectLayerKHR { } } +/// Parameters for a semaphore wait operation in a queue present operation. +#[derive(Clone, Debug)] +pub struct SemaphorePresentInfo { + /// The semaphore to wait for. + /// + /// There is no default value. + pub semaphore: Arc, + + pub _ne: crate::NonExhaustive, +} + +impl SemaphorePresentInfo { + /// Returns a `SemaphorePresentInfo` with the specified `semaphore`. + #[inline] + pub fn new(semaphore: Arc) -> Self { + Self { + semaphore, + _ne: crate::NonExhaustive(()), + } + } + + pub(crate) fn validate(&self, device: &Device) -> Result<(), Box> { + let &Self { + ref semaphore, + _ne: _, + } = self; + + // VUID-VkPresentInfoKHR-commonparent + assert_eq!(device, semaphore.device().as_ref()); + + Ok(()) + } +} + /// Represents a swapchain image being presented on the screen. #[must_use = "Dropping this object will immediately block the thread until the GPU has finished processing the submission"] pub struct PresentFuture

@@ -640,13 +829,16 @@ where Ok(match self.previous.build_submission()? { SubmitAnyBuilder::Empty => SubmitAnyBuilder::QueuePresent(PresentInfo { - swapchain_infos: vec![self.swapchain_info.clone()], + swapchains: vec![self.swapchain_info.clone()], ..Default::default() }), SubmitAnyBuilder::SemaphoresWait(semaphores) => { SubmitAnyBuilder::QueuePresent(PresentInfo { - wait_semaphores: semaphores.into_iter().collect(), - swapchain_infos: vec![self.swapchain_info.clone()], + wait_semaphores: semaphores + .into_iter() + .map(SemaphorePresentInfo::new) + .collect(), + swapchains: vec![self.swapchain_info.clone()], ..Default::default() }) } @@ -654,7 +846,7 @@ where self.previous.flush()?; SubmitAnyBuilder::QueuePresent(PresentInfo { - swapchain_infos: vec![self.swapchain_info.clone()], + swapchains: vec![self.swapchain_info.clone()], ..Default::default() }) } @@ -662,26 +854,24 @@ where self.previous.flush()?; SubmitAnyBuilder::QueuePresent(PresentInfo { - swapchain_infos: vec![self.swapchain_info.clone()], + swapchains: vec![self.swapchain_info.clone()], ..Default::default() }) } SubmitAnyBuilder::QueuePresent(mut present_info) => { - if present_info.swapchain_infos.first().map_or(false, |prev| { + if present_info.swapchains.first().map_or(false, |prev| { prev.present_mode.is_some() != self.swapchain_info.present_mode.is_some() }) { // If the present mode Option variants don't match, create a new command. self.previous.flush()?; SubmitAnyBuilder::QueuePresent(PresentInfo { - swapchain_infos: vec![self.swapchain_info.clone()], + swapchains: vec![self.swapchain_info.clone()], ..Default::default() }) } else { // Otherwise, add our swapchain to the previous. - present_info - .swapchain_infos - .push(self.swapchain_info.clone()); + present_info.swapchains.push(self.swapchain_info.clone()); SubmitAnyBuilder::QueuePresent(present_info) } @@ -701,21 +891,17 @@ where SubmitAnyBuilder::QueuePresent(present_info) => { let PresentInfo { wait_semaphores: _, - swapchain_infos, + swapchains, _ne: _, } = &present_info; - let has_present_mode = swapchain_infos - .first() - .map_or(false, |first| first.present_mode.is_some()); - - for swapchain_info in swapchain_infos { + for swapchain_info in swapchains { let &SwapchainPresentInfo { ref swapchain, image_index: _, present_id, present_regions: _, - present_mode, + present_mode: _, _ne: _, } = swapchain_info; @@ -731,25 +917,6 @@ where }) .into()); } - - if let Some(present_mode) = present_mode { - assert!(has_present_mode); - - if !swapchain.present_modes().contains(&present_mode) { - return Err(Box::new(ValidationError { - problem: "the requested present mode is not one of the modes \ - in `swapchain.present_modes()`" - .into(), - vuids: &[ - "VUID-VkSwapchainPresentModeInfoEXT-pPresentModes-07761", - ], - ..Default::default() - }) - .into()); - } - } else { - assert!(!has_present_mode); - } } match self.previous.check_swapchain_image_acquired( @@ -769,9 +936,7 @@ where } } - Ok(self - .queue - .with(|mut q| q.present_unchecked(present_info))? + Ok(queue_present(&self.queue, present_info)? .map(|r| r.map(|_| ())) .fold(Ok(()), Result::and)?) } diff --git a/vulkano/src/sync/fence.rs b/vulkano/src/sync/fence.rs index b8c629b1..f69f74dd 100644 --- a/vulkano/src/sync/fence.rs +++ b/vulkano/src/sync/fence.rs @@ -9,15 +9,38 @@ //! A fence provides synchronization between the device and the host, or between an external source //! and the host. +//! +//! A fence has two states: **signaled** and **unsignaled**. +//! +//! The device can only perform one operation on a fence: +//! - A **fence signal operation** will put the fence into the signaled state. +//! +//! The host can poll a fence's status, wait for it to become signaled, or reset the fence back +//! to the unsignaled state. +//! +//! # Queue-to-host synchronization +//! +//! The primary use of a fence is to know when a queue operation has completed executing. +//! When adding a command to a queue, a fence can be provided with the command, to be signaled +//! when the operation finishes. You can check for a fence's current status by calling +//! `is_signaled`, `wait` or `await` on it. If the fence is found to be signaled, that means that +//! the queue has completed the operation that is associated with the fence, and all operations +//! that happened-before it have been completed as well. +//! +//! # Safety +//! +//! - There must never be more than one fence signal operation queued at any given time. +//! - The fence must be unsignaled at the time the function (for example [`submit`]) is called. +//! +//! [`submit`]: crate::device::QueueGuard::submit use crate::{ - device::{physical::PhysicalDevice, Device, DeviceOwned, Queue}, + device::{physical::PhysicalDevice, Device, DeviceOwned}, instance::InstanceOwnedDebugWrapper, macros::{impl_id_counter, vulkan_bitflags, vulkan_bitflags_enum}, Requires, RequiresAllOf, RequiresOneOf, Validated, ValidationError, Version, VulkanError, VulkanObject, }; -use parking_lot::{Mutex, MutexGuard}; use smallvec::SmallVec; use std::fs::File; use std::{ @@ -26,32 +49,12 @@ use std::{ num::NonZeroU64, pin::Pin, ptr, - sync::{Arc, Weak}, + sync::Arc, task::{Context, Poll}, time::Duration, }; /// A two-state synchronization primitive that is signalled by the device and waited on by the host. -/// -/// # Queue-to-host synchronization -/// -/// The primary use of a fence is to know when a queue operation has completed executing. -/// When adding a command to a queue, a fence can be provided with the command, to be signaled -/// when the operation finishes. You can check for a fence's current status by calling -/// `is_signaled`, `wait` or `await` on it. If the fence is found to be signaled, that means that -/// the queue has completed the operation that is associated with the fence, and all operations that -/// were submitted before it have been completed as well. -/// -/// When a queue command accesses a resource, it must be kept alive until the queue command has -/// finished executing, and you may not be allowed to perform certain other operations (or even any) -/// while the resource is in use. By calling `is_signaled`, `wait` or `await`, the queue will be -/// notified when the fence is signaled, so that all resources of the associated queue operation and -/// preceding operations can be released. -/// -/// Because of this, it is highly recommended to call `is_signaled`, `wait` or `await` on your fences. -/// Otherwise, the queue will hold onto resources indefinitely (using up memory) -/// and resource locks will not be released, which may cause errors when submitting future -/// queue operations. #[derive(Debug)] pub struct Fence { handle: ash::vk::Fence, @@ -62,7 +65,6 @@ pub struct Fence { export_handle_types: ExternalFenceHandleTypes, must_put_in_pool: bool, - state: Mutex, } impl Fence { @@ -141,10 +143,6 @@ impl Fence { export_handle_types, must_put_in_pool: false, - state: Mutex::new(FenceState { - is_signaled: flags.intersects(FenceCreateFlags::SIGNALED), - ..Default::default() - }), }) } @@ -176,7 +174,6 @@ impl Fence { export_handle_types: ExternalFenceHandleTypes::empty(), must_put_in_pool: true, - state: Mutex::new(Default::default()), } } None => { @@ -218,10 +215,6 @@ impl Fence { export_handle_types, must_put_in_pool: false, - state: Mutex::new(FenceState { - is_signaled: flags.intersects(FenceCreateFlags::SIGNALED), - ..Default::default() - }), } } @@ -240,84 +233,44 @@ impl Fence { /// Returns true if the fence is signaled. #[inline] pub fn is_signaled(&self) -> Result { - let queue_to_signal = { - let mut state = self.state(); - - // If the fence is already signaled, or it's unsignaled but there's no queue that - // could signal it, return the currently known value. - if let Some(is_signaled) = state.is_signaled() { - return Ok(is_signaled); - } - - // We must ask Vulkan for the state. - let result = unsafe { - let fns = self.device.fns(); - (fns.v1_0.get_fence_status)(self.device.handle(), self.handle) - }; - - match result { - ash::vk::Result::SUCCESS => unsafe { state.set_signaled() }, - ash::vk::Result::NOT_READY => return Ok(false), - err => return Err(VulkanError::from(err)), - } + let result = unsafe { + let fns = self.device.fns(); + (fns.v1_0.get_fence_status)(self.device.handle(), self.handle) }; - // If we have a queue that we need to signal our status to, - // do so now after the state lock is dropped, to avoid deadlocks. - if let Some(queue) = queue_to_signal { - unsafe { - queue.with(|mut q| q.fence_signaled(self)); - } + match result { + ash::vk::Result::SUCCESS => Ok(true), + ash::vk::Result::NOT_READY => Ok(false), + err => Err(VulkanError::from(err)), } - - Ok(true) } /// Waits until the fence is signaled, or at least until the timeout duration has elapsed. /// /// If you pass a duration of 0, then the function will return without blocking. pub fn wait(&self, timeout: Option) -> Result<(), VulkanError> { - let queue_to_signal = { - let mut state = self.state.lock(); + let timeout_ns = timeout.map_or(u64::MAX, |timeout| { + timeout + .as_secs() + .saturating_mul(1_000_000_000) + .saturating_add(timeout.subsec_nanos() as u64) + }); - // If the fence is already signaled, we don't need to wait. - if state.is_signaled().unwrap_or(false) { - return Ok(()); - } - - let timeout_ns = timeout.map_or(u64::MAX, |timeout| { - timeout - .as_secs() - .saturating_mul(1_000_000_000) - .saturating_add(timeout.subsec_nanos() as u64) - }); - - let result = unsafe { - let fns = self.device.fns(); - (fns.v1_0.wait_for_fences)( - self.device.handle(), - 1, - &self.handle, - ash::vk::TRUE, - timeout_ns, - ) - }; - - match result { - ash::vk::Result::SUCCESS => unsafe { state.set_signaled() }, - err => return Err(VulkanError::from(err)), - } + let result = unsafe { + let fns = self.device.fns(); + (fns.v1_0.wait_for_fences)( + self.device.handle(), + 1, + &self.handle, + ash::vk::TRUE, + timeout_ns, + ) }; - // If we have a queue that we need to signal our status to, - // do so now after the state lock is dropped, to avoid deadlocks. - if let Some(queue) = queue_to_signal { - unsafe { - queue.with(|mut q| q.fence_signaled(self)); - } + match result { + ash::vk::Result::SUCCESS => Ok(()), + err => Err(VulkanError::from(err)), } - - Ok(()) } /// Waits for multiple fences at once. @@ -358,153 +311,96 @@ impl Fence { fences: impl IntoIterator, timeout: Option, ) -> Result<(), VulkanError> { - let queues_to_signal: SmallVec<[_; 8]> = { - let iter = fences.into_iter(); - let mut fences_vk: SmallVec<[_; 8]> = SmallVec::new(); - let mut fences: SmallVec<[_; 8]> = SmallVec::new(); - let mut states: SmallVec<[_; 8]> = SmallVec::new(); + let iter = fences.into_iter(); + let mut fences_vk: SmallVec<[_; 8]> = SmallVec::new(); + let mut fences: SmallVec<[_; 8]> = SmallVec::new(); - for fence in iter { - let state = fence.state.lock(); - - // Skip the fences that are already signaled. - if !state.is_signaled().unwrap_or(false) { - fences_vk.push(fence.handle); - fences.push(fence); - states.push(state); - } - } - - // VUID-vkWaitForFences-fenceCount-arraylength - // If there are no fences, or all the fences are signaled, we don't need to wait. - if fences_vk.is_empty() { - return Ok(()); - } - - let device = &fences[0].device; - let timeout_ns = timeout.map_or(u64::MAX, |timeout| { - timeout - .as_secs() - .saturating_mul(1_000_000_000) - .saturating_add(timeout.subsec_nanos() as u64) - }); - - let result = { - let fns = device.fns(); - (fns.v1_0.wait_for_fences)( - device.handle(), - fences_vk.len() as u32, - fences_vk.as_ptr(), - ash::vk::TRUE, // TODO: let the user choose false here? - timeout_ns, - ) - }; - - match result { - ash::vk::Result::SUCCESS => fences - .into_iter() - .zip(&mut states) - .filter_map(|(fence, state)| state.set_signaled().map(|state| (state, fence))) - .collect(), - err => return Err(VulkanError::from(err)), - } - }; - - // If we have queues that we need to signal our status to, - // do so now after the state locks are dropped, to avoid deadlocks. - for (queue, fence) in queues_to_signal { - queue.with(|mut q| q.fence_signaled(fence)); + for fence in iter { + fences_vk.push(fence.handle); + fences.push(fence); } - Ok(()) + // VUID-vkWaitForFences-fenceCount-arraylength + // If there are no fences, we don't need to wait. + if fences_vk.is_empty() { + return Ok(()); + } + + let device = &fences[0].device; + let timeout_ns = timeout.map_or(u64::MAX, |timeout| { + timeout + .as_secs() + .saturating_mul(1_000_000_000) + .saturating_add(timeout.subsec_nanos() as u64) + }); + + let result = { + let fns = device.fns(); + (fns.v1_0.wait_for_fences)( + device.handle(), + fences_vk.len() as u32, + fences_vk.as_ptr(), + ash::vk::TRUE, // TODO: let the user choose false here? + timeout_ns, + ) + }; + + match result { + ash::vk::Result::SUCCESS => Ok(()), + err => Err(VulkanError::from(err)), + } } /// Resets the fence. /// - /// The fence must not be in use by a queue operation. + /// # Safety + /// + /// - The fence must not be in use by the device. #[inline] - pub fn reset(&self) -> Result<(), Validated> { - let mut state = self.state.lock(); - self.validate_reset(&state)?; + pub unsafe fn reset(&self) -> Result<(), Validated> { + self.validate_reset()?; - unsafe { Ok(self.reset_unchecked_locked(&mut state)?) } + unsafe { Ok(self.reset_unchecked()?) } } - fn validate_reset(&self, state: &FenceState) -> Result<(), Box> { - if state.is_in_queue() { - return Err(Box::new(ValidationError { - problem: "the fence is in use".into(), - vuids: &["VUID-vkResetFences-pFences-01123"], - ..Default::default() - })); - } - + fn validate_reset(&self) -> Result<(), Box> { Ok(()) } #[cfg_attr(not(feature = "document_unchecked"), doc(hidden))] - #[inline] pub unsafe fn reset_unchecked(&self) -> Result<(), VulkanError> { - let mut state = self.state.lock(); - - self.reset_unchecked_locked(&mut state) - } - - unsafe fn reset_unchecked_locked(&self, state: &mut FenceState) -> Result<(), VulkanError> { let fns = self.device.fns(); (fns.v1_0.reset_fences)(self.device.handle(), 1, &self.handle) .result() .map_err(VulkanError::from)?; - state.reset(); - Ok(()) } /// Resets multiple fences at once. /// - /// The fences must not be in use by a queue operation. + /// # Safety /// - /// # Panics - /// - /// - Panics if not all fences belong to the same device. - pub fn multi_reset<'a>( + /// - The elements of `fences` must not be in use by the device. + pub unsafe fn multi_reset<'a>( fences: impl IntoIterator, ) -> Result<(), Validated> { - let (fences, mut states): (SmallVec<[_; 8]>, SmallVec<[_; 8]>) = fences - .into_iter() - .map(|fence| { - let state = fence.state.lock(); - (fence, state) - }) - .unzip(); - Self::validate_multi_reset(&fences, &states)?; + let fences: SmallVec<[_; 8]> = fences.into_iter().collect(); + Self::validate_multi_reset(&fences)?; - unsafe { Ok(Self::multi_reset_unchecked_locked(&fences, &mut states)?) } + unsafe { Ok(Self::multi_reset_unchecked(fences)?) } } - fn validate_multi_reset( - fences: &[&Fence], - states: &[MutexGuard<'_, FenceState>], - ) -> Result<(), Box> { + fn validate_multi_reset(fences: &[&Fence]) -> Result<(), Box> { if fences.is_empty() { return Ok(()); } let device = &fences[0].device; - for (fence_index, (fence, state)) in fences.iter().zip(states).enumerate() { + for fence in fences { // VUID-vkResetFences-pFences-parent assert_eq!(device, &fence.device); - - if state.is_in_queue() { - return Err(Box::new(ValidationError { - context: format!("fences[{}]", fence_index).into(), - problem: "the fence is in use".into(), - vuids: &["VUID-vkResetFences-pFences-01123"], - ..Default::default() - })); - } } Ok(()) @@ -514,21 +410,8 @@ impl Fence { pub unsafe fn multi_reset_unchecked<'a>( fences: impl IntoIterator, ) -> Result<(), VulkanError> { - let (fences, mut states): (SmallVec<[_; 8]>, SmallVec<[_; 8]>) = fences - .into_iter() - .map(|fence| { - let state = fence.state.lock(); - (fence, state) - }) - .unzip(); + let fences: SmallVec<[_; 8]> = fences.into_iter().collect(); - Self::multi_reset_unchecked_locked(&fences, &mut states) - } - - unsafe fn multi_reset_unchecked_locked( - fences: &[&Fence], - states: &mut [MutexGuard<'_, FenceState>], - ) -> Result<(), VulkanError> { if fences.is_empty() { return Ok(()); } @@ -541,10 +424,6 @@ impl Fence { .result() .map_err(VulkanError::from)?; - for state in states { - state.reset(); - } - Ok(()) } @@ -552,21 +431,27 @@ impl Fence { /// /// The [`khr_external_fence_fd`](crate::device::DeviceExtensions::khr_external_fence_fd) /// extension must be enabled on the device. + /// + /// # Safety + /// + /// - If `handle_type` has copy transference, then the fence must be signaled, or a signal + /// operation on the fence must be pending. + /// - The fence must not currently have an imported payload from a swapchain acquire operation. + /// - If the fence has an imported payload, its handle type must allow re-exporting as + /// `handle_type`, as returned by [`PhysicalDevice::external_fence_properties`]. #[inline] - pub fn export_fd( + pub unsafe fn export_fd( &self, handle_type: ExternalFenceHandleType, ) -> Result> { - let mut state = self.state.lock(); - self.validate_export_fd(handle_type, &state)?; + self.validate_export_fd(handle_type)?; - unsafe { Ok(self.export_fd_unchecked_locked(handle_type, &mut state)?) } + unsafe { Ok(self.export_fd_unchecked(handle_type)?) } } fn validate_export_fd( &self, handle_type: ExternalFenceHandleType, - state: &FenceState, ) -> Result<(), Box> { if !self.device.enabled_extensions().khr_external_fence_fd { return Err(Box::new(ValidationError { @@ -604,73 +489,13 @@ impl Fence { })); } - if handle_type.has_copy_transference() - && !(state.is_signaled().unwrap_or(false) || state.is_in_queue()) - { - return Err(Box::new(ValidationError { - problem: "`handle_type` has copy transference, but \ - the fence is not signaled, and \ - a signal operation on the fence is not pending" - .into(), - vuids: &["VUID-VkFenceGetFdInfoKHR-handleType-01454"], - ..Default::default() - })); - } - - if let Some(imported_handle_type) = state.current_import { - match imported_handle_type { - ImportType::SwapchainAcquire => { - return Err(Box::new(ValidationError { - problem: "the fence currently has an imported payload from a \ - swapchain acquire operation" - .into(), - vuids: &["VUID-VkFenceGetFdInfoKHR-fence-01455"], - ..Default::default() - })); - } - ImportType::ExternalFence(imported_handle_type) => { - let external_fence_properties = unsafe { - self.device - .physical_device() - .external_fence_properties_unchecked(ExternalFenceInfo::handle_type( - handle_type, - )) - }; - - if !external_fence_properties - .export_from_imported_handle_types - .intersects(imported_handle_type.into()) - { - return Err(Box::new(ValidationError { - problem: "the fence currently has an imported payload, whose type \ - does not allow re-exporting as `handle_type`, as \ - returned by `PhysicalDevice::external_fence_properties`" - .into(), - vuids: &["VUID-VkFenceGetFdInfoKHR-fence-01455"], - ..Default::default() - })); - } - } - } - } - Ok(()) } #[cfg_attr(not(feature = "document_unchecked"), doc(hidden))] - #[inline] pub unsafe fn export_fd_unchecked( &self, handle_type: ExternalFenceHandleType, - ) -> Result { - let mut state = self.state.lock(); - self.export_fd_unchecked_locked(handle_type, &mut state) - } - - unsafe fn export_fd_unchecked_locked( - &self, - handle_type: ExternalFenceHandleType, - state: &mut FenceState, ) -> Result { let info_vk = ash::vk::FenceGetFdInfoKHR { fence: self.handle, @@ -688,8 +513,6 @@ impl Fence { .result() .map_err(VulkanError::from)?; - state.export(handle_type); - #[cfg(unix)] { use std::os::unix::io::FromRawFd; @@ -707,21 +530,29 @@ impl Fence { /// /// The [`khr_external_fence_win32`](crate::device::DeviceExtensions::khr_external_fence_win32) /// extension must be enabled on the device. + /// + /// # Safety + /// + /// - If `handle_type` has copy transference, then the fence must be signaled, or a signal + /// operation on the fence must be pending. + /// - The fence must not currently have an imported payload from a swapchain acquire operation. + /// - If the fence has an imported payload, its handle type must allow re-exporting as + /// `handle_type`, as returned by [`PhysicalDevice::external_fence_properties`]. + /// - If `handle_type` is `ExternalFenceHandleType::OpaqueWin32`, then a handle of this type + /// must not have been already exported from this fence. #[inline] pub fn export_win32_handle( &self, handle_type: ExternalFenceHandleType, ) -> Result<*mut std::ffi::c_void, Validated> { - let mut state = self.state.lock(); - self.validate_export_win32_handle(handle_type, &state)?; + self.validate_export_win32_handle(handle_type)?; - unsafe { Ok(self.export_win32_handle_unchecked_locked(handle_type, &mut state)?) } + unsafe { Ok(self.export_win32_handle_unchecked(handle_type)?) } } fn validate_export_win32_handle( &self, handle_type: ExternalFenceHandleType, - state: &FenceState, ) -> Result<(), Box> { if !self.device.enabled_extensions().khr_external_fence_win32 { return Err(Box::new(ValidationError { @@ -759,85 +590,13 @@ impl Fence { })); } - if matches!(handle_type, ExternalFenceHandleType::OpaqueWin32) - && state.is_exported(handle_type) - { - return Err(Box::new(ValidationError { - problem: "`handle_type` is `ExternalFenceHandleType::OpaqueWin32`, but \ - a handle of this type has already been exported from this fence" - .into(), - vuids: &["VUID-VkFenceGetWin32HandleInfoKHR-handleType-01449"], - ..Default::default() - })); - } - - if handle_type.has_copy_transference() - && !(state.is_signaled().unwrap_or(false) || state.is_in_queue()) - { - return Err(Box::new(ValidationError { - problem: "`handle_type` has copy transference, but \ - the fence is not signaled, and \ - a signal operation on the fence is not pending" - .into(), - vuids: &["VUID-VkFenceGetWin32HandleInfoKHR-handleType-01451"], - ..Default::default() - })); - } - - if let Some(imported_handle_type) = state.current_import { - match imported_handle_type { - ImportType::SwapchainAcquire => { - return Err(Box::new(ValidationError { - problem: "the fence currently has an imported payload from a \ - swapchain acquire operation" - .into(), - vuids: &["VUID-VkFenceGetWin32HandleInfoKHR-fence-01450"], - ..Default::default() - })); - } - ImportType::ExternalFence(imported_handle_type) => { - let external_fence_properties = unsafe { - self.device - .physical_device() - .external_fence_properties_unchecked(ExternalFenceInfo::handle_type( - handle_type, - )) - }; - - if !external_fence_properties - .export_from_imported_handle_types - .intersects(imported_handle_type.into()) - { - return Err(Box::new(ValidationError { - problem: "the fence currently has an imported payload, whose type \ - does not allow re-exporting as `handle_type`, as \ - returned by `PhysicalDevice::external_fence_properties`" - .into(), - vuids: &["VUID-VkFenceGetWin32HandleInfoKHR-fence-01450"], - ..Default::default() - })); - } - } - } - } - Ok(()) } #[cfg_attr(not(feature = "document_unchecked"), doc(hidden))] - #[inline] pub unsafe fn export_win32_handle_unchecked( &self, handle_type: ExternalFenceHandleType, - ) -> Result<*mut std::ffi::c_void, VulkanError> { - let mut state = self.state.lock(); - self.export_win32_handle_unchecked_locked(handle_type, &mut state) - } - - unsafe fn export_win32_handle_unchecked_locked( - &self, - handle_type: ExternalFenceHandleType, - state: &mut FenceState, ) -> Result<*mut std::ffi::c_void, VulkanError> { let info_vk = ash::vk::FenceGetWin32HandleInfoKHR { fence: self.handle, @@ -855,8 +614,6 @@ impl Fence { .result() .map_err(VulkanError::from)?; - state.export(handle_type); - Ok(output.assume_init()) } @@ -867,6 +624,7 @@ impl Fence { /// /// # Safety /// + /// - The fence must not be in use by the device. /// - If in `import_fence_fd_info`, `handle_type` is `ExternalHandleType::OpaqueFd`, /// then `file` must represent a fence that was exported from Vulkan or a compatible API, /// with a driver and device UUID equal to those of the device that owns `self`. @@ -875,16 +633,14 @@ impl Fence { &self, import_fence_fd_info: ImportFenceFdInfo, ) -> Result<(), Validated> { - let mut state = self.state.lock(); - self.validate_import_fd(&import_fence_fd_info, &state)?; + self.validate_import_fd(&import_fence_fd_info)?; - Ok(self.import_fd_unchecked_locked(import_fence_fd_info, &mut state)?) + Ok(self.import_fd_unchecked(import_fence_fd_info)?) } fn validate_import_fd( &self, import_fence_fd_info: &ImportFenceFdInfo, - state: &FenceState, ) -> Result<(), Box> { if !self.device.enabled_extensions().khr_external_fence_fd { return Err(Box::new(ValidationError { @@ -895,14 +651,6 @@ impl Fence { })); } - if state.is_in_queue() { - return Err(Box::new(ValidationError { - problem: "the fence is in use".into(), - vuids: &["VUID-vkImportFenceFdKHR-fence-01463"], - ..Default::default() - })); - } - import_fence_fd_info .validate(&self.device) .map_err(|err| err.add_context("import_fence_fd_info"))?; @@ -911,19 +659,9 @@ impl Fence { } #[cfg_attr(not(feature = "document_unchecked"), doc(hidden))] - #[inline] pub unsafe fn import_fd_unchecked( &self, import_fence_fd_info: ImportFenceFdInfo, - ) -> Result<(), VulkanError> { - let mut state = self.state.lock(); - self.import_fd_unchecked_locked(import_fence_fd_info, &mut state) - } - - unsafe fn import_fd_unchecked_locked( - &self, - import_fence_fd_info: ImportFenceFdInfo, - state: &mut FenceState, ) -> Result<(), VulkanError> { let ImportFenceFdInfo { flags, @@ -957,8 +695,6 @@ impl Fence { .result() .map_err(VulkanError::from)?; - state.import(handle_type, flags.intersects(FenceImportFlags::TEMPORARY)); - Ok(()) } @@ -969,6 +705,7 @@ impl Fence { /// /// # Safety /// + /// - The fence must not be in use by the device. /// - In `import_fence_win32_handle_info`, `handle` must represent a fence that was exported /// from Vulkan or a compatible API, with a driver and device UUID equal to those of the /// device that owns `self`. @@ -977,16 +714,14 @@ impl Fence { &self, import_fence_win32_handle_info: ImportFenceWin32HandleInfo, ) -> Result<(), Validated> { - let mut state = self.state.lock(); - self.validate_import_win32_handle(&import_fence_win32_handle_info, &state)?; + self.validate_import_win32_handle(&import_fence_win32_handle_info)?; - Ok(self.import_win32_handle_unchecked_locked(import_fence_win32_handle_info, &mut state)?) + Ok(self.import_win32_handle_unchecked(import_fence_win32_handle_info)?) } fn validate_import_win32_handle( &self, import_fence_win32_handle_info: &ImportFenceWin32HandleInfo, - state: &FenceState, ) -> Result<(), Box> { if !self.device.enabled_extensions().khr_external_fence_win32 { return Err(Box::new(ValidationError { @@ -997,14 +732,6 @@ impl Fence { })); } - if state.is_in_queue() { - return Err(Box::new(ValidationError { - problem: "the fence is in use".into(), - vuids: &["VUID-vkImportFenceWin32HandleKHR-fence-04448"], - ..Default::default() - })); - } - import_fence_win32_handle_info .validate(&self.device) .map_err(|err| err.add_context("import_fence_win32_handle_info"))?; @@ -1013,19 +740,9 @@ impl Fence { } #[cfg_attr(not(feature = "document_unchecked"), doc(hidden))] - #[inline] pub unsafe fn import_win32_handle_unchecked( &self, import_fence_win32_handle_info: ImportFenceWin32HandleInfo, - ) -> Result<(), VulkanError> { - let mut state = self.state.lock(); - self.import_win32_handle_unchecked_locked(import_fence_win32_handle_info, &mut state) - } - - unsafe fn import_win32_handle_unchecked_locked( - &self, - import_fence_win32_handle_info: ImportFenceWin32HandleInfo, - state: &mut FenceState, ) -> Result<(), VulkanError> { let ImportFenceWin32HandleInfo { flags, @@ -1051,15 +768,9 @@ impl Fence { .result() .map_err(VulkanError::from)?; - state.import(handle_type, flags.intersects(FenceImportFlags::TEMPORARY)); - Ok(()) } - pub(crate) fn state(&self) -> MutexGuard<'_, FenceState> { - self.state.lock() - } - // Shared by Fence and FenceSignalFuture pub(crate) fn poll_impl(&self, cx: &mut Context<'_>) -> Poll> { // Vulkan only allows polling of the fence status, so we have to use a spin future. @@ -1527,124 +1238,6 @@ pub struct ExternalFenceProperties { pub compatible_handle_types: ExternalFenceHandleTypes, } -#[derive(Debug, Default)] -pub(crate) struct FenceState { - is_signaled: bool, - pending_signal: Option>, - - reference_exported: bool, - exported_handle_types: ExternalFenceHandleTypes, - current_import: Option, - permanent_import: Option, -} - -impl FenceState { - /// If the fence is not in a queue and has no external references, returns the current status. - #[inline] - fn is_signaled(&self) -> Option { - // If either of these is true, we can't be certain of the status. - if self.is_in_queue() || self.has_external_reference() { - None - } else { - Some(self.is_signaled) - } - } - - #[inline] - fn is_in_queue(&self) -> bool { - self.pending_signal.is_some() - } - - /// Returns whether there are any potential external references to the fence payload. - /// That is, the fence has been exported by reference transference, or imported. - #[inline] - fn has_external_reference(&self) -> bool { - self.reference_exported || self.current_import.is_some() - } - - #[allow(dead_code)] - #[inline] - fn is_exported(&self, handle_type: ExternalFenceHandleType) -> bool { - self.exported_handle_types.intersects(handle_type.into()) - } - - #[inline] - pub(crate) unsafe fn add_queue_signal(&mut self, queue: &Arc) { - self.pending_signal = Some(Arc::downgrade(queue)); - } - - /// Called when a fence first discovers that it is signaled. - /// Returns the queue that should be informed about it. - #[inline] - unsafe fn set_signaled(&mut self) -> Option> { - self.is_signaled = true; - - // Fences with external references can't be used to determine queue completion. - if self.has_external_reference() { - self.pending_signal = None; - None - } else { - self.pending_signal.take().and_then(|queue| queue.upgrade()) - } - } - - /// Called when a queue is unlocking resources. - #[inline] - pub(crate) unsafe fn set_signal_finished(&mut self) { - self.is_signaled = true; - self.pending_signal = None; - } - - #[inline] - unsafe fn reset(&mut self) { - debug_assert!(!self.is_in_queue()); - self.current_import = self.permanent_import.map(Into::into); - self.is_signaled = false; - } - - #[allow(dead_code)] - #[inline] - unsafe fn export(&mut self, handle_type: ExternalFenceHandleType) { - self.exported_handle_types |= handle_type.into(); - - if handle_type.has_copy_transference() { - self.reset(); - } else { - self.reference_exported = true; - } - } - - #[allow(dead_code)] - #[inline] - unsafe fn import(&mut self, handle_type: ExternalFenceHandleType, temporary: bool) { - debug_assert!(!self.is_in_queue()); - self.current_import = Some(handle_type.into()); - - if !temporary { - self.permanent_import = Some(handle_type); - } - } - - #[inline] - pub(crate) unsafe fn import_swapchain_acquire(&mut self) { - debug_assert!(!self.is_in_queue()); - self.current_import = Some(ImportType::SwapchainAcquire); - } -} - -#[derive(Clone, Copy, Debug)] -enum ImportType { - SwapchainAcquire, - ExternalFence(ExternalFenceHandleType), -} - -impl From for ImportType { - #[inline] - fn from(handle_type: ExternalFenceHandleType) -> Self { - Self::ExternalFence(handle_type) - } -} - #[cfg(test)] mod tests { use crate::{ @@ -1703,7 +1296,11 @@ mod tests { }, ) .unwrap(); - fence.reset().unwrap(); + + unsafe { + fence.reset().unwrap(); + } + assert!(!fence.is_signaled().unwrap()); } @@ -1760,7 +1357,7 @@ mod tests { ) .unwrap(); - let _ = Fence::multi_reset([&fence1, &fence2]); + let _ = unsafe { Fence::multi_reset([&fence1, &fence2]) }; }); } diff --git a/vulkano/src/sync/future/fence_signal.rs b/vulkano/src/sync/future/fence_signal.rs index 02c5b586..7b5b950b 100644 --- a/vulkano/src/sync/future/fence_signal.rs +++ b/vulkano/src/sync/future/fence_signal.rs @@ -16,7 +16,7 @@ use crate::{ swapchain::Swapchain, sync::{ fence::Fence, - future::{AccessError, SubmitAnyBuilder}, + future::{queue_bind_sparse, queue_present, queue_submit, AccessError, SubmitAnyBuilder}, PipelineStages, }, DeviceSize, Validated, ValidationError, VulkanError, @@ -243,35 +243,36 @@ where SubmitAnyBuilder::Empty => { debug_assert!(!partially_flushed); - queue - .with(|mut q| { - q.submit_unchecked([Default::default()], Some(new_fence.clone())) - }) - .map_err(|err| OutcomeErr::Full(err.into())) + queue_submit( + &queue, + Default::default(), + Some(new_fence.clone()), + &previous, + ) + .map_err(OutcomeErr::Full) } SubmitAnyBuilder::SemaphoresWait(semaphores) => { debug_assert!(!partially_flushed); - queue - .with(|mut q| { - q.submit_unchecked( - [SubmitInfo { - wait_semaphores: semaphores - .into_iter() - .map(|semaphore| { - SemaphoreSubmitInfo { - // TODO: correct stages ; hard - stages: PipelineStages::ALL_COMMANDS, - ..SemaphoreSubmitInfo::semaphore(semaphore) - } - }) - .collect(), - ..Default::default() - }], - None, - ) - }) - .map_err(|err| OutcomeErr::Full(err.into())) + queue_submit( + &queue, + SubmitInfo { + wait_semaphores: semaphores + .into_iter() + .map(|semaphore| { + SemaphoreSubmitInfo { + // TODO: correct stages ; hard + stages: PipelineStages::ALL_COMMANDS, + ..SemaphoreSubmitInfo::new(semaphore) + } + }) + .collect(), + ..Default::default() + }, + None, + &previous, + ) + .map_err(OutcomeErr::Full) } SubmitAnyBuilder::CommandBuffer(submit_info, fence) => { debug_assert!(!partially_flushed); @@ -282,15 +283,7 @@ where // assertion. assert!(fence.is_none()); - queue - .with(|mut q| { - q.submit_with_future( - submit_info, - Some(new_fence.clone()), - &previous, - &queue, - ) - }) + queue_submit(&queue, submit_info, Some(new_fence.clone()), &previous) .map_err(OutcomeErr::Full) } SubmitAnyBuilder::BindSparse(bind_infos, fence) => { @@ -302,19 +295,20 @@ where .queue_flags .intersects(QueueFlags::SPARSE_BINDING)); - queue - .with(|mut q| q.bind_sparse_unchecked(bind_infos, Some(new_fence.clone()))) - .map_err(|err| OutcomeErr::Full(err.into())) + queue_bind_sparse(&queue, bind_infos, Some(new_fence.clone())) + .map_err(OutcomeErr::Full) } SubmitAnyBuilder::QueuePresent(present_info) => { if partially_flushed { - queue - .with(|mut q| { - q.submit_unchecked([Default::default()], Some(new_fence.clone())) - }) - .map_err(|err| OutcomeErr::Partial(err.into())) + queue_submit( + &queue, + Default::default(), + Some(new_fence.clone()), + &previous, + ) + .map_err(OutcomeErr::Partial) } else { - for swapchain_info in &present_info.swapchain_infos { + for swapchain_info in &present_info.swapchains { if swapchain_info.present_id.map_or(false, |present_id| { !swapchain_info.swapchain.try_claim_present_id(present_id) }) { @@ -346,20 +340,18 @@ where } } - let intermediary_result = queue - .with(|mut q| q.present_unchecked(present_info))? + let intermediary_result = queue_present(&queue, present_info)? .map(|r| r.map(|_| ())) .fold(Ok(()), Result::and); match intermediary_result { - Ok(()) => queue - .with(|mut q| { - q.submit_unchecked( - [Default::default()], - Some(new_fence.clone()), - ) - }) - .map_err(|err| OutcomeErr::Partial(err.into())), + Ok(()) => queue_submit( + &queue, + Default::default(), + Some(new_fence.clone()), + &previous, + ) + .map_err(OutcomeErr::Partial), Err(err) => Err(OutcomeErr::Full(err.into())), } } diff --git a/vulkano/src/sync/future/mod.rs b/vulkano/src/sync/future/mod.rs index b186dfe1..2537aef6 100644 --- a/vulkano/src/sync/future/mod.rs +++ b/vulkano/src/sync/future/mod.rs @@ -98,22 +98,26 @@ pub use self::{ }; use super::{fence::Fence, semaphore::Semaphore}; use crate::{ - buffer::Buffer, + buffer::{Buffer, BufferState}, command_buffer::{ - CommandBufferExecError, CommandBufferExecFuture, PrimaryCommandBufferAbstract, SubmitInfo, + CommandBufferExecError, CommandBufferExecFuture, CommandBufferResourcesUsage, + CommandBufferState, CommandBufferSubmitInfo, CommandBufferUsage, + PrimaryCommandBufferAbstract, SubmitInfo, }, device::{DeviceOwned, Queue}, - image::{Image, ImageLayout}, + image::{Image, ImageLayout, ImageState}, memory::BindSparseInfo, swapchain::{self, PresentFuture, PresentInfo, Swapchain, SwapchainPresentInfo}, - DeviceSize, Validated, VulkanError, + DeviceSize, Validated, ValidationError, VulkanError, VulkanObject, }; -use smallvec::SmallVec; +use ahash::HashMap; +use parking_lot::MutexGuard; +use smallvec::{smallvec, SmallVec}; use std::{ error::Error, fmt::{Display, Error as FmtError, Formatter}, ops::Range, - sync::Arc, + sync::{atomic::Ordering, Arc}, }; mod fence_signal; @@ -554,3 +558,321 @@ impl From for AccessCheckError { AccessCheckError::Denied(err) } } + +pub(crate) unsafe fn queue_bind_sparse( + queue: &Arc, + bind_infos: impl IntoIterator, + fence: Option>, +) -> Result<(), Validated> { + let bind_infos: SmallVec<[_; 4]> = bind_infos.into_iter().collect(); + queue.with(|mut queue_guard| queue_guard.bind_sparse_unchecked(&bind_infos, fence.as_ref()))?; + + Ok(()) +} + +pub(crate) unsafe fn queue_present( + queue: &Arc, + present_info: PresentInfo, +) -> Result>, Validated> { + let results: SmallVec<[_; 1]> = queue + .with(|mut queue_guard| queue_guard.present(&present_info))? + .collect(); + + let PresentInfo { + wait_semaphores: _, + swapchains, + _ne: _, + } = &present_info; + + // If a presentation results in a loss of full-screen exclusive mode, + // signal that to the relevant swapchain. + for (&result, swapchain_info) in results.iter().zip(swapchains) { + if result == Err(VulkanError::FullScreenExclusiveModeLost) { + swapchain_info + .swapchain + .full_screen_exclusive_held() + .store(false, Ordering::SeqCst); + } + } + + Ok(results.into_iter()) +} + +pub(crate) unsafe fn queue_submit( + queue: &Arc, + submit_info: SubmitInfo, + fence: Option>, + future: &dyn GpuFuture, +) -> Result<(), Validated> { + let submit_infos: SmallVec<[_; 4]> = smallvec![submit_info]; + let mut states = States::from_submit_infos(&submit_infos); + + for submit_info in &submit_infos { + for command_buffer_submit_info in &submit_info.command_buffers { + let &CommandBufferSubmitInfo { + ref command_buffer, + _ne: _, + } = command_buffer_submit_info; + + let state = states + .command_buffers + .get(&command_buffer.handle()) + .unwrap(); + + match command_buffer.usage() { + CommandBufferUsage::OneTimeSubmit => { + if state.has_been_submitted() { + return Err(Box::new(ValidationError { + problem: "a command buffer, or one of the secondary \ + command buffers it executes, was created with the \ + `CommandBufferUsage::OneTimeSubmit` usage, but \ + it has already been submitted in the past" + .into(), + vuids: &["VUID-vkQueueSubmit2-commandBuffer-03874"], + ..Default::default() + }) + .into()); + } + } + CommandBufferUsage::MultipleSubmit => { + if state.is_submit_pending() { + return Err(Box::new(ValidationError { + problem: "a command buffer, or one of the secondary \ + command buffers it executes, was not created with the \ + `CommandBufferUsage::SimultaneousUse` usage, but \ + it is already in use by the device" + .into(), + vuids: &["VUID-vkQueueSubmit2-commandBuffer-03875"], + ..Default::default() + }) + .into()); + } + } + CommandBufferUsage::SimultaneousUse => (), + } + + let CommandBufferResourcesUsage { + buffers, + images, + buffer_indices: _, + image_indices: _, + } = command_buffer.resources_usage(); + + for usage in buffers { + let state = states.buffers.get_mut(&usage.buffer.handle()).unwrap(); + + for (range, range_usage) in usage.ranges.iter() { + match future.check_buffer_access( + &usage.buffer, + range.clone(), + range_usage.mutable, + queue, + ) { + Err(AccessCheckError::Denied(error)) => { + return Err(Box::new(ValidationError { + problem: format!( + "access to a resource has been denied \ + (resource use: {:?}, error: {})", + range_usage.first_use, error + ) + .into(), + ..Default::default() + }) + .into()); + } + Err(AccessCheckError::Unknown) => { + let result = if range_usage.mutable { + state.check_gpu_write(range.clone()) + } else { + state.check_gpu_read(range.clone()) + }; + + if let Err(error) = result { + return Err(Box::new(ValidationError { + problem: format!( + "access to a resource has been denied \ + (resource use: {:?}, error: {})", + range_usage.first_use, error + ) + .into(), + ..Default::default() + }) + .into()); + } + } + _ => (), + } + } + } + + for usage in images { + let state = states.images.get_mut(&usage.image.handle()).unwrap(); + + for (range, range_usage) in usage.ranges.iter() { + match future.check_image_access( + &usage.image, + range.clone(), + range_usage.mutable, + range_usage.expected_layout, + queue, + ) { + Err(AccessCheckError::Denied(error)) => { + return Err(Box::new(ValidationError { + problem: format!( + "access to a resource has been denied \ + (resource use: {:?}, error: {})", + range_usage.first_use, error + ) + .into(), + ..Default::default() + }) + .into()); + } + Err(AccessCheckError::Unknown) => { + let result = if range_usage.mutable { + state.check_gpu_write(range.clone(), range_usage.expected_layout) + } else { + state.check_gpu_read(range.clone(), range_usage.expected_layout) + }; + + if let Err(error) = result { + return Err(Box::new(ValidationError { + problem: format!( + "access to a resource has been denied \ + (resource use: {:?}, error: {})", + range_usage.first_use, error + ) + .into(), + ..Default::default() + }) + .into()); + } + } + _ => (), + }; + } + } + } + } + + queue.with(|mut queue_guard| queue_guard.submit(&submit_infos, fence.as_ref()))?; + + for submit_info in &submit_infos { + let SubmitInfo { + wait_semaphores: _, + command_buffers, + signal_semaphores: _, + _ne: _, + } = submit_info; + + for command_buffer_submit_info in command_buffers { + let CommandBufferSubmitInfo { + command_buffer, + _ne: _, + } = command_buffer_submit_info; + + let state = states + .command_buffers + .get_mut(&command_buffer.handle()) + .unwrap(); + state.add_queue_submit(); + + let CommandBufferResourcesUsage { + buffers, + images, + buffer_indices: _, + image_indices: _, + } = command_buffer.resources_usage(); + + for usage in buffers { + let state = states.buffers.get_mut(&usage.buffer.handle()).unwrap(); + + for (range, range_usage) in usage.ranges.iter() { + if range_usage.mutable { + state.gpu_write_lock(range.clone()); + } else { + state.gpu_read_lock(range.clone()); + } + } + } + + for usage in images { + let state = states.images.get_mut(&usage.image.handle()).unwrap(); + + for (range, range_usage) in usage.ranges.iter() { + if range_usage.mutable { + state.gpu_write_lock(range.clone(), range_usage.final_layout); + } else { + state.gpu_read_lock(range.clone()); + } + } + } + } + } + + Ok(()) +} + +// This struct exists to ensure that every object gets locked exactly once. +// Otherwise we get deadlocks. +#[derive(Debug)] +struct States<'a> { + buffers: HashMap>, + command_buffers: HashMap>, + images: HashMap>, +} + +impl<'a> States<'a> { + fn from_submit_infos(submit_infos: &'a [SubmitInfo]) -> Self { + let mut buffers = HashMap::default(); + let mut command_buffers = HashMap::default(); + let mut images = HashMap::default(); + + for submit_info in submit_infos { + let SubmitInfo { + wait_semaphores: _, + command_buffers: info_command_buffers, + signal_semaphores: _, + _ne: _, + } = submit_info; + + for command_buffer_submit_info in info_command_buffers { + let &CommandBufferSubmitInfo { + ref command_buffer, + _ne: _, + } = command_buffer_submit_info; + + command_buffers + .entry(command_buffer.handle()) + .or_insert_with(|| command_buffer.state()); + + let CommandBufferResourcesUsage { + buffers: buffers_usage, + images: images_usage, + buffer_indices: _, + image_indices: _, + } = command_buffer.resources_usage(); + + for usage in buffers_usage { + let buffer = &usage.buffer; + buffers + .entry(buffer.handle()) + .or_insert_with(|| buffer.state()); + } + + for usage in images_usage { + let image = &usage.image; + images + .entry(image.handle()) + .or_insert_with(|| image.state()); + } + } + } + + Self { + buffers, + command_buffers, + images, + } + } +} diff --git a/vulkano/src/sync/future/semaphore_signal.rs b/vulkano/src/sync/future/semaphore_signal.rs index 2442edcd..61246ed8 100644 --- a/vulkano/src/sync/future/semaphore_signal.rs +++ b/vulkano/src/sync/future/semaphore_signal.rs @@ -7,14 +7,18 @@ // notice may not be copied, modified, or distributed except // according to those terms. -use super::{AccessCheckError, GpuFuture, SubmitAnyBuilder}; +use super::{queue_present, AccessCheckError, GpuFuture, SubmitAnyBuilder}; use crate::{ buffer::Buffer, command_buffer::{SemaphoreSubmitInfo, SubmitInfo}, device::{Device, DeviceOwned, Queue}, image::{Image, ImageLayout}, swapchain::Swapchain, - sync::{future::AccessError, semaphore::Semaphore, PipelineStages}, + sync::{ + future::{queue_submit, AccessError}, + semaphore::Semaphore, + PipelineStages, + }, DeviceSize, Validated, ValidationError, VulkanError, }; use parking_lot::Mutex; @@ -89,51 +93,49 @@ where match self.previous.build_submission()? { SubmitAnyBuilder::Empty => { - queue.with(|mut q| { - q.submit_unchecked( - [SubmitInfo { - signal_semaphores: vec![SemaphoreSubmitInfo::semaphore( - self.semaphore.clone(), - )], - ..Default::default() - }], - None, - ) - })?; + queue_submit( + &queue, + SubmitInfo { + signal_semaphores: vec![SemaphoreSubmitInfo::new( + self.semaphore.clone(), + )], + ..Default::default() + }, + None, + &self.previous, + )?; } SubmitAnyBuilder::SemaphoresWait(semaphores) => { - queue.with(|mut q| { - q.submit_unchecked( - [SubmitInfo { - wait_semaphores: semaphores - .into_iter() - .map(|semaphore| { - SemaphoreSubmitInfo { - // TODO: correct stages ; hard - stages: PipelineStages::ALL_COMMANDS, - ..SemaphoreSubmitInfo::semaphore(semaphore) - } - }) - .collect(), - signal_semaphores: vec![SemaphoreSubmitInfo::semaphore( - self.semaphore.clone(), - )], - ..Default::default() - }], - None, - ) - })?; + queue_submit( + &queue, + SubmitInfo { + wait_semaphores: semaphores + .into_iter() + .map(|semaphore| { + SemaphoreSubmitInfo { + // TODO: correct stages ; hard + stages: PipelineStages::ALL_COMMANDS, + ..SemaphoreSubmitInfo::new(semaphore) + } + }) + .collect(), + signal_semaphores: vec![SemaphoreSubmitInfo::new( + self.semaphore.clone(), + )], + ..Default::default() + }, + None, + &self.previous, + )?; } SubmitAnyBuilder::CommandBuffer(mut submit_info, fence) => { debug_assert!(submit_info.signal_semaphores.is_empty()); submit_info .signal_semaphores - .push(SemaphoreSubmitInfo::semaphore(self.semaphore.clone())); + .push(SemaphoreSubmitInfo::new(self.semaphore.clone())); - queue.with(|mut q| { - q.submit_with_future(submit_info, fence, &self.previous, &queue) - })?; + queue_submit(&queue, submit_info, fence, &self.previous)?; } SubmitAnyBuilder::BindSparse(_, _) => { unimplemented!() // TODO: how to do that? @@ -142,7 +144,7 @@ where builder.submit(&queue)?;*/ } SubmitAnyBuilder::QueuePresent(present_info) => { - for swapchain_info in &present_info.swapchain_infos { + for swapchain_info in &present_info.swapchains { if swapchain_info.present_id.map_or(false, |present_id| { !swapchain_info.swapchain.try_claim_present_id(present_id) }) { @@ -174,22 +176,22 @@ where } } - queue.with(|mut q| { - q.present_unchecked(present_info)? - .map(|r| r.map(|_| ())) - .fold(Ok(()), Result::and)?; - // FIXME: problematic because if we return an error and flush() is called again, then we'll submit the present twice - q.submit_unchecked( - [SubmitInfo { - signal_semaphores: vec![SemaphoreSubmitInfo::semaphore( - self.semaphore.clone(), - )], - ..Default::default() - }], - None, - )?; - Ok::<_, Validated>(()) - })?; + queue_present(&queue, present_info)? + .map(|r| r.map(|_| ())) + .fold(Ok(()), Result::and)?; + + // FIXME: problematic because if we return an error and flush() is called again, then we'll submit the present twice + queue_submit( + &queue, + SubmitInfo { + signal_semaphores: vec![SemaphoreSubmitInfo::new( + self.semaphore.clone(), + )], + ..Default::default() + }, + None, + &self.previous, + )?; } }; @@ -267,7 +269,10 @@ where self.flush().unwrap(); // Block until the queue finished. self.queue().unwrap().with(|mut q| q.wait_idle()).unwrap(); - unsafe { self.previous.signal_finished() }; + + unsafe { + self.signal_finished(); + } } } } diff --git a/vulkano/src/sync/semaphore.rs b/vulkano/src/sync/semaphore.rs index ab145014..dec0a019 100644 --- a/vulkano/src/sync/semaphore.rs +++ b/vulkano/src/sync/semaphore.rs @@ -9,22 +9,42 @@ //! A semaphore provides synchronization between multiple queues, with non-command buffer //! commands on the same queue, or between the device and an external source. +//! +//! A semaphore has two states: **signaled** and **unsignaled**. +//! Only the device can perform operations on a semaphore, +//! the host cannot perform any operations on it. +//! +//! Two operations can be performed on a semaphore: +//! - A **semaphore signal operation** will put the semaphore into the signaled state. +//! - A **semaphore wait operation** will block execution of the operation is associated with, +//! as long as the semaphore is in the unsignaled state. Once the semaphore is in the signaled +//! state, the semaphore is put back in the unsignaled state and execution continues. +//! +//! Semaphore signals and waits must always occur in pairs: one signal operation is paired with one +//! wait operation. If a semaphore is signaled without waiting for it, it stays in the signaled +//! state until it is waited for, or destroyed. +//! +//! # Safety +//! +//! - When a semaphore signal operation is executed on the device, +//! the semaphore must be in the unsignaled state. +//! In other words, the same semaphore cannot be signalled by multiple commands; +//! there must always be a wait operation in between them. +//! - There must never be more than one semaphore wait operation executing on the same semaphore +//! at the same time. +//! - When a semaphore wait operation is queued as part of a command, +//! the semaphore must already be in the signaled state, or +//! the signal operation that it waits for must have been queued previously +//! (as part of a previous command, or an earlier batch within the same command). use crate::{ - device::{physical::PhysicalDevice, Device, DeviceOwned, Queue}, + device::{physical::PhysicalDevice, Device, DeviceOwned}, instance::InstanceOwnedDebugWrapper, macros::{impl_id_counter, vulkan_bitflags, vulkan_bitflags_enum}, Requires, RequiresAllOf, RequiresOneOf, Validated, ValidationError, Version, VulkanError, VulkanObject, }; -use parking_lot::{Mutex, MutexGuard}; -use std::{ - fs::File, - mem::MaybeUninit, - num::NonZeroU64, - ptr, - sync::{Arc, Weak}, -}; +use std::{fs::File, mem::MaybeUninit, num::NonZeroU64, ptr, sync::Arc}; /// Used to provide synchronization between command buffers during their execution. /// @@ -39,7 +59,6 @@ pub struct Semaphore { export_handle_types: ExternalSemaphoreHandleTypes, must_put_in_pool: bool, - state: Mutex, } impl Semaphore { @@ -128,7 +147,6 @@ impl Semaphore { export_handle_types: ExternalSemaphoreHandleTypes::empty(), must_put_in_pool: true, - state: Mutex::new(Default::default()), }, None => { // Pool is empty, alloc new semaphore @@ -167,7 +185,6 @@ impl Semaphore { export_handle_types, must_put_in_pool: false, - state: Mutex::new(Default::default()), } } @@ -178,21 +195,28 @@ impl Semaphore { } /// Exports the semaphore into a POSIX file descriptor. The caller owns the returned `File`. + /// + /// # Safety + /// + /// - If `handle_type` has copy transference, then the semaphore must be signaled, or a signal + /// operation on the semaphore must be pending, and no wait operations must be pending. + /// - The semaphore must not currently have an imported payload from a swapchain acquire + /// operation. + /// - If the semaphore has an imported payload, its handle type must allow re-exporting as + /// `handle_type`, as returned by [`PhysicalDevice::external_semaphore_properties`]. #[inline] - pub fn export_fd( + pub unsafe fn export_fd( &self, handle_type: ExternalSemaphoreHandleType, ) -> Result> { - let mut state = self.state.lock(); - self.validate_export_fd(handle_type, &state)?; + self.validate_export_fd(handle_type)?; - unsafe { Ok(self.export_fd_unchecked_locked(handle_type, &mut state)?) } + unsafe { Ok(self.export_fd_unchecked(handle_type)?) } } fn validate_export_fd( &self, handle_type: ExternalSemaphoreHandleType, - state: &SemaphoreState, ) -> Result<(), Box> { if !self.device.enabled_extensions().khr_external_semaphore_fd { return Err(Box::new(ValidationError { @@ -230,86 +254,13 @@ impl Semaphore { })); } - if let Some(imported_handle_type) = state.current_import { - match imported_handle_type { - ImportType::SwapchainAcquire => { - return Err(Box::new(ValidationError { - problem: "the semaphore currently has an imported payload from a \ - swapchain acquire operation" - .into(), - vuids: &["VUID-VkSemaphoreGetFdInfoKHR-semaphore-01133"], - ..Default::default() - })); - } - ImportType::ExternalSemaphore(imported_handle_type) => { - let external_semaphore_properties = unsafe { - self.device - .physical_device() - .external_semaphore_properties_unchecked( - ExternalSemaphoreInfo::handle_type(handle_type), - ) - }; - - if !external_semaphore_properties - .export_from_imported_handle_types - .intersects(imported_handle_type.into()) - { - return Err(Box::new(ValidationError { - problem: "the semaphore currently has an imported payload, whose type \ - does not allow re-exporting as `handle_type`, as \ - returned by `PhysicalDevice::external_semaphore_properties`" - .into(), - vuids: &["VUID-VkSemaphoreGetFdInfoKHR-semaphore-01133"], - ..Default::default() - })); - } - } - } - } - - if handle_type.has_copy_transference() { - if state.is_wait_pending() { - return Err(Box::new(ValidationError { - problem: "`handle_type` has copy transference, but \ - a wait operation on the semaphore is pending" - .into(), - vuids: &["VUID-VkSemaphoreGetFdInfoKHR-handleType-01134"], - ..Default::default() - })); - } - - if !(state.is_signaled().unwrap_or(false) || state.is_signal_pending()) { - return Err(Box::new(ValidationError { - problem: "`handle_type` has copy transference, but \ - the semaphore is not signaled, and \ - a signal operation on the semaphore is not pending" - .into(), - vuids: &[ - "VUID-VkSemaphoreGetFdInfoKHR-handleType-01135", - "VUID-VkSemaphoreGetFdInfoKHR-handleType-03254", - ], - ..Default::default() - })); - } - } - Ok(()) } #[cfg_attr(not(feature = "document_unchecked"), doc(hidden))] - #[inline] pub unsafe fn export_fd_unchecked( &self, handle_type: ExternalSemaphoreHandleType, - ) -> Result { - let mut state = self.state.lock(); - self.export_fd_unchecked_locked(handle_type, &mut state) - } - - unsafe fn export_fd_unchecked_locked( - &self, - handle_type: ExternalSemaphoreHandleType, - state: &mut SemaphoreState, ) -> Result { let info_vk = ash::vk::SemaphoreGetFdInfoKHR { semaphore: self.handle, @@ -327,8 +278,6 @@ impl Semaphore { .result() .map_err(VulkanError::from)?; - state.export(handle_type); - #[cfg(unix)] { use std::os::unix::io::FromRawFd; @@ -346,21 +295,31 @@ impl Semaphore { /// /// The [`khr_external_semaphore_win32`](crate::device::DeviceExtensions::khr_external_semaphore_win32) /// extension must be enabled on the device. + /// + /// # Safety + /// + /// - If `handle_type` has copy transference, then the semaphore must be signaled, or a signal + /// operation on the semaphore must be pending, and no wait operations must be pending. + /// - The semaphore must not currently have an imported payload from a swapchain acquire + /// operation. + /// - If the semaphore has an imported payload, its handle type must allow re-exporting as + /// `handle_type`, as returned by [`PhysicalDevice::external_semaphore_properties`]. + /// - If `handle_type` is `ExternalSemaphoreHandleType::OpaqueWin32` or + /// `ExternalSemaphoreHandleType::D3D12Fence`, then a handle of this type must not have been + /// already exported from this semaphore. #[inline] pub fn export_win32_handle( &self, handle_type: ExternalSemaphoreHandleType, ) -> Result<*mut std::ffi::c_void, Validated> { - let mut state = self.state.lock(); - self.validate_export_win32_handle(handle_type, &state)?; + self.validate_export_win32_handle(handle_type)?; - unsafe { Ok(self.export_win32_handle_unchecked_locked(handle_type, &mut state)?) } + unsafe { Ok(self.export_win32_handle_unchecked(handle_type)?) } } fn validate_export_win32_handle( &self, handle_type: ExternalSemaphoreHandleType, - state: &SemaphoreState, ) -> Result<(), Box> { if !self .device @@ -405,98 +364,13 @@ impl Semaphore { })); } - if matches!( - handle_type, - ExternalSemaphoreHandleType::OpaqueWin32 | ExternalSemaphoreHandleType::D3D12Fence - ) && state.is_exported(handle_type) - { - return Err(Box::new(ValidationError { - problem: "`handle_type` is `ExternalSemaphoreHandleType::OpaqueWin32` or \ - `ExternalSemaphoreHandleType::D3D12Fence`, but \ - a handle of this type has already been exported from this semaphore" - .into(), - vuids: &["VUID-VkSemaphoreGetWin32HandleInfoKHR-handleType-01127"], - ..Default::default() - })); - } - - if let Some(imported_handle_type) = state.current_import { - match imported_handle_type { - ImportType::SwapchainAcquire => { - return Err(Box::new(ValidationError { - problem: "the semaphore currently has an imported payload from a \ - swapchain acquire operation" - .into(), - vuids: &["VUID-VkSemaphoreGetWin32HandleInfoKHR-semaphore-01128"], - ..Default::default() - })); - } - ImportType::ExternalSemaphore(imported_handle_type) => { - let external_semaphore_properties = unsafe { - self.device - .physical_device() - .external_semaphore_properties_unchecked( - ExternalSemaphoreInfo::handle_type(handle_type), - ) - }; - - if !external_semaphore_properties - .export_from_imported_handle_types - .intersects(imported_handle_type.into()) - { - return Err(Box::new(ValidationError { - problem: "the semaphore currently has an imported payload, whose type \ - does not allow re-exporting as `handle_type`, as \ - returned by `PhysicalDevice::external_semaphore_properties`" - .into(), - vuids: &["VUID-VkSemaphoreGetWin32HandleInfoKHR-semaphore-01128"], - ..Default::default() - })); - } - } - } - } - - if handle_type.has_copy_transference() { - if state.is_wait_pending() { - return Err(Box::new(ValidationError { - problem: "`handle_type` has copy transference, but \ - a wait operation on the semaphore is pending" - .into(), - vuids: &["VUID-VkSemaphoreGetWin32HandleInfoKHR-handleType-01129"], - ..Default::default() - })); - } - - if !(state.is_signaled().unwrap_or(false) || state.is_signal_pending()) { - return Err(Box::new(ValidationError { - problem: "`handle_type` has copy transference, but \ - the semaphore is not signaled, and \ - a signal operation on the semaphore is not pending" - .into(), - vuids: &["VUID-VkSemaphoreGetWin32HandleInfoKHR-handleType-01130"], - ..Default::default() - })); - } - } - Ok(()) } #[cfg_attr(not(feature = "document_unchecked"), doc(hidden))] - #[inline] pub unsafe fn export_win32_handle_unchecked( &self, handle_type: ExternalSemaphoreHandleType, - ) -> Result<*mut std::ffi::c_void, VulkanError> { - let mut state = self.state.lock(); - self.export_win32_handle_unchecked_locked(handle_type, &mut state) - } - - unsafe fn export_win32_handle_unchecked_locked( - &self, - handle_type: ExternalSemaphoreHandleType, - state: &mut SemaphoreState, ) -> Result<*mut std::ffi::c_void, VulkanError> { let info_vk = ash::vk::SemaphoreGetWin32HandleInfoKHR { semaphore: self.handle, @@ -513,27 +387,32 @@ impl Semaphore { .result() .map_err(VulkanError::from)?; - state.export(handle_type); - Ok(output.assume_init()) } /// Exports the semaphore into a Zircon event handle. + /// + /// # Safety + /// + /// - If `handle_type` has copy transference, then the semaphore must be signaled, or a signal + /// operation on the semaphore must be pending, and no wait operations must be pending. + /// - The semaphore must not currently have an imported payload from a swapchain acquire + /// operation. + /// - If the semaphore has an imported payload, its handle type must allow re-exporting as + /// `handle_type`, as returned by [`PhysicalDevice::external_semaphore_properties`]. #[inline] - pub fn export_zircon_handle( + pub unsafe fn export_zircon_handle( &self, handle_type: ExternalSemaphoreHandleType, ) -> Result> { - let mut state = self.state.lock(); - self.validate_export_zircon_handle(handle_type, &state)?; + self.validate_export_zircon_handle(handle_type)?; - unsafe { Ok(self.export_zircon_handle_unchecked_locked(handle_type, &mut state)?) } + unsafe { Ok(self.export_zircon_handle_unchecked(handle_type)?) } } fn validate_export_zircon_handle( &self, handle_type: ExternalSemaphoreHandleType, - state: &SemaphoreState, ) -> Result<(), Box> { if !self.device.enabled_extensions().fuchsia_external_semaphore { return Err(Box::new(ValidationError { @@ -566,83 +445,13 @@ impl Semaphore { })); } - if let Some(imported_handle_type) = state.current_import { - match imported_handle_type { - ImportType::SwapchainAcquire => { - return Err(Box::new(ValidationError { - problem: "the semaphore currently has an imported payload from a \ - swapchain acquire operation" - .into(), - vuids: &["VUID-VkSemaphoreGetZirconHandleInfoFUCHSIA-semaphore-04759"], - ..Default::default() - })); - } - ImportType::ExternalSemaphore(imported_handle_type) => { - let external_semaphore_properties = unsafe { - self.device - .physical_device() - .external_semaphore_properties_unchecked( - ExternalSemaphoreInfo::handle_type(handle_type), - ) - }; - - if !external_semaphore_properties - .export_from_imported_handle_types - .intersects(imported_handle_type.into()) - { - return Err(Box::new(ValidationError { - problem: "the semaphore currently has an imported payload, whose type \ - does not allow re-exporting as `handle_type`, as \ - returned by `PhysicalDevice::external_semaphore_properties`" - .into(), - vuids: &["VUID-VkSemaphoreGetZirconHandleInfoFUCHSIA-semaphore-04759"], - ..Default::default() - })); - } - } - } - } - - if handle_type.has_copy_transference() { - if state.is_wait_pending() { - return Err(Box::new(ValidationError { - problem: "`handle_type` has copy transference, but \ - a wait operation on the semaphore is pending" - .into(), - vuids: &["VUID-VkSemaphoreGetZirconHandleInfoFUCHSIA-handleType-04760"], - ..Default::default() - })); - } - - if !(state.is_signaled().unwrap_or(false) || state.is_signal_pending()) { - return Err(Box::new(ValidationError { - problem: "`handle_type` has copy transference, but \ - the semaphore is not signaled, and \ - a signal operation on the semaphore is not pending" - .into(), - vuids: &["VUID-VkSemaphoreGetZirconHandleInfoFUCHSIA-handleType-04761"], - ..Default::default() - })); - } - } - Ok(()) } #[cfg_attr(not(feature = "document_unchecked"), doc(hidden))] - #[inline] pub unsafe fn export_zircon_handle_unchecked( &self, handle_type: ExternalSemaphoreHandleType, - ) -> Result { - let mut state = self.state.lock(); - self.export_zircon_handle_unchecked_locked(handle_type, &mut state) - } - - unsafe fn export_zircon_handle_unchecked_locked( - &self, - handle_type: ExternalSemaphoreHandleType, - state: &mut SemaphoreState, ) -> Result { let info_vk = ash::vk::SemaphoreGetZirconHandleInfoFUCHSIA { semaphore: self.handle, @@ -661,8 +470,6 @@ impl Semaphore { .result() .map_err(VulkanError::from)?; - state.export(handle_type); - Ok(output.assume_init()) } @@ -673,6 +480,7 @@ impl Semaphore { /// /// # Safety /// + /// - The semaphore must not be in use by the device. /// - If in `import_semaphore_fd_info`, `handle_type` is `ExternalHandleType::OpaqueFd`, /// then `file` must represent a binary semaphore that was exported from Vulkan or a /// compatible API, with a driver and device UUID equal to those of the device that owns @@ -682,16 +490,14 @@ impl Semaphore { &self, import_semaphore_fd_info: ImportSemaphoreFdInfo, ) -> Result<(), Validated> { - let mut state = self.state.lock(); - self.validate_import_fd(&import_semaphore_fd_info, &state)?; + self.validate_import_fd(&import_semaphore_fd_info)?; - Ok(self.import_fd_unchecked_locked(import_semaphore_fd_info, &mut state)?) + Ok(self.import_fd_unchecked(import_semaphore_fd_info)?) } fn validate_import_fd( &self, import_semaphore_fd_info: &ImportSemaphoreFdInfo, - state: &SemaphoreState, ) -> Result<(), Box> { if !self.device.enabled_extensions().khr_external_semaphore_fd { return Err(Box::new(ValidationError { @@ -702,14 +508,6 @@ impl Semaphore { })); } - if state.is_in_queue() { - return Err(Box::new(ValidationError { - problem: "the semaphore is in use".into(), - vuids: &["VUID-vkImportSemaphoreFdKHR-semaphore-01142"], - ..Default::default() - })); - } - import_semaphore_fd_info .validate(&self.device) .map_err(|err| err.add_context("import_semaphore_fd_info"))?; @@ -718,19 +516,9 @@ impl Semaphore { } #[cfg_attr(not(feature = "document_unchecked"), doc(hidden))] - #[inline] pub unsafe fn import_fd_unchecked( &self, import_semaphore_fd_info: ImportSemaphoreFdInfo, - ) -> Result<(), VulkanError> { - let mut state = self.state.lock(); - self.import_fd_unchecked_locked(import_semaphore_fd_info, &mut state) - } - - unsafe fn import_fd_unchecked_locked( - &self, - import_semaphore_fd_info: ImportSemaphoreFdInfo, - state: &mut SemaphoreState, ) -> Result<(), VulkanError> { let ImportSemaphoreFdInfo { flags, @@ -764,11 +552,6 @@ impl Semaphore { .result() .map_err(VulkanError::from)?; - state.import( - handle_type, - flags.intersects(SemaphoreImportFlags::TEMPORARY), - ); - Ok(()) } @@ -779,6 +562,7 @@ impl Semaphore { /// /// # Safety /// + /// - The semaphore must not be in use by the device. /// - In `import_semaphore_win32_handle_info`, `handle` must represent a binary semaphore that /// was exported from Vulkan or a compatible API, with a driver and device UUID equal to /// those of the device that owns `self`. @@ -787,17 +571,14 @@ impl Semaphore { &self, import_semaphore_win32_handle_info: ImportSemaphoreWin32HandleInfo, ) -> Result<(), Validated> { - let mut state = self.state.lock(); - self.validate_import_win32_handle(&import_semaphore_win32_handle_info, &state)?; + self.validate_import_win32_handle(&import_semaphore_win32_handle_info)?; - Ok(self - .import_win32_handle_unchecked_locked(import_semaphore_win32_handle_info, &mut state)?) + Ok(self.import_win32_handle_unchecked(import_semaphore_win32_handle_info)?) } fn validate_import_win32_handle( &self, import_semaphore_win32_handle_info: &ImportSemaphoreWin32HandleInfo, - state: &SemaphoreState, ) -> Result<(), Box> { if !self .device @@ -812,14 +593,6 @@ impl Semaphore { })); } - if state.is_in_queue() { - return Err(Box::new(ValidationError { - problem: "the semaphore is in use".into(), - // vuids? - ..Default::default() - })); - } - import_semaphore_win32_handle_info .validate(&self.device) .map_err(|err| err.add_context("import_semaphore_win32_handle_info"))?; @@ -828,19 +601,9 @@ impl Semaphore { } #[cfg_attr(not(feature = "document_unchecked"), doc(hidden))] - #[inline] pub unsafe fn import_win32_handle_unchecked( &self, import_semaphore_win32_handle_info: ImportSemaphoreWin32HandleInfo, - ) -> Result<(), VulkanError> { - let mut state = self.state.lock(); - self.import_win32_handle_unchecked_locked(import_semaphore_win32_handle_info, &mut state) - } - - unsafe fn import_win32_handle_unchecked_locked( - &self, - import_semaphore_win32_handle_info: ImportSemaphoreWin32HandleInfo, - state: &mut SemaphoreState, ) -> Result<(), VulkanError> { let ImportSemaphoreWin32HandleInfo { flags, @@ -864,11 +627,6 @@ impl Semaphore { .result() .map_err(VulkanError::from)?; - state.import( - handle_type, - flags.intersects(SemaphoreImportFlags::TEMPORARY), - ); - Ok(()) } @@ -879,6 +637,7 @@ impl Semaphore { /// /// # Safety /// + /// - The semaphore must not be in use by the device. /// - In `import_semaphore_zircon_handle_info`, `zircon_handle` must have `ZX_RIGHTS_BASIC` and /// `ZX_RIGHTS_SIGNAL`. #[inline] @@ -886,19 +645,14 @@ impl Semaphore { &self, import_semaphore_zircon_handle_info: ImportSemaphoreZirconHandleInfo, ) -> Result<(), Validated> { - let mut state = self.state.lock(); - self.validate_import_zircon_handle(&import_semaphore_zircon_handle_info, &state)?; + self.validate_import_zircon_handle(&import_semaphore_zircon_handle_info)?; - Ok(self.import_zircon_handle_unchecked_locked( - import_semaphore_zircon_handle_info, - &mut state, - )?) + Ok(self.import_zircon_handle_unchecked(import_semaphore_zircon_handle_info)?) } fn validate_import_zircon_handle( &self, import_semaphore_zircon_handle_info: &ImportSemaphoreZirconHandleInfo, - state: &SemaphoreState, ) -> Result<(), Box> { if !self.device.enabled_extensions().fuchsia_external_semaphore { return Err(Box::new(ValidationError { @@ -909,14 +663,6 @@ impl Semaphore { })); } - if state.is_in_queue() { - return Err(Box::new(ValidationError { - problem: "the semaphore is in use".into(), - vuids: &["VUID-vkImportSemaphoreZirconHandleFUCHSIA-semaphore-04764"], - ..Default::default() - })); - } - import_semaphore_zircon_handle_info .validate(&self.device) .map_err(|err| err.add_context("import_semaphore_zircon_handle_info"))?; @@ -925,19 +671,9 @@ impl Semaphore { } #[cfg_attr(not(feature = "document_unchecked"), doc(hidden))] - #[inline] pub unsafe fn import_zircon_handle_unchecked( &self, import_semaphore_zircon_handle_info: ImportSemaphoreZirconHandleInfo, - ) -> Result<(), VulkanError> { - let mut state = self.state.lock(); - self.import_zircon_handle_unchecked_locked(import_semaphore_zircon_handle_info, &mut state) - } - - unsafe fn import_zircon_handle_unchecked_locked( - &self, - import_semaphore_zircon_handle_info: ImportSemaphoreZirconHandleInfo, - state: &mut SemaphoreState, ) -> Result<(), VulkanError> { let ImportSemaphoreZirconHandleInfo { flags, @@ -960,17 +696,8 @@ impl Semaphore { .result() .map_err(VulkanError::from)?; - state.import( - handle_type, - flags.intersects(SemaphoreImportFlags::TEMPORARY), - ); - Ok(()) } - - pub(crate) fn state(&self) -> MutexGuard<'_, SemaphoreState> { - self.state.lock() - } } impl Drop for Semaphore { @@ -1498,136 +1225,6 @@ pub struct ExternalSemaphoreProperties { pub compatible_handle_types: ExternalSemaphoreHandleTypes, } -#[derive(Debug, Default)] -pub(crate) struct SemaphoreState { - is_signaled: bool, - pending_signal: Option, - pending_wait: Option>, - - reference_exported: bool, - exported_handle_types: ExternalSemaphoreHandleTypes, - current_import: Option, - permanent_import: Option, -} - -impl SemaphoreState { - /// If the semaphore does not have a pending operation and has no external references, - /// returns the current status. - #[inline] - fn is_signaled(&self) -> Option { - // If any of these is true, we can't be certain of the status. - if self.pending_signal.is_some() - || self.pending_wait.is_some() - || self.has_external_reference() - { - None - } else { - Some(self.is_signaled) - } - } - - #[inline] - fn is_signal_pending(&self) -> bool { - self.pending_signal.is_some() - } - - #[inline] - fn is_wait_pending(&self) -> bool { - self.pending_wait.is_some() - } - - #[inline] - fn is_in_queue(&self) -> bool { - matches!(self.pending_signal, Some(SignalType::Queue(_))) || self.pending_wait.is_some() - } - - /// Returns whether there are any potential external references to the semaphore payload. - /// That is, the semaphore has been exported by reference transference, or imported. - #[inline] - fn has_external_reference(&self) -> bool { - self.reference_exported || self.current_import.is_some() - } - - #[allow(dead_code)] - #[inline] - fn is_exported(&self, handle_type: ExternalSemaphoreHandleType) -> bool { - self.exported_handle_types.intersects(handle_type.into()) - } - - #[inline] - pub(crate) unsafe fn add_queue_signal(&mut self, queue: &Arc) { - self.pending_signal = Some(SignalType::Queue(Arc::downgrade(queue))); - } - - #[inline] - pub(crate) unsafe fn add_queue_wait(&mut self, queue: &Arc) { - self.pending_wait = Some(Arc::downgrade(queue)); - } - - /// Called when a queue is unlocking resources. - #[inline] - pub(crate) unsafe fn set_signal_finished(&mut self) { - self.pending_signal = None; - self.is_signaled = true; - } - - /// Called when a queue is unlocking resources. - #[inline] - pub(crate) unsafe fn set_wait_finished(&mut self) { - self.pending_wait = None; - self.current_import = self.permanent_import.map(Into::into); - self.is_signaled = false; - } - - #[allow(dead_code)] - #[inline] - unsafe fn export(&mut self, handle_type: ExternalSemaphoreHandleType) { - self.exported_handle_types |= handle_type.into(); - - if handle_type.has_copy_transference() { - self.current_import = self.permanent_import.map(Into::into); - self.is_signaled = false; - } else { - self.reference_exported = true; - } - } - - #[allow(dead_code)] - #[inline] - unsafe fn import(&mut self, handle_type: ExternalSemaphoreHandleType, temporary: bool) { - self.current_import = Some(handle_type.into()); - - if !temporary { - self.permanent_import = Some(handle_type); - } - } - - #[inline] - pub(crate) unsafe fn swapchain_acquire(&mut self) { - self.pending_signal = Some(SignalType::SwapchainAcquire); - self.current_import = Some(ImportType::SwapchainAcquire); - } -} - -#[derive(Clone, Debug)] -enum SignalType { - Queue(Weak), - SwapchainAcquire, -} - -#[derive(Clone, Copy, Debug)] -enum ImportType { - SwapchainAcquire, - ExternalSemaphore(ExternalSemaphoreHandleType), -} - -impl From for ImportType { - #[inline] - fn from(handle_type: ExternalSemaphoreHandleType) -> Self { - Self::ExternalSemaphore(handle_type) - } -} - #[cfg(test)] mod tests { use crate::{ @@ -1717,8 +1314,9 @@ mod tests { }, ) .unwrap(); - let _fd = sem - .export_fd(ExternalSemaphoreHandleType::OpaqueFd) - .unwrap(); + let _fd = unsafe { + sem.export_fd(ExternalSemaphoreHandleType::OpaqueFd) + .unwrap() + }; } }