diff --git a/Docs/Whats_New.md b/Docs/Whats_New.md index 1272d2c5a..168635786 100644 --- a/Docs/Whats_New.md +++ b/Docs/Whats_New.md @@ -23,6 +23,8 @@ Released TBD - `vkAllocateDescriptorSets()`: Per Vulkan spec, if any descriptor set allocation fails, populate all descriptor set pointers with `VK_NULL_HANDLE`. In addition, return `VK_ERROR_FRAGMENTED_POOL` if failure was due to pool fragmentation. +- `vkUpdateDescriptorSets()`: Per Vulkan spec, allow write or copy beyond the + end of a descriptor binding count, including inline uniform block descriptors. - Fix rendering issue with render pass that immediately follows a kernel dispatch. - 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. diff --git a/MoltenVK/MoltenVK/Commands/MVKMTLBufferAllocation.mm b/MoltenVK/MoltenVK/Commands/MVKMTLBufferAllocation.mm index f0d8017c1..10ee00e38 100644 --- a/MoltenVK/MoltenVK/Commands/MVKMTLBufferAllocation.mm +++ b/MoltenVK/MoltenVK/Commands/MVKMTLBufferAllocation.mm @@ -82,7 +82,6 @@ } } - MVKMTLBufferAllocationPool::MVKMTLBufferAllocationPool(MVKDevice* device, NSUInteger allocationLength, bool makeThreadSafe, bool isDedicated, MTLStorageMode mtlStorageMode) : MVKObjectPool(true), diff --git a/MoltenVK/MoltenVK/GPUObjects/MVKDescriptor.h b/MoltenVK/MoltenVK/GPUObjects/MVKDescriptor.h index 92e6504dc..8e8cd5258 100644 --- a/MoltenVK/MoltenVK/GPUObjects/MVKDescriptor.h +++ b/MoltenVK/MoltenVK/GPUObjects/MVKDescriptor.h @@ -381,18 +381,33 @@ class MVKInlineUniformBlockDescriptor : public MVKDescriptor { void write(MVKDescriptorSetLayoutBinding* mvkDSLBind, MVKDescriptorSet* mvkDescSet, - uint32_t dstOffset, // Inline buffers use this parameter as an offset, not an index + uint32_t dstIdx, uint32_t srcIdx, size_t srcStride, - const void* pData) override; + const void* pData) override {} void read(MVKDescriptorSetLayoutBinding* mvkDSLBind, MVKDescriptorSet* mvkDescSet, - uint32_t srcOffset, // For inline buffers we are using this parameter as src offset not as dst descIdx + uint32_t dstIndex, VkDescriptorImageInfo* pImageInfo, VkDescriptorBufferInfo* pBufferInfo, VkBufferView* pTexelBufferView, - VkWriteDescriptorSetInlineUniformBlockEXT* inlineUniformBlock) override; + VkWriteDescriptorSetInlineUniformBlockEXT* inlineUniformBlock) override {} + + uint32_t writeBytes(MVKDescriptorSetLayoutBinding* mvkDSLBind, + MVKDescriptorSet* mvkDescSet, + uint32_t dstOffset, + uint32_t srcOffset, + uint32_t byteCount, + const VkWriteDescriptorSetInlineUniformBlockEXT* pInlineUniformBlock); + + uint32_t readBytes(MVKDescriptorSetLayoutBinding* mvkDSLBind, + MVKDescriptorSet* mvkDescSet, + uint32_t dstOffset, + uint32_t srcOffset, + uint32_t byteCount, + const VkWriteDescriptorSetInlineUniformBlockEXT* pInlineUniformBlock); + void encodeResourceUsage(MVKResourcesCommandEncoderState* rezEncState, MVKDescriptorSetLayoutBinding* mvkDSLBind, diff --git a/MoltenVK/MoltenVK/GPUObjects/MVKDescriptor.mm b/MoltenVK/MoltenVK/GPUObjects/MVKDescriptor.mm index 716af8eca..a3807e2ae 100644 --- a/MoltenVK/MoltenVK/GPUObjects/MVKDescriptor.mm +++ b/MoltenVK/MoltenVK/GPUObjects/MVKDescriptor.mm @@ -898,21 +898,26 @@ void mvkPopulateShaderConversionConfig(mvk::SPIRVToMSLConversionConfiguration& s } } -void MVKInlineUniformBlockDescriptor::write(MVKDescriptorSetLayoutBinding* mvkDSLBind, - MVKDescriptorSet* mvkDescSet, - uint32_t dstOffset, - uint32_t srcIdx, - size_t srcStride, - const void* pData) { +uint32_t MVKInlineUniformBlockDescriptor::writeBytes(MVKDescriptorSetLayoutBinding* mvkDSLBind, + MVKDescriptorSet* mvkDescSet, + uint32_t dstOffset, + uint32_t srcOffset, + uint32_t byteCount, + const VkWriteDescriptorSetInlineUniformBlockEXT* pInlineUniformBlock) { + uint32_t dataLen = 0; + uint32_t dstBuffSize = mvkDSLBind->_info.descriptorCount; + uint32_t srcBuffSize = pInlineUniformBlock->dataSize; + if (dstOffset < dstBuffSize && srcOffset < srcBuffSize) { + dataLen = std::min({ byteCount, dstBuffSize - dstOffset, srcBuffSize - srcOffset }); + } + // Ensure there is a destination to write to - uint32_t buffSize = mvkDSLBind->_info.descriptorCount; - if ( !_mvkMTLBufferAllocation ) { _mvkMTLBufferAllocation = mvkDescSet->acquireMTLBufferRegion(buffSize); } - - uint8_t* data = getData(); - const auto& pInlineUniformBlock = *(VkWriteDescriptorSetInlineUniformBlockEXT*)pData; - if (data && pInlineUniformBlock.pData && dstOffset < buffSize) { - uint32_t dataLen = std::min(pInlineUniformBlock.dataSize, buffSize - dstOffset); - memcpy(data + dstOffset, pInlineUniformBlock.pData, dataLen); + if ( !_mvkMTLBufferAllocation ) { _mvkMTLBufferAllocation = mvkDescSet->acquireMTLBufferRegion(dstBuffSize); } + + uint8_t* pDstData = getData(); + uint8_t* pSrcData = (uint8_t*)pInlineUniformBlock->pData; + if (pDstData && pSrcData && dataLen) { + memcpy(pDstData + dstOffset, pSrcData + srcOffset, dataLen); } // Write resource to Metal argument buffer @@ -923,21 +928,29 @@ void mvkPopulateShaderConversionConfig(mvk::SPIRVToMSLConversionConfiguration& s _mvkMTLBufferAllocation ? _mvkMTLBufferAllocation->_offset : 0, argIdx); } + + return dataLen; } -void MVKInlineUniformBlockDescriptor::read(MVKDescriptorSetLayoutBinding* mvkDSLBind, - MVKDescriptorSet* mvkDescSet, - uint32_t srcOffset, - VkDescriptorImageInfo* pImageInfo, - VkDescriptorBufferInfo* pBufferInfo, - VkBufferView* pTexelBufferView, - VkWriteDescriptorSetInlineUniformBlockEXT* pInlineUniformBlock) { - uint8_t* data = getData(); - uint32_t buffSize = mvkDSLBind->_info.descriptorCount; - if (data && pInlineUniformBlock->pData && srcOffset < buffSize) { - uint32_t dataLen = std::min(pInlineUniformBlock->dataSize, buffSize - srcOffset); - memcpy((void*)pInlineUniformBlock->pData, data + srcOffset, dataLen); +uint32_t MVKInlineUniformBlockDescriptor::readBytes(MVKDescriptorSetLayoutBinding* mvkDSLBind, + MVKDescriptorSet* mvkDescSet, + uint32_t dstOffset, + uint32_t srcOffset, + uint32_t byteCount, + const VkWriteDescriptorSetInlineUniformBlockEXT* pInlineUniformBlock) { + uint32_t dataLen = 0; + uint32_t dstBuffSize = pInlineUniformBlock->dataSize; + uint32_t srcBuffSize = mvkDSLBind->_info.descriptorCount; + if (dstOffset < dstBuffSize && srcOffset < srcBuffSize) { + dataLen = std::min({ byteCount, dstBuffSize - dstOffset, srcBuffSize - srcOffset }); + } + + uint8_t* pDstData = (uint8_t*)pInlineUniformBlock->pData; + uint8_t* pSrcData = getData(); + if (pDstData && pSrcData && dataLen) { + memcpy(pDstData + dstOffset, pSrcData + srcOffset, dataLen); } + return dataLen; } void MVKInlineUniformBlockDescriptor::encodeResourceUsage(MVKResourcesCommandEncoderState* rezEncState, diff --git a/MoltenVK/MoltenVK/GPUObjects/MVKDescriptorSet.h b/MoltenVK/MoltenVK/GPUObjects/MVKDescriptorSet.h index 62a2e7b0d..e750e588c 100644 --- a/MoltenVK/MoltenVK/GPUObjects/MVKDescriptorSet.h +++ b/MoltenVK/MoltenVK/GPUObjects/MVKDescriptorSet.h @@ -129,7 +129,7 @@ class MVKDescriptorSetLayout : public MVKVulkanAPIDeviceObject { void propagateDebugName() override {} uint32_t getDescriptorCount(uint32_t variableDescriptorCount); uint32_t getDescriptorIndex(uint32_t binding, uint32_t elementIndex = 0) { return getBinding(binding)->getDescriptorIndex(elementIndex); } - MVKDescriptorSetLayoutBinding* getBinding(uint32_t binding) { return &_bindings[_bindingToIndex[binding]]; } + MVKDescriptorSetLayoutBinding* getBinding(uint32_t binding, uint32_t bindingIndexOffset = 0); uint32_t getBufferSizeBufferArgBuferIndex() { return 0; } id getMTLArgumentEncoder(uint32_t variableDescriptorCount); uint64_t getMetal3ArgumentBufferEncodedLength(uint32_t variableDescriptorCount); diff --git a/MoltenVK/MoltenVK/GPUObjects/MVKDescriptorSet.mm b/MoltenVK/MoltenVK/GPUObjects/MVKDescriptorSet.mm index 1c643e467..4dcb1d89d 100644 --- a/MoltenVK/MoltenVK/GPUObjects/MVKDescriptorSet.mm +++ b/MoltenVK/MoltenVK/GPUObjects/MVKDescriptorSet.mm @@ -350,6 +350,17 @@ static void populateAuxBuffer(mvk::SPIRVToMSLConversionConfiguration& shaderConf return descCnt; } +MVKDescriptorSetLayoutBinding* MVKDescriptorSetLayout::getBinding(uint32_t binding, uint32_t bindingIndexOffset) { + auto itr = _bindingToIndex.find(binding); + if (itr != _bindingToIndex.end()) { + uint32_t bindIdx = itr->second + bindingIndexOffset; + if (bindIdx < _bindings.size()) { + return &_bindings[bindIdx]; + } + } + return nullptr; +} + MVKDescriptorSetLayout::MVKDescriptorSetLayout(MVKDevice* device, const VkDescriptorSetLayoutCreateInfo* pCreateInfo) : MVKVulkanAPIDeviceObject(device) { @@ -430,21 +441,32 @@ static void populateAuxBuffer(mvk::SPIRVToMSLConversionConfiguration& shaderConf size_t srcStride, const void* pData) { - MVKDescriptorSetLayoutBinding* mvkDSLBind = _layout->getBinding(pDescriptorAction->dstBinding); - VkDescriptorType descType = mvkDSLBind->getDescriptorType(); - if (descType == VK_DESCRIPTOR_TYPE_INLINE_UNIFORM_BLOCK_EXT) { - // For inline buffers dstArrayElement is a byte offset - getDescriptor(pDescriptorAction->dstBinding)->write(mvkDSLBind, this, pDescriptorAction->dstArrayElement, 0, srcStride, pData); + auto* mvkDSLBind = _layout->getBinding(pDescriptorAction->dstBinding); + if (mvkDSLBind->getDescriptorType() == VK_DESCRIPTOR_TYPE_INLINE_UNIFORM_BLOCK_EXT) { + // For inline buffers, descriptorCount is a byte count and dstArrayElement is a byte offset. + // If needed, Vulkan allows updates to extend into subsequent bindings that are of the same type, + // so iterate layout bindings and their associated descriptors, until all bytes are updated. + const auto* pInlineUniformBlock = (VkWriteDescriptorSetInlineUniformBlockEXT*)pData; + uint32_t numBytesToCopy = pDescriptorAction->descriptorCount; + uint32_t dstOffset = pDescriptorAction->dstArrayElement; + uint32_t srcOffset = 0; + while (mvkDSLBind && numBytesToCopy > 0 && srcOffset < pInlineUniformBlock->dataSize) { + auto* mvkDesc = (MVKInlineUniformBlockDescriptor*)_descriptors[mvkDSLBind->_descriptorIndex]; + auto numBytesMoved = mvkDesc->writeBytes(mvkDSLBind, this, dstOffset, srcOffset, numBytesToCopy, pInlineUniformBlock); + numBytesToCopy -= numBytesMoved; + dstOffset = 0; + srcOffset += numBytesMoved; + mvkDSLBind = _layout->getBinding(mvkDSLBind->getBinding(), 1); // Next binding if needed + } } else { - uint32_t dslBindDescCnt = mvkDSLBind->getDescriptorCount(_variableDescriptorCount); + // We don't test against the descriptor count of the binding, because Vulkan allows + // updates to extend into subsequent bindings that are of the same type, if needed. uint32_t srcElemIdx = 0; uint32_t dstElemIdx = pDescriptorAction->dstArrayElement; - uint32_t descIdx = _layout->getDescriptorIndex(pDescriptorAction->dstBinding, dstElemIdx); - if (dstElemIdx < dslBindDescCnt) { - uint32_t elemCnt = std::min(pDescriptorAction->descriptorCount, dslBindDescCnt - dstElemIdx); - while (srcElemIdx < elemCnt) { - _descriptors[descIdx++]->write(mvkDSLBind, this, dstElemIdx++, srcElemIdx++, srcStride, pData); - } + uint32_t descIdx = _layout->getDescriptorIndex(pDescriptorAction->dstBinding, dstElemIdx); + uint32_t descCnt = pDescriptorAction->descriptorCount; + while (srcElemIdx < descCnt) { + _descriptors[descIdx++]->write(mvkDSLBind, this, dstElemIdx++, srcElemIdx++, srcStride, pData); } } } @@ -458,18 +480,29 @@ static void populateAuxBuffer(mvk::SPIRVToMSLConversionConfiguration& shaderConf MVKDescriptorSetLayoutBinding* mvkDSLBind = _layout->getBinding(pDescriptorCopy->srcBinding); VkDescriptorType descType = mvkDSLBind->getDescriptorType(); if (descType == VK_DESCRIPTOR_TYPE_INLINE_UNIFORM_BLOCK_EXT) { - // For inline buffers srcArrayElement is a byte offset - getDescriptor(pDescriptorCopy->srcBinding)->read(mvkDSLBind, this, pDescriptorCopy->srcArrayElement, pImageInfo, pBufferInfo, pTexelBufferView, pInlineUniformBlock); + // For inline buffers, descriptorCount is a byte count and dstArrayElement is a byte offset. + // If needed, Vulkan allows updates to extend into subsequent bindings that are of the same type, + // so iterate layout bindings and their associated descriptors, until all bytes are updated. + uint32_t numBytesToCopy = pDescriptorCopy->descriptorCount; + uint32_t dstOffset = 0; + uint32_t srcOffset = pDescriptorCopy->srcArrayElement; + while (mvkDSLBind && numBytesToCopy > 0 && dstOffset < pInlineUniformBlock->dataSize) { + auto* mvkDesc = (MVKInlineUniformBlockDescriptor*)_descriptors[mvkDSLBind->_descriptorIndex]; + auto numBytesMoved = mvkDesc->readBytes(mvkDSLBind, this, dstOffset, srcOffset, numBytesToCopy, pInlineUniformBlock); + numBytesToCopy -= numBytesMoved; + dstOffset += numBytesMoved; + srcOffset = 0; + mvkDSLBind = _layout->getBinding(mvkDSLBind->getBinding(), 1); // Next binding if needed + } } else { - uint32_t dslBindDescCnt = mvkDSLBind->getDescriptorCount(_variableDescriptorCount); - uint32_t dstElemIdx = 0; + // We don't test against the descriptor count of the binding, because Vulkan allows + // updates to extend into subsequent bindings that are of the same type, if needed. uint32_t srcElemIdx = pDescriptorCopy->srcArrayElement; - uint32_t descIdx = _layout->getDescriptorIndex(pDescriptorCopy->srcBinding, srcElemIdx); - if (srcElemIdx < dslBindDescCnt) { - uint32_t elemCnt = std::min(pDescriptorCopy->descriptorCount, dslBindDescCnt - srcElemIdx); - while (dstElemIdx < elemCnt) { - _descriptors[descIdx++]->read(mvkDSLBind, this, dstElemIdx++, pImageInfo, pBufferInfo, pTexelBufferView, pInlineUniformBlock); - } + uint32_t dstElemIdx = 0; + uint32_t descIdx = _layout->getDescriptorIndex(pDescriptorCopy->srcBinding, srcElemIdx); + uint32_t descCnt = pDescriptorCopy->descriptorCount; + while (dstElemIdx < descCnt) { + _descriptors[descIdx++]->read(mvkDSLBind, this, dstElemIdx++, pImageInfo, pBufferInfo, pTexelBufferView, pInlineUniformBlock); } } }