From 28ed12aa93d0fb46a551b67b156e26701528bbff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Sun, 8 Oct 2023 11:54:37 +0200 Subject: [PATCH] Simplify descriptor pool creation --- Common/GPU/Vulkan/VulkanDescSet.cpp | 34 ++++++++++++++++++++++--- Common/GPU/Vulkan/VulkanDescSet.h | 11 +++++++- Common/GPU/Vulkan/VulkanRenderManager.h | 10 +------- Common/GPU/Vulkan/thin3d_vulkan.cpp | 22 +++------------- GPU/Vulkan/DrawEngineVulkan.cpp | 24 +++-------------- GPU/Vulkan/VulkanUtil.cpp | 19 +++++--------- 6 files changed, 57 insertions(+), 63 deletions(-) diff --git a/Common/GPU/Vulkan/VulkanDescSet.cpp b/Common/GPU/Vulkan/VulkanDescSet.cpp index b4a654bb67..0139a2356a 100644 --- a/Common/GPU/Vulkan/VulkanDescSet.cpp +++ b/Common/GPU/Vulkan/VulkanDescSet.cpp @@ -4,13 +4,40 @@ VulkanDescSetPool::~VulkanDescSetPool() { _assert_msg_(descPool_ == VK_NULL_HANDLE, "VulkanDescSetPool %s never destroyed", tag_); } -void VulkanDescSetPool::Create(VulkanContext *vulkan, const VkDescriptorPoolCreateInfo &info, const std::vector &sizes) { +void VulkanDescSetPool::Create(VulkanContext *vulkan, const BindingType *bindingTypes, uint32_t bindingTypesCount, uint32_t descriptorCount) { _assert_msg_(descPool_ == VK_NULL_HANDLE, "VulkanDescSetPool::Create when already exists"); vulkan_ = vulkan; - info_ = info; - sizes_ = sizes; + info_ = { VK_STRUCTURE_TYPE_DESCRIPTOR_POOL_CREATE_INFO }; + info_.maxSets = descriptorCount; + _dbg_assert_(sizes_.empty()); + uint32_t storageImageCount = 0; + uint32_t storageBufferCount = 0; + uint32_t combinedImageSamplerCount = 0; + uint32_t uniformBufferDynamicCount = 0; + for (uint32_t i = 0; i < bindingTypesCount; i++) { + switch (bindingTypes[i]) { + case BindingType::COMBINED_IMAGE_SAMPLER: combinedImageSamplerCount++; break; + case BindingType::UNIFORM_BUFFER_DYNAMIC_VERTEX: + case BindingType::UNIFORM_BUFFER_DYNAMIC_ALL: uniformBufferDynamicCount++; break; + case BindingType::STORAGE_BUFFER_VERTEX: + case BindingType::STORAGE_BUFFER_COMPUTE: storageBufferCount++; break; + case BindingType::STORAGE_IMAGE_COMPUTE: storageImageCount++; break; + } + } + if (storageImageCount) { + sizes_.push_back(VkDescriptorPoolSize{ VK_DESCRIPTOR_TYPE_STORAGE_IMAGE, storageImageCount * descriptorCount }); + } + if (uniformBufferDynamicCount) { + sizes_.push_back(VkDescriptorPoolSize{ VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER_DYNAMIC, uniformBufferDynamicCount * descriptorCount }); + } + if (combinedImageSamplerCount) { + sizes_.push_back(VkDescriptorPoolSize{ VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER, combinedImageSamplerCount * descriptorCount }); + } + if (storageImageCount) { + sizes_.push_back(VkDescriptorPoolSize{ VK_DESCRIPTOR_TYPE_STORAGE_IMAGE, storageImageCount * descriptorCount }); + } VkResult res = Recreate(false); _assert_msg_(res == VK_SUCCESS, "Could not create VulkanDescSetPool %s", tag_); } @@ -66,6 +93,7 @@ void VulkanDescSetPool::Destroy() { clear_(); usage_ = 0; } + sizes_.clear(); } VkResult VulkanDescSetPool::Recreate(bool grow) { diff --git a/Common/GPU/Vulkan/VulkanDescSet.h b/Common/GPU/Vulkan/VulkanDescSet.h index 8c9ae70c39..e42e24b30c 100644 --- a/Common/GPU/Vulkan/VulkanDescSet.h +++ b/Common/GPU/Vulkan/VulkanDescSet.h @@ -6,6 +6,15 @@ #include #include +enum class BindingType { + COMBINED_IMAGE_SAMPLER, + UNIFORM_BUFFER_DYNAMIC_VERTEX, + UNIFORM_BUFFER_DYNAMIC_ALL, + STORAGE_BUFFER_VERTEX, + STORAGE_BUFFER_COMPUTE, + STORAGE_IMAGE_COMPUTE, +}; + // Only appropriate for use in a per-frame pool. class VulkanDescSetPool { public: @@ -16,7 +25,7 @@ public: void Setup(const std::function &clear) { clear_ = clear; } - void Create(VulkanContext *vulkan, const VkDescriptorPoolCreateInfo &info, const std::vector &sizes); + void Create(VulkanContext *vulkan, const BindingType *bindingTypes, uint32_t bindingTypesCount, uint32_t descriptorCount); // Allocate a new set, which may resize and empty the current sets. // Use only for the current frame, unless in a cache cleared by clear_. VkDescriptorSet Allocate(int n, const VkDescriptorSetLayout *layouts, const char *tag); diff --git a/Common/GPU/Vulkan/VulkanRenderManager.h b/Common/GPU/Vulkan/VulkanRenderManager.h index b8617ff87b..353dd5e7e2 100644 --- a/Common/GPU/Vulkan/VulkanRenderManager.h +++ b/Common/GPU/Vulkan/VulkanRenderManager.h @@ -23,6 +23,7 @@ #include "Common/GPU/MiscTypes.h" #include "Common/GPU/Vulkan/VulkanQueueRunner.h" #include "Common/GPU/Vulkan/VulkanFramebuffer.h" +#include "Common/GPU/Vulkan/VulkanDescSet.h" #include "Common/GPU/thin3d.h" // Forward declaration @@ -184,15 +185,6 @@ struct CompileQueueEntry { VkSampleCountFlagBits sampleCount; }; -enum class BindingType { - COMBINED_IMAGE_SAMPLER, - UNIFORM_BUFFER_DYNAMIC_VERTEX, - UNIFORM_BUFFER_DYNAMIC_ALL, - STORAGE_BUFFER_VERTEX, - STORAGE_BUFFER_COMPUTE, - STORAGE_IMAGE_COMPUTE, -}; - // Note that we only support a single descriptor set due to compatibility with some ancient devices. // We should probably eventually give that up. struct VKRPipelineLayout { diff --git a/Common/GPU/Vulkan/thin3d_vulkan.cpp b/Common/GPU/Vulkan/thin3d_vulkan.cpp index c0add6bfc7..709a2dcf98 100644 --- a/Common/GPU/Vulkan/thin3d_vulkan.cpp +++ b/Common/GPU/Vulkan/thin3d_vulkan.cpp @@ -1039,26 +1039,9 @@ VKContext::VKContext(VulkanContext *vulkan, bool useRenderThread) caps_.deviceID = deviceProps.deviceID; device_ = vulkan->GetDevice(); - std::vector dpTypes; - dpTypes.resize(2); - dpTypes[0].descriptorCount = 200; - dpTypes[0].type = VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER_DYNAMIC; - dpTypes[1].descriptorCount = 200 * MAX_BOUND_TEXTURES; - dpTypes[1].type = VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER; - - VkDescriptorPoolCreateInfo dp{ VK_STRUCTURE_TYPE_DESCRIPTOR_POOL_CREATE_INFO }; - // Don't want to mess around with individually freeing these, let's go dynamic each frame. - dp.flags = 0; - // 200 textures per frame was not enough for the UI. - dp.maxSets = 4096; - VkBufferUsageFlags usage = VK_BUFFER_USAGE_INDEX_BUFFER_BIT | VK_BUFFER_USAGE_UNIFORM_BUFFER_BIT | VK_BUFFER_USAGE_STORAGE_BUFFER_BIT | VK_BUFFER_USAGE_VERTEX_BUFFER_BIT | VK_BUFFER_USAGE_TRANSFER_SRC_BIT; push_ = new VulkanPushPool(vulkan_, "pushBuffer", 4 * 1024 * 1024, usage); - for (int i = 0; i < VulkanContext::MAX_INFLIGHT_FRAMES; i++) { - frame_[i].descriptorPool.Create(vulkan_, dp, dpTypes); - } - // binding 0 - uniform data // binding 1 - combined sampler/image 0 // binding 2 - combined sampler/image 1 @@ -1068,9 +1051,12 @@ VKContext::VKContext(VulkanContext *vulkan, bool useRenderThread) for (int i = 0; i < MAX_BOUND_TEXTURES; ++i) { bindings[1 + i] = BindingType::COMBINED_IMAGE_SAMPLER; } - pipelineLayout_ = renderManager_.CreatePipelineLayout(bindings, ARRAY_SIZE(bindings), caps_.geometryShaderSupported, "thin3d_layout"); + for (int i = 0; i < VulkanContext::MAX_INFLIGHT_FRAMES; i++) { + frame_[i].descriptorPool.Create(vulkan_, bindings, ARRAY_SIZE(bindings), 1024); + } + VkPipelineCacheCreateInfo pc{ VK_STRUCTURE_TYPE_PIPELINE_CACHE_CREATE_INFO }; VkResult res = vkCreatePipelineCache(vulkan_->GetDevice(), &pc, nullptr, &pipelineCache_); _assert_(VK_SUCCESS == res); diff --git a/GPU/Vulkan/DrawEngineVulkan.cpp b/GPU/Vulkan/DrawEngineVulkan.cpp index ba0932716b..5a99d5ae26 100644 --- a/GPU/Vulkan/DrawEngineVulkan.cpp +++ b/GPU/Vulkan/DrawEngineVulkan.cpp @@ -88,9 +88,9 @@ void DrawEngineVulkan::InitDeviceObjects() { BindingType::UNIFORM_BUFFER_DYNAMIC_ALL, // uniforms BindingType::UNIFORM_BUFFER_DYNAMIC_VERTEX, // lights BindingType::UNIFORM_BUFFER_DYNAMIC_VERTEX, // bones - BindingType::STORAGE_BUFFER, // tess - BindingType::STORAGE_BUFFER, - BindingType::STORAGE_BUFFER, + BindingType::STORAGE_BUFFER_VERTEX, // tess + BindingType::STORAGE_BUFFER_VERTEX, + BindingType::STORAGE_BUFFER_VERTEX, }; VulkanContext *vulkan = (VulkanContext *)draw_->GetNativeObject(Draw::NativeObject::CONTEXT); @@ -100,27 +100,11 @@ void DrawEngineVulkan::InitDeviceObjects() { pipelineLayout_ = renderManager->CreatePipelineLayout(bindingTypes, ARRAY_SIZE(bindingTypes), draw_->GetDeviceCaps().geometryShaderSupported, "drawengine_layout"); static constexpr int DEFAULT_DESC_POOL_SIZE = 512; - std::vector dpTypes; - dpTypes.resize(4); - dpTypes[0].descriptorCount = DEFAULT_DESC_POOL_SIZE * 3; - dpTypes[0].type = VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER_DYNAMIC; - dpTypes[1].descriptorCount = DEFAULT_DESC_POOL_SIZE * 3; // Don't use these for tess anymore, need max three per set. - dpTypes[1].type = VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER; - dpTypes[2].descriptorCount = DEFAULT_DESC_POOL_SIZE * 3; // TODO: Use a separate layout when no spline stuff is needed to reduce the need for these. - dpTypes[2].type = VK_DESCRIPTOR_TYPE_STORAGE_BUFFER; - dpTypes[3].descriptorCount = DEFAULT_DESC_POOL_SIZE; // For the frame global uniform buffer. Might need to allocate multiple times. - dpTypes[3].type = VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER; - - VkDescriptorPoolCreateInfo dp{ VK_STRUCTURE_TYPE_DESCRIPTOR_POOL_CREATE_INFO }; - // Don't want to mess around with individually freeing these. - // We zap the whole pool every few frames. - dp.flags = 0; - dp.maxSets = DEFAULT_DESC_POOL_SIZE; // We are going to use one-shot descriptors in the initial implementation. Might look into caching them // if creating and updating them turns out to be expensive. for (int i = 0; i < VulkanContext::MAX_INFLIGHT_FRAMES; i++) { - frame_[i].descPool.Create(vulkan, dp, dpTypes); + frame_[i].descPool.Create(vulkan, bindingTypes, ARRAY_SIZE(bindingTypes), DEFAULT_DESC_POOL_SIZE); // Note that pushUBO_ is also used for tessellation data (search for SetPushBuffer), and to upload // the null texture. This should be cleaned up... diff --git a/GPU/Vulkan/VulkanUtil.cpp b/GPU/Vulkan/VulkanUtil.cpp index b8dfe9e3d4..8fc286f97c 100644 --- a/GPU/Vulkan/VulkanUtil.cpp +++ b/GPU/Vulkan/VulkanUtil.cpp @@ -60,6 +60,12 @@ void VulkanComputeShaderManager::InitDeviceObjects(Draw::DrawContext *draw) { VkResult res = vkCreatePipelineCache(vulkan_->GetDevice(), &pc, nullptr, &pipelineCache_); _assert_(VK_SUCCESS == res); + static const BindingType bindingTypes[3] = { + BindingType::STORAGE_IMAGE_COMPUTE, + BindingType::STORAGE_BUFFER_COMPUTE, + BindingType::STORAGE_BUFFER_COMPUTE, + }; + VkDescriptorSetLayoutBinding bindings[3] = {}; bindings[0].descriptorCount = 1; bindings[0].descriptorType = VK_DESCRIPTOR_TYPE_STORAGE_IMAGE; @@ -82,19 +88,8 @@ void VulkanComputeShaderManager::InitDeviceObjects(Draw::DrawContext *draw) { res = vkCreateDescriptorSetLayout(device, &dsl, nullptr, &descriptorSetLayout_); _assert_(VK_SUCCESS == res); - std::vector dpTypes; - dpTypes.resize(2); - dpTypes[0].descriptorCount = 8192; - dpTypes[0].type = VK_DESCRIPTOR_TYPE_STORAGE_BUFFER; - dpTypes[1].descriptorCount = 4096; - dpTypes[1].type = VK_DESCRIPTOR_TYPE_STORAGE_IMAGE; - - VkDescriptorPoolCreateInfo dp = { VK_STRUCTURE_TYPE_DESCRIPTOR_POOL_CREATE_INFO }; - dp.flags = 0; // Don't want to mess around with individually freeing these, let's go fixed each frame and zap the whole array. Might try the dynamic approach later. - dp.maxSets = 4096; // GTA can end up creating more than 1000 textures in the first frame! - for (int i = 0; i < ARRAY_SIZE(frameData_); i++) { - frameData_[i].descPool.Create(vulkan_, dp, dpTypes); + frameData_[i].descPool.Create(vulkan_, bindingTypes, ARRAY_SIZE(bindingTypes), 4096); frameData_[i].descPoolUsed = false; }