From 3aa6720f3c2f04338ef7c7831a1f4c4b0340712f Mon Sep 17 00:00:00 2001 From: AnyOldName3 Date: Fri, 4 Apr 2025 19:52:20 +0100 Subject: [PATCH 1/3] Use partial pipeline layout compatibility for inherited state Otherwise you can't have nice things like the same view descriptor set for all shadersets. --- include/vsg/utils/ShaderSet.h | 5 +- .../utils/GraphicsPipelineConfigurator.cpp | 6 +-- src/vsg/utils/ShaderSet.cpp | 46 +++++++++++++++++++ 3 files changed, 53 insertions(+), 4 deletions(-) diff --git a/include/vsg/utils/ShaderSet.h b/include/vsg/utils/ShaderSet.h index e3bf15ac4d..5cba36a42d 100644 --- a/include/vsg/utils/ShaderSet.h +++ b/include/vsg/utils/ShaderSet.h @@ -177,9 +177,12 @@ namespace vsg /// create the descriptor set layout. virtual ref_ptr createDescriptorSetLayout(const std::set& defines, uint32_t set) const; - /// return true of specified pipeline layout is compatible with what is required for this ShaderSet + /// return true if specified pipeline layout is compatible with what is required for this ShaderSet virtual bool compatiblePipelineLayout(const PipelineLayout& layout, const std::set& defines) const; + /// return true if specified pipeline layout is partially compatible with what is required for this ShaderSet + virtual bool partiallyCompatiblePipelineLayout(const PipelineLayout& layout, const std::set& defines, bool onlyPushConstants, uint32_t descriptorSet) const; + /// create the pipeline layout for all descriptor sets enabled by specified defines or required by default. inline ref_ptr createPipelineLayout(const std::set& defines) { return createPipelineLayout(defines, descriptorSetRange()); } diff --git a/src/vsg/utils/GraphicsPipelineConfigurator.cpp b/src/vsg/utils/GraphicsPipelineConfigurator.cpp index 698139c635..34ea0328b2 100644 --- a/src/vsg/utils/GraphicsPipelineConfigurator.cpp +++ b/src/vsg/utils/GraphicsPipelineConfigurator.cpp @@ -554,7 +554,7 @@ void GraphicsPipelineConfigurator::_assignInheritedSets() { if (!bds.descriptorSet || !bds.descriptorSet->setLayout || !gpc.descriptorConfigurator) return; - if (gpc.shaderSet->compatiblePipelineLayout(*bds.layout, gpc.shaderHints->defines)) + if (gpc.shaderSet->partiallyCompatiblePipelineLayout(*bds.layout, gpc.shaderHints->defines, true, bds.firstSet)) { gpc.inheritedSets.insert(bds.firstSet); } @@ -564,7 +564,7 @@ void GraphicsPipelineConfigurator::_assignInheritedSets() { if (!gpc.descriptorConfigurator) return; - if (gpc.shaderSet->compatiblePipelineLayout(*bds.layout, gpc.shaderHints->defines)) + if (gpc.shaderSet->partiallyCompatiblePipelineLayout(*bds.layout, gpc.shaderHints->defines, true, bds.firstSet + bds.descriptorSets.size() - 1)) { for (size_t i = 0; i < bds.descriptorSets.size(); ++i) { @@ -575,7 +575,7 @@ void GraphicsPipelineConfigurator::_assignInheritedSets() void apply(const BindViewDescriptorSets& bvds) override { - if (!gpc.shaderSet->compatiblePipelineLayout(*bvds.layout, gpc.shaderHints->defines)) + if (!gpc.shaderSet->partiallyCompatiblePipelineLayout(*bvds.layout, gpc.shaderHints->defines, true, bvds.firstSet)) { return; } diff --git a/src/vsg/utils/ShaderSet.cpp b/src/vsg/utils/ShaderSet.cpp index 6441aa5b04..bc17d231c9 100644 --- a/src/vsg/utils/ShaderSet.cpp +++ b/src/vsg/utils/ShaderSet.cpp @@ -561,6 +561,13 @@ bool ShaderSet::compatiblePipelineLayout(const PipelineLayout& layout, const std ++set; } +#ifdef VK_EXT_graphics_pipeline_library + if (layout.flags & VK_PIPELINE_LAYOUT_CREATE_INDEPENDENT_SETS_BIT_EXT) + { + return false; + } +#endif + PushConstantRanges ranges; for (auto& pcr : pushConstantRanges) { @@ -578,6 +585,45 @@ bool ShaderSet::compatiblePipelineLayout(const PipelineLayout& layout, const std return true; } +bool vsg::ShaderSet::partiallyCompatiblePipelineLayout(const PipelineLayout& layout, const std::set& defines, bool onlyPushConstants, uint32_t descriptorSet) const +{ + PushConstantRanges ranges; + for (auto& pcr : pushConstantRanges) + { + if (pcr.define.empty() || defines.count(pcr.define) == 1) + { + ranges.push_back(pcr.range); + } + } + + if (compare_value_container(layout.pushConstantRanges, ranges) != 0) + { + return false; + } + + if (onlyPushConstants) + { + return true; + } + +#ifdef VK_EXT_graphics_pipeline_library + if (layout.flags & VK_PIPELINE_LAYOUT_CREATE_INDEPENDENT_SETS_BIT_EXT) + { + return false; + } +#endif + + for (uint32_t set = 0; set <= std::min(layout.setLayouts.size() - 1, size_t(descriptorSet)); ++set) + { + if (layout.setLayouts[set] && !compatibleDescriptorSetLayout(*layout.setLayouts[set], defines, set)) + { + return false; + } + } + + return true; +} + ref_ptr ShaderSet::createPipelineLayout(const std::set& defines, std::pair range) const { DescriptorSetLayouts descriptorSetLayouts; From 1a6ec2ca5fcb3c0fab58e679a678a6a4e3b0a466 Mon Sep 17 00:00:00 2001 From: AnyOldName3 Date: Tue, 8 Apr 2025 18:36:26 +0100 Subject: [PATCH 2/3] Fix incorrect parameter and narrowing conversion warning --- src/vsg/utils/GraphicsPipelineConfigurator.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/vsg/utils/GraphicsPipelineConfigurator.cpp b/src/vsg/utils/GraphicsPipelineConfigurator.cpp index 34ea0328b2..8e30fde887 100644 --- a/src/vsg/utils/GraphicsPipelineConfigurator.cpp +++ b/src/vsg/utils/GraphicsPipelineConfigurator.cpp @@ -554,7 +554,7 @@ void GraphicsPipelineConfigurator::_assignInheritedSets() { if (!bds.descriptorSet || !bds.descriptorSet->setLayout || !gpc.descriptorConfigurator) return; - if (gpc.shaderSet->partiallyCompatiblePipelineLayout(*bds.layout, gpc.shaderHints->defines, true, bds.firstSet)) + if (gpc.shaderSet->partiallyCompatiblePipelineLayout(*bds.layout, gpc.shaderHints->defines, false, bds.firstSet)) { gpc.inheritedSets.insert(bds.firstSet); } @@ -564,7 +564,7 @@ void GraphicsPipelineConfigurator::_assignInheritedSets() { if (!gpc.descriptorConfigurator) return; - if (gpc.shaderSet->partiallyCompatiblePipelineLayout(*bds.layout, gpc.shaderHints->defines, true, bds.firstSet + bds.descriptorSets.size() - 1)) + if (gpc.shaderSet->partiallyCompatiblePipelineLayout(*bds.layout, gpc.shaderHints->defines, false, bds.firstSet + static_cast(bds.descriptorSets.size()) - 1)) { for (size_t i = 0; i < bds.descriptorSets.size(); ++i) { @@ -575,7 +575,7 @@ void GraphicsPipelineConfigurator::_assignInheritedSets() void apply(const BindViewDescriptorSets& bvds) override { - if (!gpc.shaderSet->partiallyCompatiblePipelineLayout(*bvds.layout, gpc.shaderHints->defines, true, bvds.firstSet)) + if (!gpc.shaderSet->partiallyCompatiblePipelineLayout(*bvds.layout, gpc.shaderHints->defines, false, bvds.firstSet)) { return; } From 8e575d75d5cabd50d349abadc9983260ca5bd8d6 Mon Sep 17 00:00:00 2001 From: AnyOldName3 Date: Wed, 9 Apr 2025 16:06:34 +0100 Subject: [PATCH 3/3] Make null checks match the new requirements --- src/vsg/utils/GraphicsPipelineConfigurator.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/vsg/utils/GraphicsPipelineConfigurator.cpp b/src/vsg/utils/GraphicsPipelineConfigurator.cpp index 8e30fde887..f886e9f028 100644 --- a/src/vsg/utils/GraphicsPipelineConfigurator.cpp +++ b/src/vsg/utils/GraphicsPipelineConfigurator.cpp @@ -552,7 +552,7 @@ void GraphicsPipelineConfigurator::_assignInheritedSets() void apply(const BindDescriptorSet& bds) override { - if (!bds.descriptorSet || !bds.descriptorSet->setLayout || !gpc.descriptorConfigurator) return; + if (!bds.layout || !gpc.descriptorConfigurator) return; if (gpc.shaderSet->partiallyCompatiblePipelineLayout(*bds.layout, gpc.shaderHints->defines, false, bds.firstSet)) { @@ -562,7 +562,7 @@ void GraphicsPipelineConfigurator::_assignInheritedSets() void apply(const BindDescriptorSets& bds) override { - if (!gpc.descriptorConfigurator) return; + if (!bds.layout || !gpc.descriptorConfigurator) return; if (gpc.shaderSet->partiallyCompatiblePipelineLayout(*bds.layout, gpc.shaderHints->defines, false, bds.firstSet + static_cast(bds.descriptorSets.size()) - 1)) {