Commit 53407521 by Malcolm Bechard

Fix issue with remapping global uniform blocks

Avoid adding global uniform blocks to stages that don't already have it. Otherwise multiple stages point to the same block object, and a remapping that occurs later on will change the mapping on multiple stages.
parent 48f08c60
#version 460
layout(location = 0) out vec4 fragColor;
uniform sampler2D sTexture;
in vec4 Color;
in vec2 UV;
void main()
{
fragColor = Color * texture(sTexture, UV.st).r;
}
#version 460
in vec2 aPos;
in vec2 aUV;
in vec4 aColor;
uniform mat4 projectionMatrix;
out vec4 Color;
out vec2 UV;
void main()
{
Color = aColor;
UV = aUV;
gl_Position = projectionMatrix * vec4(aPos, 0, 1);
}
......@@ -2124,7 +2124,12 @@ bool TProgram::crossStageCheck(EShMessages) {
// copy final definition of global block back into each stage
for (unsigned int i = 0; i < activeStages.size(); ++i) {
activeStages[i]->mergeGlobalUniformBlocks(*infoSink, uniforms);
// We only want to merge into already existing global uniform blocks.
// A stage that doesn't already know about the global doesn't care about it's content.
// Otherwise we end up pointing to the same object between different stages
// and that will break binding/set remappings
bool mergeExistingOnly = true;
activeStages[i]->mergeGlobalUniformBlocks(*infoSink, uniforms, mergeExistingOnly);
}
// compare cross stage symbols for each stage boundary
......
......@@ -108,7 +108,8 @@ void TIntermediate::mergeUniformObjects(TInfoSink& infoSink, TIntermediate& unit
unitLinkerObjects.resize(end - unitLinkerObjects.begin());
// merge uniforms and do error checking
mergeGlobalUniformBlocks(infoSink, unit);
bool mergeExistingOnly = false;
mergeGlobalUniformBlocks(infoSink, unit, mergeExistingOnly);
mergeLinkerObjects(infoSink, linkerObjects, unitLinkerObjects, unit.getStage());
}
......@@ -362,7 +363,8 @@ void TIntermediate::mergeTrees(TInfoSink& infoSink, TIntermediate& unit)
remapIds(idMaps, idShift + 1, unit);
mergeBodies(infoSink, globals, unitGlobals);
mergeGlobalUniformBlocks(infoSink, unit);
bool mergeExistingOnly = false;
mergeGlobalUniformBlocks(infoSink, unit, mergeExistingOnly);
mergeLinkerObjects(infoSink, linkerObjects, unitLinkerObjects, unit.getStage());
ioAccessed.insert(unit.ioAccessed.begin(), unit.ioAccessed.end());
}
......@@ -525,7 +527,7 @@ static inline bool isSameInterface(TIntermSymbol* symbol, EShLanguage stage, TIn
// merge the members of different stages to allow them to be linked properly
// as a single block
//
void TIntermediate::mergeGlobalUniformBlocks(TInfoSink& infoSink, TIntermediate& unit)
void TIntermediate::mergeGlobalUniformBlocks(TInfoSink& infoSink, TIntermediate& unit, bool mergeExistingOnly)
{
TIntermSequence& linkerObjects = findLinkerObjects()->getSequence();
TIntermSequence& unitLinkerObjects = unit.findLinkerObjects()->getSequence();
......@@ -552,7 +554,7 @@ void TIntermediate::mergeGlobalUniformBlocks(TInfoSink& infoSink, TIntermediate&
auto itUnitBlock = unitDefaultBlocks.begin();
for (; itUnitBlock != unitDefaultBlocks.end(); itUnitBlock++) {
bool add = true;
bool add = !mergeExistingOnly;
auto itBlock = defaultBlocks.begin();
for (; itBlock != defaultBlocks.end(); itBlock++) {
......
......@@ -915,7 +915,7 @@ public:
void merge(TInfoSink&, TIntermediate&);
void finalCheck(TInfoSink&, bool keepUncalled);
void mergeGlobalUniformBlocks(TInfoSink& infoSink, TIntermediate& unit);
void mergeGlobalUniformBlocks(TInfoSink& infoSink, TIntermediate& unit, bool mergeExistingOnly);
void mergeUniformObjects(TInfoSink& infoSink, TIntermediate& unit);
void checkStageIO(TInfoSink&, TIntermediate&);
......
......@@ -48,6 +48,7 @@ namespace {
struct vkRelaxedData {
std::vector<std::string> fileNames;
std::vector<std::vector<std::string>> resourceSetBindings;
};
using VulkanRelaxedTest = GlslangTest <::testing::TestWithParam<vkRelaxedData>>;
......@@ -191,6 +192,7 @@ bool verifyIOMapping(std::string& linkingError, glslang::TProgram& program) {
TEST_P(VulkanRelaxedTest, FromFile)
{
const auto& fileNames = GetParam().fileNames;
const auto& resourceSetBindings = GetParam().resourceSetBindings;
Semantics semantics = Semantics::Vulkan;
const size_t fileCount = fileNames.size();
const EShMessages controls = DeriveOptions(Source::GLSL, semantics, Target::BothASTAndSpv);
......@@ -230,6 +232,12 @@ TEST_P(VulkanRelaxedTest, FromFile)
result.linkingOutput = program.getInfoLog();
result.linkingError = program.getInfoDebugLog();
if (!resourceSetBindings.empty()) {
assert(resourceSetBindings.size() == fileNames.size());
for (int i = 0; i < shaders.size(); i++)
shaders[i]->setResourceSetBinding(resourceSetBindings[i]);
}
unsigned int stage = 0;
glslang::TIntermediate* firstIntermediate = nullptr;
while (!program.getIntermediate((EShLanguage)stage) && stage < EShLangCount) { stage++; }
......@@ -287,6 +295,7 @@ INSTANTIATE_TEST_SUITE_P(
{{"vk.relaxed.link1.frag", "vk.relaxed.link2.frag"}},
{{"vk.relaxed.stagelink.vert", "vk.relaxed.stagelink.frag"}},
{{"vk.relaxed.errorcheck.vert", "vk.relaxed.errorcheck.frag"}},
{{"vk.relaxed.changeSet.vert", "vk.relaxed.changeSet.frag" }, { {"0"}, {"1"} } },
}))
);
// clang-format on
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment