Merge #121

121: Descriptor iterator r=msiglreith a=kvark

This PR reduces the amount of magic going on during the descriptor writes, makes us move less stuff on the stack.

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

Cargo.lock

@@ -278,14 +278,14 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
 [[package]]
 name = "gfx-backend-dx11"
 version = "0.1.0"
-source = "git+https://github.com/gfx-rs/gfx#514973f6a2f15276adeb896cdc098b74bc1d4a2f"
+source = "git+https://github.com/gfx-rs/gfx#172a851bafa18d46887d2f3ae833cbfd56e192db"
 dependencies = [
  "bitflags 1.0.3 (registry+https://github.com/rust-lang/crates.io-index)",
  "derivative 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "gfx-hal 0.1.0 (git+https://github.com/gfx-rs/gfx)",
  "log 0.4.3 (registry+https://github.com/rust-lang/crates.io-index)",
  "smallvec 0.6.3 (registry+https://github.com/rust-lang/crates.io-index)",
- "spirv_cross 0.9.6 (registry+https://github.com/rust-lang/crates.io-index)",
+ "spirv_cross 0.9.7 (registry+https://github.com/rust-lang/crates.io-index)",
  "winapi 0.3.5 (registry+https://github.com/rust-lang/crates.io-index)",
  "winit 0.16.2 (registry+https://github.com/rust-lang/crates.io-index)",
  "wio 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -294,14 +294,14 @@ dependencies = [
 [[package]]
 name = "gfx-backend-dx12"
 version = "0.1.0"
-source = "git+https://github.com/gfx-rs/gfx#514973f6a2f15276adeb896cdc098b74bc1d4a2f"
+source = "git+https://github.com/gfx-rs/gfx#172a851bafa18d46887d2f3ae833cbfd56e192db"
 dependencies = [
  "bitflags 1.0.3 (registry+https://github.com/rust-lang/crates.io-index)",
  "derivative 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "gfx-hal 0.1.0 (git+https://github.com/gfx-rs/gfx)",
  "log 0.4.3 (registry+https://github.com/rust-lang/crates.io-index)",
  "smallvec 0.6.3 (registry+https://github.com/rust-lang/crates.io-index)",
- "spirv_cross 0.9.6 (registry+https://github.com/rust-lang/crates.io-index)",
+ "spirv_cross 0.9.7 (registry+https://github.com/rust-lang/crates.io-index)",
  "winapi 0.3.5 (registry+https://github.com/rust-lang/crates.io-index)",
  "winit 0.16.2 (registry+https://github.com/rust-lang/crates.io-index)",
  "wio 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -310,12 +310,11 @@ dependencies = [
 [[package]]
 name = "gfx-backend-metal"
 version = "0.1.0"
-source = "git+https://github.com/gfx-rs/gfx#514973f6a2f15276adeb896cdc098b74bc1d4a2f"
+source = "git+https://github.com/gfx-rs/gfx#172a851bafa18d46887d2f3ae833cbfd56e192db"
 dependencies = [
  "bitflags 1.0.3 (registry+https://github.com/rust-lang/crates.io-index)",
  "block 0.1.6 (registry+https://github.com/rust-lang/crates.io-index)",
  "cocoa 0.15.0 (registry+https://github.com/rust-lang/crates.io-index)",
- "core-foundation 0.6.1 (registry+https://github.com/rust-lang/crates.io-index)",
  "core-graphics 0.14.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "dispatch 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)",
  "foreign-types 0.3.2 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -325,7 +324,7 @@ dependencies = [
  "objc 0.2.5 (registry+https://github.com/rust-lang/crates.io-index)",
  "parking_lot 0.6.3 (registry+https://github.com/rust-lang/crates.io-index)",
  "smallvec 0.6.3 (registry+https://github.com/rust-lang/crates.io-index)",
- "spirv_cross 0.9.6 (registry+https://github.com/rust-lang/crates.io-index)",
+ "spirv_cross 0.9.7 (registry+https://github.com/rust-lang/crates.io-index)",
  "storage-map 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)",
  "winit 0.16.2 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
@@ -333,7 +332,7 @@ dependencies = [
 [[package]]
 name = "gfx-backend-vulkan"
 version = "0.1.0"
-source = "git+https://github.com/gfx-rs/gfx#514973f6a2f15276adeb896cdc098b74bc1d4a2f"
+source = "git+https://github.com/gfx-rs/gfx#172a851bafa18d46887d2f3ae833cbfd56e192db"
 dependencies = [
  "ash 0.24.4 (registry+https://github.com/rust-lang/crates.io-index)",
  "byteorder 1.2.3 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -350,7 +349,7 @@ dependencies = [
 [[package]]
 name = "gfx-hal"
 version = "0.1.0"
-source = "git+https://github.com/gfx-rs/gfx#514973f6a2f15276adeb896cdc098b74bc1d4a2f"
+source = "git+https://github.com/gfx-rs/gfx#172a851bafa18d46887d2f3ae833cbfd56e192db"
 dependencies = [
  "bitflags 1.0.3 (registry+https://github.com/rust-lang/crates.io-index)",
  "failure 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -614,7 +613,6 @@ dependencies = [
  "lazy_static 1.0.2 (registry+https://github.com/rust-lang/crates.io-index)",
  "log 0.4.3 (registry+https://github.com/rust-lang/crates.io-index)",
  "renderdoc 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)",
- "smallvec 0.6.3 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
 
 [[package]]
@@ -761,7 +759,7 @@ dependencies = [
 
 [[package]]
 name = "spirv_cross"
-version = "0.9.6"
+version = "0.9.7"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 dependencies = [
  "cc 1.0.18 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -1198,7 +1196,7 @@ dependencies = [
 "checksum shared_library 0.1.9 (registry+https://github.com/rust-lang/crates.io-index)" = "5a9e7e0f2bfae24d8a5b5a66c5b257a83c7412304311512a0c054cd5e619da11"
 "checksum smallvec 0.6.3 (registry+https://github.com/rust-lang/crates.io-index)" = "26df3bb03ca5eac2e64192b723d51f56c1b1e0860e7c766281f4598f181acdc8"
 "checksum smithay-client-toolkit 0.2.6 (registry+https://github.com/rust-lang/crates.io-index)" = "2051bffc6cbf271176e8ba1527f801b6444567daee15951ff5152aaaf7777b2f"
-"checksum spirv_cross 0.9.6 (registry+https://github.com/rust-lang/crates.io-index)" = "73a62edb99e26999160cf3dcdf9c181b8ba01825f7e016f9b9e70f7000d28548"
+"checksum spirv_cross 0.9.7 (registry+https://github.com/rust-lang/crates.io-index)" = "0d0c037ff60c5e8a31ee19b784dc51344663368f3879f6e30f461f686f96ba83"
 "checksum stable_deref_trait 1.1.0 (registry+https://github.com/rust-lang/crates.io-index)" = "ffbc596e092fe5f598b12ef46cc03754085ac2f4d8c739ad61c4ae266cc3b3fa"
 "checksum storage-map 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)" = "16bc49017309bb047d22700c91811dca0b8c13561f94e3d8784508edeb91b00a"
 "checksum syn 0.10.8 (registry+https://github.com/rust-lang/crates.io-index)" = "58fd09df59565db3399efbba34ba8a2fec1307511ebd245d0061ff9d42691673"

libportability-gfx/Cargo.toml

@@ -19,7 +19,6 @@ metal-capture = ["gfx-backend-metal/auto-capture"]
 [dependencies]
 lazy_static = "1.0"
 log = { version = "0.4", features = ["release_max_level_error"] }
-smallvec = "0.6"
 
 [dependencies.env_logger]
 version = "0.5"

libportability-gfx/src/impls.rs

@@ -9,8 +9,6 @@ use hal::device::WaitFor;
 use hal::pool::RawCommandPool;
 use hal::queue::RawCommandQueue;
 
-use smallvec::SmallVec;
-
 use std::ffi::{CStr, CString};
 use std::os::raw::c_int;
 #[cfg(feature = "renderdoc")]
@@ -2341,6 +2339,7 @@ pub extern "C" fn gfxCreateDescriptorPool(
     pDescriptorPool: *mut VkDescriptorPool,
 ) -> VkResult {
     let info = unsafe { &*pCreateInfo };
+    let max_sets = info.maxSets as usize;
 
     let pool_sizes = unsafe {
         slice::from_raw_parts(info.pPoolSizes, info.poolSizeCount as _)
@@ -2357,12 +2356,12 @@ pub extern "C" fn gfxCreateDescriptorPool(
 
     let pool = super::DescriptorPool {
         raw: gpu.device
-            .create_descriptor_pool(info.maxSets as _, ranges),
+            .create_descriptor_pool(max_sets, ranges),
-        temp_sets: Vec::with_capacity(info.maxSets as _),
+        temp_sets: Vec::with_capacity(max_sets),
         set_handles: if info.flags & VkDescriptorPoolCreateFlagBits::VK_DESCRIPTOR_POOL_CREATE_FREE_DESCRIPTOR_SET_BIT as u32 != 0 {
             None
         } else {
-            Some(Vec::new())
+            Some(Vec::with_capacity(max_sets))
         },
     };
 
@@ -2417,7 +2416,6 @@ pub extern "C" fn gfxAllocateDescriptorSets(
         .iter()
         .map(|layout| &**layout);
 
-    assert!(temp_sets.is_empty());
     match raw.allocate_sets(layouts, temp_sets) {
         Ok(()) => {
             assert_eq!(temp_sets.len(), info.descriptorSetCount as usize);
@@ -2464,6 +2462,72 @@ pub extern "C" fn gfxFreeDescriptorSets(
 
     VkResult::VK_SUCCESS
 }
+
+struct DescriptorIter<'a> {
+    ty: pso::DescriptorType,
+    image_infos: slice::Iter<'a, VkDescriptorImageInfo>,
+    buffer_infos: slice::Iter<'a, VkDescriptorBufferInfo>,
+    texel_buffer_views: slice::Iter<'a, VkBufferView>,
+}
+impl<'a> Iterator for DescriptorIter<'a> {
+    type Item = pso::Descriptor<'a, B>;
+    fn next(&mut self) -> Option<Self::Item> {
+        match self.ty {
+            pso::DescriptorType::Sampler => {
+                self.image_infos.next().map(|image| {
+                    pso::Descriptor::Sampler(&*image.sampler)
+                })
+            }
+            pso::DescriptorType::InputAttachment |
+            pso::DescriptorType::SampledImage |
+            pso::DescriptorType::StorageImage => {
+                self.image_infos.next().map(|image| {
+                    pso::Descriptor::Image(
+                        &*image.imageView,
+                        conv::map_image_layout(image.imageLayout),
+                    )
+                })
+            }
+            pso::DescriptorType::UniformTexelBuffer => {
+                self.texel_buffer_views.next().map(|view| {
+                    pso::Descriptor::UniformTexelBuffer(&**view)
+                })
+            }
+            pso::DescriptorType::StorageTexelBuffer => {
+                self.texel_buffer_views.next().map(|view| {
+                    pso::Descriptor::StorageTexelBuffer(&**view)
+                })
+            }
+            pso::DescriptorType::UniformBuffer |
+            pso::DescriptorType::StorageBuffer |
+            pso::DescriptorType::UniformBufferDynamic |
+            pso::DescriptorType::StorageBufferDynamic => {
+                self.buffer_infos.next().map(|buffer| {
+                    let end = if buffer.range as i32 == VK_WHOLE_SIZE {
+                        None
+                    } else {
+                        Some(buffer.offset + buffer.range)
+                    };
+                    pso::Descriptor::Buffer(
+                        // Non-sparse buffer need to be bound to device memory.
+                        buffer.buffer.expect("Buffer needs to be bound"),
+                        Some(buffer.offset) .. end,
+                    )
+                })
+            }
+            pso::DescriptorType::CombinedImageSampler => {
+                self.image_infos.next().map(|image| {
+                    pso::Descriptor::CombinedImageSampler(
+                        &*image.imageView,
+                        conv::map_image_layout(image.imageLayout),
+                        &*image.sampler,
+                    )
+                })
+            }
+        }
+    }
+}
+
 #[inline]
 pub extern "C" fn gfxUpdateDescriptorSets(
     gpu: VkDevice,
@@ -2476,83 +2540,18 @@ pub extern "C" fn gfxUpdateDescriptorSets(
         slice::from_raw_parts(pDescriptorWrites, descriptorWriteCount as _)
     };
     let writes = write_infos.iter().map(|write| {
-        let image_info = unsafe {
+        let descriptors = DescriptorIter {
-            slice::from_raw_parts(write.pImageInfo, write.descriptorCount as _)
+            ty: conv::map_descriptor_type(write.descriptorType),
+            image_infos: unsafe {
+                slice::from_raw_parts(write.pImageInfo, write.descriptorCount as _)
+            }.iter(),
+            buffer_infos: unsafe {
+                slice::from_raw_parts(write.pBufferInfo, write.descriptorCount as _)
+            }.iter(),
+            texel_buffer_views: unsafe {
+                slice::from_raw_parts(write.pTexelBufferView, write.descriptorCount as _)
+            }.iter()
         };
-        let buffer_info = unsafe {
-            slice::from_raw_parts(write.pBufferInfo, write.descriptorCount as _)
-        };
-        let texel_buffer_views = unsafe {
-            slice::from_raw_parts(write.pTexelBufferView, write.descriptorCount as _)
-        };
-
-        let ty = conv::map_descriptor_type(write.descriptorType);
-        let descriptors: SmallVec<[_; 4]> = match ty {
-            pso::DescriptorType::Sampler => {
-                image_info
-                    .into_iter()
-                    .map(|image| pso::Descriptor::Sampler(&*image.sampler))
-                    .collect()
-            }
-            pso::DescriptorType::InputAttachment |
-            pso::DescriptorType::SampledImage |
-            pso::DescriptorType::StorageImage => {
-                image_info
-                    .into_iter()
-                    .map(|image| pso::Descriptor::Image(
-                        &*image.imageView,
-                        conv::map_image_layout(image.imageLayout),
-                    ))
-                    .collect()
-            }
-            pso::DescriptorType::UniformTexelBuffer => {
-                texel_buffer_views
-                    .into_iter()
-                    .map(|view| pso::Descriptor::UniformTexelBuffer(
-                        &**view,
-                    ))
-                    .collect()
-            }
-            pso::DescriptorType::StorageTexelBuffer => {
-                texel_buffer_views
-                    .into_iter()
-                    .map(|view| pso::Descriptor::StorageTexelBuffer(
-                        &**view,
-                    ))
-                    .collect()
-            }
-            pso::DescriptorType::UniformBuffer |
-            pso::DescriptorType::StorageBuffer |
-            pso::DescriptorType::UniformBufferDynamic |
-            pso::DescriptorType::StorageBufferDynamic => {
-                buffer_info
-                    .into_iter()
-                    .map(|buffer| {
-                        let end = if buffer.range as i32 == VK_WHOLE_SIZE {
-                            None
-                        } else {
-                            Some(buffer.offset + buffer.range)
-                        };
-                        pso::Descriptor::Buffer(
-                            // Non-sparse buffer need to be bound to device memory.
-                            buffer.buffer.expect("Buffer needs to be bound"),
-                            Some(buffer.offset) .. end,
-                        )
-                    })
-                    .collect()
-            }
-            pso::DescriptorType::CombinedImageSampler => {
-                image_info
-                    .into_iter()
-                    .map(|image| pso::Descriptor::CombinedImageSampler(
-                        &*image.imageView,
-                        conv::map_image_layout(image.imageLayout),
-                        &*image.sampler,
-                    ))
-                    .collect()
-            }
-        };
-
         pso::DescriptorSetWrite {
             set: &*write.dstSet,
             binding: write.dstBinding as _,

libportability-gfx/src/lib.rs

@@ -18,7 +18,6 @@ extern crate gfx_backend_vulkan as back;
 extern crate lazy_static;
 #[macro_use]
 extern crate log;
-extern crate smallvec;
 #[cfg(feature = "env_logger")]
 extern crate env_logger;
 #[cfg(feature = "renderdoc")]