refactor(render): slight leveldrawpass refactors

5 files changed, 91 insertions, 61 deletions

diff --git a/stockton-render/src/draw/draw_passes/cons.rs b/stockton-render/src/draw/draw_passes/cons.rs
index 8f912ec..542c26a 100644
--- a/stockton-render/src/draw/draw_passes/cons.rs
+++ b/stockton-render/src/draw/draw_passes/cons.rs
@@ -33,21 +33,3 @@ impl<A: DrawPass, B: DrawPass> DrawPass for ConsDrawPass<A, B> {
         self.b.deactivate(device)
     }
 }
-
-/// A draw pass that does nothing. Can be used at the end of sequences if there's an odd number of draw passes.
-pub struct NilDrawPass;
-
-impl DrawPass for NilDrawPass {
-    fn queue_draw(
-        &mut self,
-        _input: &Session,
-        _img_view: &ImageViewT,
-        _cmd_buffer: &mut CommandBufferT,
-    ) -> Result<()> {
-        Ok(())
-    }
-
-    fn deactivate(self, _device: &mut Arc<RwLock<DeviceT>>) -> Result<()> {
-        Ok(())
-    }
-}
diff --git a/stockton-render/src/draw/draw_passes/level.rs b/stockton-render/src/draw/draw_passes/level.rs
index 3a4e748..d23990d 100644
--- a/stockton-render/src/draw/draw_passes/level.rs
+++ b/stockton-render/src/draw/draw_passes/level.rs
@@ -1,6 +1,6 @@
 //! Minimal code for drawing any level, based on traits from stockton-levels
 
