Merge #106

106: Fix descriptor set leak r=grovesNL a=kvark Fixes #99 Co-authored-by: Dzmitry Malyshau <[email protected]>
gfx-rs · Jun 25, 2018 · 0764227 · 0764227
2 parents efb6e9a + 1e3541c
commit 0764227
Show file tree

Hide file tree

Showing 5 changed files with 49 additions and 26 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Makefile b/Makefile
@@ -12,6 +12,7 @@ DEQP_DIR=$(CTS_DIR)/build/external/vulkancts/modules/vulkan/
 DEQP=cd $(DEQP_DIR) && RUST_LOG=debug LD_LIBRARY_PATH=$(FULL_LIBRARY_PATH) ./deqp-vk
 DOTA_DIR=../dota2/bin/osx64
 DOTA_EXE=$(DOTA_DIR)/dota2.app/Contents/MacOS/dota2
+DOTA_PARAMS=-vulkan_disable_occlusion_queries -vulkan_scene_system_job_cost 2
 
 RUST_BACKTRACE:=1
 BACKEND:=gl
@@ -45,7 +46,7 @@ FULL_LIBRARY_PATH=$(CURDIR)/target/debug
 LIBRARY=target/debug/libportability.$(LIB_EXTENSION)
 LIBRARY_FAST=target/release/libportability.$(LIB_EXTENSION)
 
-.PHONY: all rebuild debug release debug-version release-version binding run cts clean cherry dota-debug dota-release
+.PHONY: all rebuild debug release version-debug version-release binding run cts clean cherry dota-debug dota-release
 
 all: $(TARGET)
 
@@ -57,17 +58,17 @@ debug:
 
 release: $(LIBRARY_FAST)
 
-debug-version:
+version-debug:
 	cargo rustc --manifest-path libportability/Cargo.toml --features $(BACKEND),portability-gfx/env_logger -- -Clink-arg="-current_version 1.0.0" -Clink-arg="-compatibility_version 1.0.0"
 
-release-version:
+version-release:
 	cargo rustc --release --manifest-path libportability/Cargo.toml --features $(BACKEND) -- -Clink-arg="-current_version 1.0.0" -Clink-arg="-compatibility_version 1.0.0"
 
-dota-debug: debug-version
-	DYLD_LIBRARY_PATH=`pwd`/target/debug:`pwd`/$(DOTA_DIR) $(DOTA_EXE)
+dota-debug: version-debug $(DOTA_EXE)
+	DYLD_LIBRARY_PATH=`pwd`/target/debug:`pwd`/$(DOTA_DIR) $(DOTA_EXE) $(DOTA_PARAMS)
 
-dota-release: release-version
-	DYLD_LIBRARY_PATH=`pwd`/target/release:`pwd`/$(DOTA_DIR) $(DOTA_EXE)
+dota-release: version-release $(DOTA_EXE)
+	DYLD_LIBRARY_PATH=`pwd`/target/release:`pwd`/$(DOTA_DIR) $(DOTA_EXE) $(DOTA_PARAMS)
 
 binding: $(BINDING)
 

diff --git a/libportability-gfx/src/conv.rs b/libportability-gfx/src/conv.rs
@@ -549,7 +549,7 @@ pub fn map_front_face(face: VkFrontFace) -> pso::FrontFace {
     }
 }
 
