From b0832072fc1b9a41f18078734ab0d0a6eda35b8c Mon Sep 17 00:00:00 2001
From: Andrew Hickman <andrew.hickman1@sky.com>
Date: Sat, 4 Aug 2018 13:38:33 +0100
Subject: [PATCH] Avoid allocating a DynamicState every frame (#1008)

* Avoid allocating a DynamicState every frame

* Don't mutate the DynamicState

* Undo DynamicState::dynamic_state doc change
---
 CHANGELOG.md                               |  1 +
 examples/src/bin/image.rs                  | 26 +++++++++++++--------
 examples/src/bin/runtime-shader.rs         |  2 +-
 examples/src/bin/teapot.rs                 | 26 +++++++++++++--------
 examples/src/bin/triangle.rs               | 27 ++++++++++++++--------
 vulkano/src/command_buffer/auto.rs         | 20 ++++++++--------
 vulkano/src/command_buffer/state_cacher.rs | 15 +++++++-----
 7 files changed, 72 insertions(+), 45 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 5053fd2c..f8ec691c 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -5,6 +5,7 @@
 - Allow custom implementations of `RenderPassDesc` to specify `VK_SUBPASS_EXTERNAL` as a dependency source or destination
 - Added `vulkano_win::create_vk_surface` which allows creating a surface safely without taking ownership of
   the window.
+- `AutoCommandBufferBuilder::draw` and friends no longer consume the `DynamicState` argument, allowing reuse between calls.
 
 # Version 0.9.0 (2018-03-13)
 
diff --git a/examples/src/bin/image.rs b/examples/src/bin/image.rs
index ed7e8629..7e0f8ce7 100644
--- a/examples/src/bin/image.rs
+++ b/examples/src/bin/image.rs
@@ -145,6 +145,16 @@ fn main() {
 
     let mut previous_frame_end = Box::new(tex_future) as Box<GpuFuture>;
 
+    let mut dynamic_state =  vulkano::command_buffer::DynamicState {
+        line_width: None,
+        viewports: Some(vec![vulkano::pipeline::viewport::Viewport {
+            origin: [0.0, 0.0],
+            dimensions: [dimensions[0] as f32, dimensions[1] as f32],
+            depth_range: 0.0 .. 1.0,
+        }]),
+        scissors: None,
+    };
+
     loop {
         previous_frame_end.cleanup_finished();
         if recreate_swapchain {
@@ -166,6 +176,12 @@ fn main() {
 
             framebuffers = None;
 
+            dynamic_state.viewports = Some(vec![vulkano::pipeline::viewport::Viewport {
+                origin: [0.0, 0.0],
+                dimensions: [dimensions[0] as f32, dimensions[1] as f32],
+                depth_range: 0.0 .. 1.0,
+            }]);
+
             recreate_swapchain = false;
         }
 
@@ -194,15 +210,7 @@ fn main() {
                 framebuffers.as_ref().unwrap()[image_num].clone(), false,
                 vec![[0.0, 0.0, 1.0, 1.0].into()]).unwrap()
             .draw(pipeline.clone(),
-                   vulkano::command_buffer::DynamicState {
-                      line_width: None,
-                      viewports: Some(vec![vulkano::pipeline::viewport::Viewport {
-                          origin: [0.0, 0.0],
-                          dimensions: [dimensions[0] as f32, dimensions[1] as f32],
-                          depth_range: 0.0 .. 1.0,
-                      }]),
-                      scissors: None,
-                  },
+                  &dynamic_state,
                   vertex_buffer.clone(),
                   set.clone(), ()).unwrap()
             .end_render_pass().unwrap()
diff --git a/examples/src/bin/runtime-shader.rs b/examples/src/bin/runtime-shader.rs
index 6663082c..99e9f4f5 100644
--- a/examples/src/bin/runtime-shader.rs
+++ b/examples/src/bin/runtime-shader.rs
@@ -448,7 +448,7 @@ fn main() {
             ).unwrap()
             .draw(
                 graphics_pipeline.clone(),
-                DynamicState::none(),
+                &DynamicState::none(),
                 vertex_buffer.clone(),
                 (),
                 (),
diff --git a/examples/src/bin/teapot.rs b/examples/src/bin/teapot.rs
index 5ccf9da4..3593e9a4 100644
--- a/examples/src/bin/teapot.rs
+++ b/examples/src/bin/teapot.rs
@@ -138,6 +138,16 @@ fn main() {
     let mut previous_frame = Box::new(vulkano::sync::now(device.clone())) as Box<GpuFuture>;
     let rotation_start = std::time::Instant::now();
 
+    let mut dynamic_state = vulkano::command_buffer::DynamicState {
+        line_width: None,
+        viewports: Some(vec![vulkano::pipeline::viewport::Viewport {
+            origin: [0.0, 0.0],
+            dimensions: [dimensions[0] as f32, dimensions[1] as f32],
+            depth_range: 0.0 .. 1.0,
+        }]),
+        scissors: None,
+    };
+
     loop {
         previous_frame.cleanup_finished();
 
@@ -165,6 +175,12 @@ fn main() {
 
             proj = cgmath::perspective(cgmath::Rad(std::f32::consts::FRAC_PI_2), { dimensions[0] as f32 / dimensions[1] as f32 }, 0.01, 100.0);
 
+            dynamic_state.viewports = Some(vec![vulkano::pipeline::viewport::Viewport {
+                origin: [0.0, 0.0],
+                dimensions: [dimensions[0] as f32, dimensions[1] as f32],
+                depth_range: 0.0 .. 1.0,
+            }]);
+
             recreate_swapchain = false;
         }
 
@@ -216,15 +232,7 @@ fn main() {
                 ]).unwrap()
             .draw_indexed(
                 pipeline.clone(),
-                vulkano::command_buffer::DynamicState {
-                      line_width: None,
-                      viewports: Some(vec![vulkano::pipeline::viewport::Viewport {
-                          origin: [0.0, 0.0],
-                          dimensions: [dimensions[0] as f32, dimensions[1] as f32],
-                          depth_range: 0.0 .. 1.0,
-                      }]),
-                      scissors: None,
-                },
+                &dynamic_state,
                 (vertex_buffer.clone(), normals_buffer.clone()), 
                 index_buffer.clone(), set.clone(), ()).unwrap()
             .end_render_pass().unwrap()
diff --git a/examples/src/bin/triangle.rs b/examples/src/bin/triangle.rs
index 19f39ed1..f7ffdda0 100644
--- a/examples/src/bin/triangle.rs
+++ b/examples/src/bin/triangle.rs
@@ -330,6 +330,16 @@ void main() {
     // that, we store the submission of the previous frame here.
     let mut previous_frame_end = Box::new(now(device.clone())) as Box<GpuFuture>;
 
+    let mut dynamic_state = DynamicState {
+        line_width: None,
+        viewports: Some(vec![Viewport {
+            origin: [0.0, 0.0],
+            dimensions: [dimensions[0] as f32, dimensions[1] as f32],
+            depth_range: 0.0 .. 1.0,
+        }]),
+        scissors: None,
+    };
+
     loop {
         // It is important to call this function from time to time, otherwise resources will keep
         // accumulating and you will eventually reach an out of memory error.
@@ -359,6 +369,12 @@ void main() {
 
             framebuffers = None;
 
+            dynamic_state.viewports = Some(vec![Viewport {
+                origin: [0.0, 0.0],
+                dimensions: [dimensions[0] as f32, dimensions[1] as f32],
+                depth_range: 0.0 .. 1.0,
+            }]);
+
             recreate_swapchain = false;
         }
 
@@ -416,16 +432,7 @@ void main() {
             // The last two parameters contain the list of resources to pass to the shaders.
             // Since we used an `EmptyPipeline` object, the objects have to be `()`.
             .draw(pipeline.clone(),
-                  DynamicState {
-                      line_width: None,
-                      // TODO: Find a way to do this without having to dynamically allocate a Vec every frame.
-                      viewports: Some(vec![Viewport {
-                          origin: [0.0, 0.0],
-                          dimensions: [dimensions[0] as f32, dimensions[1] as f32],
-                          depth_range: 0.0 .. 1.0,
-                      }]),
-                      scissors: None,
-                  },
+                  &dynamic_state,
                   vertex_buffer.clone(), (), ())
             .unwrap()
 
diff --git a/vulkano/src/command_buffer/auto.rs b/vulkano/src/command_buffer/auto.rs
index aae10c53..0d6ee929 100644
--- a/vulkano/src/command_buffer/auto.rs
+++ b/vulkano/src/command_buffer/auto.rs
@@ -981,7 +981,7 @@ impl<P> AutoCommandBufferBuilder<P> {
     }
 
     #[inline]
-    pub fn draw<V, Gp, S, Pc>(mut self, pipeline: Gp, dynamic: DynamicState, vertices: V, sets: S,
+    pub fn draw<V, Gp, S, Pc>(mut self, pipeline: Gp, dynamic: &DynamicState, vertices: V, sets: S,
                               constants: Pc)
                               -> Result<Self, DrawError>
         where Gp: GraphicsPipelineAbstract + VertexSource<V> + Send + Sync + 'static + Clone, // TODO: meh for Clone
@@ -991,7 +991,7 @@ impl<P> AutoCommandBufferBuilder<P> {
             // TODO: must check that pipeline is compatible with render pass
 
             self.ensure_inside_render_pass_inline(&pipeline)?;
-            check_dynamic_state_validity(&pipeline, &dynamic)?;
+            check_dynamic_state_validity(&pipeline, dynamic)?;
             check_push_constants_validity(&pipeline, &constants)?;
             check_descriptor_sets_validity(&pipeline, &sets)?;
             let vb_infos = check_vertex_buffers(&pipeline, vertices)?;
@@ -1005,7 +1005,7 @@ impl<P> AutoCommandBufferBuilder<P> {
             let dynamic = self.state_cacher.dynamic_state(dynamic);
 
             push_constants(&mut self.inner, pipeline.clone(), constants);
-            set_state(&mut self.inner, dynamic);
+            set_state(&mut self.inner, &dynamic);
             descriptor_sets(&mut self.inner,
                             &mut self.state_cacher,
                             true,
@@ -1026,7 +1026,7 @@ impl<P> AutoCommandBufferBuilder<P> {
     }
 
     #[inline]
-    pub fn draw_indexed<V, Gp, S, Pc, Ib, I>(mut self, pipeline: Gp, dynamic: DynamicState,
+    pub fn draw_indexed<V, Gp, S, Pc, Ib, I>(mut self, pipeline: Gp, dynamic: &DynamicState,
                                              vertices: V, index_buffer: Ib, sets: S, constants: Pc)
                                              -> Result<Self, DrawIndexedError>
         where Gp: GraphicsPipelineAbstract + VertexSource<V> + Send + Sync + 'static + Clone, // TODO: meh for Clone
@@ -1039,7 +1039,7 @@ impl<P> AutoCommandBufferBuilder<P> {
 
             self.ensure_inside_render_pass_inline(&pipeline)?;
             let ib_infos = check_index_buffer(self.device(), &index_buffer)?;
-            check_dynamic_state_validity(&pipeline, &dynamic)?;
+            check_dynamic_state_validity(&pipeline, dynamic)?;
             check_push_constants_validity(&pipeline, &constants)?;
             check_descriptor_sets_validity(&pipeline, &sets)?;
             let vb_infos = check_vertex_buffers(&pipeline, vertices)?;
@@ -1059,7 +1059,7 @@ impl<P> AutoCommandBufferBuilder<P> {
             let dynamic = self.state_cacher.dynamic_state(dynamic);
 
             push_constants(&mut self.inner, pipeline.clone(), constants);
-            set_state(&mut self.inner, dynamic);
+            set_state(&mut self.inner, &dynamic);
             descriptor_sets(&mut self.inner,
                             &mut self.state_cacher,
                             true,
@@ -1079,7 +1079,7 @@ impl<P> AutoCommandBufferBuilder<P> {
     }
 
     #[inline]
-    pub fn draw_indirect<V, Gp, S, Pc, Ib>(mut self, pipeline: Gp, dynamic: DynamicState,
+    pub fn draw_indirect<V, Gp, S, Pc, Ib>(mut self, pipeline: Gp, dynamic: &DynamicState,
                                            vertices: V, indirect_buffer: Ib, sets: S, constants: Pc)
                                            -> Result<Self, DrawIndirectError>
         where Gp: GraphicsPipelineAbstract + VertexSource<V> + Send + Sync + 'static + Clone, // TODO: meh for Clone
@@ -1094,7 +1094,7 @@ impl<P> AutoCommandBufferBuilder<P> {
             // TODO: must check that pipeline is compatible with render pass
 
             self.ensure_inside_render_pass_inline(&pipeline)?;
-            check_dynamic_state_validity(&pipeline, &dynamic)?;
+            check_dynamic_state_validity(&pipeline, dynamic)?;
             check_push_constants_validity(&pipeline, &constants)?;
             check_descriptor_sets_validity(&pipeline, &sets)?;
             let vb_infos = check_vertex_buffers(&pipeline, vertices)?;
@@ -1110,7 +1110,7 @@ impl<P> AutoCommandBufferBuilder<P> {
             let dynamic = self.state_cacher.dynamic_state(dynamic);
 
             push_constants(&mut self.inner, pipeline.clone(), constants);
-            set_state(&mut self.inner, dynamic);
+            set_state(&mut self.inner, &dynamic);
             descriptor_sets(&mut self.inner,
                             &mut self.state_cacher,
                             true,
@@ -1304,7 +1304,7 @@ unsafe fn push_constants<P, Pl, Pc>(destination: &mut SyncCommandBufferBuilder<P
 }
 
 // Shortcut function to change the state of the pipeline.
-unsafe fn set_state<P>(destination: &mut SyncCommandBufferBuilder<P>, dynamic: DynamicState) {
+unsafe fn set_state<P>(destination: &mut SyncCommandBufferBuilder<P>, dynamic: &DynamicState) {
     if let Some(line_width) = dynamic.line_width {
         destination.set_line_width(line_width);
     }
diff --git a/vulkano/src/command_buffer/state_cacher.rs b/vulkano/src/command_buffer/state_cacher.rs
index af6b6f94..0bc676ae 100644
--- a/vulkano/src/command_buffer/state_cacher.rs
+++ b/vulkano/src/command_buffer/state_cacher.rs
@@ -91,13 +91,16 @@ impl StateCacher {
     ///
     /// This function also updates the state cacher. The state cacher assumes that the state
     /// changes are going to be performed after this function returns.
-    pub fn dynamic_state(&mut self, mut incoming: DynamicState) -> DynamicState {
+    pub fn dynamic_state(&mut self, incoming: &DynamicState) -> DynamicState {
+        let mut changed = DynamicState::none();
+
         macro_rules! cmp {
             ($field:ident) => (
-                if self.dynamic_state.$field == incoming.$field {
-                    incoming.$field = None;
-                } else if incoming.$field.is_some() {
-                    self.dynamic_state.$field = incoming.$field.clone();
+                if self.dynamic_state.$field != incoming.$field {
+                    changed.$field = incoming.$field.clone();
+                    if incoming.$field.is_some() {
+                        self.dynamic_state.$field = incoming.$field.clone();
+                    }
                 }
             );
         }
@@ -106,7 +109,7 @@ impl StateCacher {
         cmp!(viewports);
         cmp!(scissors);
 
-        incoming
+        changed
     }
 
     /// Starts the process of comparing a list of descriptor sets to the descriptor sets currently