-use super::{DrawPass, IntoDrawPass};
+use super::{util::TargetSpecificResources, DrawPass, IntoDrawPass};
 use crate::{
     draw::{
         buffers::{
@@ -69,9 +69,8 @@ pub struct LevelDrawPass<'a, M> {
     active_camera: Entity,
     draw_buffers: DrawBuffers<'a, UvPoint>,
 
-    framebuffers: Vec<FramebufferT>,
-    depth_buffers: Vec<DedicatedLoadedImage>,
-    next_resources: usize,
+    framebuffers: TargetSpecificResources<FramebufferT>,
+    depth_buffers: TargetSpecificResources<DedicatedLoadedImage>,
 
     _d: PhantomData<M>,
 }
@@ -106,9 +105,8 @@ where
         let map = map_lock.read().map_err(|_| LockPoisoned::Map)?;
 
         // Get framebuffer and depth buffer
-        let fb = &self.framebuffers[self.next_resources];
-        let db = &self.depth_buffers[self.next_resources];
-        self.next_resources = (self.next_resources + 1) % self.framebuffers.len();
+        let fb = self.framebuffers.get_next();
+        let db = self.depth_buffers.get_next();
 
         unsafe {
             cmd_buffer.begin_render_pass(
@@ -253,10 +251,10 @@ where
             let mut device = device_lock.write().map_err(|_| LockPoisoned::Device)?;
             self.pipeline.deactivate(&mut device);
             self.draw_buffers.deactivate(&mut device);
-            for fb in self.framebuffers.into_iter() {
+            for fb in self.framebuffers.dissolve() {
                 device.destroy_framebuffer(fb);
             }
-            for db in self.depth_buffers.into_iter() {
+            for db in self.depth_buffers.dissolve() {
                 db.deactivate(&mut device);
             }
         }
@@ -391,45 +389,43 @@ where
                 )
                 .context("Error building pipeline")?;
 
-            let mut framebuffers = Vec::with_capacity(swapchain_properties.image_count as usize);
-            let mut depth_buffers = Vec::with_capacity(swapchain_properties.image_count as usize);
-            let fat = FramebufferAttachment {
-                usage: Usage::COLOR_ATTACHMENT,
-                format: swapchain_properties.format,
-                view_caps: ViewCapabilities::empty(),
-            };
+            let fat = swapchain_properties.framebuffer_attachment();
             let dat = FramebufferAttachment {
                 usage: Usage::DEPTH_STENCIL_ATTACHMENT,
                 format: swapchain_properties.depth_format,
                 view_caps: ViewCapabilities::empty(),
             };
-            for _i in 0..swapchain_properties.image_count {
-                unsafe {
-                    framebuffers.push(device.create_framebuffer(
+            let framebuffers = TargetSpecificResources::new(
+                || unsafe {
+                    Ok(device.create_framebuffer(
                         &pipeline.renderpass,
                         IntoIter::new([fat.clone(), dat.clone()]),
                         swapchain_properties.extent,
-                    )?);
-                    depth_buffers.push(
-                        DedicatedLoadedImage::new(
-                            &mut device,
-                            adapter,
-                            swapchain_properties.depth_format,
-                            Usage::DEPTH_STENCIL_ATTACHMENT,
-                            SubresourceRange {
-                                aspects: Aspects::DEPTH,
-                                level_start: 0,
-                                level_count: Some(1),
-                                layer_start: 0,
-                                layer_count: Some(1),
-                            },
-                            swapchain_properties.extent.width as usize,
-                            swapchain_properties.extent.height as usize,
-                        )
-                        .context("Error creating depth buffer")?,
+                    )?)
+                },
+                swapchain_properties.image_count as usize,
+            )?;
+            let depth_buffers = TargetSpecificResources::new(
+                || {
+                    DedicatedLoadedImage::new(
+                        &mut device,
+                        adapter,
+                        swapchain_properties.depth_format,
+                        Usage::DEPTH_STENCIL_ATTACHMENT,
+                        SubresourceRange {
+                            aspects: Aspects::DEPTH,
+                            level_start: 0,
+                            level_count: Some(1),
+                            layer_start: 0,
+                            layer_count: Some(1),
+                        },
+                        swapchain_properties.extent.width as usize,
+                        swapchain_properties.extent.height as usize,
                     )
-                }
-            }
+                    .context("Error creating depth buffer")
+                },
+                swapchain_properties.image_count as usize,
+            )?;
 
             (draw_buffers, pipeline, framebuffers, depth_buffers)
         };
@@ -442,7 +438,6 @@ where
             _d: PhantomData,
             framebuffers,
             depth_buffers,
-            next_resources: 0,
         })
     }
 
diff --git a/stockton-render/src/draw/draw_passes/mod.rs b/stockton-render/src/draw/draw_passes/mod.rs
index 70f1786..1c99614 100644
--- a/stockton-render/src/draw/draw_passes/mod.rs
+++ b/stockton-render/src/draw/draw_passes/mod.rs
@@ -9,8 +9,9 @@ use anyhow::Result;
 
 mod cons;
 mod level;
+pub mod util;
 
-pub use cons::{ConsDrawPass, NilDrawPass};
+pub use cons::ConsDrawPass;
 pub use level::LevelDrawPass;
 
 /// One of several 'passes' that draw on each frame.
diff --git a/stockton-render/src/draw/draw_passes/util.rs b/stockton-render/src/draw/draw_passes/util.rs
new file mode 100644
index 0000000..a42f870
--- /dev/null
+++ b/stockton-render/src/draw/draw_passes/util.rs
@@ -0,0 +1,41 @@
+//! Utility structs & functions
+
+use anyhow::Result;
+
+/// Keeps a given resource for each swapchain image
+pub struct TargetSpecificResources<T> {
+    elements: Vec<T>,
+    next_idx: usize,
+}
+
+impl<T> TargetSpecificResources<T> {
+    /// Create a new set of resources, given a function to generate them and the count
+    /// In most cases, count should be swapchain_properties.image_count
+    pub fn new<F>(generator: F, count: usize) -> Result<Self>
+    where
+        F: FnMut() -> Result<T>,
+    {
+        let mut elements = Vec::with_capacity(count);
+        for _ in 0..count {
+            elements.push(generator()?);
+        }
+
+        Ok(TargetSpecificResources {
+            elements,
+            next_idx: 0,
+        })
+    }
+
+    /// Get the next resource, wrapping around if necessary.
+    pub fn get_next<'a>(&'a mut self) -> &'a T {
+        let el = &self.elements[self.next_idx];
+        self.next_idx = (self.next_idx + 1) % self.elements.len();
+        el
+    }
+
+    /// Dissolve the resource set, returning an iterator over each item.
+    /// In most cases, each item will need deactivated.
+    pub fn dissolve(self) -> impl Iterator<Item = T> {
+        self.elements.into_iter()
+    }
+}
diff --git a/stockton-render/src/draw/target.rs b/stockton-render/src/draw/target.rs
index de803aa..3861192 100644
--- a/stockton-render/src/draw/target.rs
+++ b/stockton-render/src/draw/target.rs
@@ -9,13 +9,16 @@ use std::{
 use hal::{
     command::CommandBufferFlags,
     format::{Aspects, ChannelType, Format, ImageFeature},
-    image::{Access, Extent, Layout, SubresourceRange, Usage as ImgUsage},
+    image::{
+        Access, Extent, FramebufferAttachment, Layout, SubresourceRange, Usage as ImgUsage,
+        ViewCapabilities,
+    },
     memory::{Barrier, Dependencies},
     pso::{PipelineStage, Viewport},
     window::{CompositeAlphaMode, Extent2D, PresentMode, SwapchainConfig},
 };
 
-use super::{draw_passes::DrawPass};
+use super::draw_passes::DrawPass;
 use crate::{error::EnvironmentError, types::*};
 use anyhow::{Context, Result};
 use stockton_types::Session;
@@ -107,6 +110,14 @@ impl SwapchainProperties {
             },
         })
     }
+
+    pub fn framebuffer_attachment(&self) -> FramebufferAttachment {
+        FramebufferAttachment {
+            usage: ImgUsage::COLOR_ATTACHMENT,
+            format: self.format,
+            view_caps: ViewCapabilities::empty(),
+        }
+    }
 }
 
 pub struct TargetChain {