Merge #2185

2185: Various Metal performance optimizations r=grovesNL a=kvark

Helps #2161 . I'm now getting 80-85 fps on the test run.
Aside from Metal, also changes HAL to avoid heap allocation for vertex buffer binding.
PR checklist:
- [x] `make` succeeds (on *nix)
- [x] `make reftests` succeeds
- [x] tested examples with the following backends: metal


Co-authored-by: Dzmitry Malyshau <kvarkus@gmail.com>

Author: bors[bot]

Makefile

@@ -36,7 +36,7 @@ else
 endif
 
 
-.PHONY: all check test reftests travis-sdl2
+.PHONY: all check quad test reftests travis-sdl2
 
 all: check test
 
@@ -62,6 +62,9 @@ reftests-ci:
 	cd src/warden && cargo test --features "gl"
 	cd src/warden && cargo run --features "gl" -- ci #TODO: "gl-headless"
 
+quad:
+	cd examples && cargo run --bin quad --features ${FEATURES_HAL}
+
 travis-sdl2:
 	#TODO
 	#if [ -e $(SDL2_CONFIG) ]; then exit 1; fi

examples/quad/main.rs

@@ -30,8 +30,8 @@ use hal::pso::{PipelineStage, ShaderStageFlags, Specialization};
 use hal::queue::Submission;
 
 use std::fs;
-use std::io::Cursor;
-use std::io::Read;
+use std::io::{Cursor, Read};
+
 
 const ENTRY_NAME: &str = "main";
 
