wgpu_hal::vulkan: Introduce SemaphoreList helper type. (#7749)

wgpu-hal/src/vulkan/adapter.rs

@@ -2156,7 +2156,7 @@ impl super::Adapter {
             device: Arc::clone(&shared),
             family_index,
             relay_semaphores: Mutex::new(relay_semaphores),
-            signal_semaphores: Mutex::new((Vec::new(), Vec::new())),
+            signal_semaphores: Default::default(),
         };
 
         let mem_allocator = {

wgpu-hal/src/vulkan/mod.rs

@@ -31,11 +31,12 @@ mod device;
 mod drm;
 mod instance;
 mod sampler;
+mod semaphore_list;
 
 pub use adapter::PhysicalDeviceFeatures;
 
 use alloc::{boxed::Box, ffi::CString, sync::Arc, vec::Vec};
-use core::{borrow::Borrow, ffi::CStr, fmt, mem, num::NonZeroU32, ops::DerefMut};
+use core::{borrow::Borrow, ffi::CStr, fmt, mem, num::NonZeroU32};
 
 use arrayvec::ArrayVec;
 use ash::{ext, khr, vk};
@@ -46,6 +47,8 @@ use parking_lot::{Mutex, RwLock};
 use naga::FastHashMap;
 use wgt::InternalCounter;
 
+use semaphore_list::SemaphoreList;
+
 const MILLIS_TO_NANOS: u64 = 1_000_000;
 const MAX_TOTAL_ATTACHMENTS: usize = crate::MAX_COLOR_ATTACHMENTS * 2 + 1;
 
@@ -759,7 +762,7 @@ pub struct Queue {
     device: Arc<DeviceShared>,
     family_index: u32,
     relay_semaphores: Mutex<RelaySemaphores>,
-    signal_semaphores: Mutex<(Vec<vk::Semaphore>, Vec<u64>)>,
+    signal_semaphores: Mutex<SemaphoreList>,
 }
 
 impl Queue {
@@ -1202,8 +1205,7 @@ impl crate::Queue for Queue {
 
         let mut wait_stage_masks = Vec::new();
         let mut wait_semaphores = Vec::new();
-        let mut signal_semaphores = Vec::new();
-        let mut signal_values = Vec::new();
+        let mut signal_semaphores = SemaphoreList::default();
 
         // Double check that the same swapchain image isn't being given to us multiple times,
         // as that will deadlock when we try to lock them all.
@@ -1242,17 +1244,12 @@ impl crate::Queue for Queue {
             // Get a semaphore to signal when we're done writing to this surface
             // image. Presentation of this image will wait for this.
             let signal_semaphore = swapchain_semaphore.get_submit_signal_semaphore(&self.device)?;
-            signal_semaphores.push(signal_semaphore);
-            signal_values.push(!0);
+            signal_semaphores.push_binary(signal_semaphore);
         }
 
-        let mut guards = self.signal_semaphores.lock();
-        let (ref mut pending_signal_semaphores, ref mut pending_signal_semaphore_values) =
-            guards.deref_mut();
-        assert!(pending_signal_semaphores.len() == pending_signal_semaphore_values.len());
-        if !pending_signal_semaphores.is_empty() {
-            signal_semaphores.append(pending_signal_semaphores);
-            signal_values.append(pending_signal_semaphore_values);
+        let mut guard = self.signal_semaphores.lock();
+        if !guard.is_empty() {
+            signal_semaphores.append(&mut guard);
         }
 
         // In order for submissions to be strictly ordered, we encode a dependency between each submission
@@ -1264,15 +1261,13 @@ impl crate::Queue for Queue {
             wait_semaphores.push(sem);
         }
 
-        signal_semaphores.push(semaphore_state.signal);
-        signal_values.push(!0);
+        signal_semaphores.push_binary(semaphore_state.signal);
 
         // We need to signal our wgpu::Fence if we have one, this adds it to the signal list.
         signal_fence.maintain(&self.device.raw)?;
         match *signal_fence {
             Fence::TimelineSemaphore(raw) => {
-                signal_semaphores.push(raw);
-                signal_values.push(signal_value);
+                signal_semaphores.push_timeline(raw, signal_value);
             }
             Fence::FencePool {
                 ref mut active,
@@ -1301,16 +1296,10 @@ impl crate::Queue for Queue {
 
         vk_info = vk_info
             .wait_semaphores(&wait_semaphores)
-            .wait_dst_stage_mask(&wait_stage_masks)
-            .signal_semaphores(&signal_semaphores);
+            .wait_dst_stage_mask(&wait_stage_masks);
 
-        let mut vk_timeline_info;
-
-        if self.device.private_caps.timeline_semaphores {
-            vk_timeline_info =
-                vk::TimelineSemaphoreSubmitInfo::default().signal_semaphore_values(&signal_values);
-            vk_info = vk_info.push_next(&mut vk_timeline_info);
-        }
+        let mut vk_timeline_info = mem::MaybeUninit::uninit();
+        vk_info = signal_semaphores.add_to_submit(vk_info, &mut vk_timeline_info);
 
         profiling::scope!("vkQueueSubmit");
         unsafe {
@@ -1389,10 +1378,12 @@ impl Queue {
     }
 
     pub fn add_signal_semaphore(&self, semaphore: vk::Semaphore, semaphore_value: Option<u64>) {
-        let mut guards = self.signal_semaphores.lock();
-        let (ref mut semaphores, ref mut semaphore_values) = guards.deref_mut();
-        semaphores.push(semaphore);
-        semaphore_values.push(semaphore_value.unwrap_or(!0));
+        let mut guard = self.signal_semaphores.lock();
+        if let Some(value) = semaphore_value {
+            guard.push_timeline(semaphore, value);
+        } else {
+            guard.push_binary(semaphore);
+        }
     }
 }
 

wgpu-hal/src/vulkan/semaphore_list.rs

@@ -0,0 +1,115 @@
+//! Definition of the [`SemaphoreList`] type.
+
+use alloc::vec::Vec;
+use ash::vk;
+use core::mem::MaybeUninit;
+
+/// A list of Vulkan semaphores to signal.
+///
+/// This represents a list of binary or timeline semaphores, together
+/// with values for the timeline semaphores.
+///
+/// This type ensures that the array of semaphores to be signaled
+/// stays aligned with the array of values for timeline semaphores
+/// appearing in that list. The [`add_to_submit`] method prepares the
+/// `vkQueueSubmit` arguments appropriately for whatever semaphores we
+/// actually have.
+///
+/// [`add_to_submit`]: SemaphoreList::add_to_submit
+#[derive(Debug, Default)]
+pub struct SemaphoreList {
+    /// Semaphores to signal.
+    ///
+    /// This can be a mix of binary and timeline semaphores.
+    semaphores: Vec<vk::Semaphore>,
+
+    /// Values for timeline semaphores.
+    ///
+    /// If no timeline semaphores are present in [`semaphores`], this
+    /// is empty. If any timeline semaphores are present, then this
+    /// has the same length as [`semaphores`], with dummy !0 values
+    /// in the elements corresponding to binary semaphores, since
+    /// Vulkan ignores these.
+    ///
+    /// [`semaphores`]: Self::semaphores
+    values: Vec<u64>,
+}
+
+impl SemaphoreList {
+    pub fn is_empty(&self) -> bool {
+        self.semaphores.is_empty()
+    }
+
+    /// Add this list to the semaphores to be signalled by a `vkQueueSubmit` call.
+    ///
+    /// - Set `submit_info`'s `pSignalSemaphores` list to this list's
+    ///   semaphores.
+    ///
+    /// - If this list contains any timeline semaphores, then initialize
+    ///   `timeline_info`, set its `pSignalSemaphoreValues` to this
+    ///   list's values, and add it to `submit_info`s extension chain.
+    ///
+    /// Return the revised `submit_info` value.
+    pub fn add_to_submit<'i, 's: 'i>(
+        &'s self,
+        submit_info: vk::SubmitInfo<'i>,
+        timeline_info: &'i mut MaybeUninit<vk::TimelineSemaphoreSubmitInfo<'i>>,
+    ) -> vk::SubmitInfo<'i> {
+        self.check();
+        let mut submit_info = submit_info.signal_semaphores(&self.semaphores);
+        if !self.values.is_empty() {
+            let timeline_info = timeline_info.write(
+                vk::TimelineSemaphoreSubmitInfo::default().signal_semaphore_values(&self.values),
+            );
+            submit_info = submit_info.push_next(timeline_info);
+        }
+        submit_info
+    }
+
+    /// Add a binary semaphore to this list.
+    pub fn push_binary(&mut self, semaphore: vk::Semaphore) {
+        self.semaphores.push(semaphore);
+        // Push a dummy value if necessary.
+        if !self.values.is_empty() {
+            self.values.push(!0);
+        }
+        self.check();
+    }
+
+    /// Add a timeline semaphore to this list, to be signalled with
+    /// `value`.
+    pub fn push_timeline(&mut self, semaphore: vk::Semaphore, value: u64) {
+        self.pad_values();
+        self.semaphores.push(semaphore);
+        self.values.push(value);
+        self.check();
+    }
+
+    /// Append `other` to `self`, leaving `other` empty.
+    pub fn append(&mut self, other: &mut Self) {
+        // If we're about to receive values, ensure we're aligned first.
+        if !other.values.is_empty() {
+            self.pad_values();
+        }
+        self.semaphores.append(&mut other.semaphores);
+        self.values.append(&mut other.values);
+        // If we had values, but `other` did not, re-align.
+        if !self.values.is_empty() {
+            self.pad_values();
+        }
+        self.check();
+    }
+
+    /// Pad `self.values` with dummy values for binary semaphores,
+    /// in preparation for adding a timeline semaphore value.
+    ///
+    /// This is a no-op if we already have values.
+    fn pad_values(&mut self) {
+        self.values.resize(self.semaphores.len(), !0);
+    }
+
+    #[track_caller]
+    fn check(&self) {
+        debug_assert!(self.values.is_empty() || self.values.len() == self.semaphores.len());
+    }
+}