Make descriptorIndexingVariableDescriptorCount feature optional.

Workarounding Metal Argument Buffer bug.
stripe2933 · Feb 23, 2025 · 69d7c2b · 69d7c2b
1 parent 8918a90
commit 69d7c2b
Show file tree

Hide file tree

Showing 5 changed files with 86 additions and 21 deletions.
diff --git a/README.md b/README.md
@@ -112,14 +112,14 @@ The extensions and feature used in this application are quite common in the mode
     - `bufferDeviceAddress`
     - `descriptorIndexing`
     - `descriptorBindingSampledImageUpdateAfterBind`
-    - `descriptorBindingVariableDescriptorCount`
     - `runtimeDescriptorArray`
     - `storageBuffer8BitAccess`
     - `uniformAndStorageBuffer8BitAccess`
     - `scalarBlockLayout`
     - `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
@@ -183,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 {
@@ -202,7 +210,7 @@ auto vk_gltf_viewer::vulkan::Gpu::createDevice() -> vk::raii::Device {
             .setBufferDeviceAddress(true)
             .setDescriptorIndexing(true)
             .setDescriptorBindingSampledImageUpdateAfterBind(true)
-            .setDescriptorBindingVariableDescriptorCount(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,6 +132,7 @@ namespace vk_gltf_viewer::vulkan {
         std::variant<ag::Swapchain, std::reference_wrapper<ag::Swapchain>> imGuiSwapchainAttachmentGroup;
 
         // Descriptor pools.
+        std::optional<vk::raii::DescriptorPool> assetDescriptorPool; // nullopt if Gpu::supportVariableDescriptorCount is true.
         vk::raii::DescriptorPool descriptorPool;
 
         // Descriptor sets.
@@ -153,7 +154,14 @@ namespace vk_gltf_viewer::vulkan {
             , cubeIndices { gpu.allocator }
             , cubemapSampler { gpu.device }
             , brdfLutSampler { gpu.device }
-            , assetDescriptorSetLayout { gpu }
+            , 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 }
@@ -165,11 +173,15 @@ namespace vk_gltf_viewer::vulkan {
             , weightedBlendedCompositionRenderer { gpu.device, sceneRenderPass }
             , swapchainAttachmentGroup { gpu, swapchainExtent, swapchainImages }
             , imGuiSwapchainAttachmentGroup { getImGuiSwapchainAttachmentGroup() }
-            , descriptorPool {
-                gpu.device,
-                getPoolSizes(imageBasedLightingDescriptorSetLayout, skyboxDescriptorSetLayout, assetDescriptorSetLayout)
-                    .getDescriptorPoolCreateInfo(vk::DescriptorPoolCreateFlagBits::eFreeDescriptorSet | vk::DescriptorPoolCreateFlagBits::eUpdateAfterBind),
-            }
+            , 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(imageBasedLightingDescriptorSet, skyboxDescriptorSet)
                 = vku::allocateDescriptorSets(*gpu.device, *descriptorPool, std::tie(
@@ -246,19 +258,39 @@ namespace vk_gltf_viewer::vulkan {
             }
 
             const GltfAsset &inner = gltfAsset.emplace(asset, directory, nodeWorldTransforms, orderedPrimitives, gpu, adapter);
-            (*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],
-            };
+            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;
             imageInfos.reserve(textureCount);

diff --git a/interface/vulkan/descriptor_set_layout/Asset.cppm b/interface/vulkan/descriptor_set_layout/Asset.cppm
@@ -29,6 +29,30 @@ namespace vk_gltf_viewer::vulkan::dsl {
                 },
             }.get() } { }
 
+        Asset(const Gpu &gpu [[clang::lifetimebound]], std::uint32_t textureCount)
+            : 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 },
+                        { textureCount, vk::ShaderStageFlagBits::eFragment })),
+                },
+                vk::DescriptorSetLayoutBindingFlagsCreateInfo {
+                    vku::unsafeProxy<vk::DescriptorBindingFlags>({
+                        {},
+                        {},
+                        {},
+                        {},
+                        {},
+                        vk::DescriptorBindingFlagBits::eUpdateAfterBind,
+                    }),
+                },
+            }.get() } { }
+
         /**
          * Get maximum available texture count for asset, including the fallback texture.
          * @param gpu The GPU object that is storing <tt>maxPerStageDescriptorUpdateAfterBindSamplers</tt> which have been retrieved from physical device selection.