diff --git a/filament/backend/src/vulkan/VulkanDriver.cpp b/filament/backend/src/vulkan/VulkanDriver.cpp index cee5344436..957b2be066 100644 --- a/filament/backend/src/vulkan/VulkanDriver.cpp +++ b/filament/backend/src/vulkan/VulkanDriver.cpp @@ -395,6 +395,8 @@ void VulkanDriver::collectGarbage() { FVK_SYSTRACE_SCOPE(); // Command buffers need to be submitted and completed before other resources can be gc'd. mCommands.gc(); + + mExternalImageManager.gc(); mDescriptorSetCache.gc(); mStagePool.gc(); mBufferCache.gc(); @@ -417,7 +419,6 @@ void VulkanDriver::beginFrame(int64_t monotonic_clock_ns, // // This will let us check if any VulkanBuffer is currently in flight or not. mCommands.gc(); - if (mAppState.hasExternalSamplers()) { mExternalImageManager.onBeginFrame(); } @@ -2086,14 +2087,15 @@ void VulkanDriver::bindDescriptorSet( void VulkanDriver::draw2(uint32_t indexOffset, uint32_t indexCount, uint32_t instanceCount) { FVK_SYSTRACE_SCOPE(); - VkCommandBuffer cmdbuffer = mCurrentRenderPass.commandBuffer->buffer(); - auto const& [doBindInDraw, bundle] = mPipelineState.bindInDraw; + VulkanCommandBuffer* bufferHolder = mCurrentRenderPass.commandBuffer; + VkCommandBuffer cmdbuffer = bufferHolder->buffer(); + auto const [doBindInDraw, bundle] = mPipelineState.bindInDraw; fvkutils::DescriptorSetMask setsWithExternalSamplers = {}; if (doBindInDraw) { auto& layoutHandles = bundle.dsLayoutHandles; - setsWithExternalSamplers = mExternalImageManager.prepareBindSets(layoutHandles, - mDescriptorSetCache.getBoundSets()); + setsWithExternalSamplers = mExternalImageManager.prepareBindSets(bufferHolder, + layoutHandles, mDescriptorSetCache.getBoundSets()); VulkanDescriptorSetLayout::DescriptorSetLayoutArray vklayouts; for (size_t i = 0; i < layoutHandles.size(); i++) { @@ -2115,7 +2117,7 @@ void VulkanDriver::draw2(uint32_t indexOffset, uint32_t indexCount, uint32_t ins } mPipelineState.bindInDraw.first = false; } - mDescriptorSetCache.commit(mCurrentRenderPass.commandBuffer, mPipelineState.pipelineLayout, + mDescriptorSetCache.commit(bufferHolder, mPipelineState.pipelineLayout, setsWithExternalSamplers, mPipelineState.descriptorSetMask); // Finally, make the actual draw call. TODO: support subranges diff --git a/filament/backend/src/vulkan/VulkanExternalImageManager.cpp b/filament/backend/src/vulkan/VulkanExternalImageManager.cpp index 18ecd02642..c93a0de161 100644 --- a/filament/backend/src/vulkan/VulkanExternalImageManager.cpp +++ b/filament/backend/src/vulkan/VulkanExternalImageManager.cpp @@ -16,6 +16,7 @@ #include "VulkanExternalImageManager.h" +#include "VulkanCommands.h" #include "VulkanDescriptorSetCache.h" #include "VulkanDescriptorSetLayoutCache.h" #include "VulkanSamplerCache.h" @@ -51,19 +52,19 @@ ImageData& findImage(std::vector& images, } void copySet(VkDevice device, VkDescriptorSet srcSet, VkDescriptorSet dstSet, Bitmask bindings) { - // TODO: fix the size for better memory management - std::vector copies; + VkCopyDescriptorSet copies[sizeof(fvkutils::UniformBufferBitmask)]; + size_t count = 0; bindings.forEachSetBit([&](size_t index) { - copies.push_back({ + copies[count++] ={ .sType = VK_STRUCTURE_TYPE_COPY_DESCRIPTOR_SET, .srcSet = srcSet, .srcBinding = (uint32_t) index, .dstSet = dstSet, .dstBinding = (uint32_t) index, .descriptorCount = 1, - }); + }; }); - vkUpdateDescriptorSets(device, 0, nullptr, copies.size(), copies.data()); + vkUpdateDescriptorSets(device, 0, nullptr, count, copies); } Bitmask foldBitsInHalf(Bitmask bitset) { @@ -103,7 +104,8 @@ void VulkanExternalImageManager::onBeginFrame() { }); } -fvkutils::DescriptorSetMask VulkanExternalImageManager::prepareBindSets(LayoutArray const& layouts, +fvkutils::DescriptorSetMask VulkanExternalImageManager::prepareBindSets( + VulkanCommandBuffer* bufferHolder, LayoutArray const& layouts, SetArray const& sets) { fvkutils::DescriptorSetMask shouldUseExternalSampler{}; for (uint8_t i = 0; i < sets.size(); i++) { @@ -113,7 +115,7 @@ fvkutils::DescriptorSetMask VulkanExternalImageManager::prepareBindSets(LayoutAr continue; } if (hasExternalSampler(set)) { - updateSetAndLayout(set, layout); + updateSetAndLayout(bufferHolder, set, layout); shouldUseExternalSampler.set(i); } } @@ -128,6 +130,7 @@ bool VulkanExternalImageManager::hasExternalSampler( } void VulkanExternalImageManager::updateSetAndLayout( + VulkanCommandBuffer* bufferHolder, fvkmemory::resource_ptr set, fvkmemory::resource_ptr layout) { utils::FixedCapacityVector< @@ -143,15 +146,9 @@ void VulkanExternalImageManager::updateSetAndLayout( auto& imageData = findImage(mImages, bindingInfo.image); updateImage(&imageData); - auto samplerParams = bindingInfo.samplerParams; - // according to spec, these must match chromaFilter - // https://registry.khronos.org/vulkan/specs/latest/man/html/VkSamplerCreateInfo.html#VUID-VkSamplerCreateInfo-minFilter-01645 - samplerParams.filterMag = SamplerMagFilter::NEAREST; - samplerParams.filterMin = SamplerMinFilter::NEAREST; - auto sampler = mSamplerCache->getSampler({ - .sampler = samplerParams, - .conversion = imageData.conversion, + .sampler = bindingInfo.samplerParams, + .conversion = imageData.image->getYcbcrConversion(), }); actualExternalSamplers.set(bindingInfo.binding); samplerAndBindings.push_back({ bindingInfo.binding, sampler, bindingInfo.image }); @@ -176,29 +173,44 @@ void VulkanExternalImageManager::updateSetAndLayout( VkDescriptorSetLayout const newLayout = mDescriptorSetLayoutCache->getVkLayout(layout->bitmask, actualExternalSamplers, outSamplers); - // Need to copy the set + // Below, the idea is that we will bind a new set (because the sampler binding of an exisiting + // set needs to change to an external sampler). We will copy from an old external sampler set + // (if it exists) or the non-external version of the set. + + // Note that we always need to copy the set, because we are about to update a set that might + // have been bound already. We cannot do that without the VK_EXT_descriptor_indexing (core + // in 1.2). VkDescriptorSet const oldSet = set->getExternalSamplerVkSet(); - if (oldLayout != newLayout || oldSet == VK_NULL_HANDLE) { - // Build a new descriptor set from the new layout - VkDescriptorSet const newSet = mDescriptorSetCache->getVkSet(layout->count, newLayout); - auto const ubo = layout->bitmask.ubo | layout->bitmask.dynamicUbo; - auto const samplers = layout->bitmask.sampler & (~actualExternalSamplers); + // Build a new descriptor set from the new layout + VkDescriptorSet const newSet = mDescriptorSetCache->getVkSet(layout->count, newLayout); + mInUseSets.insert({ + newSet, + { + .fenceStatus = bufferHolder->getFenceStatus(), + .layout = newLayout, + .layoutCount = layout->count, + }, + }); - // Each bitmask denotes a binding index, and separated into two stages - vertex and buffer - // We fold the two stages into just the lower half of the bits to denote a combined set of - // bindings. - Bitmask const copyBindings = foldBitsInHalf(ubo | samplers); - VkDescriptorSet const srcSet = oldSet != VK_NULL_HANDLE ? oldSet : set->getVkSet(); - copySet(mPlatform->getDevice(), srcSet, newSet, copyBindings); + auto const ubo = layout->bitmask.ubo | layout->bitmask.dynamicUbo; + auto const samplers = layout->bitmask.sampler & (~actualExternalSamplers); - set->setExternalSamplerVkSet(newSet, - [&descriptorSetCache = mDescriptorSetCache, layoutCount = layout->count, newLayout, - newSet](VulkanDescriptorSet*) { - descriptorSetCache->manualRecycle(layoutCount, newLayout, newSet); - }); - if (oldLayout != newLayout) { - layout->setExternalSamplerVkLayout(newLayout); - } + // Each bitmask denotes a binding index, and separated into two stages - vertex and buffer + // We fold the two stages into just the lower half of the bits to denote a combined set of + // bindings. + Bitmask const copyBindings = foldBitsInHalf(ubo | samplers); + VkDescriptorSet const srcSet = oldSet != VK_NULL_HANDLE ? oldSet : set->getVkSet(); + copySet(mPlatform->getDevice(), srcSet, newSet, copyBindings); + + set->setExternalSamplerVkSet(newSet); + if (oldSet) { + // Now that we're replacing the old set, we can recycle it once it's no longer referenced in + // a command buffer. + mInUseSets[oldSet].recyclable = true; + } + + if (oldLayout != newLayout) { + layout->setExternalSamplerVkLayout(newLayout); } // Update the external samplers in the set @@ -240,13 +252,7 @@ void VulkanExternalImageManager::updateImage(ImageData* image) { image->hasBeenValidated = true; auto metadata = mPlatform->extractExternalImageMetadata(image->platformHandle); - auto vkYcbcr = getVkSamplerYcbcrConversion(metadata); - if (vkYcbcr == image->conversion) { - return; - } - - image->image->setYcbcrConversion(vkYcbcr); - image->conversion = vkYcbcr; + image->image->setYcbcrConversion(getVkSamplerYcbcrConversion(metadata)); return; } @@ -262,6 +268,10 @@ void VulkanExternalImageManager::bindExternallySampledTexture( // Should we do duplicate validation here? auto& imageData = findImage(mImages, image); mSetBindings.push_back({ bindingPoint, imageData.image, set, samplerParams }); + // according to spec, these must match chromaFilter + // https://registry.khronos.org/vulkan/specs/latest/man/html/VkSamplerCreateInfo.html#VUID-VkSamplerCreateInfo-minFilter-01645 + samplerParams.filterMag = SamplerMagFilter::NEAREST; + samplerParams.filterMin = SamplerMinFilter::NEAREST; } void VulkanExternalImageManager::addExternallySampledTexture( @@ -293,4 +303,20 @@ void VulkanExternalImageManager::clearTextureBinding( }); } +void VulkanExternalImageManager::gc() { + std::vector removedSets; + removedSets.reserve(mInUseSets.size()); + for (auto const& [set, boundSetInfo]: mInUseSets) { + // It's no longer in use by the command buffer. We're free to recycle it. + if (boundSetInfo.fenceStatus->getStatus() == VK_SUCCESS && boundSetInfo.recyclable) { + removedSets.push_back(set); + mDescriptorSetCache->manualRecycle(boundSetInfo.layoutCount, boundSetInfo.layout, set); + } + } + + for (auto set: removedSets) { + mInUseSets.erase(set); + } +} + } // namesapce filament::backend diff --git a/filament/backend/src/vulkan/VulkanExternalImageManager.h b/filament/backend/src/vulkan/VulkanExternalImageManager.h index 0f3173fc5e..1cd9e3b6e6 100644 --- a/filament/backend/src/vulkan/VulkanExternalImageManager.h +++ b/filament/backend/src/vulkan/VulkanExternalImageManager.h @@ -18,6 +18,8 @@ #define TNT_FILAMENT_BACKEND_CACHING_VULKANEXTERNALIMAGEMANAGER_H #include "VulkanHandles.h" +#include "vulkan/VulkanAsyncHandles.h" +#include "vulkan/VulkanCommands.h" #include @@ -58,7 +60,9 @@ public: // Returns bitmask to indicate whether or not to use the external sampler version of each // descriptor set. - fvkutils::DescriptorSetMask prepareBindSets(LayoutArray const& layouts, SetArray const& sets); + fvkutils::DescriptorSetMask prepareBindSets( + VulkanCommandBuffer* bufferHolder, + LayoutArray const& layouts, SetArray const& sets); void removeDescriptorSet(fvkmemory::resource_ptr set); @@ -83,13 +87,15 @@ public: fvkmemory::resource_ptr image; Platform::ExternalImageHandle platformHandle; bool hasBeenValidated = false; // indicates whether the image has been validated *this frame* - VkSamplerYcbcrConversion conversion = VK_NULL_HANDLE; }; bool hasExternalSampler(fvkmemory::resource_ptr set) const; + void gc(); + private: - void updateSetAndLayout(fvkmemory::resource_ptr set, + void updateSetAndLayout(VulkanCommandBuffer* bufferHolder, + fvkmemory::resource_ptr set, fvkmemory::resource_ptr layout); void updateImage(ImageData* imageData); @@ -114,6 +120,15 @@ private: // Use vectors instead of hash maps because we only expect small number of entries. std::vector mSetBindings; std::vector mImages; + + // Keep metadata to manage recycling sets that have been bound and used. + struct BoundSetInfo { + std::shared_ptr fenceStatus; + VkDescriptorSetLayout layout = VK_NULL_HANDLE; + VulkanDescriptorSetLayout::Count layoutCount = {}; + bool recyclable = false; + }; + std::unordered_map mInUseSets; }; } // filament::backend diff --git a/filament/backend/src/vulkan/VulkanHandles.h b/filament/backend/src/vulkan/VulkanHandles.h index c169546d47..6befdb8b80 100644 --- a/filament/backend/src/vulkan/VulkanHandles.h +++ b/filament/backend/src/vulkan/VulkanHandles.h @@ -172,9 +172,6 @@ public: if (mOnRecycleFn) { mOnRecycleFn(this); } - if (mOnRecycleExternalSamplerFn) { - mOnRecycleExternalSamplerFn(this); - } } VkDescriptorSet getVkSet() const noexcept { @@ -185,12 +182,8 @@ public: return mExternalSamplerVkSet; } - void setExternalSamplerVkSet(VkDescriptorSet vkset, OnRecycle onRecycle) { + void setExternalSamplerVkSet(VkDescriptorSet vkset) { mExternalSamplerVkSet = vkset; - if (mOnRecycleExternalSamplerFn) { - mOnRecycleExternalSamplerFn(this); - } - mOnRecycleExternalSamplerFn = onRecycle; } void setOffsets(backend::DescriptorSetOffsetArray&& offsets) noexcept { @@ -221,7 +214,6 @@ private: backend::DescriptorSetOffsetArray mOffsets; std::vector> mResources; OnRecycle mOnRecycleFn; - OnRecycle mOnRecycleExternalSamplerFn; fvkutils::UniformBufferBitmask mUboMask; }; diff --git a/filament/backend/src/vulkan/VulkanTexture.cpp b/filament/backend/src/vulkan/VulkanTexture.cpp index 7e6064391f..d3816432b2 100644 --- a/filament/backend/src/vulkan/VulkanTexture.cpp +++ b/filament/backend/src/vulkan/VulkanTexture.cpp @@ -853,6 +853,10 @@ void VulkanTexture::setYcbcrConversion(VkSamplerYcbcrConversion conversion) { } } +VkSamplerYcbcrConversion VulkanTexture::getYcbcrConversion() const noexcept { + return mState->mYcbcr.conversion; +} + VulkanLayout VulkanTexture::getLayout(uint32_t layer, uint32_t level) const { assert_invariant(level <= 0xffff && layer <= 0xffff); const uint32_t key = (layer << 16) | level; diff --git a/filament/backend/src/vulkan/VulkanTexture.h b/filament/backend/src/vulkan/VulkanTexture.h index 02c88861d0..be67fa3d7e 100644 --- a/filament/backend/src/vulkan/VulkanTexture.h +++ b/filament/backend/src/vulkan/VulkanTexture.h @@ -219,6 +219,8 @@ struct VulkanTexture : public HwTexture, fvkmemory::Resource { // happens. void setYcbcrConversion(VkSamplerYcbcrConversion conversion); + VkSamplerYcbcrConversion getYcbcrConversion() const noexcept; + #if FVK_ENABLED(FVK_DEBUG_TEXTURE) void print() const; #endif