From 450eb209b61422ba0b329aeb74198927bcd81975 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Wed, 12 Aug 2026 02:38:04 -0400 Subject: [PATCH] [Fix, Test] (MG_State, MG_Impl): the program interface of a separable program is its own first and last stage, not vertex and fragment --- .../GLImpl/Program/ProgramInterface.cpp | 87 ++++++++- .../GLState/ProgramState/ProgramLinkTask.cpp | 28 ++- .../MG_Test/Program/ProgramInterfaceTest.cpp | 174 ++++++++++++++++-- 3 files changed, 271 insertions(+), 18 deletions(-) diff --git a/MobileGL/MG_Impl/GLImpl/Program/ProgramInterface.cpp b/MobileGL/MG_Impl/GLImpl/Program/ProgramInterface.cpp index 274e3962..1137ad5f 100644 --- a/MobileGL/MG_Impl/GLImpl/Program/ProgramInterface.cpp +++ b/MobileGL/MG_Impl/GLImpl/Program/ProgramInterface.cpp @@ -210,11 +210,66 @@ namespace MobileGL::MG_Impl::GLImpl::ProgramInterface { // ---- model construction -------------------------------------------------------- + // GL_REFERENCED_BY_*_SHADER for an ARRAYED block instance, refined per element. + // + // glslang records a block reference by walking up to the base symbol and calling + // addBlockName with the whole ARRAY type, which ORs the referencing stage into every + // element at once - it has not resolved the subscript yet at that point. So reading + // "e[0].b" marks both TrickyBlock[0] and TrickyBlock[1] as referenced by the fragment + // stage (KHR-GL43.program_interface_query.uniform-block-types). + // + // The MEMBER masks are exact: EShReflectionAllBlockVariables enumerates every member of + // every element with the stage mask suppressed, and only the dereference chain actually + // walked turns a bit on - and that chain carries the subscript. So the union of a block + // instance's members is the reference set of that instance. + // + // Applied ONLY to arrayed instances, because for a scalar block glslang is already exact. + // Note the union is used even when it is empty: an array element nobody dereferenced has + // no member bits and is genuinely referenced by nobody, which is the whole point - falling + // back to the block's own mask there would restore the over-approximation. + Vector BuildBlockStagesFromMembers(const glslang::TProgram& reflection, Int blockCount) { + auto& mutableReflection = const_cast(reflection); + Vector stagesByBlock(static_cast(blockCount < 0 ? 0 : blockCount), 0u); + const Int uniformCount = mutableReflection.getNumUniformVariables(); + for (Int index = 0; index < uniformCount; ++index) { + const auto& uniform = mutableReflection.getUniform(index); + const Int owner = uniform.index; + if (owner < 0 || owner >= blockCount) continue; + stagesByBlock[static_cast(owner)] |= static_cast(uniform.stages); + } + return stagesByBlock; + } + + // UNIFORM blocks only, and that scope is load-bearing rather than cautious. The member + // names glslang produces for a uniform block array carry the subscript + // ("TrickyBlock[0].b", via EShReflectionStrictArraySuffix), so each element's members are + // distinct entries and the bits land on the right one. A SHADER STORAGE block array does + // NOT get that treatment - its buffer variables reflect under one subscript-free spelling + // shared by every element - so a union over them credits element 0 and starves the rest. + // KHR-GL43.program_interface_query.ssb-types is the case that says so: it reads ss[0] and + // ss[1] and requires both to report the fragment stage, which only glslang's own + // (deliberately over-approximating) block mask gets right. Storage and atomic-counter + // blocks therefore keep that mask untouched. + Uint32 UniformBlockStages(const glslang::TObjectReflection& block, const Vector& stagesFromMembers, + Int tIndex) { + String arrayBase; + Uint element = 0; + Bool malformed = false; + if (!SplitTrailingSubscript(block.name, arrayBase, element, malformed) || malformed) { + return static_cast(block.stages); + } + if (tIndex < 0 || tIndex >= static_cast(stagesFromMembers.size())) { + return static_cast(block.stages); + } + return stagesFromMembers[static_cast(tIndex)]; + } + void BuildBlocks(ProgramObject& program, const glslang::TProgram& reflection, Model& model, Vector& blockKind, Vector& blockInterfaceIndex) { const Int blockCount = const_cast(reflection).getNumUniformBlocks(); blockKind.assign(blockCount, BlockKind::Uniform); blockInterfaceIndex.assign(blockCount, -1); + const Vector stagesFromMembers = BuildBlockStagesFromMembers(reflection, blockCount); for (Int tIndex = 0; tIndex < blockCount; ++tIndex) { const auto& block = const_cast(reflection).getUniformBlock(tIndex); @@ -260,8 +315,8 @@ namespace MobileGL::MG_Impl::GLImpl::ProgramInterface { resource.bufferDataSize = static_cast(program.GetUBOSizeAt(glIndex)); const Int tIndex = program.TProgramBlockIndex(static_cast(glIndex)); if (tIndex >= 0 && tIndex < blockCount) { - resource.stages = - static_cast(const_cast(reflection).getUniformBlock(tIndex).stages); + resource.stages = UniformBlockStages(const_cast(reflection).getUniformBlock(tIndex), + stagesFromMembers, tIndex); } model.uniformBlocks.push_back(Move(resource)); } @@ -352,6 +407,17 @@ namespace MobileGL::MG_Impl::GLImpl::ProgramInterface { } } + // A built-in interface block that a shader redeclares with fewer members keeps the + // omitted ones in its type when the redeclaration is ANONYMOUS - glslang hides them + // (basic type void) instead of erasing them, because the original shared declaration + // has to stay usable. Only the instance-named form erases. So a separable vertex + // program that redeclares `out gl_PerVertex { vec4 gl_Position; }` still carries + // gl_PointSize and gl_ClipDistance through the block-unwrapping reflection, and they + // are not part of its output interface. + Bool IsHiddenBlockMember(const glslang::TType* type) { + return type != nullptr && type->getBasicType() == glslang::EbtVoid; + } + void BuildStageIO(ProgramObject& program, const glslang::TProgram& reflection, Model& model) { auto& mutableReflection = const_cast(reflection); @@ -359,6 +425,7 @@ namespace MobileGL::MG_Impl::GLImpl::ProgramInterface { for (Int index = 0; index < inputCount; ++index) { const auto& refl = mutableReflection.getPipeInput(index); const glslang::TType* type = refl.getType(); + if (IsHiddenBlockMember(type)) continue; Resource resource; // The Vulkan-semantics parse reflects the vertex builtins under their SPIR-V // names; GL enumerates the GL spellings. @@ -373,18 +440,28 @@ namespace MobileGL::MG_Impl::GLImpl::ProgramInterface { model.programInputs.push_back(Move(resource)); } + // A color number, and therefore a color INDEX, exists only for a fragment stage's + // outputs. The output interface belongs to the program's last stage, so for a + // separable tessellation/geometry/vertex program these are varyings: asking the + // frag-data maps about them can still answer a location (a tess-control output + // carries its own layout(location=N)), and a location then manufactures a color + // index of 0 where GL requires -1 + // (KHR-GL43.program_interface_query.separate-programs-tess-control). + const Bool lastStageIsFragment = mutableReflection.getIntermediate(EShLangFragment) != nullptr; const Int outputCount = mutableReflection.getNumPipeOutputs(); for (Int index = 0; index < outputCount; ++index) { const auto& refl = mutableReflection.getPipeOutput(index); const glslang::TType* type = refl.getType(); + if (IsHiddenBlockMember(type)) continue; Resource resource; resource.name = WithArraySuffix(refl.name, type); resource.type = static_cast(refl.glDefineType); resource.arraySize = ArraySizeOf(type, refl.size); resource.location = MappedLocation(program.GetFragmentDataLocation(refl.name.c_str())); - if (resource.location < 0) { - // A built-in output (gl_FragDepth, gl_SampleMask) and a non-fragment stage - // output both have no location, and therefore no color index either. + if (resource.location < 0 || !lastStageIsFragment) { + // A built-in output (gl_FragDepth, gl_SampleMask) has no location, and a + // non-fragment stage's outputs have no color number at all - either way there + // is no color index. resource.locationIndex = -1; } else { resource.locationIndex = program.GetFragmentDataIndex(refl.name.c_str()); diff --git a/MobileGL/MG_State/GLState/ProgramState/ProgramLinkTask.cpp b/MobileGL/MG_State/GLState/ProgramState/ProgramLinkTask.cpp index f90f79b6..63ad6ccb 100644 --- a/MobileGL/MG_State/GLState/ProgramState/ProgramLinkTask.cpp +++ b/MobileGL/MG_State/GLState/ProgramState/ProgramLinkTask.cpp @@ -563,8 +563,20 @@ namespace MobileGL::MG_State::GLState { // - SharedStd140UBO: a DECLARED uniform block is active even when no member is // ever read (reflected from the linker objects). PreprocessShaderSource coerces // every block to std140, so this covers all of them. + // - IntermediateIO: GL_PROGRAM_INPUT is the input interface of the program's FIRST + // stage and GL_PROGRAM_OUTPUT the output interface of its LAST one. Without this + // glslang hardcodes those boundaries to vertex/fragment, so a separable program + // made of one non-vertex stage has an empty input interface and one made of a + // non-fragment stage an empty output interface + // (KHR-GL43.program_interface_query.separate-programs-*). + // - UnwrapIOBlocks: an inter-stage interface block enumerates as its MEMBERS - + // "Color.r", and "gl_Position" for an anonymous gl_PerVertex - not as the block + // instance. Only reachable through IntermediateIO: a vertex stage's inputs and a + // fragment stage's outputs can never be blocks, so this is inert for a program + // whose boundary stages are the hardcoded ones. if (!artifacts.program->buildReflection(EShReflectionStrictArraySuffix | EShReflectionBasicArraySuffix | - EShReflectionAllBlockVariables | EShReflectionSharedStd140UBO)) { + EShReflectionAllBlockVariables | EShReflectionSharedStd140UBO | + EShReflectionIntermediateIO | EShReflectionUnwrapIOBlocks)) { artifacts.linkStatus = false; artifacts.infoLog = "Build reflection failed."; DeferLog(std::format("ProgramObject {}: DoReflection - buildReflection() returned false", @@ -852,7 +864,14 @@ namespace MobileGL::MG_State::GLState { } // ------------ attributes (vertex in) --------------- - Int inCount = artifacts.program->getNumPipeInputs(); + // The pipe-input list is the input interface of the program's FIRST stage, which is only + // the vertex attribute set when the program actually HAS a vertex stage. A separable + // fragment/geometry/tessellation program reflects its own stage inputs here, and those are + // varyings - registering them as vertex attributes would hand glGetActiveAttrib and the + // attribute location table interstage varyings. + Int inCount = artifacts.program->getIntermediate(EShLangVertex) != nullptr + ? artifacts.program->getNumPipeInputs() + : 0; MGLOG_D("ProgramObject %u: Reflection - pipe input count (attributes) = %d", in.externalIndex, inCount); Int maxLoc = -1; @@ -951,6 +970,11 @@ namespace MobileGL::MG_State::GLState { Bool ProgramLinkTask::ValidateFragmentOutputLocations() { if (!artifacts.program) return false; + // The pipe-output list is the output interface of the program's LAST stage. Only a + // fragment stage's outputs are color numbers indexed against GL_MAX_DRAW_BUFFERS; a + // separable vertex/geometry/tessellation program's outputs are varyings, and holding + // them to the draw-buffer range fails the link of every such program. + if (artifacts.program->getIntermediate(EShLangFragment) == nullptr) return true; UnorderedMap colorNumberOwners; const Int outputCount = artifacts.program->getNumPipeOutputs(); diff --git a/MobileGL/MG_Test/Program/ProgramInterfaceTest.cpp b/MobileGL/MG_Test/Program/ProgramInterfaceTest.cpp index 73fbee8b..01e0a1f5 100644 --- a/MobileGL/MG_Test/Program/ProgramInterfaceTest.cpp +++ b/MobileGL/MG_Test/Program/ProgramInterfaceTest.cpp @@ -121,12 +121,6 @@ void main() { color = vec4(0, 1, 0, 1); } ExpectLinked(p); ClearErrors(); - // KNOWN GAP, not an expectation: a separable FRAGMENT program's own inputs are not - // in the reflection at all - glslang builds the "pipe input" list from the vertex - // stage unless EShReflectionIntermediateIO is set, and setting that makes a - // vertex-only separable program report its VS outputs as fragment outputs, which - // fails ValidateFragmentOutputLocations and breaks glCreateShaderProgramv. So - // GL_PROGRAM_INPUT is empty here until that validation is stage-aware. EXPECT_EQ(Interfaceiv(p, GL_PROGRAM_OUTPUT, GL_ACTIVE_RESOURCES), 1); EXPECT_EQ(Interfaceiv(p, GL_PROGRAM_OUTPUT, GL_MAX_NAME_LENGTH), 6); @@ -804,6 +798,163 @@ void main(void) { EXPECT_EQ(TakeError(), GL_NO_ERROR); } + // ------------------------------------------------------------- uniform-block-types -- + // GL_REFERENCED_BY_*_SHADER is per block INSTANCE. An array of uniform blocks enumerates + // one resource per element, and only the elements a stage actually dereferences are + // referenced by it - declaring the array is not referencing every element. + TEST_F(ProgramInterfaceTest, ArrayedUniformBlockReferencedByIsPerElement) { + const char* vs = R"(#version 430 +in vec4 position; +uniform SimpleBlock { mat3x2 a; mat4 b; vec4 c; }; +void main(void) { + float tmp = a[0][1] * b[1][2] * c.x; + gl_Position = position * tmp; +} +)"; + const char* fs = R"(#version 430 +uniform TrickyBlock { mat4 b; uint c; } e[2]; +out vec4 color; +void main() { color = vec4(0, 1, 0, 1) * e[0].b[0][0]; } +)"; + const GLuint p = MakeProgram(vs, fs); + BindAttribLocation(p, 0, "position"); + BindFragDataLocation(p, 0, "color"); + LinkProgram(p); + ExpectLinked(p); + ClearErrors(); + + EXPECT_EQ(Interfaceiv(p, GL_UNIFORM_BLOCK, GL_ACTIVE_RESOURCES), 3); + ExpectResource(p, GL_UNIFORM_BLOCK, "TrickyBlock[0]", "TrickyBlock[0]"); + ExpectResource(p, GL_UNIFORM_BLOCK, "TrickyBlock[1]", "TrickyBlock[1]"); + + const std::vector refProps = {GL_REFERENCED_BY_VERTEX_SHADER, GL_REFERENCED_BY_FRAGMENT_SHADER}; + EXPECT_EQ(PropsOf(p, GL_UNIFORM_BLOCK, "SimpleBlock", refProps), (std::vector{1, 0})); + EXPECT_EQ(PropsOf(p, GL_UNIFORM_BLOCK, "TrickyBlock[0]", refProps), (std::vector{0, 1})); + EXPECT_EQ(PropsOf(p, GL_UNIFORM_BLOCK, "TrickyBlock[1]", refProps), (std::vector{0, 0})) + << "the unreferenced element of a block array must not inherit its sibling's stages"; + EXPECT_EQ(TakeError(), GL_NO_ERROR); + } + + // The boundary of the rule above, and the case that caught it on device + // (KHR-GL43.program_interface_query.ssb-types): a SHADER STORAGE block array's buffer + // variables reflect under ONE subscript-free spelling shared by every element, so a union + // over them credits element 0 and starves every other element - even the ones the shader + // plainly reads. Storage blocks keep glslang's own mask, and both elements here must report + // the fragment stage. + TEST_F(ProgramInterfaceTest, ArrayedStorageBlockKeepsGlslangStagesForEveryElement) { + const char* vs = R"(#version 430 +in vec4 position; +void main(void) { gl_Position = position; } +)"; + const char* fs = R"(#version 430 +layout(binding = 4) buffer SimpleStorage { vec4 a; } ss[2]; +out vec4 color; +void main() { color = ss[0].a + ss[1].a; } +)"; + const GLuint p = MakeProgram(vs, fs); + BindAttribLocation(p, 0, "position"); + BindFragDataLocation(p, 0, "color"); + LinkProgram(p); + ExpectLinked(p); + ClearErrors(); + + const std::vector refProps = {GL_REFERENCED_BY_VERTEX_SHADER, GL_REFERENCED_BY_FRAGMENT_SHADER}; + for (const char* name : {"SimpleStorage[0]", "SimpleStorage[1]"}) { + EXPECT_EQ(PropsOf(p, GL_SHADER_STORAGE_BLOCK, name, refProps), (std::vector{0, 1})) + << "for " << name << ": a storage block the fragment stage reads must say so"; + } + EXPECT_EQ(TakeError(), GL_NO_ERROR); + } + + // --------------------------------------------------------- separate-programs-vertex -- + // A vertex-only separable program's OUTPUT interface is its own stage outputs, and an + // inter-stage block enumerates as its members: "Color.r", and "gl_Position" for the + // anonymous gl_PerVertex redeclaration - never the block instance name "vs_color". + TEST_F(ProgramInterfaceTest, SeparableVertexProgramEnumeratesItsOwnOutputBlockMembers) { + const char* vs = R"(#version 430 core +layout(location = 0) in vec4 in_vertex; +out Color { float r, g, b; vec4 iLikePie; } vs_color; +out gl_PerVertex { vec4 gl_Position; }; +uniform float u; +uniform vec4 v; +void main() { + gl_Position = in_vertex; + vs_color.r = u; vs_color.g = 0.0; vs_color.b = 0.0; vs_color.iLikePie = v; +} +)"; + const GLuint p = CreateShaderProgramv(GL_VERTEX_SHADER, 1, &vs); + ExpectLinked(p); + ClearErrors(); + + // Exactly 5: the four Color members plus gl_Position. The anonymous gl_PerVertex + // redeclaration drops gl_PointSize and gl_ClipDistance, which glslang keeps in the + // block type as hidden members rather than erasing. + EXPECT_EQ(Interfaceiv(p, GL_PROGRAM_INPUT, GL_ACTIVE_RESOURCES), 1); + EXPECT_EQ(Interfaceiv(p, GL_PROGRAM_INPUT, GL_MAX_NAME_LENGTH), 10); + EXPECT_EQ(Interfaceiv(p, GL_PROGRAM_OUTPUT, GL_ACTIVE_RESOURCES), 5); + EXPECT_EQ(Interfaceiv(p, GL_PROGRAM_OUTPUT, GL_MAX_NAME_LENGTH), 15); + + ExpectResource(p, GL_PROGRAM_INPUT, "in_vertex", "in_vertex"); + for (const char* name : {"Color.r", "Color.g", "Color.b", "Color.iLikePie", "gl_Position"}) { + ExpectResource(p, GL_PROGRAM_OUTPUT, name, name); + } + EXPECT_EQ(GetProgramResourceIndex(p, GL_PROGRAM_OUTPUT, "vs_color"), GL_INVALID_INDEX) + << "the block instance is not the resource; its members are"; + + // A vertex-stage output has no color number, hence no index either, and it is not + // per-patch. NAME_LENGTH/TYPE/ARRAY_SIZE come from the member, not the block. + EXPECT_EQ(PropsOf(p, GL_PROGRAM_OUTPUT, "Color.iLikePie", + {GL_NAME_LENGTH, GL_TYPE, GL_ARRAY_SIZE, GL_REFERENCED_BY_COMPUTE_SHADER, + GL_REFERENCED_BY_FRAGMENT_SHADER, GL_REFERENCED_BY_GEOMETRY_SHADER, + GL_REFERENCED_BY_TESS_CONTROL_SHADER, GL_REFERENCED_BY_TESS_EVALUATION_SHADER, + GL_REFERENCED_BY_VERTEX_SHADER, GL_IS_PER_PATCH, GL_LOCATION_INDEX}), + (std::vector{15, GL_FLOAT_VEC4, 1, 0, 0, 0, 0, 0, 1, 0, -1})); + EXPECT_EQ(TakeError(), GL_NO_ERROR); + } + + // ------------------------------------------------------- separate-programs-geometry -- + // Both boundaries are a middle stage here. The input block carries an instance name + // (gl_in[]) so its members are prefixed with the BLOCK name, and the arrayed-ness of the + // block itself is dropped: "gl_PerVertex.gl_Position", array size 1. + TEST_F(ProgramInterfaceTest, SeparableGeometryProgramEnumeratesBothBlockBoundaries) { + const char* gs = R"(#version 430 +layout(triangles) in; +layout(triangle_strip, max_vertices = 4) out; +out gl_PerVertex { vec4 gl_Position; float gl_PointSize; float gl_ClipDistance[]; }; +in gl_PerVertex { vec4 gl_Position; float gl_PointSize; float gl_ClipDistance[]; } gl_in[]; +void main() { + gl_Position = vec4(-1, 1, 0, 1); EmitVertex(); + gl_Position = gl_in[0].gl_Position; EmitVertex(); + EndPrimitive(); +} +)"; + const GLuint p = CreateShaderProgramv(GL_GEOMETRY_SHADER, 1, &gs); + ExpectLinked(p); + ClearErrors(); + + ExpectResource(p, GL_PROGRAM_INPUT, "gl_PerVertex.gl_Position", "gl_PerVertex.gl_Position"); + ExpectResource(p, GL_PROGRAM_OUTPUT, "gl_Position", "gl_Position"); + // A non-fragment stage's outputs are varyings: no color number, so no color index. + EXPECT_EQ(PropsOf(p, GL_PROGRAM_OUTPUT, "gl_Position", {GL_LOCATION_INDEX}), (std::vector{-1})); + + const std::vector stageProps = { + GL_NAME_LENGTH, + GL_TYPE, + GL_ARRAY_SIZE, + GL_REFERENCED_BY_COMPUTE_SHADER, + GL_REFERENCED_BY_FRAGMENT_SHADER, + GL_REFERENCED_BY_GEOMETRY_SHADER, + GL_REFERENCED_BY_TESS_CONTROL_SHADER, + GL_REFERENCED_BY_TESS_EVALUATION_SHADER, + GL_REFERENCED_BY_VERTEX_SHADER, + GL_IS_PER_PATCH}; + EXPECT_EQ(PropsOf(p, GL_PROGRAM_INPUT, "gl_PerVertex.gl_Position", stageProps), + (std::vector{25, GL_FLOAT_VEC4, 1, 0, 0, 1, 0, 0, 0, 0})); + EXPECT_EQ(PropsOf(p, GL_PROGRAM_OUTPUT, "gl_Position", stageProps), + (std::vector{12, GL_FLOAT_VEC4, 1, 0, 0, 1, 0, 0, 0, 0})); + EXPECT_EQ(TakeError(), GL_NO_ERROR); + } + // ------------------------------------------------------- separate-programs-fragment -- TEST_F(ProgramInterfaceTest, SeparableFragmentProgramSeparatesUniformsFromBufferVariables) { const char* fs = R"(#version 430 @@ -817,11 +968,12 @@ void main() { fs_color = vs_color + x + a; } ExpectLinked(p); ClearErrors(); - // KNOWN GAP, not an expectation - the same one SimpleShaders documents: a separable - // FRAGMENT program's own inputs are absent from the glslang reflection, so - // GL_PROGRAM_INPUT is empty. Spec-correct values here would be 1 and 9 ("vs_color"). - EXPECT_EQ(Interfaceiv(p, GL_PROGRAM_INPUT, GL_ACTIVE_RESOURCES), 0); - EXPECT_EQ(Interfaceiv(p, GL_PROGRAM_INPUT, GL_MAX_NAME_LENGTH), 0); + // GL_PROGRAM_INPUT is the input interface of the program's FIRST stage, which for a + // separable fragment program is the fragment stage: "vs_color", name length 9. + EXPECT_EQ(Interfaceiv(p, GL_PROGRAM_INPUT, GL_ACTIVE_RESOURCES), 1); + EXPECT_EQ(Interfaceiv(p, GL_PROGRAM_INPUT, GL_MAX_NAME_LENGTH), 9); + EXPECT_EQ(GetProgramResourceIndex(p, GL_PROGRAM_INPUT, "vs_color"), 0u); + EXPECT_EQ(ResourceName(p, GL_PROGRAM_INPUT, 0), "vs_color"); EXPECT_EQ(Interfaceiv(p, GL_PROGRAM_OUTPUT, GL_ACTIVE_RESOURCES), 1); EXPECT_EQ(Interfaceiv(p, GL_PROGRAM_OUTPUT, GL_MAX_NAME_LENGTH), 9); // The buffer variable is NOT a uniform, even though the frontend reflection keeps