Skip to content

Commit

Permalink
Merge pull request #2335 from billhollings/fix-arg-buffs-ios-tier1-im…
Browse files Browse the repository at this point in the history
…g-write

Fix shader conversion failure when using storage images
  • Loading branch information
billhollings authored Sep 14, 2024
2 parents 2f7ae76 + e5ffdb7 commit 5409e80
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 12 deletions.
3 changes: 2 additions & 1 deletion Docs/Whats_New.md
Original file line number Diff line number Diff line change
Expand Up @@ -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()`.
Expand Down
7 changes: 0 additions & 7 deletions MoltenVK/MoltenVK/GPUObjects/MVKDescriptor.mm
Original file line number Diff line number Diff line change
Expand Up @@ -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()));
}
Expand Down
1 change: 1 addition & 0 deletions MoltenVK/MoltenVK/GPUObjects/MVKDescriptorSet.h
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@ class MVKDescriptorSetLayout : public MVKVulkanAPIDeviceObject {
uint32_t getBufferSizeBufferArgBuferIndex() { return 0; }
id <MTLArgumentEncoder> getMTLArgumentEncoder(uint32_t variableDescriptorCount);
uint64_t getMetal3ArgumentBufferEncodedLength(uint32_t variableDescriptorCount);
bool checkCanUseArgumentBuffers(const VkDescriptorSetLayoutCreateInfo* pCreateInfo);
std::string getLogDescription();

MVKSmallVector<MVKDescriptorSetLayoutBinding> _bindings;
Expand Down
30 changes: 26 additions & 4 deletions MoltenVK/MoltenVK/GPUObjects/MVKDescriptorSet.mm
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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.
Expand All @@ -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);
Expand All @@ -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
Expand Down

0 comments on commit 5409e80

Please sign in to comment.