Fixes to vkCmdPushDescriptorSetWithTemplateKHR().

Docs/Whats_New.md

@@ -28,6 +28,8 @@ Released TBD
 - Update `VkFormat` capabilities based on latest Metal docs.
 - Fix rendering issue with render pass that immediately follows a kernel dispatch.
 - Fix race condition when `VkImage` destroyed while used by descriptor.
+- Fix crash in `vkCmdPushDescriptorSetWithTemplateKHR()` when entries in 
+  `VkDescriptorUpdateTemplateCreateInfo` are not sorted by offset.
 - Ensure all MoltenVK config info set by `VK_EXT_layer_settings` is used.
 - Move primitive-restart-disabled warning from renderpass to pipeline creation, to reduce voluminous log noise.
 - iOS: Support storage images in _Metal_ argument buffers.

MoltenVK/MoltenVK/Commands/MVKCmdPipeline.h

@@ -298,10 +298,11 @@ class MVKCmdPushDescriptorSetWithTemplate : public MVKCommand {
 protected:
 	MVKCommandTypePool<MVKCommand>* getTypePool(MVKCommandPool* cmdPool) override;
 
-	MVKDescriptorUpdateTemplate* _descUpdateTemplate;
+	MVKDescriptorUpdateTemplate* _descUpdateTemplate = nullptr;
 	MVKPipelineLayout* _pipelineLayout = nullptr;
 	void* _pData = nullptr;
-	uint32_t _set;
+	size_t _dataSize = 0;
+	uint32_t _set = 0;
 };
 
 

MoltenVK/MoltenVK/Commands/MVKCmdPipeline.mm

