From 2f9f3cd6a0cb69d84c7a3df15cd5fbd22d25e04c Mon Sep 17 00:00:00 2001 From: baldurk Date: Wed, 10 Apr 2024 21:28:48 +0100 Subject: [PATCH] Fix GL reflection unit tests * GL no longer reflects out fixed bindings and relies purely on queried uniforms. --- renderdoc/data/glsl_shaders.cpp | 97 ++++++++++++++----- .../driver/shaders/spirv/spirv_reflect.cpp | 21 ++-- 2 files changed, 84 insertions(+), 34 deletions(-) diff --git a/renderdoc/data/glsl_shaders.cpp b/renderdoc/data/glsl_shaders.cpp index 691d9aa6e..315e27a19 100644 --- a/renderdoc/data/glsl_shaders.cpp +++ b/renderdoc/data/glsl_shaders.cpp @@ -366,8 +366,12 @@ void main() { const ShaderResource &res = refl.readWriteResources[0]; INFO("read-write resource: " << res.name.c_str()); - CHECK(res.fixedBindSetOrSpace == 0); - CHECK(res.fixedBindNumber == 0); + // GLSL does not have register bindings as they're dynamic + if(testType != ShaderType::GLSL) + { + CHECK(res.fixedBindSetOrSpace == 0); + CHECK(res.fixedBindNumber == 0); + } CHECK(res.bindArraySize == 1); CHECK(res.textureType == TextureType::Buffer); CHECK(res.variableType.members.empty()); @@ -411,8 +415,12 @@ void main() { const ShaderSampler &samp = refl.samplers[0]; INFO("read-only resource: " << samp.name.c_str()); - CHECK(samp.fixedBindSetOrSpace == 1); - CHECK(samp.fixedBindNumber == 2); + // GLSL does not have register bindings as they're dynamic + if(testType != ShaderType::GLSL) + { + CHECK(samp.fixedBindSetOrSpace == 1); + CHECK(samp.fixedBindNumber == 2); + } CHECK(samp.bindArraySize == 1); } } @@ -424,8 +432,12 @@ void main() { const ShaderResource &res = refl.readOnlyResources[0]; INFO("read-only resource: " << res.name.c_str()); - CHECK(res.fixedBindSetOrSpace == 2); - CHECK(res.fixedBindNumber == 4); + // GLSL does not have register bindings as they're dynamic + if(testType != ShaderType::GLSL) + { + CHECK(res.fixedBindSetOrSpace == 2); + CHECK(res.fixedBindNumber == 4); + } CHECK(res.bindArraySize == 1); CHECK(res.textureType == TextureType::Texture2D); CHECK(res.variableType.members.empty()); @@ -437,8 +449,12 @@ void main() { const ShaderResource &res = refl.readOnlyResources[1]; INFO("read-only resource: " << res.name.c_str()); - CHECK(res.fixedBindSetOrSpace == 2); - CHECK(res.fixedBindNumber == 5); + // GLSL does not have register bindings as they're dynamic + if(testType != ShaderType::GLSL) + { + CHECK(res.fixedBindSetOrSpace == 2); + CHECK(res.fixedBindNumber == 5); + } CHECK(res.bindArraySize == 1); CHECK(res.textureType == TextureType::Texture2D); CHECK(res.variableType.members.empty()); @@ -474,7 +490,6 @@ void main() { const ConstantBlock &cblock = refl.constantBlocks[0]; INFO("UBO: " << cblock.name.c_str()); - CHECK(cblock.fixedBindNumber == 0); CHECK(cblock.bindArraySize == 1); CHECK(!cblock.bufferBacked); CHECK(cblock.compileConstants); @@ -837,8 +852,12 @@ void main() { const ConstantBlock &cblock = refl.constantBlocks[0]; INFO("UBO: " << cblock.name.c_str()); - CHECK(cblock.fixedBindSetOrSpace == 0); - CHECK(cblock.fixedBindNumber == 8); + // GLSL does not have register bindings as they're dynamic + if(testType != ShaderType::GLSL) + { + CHECK(cblock.fixedBindSetOrSpace == 0); + CHECK(cblock.fixedBindNumber == 8); + } CHECK(cblock.bindArraySize == 1); CHECK(cblock.bufferBacked); CHECK(cblock.byteSize == 272); @@ -1019,8 +1038,12 @@ void main() { const ShaderResource &res = refl.readOnlyResources[0]; INFO("read-only resource: " << res.name.c_str()); - CHECK(res.fixedBindSetOrSpace == 0); - CHECK(res.fixedBindNumber == 3); + // GLSL does not have register bindings as they're dynamic + if(testType != ShaderType::GLSL) + { + CHECK(res.fixedBindSetOrSpace == 0); + CHECK(res.fixedBindNumber == 3); + } CHECK(res.bindArraySize == 1); CHECK(res.textureType == TextureType::Texture2D); CHECK(res.variableType.members.empty()); @@ -1032,8 +1055,12 @@ void main() { const ShaderResource &res = refl.readOnlyResources[1]; INFO("read-only resource: " << res.name.c_str()); - CHECK(res.fixedBindSetOrSpace == 0); - CHECK(res.fixedBindNumber == 5); + // GLSL does not have register bindings as they're dynamic + if(testType != ShaderType::GLSL) + { + CHECK(res.fixedBindSetOrSpace == 0); + CHECK(res.fixedBindNumber == 5); + } CHECK(res.bindArraySize == 1); CHECK(res.textureType == TextureType::Texture3D); CHECK(res.variableType.members.empty()); @@ -1045,8 +1072,12 @@ void main() { const ShaderResource &res = refl.readOnlyResources[2]; INFO("read-only resource: " << res.name.c_str()); - CHECK(res.fixedBindSetOrSpace == 0); - CHECK(res.fixedBindNumber == 7); + // GLSL does not have register bindings as they're dynamic + if(testType != ShaderType::GLSL) + { + CHECK(res.fixedBindSetOrSpace == 0); + CHECK(res.fixedBindNumber == 7); + } CHECK(res.bindArraySize == 1); CHECK(res.textureType == TextureType::Buffer); CHECK(res.variableType.members.empty()); @@ -1637,8 +1668,12 @@ void main() { const ShaderResource &res = refl.readWriteResources[0]; INFO("read-write resource: " << res.name.c_str()); - CHECK(res.fixedBindSetOrSpace == 0); - CHECK(res.fixedBindNumber == 2); + // GLSL does not have register bindings as they're dynamic + if(testType != ShaderType::GLSL) + { + CHECK(res.fixedBindSetOrSpace == 0); + CHECK(res.fixedBindNumber == 2); + } CHECK(res.bindArraySize == 1); CHECK(res.textureType == TextureType::Buffer); @@ -1732,8 +1767,12 @@ void main() { const ShaderResource &res = refl.readWriteResources[1]; INFO("read-write resource: " << res.name.c_str()); - CHECK(res.fixedBindSetOrSpace == 0); - CHECK(res.fixedBindNumber == 5); + // GLSL does not have register bindings as they're dynamic + if(testType != ShaderType::GLSL) + { + CHECK(res.fixedBindSetOrSpace == 0); + CHECK(res.fixedBindNumber == 5); + } CHECK(res.bindArraySize == 1); CHECK(res.textureType == TextureType::Buffer); @@ -2557,8 +2596,12 @@ void main() { const ShaderResource &res = refl.readOnlyResources[i]; INFO("read-only resource: " << res.name.c_str()); - CHECK(res.fixedBindSetOrSpace == 0); - CHECK(res.fixedBindNumber == 3 + i); + // GLSL does not have register bindings as they're dynamic + if(testType != ShaderType::GLSL) + { + CHECK(res.fixedBindSetOrSpace == 0); + CHECK(res.fixedBindNumber == 3 + i); + } CHECK(res.bindArraySize == arraySizeRO); CHECK(res.textureType == TextureType::Texture2D); CHECK(res.variableType.members.empty()); @@ -2583,8 +2626,12 @@ void main() { const ShaderResource &res = refl.readWriteResources[i]; INFO("read-write resource: " << res.name.c_str()); - CHECK(res.fixedBindSetOrSpace == 0); - CHECK(res.fixedBindNumber == 2 + i); + // GLSL does not have register bindings as they're dynamic + if(testType != ShaderType::GLSL) + { + CHECK(res.fixedBindSetOrSpace == 0); + CHECK(res.fixedBindNumber == 2 + i); + } CHECK(res.bindArraySize == arraySizeRW); CHECK(res.textureType == TextureType::Buffer); diff --git a/renderdoc/driver/shaders/spirv/spirv_reflect.cpp b/renderdoc/driver/shaders/spirv/spirv_reflect.cpp index bb04a2176..cdb64648e 100644 --- a/renderdoc/driver/shaders/spirv/spirv_reflect.cpp +++ b/renderdoc/driver/shaders/spirv/spirv_reflect.cpp @@ -1362,16 +1362,19 @@ void Reflector::MakeReflection(const GraphicsAPI sourceAPI, const ShaderStage st // On GL if we have a location and no binding, put that in as the bind. It is not used // otherwise on GL as the bindings are dynamic. This should only happen for // bare uniforms and not for texture/buffer type uniforms which should have a binding - if(sourceAPI == GraphicsAPI::OpenGL && - (decorations[global.id].flags & (Decorations::HasLocation | Decorations::HasBinding)) == - Decorations::HasLocation) + if(sourceAPI == GraphicsAPI::OpenGL) { - bind = decorations[global.id].location; - } - else if(sourceAPI == GraphicsAPI::OpenGL && - (decorations[global.id].flags & Decorations::HasLocation) == Decorations::NoFlags) - { - bind = ~0U; + Decorations::Flags flags = Decorations::Flags( + decorations[global.id].flags & (Decorations::HasLocation | Decorations::HasBinding)); + + if(flags == Decorations::HasLocation) + { + bind = decorations[global.id].location; + } + else if(flags == Decorations::NoFlags) + { + bind = ~0U; + } } if(usedIds.find(global.id) == usedIds.end())