-pub fn map_primitive_topology(topology: VkPrimitiveTopology, patch_size: PatchSize) -> Option<hal::Primitive> {
+pub fn map_primitive_topology(topology: VkPrimitiveTopology, patch_size: PatchSize) -> Option<Primitive> {
     use super::VkPrimitiveTopology::*;
 
     Some(match topology {

diff --git a/libportability-gfx/src/impls.rs b/libportability-gfx/src/impls.rs
@@ -2360,9 +2360,6 @@ pub extern "C" fn gfxCreateDescriptorPool(
     pDescriptorPool: *mut VkDescriptorPool,
 ) -> VkResult {
     let info = unsafe { &*pCreateInfo };
-    if info.flags != 0 {
-        warn!("gfxCreateDescriptorPool flags are not supported: 0x{:x}", info.flags);
-    }
 
     let pool_sizes = unsafe {
         slice::from_raw_parts(info.pPoolSizes, info.poolSizeCount as _)
@@ -2378,8 +2375,15 @@ pub extern "C" fn gfxCreateDescriptorPool(
         })
         .collect::<Vec<_>>();
 
-    let pool = gpu.device
-        .create_descriptor_pool(info.maxSets as _, &ranges);
+    let pool = super::DescriptorPool {
+        raw: gpu.device
+            .create_descriptor_pool(info.maxSets as _, &ranges),
+        sets: if info.flags & VkDescriptorPoolCreateFlagBits::VK_DESCRIPTOR_POOL_CREATE_FREE_DESCRIPTOR_SET_BIT as u32 != 0 {
+            None
+        } else {
+            Some(Vec::new())
+        },
+    };
 
     unsafe { *pDescriptorPool = Handle::new(pool); }
     VkResult::VK_SUCCESS
@@ -2391,7 +2395,12 @@ pub extern "C" fn gfxDestroyDescriptorPool(
     _pAllocator: *const VkAllocationCallbacks,
 ) {
     if let Some(pool) = descriptorPool.unbox() {
-        gpu.device.destroy_descriptor_pool(pool);
+        gpu.device.destroy_descriptor_pool(pool.raw);
+        if let Some(sets) = pool.sets {
+            for set in sets {
+                let _ = set.unbox();
+            }
+        }
     }
 }
 #[inline]
@@ -2400,7 +2409,12 @@ pub extern "C" fn gfxResetDescriptorPool(
     mut descriptorPool: VkDescriptorPool,
     _flags: VkDescriptorPoolResetFlags,
 ) -> VkResult {
-    descriptorPool.reset();
+    descriptorPool.raw.reset();
+    if let Some(ref mut sets) = descriptorPool.sets {
+        for set in sets.drain(..) {
+            let _ = set.unbox();
+        }
+    }
     VkResult::VK_SUCCESS
 }
 #[inline]
@@ -2419,7 +2433,7 @@ pub extern "C" fn gfxAllocateDescriptorSets(
         .iter()
         .map(|layout| &**layout);
 
-    let descriptor_sets = pool.allocate_sets(layouts);
+    let descriptor_sets = pool.raw.allocate_sets(layouts);
     let sets = unsafe {
         slice::from_raw_parts_mut(pDescriptorSets, info.descriptorSetCount as _)
     };
@@ -2434,6 +2448,9 @@ pub extern "C" fn gfxAllocateDescriptorSets(
                 pso::AllocationError::FragmentedPool => VkResult::VK_ERROR_FRAGMENTED_POOL,
             },
         };
+        if let Some(ref mut local_sets) = pool.sets {
+            local_sets.push(*set);
+        }
     }
 
     VkResult::VK_SUCCESS
@@ -2448,8 +2465,9 @@ pub extern "C" fn gfxFreeDescriptorSets(
     let descriptor_sets = unsafe {
         slice::from_raw_parts(pDescriptorSets, descriptorSetCount as _)
     };
+    assert!(descriptorPool.sets.is_none());
 
-    descriptorPool.free_sets(
+    descriptorPool.raw.free_sets(
         &descriptor_sets
             .into_iter()
             .filter_map(|set| set.unbox())
@@ -3812,9 +3830,8 @@ pub extern "C" fn gfxGetPhysicalDeviceSurfacePresentModesKHR(
     pPresentModeCount: *mut u32,
     pPresentModes: *mut VkPresentModeKHR,
 ) -> VkResult {
-    let present_modes = surface
-        .compatibility(&adapter.physical_device)
-        .2;
+    let (_, _, present_modes) = surface
+        .compatibility(&adapter.physical_device);
 
     let num_present_modes = present_modes.len();
 

diff --git a/libportability-gfx/src/lib.rs b/libportability-gfx/src/lib.rs
@@ -45,7 +45,7 @@ pub type VkCommandBuffer = DispatchHandle<<B as hal::Backend>::CommandBuffer>;
 pub type VkDeviceMemory = Handle<<B as hal::Backend>::Memory>;
 pub type VkDescriptorSetLayout = Handle<<B as hal::Backend>::DescriptorSetLayout>;
 pub type VkPipelineLayout = Handle<<B as hal::Backend>::PipelineLayout>;
-pub type VkDescriptorPool = Handle<<B as hal::Backend>::DescriptorPool>;
+pub type VkDescriptorPool = Handle<DescriptorPool<B>>;
 pub type VkDescriptorSet = Handle<<B as hal::Backend>::DescriptorSet>;
 pub type VkSampler = Handle<<B as hal::Backend>::Sampler>;
 pub type VkBufferView = Handle<<B as hal::Backend>::BufferView>;
@@ -78,6 +78,11 @@ pub struct Gpu<B: hal::Backend> {
     capturing: *mut (),
 }
 
+pub struct DescriptorPool<B: hal::Backend> {
+    raw: B::DescriptorPool,
+    sets: Option<Vec<VkDescriptorSet>>,
+}
+
 pub enum Pipeline<B: hal::Backend> {
     Graphics(B::GraphicsPipeline),
     Compute(B::ComputePipeline),