@@ -478,49 +478,20 @@
 														 uint32_t set,
 														 const void* pData) {
 	if (_pipelineLayout) { _pipelineLayout->release(); }
-
-	_descUpdateTemplate = (MVKDescriptorUpdateTemplate*)descUpdateTemplate;
 	_pipelineLayout = (MVKPipelineLayout*)layout;
-	_set = set;
-
 	_pipelineLayout->retain();
+	_set = set;
+	_descUpdateTemplate = (MVKDescriptorUpdateTemplate*)descUpdateTemplate;
 
-	if (_pData) delete[] (char*)_pData;
-	// Work out how big the memory block in pData is.
-	const VkDescriptorUpdateTemplateEntry* pEntry =
-		_descUpdateTemplate->getEntry(_descUpdateTemplate->getNumberOfEntries()-1);
-	size_t size = pEntry->offset;
-	// If we were given a stride, use that; otherwise, assume only one info
-	// struct of the appropriate type.
-	if (pEntry->stride)
-		size += pEntry->stride * pEntry->descriptorCount;
-	else switch (pEntry->descriptorType) {
-
-		case VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER_DYNAMIC:
-		case VK_DESCRIPTOR_TYPE_STORAGE_BUFFER_DYNAMIC:
-		case VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER:
-		case VK_DESCRIPTOR_TYPE_STORAGE_BUFFER:
-			size += sizeof(VkDescriptorBufferInfo);
-			break;
-
-		case VK_DESCRIPTOR_TYPE_SAMPLED_IMAGE:
-		case VK_DESCRIPTOR_TYPE_STORAGE_IMAGE:
-		case VK_DESCRIPTOR_TYPE_INPUT_ATTACHMENT:
-		case VK_DESCRIPTOR_TYPE_SAMPLER:
-		case VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER:
-			size += sizeof(VkDescriptorImageInfo);
-			break;
-
-		case VK_DESCRIPTOR_TYPE_UNIFORM_TEXEL_BUFFER:
-		case VK_DESCRIPTOR_TYPE_STORAGE_TEXEL_BUFFER:
-			size += sizeof(VkBufferView);
-			break;
-
-		default:
-			break;
+	size_t oldSize = _dataSize;
+	_dataSize = _descUpdateTemplate->getSize();
+	if (_dataSize > oldSize) {
+		free(_pData);
+		_pData = malloc(_dataSize);
+	}
+	if (_pData && pData) {
+		mvkCopy(_pData, pData, _dataSize);
 	}
-	_pData = new char[size];
-	memcpy(_pData, pData, size);
 
 	// Validate by encoding on a null encoder
 	encode(nullptr);
@@ -533,7 +504,7 @@
 
 MVKCmdPushDescriptorSetWithTemplate::~MVKCmdPushDescriptorSetWithTemplate() {
 	if (_pipelineLayout) { _pipelineLayout->release(); }
-	if (_pData) delete[] (char*)_pData;
+	free(_pData);
 }
 
 

MoltenVK/MoltenVK/GPUObjects/MVKDescriptorSet.h

@@ -339,6 +339,9 @@ class MVKDescriptorUpdateTemplate : public MVKVulkanAPIDeviceObject {
 	/** Get the total number of entries. */
 	uint32_t getNumberOfEntries() const;
 
+	/** Get the total number of bytes of data requried by this template. */
+	size_t getSize() const { return _size; }
+
 	/** Get the type of this template. */
 	VkDescriptorUpdateTemplateType getType() const;
 
@@ -354,9 +357,10 @@ class MVKDescriptorUpdateTemplate : public MVKVulkanAPIDeviceObject {
 protected:
 	void propagateDebugName() override {}
 
+	MVKSmallVector<VkDescriptorUpdateTemplateEntry, 1> _entries;
+	size_t _size = 0;
 	VkPipelineBindPoint _pipelineBindPoint;
 	VkDescriptorUpdateTemplateType _type;
-	MVKSmallVector<VkDescriptorUpdateTemplateEntry, 1> _entries;
 };
 
 #pragma mark -

MoltenVK/MoltenVK/GPUObjects/MVKDescriptorSet.mm

@@ -1142,10 +1142,45 @@ static void populateAuxBuffer(mvk::SPIRVToMSLConversionConfiguration& shaderConf
 
 MVKDescriptorUpdateTemplate::MVKDescriptorUpdateTemplate(MVKDevice* device,
 														 const VkDescriptorUpdateTemplateCreateInfo* pCreateInfo) :
-	MVKVulkanAPIDeviceObject(device), _pipelineBindPoint(pCreateInfo->pipelineBindPoint), _type(pCreateInfo->templateType) {
+MVKVulkanAPIDeviceObject(device), _pipelineBindPoint(pCreateInfo->pipelineBindPoint), _type(pCreateInfo->templateType) {
 
-	for (uint32_t i = 0; i < pCreateInfo->descriptorUpdateEntryCount; i++)
-		_entries.push_back(pCreateInfo->pDescriptorUpdateEntries[i]);
+	for (uint32_t i = 0; i < pCreateInfo->descriptorUpdateEntryCount; i++) {
+		const auto& entry = pCreateInfo->pDescriptorUpdateEntries[i];
+		_entries.push_back(entry);
+
+		// Accumulate the size of the template. If we were given a stride, use that;
+		// otherwise, assume only one info struct of the appropriate type.
+		size_t entryEnd = entry.offset;
+		if (entry.stride) {
+			entryEnd += entry.stride * entry.descriptorCount;
+		} else {
+			switch (entry.descriptorType) {
+				case VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER_DYNAMIC:
+				case VK_DESCRIPTOR_TYPE_STORAGE_BUFFER_DYNAMIC:
+				case VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER:
+				case VK_DESCRIPTOR_TYPE_STORAGE_BUFFER:
+					entryEnd += sizeof(VkDescriptorBufferInfo);
+					break;
+
+				case VK_DESCRIPTOR_TYPE_SAMPLED_IMAGE:
+				case VK_DESCRIPTOR_TYPE_STORAGE_IMAGE:
+				case VK_DESCRIPTOR_TYPE_INPUT_ATTACHMENT:
+				case VK_DESCRIPTOR_TYPE_SAMPLER:
+				case VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER:
+					entryEnd += sizeof(VkDescriptorImageInfo);
+					break;
+
+				case VK_DESCRIPTOR_TYPE_UNIFORM_TEXEL_BUFFER:
+				case VK_DESCRIPTOR_TYPE_STORAGE_TEXEL_BUFFER:
+					entryEnd += sizeof(VkBufferView);
+					break;
+
+				default:
+					break;
+			}
+		}
+		_size = std::max(_size, entryEnd);
+	}
 }
 
 

MoltenVK/MoltenVK/Utility/MVKFoundation.h

@@ -571,13 +571,22 @@ static void mvkClear(const T* pVal, size_t count = 1) { mvkClear((T*)pVal, count
 /**
  * If pSrc and pDst are both not null, copies the contents of the source value to the
  * destination value. The optional count allows copying of multiple elements in an array.
+ * Supports void pointers, and copies single values via direct assignment.
  */
 template<typename T>
 static void mvkCopy(T* pDst, const T* pSrc, size_t count = 1) {
-	if ( !pDst || !pSrc ) { return; }			// Bad pointers
-	if (pDst == pSrc) { return; }				// Same object
-	if constexpr(std::is_arithmetic_v<T>) { if (count == 1) { *pDst = *pSrc; } }  // Fast copy of a single primitive
-	memcpy(pDst, pSrc, sizeof(T) * count);		// Memory copy of complex content or array
+	if ( !pDst || !pSrc ) { return; }				// Bad pointers
+	if (pDst == pSrc) { return; }					// Same object
+
+	if constexpr(std::is_void_v<T>) {
+		memcpy(pDst, pSrc, count);					// Copy as bytes
+	} else {
+		if (count == 1) {
+			*pDst = *pSrc;  						// Fast copy of a single value
+		} else {
+			memcpy(pDst, pSrc, sizeof(T) * count);	// Memory copy of value array
+		}
+	}
 }
 
 /**

MoltenVK/MoltenVK/Vulkan/vulkan.mm

@@ -3121,7 +3121,7 @@ MVK_PUBLIC_VULKAN_SYMBOL void vkCmdPushDescriptorSetKHR(
 
 MVK_PUBLIC_VULKAN_SYMBOL void vkCmdPushDescriptorSetWithTemplateKHR(
     VkCommandBuffer                            commandBuffer,
-    VkDescriptorUpdateTemplate              descriptorUpdateTemplate,
+    VkDescriptorUpdateTemplate                 descriptorUpdateTemplate,
     VkPipelineLayout                           layout,
     uint32_t                                   set,
     const void*                                pData) {