From e5ffdb740e0a597342b65bf3c95adb0f73e529e3 Mon Sep 17 00:00:00 2001 From: Bill Hollings Date: Sat, 14 Sep 2024 16:47:50 -0400 Subject: [PATCH] Fix shader conversion failure when using storage images on iOS & tvOS with Tier 1 argument buffer support. - MVKDescriptorSetLayout: Test for iOS & tvOS Tier 1 storage images in any binding before construction of MVKDescriptorSetLayoutBindings. --- Docs/Whats_New.md | 3 +- MoltenVK/MoltenVK/GPUObjects/MVKDescriptor.mm | 7 ----- .../MoltenVK/GPUObjects/MVKDescriptorSet.h | 1 + .../MoltenVK/GPUObjects/MVKDescriptorSet.mm | 30 ++++++++++++++++--- 4 files changed, 29 insertions(+), 12 deletions(-) diff --git a/Docs/Whats_New.md b/Docs/Whats_New.md index c56890364..cea5d338a 100644 --- a/Docs/Whats_New.md +++ b/Docs/Whats_New.md @@ -28,15 +28,16 @@ Released 2024-09-24 - Update `VkFormat` capabilities based on latest Metal docs. - Ensure all MoltenVK config info set by `VK_EXT_layer_settings` is used. - Support storage images in Metal argument buffers on _iOS_. +- `vkUpdateDescriptorSets()`: Support writing beyond descriptor binding size if subsequent bindings are of same type. - 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. - Fix issue where `vkQueueWaitIdle()` and `vkDeviceWaitIdle()` were not waiting for all commands to be enqueued before enqueuing wait operation. +- Fix shader conversion failure when using storage images on _iOS_ & _tvOS_ with Tier 1 argument buffer support. - Fix memory leak in debug utils messenger. - Fix build failure on _VisionOS 2.0_ platform. -- `vkUpdateDescriptorSets()`: Support writing beyond descriptor binding size if subsequent bindings are of same type. - Support `VK_FORMAT_A2B10G10R10_UNORM_PACK32` and `VK_FORMAT_A2R10G10B10_UNORM_PACK32` formats as surface formats on all platforms. - Add `MTLStoreAction` mapping for `VK_ATTACHMENT_STORE_OP_NONE`. - Add estimate of `presentMargin` in returned data from `vkGetPastPresentationTimingGOOGLE()`. diff --git a/MoltenVK/MoltenVK/GPUObjects/MVKDescriptor.mm b/MoltenVK/MoltenVK/GPUObjects/MVKDescriptor.mm index a3807e2ae..94843937f 100644 --- a/MoltenVK/MoltenVK/GPUObjects/MVKDescriptor.mm +++ b/MoltenVK/MoltenVK/GPUObjects/MVKDescriptor.mm @@ -724,13 +724,6 @@ void mvkPopulateShaderConversionConfig(mvk::SPIRVToMSLConversionConfiguration& s setResourceIndexOffset(textureIndex, 1); if (!getMetalFeatures().nativeTextureAtomics) { setResourceIndexOffset(bufferIndex, 1); } -#if MVK_IOS_OR_TVOS - // iOS Tier 1 argument buffers do not support writable images. - if (getMetalFeatures().argumentBuffersTier < MTLArgumentBuffersTier2) { - _layout->_canUseMetalArgumentBuffer = false; - } -#endif - if (pBinding->descriptorCount > 1 && !mtlFeats.arrayOfTextures) { _layout->setConfigurationResult(reportError(VK_ERROR_FEATURE_NOT_PRESENT, "Device %s does not support arrays of textures.", _device->getName())); } diff --git a/MoltenVK/MoltenVK/GPUObjects/MVKDescriptorSet.h b/MoltenVK/MoltenVK/GPUObjects/MVKDescriptorSet.h index 9bf36a75c..ebf6d51db 100644 --- a/MoltenVK/MoltenVK/GPUObjects/MVKDescriptorSet.h +++ b/MoltenVK/MoltenVK/GPUObjects/MVKDescriptorSet.h @@ -133,6 +133,7 @@ class MVKDescriptorSetLayout : public MVKVulkanAPIDeviceObject { uint32_t getBufferSizeBufferArgBuferIndex() { return 0; } id getMTLArgumentEncoder(uint32_t variableDescriptorCount); uint64_t getMetal3ArgumentBufferEncodedLength(uint32_t variableDescriptorCount); + bool checkCanUseArgumentBuffers(const VkDescriptorSetLayoutCreateInfo* pCreateInfo); std::string getLogDescription(); MVKSmallVector _bindings; diff --git a/MoltenVK/MoltenVK/GPUObjects/MVKDescriptorSet.mm b/MoltenVK/MoltenVK/GPUObjects/MVKDescriptorSet.mm index 4d6931b54..9d88b9d3f 100644 --- a/MoltenVK/MoltenVK/GPUObjects/MVKDescriptorSet.mm +++ b/MoltenVK/MoltenVK/GPUObjects/MVKDescriptorSet.mm @@ -363,10 +363,6 @@ static void populateAuxBuffer(mvk::SPIRVToMSLConversionConfiguration& shaderConf MVKDescriptorSetLayout::MVKDescriptorSetLayout(MVKDevice* device, const VkDescriptorSetLayoutCreateInfo* pCreateInfo) : MVKVulkanAPIDeviceObject(device) { - - _isPushDescriptorLayout = mvkIsAnyFlagEnabled(pCreateInfo->flags, VK_DESCRIPTOR_SET_LAYOUT_CREATE_PUSH_DESCRIPTOR_BIT_KHR); - _canUseMetalArgumentBuffer = !_isPushDescriptorLayout; // Push descriptors don't use argument buffers - const VkDescriptorBindingFlags* pBindingFlags = nullptr; for (const auto* next = (VkBaseInStructure*)pCreateInfo->pNext; next; next = next->pNext) { switch (next->sType) { @@ -382,6 +378,9 @@ static void populateAuxBuffer(mvk::SPIRVToMSLConversionConfiguration& shaderConf } } + _isPushDescriptorLayout = mvkIsAnyFlagEnabled(pCreateInfo->flags, VK_DESCRIPTOR_SET_LAYOUT_CREATE_PUSH_DESCRIPTOR_BIT_KHR); + _canUseMetalArgumentBuffer = checkCanUseArgumentBuffers(pCreateInfo); // After _isPushDescriptorLayout + // The bindings in VkDescriptorSetLayoutCreateInfo do not need to be provided in order of binding number. // However, several subsequent operations, such as the dynamic offsets in vkCmdBindDescriptorSets() // are ordered by binding number. To prepare for this, sort the bindings by binding number. @@ -403,6 +402,7 @@ static void populateAuxBuffer(mvk::SPIRVToMSLConversionConfiguration& shaderConf return bindInfo1.pBinding->binding < bindInfo2.pBinding->binding; }); + // Create bindings. Must be done after _isPushDescriptorLayout & _canUseMetalArgumentBuffer are set. uint32_t dslDescCnt = 0; uint32_t dslMTLRezCnt = needsBuffSizeAuxBuff ? 1 : 0; // If needed, leave a slot for the buffer sizes buffer at front. _bindings.reserve(bindCnt); @@ -424,6 +424,28 @@ static void populateAuxBuffer(mvk::SPIRVToMSLConversionConfiguration& shaderConf return descStr.str(); } +// Check if argument buffers can be used, and return findings. +// Must be called after setting _isPushDescriptorLayout. +bool MVKDescriptorSetLayout::checkCanUseArgumentBuffers(const VkDescriptorSetLayoutCreateInfo* pCreateInfo) { + +// iOS Tier 1 argument buffers do not support writable images. +#if MVK_IOS_OR_TVOS + if (getMetalFeatures().argumentBuffersTier < MTLArgumentBuffersTier2) { + for (uint32_t bindIdx = 0; bindIdx < pCreateInfo->bindingCount; bindIdx++) { + switch (pCreateInfo->pBindings[bindIdx].descriptorType) { + case VK_DESCRIPTOR_TYPE_STORAGE_IMAGE: + case VK_DESCRIPTOR_TYPE_STORAGE_TEXEL_BUFFER: + return false; + default: + break; + } + } + } +#endif + + return !_isPushDescriptorLayout; // Push descriptors don't use argument buffers +} + #pragma mark - #pragma mark MVKDescriptorSet