@@ -480,7 +480,7 @@ fn main() {
             cmd_buffer.set_viewports(0, &[viewport.clone()]);
             cmd_buffer.set_scissors(0, &[viewport.rect]);
             cmd_buffer.bind_graphics_pipeline(&pipeline);
-            cmd_buffer.bind_vertex_buffers(0, pso::VertexBufferSet(vec![(&vertex_buffer, 0)]));
+            cmd_buffer.bind_vertex_buffers(0, Some((&vertex_buffer, 0)));
             cmd_buffer.bind_graphics_descriptor_sets(&pipeline_layout, 0, Some(&desc_set), &[]); //TODO
 
             {

src/backend/dx11/src/lib.rs

@@ -866,9 +866,14 @@ impl hal::command::RawCommandBuffer<Backend> for CommandBuffer {
         }
     }
 
-    fn bind_vertex_buffers(&mut self, first_binding: u32, vbs: pso::VertexBufferSet<Backend>) {
-        let (buffers, offsets): (Vec<*mut d3d11::ID3D11Buffer>, Vec<u32>) = vbs.0.iter()
-            .map(|(buf, offset)| (buf.internal.raw, *offset as u32))
+    fn bind_vertex_buffers<I, T>(&mut self, first_binding: u32, buffers: I)
+    where
+        I: IntoIterator<Item = (T, buffer::Offset)>,
+        T: Borrow<Buffer>,
+    {
+        let (buffers, offsets): (Vec<*mut d3d11::ID3D11Buffer>, Vec<u32>) = buffers
+            .into_iter()
+            .map(|(buf, offset)| (buf.borrow().internal.raw, offset as u32))
             .unzip();
 
         // TODO: strides

src/backend/dx12/src/command.rs

@@ -1634,15 +1634,21 @@ impl com::RawCommandBuffer<Backend> for CommandBuffer {
         }
     }
 
-    fn bind_vertex_buffers(&mut self, first_binding: u32, vbs: pso::VertexBufferSet<Backend>) {
+    fn bind_vertex_buffers<I, T>(&mut self, first_binding: u32, buffers: I)
+    where
+        I: IntoIterator<Item = (T, buffer::Offset)>,
+        T: Borrow<n::Buffer>,
+    {
         // Only cache the vertex buffer views as we don't know the stride (PSO).
         assert!(first_binding as usize <= MAX_VERTEX_BUFFERS);
-        for (&(buffer, offset), view) in vbs.0.iter()
+        for ((buffer, offset), view) in buffers
+            .into_iter()
             .zip(self.vertex_buffer_views[first_binding as _..].iter_mut())
         {
-            let base = unsafe { (*buffer.resource).GetGPUVirtualAddress() };
-            view.BufferLocation = base + offset as u64;
-            view.SizeInBytes = buffer.size_in_bytes - offset as u32;
+            let b = buffer.borrow();
+            let base = unsafe { (*b.resource).GetGPUVirtualAddress() };
+            view.BufferLocation = base + offset;
+            view.SizeInBytes = b.size_in_bytes - offset as u32;
         }
     }
 

src/backend/empty/src/lib.rs

@@ -499,7 +499,11 @@ impl command::RawCommandBuffer<Backend> for RawCommandBuffer {
         unimplemented!()
     }
 
-    fn bind_vertex_buffers(&mut self, _: u32, _: pso::VertexBufferSet<Backend>) {
+    fn bind_vertex_buffers<I, T>(&mut self, _: u32, _: I)
+    where
+        I: IntoIterator<Item = (T, buffer::Offset)>,
+        T: Borrow<()>,
+    {
         unimplemented!()
     }
 

src/backend/gl/src/command.rs

@@ -698,17 +698,20 @@ impl command::RawCommandBuffer<Backend> for RawCommandBuffer {
         self.push_cmd(Command::BindIndexBuffer(ibv.buffer.raw));
     }
 
-    fn bind_vertex_buffers(&mut self, _first_binding: u32, vbs: hal::pso::VertexBufferSet<Backend>) {
-        if vbs.0.len() == 0 {
-            return
-        }
-
-        let needed_length = vbs.0.iter().map(|vb| vb.1).max().unwrap() + 1;
-
-        self.cache.vertex_buffers.resize(needed_length as usize, 0);
-
-        for vb in vbs.0 {
-            self.cache.vertex_buffers[vb.1 as usize] = vb.0.raw;
+    fn bind_vertex_buffers<I, T>(&mut self, first_binding: u32, buffers: I)
+    where
+        I: IntoIterator<Item = (T, buffer::Offset)>,
+        T: Borrow<n::Buffer>,
+    {
+        for (i, (buffer, offset)) in buffers.into_iter().enumerate() {
+            let index = first_binding as usize + i;
+            if self.cache.vertex_buffers.len() <= index {
+                self.cache.vertex_buffers.resize(index+1, 0);
+            }
+            self.cache.vertex_buffers[index] = buffer.borrow().raw;
+            if offset != 0 {
+                error!("Vertex buffer offset {} is not supported", offset);
+            }
         }
     }
 

src/backend/metal/src/command.rs

@@ -821,11 +821,11 @@ impl CommandSink {
     ) where
         I: Iterator<Item = soft::RenderCommand<&'a soft::Own>>,
     {
+        //assert!(AutoReleasePool::is_active());
         self.stop_encoding();
 
         match *self {
             CommandSink::Immediate { ref cmd_buffer, ref mut encoder_state, .. } => {
-                let _ap = AutoreleasePool::new();
                 let encoder = cmd_buffer.new_render_command_encoder(descriptor);
                 for command in init_commands {
                     exec_render(encoder, command);
@@ -1446,7 +1446,7 @@ impl pool::RawCommandPool<Backend> for CommandPool {
                 framebuffer_inner: native::FramebufferInner {
                     extent: Extent::default(),
                     aspects: Aspects::empty(),
-                    colors: Vec::new(),
+                    colors: SmallVec::new(),
                     depth_stencil: None,
                 }
             },
@@ -1653,6 +1653,8 @@ impl com::RawCommandBuffer<Backend> for CommandBuffer {
         T: IntoIterator,
         T::Item: Borrow<SubresourceRange>,
     {
+        let _ap = AutoreleasePool::new();
+
         let CommandBufferInner {
             ref mut retained_textures,
             ref mut sink,
@@ -1690,51 +1692,63 @@ impl com::RawCommandBuffer<Backend> for CommandBuffer {
                 &*image.raw
             };
 
-            let clear_color_attachment = sub.aspects.contains(Aspects::COLOR);
-            if image.format_desc.aspects.contains(Aspects::COLOR) {
+            let color_attachment = if image.format_desc.aspects.contains(Aspects::COLOR) {
                 let attachment = descriptor
                     .color_attachments()
                     .object_at(0)
                     .unwrap();
                 attachment.set_texture(Some(texture));
                 attachment.set_store_action(metal::MTLStoreAction::Store);
-                if clear_color_attachment {
+                if sub.aspects.contains(Aspects::COLOR) {
                     attachment.set_load_action(metal::MTLLoadAction::Clear);
                     attachment.set_clear_color(clear_color.clone());
+                    Some(attachment)
                 } else {
                     attachment.set_load_action(metal::MTLLoadAction::Load);
+                    None
                 }
-            }
+            } else {
+                assert!(!sub.aspects.contains(Aspects::COLOR));
+                None
+            };
 
-            let clear_depth_attachment = sub.aspects.contains(Aspects::DEPTH);
-            if image.format_desc.aspects.contains(Aspects::DEPTH) {
+            let depth_attachment = if image.format_desc.aspects.contains(Aspects::DEPTH) {
                 let attachment = descriptor
                     .depth_attachment()
                     .unwrap();
                 attachment.set_texture(Some(texture));
                 attachment.set_store_action(metal::MTLStoreAction::Store);
-                if clear_depth_attachment {
+                if sub.aspects.contains(Aspects::DEPTH) {
                     attachment.set_load_action(metal::MTLLoadAction::Clear);
                     attachment.set_clear_depth(depth_stencil.depth as _);
+                    Some(attachment)
                 } else {
                     attachment.set_load_action(metal::MTLLoadAction::Load);
+                    None
                 }
-            }
+            } else {
+                assert!(!sub.aspects.contains(Aspects::DEPTH));
+                None
+            };
 
-            let clear_stencil_attachment = sub.aspects.contains(Aspects::STENCIL);
-            if image.format_desc.aspects.contains(Aspects::STENCIL) {
+            let stencil_attachment = if image.format_desc.aspects.contains(Aspects::STENCIL) {
                 let attachment = descriptor
                     .stencil_attachment()
                     .unwrap();
                 attachment.set_texture(Some(texture));
                 attachment.set_store_action(metal::MTLStoreAction::Store);
-                if clear_stencil_attachment {
+                if sub.aspects.contains(Aspects::STENCIL) {
                     attachment.set_load_action(metal::MTLLoadAction::Clear);
                     attachment.set_clear_stencil(depth_stencil.stencil);
+                    Some(attachment)
                 } else {
                     attachment.set_load_action(metal::MTLLoadAction::Load);
+                    None
                 }
-            }
+            } else {
+                assert!(!sub.aspects.contains(Aspects::STENCIL));
+                None
+            };
 
             for layer in layers {
                 for level in sub.levels.clone() {
@@ -1746,29 +1760,19 @@ impl com::RawCommandBuffer<Backend> for CommandBuffer {
                         descriptor.set_render_target_array_length(num_layers);
                     };
 
-                    if clear_color_attachment {
-                        let attachment = descriptor
-                            .color_attachments()
-                            .object_at(0)
-                            .unwrap();
+                    if let Some(attachment) = color_attachment {
                         attachment.set_level(level as _);
                         if !CLEAR_IMAGE_ARRAY {
                             attachment.set_slice(layer as _);
                         }
                     }
-                    if clear_depth_attachment {
-                        let attachment = descriptor
-                            .depth_attachment()
-                            .unwrap();
+                    if let Some(attachment) = depth_attachment {
                         attachment.set_level(level as _);
                         if !CLEAR_IMAGE_ARRAY {
                             attachment.set_slice(layer as _);
                         }
                     }
-                    if clear_stencil_attachment {
-                        let attachment = descriptor
-                            .stencil_attachment()
-                            .unwrap();
+                    if let Some(attachment) = stencil_attachment {
                         attachment.set_level(level as _);
                         if !CLEAR_IMAGE_ARRAY {
                             attachment.set_slice(layer as _);
@@ -2015,6 +2019,8 @@ impl com::RawCommandBuffer<Backend> for CommandBuffer {
         T: IntoIterator,
         T::Item: Borrow<com::ImageBlit>
     {
+        let _ap = AutoreleasePool::new();
+
         let vertices = &mut self.temp.blit_vertices;
         vertices.clear();
 
@@ -2218,7 +2224,9 @@ impl com::RawCommandBuffer<Backend> for CommandBuffer {
                 .chain(&extra)
                 .cloned();
 
-            inner.sink().begin_render_pass(false, &descriptor, commands);
+            inner
+                .sink()
+                .begin_render_pass(false, &descriptor, commands);
         }
     }
 
@@ -2233,13 +2241,26 @@ impl com::RawCommandBuffer<Backend> for CommandBuffer {
         });
     }
 
-    fn bind_vertex_buffers(&mut self, first_binding: u32, buffer_set: pso::VertexBufferSet<Backend>) {
-        while self.state.vertex_buffers.len() < first_binding as usize + buffer_set.0.len() {
-            self.state.vertex_buffers.push(None);
-        }
-        for (i, &(buffer, offset)) in buffer_set.0.iter().enumerate() {
-            let buffer_ptr = BufferPtr(buffer.raw.as_ptr());
-            self.state.vertex_buffers[first_binding as usize + i] = Some((buffer_ptr, buffer.range.start + offset));
+
+    fn bind_vertex_buffers<I, T>(&mut self, first_binding: u32, buffers: I)
+    where
+        I: IntoIterator<Item = (T, buffer::Offset)>,
+        T: Borrow<native::Buffer>,
+    {
+        if self.state.vertex_buffers.len() <= first_binding as usize {
+            self.state.vertex_buffers.resize(first_binding as usize + 1, None);
+        }
+        for (i, (buffer, offset)) in buffers.into_iter().enumerate() {
+            let b = buffer.borrow();
+            let buffer_ptr = BufferPtr(b.raw.as_ptr());
+            let index = first_binding as usize + i;
+            let value = Some((buffer_ptr, b.range.start + offset));
+            if index >= self.state.vertex_buffers.len() {
+                debug_assert_eq!(index, self.state.vertex_buffers.len());
+                self.state.vertex_buffers.push(value);
+            } else {
+                self.state.vertex_buffers[index] = value;
+            }
         }
 
         let mask = self.state.set_vertex_buffers();