[mtl] improved semaphore synchronization

Author: kvark

src/backend/metal/src/command.rs

@@ -1211,6 +1211,22 @@ impl CommandQueue {
             shared,
         }
     }
+
+    fn wait<I>(&mut self, wait_semaphores: I)
+    where
+        I: IntoIterator,
+        I::Item: Borrow<native::Semaphore>,
+    {
+        for semaphore in wait_semaphores {
+            let sem = semaphore.borrow();
+            if let Some(ref system) = sem.system {
+                system.wait(!0);
+            }
+            if let Some(swap_image) = sem.image_ready.lock().unwrap().take() {
+                swap_image.wait_until_ready();
+            }
+        }
+    }
 }
 
 impl RawCommandQueue<Backend> for CommandQueue {
@@ -1222,17 +1238,20 @@ impl RawCommandQueue<Backend> for CommandQueue {
         IC::Item: Borrow<CommandBuffer>,
     {
         debug!("submitting with fence {:?}", fence);
-        // FIXME: wait for semaphores!
 
-        // FIXME: multiple buffers signaling!
-        let signal_block = if !submit.signal_semaphores.is_empty() {
-            let semaphores_copy: Vec<_> = submit.signal_semaphores.iter().map(|semaphore| {
-                semaphore.0
-            }).collect();
+        self.wait(submit.wait_semaphores.iter().map(|&(s, _)| s));
+
+        let system_semaphores = submit.signal_semaphores
+            .into_iter()
+            .filter_map(|semaphore| {
+                semaphore.system.clone()
+            })
+            .collect::<Vec<_>>();
+        let signal_block = if !system_semaphores.is_empty() {
             //Note: careful with those `ConcreteBlock::copy()` calls!
             Some(ConcreteBlock::new(move |_cb: *mut ()| -> () {
-                for semaphore in semaphores_copy.iter() {
-                    native::dispatch_semaphore_signal(*semaphore);
+                for semaphore in &system_semaphores {
+                    semaphore.signal();
                 }
             }).copy())
         } else {
@@ -1297,18 +1316,19 @@ impl RawCommandQueue<Backend> for CommandQueue {
         }
     }
 
-    fn present<IS, S, IW>(&mut self, swapchains: IS, _wait_semaphores: IW) -> Result<(), ()>
+    fn present<IS, S, IW>(&mut self, swapchains: IS, wait_semaphores: IW) -> Result<(), ()>
     where
         IS: IntoIterator<Item = (S, SwapImageIndex)>,
         S: Borrow<window::Swapchain>,
         IW: IntoIterator,
         IW::Item: Borrow<native::Semaphore>,
     {
+        self.wait(wait_semaphores);
+
         let queue = self.shared.queue.lock().unwrap();
         let command_buffer = queue.raw.new_command_buffer();
 
         for (swapchain, index) in swapchains {
-            // TODO: wait for semaphores
             debug!("presenting frame {}", index);
             let drawable = swapchain.borrow().take_drawable(index);
             command_buffer.present_drawable(&drawable);

src/backend/metal/src/device.rs

@@ -264,6 +264,7 @@ impl PhysicalDevice {
         let private_caps = {
             let device = &*shared.device.lock().unwrap();
             PrivateCapabilities {
+                exposed_queues: 1,
                 resource_heaps: Self::supports_any(device, RESOURCE_HEAP_SUPPORT),
                 argument_buffers: Self::supports_any(device, ARGUMENT_BUFFER_SUPPORT) && false, //TODO
                 shared_textures: !Self::is_mac(device),
@@ -328,7 +329,9 @@ impl hal::PhysicalDevice<Backend> for PhysicalDevice {
         }
 
         let mut queue_group = hal::backend::RawQueueGroup::new(family);
-        queue_group.add_queue(command::CommandQueue::new(self.shared.clone()));
+        for _ in 0 .. self.private_caps.exposed_queues {
+            queue_group.add_queue(command::CommandQueue::new(self.shared.clone()));
+        }
 
         let device = Device {
             shared: self.shared.clone(),
@@ -1325,7 +1328,16 @@ impl hal::Device<Backend> for Device {
     }
 
     fn create_semaphore(&self) -> n::Semaphore {
-        unsafe { n::Semaphore(n::dispatch_semaphore_create(1)) } // Returns retained
+        n::Semaphore {
+            // Semaphore synchronization between command buffers of the same queue
+            // is useless, don't bother even creating one.
+            system: if self.private_caps.exposed_queues > 1 {
+                Some(n::SystemSemaphore::new())
+            } else {
+                None
+            },
+            image_ready: Arc::new(Mutex::new(None)),
+        }
     }
 
     fn create_descriptor_pool<I>(&self, _max_sets: usize, descriptor_ranges: I) -> n::DescriptorPool
@@ -1524,8 +1536,7 @@ impl hal::Device<Backend> for Device {
     fn destroy_framebuffer(&self, _buffer: n::Framebuffer) {
     }
 
-    fn destroy_semaphore(&self, semaphore: n::Semaphore) {
-        unsafe { n::dispatch_release(semaphore.0) }
+    fn destroy_semaphore(&self, _semaphore: n::Semaphore) {
     }
 
     fn allocate_memory(&self, memory_type: hal::MemoryTypeId, size: u64) -> Result<n::Memory, OutOfMemory> {

src/backend/metal/src/lib.rs

@@ -198,6 +198,7 @@ impl hal::Backend for Backend {
 
 #[derive(Clone, Copy, Debug)]
 struct PrivateCapabilities {
+    exposed_queues: usize,
     resource_heaps: bool,
     argument_buffers: bool,
     shared_textures: bool,

src/backend/metal/src/native.rs

@@ -1,5 +1,6 @@
 use Backend;
 use internal::Channel;
+use window::SwapchainImage;
 
 use std::collections::HashMap;
 use std::ops::Range;
@@ -210,10 +211,10 @@ unsafe impl Send for Sampler {}
 unsafe impl Sync for Sampler {}
 
 #[derive(Debug)]
-pub struct Semaphore(pub(crate) *mut c_void);
-
-unsafe impl Send for Semaphore {}
-unsafe impl Sync for Semaphore {}
+pub struct Semaphore {
+    pub(crate) system: Option<SystemSemaphore>,
+    pub(crate) image_ready: Arc<Mutex<Option<SwapchainImage>>>,
+}
 
 #[derive(Debug)]
 pub struct Buffer {
@@ -481,21 +482,50 @@ pub struct FenceInner {
 pub type Fence = Arc<FenceInner>;
 
 extern "C" {
-    #[allow(dead_code)]
-    pub fn dispatch_semaphore_wait(
+    fn dispatch_semaphore_wait(
         semaphore: *mut c_void,
         timeout: u64,
     ) -> c_long;
 
-    pub fn dispatch_semaphore_signal(
+    fn dispatch_semaphore_signal(
         semaphore: *mut c_void,
     ) -> c_long;
 
-    pub fn dispatch_semaphore_create(
+    fn dispatch_semaphore_create(
         value: c_long,
     ) -> *mut c_void;
 
-    pub fn dispatch_release(
+    fn dispatch_release(
         object: *mut c_void,
     );
 }
+
+#[derive(Clone, Debug)]
+pub struct SystemSemaphore(*mut c_void);
+unsafe impl Send for SystemSemaphore {}
+unsafe impl Sync for SystemSemaphore {}
+
+impl Drop for SystemSemaphore {
+    fn drop(&mut self) {
+        unsafe {
+            dispatch_release(self.0)
+        }
+    }
+}
+impl SystemSemaphore {
+    pub(crate) fn new() -> Self {
+        SystemSemaphore(unsafe {
+            dispatch_semaphore_create(1)
+        })
+    }
+    pub(crate) fn signal(&self) {
+        unsafe {
+            dispatch_semaphore_signal(self.0);
+        }
+    }
+    pub(crate) fn wait(&self, timeout: u64) {
+        unsafe {
+            dispatch_semaphore_wait(self.0, timeout);
+        }
+    }
+}