Fix crash when binding descriptor set to layout that has been destroyed and recreated.
Remove the descriptor set pool associated with a MVKDescriptorSetLayout when the layout
is destroyed. MVKDescriptorPool and MVKDescriptorSetLayout track each other to tell the
other when it has been destroyed.
diff --git a/Docs/Whats_New.md b/Docs/Whats_New.md
index 692ddba..3aabc51 100644
--- a/Docs/Whats_New.md
+++ b/Docs/Whats_New.md
@@ -19,7 +19,17 @@
Released TBD
- Add support for extensions:
+ - `VK_KHR_swapchain_mutable_format`
+ - `VK_KHR_uniform_buffer_standard_layout`
- `VK_EXT_metal_surface`
+ - `VK_EXT_post_depth_coverage`
+ - `VK_EXT_scalar_block_layout`
+ - `VK_EXT_shader_stencil_export`
+ - `VK_EXT_swapchain_colorspace`
+ - `VK_EXT_texel_buffer_alignment`
+ - `VK_AMD_shader_image_load_store_lod`
+ - `VK_AMD_shader_trinary_minmax`
+ - `VK_INTEL_shader_integer_functions2`
- For shaders created directly from MSL, set function name from
`VkPipelineShaderStageCreateInfo::pName`.
- On iOS GPU family 2 and earlier, support immutable depth-compare samplers
@@ -33,6 +43,7 @@
- Set Metal texture usage to allow texture copy via view.
- Fix memory leak in debug marker and debug utils labelling.
- Fix issue with push constants used across multiple draw calls not being applied.
+- Fix crash when binding descriptor set to layout that has been destroyed and recreated.
- Return error when `MVKImage` created as 1D attachment.
- Reduce use of autoreleased Obj-C objects, and ensure those remaining are
covered by deliberate autorelease pools.
@@ -77,7 +88,7 @@
- Fix tessellated indirect draws using wrong kernels to map parameters.
- Work around potential Metal bug with stage-in indirect buffers.
- Fix zero local threadgroup size in indirect tessellated rendering.
- - Fix [[attribute]] assignment for tessellation evaluation shaders.
+ - Fix `[[attribute]]` assignment for tessellation evaluation shaders.
- `VkSemaphore` optionally uses `MTLEvent`, if available and
`MVK_ALLOW_METAL_EVENTS` environment variable is enabled.
- Add `vkSetWorkgroupSizeMVK()` to set compute kernel workgroup size
@@ -102,7 +113,7 @@
- Fix unused attachments terminating loop early.
- Fix offset of buffer view relative to buffer offset within device memory.
- Guard against missing Metal pipeline states when pipeline compilation fails.
-- MVKBuffer: Force managed storage for linear textures on shared buffers.
+- `MVKBuffer`: Force managed storage for linear textures on shared buffers.
- Use device address space when decompressing DXT image data.
- Added missing `texelBufferTextureWidth` setting in `MVKComputePipeline::getMTLFunction()`.
- Fixes and consolidation of external library header references.
@@ -130,7 +141,7 @@
- MSL: Support Invariant qualifier on position.
- MSL: Support stencil export.
- Deal with case where a block is somehow emitted in a duplicated fashion.
- - Fix infinite loop when OpAtomic* temporaries are used in other blocks.
+ - Fix infinite loop when `OpAtomic*` temporaries are used in other blocks.
- Fix tests for device->constant address space change in MSL tessellation control shader generation.
- Accept SPIR-V 1.4 version.
@@ -274,7 +285,7 @@
- Allow default GPU Capture scope to be assigned to any queue in any queue family.
- VkPhysicalDevice: Correct some features and limits.
- Stop advertising atomic image support.
-- vkSetMTLTextureMVK() function retains texture object.
+- `vkSetMTLTextureMVK()` function retains texture object.
- Log to stderr instead of stdout.
- `fetchDependencies`: build `spirv-tools` when attached via symlink.
- Enhancements to `MVKVector`, and set appropriate inline sizing usages.
diff --git a/MoltenVK/MoltenVK/GPUObjects/MVKDescriptorSet.h b/MoltenVK/MoltenVK/GPUObjects/MVKDescriptorSet.h
index 90cf292..6bd5ae8 100644
--- a/MoltenVK/MoltenVK/GPUObjects/MVKDescriptorSet.h
+++ b/MoltenVK/MoltenVK/GPUObjects/MVKDescriptorSet.h
@@ -167,20 +167,25 @@
/** Returns true if this layout is for push descriptors only. */
bool isPushDescriptorLayout() const { return _isPushDescriptorLayout; }
- /** Constructs an instance for the specified device. */
MVKDescriptorSetLayout(MVKDevice* device, const VkDescriptorSetLayoutCreateInfo* pCreateInfo);
+ ~MVKDescriptorSetLayout();
+
protected:
friend class MVKDescriptorSetLayoutBinding;
friend class MVKPipelineLayout;
friend class MVKDescriptorSet;
+ friend class MVKDescriptorPool;
void propogateDebugName() override {}
-
+ void addDescriptorPool(MVKDescriptorPool* mvkDescPool) { _descriptorPools.insert(mvkDescPool); }
+ void removeDescriptorPool(MVKDescriptorPool* mvkDescPool) { _descriptorPools.erase(mvkDescPool); }
+
MVKVectorInline<MVKDescriptorSetLayoutBinding, 8> _bindings;
std::unordered_map<uint32_t, uint32_t> _bindingToIndex;
MVKShaderResourceBinding _mtlResourceCounts;
+ std::unordered_set<MVKDescriptorPool*> _descriptorPools;
bool _isPushDescriptorLayout : 1;
};
@@ -357,6 +362,9 @@
/** Destoys all currently allocated descriptor sets. */
VkResult reset(VkDescriptorPoolResetFlags flags);
+ /** Removes the pool associated with a descriptor set layout. */
+ void removeDescriptorSetPool(MVKDescriptorSetLayout* mvkDescSetLayout);
+
/** Constructs an instance for the specified device. */
MVKDescriptorPool(MVKDevice* device, const VkDescriptorPoolCreateInfo* pCreateInfo);
@@ -366,6 +374,7 @@
protected:
void propogateDebugName() override {}
MVKDescriptorSetPool* getDescriptorSetPool(MVKDescriptorSetLayout* mvkDescSetLayout);
+ void returnDescriptorSet(MVKDescriptorSet* mvkDescSet);
uint32_t _maxSets;
std::unordered_set<MVKDescriptorSet*> _allocatedSets;
diff --git a/MoltenVK/MoltenVK/GPUObjects/MVKDescriptorSet.mm b/MoltenVK/MoltenVK/GPUObjects/MVKDescriptorSet.mm
index 1396bac..1f97ac0 100644
--- a/MoltenVK/MoltenVK/GPUObjects/MVKDescriptorSet.mm
+++ b/MoltenVK/MoltenVK/GPUObjects/MVKDescriptorSet.mm
@@ -652,6 +652,10 @@
}
}
+MVKDescriptorSetLayout::~MVKDescriptorSetLayout() {
+ for (auto& dsPool : _descriptorPools) { dsPool->removeDescriptorSetPool(this); }
+}
+
#pragma mark -
#pragma mark MVKDescriptorBinding
@@ -1021,41 +1025,66 @@
VkResult MVKDescriptorPool::freeDescriptorSets(uint32_t count, const VkDescriptorSet* pDescriptorSets) {
for (uint32_t dsIdx = 0; dsIdx < count; dsIdx++) {
MVKDescriptorSet* mvkDS = (MVKDescriptorSet*)pDescriptorSets[dsIdx];
- if (_allocatedSets.erase(mvkDS)) {
- getDescriptorSetPool(mvkDS->_pLayout)->returnObject(mvkDS);
- }
+ if (_allocatedSets.erase(mvkDS)) { returnDescriptorSet(mvkDS); }
}
return VK_SUCCESS;
}
// Return any allocated descriptor sets to their pools
VkResult MVKDescriptorPool::reset(VkDescriptorPoolResetFlags flags) {
- for (auto& mvkDS : _allocatedSets) {
- getDescriptorSetPool(mvkDS->_pLayout)->returnObject(mvkDS);
- }
+ for (auto& mvkDS : _allocatedSets) { returnDescriptorSet(mvkDS); }
_allocatedSets.clear();
return VK_SUCCESS;
}
+// Returns the descriptor set to its pool, or if that pool doesn't exist, the descriptor set is destroyed
+void MVKDescriptorPool::returnDescriptorSet(MVKDescriptorSet* mvkDescSet) {
+ MVKDescriptorSetLayout* dsLayout = mvkDescSet->_pLayout;
+ MVKDescriptorSetPool* dsPool = dsLayout ? _descriptorSetPools[dsLayout] : nullptr;
+ if (dsPool) {
+ dsPool->returnObject(mvkDescSet);
+ } else {
+ mvkDescSet->destroy();
+ _descriptorSetPools.erase(dsLayout);
+ }
+}
+
// Returns the pool of descriptor sets that use a specific layout, lazily creating it if necessary
MVKDescriptorSetPool* MVKDescriptorPool::getDescriptorSetPool(MVKDescriptorSetLayout* mvkDescSetLayout) {
MVKDescriptorSetPool* dsp = _descriptorSetPools[mvkDescSetLayout];
if ( !dsp ) {
dsp = new MVKDescriptorSetPool(_device);
_descriptorSetPools[mvkDescSetLayout] = dsp;
+ mvkDescSetLayout->addDescriptorPool(this); // tell layout to track me
}
return dsp;
}
+// Remove the descriptor set pool associated with the descriptor set layout,
+// and make sure any allocated sets don't try to return back to their pools.
+void MVKDescriptorPool::removeDescriptorSetPool(MVKDescriptorSetLayout* mvkDescSetLayout) {
+ MVKDescriptorSetPool* dsp = _descriptorSetPools[mvkDescSetLayout];
+ if (dsp) { dsp->destroy(); }
+ _descriptorSetPools.erase(mvkDescSetLayout);
+
+ for (auto& mvkDS : _allocatedSets) {
+ if (mvkDS->_pLayout == mvkDescSetLayout) { mvkDS->_pLayout = nullptr; }
+ }
+}
+
MVKDescriptorPool::MVKDescriptorPool(MVKDevice* device,
const VkDescriptorPoolCreateInfo* pCreateInfo) : MVKVulkanAPIDeviceObject(device) {
_maxSets = pCreateInfo->maxSets;
}
-// Return any allocated sets to their pools and then destroy all the pools.
+// Return any allocated sets to their pools and then destroy all the pools,
+// and ensure any descriptor set layouts used as keys are notified.
MVKDescriptorPool::~MVKDescriptorPool() {
reset(0);
- for (auto& pair : _descriptorSetPools) { pair.second->destroy(); }
+ for (auto& pair : _descriptorSetPools) {
+ pair.first->removeDescriptorPool(this);
+ if (pair.second) { pair.second->destroy(); }
+ }
}