Skip to content

Commit

Permalink
Merge pull request #18320 from hrydgard/simplify-desc-set-layout
Browse files Browse the repository at this point in the history
Vulkan: Simplify pipeline and descriptor set layout, pool creation
  • Loading branch information
hrydgard committed Oct 8, 2023
2 parents 5bb1db5 + c73e235 commit 43cdb88
Show file tree
Hide file tree
Showing 12 changed files with 197 additions and 173 deletions.
36 changes: 32 additions & 4 deletions Common/GPU/Vulkan/VulkanDescSet.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<VkDescriptorPoolSize> &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 (combinedImageSamplerCount) {
sizes_.push_back(VkDescriptorPoolSize{ VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER, combinedImageSamplerCount * descriptorCount });
}
if (uniformBufferDynamicCount) {
sizes_.push_back(VkDescriptorPoolSize{ VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER_DYNAMIC, uniformBufferDynamicCount * descriptorCount });
}
if (storageBufferCount) {
sizes_.push_back(VkDescriptorPoolSize{ VK_DESCRIPTOR_TYPE_STORAGE_BUFFER, storageBufferCount * 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_);
}
Expand Down Expand Up @@ -66,6 +93,7 @@ void VulkanDescSetPool::Destroy() {
clear_();
usage_ = 0;
}
sizes_.clear();
}

