From ba3ee37ab313280e3dd03a170ad9da057d2310e4 Mon Sep 17 00:00:00 2001 From: Robert Osfield Date: Thu, 16 Apr 2026 15:51:59 +0100 Subject: [PATCH] Refactore how GraphicsPipelineCongfigurator::compare() handles the shaderHints->defines and descriptorConfigurator->defines together to avoid failure to match compatible configurations. --- include/vsg/state/ShaderModule.h | 2 + src/vsg/state/ShaderModule.cpp | 12 ++- .../utils/GraphicsPipelineConfigurator.cpp | 96 ++++++++++++++++++- 3 files changed, 104 insertions(+), 6 deletions(-) diff --git a/include/vsg/state/ShaderModule.h b/include/vsg/state/ShaderModule.h index a57ff2e295..c55e2407ae 100644 --- a/include/vsg/state/ShaderModule.h +++ b/include/vsg/state/ShaderModule.h @@ -59,6 +59,8 @@ namespace vsg public: ref_ptr clone(const CopyOp& copyop = {}) const override { return ShaderCompileSettings::create(*this, copyop); } + + virtual int compare_except_defines(const Object& rhs_object) const; int compare(const Object& rhs_object) const override; void read(Input& input) override; diff --git a/src/vsg/state/ShaderModule.cpp b/src/vsg/state/ShaderModule.cpp index e3f566e0e3..2a666ba3a6 100644 --- a/src/vsg/state/ShaderModule.cpp +++ b/src/vsg/state/ShaderModule.cpp @@ -40,7 +40,7 @@ ShaderCompileSettings::ShaderCompileSettings(const ShaderCompileSettings& rhs, c { } -int ShaderCompileSettings::compare(const Object& rhs_object) const +int ShaderCompileSettings::compare_except_defines(const Object& rhs_object) const { int result = Object::compare(rhs_object); if (result != 0) return result; @@ -54,7 +54,15 @@ int ShaderCompileSettings::compare(const Object& rhs_object) const if ((result = compare_value(target, rhs.target))) return result; if ((result = compare_value(forwardCompatible, rhs.forwardCompatible))) return result; if ((result = compare_value(generateDebugInfo, rhs.generateDebugInfo))) return result; - if ((result = compare_value(optimize, rhs.optimize))) return result; + return compare_value(optimize, rhs.optimize); +} + +int ShaderCompileSettings::compare(const Object& rhs_object) const +{ + int result = compare_except_defines(rhs_object); + if (result != 0) return result; + + const auto& rhs = static_cast(rhs_object); return compare_container(defines, rhs.defines); } diff --git a/src/vsg/utils/GraphicsPipelineConfigurator.cpp b/src/vsg/utils/GraphicsPipelineConfigurator.cpp index c15e3aa99c..0349d2b912 100644 --- a/src/vsg/utils/GraphicsPipelineConfigurator.cpp +++ b/src/vsg/utils/GraphicsPipelineConfigurator.cpp @@ -535,8 +535,98 @@ int GraphicsPipelineConfigurator::compare(const Object& rhs_object) const if ((result = compare_value(baseAttributeBinding, rhs.baseAttributeBinding))) return result; if ((result = compare_pointer(shaderSet, rhs.shaderSet))) return result; - if ((result = compare_pointer(shaderHints, rhs.shaderHints))) return result; - if ((result = compare_pointer_container(inheritedState, rhs.inheritedState))) return result; + // + // defines settings for ShaderHints may not yet be in final state, so comparing them between compatible GraphicsPipelineConfigurator + // objects can lead is not returning a match when just comparing shaderHint->defines. + // To resolve this do the shint hint compare wihtout the defines, and then do the comparison of defines + // by handling both the respective descriptorConfigurator->defines and shaderHints->defines when comparing to the rhs. + // + // The compare_shaderHints, IteratorPair and Iterator structs all exist in service of this mutliset comparison. + // + + auto compare_shaderHints = [](const ref_ptr& lsh, const ref_ptr& rsh) -> int + { + if (lsh == rsh) return 0; + return lsh ? (rsh ? lsh->compare_except_defines(*rsh) : 1) : (rsh ? -1 : 0); + }; + + if ((result = compare_shaderHints(shaderHints, rhs.shaderHints))) return result; + + struct IteratorPair + { + using iterator = std::set::const_iterator; + iterator itr; + iterator end; + + explicit IteratorPair(const std::set& values) : itr(values.begin()), end(values.end()) {} + + bool valid() const { return itr != end; } + }; + + struct Iterator + { + IteratorPair lhs; + IteratorPair rhs; + + explicit Iterator(IteratorPair in_lhs, IteratorPair in_rhs) : lhs(in_lhs), rhs(in_rhs) {} + + bool valid() const { return lhs.valid() || rhs.valid(); } + + /// only call when valid() return true + const std::string& value() const + { + if (lhs.valid()) + { + if (rhs.valid()) + { + if (*lhs.itr < *rhs.itr) return *lhs.itr; + else if (*rhs.itr < *lhs.itr ) return *rhs.itr; + else { return *rhs.itr; } + } + else return *lhs.itr; + } + else return *rhs.itr; + } + + bool advance() + { + if (lhs.valid()) + { + if (rhs.valid()) + { + if (*lhs.itr < *rhs.itr) lhs.itr++; + else if (*rhs.itr < *lhs.itr) rhs.itr++; + else { ++lhs.itr; ++rhs.itr; } + } + else lhs.itr++; + } + else if (rhs.valid()) rhs.itr++; + + return valid(); + } + }; + + auto local_compare = [](Iterator ilhs, Iterator irhs) -> int + { + while(ilhs.valid() && irhs.valid()) + { + const auto& lhs_value = ilhs.value(); + const auto& rhs_value = irhs.value(); + if (lhs_value < rhs_value) return -1; + else if (lhs_value > rhs_value) return 1; + + ilhs.advance(); + irhs.advance(); + } + + if (ilhs.valid()) return 1; + return irhs.valid() ? -1 : 0; + }; + + result = local_compare(Iterator(IteratorPair(descriptorConfigurator->defines), IteratorPair(shaderHints->defines)), + Iterator(IteratorPair(rhs.descriptorConfigurator->defines), IteratorPair(rhs.shaderHints->defines))); + + if (result) return result; return compare_pointer(descriptorConfigurator, rhs.descriptorConfigurator); } @@ -612,8 +702,6 @@ void GraphicsPipelineConfigurator::_assignInheritedSets() void GraphicsPipelineConfigurator::init() { - // if (!descriptorConfigurator) descriptorConfigurator = DescriptorConfigurator::create(shaderSet); - _assignInheritedSets(); if (descriptorConfigurator)