Fixes NotAPenguin0#70 and NotAPenguin0#61

src/allocator/scratch_allocator.rs

@@ -71,6 +71,7 @@ pub struct ScratchAllocator<A: Allocator = DefaultAllocator> {
     allocator: A,
     buffers: Vec<Buffer<A>>,
     current_buffer: usize,
+    largest_block_size: vk::DeviceSize,
     local_offset: vk::DeviceSize,
     chunk_size: vk::DeviceSize,
     alignment: vk::DeviceSize,
@@ -126,6 +127,10 @@ impl<A: Allocator> ScratchAllocator<A> {
             local_offset: 0,
             chunk_size,
             current_buffer: 0,
+
+            // we already pre-allocate a block so this cannot be 0 
+            largest_block_size: chunk_size,
+
             alignment,
             device,
             allocator: allocator.clone(),
@@ -165,6 +170,11 @@ impl<A: Allocator> ScratchAllocator<A> {
             let whole_buffer_size = size.max(self.chunk_size);
             let whole_buffer_size = ((whole_buffer_size as f32) / (self.alignment as f32)).ceil() as u64 * self.alignment;
 
+            // Per #70, allocate a block twice the size the largest current block (if possible)
+            // In case the allocation is bigger than the twice the largest block size, simply use the largest one
+            let whole_buffer_size = (self.largest_block_size * 2).max(whole_buffer_size);
+            self.largest_block_size = self.largest_block_size.max(whole_buffer_size);
+
             // Create a new chunked buffer with the chunk size 
             let buffer = Buffer::new(self.device.clone(), &mut self.allocator, whole_buffer_size, MemoryType::CpuToGpu)?;
             if !buffer.is_mapped() {
@@ -213,29 +223,19 @@ impl<A: Allocator> ScratchAllocator<A> {
     ///     Ok(())
     /// }
     /// ```
-    pub unsafe fn reset(&mut self, compressed_sized: Option<vk::DeviceSize>) -> Result<()> {
+    pub unsafe fn reset(&mut self) -> Result<()> {
         // Compressed size after reset when we have multiple buffers
         if self.buffers.len() > 1 {
-            let compressed_size = compressed_sized.map(|size| {
-                ((size as f32) / (self.alignment as f32)).ceil() as u64 * self.alignment
-            }).unwrap_or_else(|| {
-                // We know that the sizes of the buffers is always aligned, so we shouldn't need to re-align
-                self.buffers.iter().map(|buf| buf.size()).sum()
-            });
-
-            // Create a new buffer that should contain *all* allocations during the frame 
-            let buffer = Buffer::new(self.device.clone(), &mut self.allocator, compressed_size, MemoryType::CpuToGpu)?;
-            if !buffer.is_mapped() {
-                anyhow::bail!(Error::UnmappableBuffer);
-            }
-
+            // Find the largest block (could optimize this by storing index instead)
+            let index = self.buffers.iter().position(|buffer| buffer.size() == self.largest_block_size).unwrap();
+            let buffer = self.buffers.remove(index);
             self.buffers.clear();
             self.buffers.push(buffer);
         }
-
+    
         self.current_buffer = 0;
         self.local_offset = 0;
-
+    
         return Ok(());
     }
 }
@@ -244,6 +244,6 @@ impl<A: Allocator> Poolable for ScratchAllocator<A> {
     type Key = ();
 
     fn on_release(&mut self) {
-        unsafe { self.reset(None).unwrap() }
+        unsafe { self.reset().unwrap() }
     }
-}
+}

src/core/app_info.rs

@@ -61,9 +61,11 @@ pub struct QueueRequest {
 /// // Enable an optional Vulkan feature.
 /// requirements.features.sampler_anisotropy = vk::TRUE;
 /// ```
-#[derive(Default, Debug)]
+#[derive(Derivative)]
+#[derivative(Debug)]
+#[derivative(Default)]
 pub struct GPURequirements {
-/// Whether a dedicated GPU is required. Setting this to true will discard integrated GPUs.
+    /// Whether a dedicated GPU is required. Setting this to true will discard integrated GPUs.
     pub dedicated: bool,
     /// Minimum amount of video memory required, in bytes. Note that this might count shared memory if RAM is shared.
     pub min_video_memory: usize,
@@ -86,6 +88,12 @@ pub struct GPURequirements {
     pub features_1_3: vk::PhysicalDeviceVulkan13Features,
     /// Vulkan device extensions that should be present and enabled.
     pub device_extensions: Vec<String>,
+    /// Callback function that will be used to select between multiple physical devices
+    /// The returned integer is a theoretical score for each physical device
+    /// Physical Devices with higher "scores" will be prioritized
+    #[derivative(Debug = "ignore")]
+    #[derivative(Default(value = "Box::new(|_| 0)"))]
+    pub scoring_callback: Box<dyn Fn(&crate::PhysicalDevice) -> usize>,
 }
 
 /// Extra data that is stored within the AppSettings whenever we want to enable renderable Surfaces 

src/core/physical_device.rs

@@ -39,6 +39,7 @@ pub struct PhysicalDevice {
 
 impl PhysicalDevice {
     /// Selects the best available physical device from the given requirements and parameters.
+    /// Also makes use of the given user callback for selecting between multiple candidates
     pub fn select(
         instance: &Instance,
         surface: Option<&Surface>,
@@ -49,9 +50,9 @@ impl PhysicalDevice {
             return Err(anyhow::Error::from(Error::NoGPU));
         }
 
-        devices
+        let mut candidates = devices
             .iter()
-            .find_map(|device| -> Option<PhysicalDevice> {
+            .filter_map(|device| -> Option<PhysicalDevice> {
                 let mut physical_device = PhysicalDevice {
                     handle: *device,
                     properties: unsafe { instance.get_physical_device_properties(*device) },
@@ -203,7 +204,12 @@ impl PhysicalDevice {
                 trace!("Created new VkPhysicalDevice {:p}", physical_device.handle);
                 Some(physical_device)
             })
-            .ok_or_else(|| anyhow::Error::from(Error::NoGPU))
+            .map(|physical_device| ((settings.gpu_requirements.scoring_callback)(&physical_device), physical_device))
+            .collect::<Vec<_>>();
+
+        candidates.sort_by_key(|(score, _)| *score);
+        let (_, chosen) = candidates.remove(0);
+        Ok(chosen)
     }
 
     /// Get all queue families available on this device. This is different from

tests/allocator.rs

@@ -8,7 +8,7 @@ mod framework;
 
 #[test]
 pub fn basic_allocator_usage() -> Result<()> {
-    let context = framework::make_context().expect("Can initialize context.");
+    let context = framework::make_context().expect("Failed to initialize context.");
     let mut allocator = context.allocator.clone();
     let allocation = allocator.allocate(
         "allocation",
@@ -32,7 +32,7 @@ pub fn basic_allocator_usage() -> Result<()> {
 
 #[test]
 pub fn cpu_to_gpu_is_mappable() -> Result<()> {
-    let context = framework::make_context().expect("Can initialize context.");
+    let context = framework::make_context().expect("Failed to initialize context.");
     let mut allocator = context.allocator.clone();
     let allocation = allocator.allocate(
         "allocation",
@@ -55,7 +55,7 @@ const CHUNK_SIZE: u64 = 32768;
 
 #[test]
 pub fn make_scratch_allocator() -> Result<()> {
-    let mut context = framework::make_context().expect("Can initialize context.");
+    let mut context = framework::make_context().expect("Failed to initialize context.");
 
     let _scratch_allocator =
         ScratchAllocator::new(context.device.clone(), &mut context.allocator, CHUNK_SIZE)?;
@@ -64,7 +64,7 @@ pub fn make_scratch_allocator() -> Result<()> {
 
 #[test]
 pub fn use_scratch_allocator() -> Result<()> {
-    let mut context = framework::make_context().expect("Can initialize context.");
+    let mut context = framework::make_context().expect("Failed to initialize context.");
     let mut scratch_allocator =
         ScratchAllocator::new(context.device.clone(), &mut context.allocator, CHUNK_SIZE)?;
     // Try allocating a buffer that should fit in the scratch allocator's memory.
@@ -75,7 +75,7 @@ pub fn use_scratch_allocator() -> Result<()> {
 #[test]
 pub fn use_entire_scratch_allocator() -> Result<()> {
     // Try allocating the entire scratch allocator's memory for a single buffer
-    let mut context = framework::make_context().expect("Can initialize context.");
+    let mut context = framework::make_context().expect("Failed to initialize context.");
     let mut scratch_allocator =
         ScratchAllocator::new(context.device.clone(), &mut context.allocator, CHUNK_SIZE)?;
     let _buffer = scratch_allocator.allocate(1024 as u64)?;
@@ -84,7 +84,7 @@ pub fn use_entire_scratch_allocator() -> Result<()> {
 
 #[test]
 pub fn scratch_allocator_allocate_new_chunks() -> Result<()> {
-    let mut context = framework::make_context().expect("Can initialize context.");
+    let mut context = framework::make_context().expect("Failed to initialize context.");
     let mut scratch_allocator =
         ScratchAllocator::new(context.device.clone(), &mut context.allocator, 1024)?;
     // First allocate a smaller buffer
@@ -96,20 +96,20 @@ pub fn scratch_allocator_allocate_new_chunks() -> Result<()> {
 
 #[test]
 pub fn reset_scratch_allocator() -> Result<()> {
-    let mut context = framework::make_context().expect("Can initialize context.");
+    let mut context = framework::make_context().expect("Failed to initialize context.");
     let mut scratch_allocator =
         ScratchAllocator::new(context.device.clone(), &mut context.allocator, CHUNK_SIZE)?;
     // Allocate a first buffer.
-    let _buffer = scratch_allocator.allocate(800 as u64)?;
+    let buffer = scratch_allocator.allocate(800 as u64)?;
     // Now reset it, so we should be able to allocate again
-    unsafe { scratch_allocator.reset(None)?; }
+    unsafe { scratch_allocator.reset()?; }
     let _buffer = scratch_allocator.allocate(800 as u64)?;
     Ok(())
 }
 
 #[test]
 pub fn scratch_allocator_mass_allocate() -> Result<()> {
-    let mut context = framework::make_context().expect("Can initialize context.");
+    let mut context = framework::make_context().expect("Failed to initialize context.");
     let mut scratch_allocator =
         ScratchAllocator::new(context.device.clone(), &mut context.allocator, CHUNK_SIZE)?;
 
@@ -128,7 +128,7 @@ pub fn scratch_allocator_mass_allocate() -> Result<()> {
         }
 
         // Now reset it, so we should be able to allocate again
-        unsafe { scratch_allocator.reset(None)?; }
+        unsafe { scratch_allocator.reset()?; }
     }
 
     Ok(())

tests/buffer.rs

@@ -8,7 +8,7 @@ mod framework;
 
 #[test]
 pub fn alloc_buffer() -> Result<()> {
-    let mut context = framework::make_context().expect("Can initialize context.");
+    let mut context = framework::make_context().expect("Failed to initialize context.");
 
     const ALLOC_SIZE: u64 = 1024u64;
 
@@ -31,7 +31,7 @@ pub fn alloc_buffer() -> Result<()> {
 
 #[test]
 pub fn alloc_aligned_buffer() -> Result<()> {
-    let mut context = framework::make_context().expect("Can initialize context.");
+    let mut context = framework::make_context().expect("Failed to initialize context.");
 
     const ALLOC_SIZE: u64 = 1000u64;
     const ALIGN: u64 = 128;
@@ -57,7 +57,7 @@ pub fn alloc_aligned_buffer() -> Result<()> {
 
 #[test]
 pub fn buffer_view_full() -> Result<()> {
-    let mut context = framework::make_context().expect("Can initialize context.");
+    let mut context = framework::make_context().expect("Failed to initialize context.");
 
     const ALLOC_SIZE: u64 = 1024u64;
 
@@ -83,7 +83,7 @@ pub fn buffer_view_full() -> Result<()> {
 
 #[test]
 pub fn buffer_view() -> Result<()> {
-    let mut context = framework::make_context().expect("Can initialize context.");
+    let mut context = framework::make_context().expect("Failed to initialize context.");
 
     const ALLOC_SIZE: u64 = 1024u64;
 
@@ -115,7 +115,7 @@ pub fn buffer_view() -> Result<()> {
 
 #[test]
 pub fn invalid_buffer_view() -> Result<()> {
-    let mut context = framework::make_context().expect("Can initialize context.");
+    let mut context = framework::make_context().expect("Failed to initialize context.");
 
     const ALLOC_SIZE: u64 = 1024u64;
 

tests/framework/mod.rs

@@ -47,6 +47,7 @@ pub fn make_context_with_queues(
             features_1_2: Default::default(),
             features_1_3: Default::default(),
             device_extensions: vec![],
+            scoring_callback: Box::new(|_| 0)
         })
         .build();
     let (instance, phys_device, None, device, allocator, pool, exec, None, None) =
@@ -87,6 +88,7 @@ pub fn make_context_with_settings<
             features_1_2: Default::default(),
             features_1_3: Default::default(),
             device_extensions: vec![],
+            scoring_callback: Box::new(|_| 0)
         });
 
     let settings = callback(builder).build();

tests/init.rs

@@ -55,7 +55,7 @@ pub fn requesting_raytracing_does_not_fail() -> Result<()> {
 
 #[test]
 pub fn vulkan_loaded() -> Result<()> {
-    let context = framework::make_context().expect("Can initialize context.");
+    let context = framework::make_context().expect("Failed to initialize context.");
     let handle = context.instance.handle();
     assert_ne!(handle.as_raw(), 0, "VkInstance handle should not be zero");
     let loader = unsafe { context.instance.loader() };
@@ -65,7 +65,7 @@ pub fn vulkan_loaded() -> Result<()> {
 
 #[test]
 pub fn valid_device() -> Result<()> {
-    let context = framework::make_context().expect("Can initialize context.");
+    let context = framework::make_context().expect("Failed to initialize context.");
     let handle = unsafe { context.device.handle() };
     assert_ne!(handle.handle().as_raw(), 0, "VkDevice handle should not be zero");
     // Also try a vulkan function call on it to make sure it is loaded properly