VkResult VulkanDescSetPool::Recreate(bool grow) {
Expand Down
11 changes: 10 additions & 1 deletion Common/GPU/Vulkan/VulkanDescSet.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,15 @@
#include <functional>
#include <vector>

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:
Expand All @@ -16,7 +25,7 @@ class VulkanDescSetPool {
void Setup(const std::function<void()> &clear) {
clear_ = clear;
}
void Create(VulkanContext *vulkan, const VkDescriptorPoolCreateInfo &info, const std::vector<VkDescriptorPoolSize> &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);
Expand Down
4 changes: 2 additions & 2 deletions Common/GPU/Vulkan/VulkanQueueRunner.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1241,7 +1241,7 @@ void VulkanQueueRunner::PerformRenderPass(const VKRStep &step, VkCommandBuffer c

if (pipeline != VK_NULL_HANDLE) {
vkCmdBindPipeline(cmd, VK_PIPELINE_BIND_POINT_GRAPHICS, pipeline);
pipelineLayout = c.pipeline.pipelineLayout;
pipelineLayout = c.pipeline.pipelineLayout->pipelineLayout;
lastGraphicsPipeline = graphicsPipeline;
pipelineOK = true;
} else {
Expand All @@ -1263,7 +1263,7 @@ void VulkanQueueRunner::PerformRenderPass(const VKRStep &step, VkCommandBuffer c
VkPipeline pipeline = computePipeline->pipeline->BlockUntilReady();
if (pipeline != VK_NULL_HANDLE) {
vkCmdBindPipeline(cmd, VK_PIPELINE_BIND_POINT_COMPUTE, pipeline);
pipelineLayout = c.pipeline.pipelineLayout;
pipelineLayout = c.pipeline.pipelineLayout->pipelineLayout;
lastComputePipeline = computePipeline;
}
}
Expand Down
7 changes: 4 additions & 3 deletions Common/GPU/Vulkan/VulkanQueueRunner.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ class VKRFramebuffer;
struct VKRGraphicsPipeline;
struct VKRComputePipeline;
struct VKRImage;
struct VKRPipelineLayout;
struct FrameData;

enum {
Expand Down Expand Up @@ -58,15 +59,15 @@ struct VkRenderData {
union {
struct {
VkPipeline pipeline;
VkPipelineLayout pipelineLayout;
VKRPipelineLayout *pipelineLayout;
} pipeline;
struct {
VKRGraphicsPipeline *pipeline;
VkPipelineLayout pipelineLayout;
VKRPipelineLayout *pipelineLayout;
} graphics_pipeline;
struct {
VKRComputePipeline *pipeline;
VkPipelineLayout pipelineLayout;
VKRPipelineLayout *pipelineLayout;
} compute_pipeline;
struct {
VkDescriptorSet ds;
Expand Down
70 changes: 68 additions & 2 deletions Common/GPU/Vulkan/VulkanRenderManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ bool VKRGraphicsPipeline::Create(VulkanContext *vulkan, VkRenderPass compatibleR
pipe.pDynamicState = &desc->ds;
pipe.pInputAssemblyState = &inputAssembly;
pipe.pMultisampleState = &ms;
pipe.layout = desc->pipelineLayout;
pipe.layout = desc->pipelineLayout->pipelineLayout;
pipe.basePipelineHandle = VK_NULL_HANDLE;
pipe.basePipelineIndex = 0;
pipe.subpass = 0;
Expand Down Expand Up @@ -192,7 +192,7 @@ void VKRGraphicsPipeline::DestroyVariantsInstant(VkDevice device) {

VKRGraphicsPipeline::~VKRGraphicsPipeline() {
// This is called from the callbacked queued in QueueForDeletion.
// Here we are free to directly delete stuff, don't need to queue.
// When we reach here, we should already be empty, so let's assert on that.
for (size_t i = 0; i < (size_t)RenderPassType::TYPE_COUNT; i++) {
_assert_(!pipeline[i]);
}
Expand Down Expand Up @@ -1556,6 +1556,72 @@ void VulkanRenderManager::FlushSync() {
}
}

VKRPipelineLayout *VulkanRenderManager::CreatePipelineLayout(BindingType *bindingTypes, size_t bindingCount, bool geoShadersEnabled, const char *tag) {
VKRPipelineLayout *layout = new VKRPipelineLayout();
layout->tag = tag;
layout->bindingCount = (uint32_t)bindingCount;

_dbg_assert_(bindingCount <= ARRAY_SIZE(layout->bindingTypes));
memcpy(layout->bindingTypes, bindingTypes, sizeof(BindingType) * bindingCount);

VkDescriptorSetLayoutBinding bindings[VKRPipelineLayout::MAX_DESC_SET_BINDINGS];
for (int i = 0; i < bindingCount; i++) {
bindings[i].binding = i;
bindings[i].descriptorCount = 1;
bindings[i].pImmutableSamplers = nullptr;

switch (bindingTypes[i]) {
case BindingType::COMBINED_IMAGE_SAMPLER:
bindings[i].descriptorType = VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER;
bindings[i].stageFlags = VK_SHADER_STAGE_FRAGMENT_BIT;
break;
case BindingType::UNIFORM_BUFFER_DYNAMIC_VERTEX:
bindings[i].descriptorType = VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER_DYNAMIC;
bindings[i].stageFlags = VK_SHADER_STAGE_VERTEX_BIT;
break;
case BindingType::UNIFORM_BUFFER_DYNAMIC_ALL:
bindings[i].descriptorType = VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER_DYNAMIC;
bindings[i].stageFlags = VK_SHADER_STAGE_VERTEX_BIT | VK_SHADER_STAGE_FRAGMENT_BIT;
if (geoShadersEnabled) {
bindings[i].stageFlags |= VK_SHADER_STAGE_GEOMETRY_BIT;
}
break;
case BindingType::STORAGE_BUFFER_VERTEX:
bindings[i].descriptorType = VK_DESCRIPTOR_TYPE_STORAGE_BUFFER;
bindings[i].stageFlags = VK_SHADER_STAGE_VERTEX_BIT;
break;
case BindingType::STORAGE_BUFFER_COMPUTE:
bindings[i].descriptorType = VK_DESCRIPTOR_TYPE_STORAGE_BUFFER;
bindings[i].stageFlags = VK_SHADER_STAGE_COMPUTE_BIT;
break;
case BindingType::STORAGE_IMAGE_COMPUTE:
bindings[i].descriptorType = VK_DESCRIPTOR_TYPE_STORAGE_IMAGE;
bindings[i].stageFlags = VK_SHADER_STAGE_COMPUTE_BIT;
break;
default:
_dbg_assert_(false);
break;
}
}

VkDescriptorSetLayoutCreateInfo dsl = { VK_STRUCTURE_TYPE_DESCRIPTOR_SET_LAYOUT_CREATE_INFO };
dsl.bindingCount = (uint32_t)bindingCount;
dsl.pBindings = bindings;
VkResult res = vkCreateDescriptorSetLayout(vulkan_->GetDevice(), &dsl, nullptr, &layout->descriptorSetLayout);
_assert_(VK_SUCCESS == res && layout->descriptorSetLayout);

VkPipelineLayoutCreateInfo pl = { VK_STRUCTURE_TYPE_PIPELINE_LAYOUT_CREATE_INFO };
VkDescriptorSetLayout setLayouts[1] = { layout->descriptorSetLayout };
pl.setLayoutCount = ARRAY_SIZE(setLayouts);
pl.pSetLayouts = setLayouts;
res = vkCreatePipelineLayout(vulkan_->GetDevice(), &pl, nullptr, &layout->pipelineLayout);
_assert_(VK_SUCCESS == res && layout->pipelineLayout);

vulkan_->SetDebugName(layout->descriptorSetLayout, VK_OBJECT_TYPE_DESCRIPTOR_SET_LAYOUT, tag);
vulkan_->SetDebugName(layout->pipelineLayout, VK_OBJECT_TYPE_PIPELINE_LAYOUT, tag);
return layout;
}

void VulkanRenderManager::ResetStats() {
initTimeMs_.Reset();
totalGPUTimeMs_.Reset();
Expand Down
34 changes: 31 additions & 3 deletions Common/GPU/Vulkan/VulkanRenderManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -106,7 +107,7 @@ class VKRGraphicsPipelineDesc : public Draw::RefCountedObject {
VkPipelineVertexInputStateCreateInfo vis{ VK_STRUCTURE_TYPE_PIPELINE_VERTEX_INPUT_STATE_CREATE_INFO };
VkPipelineViewportStateCreateInfo views{ VK_STRUCTURE_TYPE_PIPELINE_VIEWPORT_STATE_CREATE_INFO };

VkPipelineLayout pipelineLayout = VK_NULL_HANDLE;
VKRPipelineLayout *pipelineLayout = nullptr;

// Does not include the render pass type, it's passed in separately since the
// desc is persistent.
Expand Down Expand Up @@ -184,6 +185,25 @@ struct CompileQueueEntry {
VkSampleCountFlagBits sampleCount;
};

// 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 {
~VKRPipelineLayout() {
_assert_(!pipelineLayout && !descriptorSetLayout);
}
void Destroy(VulkanContext *vulkan) {
vulkan->Delete().QueueDeletePipelineLayout(pipelineLayout);
vulkan->Delete().QueueDeleteDescriptorSetLayout(descriptorSetLayout);
}
enum { MAX_DESC_SET_BINDINGS = 10 };
BindingType bindingTypes[MAX_DESC_SET_BINDINGS];
uint32_t bindingCount;
VkPipelineLayout pipelineLayout;
VkDescriptorSetLayout descriptorSetLayout; // only support 1 for now.
int pushConstSize = 0;
const char *tag;
};

class VulkanRenderManager {
public:
VulkanRenderManager(VulkanContext *vulkan, bool useThread, HistoryBuffer<FrameTimeData, FRAME_TIME_HISTORY_LENGTH> &frameTimeHistory);
Expand Down Expand Up @@ -238,6 +258,14 @@ class VulkanRenderManager {
VKRGraphicsPipeline *CreateGraphicsPipeline(VKRGraphicsPipelineDesc *desc, PipelineFlags pipelineFlags, uint32_t variantBitmask, VkSampleCountFlagBits sampleCount, bool cacheLoad, const char *tag);
VKRComputePipeline *CreateComputePipeline(VKRComputePipelineDesc *desc);

VKRPipelineLayout *CreatePipelineLayout(VkPipelineLayout pipelineLayout, VkDescriptorSetLayout descSetLayout) {
VKRPipelineLayout *layout = new VKRPipelineLayout();
layout->pipelineLayout = pipelineLayout;
layout->descriptorSetLayout = descSetLayout;
return layout;
}
VKRPipelineLayout *CreatePipelineLayout(BindingType *bindingTypes, size_t bindingCount, bool geoShadersEnabled, const char *tag);

void ReportBadStateForDraw();

void NudgeCompilerThread() {
Expand All @@ -249,7 +277,7 @@ class VulkanRenderManager {
// This is the first call in a draw operation. Instead of asserting like we used to, you can now check the
// return value and skip the draw if we're in a bad state. In that case, call ReportBadState.
// The old assert wasn't very helpful in figuring out what caused it anyway...
bool BindPipeline(VKRGraphicsPipeline *pipeline, PipelineFlags flags, VkPipelineLayout pipelineLayout) {
bool BindPipeline(VKRGraphicsPipeline *pipeline, PipelineFlags flags, VKRPipelineLayout *pipelineLayout) {
_dbg_assert_(curRenderStep_ && curRenderStep_->stepType == VKRStepType::RENDER && pipeline != nullptr);
if (!curRenderStep_ || curRenderStep_->stepType != VKRStepType::RENDER) {
return false;
Expand All @@ -267,7 +295,7 @@ class VulkanRenderManager {
return true;
}

void BindPipeline(VKRComputePipeline *pipeline, PipelineFlags flags, VkPipelineLayout pipelineLayout) {
void BindPipeline(VKRComputePipeline *pipeline, PipelineFlags flags, VKRPipelineLayout *pipelineLayout) {
_dbg_assert_(curRenderStep_ && curRenderStep_->stepType == VKRStepType::RENDER);
_dbg_assert_(pipeline != nullptr);
VkRenderData &data = curRenderStep_->commands.push_uninitialized();
Expand Down
64 changes: 12 additions & 52 deletions Common/GPU/Vulkan/thin3d_vulkan.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -546,8 +546,7 @@ class VKContext : public DrawContext {
AutoRef<VKBuffer> curIBuffer_;
int curIBufferOffset_ = 0;

VkDescriptorSetLayout descriptorSetLayout_ = VK_NULL_HANDLE;
VkPipelineLayout pipelineLayout_ = VK_NULL_HANDLE;
VKRPipelineLayout *pipelineLayout_ = nullptr;
VkPipelineCache pipelineCache_ = VK_NULL_HANDLE;
AutoRef<VKFramebuffer> curFramebuffer_;

Expand Down Expand Up @@ -1040,64 +1039,26 @@ VKContext::VKContext(VulkanContext *vulkan, bool useRenderThread)
caps_.deviceID = deviceProps.deviceID;
device_ = vulkan->GetDevice();

std::vector<VkDescriptorPoolSize> 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
VkDescriptorSetLayoutBinding bindings[MAX_BOUND_TEXTURES + 1];
bindings[0].descriptorCount = 1;
bindings[0].pImmutableSamplers = nullptr;
bindings[0].descriptorType = VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER_DYNAMIC;
bindings[0].stageFlags = VK_SHADER_STAGE_VERTEX_BIT | VK_SHADER_STAGE_FRAGMENT_BIT;
bindings[0].binding = 0;
// ...etc
BindingType bindings[MAX_BOUND_TEXTURES + 1];
bindings[0] = BindingType::UNIFORM_BUFFER_DYNAMIC_ALL;
for (int i = 0; i < MAX_BOUND_TEXTURES; ++i) {
bindings[i + 1].descriptorCount = 1;
bindings[i + 1].pImmutableSamplers = nullptr;
bindings[i + 1].descriptorType = VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER;
bindings[i + 1].stageFlags = VK_SHADER_STAGE_FRAGMENT_BIT;
bindings[i + 1].binding = i + 1;
bindings[1 + i] = BindingType::COMBINED_IMAGE_SAMPLER;
}
pipelineLayout_ = renderManager_.CreatePipelineLayout(bindings, ARRAY_SIZE(bindings), caps_.geometryShaderSupported, "thin3d_layout");

VkDescriptorSetLayoutCreateInfo dsl = { VK_STRUCTURE_TYPE_DESCRIPTOR_SET_LAYOUT_CREATE_INFO };
dsl.bindingCount = ARRAY_SIZE(bindings);
dsl.pBindings = bindings;
VkResult res = vkCreateDescriptorSetLayout(device_, &dsl, nullptr, &descriptorSetLayout_);
_assert_(VK_SUCCESS == res);

vulkan_->SetDebugName(descriptorSetLayout_, VK_OBJECT_TYPE_DESCRIPTOR_SET_LAYOUT, "thin3d_d_layout");

VkPipelineLayoutCreateInfo pl = { VK_STRUCTURE_TYPE_PIPELINE_LAYOUT_CREATE_INFO };
pl.pPushConstantRanges = nullptr;
pl.pushConstantRangeCount = 0;
VkDescriptorSetLayout setLayouts[1] = { descriptorSetLayout_ };
pl.setLayoutCount = ARRAY_SIZE(setLayouts);
pl.pSetLayouts = setLayouts;
res = vkCreatePipelineLayout(device_, &pl, nullptr, &pipelineLayout_);
_assert_(VK_SUCCESS == res);

vulkan_->SetDebugName(pipelineLayout_, VK_OBJECT_TYPE_PIPELINE_LAYOUT, "thin3d_p_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 };
res = vkCreatePipelineCache(vulkan_->GetDevice(), &pc, nullptr, &pipelineCache_);
VkResult res = vkCreatePipelineCache(vulkan_->GetDevice(), &pc, nullptr, &pipelineCache_);
_assert_(VK_SUCCESS == res);
}

Expand All @@ -1111,8 +1072,7 @@ VKContext::~VKContext() {
}
push_->Destroy();
delete push_;
vulkan_->Delete().QueueDeleteDescriptorSetLayout(descriptorSetLayout_);
vulkan_->Delete().QueueDeletePipelineLayout(pipelineLayout_);
pipelineLayout_->Destroy(vulkan_);
vulkan_->Delete().QueueDeletePipelineCache(pipelineCache_);
}

Expand Down Expand Up @@ -1181,7 +1141,7 @@ VkDescriptorSet VKContext::GetOrCreateDescriptorSet(VkBuffer buf) {
return iter->second;
}

VkDescriptorSet descSet = frame->descriptorPool.Allocate(1, &descriptorSetLayout_, "thin3d_descset");
VkDescriptorSet descSet = frame->descriptorPool.Allocate(1, &pipelineLayout_->descriptorSetLayout, "thin3d_descset");
if (descSet == VK_NULL_HANDLE) {
ERROR_LOG(G3D, "GetOrCreateDescriptorSet failed");
return VK_NULL_HANDLE;
Expand Down
Loading

0 comments on commit 43cdb88

Please sign in to comment.