Skip to content

Commit

Permalink
Use vulkan descriptor arrays instead of flattening them (#319)
Browse files Browse the repository at this point in the history
In order to fully support resource arrays, also require and enable
shader*ArrayDynamicIndexing device features.
This allows one to use push constants to select texture within an array
of textures like in 24_push_constants

Does not add support for using push descriptors with arrays.

Dynamic indexing features are not required for just using shader
resource arrays with constant indices.
Currently, from what I can tell, only 24_push_constants requires
enabling shaderSampledImageArrayDynamicIndexing and there are some small
number of gpus that don't support those features (software lavapipe < 23).
  • Loading branch information
apazylbe authored Nov 22, 2023
1 parent b4c0480 commit 899e05d
Show file tree
Hide file tree
Showing 5 changed files with 48 additions and 24 deletions.
1 change: 1 addition & 0 deletions .github/workflows/linux-build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ jobs:
steps:
- name: Setup necessary packages
run: |
sudo add-apt-repository ppa:kisak/kisak-mesa -y
sudo apt update && sudo apt install libxrandr-dev libxinerama-dev libx11-dev libxcursor-dev libxi-dev libx11-xcb-dev clang \
mesa-vulkan-drivers
- name: Setup Vulkan SDK
Expand Down
2 changes: 1 addition & 1 deletion cmake/ShaderCompile.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ function(internal_generate_rules_for_shader TARGET_NAME)
SHADER_STAGE "${ARG_SHADER_STAGE}"
OUTPUT_FORMAT "SPV_6_6"
TARGET_FOLDER "${TARGET_NAME}"
COMPILER_FLAGS "-spirv" "-fspv-flatten-resource-arrays" "-fspv-target-env=vulkan1.1" "-fvk-use-dx-layout" "-DPPX_VULKAN=1" "-T" "${ARG_SHADER_STAGE}_6_6" "-E" "${ARG_SHADER_STAGE}main")
COMPILER_FLAGS "-spirv" "-fspv-target-env=vulkan1.1" "-fvk-use-dx-layout" "-DPPX_VULKAN=1" "-T" "${ARG_SHADER_STAGE}_6_6" "-E" "${ARG_SHADER_STAGE}main")
add_dependencies("vk_${TARGET_NAME}" "vk_${TARGET_NAME}_${ARG_SHADER_STAGE}")
endif ()
endfunction()
Expand Down
5 changes: 4 additions & 1 deletion src/ppx/grfx/grfx_descriptor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,10 @@ Result DescriptorSet::UpdateUniformBuffer(
Result DescriptorSetLayout::Create(const grfx::DescriptorSetLayoutCreateInfo* pCreateInfo)
{
// Bail if there's any binding overlaps - overlaps are not permitted to
// make D3D12 and Vulkan agreeable.
// make D3D12 and Vulkan agreeable. Even though we use descriptor arrays
// in Vulkan, we do not allow the subsequent bindings to be occupied,
// to keep descriptor binding register occupancy consistent between
// Vulkan and D3D12.
//
std::vector<ppx::RangeU32> ranges;
const size_t bindingCount = pCreateInfo->bindings.size();
Expand Down
30 changes: 9 additions & 21 deletions src/ppx/grfx/vk/vk_descriptor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -234,8 +234,8 @@ Result DescriptorSet::UpdateDescriptors(uint32_t writeCount, const grfx::WriteDe
VkWriteDescriptorSet& vkWrite = mWriteStore[mWriteCount];
vkWrite = {VK_STRUCTURE_TYPE_WRITE_DESCRIPTOR_SET};
vkWrite.dstSet = mDescriptorSet;
vkWrite.dstBinding = srcWrite.binding + srcWrite.arrayIndex;
vkWrite.dstArrayElement = 0;
vkWrite.dstBinding = srcWrite.binding;
vkWrite.dstArrayElement = srcWrite.arrayIndex;
vkWrite.descriptorCount = 1;
vkWrite.descriptorType = descriptorType;
vkWrite.pImageInfo = pImageInfo;
Expand Down Expand Up @@ -266,27 +266,15 @@ Result DescriptorSetLayout::CreateApiObjects(const grfx::DescriptorSetLayoutCrea

std::vector<VkDescriptorSetLayoutBinding> vkBindings;
for (size_t i = 0; i < pCreateInfo->bindings.size(); ++i) {
//
// NOTE: To keep D3D12 and Vulkan aligned, we do not support Vulkan's
// descriptor arrayness model. This means that the value for
// VkDescriptorSetLayoutBinding::descriptorCount is always 1.
//
// If grfx::DescriptorBinding::arrayCount is greater than 1, we
// create that many entries. The binding number is incremented
// per entry starting from the initial binding value.

//
const grfx::DescriptorBinding& baseBinding = pCreateInfo->bindings[i];

for (uint32_t bindingOffset = 0; bindingOffset < baseBinding.arrayCount; ++bindingOffset) {
VkDescriptorSetLayoutBinding vkBinding = {};
vkBinding.binding = baseBinding.binding + bindingOffset;
vkBinding.descriptorType = ToVkDescriptorType(baseBinding.type);
vkBinding.descriptorCount = 1;
vkBinding.stageFlags = ToVkShaderStageFlags(baseBinding.shaderVisiblity);
vkBinding.pImmutableSamplers = nullptr;
vkBindings.push_back(vkBinding);
}
VkDescriptorSetLayoutBinding vkBinding = {};
vkBinding.binding = baseBinding.binding;
vkBinding.descriptorType = ToVkDescriptorType(baseBinding.type);
vkBinding.descriptorCount = baseBinding.arrayCount;
vkBinding.stageFlags = ToVkShaderStageFlags(baseBinding.shaderVisiblity);
vkBinding.pImmutableSamplers = nullptr;
vkBindings.push_back(vkBinding);
}

VkDescriptorSetLayoutCreateInfo vkci = {VK_STRUCTURE_TYPE_DESCRIPTOR_SET_LAYOUT_CREATE_INFO};
Expand Down
34 changes: 33 additions & 1 deletion src/ppx/grfx/vk/vk_device.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,38 @@ Result Device::ConfigureFeatures(const grfx::DeviceCreateInfo* pCreateInfo, VkPh
features = *pFeatures;
}

// Enable shader resource array dynamic indexing.
// This can be used to choose a texture within an array based on
// a push constant, among other things.
std::vector<std::string_view> missingFeatures;
if (!foundFeatures.shaderUniformBufferArrayDynamicIndexing) {
missingFeatures.push_back("shaderUniformBufferArrayDynamicIndexing");
}
if (!foundFeatures.shaderSampledImageArrayDynamicIndexing) {
missingFeatures.push_back("shaderSampledImageArrayDynamicIndexing");
}
if (!foundFeatures.shaderStorageBufferArrayDynamicIndexing) {
missingFeatures.push_back("shaderStorageBufferArrayDynamicIndexing");
}
if (!foundFeatures.shaderStorageImageArrayDynamicIndexing) {
missingFeatures.push_back("shaderStorageImageArrayDynamicIndexing");
}

if (!missingFeatures.empty()) {
std::stringstream ss;
ss << "Device does not support required features:" << PPX_LOG_ENDL;
for (const auto& elem : missingFeatures) {
ss << " " << elem << PPX_LOG_ENDL;
}
PPX_ASSERT_MSG(false, ss.str());
return ppx::ERROR_REQUIRED_FEATURE_UNAVAILABLE;
}

features.shaderUniformBufferArrayDynamicIndexing = VK_TRUE;
features.shaderSampledImageArrayDynamicIndexing = VK_TRUE;
features.shaderStorageBufferArrayDynamicIndexing = VK_TRUE;
features.shaderStorageImageArrayDynamicIndexing = VK_TRUE;

return ppx::SUCCESS;
}

Expand Down Expand Up @@ -389,7 +421,7 @@ Result Device::CreateApiObjects(const grfx::DeviceCreateInfo* pCreateInfo)
if (vkres != VK_SUCCESS) {
// clang-format off
std::stringstream ss;
ss << "vkCreateInstance failed: " << ToString(vkres);
ss << "vkCreateDevice failed: " << ToString(vkres);
if (vkres == VK_ERROR_EXTENSION_NOT_PRESENT) {
std::vector<std::string> missing = GetNotFound(mExtensions, mFoundExtensions);
ss << PPX_LOG_ENDL;
Expand Down

0 comments on commit 899e05d

Please sign in to comment.