Use variableDescriptorCount to avoid the pipeline recreation. (#21)

* Use variableDescriptorCount to avoid the pipeline recreation. * Make descriptorIndexingVariableDescriptorCount feature optional. Workarounding Metal Argument Buffer bug.
stripe2933 · Feb 23, 2025 · bcefb3d · bcefb3d
1 parent 7c7e686
commit bcefb3d
Show file tree

Hide file tree

Showing 5 changed files with 86 additions and 26 deletions.
diff --git a/README.md b/README.md
@@ -119,6 +119,7 @@ The extensions and feature used in this application are quite common in the mode
     - `timelineSemaphore`
     - `shaderInt8`
     - (optional) `drawIndirectCount` (If not presented, GPU frustum culling will be unavailable and fallback to the CPU frustum culling.)
+    - (optional) `descriptorBindingVariableDescriptorCount` (If not presented, graphics pipelines are dependent to the asset texture count; for every asset loading, the pipelines will be recreated as their texture count is changing.)
   - `VkPhysicalDeviceDynamicRenderingFeatures`
   - `VkPhysicalDeviceSynchronization2Features`
   - `VkPhysicalDeviceExtendedDynamicStateFeaturesEXT`

diff --git a/impl/vulkan/Gpu.cpp b/impl/vulkan/Gpu.cpp
@@ -113,6 +113,7 @@ auto vk_gltf_viewer::vulkan::Gpu::selectPhysicalDevice(const vk::raii::Instance
             !vulkan12Features.bufferDeviceAddress ||
             !vulkan12Features.descriptorIndexing ||
             !vulkan12Features.descriptorBindingSampledImageUpdateAfterBind ||
+            !vulkan12Features.descriptorBindingVariableDescriptorCount ||
             !vulkan12Features.runtimeDescriptorArray ||
             !vulkan12Features.storageBuffer8BitAccess ||
             !vulkan12Features.uniformAndStorageBuffer8BitAccess ||
@@ -182,6 +183,14 @@ auto vk_gltf_viewer::vulkan::Gpu::createDevice() -> vk::raii::Device {
 
     supportDrawIndirectCount = availableFeatures.template get<vk::PhysicalDeviceVulkan12Features>().drawIndirectCount;
     supportUint8Index = availableFeatures.template get<vk::PhysicalDeviceIndexTypeUint8FeaturesKHR>().indexTypeUint8;
+#if __APPLE__
+    // MoltenVK with Metal Argument Buffer does not work with variable descriptor count.
+    // Tracked issue: https://github.com/KhronosGroup/MoltenVK/issues/2343
+    // TODO: Remove this workaround when the issue is fixed.
+    supportVariableDescriptorCount = false;
+#else
+    supportVariableDescriptorCount = availableFeatures.template get<vk::PhysicalDeviceVulkan12Features>().descriptorBindingVariableDescriptorCount;
+#endif
 
 	const vku::RefHolder queueCreateInfos = Queues::getCreateInfos(physicalDevice, queueFamilies);
     vk::StructureChain createInfo {
@@ -201,6 +210,7 @@ auto vk_gltf_viewer::vulkan::Gpu::createDevice() -> vk::raii::Device {
             .setBufferDeviceAddress(true)
             .setDescriptorIndexing(true)
             .setDescriptorBindingSampledImageUpdateAfterBind(true)
+            .setDescriptorBindingVariableDescriptorCount(supportVariableDescriptorCount)
             .setRuntimeDescriptorArray(true)
             .setStorageBuffer8BitAccess(true)
             .setUniformAndStorageBuffer8BitAccess(true)

diff --git a/interface/vulkan/Gpu.cppm b/interface/vulkan/Gpu.cppm
@@ -71,6 +71,7 @@ namespace vk_gltf_viewer::vulkan {
         std::uint32_t subgroupSize;
         std::uint32_t maxPerStageDescriptorUpdateAfterBindSamplers;
         bool supportShaderImageLoadStoreLod;
+        bool supportVariableDescriptorCount;
 
         Gpu(const vk::raii::Instance &instance [[clang::lifetimebound]], vk::SurfaceKHR surface);
         ~Gpu();

diff --git a/interface/vulkan/SharedData.cppm b/interface/vulkan/SharedData.cppm
@@ -132,7 +132,7 @@ namespace vk_gltf_viewer::vulkan {
         std::variant<ag::Swapchain, std::reference_wrapper<ag::Swapchain>> imGuiSwapchainAttachmentGroup;
 
         // Descriptor pools.
-        vk::raii::DescriptorPool textureDescriptorPool;
+        std::optional<vk::raii::DescriptorPool> assetDescriptorPool; // nullopt if Gpu::supportVariableDescriptorCount is true.
         vk::raii::DescriptorPool descriptorPool;
 
         // Descriptor sets.
@@ -154,7 +154,14 @@ namespace vk_gltf_viewer::vulkan {
             , cubeIndices { gpu.allocator }
             , cubemapSampler { gpu.device }
             , brdfLutSampler { gpu.device }
-            , assetDescriptorSetLayout { gpu, 1 } // TODO: set proper initial texture count.
+            , assetDescriptorSetLayout { [&]() {
+                if (gpu.supportVariableDescriptorCount) {
+                    return dsl::Asset { gpu };
+                }
+                else {
+                    return dsl::Asset { gpu, 1 }; // TODO: set proper initial texture count.
+                }
+            }() }
             , imageBasedLightingDescriptorSetLayout { gpu.device, cubemapSampler, brdfLutSampler }
             , skyboxDescriptorSetLayout { gpu.device, cubemapSampler }
             , sceneRenderPass { gpu.device }
@@ -166,12 +173,16 @@ namespace vk_gltf_viewer::vulkan {
             , weightedBlendedCompositionRenderer { gpu.device, sceneRenderPass }
             , swapchainAttachmentGroup { gpu, swapchainExtent, swapchainImages }
             , imGuiSwapchainAttachmentGroup { getImGuiSwapchainAttachmentGroup() }
-            , textureDescriptorPool { createTextureDescriptorPool() }
-            , descriptorPool { gpu.device, getPoolSizes(imageBasedLightingDescriptorSetLayout, skyboxDescriptorSetLayout).getDescriptorPoolCreateInfo() }
+            , descriptorPool { gpu.device, [&]() {
+                if (gpu.supportVariableDescriptorCount) {
+                    return getPoolSizes(imageBasedLightingDescriptorSetLayout, skyboxDescriptorSetLayout, assetDescriptorSetLayout)
+                        .getDescriptorPoolCreateInfo(vk::DescriptorPoolCreateFlagBits::eFreeDescriptorSet | vk::DescriptorPoolCreateFlagBits::eUpdateAfterBind);
+                }
+                else {
+                    return getPoolSizes(imageBasedLightingDescriptorSetLayout, skyboxDescriptorSetLayout).getDescriptorPoolCreateInfo();
+                }
+            }().get() }
             , fallbackTexture { gpu }{
-            std::tie(assetDescriptorSet)
-                = vku::allocateDescriptorSets(*gpu.device, *textureDescriptorPool, std::tie(
-                    assetDescriptorSetLayout));
             std::tie(imageBasedLightingDescriptorSet, skyboxDescriptorSet)
                 = vku::allocateDescriptorSets(*gpu.device, *descriptorPool, std::tie(
                     imageBasedLightingDescriptorSetLayout,
@@ -247,21 +258,38 @@ namespace vk_gltf_viewer::vulkan {
             }
 
             const GltfAsset &inner = gltfAsset.emplace(asset, directory, nodeWorldTransforms, orderedPrimitives, gpu, adapter);
-            if (assetDescriptorSetLayout.descriptorCounts[5] != textureCount) {
-                // If texture count is different, descriptor set layouts, pipeline layouts and pipelines have to be recreated.
-                depthPipelines.clear();
-                maskDepthPipelines.clear();
-                jumpFloodSeedPipelines.clear();
-                maskJumpFloodSeedPipelines.clear();
-                primitivePipelines.clear();
-                unlitPrimitivePipelines.clear();
-
-                assetDescriptorSetLayout = { gpu, textureCount };
-                primitivePipelineLayout = { gpu.device, std::tie(imageBasedLightingDescriptorSetLayout, assetDescriptorSetLayout) };
-                primitiveNoShadingPipelineLayout = { gpu.device, assetDescriptorSetLayout };
-
-                textureDescriptorPool = createTextureDescriptorPool();
-                std::tie(assetDescriptorSet) = vku::allocateDescriptorSets(*gpu.device, *textureDescriptorPool, std::tie(assetDescriptorSetLayout));
+            if (gpu.supportVariableDescriptorCount) {
+                (*gpu.device).freeDescriptorSets(*descriptorPool, assetDescriptorSet);
+                assetDescriptorSet = decltype(assetDescriptorSet) {
+                    vku::unsafe,
+                    (*gpu.device).allocateDescriptorSets(vk::StructureChain {
+                         vk::DescriptorSetAllocateInfo {
+                             *descriptorPool,
+                             *assetDescriptorSetLayout,
+                         },
+                         vk::DescriptorSetVariableDescriptorCountAllocateInfo {
+                             vk::ArrayProxyNoTemporaries<const std::uint32_t> { textureCount },
+                         }
+                     }.get())[0],
+                };
+            }
+            else {
+                if (assetDescriptorSetLayout.descriptorCounts[5] != textureCount) {
+                    // If texture count is different, descriptor set layouts, pipeline layouts and pipelines have to be recreated.
+                    depthPipelines.clear();
+                    maskDepthPipelines.clear();
+                    jumpFloodSeedPipelines.clear();
+                    maskJumpFloodSeedPipelines.clear();
+                    primitivePipelines.clear();
+                    unlitPrimitivePipelines.clear();
+
+                    assetDescriptorSetLayout = { gpu, textureCount };
+                    primitivePipelineLayout = { gpu.device, std::tie(imageBasedLightingDescriptorSetLayout, assetDescriptorSetLayout) };
+                    primitiveNoShadingPipelineLayout = { gpu.device, assetDescriptorSetLayout };
+
+                    assetDescriptorPool.emplace(gpu.device, getPoolSizes(assetDescriptorSetLayout).getDescriptorPoolCreateInfo(vk::DescriptorPoolCreateFlagBits::eUpdateAfterBind));
+                    std::tie(assetDescriptorSet) = vku::allocateDescriptorSets(*gpu.device, *assetDescriptorPool, std::tie(assetDescriptorSetLayout));
+                }
             }
 
             std::vector<vk::DescriptorImageInfo> imageInfos;
@@ -307,9 +335,5 @@ namespace vk_gltf_viewer::vulkan {
                 return decltype(imGuiSwapchainAttachmentGroup) { std::in_place_index<1>, swapchainAttachmentGroup };
             }
         }
-
-        [[nodiscard]] auto createTextureDescriptorPool() const -> vk::raii::DescriptorPool {
-            return { gpu.device, getPoolSizes(assetDescriptorSetLayout).getDescriptorPoolCreateInfo(vk::DescriptorPoolCreateFlagBits::eUpdateAfterBind) };
-        }
     };
 }
diff --git a/interface/vulkan/descriptor_set_layout/Asset.cppm b/interface/vulkan/descriptor_set_layout/Asset.cppm
@@ -5,6 +5,30 @@ export import :vulkan.Gpu;
 
 namespace vk_gltf_viewer::vulkan::dsl {
     export struct Asset : vku::DescriptorSetLayout<vk::DescriptorType::eStorageBuffer, vk::DescriptorType::eStorageBuffer, vk::DescriptorType::eStorageBuffer, vk::DescriptorType::eStorageBuffer, vk::DescriptorType::eStorageBuffer, vk::DescriptorType::eCombinedImageSampler> {
+        explicit Asset(const Gpu &gpu [[clang::lifetimebound]])
+            : DescriptorSetLayout { gpu.device, vk::StructureChain {
+                vk::DescriptorSetLayoutCreateInfo {
+                    vk::DescriptorSetLayoutCreateFlagBits::eUpdateAfterBindPool,
+                    vku::unsafeProxy(getBindings(
+                        { 1, vk::ShaderStageFlagBits::eVertex },
+                        { 1, vk::ShaderStageFlagBits::eVertex },
+                        { 1, vk::ShaderStageFlagBits::eVertex },
+                        { 1, vk::ShaderStageFlagBits::eVertex },
+                        { 1, vk::ShaderStageFlagBits::eVertex | vk::ShaderStageFlagBits::eFragment },
+                        { maxTextureCount(gpu), vk::ShaderStageFlagBits::eFragment })),
+                },
+                vk::DescriptorSetLayoutBindingFlagsCreateInfo {
+                    vku::unsafeProxy<vk::DescriptorBindingFlags>({
+                        {},
+                        {},
+                        {},
+                        {},
+                        {},
+                        vk::DescriptorBindingFlagBits::eUpdateAfterBind | vk::DescriptorBindingFlagBits::eVariableDescriptorCount,
+                    }),
+                },
+            }.get() } { }
+
         Asset(const Gpu &gpu [[clang::lifetimebound]], std::uint32_t textureCount)
             : DescriptorSetLayout { gpu.device, vk::StructureChain {
                 vk::DescriptorSetLayoutCreateInfo {