Fix stuttering; add configurable frames-in-flight

CHANGELOG.md

@@ -10,10 +10,12 @@ adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 ### Fixed
 
 - Crash/device lost while resizing the window
+- Inconsistent frame timing on certain drivers
 
 ### Added
 
 - Fullscreen demostration in `vsm_omni` example using F11 and F12 keys
+- Configurable frames-in-flight setting
 
 ## [0.6.1] - 2022-10-16
 

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "screen-13"
-version = "0.6.1"
+version = "0.6.2"
 authors = ["John Wells <[email protected]>"]
 edition = "2021"
 license = "MIT OR Apache-2.0"

src/display.rs

@@ -19,20 +19,28 @@ use {
 
 /// A physical display interface.
 pub struct Display {
-    cmd_bufs: Vec<[CommandBuffer; 3]>,
-    device: Arc<Device>,
+    cmd_buf_idx: usize,
+    cmd_bufs: Vec<CommandBuffer>,
     pool: Box<dyn ResolverPool>,
     swapchain: Swapchain,
 }
 
 impl Display {
     /// Constructs a new `Display` object.
-    pub fn new(device: &Arc<Device>, pool: Box<dyn ResolverPool>, swapchain: Swapchain) -> Self {
-        let device = Arc::clone(device);
+    pub fn new(
+        device: &Arc<Device>,
+        pool: Box<dyn ResolverPool>,
+        swapchain: Swapchain,
+        cmd_buf_count: usize,
+    ) -> Self {
+        let mut cmd_bufs = Vec::with_capacity(cmd_buf_count);
+        for _ in 0..cmd_buf_count {
+            cmd_bufs.push(CommandBuffer::create(device, device.queue.family).unwrap());
+        }
 
         Self {
-            device,
-            cmd_bufs: Default::default(),
+            cmd_buf_idx: 0,
+            cmd_bufs,
             pool,
             swapchain,
         }
@@ -76,62 +84,26 @@ impl Display {
             .expect("uninitialized swapchain image: write something each frame!");
         let mut resolver = render_graph.resolve();
         let wait_dst_stage_mask = resolver.node_pipeline_stages(swapchain_image);
-        let swapchain_node = swapchain_image;
-        let swapchain_image = resolver.unbind_node(swapchain_node);
-        let swapchain_image_idx = swapchain_image.idx as usize;
-
-        while self.cmd_bufs.len() <= swapchain_image_idx {
-            self.cmd_bufs.push([
-                CommandBuffer::create(&self.device, self.device.queue.family)?,
-                CommandBuffer::create(&self.device, self.device.queue.family)?,
-                CommandBuffer::create(&self.device, self.device.queue.family)?,
-            ]);
-        }
 
-        let cmd_bufs = &mut self.cmd_bufs[swapchain_image_idx];
-        let cmd_buf = &mut cmd_bufs[0];
+        self.cmd_buf_idx += 1;
+        self.cmd_buf_idx %= self.cmd_bufs.len();
+
+        let cmd_buf = &mut self.cmd_bufs[self.cmd_buf_idx];
 
         let started = Instant::now();
 
         unsafe {
             Self::wait_for_fence(cmd_buf)?;
         }
 
-        let mut wait_elapsed = Instant::now() - started;
-
         unsafe {
             Self::begin(cmd_buf)?;
         }
 
-        resolver.record_node_dependencies(&mut *self.pool, cmd_buf, swapchain_node)?;
-
-        unsafe {
-            trace!("submitting swapchain dependencies");
-
-            // Record up to but not including the swapchain work
-            Self::submit(
-                cmd_buf,
-                vk::SubmitInfo::builder().command_buffers(from_ref(cmd_buf)),
-            )?;
-        }
-
-        // Switch commnd buffers because we're going to be submitting with a wait semaphore on the
-        // swapchain image before we get access to record commands that use it
-        let cmd_buf = &mut cmd_bufs[1];
-
-        let wait_started = Instant::now();
-
-        unsafe {
-            Self::wait_for_fence(cmd_buf)?;
-        }
-
-        wait_elapsed += Instant::now() - wait_started;
-
-        unsafe {
-            Self::begin(cmd_buf)?;
-        }
+        // resolver.record_node_dependencies(&mut *self.pool, cmd_buf, swapchain_image)?;
+        resolver.record_node(&mut *self.pool, cmd_buf, swapchain_image)?;
 
-        resolver.record_node(&mut *self.pool, cmd_buf, swapchain_node)?;
+        let swapchain_image = resolver.unbind_node(swapchain_image);
 
         pipeline_barrier(
             &cmd_buf.device,
@@ -157,9 +129,13 @@ impl Display {
             }),
         );
 
-        unsafe {
-            trace!("submitting swapchain passes");
+        // We may have unresolved nodes; things like copies that happen after present or operations
+        // before present which use nodes that are unused in the remainder of the graph.
+        // These operations are still important, but they don't need to wait for any of the above
+        // things so we do them last
+        resolver.record_unscheduled_passes(&mut *self.pool, cmd_buf)?;
 
+        unsafe {
             Self::submit(
                 cmd_buf,
                 vk::SubmitInfo::builder()
@@ -170,49 +146,14 @@ impl Display {
             )?;
         }
 
-        let cmd_buf = &mut cmd_bufs[2];
-
-        let wait_started = Instant::now();
-
-        unsafe {
-            Self::wait_for_fence(cmd_buf)?;
-        }
-
-        wait_elapsed += Instant::now() - wait_started;
-
-        // We may have unresolved nodes; things like copies that happen after present or operations
-        // before present which use nodes that are unused in the remainder of the graph.
-        // These operations are still important, but they don't need to wait for any of the above
-        // things so we do them last
-        if !resolver.is_resolved() {
-            unsafe {
-                Self::begin(cmd_buf)?;
-            }
-
-            resolver.record_unscheduled_passes(&mut *self.pool, cmd_buf)?;
-
-            unsafe {
-                trace!("submitting unscheduled passes");
-
-                Self::submit(
-                    cmd_buf,
-                    vk::SubmitInfo::builder().command_buffers(from_ref(cmd_buf)),
-                )
-            }?;
-        }
-
-        let elapsed = Instant::now() - started - wait_elapsed;
-        trace!(
-            "🔜🔜🔜 vkQueueSubmit took {} μs (delay {} μs)",
-            elapsed.as_micros(),
-            wait_elapsed.as_micros()
-        );
+        let elapsed = Instant::now() - started;
+        trace!("🔜🔜🔜 vkQueueSubmit took {} μs", elapsed.as_micros(),);
 
         self.swapchain.present_image(swapchain_image);
 
         // Store the resolved graph because it contains bindings, leases, and other shared resources
         // that need to be kept alive until the fence is waited upon.
-        CommandBuffer::push_fenced_drop(&mut self.cmd_bufs[swapchain_image_idx][2], resolver);
+        CommandBuffer::push_fenced_drop(cmd_buf, resolver);
 
         Ok(())
     }
@@ -227,10 +168,6 @@ impl Display {
             .device
             .end_command_buffer(**cmd_buf)
             .map_err(|_| ())?;
-        cmd_buf
-            .device
-            .reset_fences(from_ref(&cmd_buf.fence))
-            .map_err(|_| ())?;
         cmd_buf
             .device
             .queue_submit(
@@ -242,10 +179,15 @@ impl Display {
     }
 
     unsafe fn wait_for_fence(cmd_buf: &mut CommandBuffer) -> Result<(), ()> {
+        use std::slice::from_ref;
+
         Device::wait_for_fence(&cmd_buf.device, &cmd_buf.fence).map_err(|_| ())?;
         CommandBuffer::drop_fenced(cmd_buf);
 
-        Ok(())
+        cmd_buf
+            .device
+            .reset_fences(from_ref(&cmd_buf.fence))
+            .map_err(|_| ())
     }
 }
 

src/driver/descriptor_set.rs

@@ -1,7 +1,7 @@
 use {
     super::{DescriptorSetLayout, Device, DriverError},
     ash::vk,
-    log::{trace, warn},
+    log::warn,
     std::{ops::Deref, sync::Arc, thread::panicking},
 };
 
@@ -157,8 +157,6 @@ impl DescriptorPool {
             .set_layouts(from_ref(layout));
         create_info.descriptor_set_count = count;
 
-        trace!("allocate_descriptor_sets");
-
         Ok(unsafe {
             this.device
                 .allocate_descriptor_sets(&create_info)

src/driver/image.rs

@@ -13,6 +13,7 @@ use {
     std::{
         collections::{hash_map::Entry, HashMap},
         fmt::{Debug, Formatter},
+        mem::take,
         ops::Deref,
         ptr::null,
         sync::{
@@ -244,11 +245,15 @@ impl Image {
     }
 
     pub(super) fn clone_raw(this: &Self) -> Self {
+        // Moves the image view cache from the current instance to the clone!
+        let mut image_view_cache = this.image_view_cache.lock();
+        let image_view_cache = take(&mut *image_view_cache);
+
         Self {
             allocation: None,
             device: Arc::clone(&this.device),
             image: this.image,
-            image_view_cache: Mutex::new(Default::default()),
+            image_view_cache: Mutex::new(image_view_cache),
             info: this.info,
             name: this.name.clone(),
             prev_access: AtomicU8::new(access_type_into_u8(AccessType::Nothing)),
@@ -307,11 +312,11 @@ impl Drop for Image {
             return;
         }
 
-        self.image_view_cache.lock().clear();
-
         // When our allocation is some we allocated ourself; otherwise somebody
         // else owns this image and we should not destroy it. Usually it's the swapchain...
         if let Some(allocation) = self.allocation.take() {
+            self.image_view_cache.lock().clear();
+
             unsafe {
                 self.device.destroy_image(self.image, None);
             }

src/event_loop.rs

@@ -187,6 +187,7 @@ impl AsRef<winit::event_loop::EventLoop<()>> for EventLoop {
 
 /// Builder for `EventLoop`.
 pub struct EventLoopBuilder {
+    cmd_buf_count: usize,
     driver_cfg: DriverConfigBuilder,
     event_loop: winit::event_loop::EventLoop<()>,
     resolver_pool: Option<Box<dyn ResolverPool>>,
@@ -202,6 +203,7 @@ impl Debug for EventLoopBuilder {
 impl Default for EventLoopBuilder {
     fn default() -> Self {
         Self {
+            cmd_buf_count: 5,
             driver_cfg: DriverConfigBuilder::default(),
             event_loop: winit::event_loop::EventLoop::new(),
             resolver_pool: None,
@@ -216,6 +218,19 @@ impl EventLoopBuilder {
         self.event_loop.available_monitors()
     }
 
+    /// Specifies the number of in-flight command buffers, which should be greater
+    /// than or equal to the desired swapchain image count.
+    ///
+    /// More command buffers mean less time waiting for previously submitted frames to complete, but
+    /// more memory in use.
+    ///
+    /// Generally a value of one or two greater than desired image count produces the smoothest
+    /// animation.
+    pub fn command_buffer_count(mut self, cmd_buf_count: usize) -> Self {
+        self.cmd_buf_count = cmd_buf_count;
+        self
+    }
+
     /// Provides a closure which configures the `DriverConfig` instance.
     pub fn configure<ConfigureFn>(mut self, configure_fn: ConfigureFn) -> Self
     where
@@ -336,7 +351,7 @@ impl EventLoopBuilder {
         let pool = self
             .resolver_pool
             .unwrap_or_else(|| Box::new(HashPool::new(&driver.device)));
-        let display = Display::new(&driver.device, pool, driver.swapchain);
+        let display = Display::new(&driver.device, pool, driver.swapchain, self.cmd_buf_count);
 
         info!(
             "display resolution: {}x{} ({}x scale)",