diff --git a/Demos/LunarG-VulkanSamples/Cube/macOS/Resources/Main.storyboard b/Demos/LunarG-VulkanSamples/Cube/macOS/Resources/Main.storyboard index d21c1493..a137a9df 100644 --- a/Demos/LunarG-VulkanSamples/Cube/macOS/Resources/Main.storyboard +++ b/Demos/LunarG-VulkanSamples/Cube/macOS/Resources/Main.storyboard @@ -1,8 +1,9 @@ - - + + - + + @@ -100,11 +101,14 @@ - + - + - + + + + @@ -119,13 +123,13 @@ - + - + diff --git a/Docs/Whats_New.md b/Docs/Whats_New.md index 2df61a3c..1b80ce41 100644 --- a/Docs/Whats_New.md +++ b/Docs/Whats_New.md @@ -12,6 +12,14 @@ Copyright (c) 2014-2018 [The Brenwill Workshop Ltd.](http://www.brenwill.com) For best results, use a Markdown reader.* +MoltenVK 1.0.24 +--------------- + +Released TBD + +- Allocate MVKDescriptorSets from a pool within MVKDescriptorPool, + + MoltenVK 1.0.23 --------------- diff --git a/MoltenVK/MoltenVK.xcodeproj/project.pbxproj b/MoltenVK/MoltenVK.xcodeproj/project.pbxproj index 5c7e63f4..a322fdb8 100644 --- a/MoltenVK/MoltenVK.xcodeproj/project.pbxproj +++ b/MoltenVK/MoltenVK.xcodeproj/project.pbxproj @@ -494,8 +494,8 @@ A98149411FB6A3F7005F00B4 /* MVKBaseObject.cpp */, A98149421FB6A3F7005F00B4 /* MVKBaseObject.h */, A98149431FB6A3F7005F00B4 /* MVKEnvironment.h */, - A98149441FB6A3F7005F00B4 /* MVKFoundation.h */, A98149451FB6A3F7005F00B4 /* MVKFoundation.cpp */, + A98149441FB6A3F7005F00B4 /* MVKFoundation.h */, A98149461FB6A3F7005F00B4 /* MVKObjectPool.h */, A98149491FB6A3F7005F00B4 /* MVKWatermark.h */, A981494A1FB6A3F7005F00B4 /* MVKWatermark.mm */, diff --git a/MoltenVK/MoltenVK/Commands/MVKCommand.h b/MoltenVK/MoltenVK/Commands/MVKCommand.h index 00ba38a4..f0806384 100644 --- a/MoltenVK/MoltenVK/Commands/MVKCommand.h +++ b/MoltenVK/MoltenVK/Commands/MVKCommand.h @@ -77,8 +77,8 @@ public: /** * Instances of this class can participate in a linked list or pool. When so participating, - * this is a reference to the next instance in the linked list. This value should only be - * managed and set by the linked list. + * this is a reference to the next instance in the list or pool. This value should only be + * managed and set by the list or pool. */ MVKCommand* _next = nullptr; diff --git a/MoltenVK/MoltenVK/Commands/MVKCommandBuffer.h b/MoltenVK/MoltenVK/Commands/MVKCommandBuffer.h index c28a3989..7560d4c6 100644 --- a/MoltenVK/MoltenVK/Commands/MVKCommandBuffer.h +++ b/MoltenVK/MoltenVK/Commands/MVKCommandBuffer.h @@ -91,8 +91,8 @@ public: /** * Instances of this class can participate in a linked list or pool. When so participating, - * this is a reference to the next instance in the linked list. This value should only be - * managed and set by the linked list. + * this is a reference to the next instance in the list or pool. This value should only be + * managed and set by the list or pool. */ MVKCommandBuffer* _next; @@ -127,13 +127,13 @@ protected: void prefill(); void clearPrefilledMTLCommandBuffer(); - MVKCommand* _head; - MVKCommand* _tail; + MVKCommand* _head = nullptr; + MVKCommand* _tail = nullptr; uint32_t _commandCount; std::atomic_flag _isExecutingNonConcurrently; VkResult _recordingResult; VkCommandBufferInheritanceInfo _secondaryInheritanceInfo; - id _prefilledMTLCmdBuffer; + id _prefilledMTLCmdBuffer = nil; bool _isSecondary; bool _doesContinueRenderPass; bool _canAcceptCommands; @@ -143,33 +143,6 @@ protected: }; -#pragma mark - -#pragma mark MVKCommandBufferPool - -/** - * A pool of MVKCommandBuffer instances. - * - * To return a MVKCommandBuffer retrieved from this pool, back to this pool, - * call the returnToPool() function on the MVKCommandBuffer instance. - */ -class MVKCommandBufferPool : public MVKObjectPool { - -public: - - /** Returns a new command instance. */ - MVKCommandBuffer* newObject() override { return new MVKCommandBuffer(_device); } - - /** - * Configures this instance to either use pooling, or not, depending on the - * value of isPooling, which defaults to true if not indicated explicitly. - */ - MVKCommandBufferPool(MVKDevice* device, bool isPooling = true) : MVKObjectPool(isPooling), _device(device) {} - -protected: - MVKDevice* _device; -}; - - #pragma mark - #pragma mark MVKCommandEncoder diff --git a/MoltenVK/MoltenVK/Commands/MVKCommandBuffer.mm b/MoltenVK/MoltenVK/Commands/MVKCommandBuffer.mm index 7c2f2327..39536374 100644 --- a/MoltenVK/MoltenVK/Commands/MVKCommandBuffer.mm +++ b/MoltenVK/MoltenVK/Commands/MVKCommandBuffer.mm @@ -179,9 +179,6 @@ void MVKCommandBuffer::clearPrefilledMTLCommandBuffer() { void MVKCommandBuffer::init(const VkCommandBufferAllocateInfo* pAllocateInfo) { _commandPool = (MVKCommandPool*)pAllocateInfo->commandPool; _isSecondary = (pAllocateInfo->level == VK_COMMAND_BUFFER_LEVEL_SECONDARY); - _head = nullptr; - _tail = nullptr; - _prefilledMTLCmdBuffer = nil; reset(0); } diff --git a/MoltenVK/MoltenVK/Commands/MVKCommandPool.h b/MoltenVK/MoltenVK/Commands/MVKCommandPool.h index 5ded1d5e..8e5e850d 100644 --- a/MoltenVK/MoltenVK/Commands/MVKCommandPool.h +++ b/MoltenVK/MoltenVK/Commands/MVKCommandPool.h @@ -171,10 +171,8 @@ public: ~MVKCommandPool() override; protected: - void freeCommandBuffer(MVKCommandBuffer* mvkCmdBuff); - - MVKCommandBufferPool _commandBufferPool; - std::unordered_set _commandBuffers; + MVKDeviceObjectPool _commandBufferPool; + std::unordered_set _allocatedCommandBuffers; MVKCommandEncodingPool _commandEncodingPool; uint32_t _queueFamilyIndex; }; diff --git a/MoltenVK/MoltenVK/Commands/MVKCommandPool.mm b/MoltenVK/MoltenVK/Commands/MVKCommandPool.mm index 559b3489..514dd78c 100644 --- a/MoltenVK/MoltenVK/Commands/MVKCommandPool.mm +++ b/MoltenVK/MoltenVK/Commands/MVKCommandPool.mm @@ -37,7 +37,7 @@ VkResult MVKCommandPool::reset(VkCommandPoolResetFlags flags) { VkCommandBufferResetFlags cmdBuffFlags = releaseRez ? VK_COMMAND_BUFFER_RESET_RELEASE_RESOURCES_BIT : 0; - for (auto& cb : _commandBuffers) { cb->reset(cmdBuffFlags); } + for (auto& cb : _allocatedCommandBuffers) { cb->reset(cmdBuffFlags); } if (releaseRez) { trim(); } @@ -54,7 +54,7 @@ VkResult MVKCommandPool::allocateCommandBuffers(const VkCommandBufferAllocateInf for (uint32_t cbIdx = 0; cbIdx < cbCnt; cbIdx++) { MVKCommandBuffer* mvkCmdBuff = _commandBufferPool.acquireObject(); mvkCmdBuff->init(pAllocateInfo); - _commandBuffers.insert(mvkCmdBuff); + _allocatedCommandBuffers.insert(mvkCmdBuff); pCmdBuffer[cbIdx] = mvkCmdBuff->getVkCommandBuffer(); if (rslt == VK_SUCCESS) { rslt = mvkCmdBuff->getConfigurationResult(); } } @@ -64,18 +64,14 @@ VkResult MVKCommandPool::allocateCommandBuffers(const VkCommandBufferAllocateInf void MVKCommandPool::freeCommandBuffers(uint32_t commandBufferCount, const VkCommandBuffer* pCommandBuffers) { for (uint32_t cbIdx = 0; cbIdx < commandBufferCount; cbIdx++) { - freeCommandBuffer(MVKCommandBuffer::getMVKCommandBuffer(pCommandBuffers[cbIdx])); + MVKCommandBuffer* mvkCmdBuff = MVKCommandBuffer::getMVKCommandBuffer(pCommandBuffers[cbIdx]); + if (_allocatedCommandBuffers.erase(mvkCmdBuff)) { + mvkCmdBuff->reset(VK_COMMAND_BUFFER_RESET_RELEASE_RESOURCES_BIT); + _commandBufferPool.returnObject(mvkCmdBuff); + } } } -void MVKCommandPool::freeCommandBuffer(MVKCommandBuffer* mvkCmdBuff) { - if ( !mvkCmdBuff ) { return; } - - mvkCmdBuff->reset(VK_COMMAND_BUFFER_RESET_RELEASE_RESOURCES_BIT); - _commandBuffers.erase(mvkCmdBuff); - _commandBufferPool.returnObject(mvkCmdBuff); -} - id MVKCommandPool::newMTLCommandBuffer(uint32_t queueIndex) { return [[_device->getQueue(_queueFamilyIndex, queueIndex)->getMTLCommandQueue() commandBuffer] retain]; } @@ -130,7 +126,8 @@ void MVKCommandPool::trim() { #pragma mark Construction MVKCommandPool::MVKCommandPool(MVKDevice* device, - const VkCommandPoolCreateInfo* pCreateInfo) : MVKBaseDeviceObject(device), + const VkCommandPoolCreateInfo* pCreateInfo) : + MVKBaseDeviceObject(device), _commandBufferPool(device), _commandEncodingPool(device), _queueFamilyIndex(pCreateInfo->queueFamilyIndex), @@ -178,7 +175,8 @@ MVKCommandPool::MVKCommandPool(MVKDevice* device, {} MVKCommandPool::~MVKCommandPool() { - auto cmdBuffs = _commandBuffers; - for (auto& cb : cmdBuffs) { freeCommandBuffer(cb); } + for (auto& mvkCB : _allocatedCommandBuffers) { + _commandBufferPool.returnObject(mvkCB); + } } diff --git a/MoltenVK/MoltenVK/Commands/MVKMTLBufferAllocation.h b/MoltenVK/MoltenVK/Commands/MVKMTLBufferAllocation.h index d3158639..8cfdb50d 100644 --- a/MoltenVK/MoltenVK/Commands/MVKMTLBufferAllocation.h +++ b/MoltenVK/MoltenVK/Commands/MVKMTLBufferAllocation.h @@ -55,8 +55,8 @@ public: /** * Instances of this class can participate in a linked list or pool. When so participating, - * this is a reference to the next instance in the linked list. This value should only be - * managed and set by the linked list. + * this is a reference to the next instance in the list or pool. This value should only be + * managed and set by the list or pool. */ MVKMTLBufferAllocation* _next = nullptr; diff --git a/MoltenVK/MoltenVK/GPUObjects/MVKDescriptorSet.h b/MoltenVK/MoltenVK/GPUObjects/MVKDescriptorSet.h index 3b7efb15..d2c0f793 100644 --- a/MoltenVK/MoltenVK/GPUObjects/MVKDescriptorSet.h +++ b/MoltenVK/MoltenVK/GPUObjects/MVKDescriptorSet.h @@ -21,7 +21,8 @@ #include "MVKDevice.h" #include "MVKImage.h" #include -#include +#include +#include #include using namespace mvk; @@ -274,14 +275,23 @@ public: VkDescriptorBufferInfo* pBufferInfo, VkBufferView* pTexelBufferView); - /** Constructs an instance for the specified device. */ - MVKDescriptorSet(MVKDevice* device, MVKDescriptorSetLayout* layout); + /** + * Instances of this class can participate in a linked list or pool. When so participating, + * this is a reference to the next instance in the list or pool. This value should only be + * managed and set by the list or pool. + */ + MVKDescriptorSet* _next; + + MVKDescriptorSet(MVKDevice* device) : MVKBaseDeviceObject(device) {} protected: friend class MVKDescriptorSetLayout; + friend class MVKDescriptorPool; + void setLayout(MVKDescriptorSetLayout* layout); MVKDescriptorBinding* getBinding(uint32_t binding); + MVKDescriptorSetLayout* _pLayout = nullptr; std::vector _bindings; }; @@ -289,6 +299,8 @@ protected: #pragma mark - #pragma mark MVKDescriptorPool +typedef MVKDeviceObjectPool MVKDescriptorSetPool; + /** Represents a Vulkan descriptor pool. */ class MVKDescriptorPool : public MVKBaseDeviceObject { @@ -312,9 +324,11 @@ public: ~MVKDescriptorPool() override; protected: - uint32_t _maxSets; - uint32_t _allocatedSetCount; - std::forward_list _allocatedSets; + MVKDescriptorSetPool* getDescriptorSetPool(MVKDescriptorSetLayout* mvkDescSetLayout); + + uint32_t _maxSets; + std::unordered_set _allocatedSets; + std::unordered_map _descriptorSetPools; }; diff --git a/MoltenVK/MoltenVK/GPUObjects/MVKDescriptorSet.mm b/MoltenVK/MoltenVK/GPUObjects/MVKDescriptorSet.mm index 346dfaa6..a8367e22 100644 --- a/MoltenVK/MoltenVK/GPUObjects/MVKDescriptorSet.mm +++ b/MoltenVK/MoltenVK/GPUObjects/MVKDescriptorSet.mm @@ -374,9 +374,6 @@ void MVKDescriptorSetLayoutBinding::populateShaderConverterContext(SPIRVToMSLCon MVKDescriptorSetLayoutBinding::MVKDescriptorSetLayoutBinding(MVKDescriptorSetLayout* layout, const VkDescriptorSetLayoutBinding* pBinding) : MVKConfigurableObject() { - - // MVKLogDebug("Creating MVKDescriptorSetLayoutBinding binding %d.", pBinding->binding); - // Determine the shader stages used by this binding _applyToVertexStage = mvkAreFlagsEnabled(pBinding->stageFlags, VK_SHADER_STAGE_VERTEX_BIT); _applyToFragmentStage = mvkAreFlagsEnabled(pBinding->stageFlags, VK_SHADER_STAGE_FRAGMENT_BIT); @@ -790,17 +787,11 @@ void MVKDescriptorSet::writeDescriptorSets(const DescriptorAction* pDescriptorAc uint32_t origCnt = pDescriptorAction->descriptorCount; uint32_t remainCnt = origCnt; -// MVKLogDebug("Writing descriptor sets with buffer info %p starting at binding point %d.", pBufferInfo, binding); - MVKDescriptorBinding* mvkDescBind = getBinding(binding); while (mvkDescBind && remainCnt > 0) { - -// MVKLogDebug("Writing MVKDescriptorBinding with binding point %d.", binding); - uint32_t srcStartIdx = origCnt - remainCnt; remainCnt = mvkDescBind->writeBindings(srcStartIdx, dstStartIdx, remainCnt, stride, pData); - binding++; // If not consumed, move to next consecutive binding point mvkDescBind = getBinding(binding); dstStartIdx = 0; // Subsequent bindings start reading at first element @@ -826,17 +817,11 @@ void MVKDescriptorSet::readDescriptorSets(const VkCopyDescriptorSet* pDescriptor uint32_t origCnt = pDescriptorCopy->descriptorCount; uint32_t remainCnt = origCnt; -// MVKLogDebug("Reading descriptor sets with buffer info %p starting at binding point %d.", pBufferInfo, binding); - MVKDescriptorBinding* mvkDescBind = getBinding(binding); while (mvkDescBind && remainCnt > 0) { - -// MVKLogDebug("Reading MVKDescriptorBinding with binding point %d.", binding); - uint32_t dstStartIdx = origCnt - remainCnt; remainCnt = mvkDescBind->readBindings(srcStartIdx, dstStartIdx, remainCnt, descType, pImageInfo, pBufferInfo, pTexelBufferView); - binding++; // If not consumed, move to next consecutive binding point mvkDescBind = getBinding(binding); srcStartIdx = 0; // Subsequent bindings start reading at first element @@ -850,14 +835,17 @@ MVKDescriptorBinding* MVKDescriptorSet::getBinding(uint32_t binding) { return nullptr; } -MVKDescriptorSet::MVKDescriptorSet(MVKDevice* device, - MVKDescriptorSetLayout* layout) : MVKBaseDeviceObject(device) { +// If the layout has changed, create the binding slots, each referencing a corresponding binding layout +void MVKDescriptorSet::setLayout(MVKDescriptorSetLayout* layout) { + if (layout != _pLayout) { + _pLayout = layout; + uint32_t bindCnt = (uint32_t)layout->_bindings.size(); - // Create the binding slots, each referencing a corresponding binding layout - uint32_t bindCnt = (uint32_t)layout->_bindings.size(); - _bindings.reserve(bindCnt); - for (uint32_t i = 0; i < bindCnt; i++) { - _bindings.emplace_back(&layout->_bindings[i]); + _bindings.clear(); + _bindings.reserve(bindCnt); + for (uint32_t i = 0; i < bindCnt; i++) { + _bindings.emplace_back(&layout->_bindings[i]); + } } } @@ -868,8 +856,7 @@ MVKDescriptorSet::MVKDescriptorSet(MVKDevice* device, VkResult MVKDescriptorPool::allocateDescriptorSets(uint32_t count, const VkDescriptorSetLayout* pSetLayouts, VkDescriptorSet* pDescriptorSets) { -// MVKLogDebug("Descriptor pool %p allocating %d descriptor sets for total %d.", this, count, _allocatedSetCount + count); - if (_allocatedSetCount + count > _maxSets) { + if (_allocatedSets.size() + count > _maxSets) { if (_device->_enabledExtensions.vk_KHR_maintenance1.enabled) { return VK_ERROR_OUT_OF_POOL_MEMORY; // Failure is an acceptable test...don't log as error. } else { @@ -879,47 +866,55 @@ VkResult MVKDescriptorPool::allocateDescriptorSets(uint32_t count, for (uint32_t dsIdx = 0; dsIdx < count; dsIdx++) { MVKDescriptorSetLayout* mvkDSL = (MVKDescriptorSetLayout*)pSetLayouts[dsIdx]; - if (mvkDSL->isPushDescriptorLayout()) continue; - MVKDescriptorSet* mvkDescSet = new MVKDescriptorSet(_device, mvkDSL); - _allocatedSets.push_front(mvkDescSet); - pDescriptorSets[dsIdx] = (VkDescriptorSet)mvkDescSet; - _allocatedSetCount++; - } - return VK_SUCCESS; -} - -VkResult MVKDescriptorPool::freeDescriptorSets(uint32_t count, const VkDescriptorSet* pDescriptorSets) { -// MVKLogDebug("Descriptor pool %p freeing %d descriptor sets from total %d.", this, count, _allocatedSetCount); - for (uint32_t dsIdx = 0; dsIdx < count; dsIdx++) { - MVKDescriptorSet* mvkDS = (MVKDescriptorSet*)pDescriptorSets[dsIdx]; - if (mvkDS) { - _allocatedSets.remove(mvkDS); - _allocatedSetCount--; - mvkDS->destroy(); + if ( !mvkDSL->isPushDescriptorLayout() ) { + MVKDescriptorSet* mvkDS = getDescriptorSetPool(mvkDSL)->acquireObject(); + mvkDS->setLayout(mvkDSL); + _allocatedSets.insert(mvkDS); + pDescriptorSets[dsIdx] = (VkDescriptorSet)mvkDS; } } return VK_SUCCESS; } -VkResult MVKDescriptorPool::reset(VkDescriptorPoolResetFlags flags) { -// MVKLogDebug("Descriptor pool %p resetting with %d descriptor sets.", this, _allocatedSetCount); - mvkDestroyContainerContents(_allocatedSets); - _allocatedSetCount = 0; +// Ensure descriptor set was actually allocated, then return to pool +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); + } + } 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); + } + _allocatedSets.clear(); + return VK_SUCCESS; +} + +// 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; + } + return dsp; +} + MVKDescriptorPool::MVKDescriptorPool(MVKDevice* device, const VkDescriptorPoolCreateInfo* pCreateInfo) : MVKBaseDeviceObject(device) { _maxSets = pCreateInfo->maxSets; - _allocatedSetCount = 0; -// MVKLogDebug("Descriptor pool %p created with max %d sets.", this, _maxSets); } -// TODO: Destroying a descriptor pool implicitly destroys all descriptor sets created from it. - +// Return any allocated sets to their pools and then destroy all the pools. MVKDescriptorPool::~MVKDescriptorPool() { -// MVKLogDebug("Descriptor pool %p destroyed with %d descriptor sets.", this, _allocatedSetCount); reset(0); + for (auto& pair : _descriptorSetPools) { pair.second->destroy(); } } @@ -1019,5 +1014,3 @@ void mvkPopulateShaderConverterContext(SPIRVToMSLConverterContext& context, ctxRB.mslSampler = ssRB.samplerIndex; context.resourceBindings.push_back(ctxRB); } - - diff --git a/MoltenVK/MoltenVK/GPUObjects/MVKDevice.h b/MoltenVK/MoltenVK/GPUObjects/MVKDevice.h index bf05a93f..7bbc8eb3 100644 --- a/MoltenVK/MoltenVK/GPUObjects/MVKDevice.h +++ b/MoltenVK/MoltenVK/GPUObjects/MVKDevice.h @@ -21,6 +21,7 @@ #include "MVKFoundation.h" #include "MVKBaseObject.h" #include "MVKLayers.h" +#include "MVKObjectPool.h" #include "vk_mvk_moltenvk.h" #include #include @@ -631,3 +632,26 @@ protected: }; +#pragma mark - +#pragma mark MVKDeviceObjectPool + +/** Manages a pool of instances of a particular object type that requires an MVKDevice during construction. */ +template +class MVKDeviceObjectPool : public MVKObjectPool { + +public: + + /** Returns a new instance. */ + T* newObject() override { return new T(_device); } + + /** + * Configures this instance for the device, and either use pooling, or not, depending + * on the value of isPooling, which defaults to true if not indicated explicitly. + */ + MVKDeviceObjectPool(MVKDevice* device, bool isPooling = true) : MVKObjectPool(isPooling), _device(device) {} + +protected: + MVKDevice* _device; +}; + +