diff --git a/filament/backend/src/vulkan/VulkanDriver.cpp b/filament/backend/src/vulkan/VulkanDriver.cpp index 4cfac1f149..e1cd3d7cf7 100644 --- a/filament/backend/src/vulkan/VulkanDriver.cpp +++ b/filament/backend/src/vulkan/VulkanDriver.cpp @@ -346,6 +346,7 @@ void VulkanDriver::terminate() { mFramebufferCache.reset(); mSamplerCache.terminate(); mDescriptorSetManager.terminate(); + mPipelineLayoutCache.terminate(); #if FVK_ENABLED(FVK_DEBUG_RESOURCE_LEAK) mResourceAllocator.print(); diff --git a/filament/backend/src/vulkan/VulkanHandles.cpp b/filament/backend/src/vulkan/VulkanHandles.cpp index 41f02212c8..9d615ce515 100644 --- a/filament/backend/src/vulkan/VulkanHandles.cpp +++ b/filament/backend/src/vulkan/VulkanHandles.cpp @@ -120,8 +120,30 @@ void addDescriptors(Bitmask mask, } } +inline VkDescriptorSetLayout createDescriptorSetLayout(VkDevice device, + VkDescriptorSetLayoutCreateInfo const& info) { + VkDescriptorSetLayout layout; + vkCreateDescriptorSetLayout(device, &info, VKALLOC, &layout); + return layout; +} + } // anonymous namespace + +VulkanDescriptorSetLayout::VulkanDescriptorSetLayout(VkDevice device, VkDescriptorSetLayoutCreateInfo const& info, + Bitmask const& bitmask) + : VulkanResource(VulkanResourceType::DESCRIPTOR_SET_LAYOUT), + mDevice(device), + vklayout(createDescriptorSetLayout(device, info)), + bitmask(bitmask), + bindings(getBindings(bitmask)), + count(Count::fromLayoutBitmask(bitmask)) { +} + +VulkanDescriptorSetLayout::~VulkanDescriptorSetLayout() { + vkDestroyDescriptorSetLayout(mDevice, vklayout, VKALLOC); +} + VulkanProgram::VulkanProgram(VkDevice device, Program const& builder) noexcept : HwProgram(builder.getName()), VulkanResource(VulkanResourceType::PROGRAM), diff --git a/filament/backend/src/vulkan/VulkanHandles.h b/filament/backend/src/vulkan/VulkanHandles.h index e41112c125..8abdccfd96 100644 --- a/filament/backend/src/vulkan/VulkanHandles.h +++ b/filament/backend/src/vulkan/VulkanHandles.h @@ -99,13 +99,12 @@ struct VulkanDescriptorSetLayout : public VulkanResource { static_assert(sizeof(Bitmask) % 8 == 0); - explicit VulkanDescriptorSetLayout(VkDescriptorSetLayout layout, Bitmask const& bitmask) - : VulkanResource(VulkanResourceType::DESCRIPTOR_SET_LAYOUT), - vklayout(layout), - bitmask(bitmask), - bindings(getBindings(bitmask)), - count(Count::fromLayoutBitmask(bitmask)) {} + explicit VulkanDescriptorSetLayout(VkDevice device, VkDescriptorSetLayoutCreateInfo const& info, + Bitmask const& bitmask); + ~VulkanDescriptorSetLayout(); + + VkDevice const mDevice; VkDescriptorSetLayout const vklayout; Bitmask const bitmask; diff --git a/filament/backend/src/vulkan/VulkanPipelineCache.cpp b/filament/backend/src/vulkan/VulkanPipelineCache.cpp index 561998312f..7cfc74162e 100644 --- a/filament/backend/src/vulkan/VulkanPipelineCache.cpp +++ b/filament/backend/src/vulkan/VulkanPipelineCache.cpp @@ -289,7 +289,6 @@ void VulkanPipelineCache::bindVertexArray(VkVertexInputAttributeDescription cons } void VulkanPipelineCache::terminate() noexcept { - // Symmetric to createLayoutsAndDescriptors. for (auto& iter : mPipelines) { vkDestroyPipeline(mDevice, iter.second.handle, VKALLOC); } diff --git a/filament/backend/src/vulkan/caching/VulkanDescriptorSetManager.cpp b/filament/backend/src/vulkan/caching/VulkanDescriptorSetManager.cpp index 7615bdae15..cad4339a1b 100644 --- a/filament/backend/src/vulkan/caching/VulkanDescriptorSetManager.cpp +++ b/filament/backend/src/vulkan/caching/VulkanDescriptorSetManager.cpp @@ -80,7 +80,8 @@ public: mCount(count), mCapacity(capacity), mSize(0), - mUnusedCount(0) { + mUnusedCount(0), + mDisableRecycling(false) { DescriptorCount const actual = mCount * capacity; VkDescriptorPoolSize sizes[4]; uint8_t npools = 0; @@ -123,9 +124,19 @@ public: DescriptorPool& operator=(DescriptorPool const&) = delete; ~DescriptorPool() { + // Note that these have to manually destroyed because they were not explicitly ref-counted. + for (auto const& [mask, sets]: mUnused) { + for (auto set: sets) { + mAllocator->destruct(set); + } + } vkDestroyDescriptorPool(mDevice, mPool, VKALLOC); } + void disableRecycling() noexcept { + mDisableRecycling = true; + } + uint16_t const& capacity() { return mCapacity; } @@ -172,6 +183,9 @@ private: Handle createSet(Bitmask const& layoutMask, VkDescriptorSet vkSet) { return mAllocator->initHandle(mAllocator, vkSet, [this, layoutMask, vkSet]() { + if (mDisableRecycling) { + return; + } // We are recycling - release the set back into the pool. Note that the // vk handle has not changed, but we need to change the backend handle to allow // for proper refcounting of resources referenced in this set. @@ -200,6 +214,8 @@ private: using UnusedSetMap = std::unordered_map>, BitmaskHashFn, BitmaskEqual>; UnusedSetMap mUnused; + + bool mDisableRecycling; }; // This is an ever-expanding pool of sets where it @@ -244,6 +260,12 @@ public: return ret; } + void disableRecycling() noexcept { + for (auto& pool: mPools) { + pool->disableRecycling(); + } + } + private: VkDevice mDevice; VulkanResourceAllocator* mAllocator; @@ -267,21 +289,19 @@ public: ~LayoutCache() { for (auto [key, layout]: mLayouts) { - auto layoutPtr = mAllocator->handle_cast(layout); - vkDestroyDescriptorSetLayout(mDevice, layoutPtr->vklayout, VKALLOC); + mAllocator->destruct(layout); } mLayouts.clear(); } void destroyLayout(Handle handle) { - auto layoutPtr = mAllocator->handle_cast(handle); for (auto [key, layout]: mLayouts) { if (layout == handle) { mLayouts.erase(key); break; } } - vkDestroyDescriptorSetLayout(mDevice, layoutPtr->vklayout, VKALLOC); + mAllocator->destruct(handle); } Handle getLayout(descset::DescriptorSetLayout const& layout) { @@ -339,10 +359,8 @@ public: .bindingCount = count, .pBindings = toBind, }; - - VkDescriptorSetLayout outLayout; - vkCreateDescriptorSetLayout(mDevice, &dlinfo, VKALLOC, &outLayout); - return (mLayouts[key] = mAllocator->initHandle(outLayout, key)); + return (mLayouts[key] = + mAllocator->initHandle(mDevice, dlinfo, key)); } private: @@ -627,29 +645,40 @@ class DescriptorSetCache { public: DescriptorSetCache(VkDevice device, VulkanResourceAllocator* allocator) : mAllocator(allocator), - mDescriptorPool(device, allocator), - mUBOCache(allocator), - mSamplerCache(allocator), - mInputAttachmentCache(allocator) {} + mDescriptorPool(std::make_unique(device, allocator)), + mUBOCache(std::make_unique>(allocator)), + mSamplerCache(std::make_unique>(allocator)), + mInputAttachmentCache( + std::make_unique>(allocator)) {} template inline std::pair get(Key const& key, VulkanDescriptorSetLayout* layout) { if constexpr (std::is_same_v) { - return get(key, mUBOCache, layout); + return get(key, *mUBOCache, layout); } else if constexpr (std::is_same_v) { - return get(key, mSamplerCache, layout); + return get(key, *mSamplerCache, layout); } else if constexpr (std::is_same_v) { - return get(key, mInputAttachmentCache, layout); + return get(key, *mInputAttachmentCache, layout); } PANIC_POSTCONDITION("Unexpected key type"); } + ~DescriptorSetCache() { + // This will prevent the descriptor sets recycling when we destroy descriptor set caches. + mDescriptorPool->disableRecycling(); + + mInputAttachmentCache.reset(); + mSamplerCache.reset(); + mUBOCache.reset(); + mDescriptorPool.reset(); + } + // gc() should be called at the end of everyframe void gc() { - mUBOCache.gc(); - mSamplerCache.gc(); - mInputAttachmentCache.gc(); + mUBOCache->gc(); + mSamplerCache->gc(); + mInputAttachmentCache->gc(); } private: @@ -660,16 +689,18 @@ private: return {set, true}; } auto set = mAllocator->handle_cast( - mDescriptorPool.obtainSet(layout)); + mDescriptorPool->obtainSet(layout)); cache.put(key, set); return {set, false}; } VulkanResourceAllocator* mAllocator; - DescriptorInfinitePool mDescriptorPool; - LRUDescriptorSetCache mUBOCache; - LRUDescriptorSetCache mSamplerCache; - LRUDescriptorSetCache mInputAttachmentCache; + + // We need to heap-allocate so that the destruction can be strictly ordered. + std::unique_ptr mDescriptorPool; + std::unique_ptr> mUBOCache; + std::unique_ptr> mSamplerCache; + std::unique_ptr> mInputAttachmentCache; }; } // anonymous namespace @@ -1095,20 +1126,14 @@ private: VulkanResourceAllocator* mAllocator; LayoutCache mLayoutCache; DescriptorSetCache mDescriptorSetCache; - bool mHaveDynamicUbos; - UBOMap mUboMap; SamplerMap mSamplerMap; std::pair mInputAttachment; - VulkanResourceManager mResources; - VkDescriptorBufferInfo mPlaceHolderBufferInfo; VkDescriptorImageInfo mPlaceHolderImageInfo; - std::unordered_map mLayoutStash; - BoundState mBoundState; VulkanDescriptorSetLayoutList mPlaceholderLayout = {}; }; diff --git a/filament/backend/src/vulkan/caching/VulkanPipelineLayoutCache.cpp b/filament/backend/src/vulkan/caching/VulkanPipelineLayoutCache.cpp index ea86f0712a..ec96418aaf 100644 --- a/filament/backend/src/vulkan/caching/VulkanPipelineLayoutCache.cpp +++ b/filament/backend/src/vulkan/caching/VulkanPipelineLayoutCache.cpp @@ -55,4 +55,10 @@ VkPipelineLayout VulkanPipelineLayoutCache::getLayout( return layout; } +void VulkanPipelineLayoutCache::terminate() noexcept { + for (auto const& [key, entry]: mPipelineLayouts) { + vkDestroyPipelineLayout(mDevice, entry.handle, VKALLOC); + } +} + }// namespace filament::backend diff --git a/filament/backend/src/vulkan/caching/VulkanPipelineLayoutCache.h b/filament/backend/src/vulkan/caching/VulkanPipelineLayoutCache.h index fff9fd2d22..375e6124d2 100644 --- a/filament/backend/src/vulkan/caching/VulkanPipelineLayoutCache.h +++ b/filament/backend/src/vulkan/caching/VulkanPipelineLayoutCache.h @@ -33,6 +33,8 @@ public: mAllocator(allocator), mTimestamp(0) {} + void terminate() noexcept; + using PipelineLayoutKey = std::array;