[Fix] (Program): read an API colour index of zero as no override, matching the IO resolver and ProgramInterface

This commit is contained in:
2026-08-27 10:52:00 -04:00
parent b6d6316333
commit 3c70b4fc0f
2 changed files with 86 additions and 5 deletions
@@ -1875,15 +1875,45 @@ namespace MobileGL::MG_State::GLState {
const Int location = explicitLocation != in.explicitFragDataLocation.end()
? static_cast<Int>(explicitLocation->second)
: static_cast<Int>(output.layoutLocation());
// glBindFragDataLocationIndexed wins over the shader's own qualifier, the same
// precedence the location above follows and the same one ProgramInterface applies.
// The colour INDEX, under the one precedence rule the whole codebase uses: a NON-ZERO
// glBindFragDataLocationIndexed index wins, and a zero (or absent) one falls back to
// the shader's own layout(index = N).
//
// Zero has to mean "no override" rather than "index 0", because glBindFragDataLocation
// IS glBindFragDataLocationIndexed with index 0 (GL_Program.cpp) and writes a real 0
// into this map. Reading that 0 as an override made a blanket
// `glBindFragDataLocation(prog, 0, "b")` over a shader that declares
// `layout(location = 0, index = 1) out vec4 b;` collapse b onto slot (0,0) next to the
// index-0 output and fail the link as an alias - while the IO resolver had left b's
// qualifier at 1, the SPIR-V still carried Index 1, and glGetProgramResourceLocationIndex
// still answered 1. Validation was rejecting a program the backend had already emitted
// correctly, which is the one case where this branch can change the answer at all: this
// runs AFTER ShaderCompiler::LinkProgram/mapIO, so for every other shape the qualifier
// already carries the resolver's verdict.
//
// The two other consumers spell the same rule: TMglGlslIoResolver only writes the API
// index into the qualifier when it is non-zero, and ProgramInterface falls back to
// type.layoutIndex when GetFragmentDataIndex answers 0. All three now agree.
//
// Against the spec (GL 4.6 core 15.2.3): where a fragment output's index is given by a
// shader layout qualifier, that value is used and anything bound through
// BindFragDataLocation(Indexed) is IGNORED - the same precedence layout(location) has
// over glBindAttribLocation. That is stricter than "non-zero API wins", and the two
// differ in exactly one shape: an explicit `index = 0` in the shader against an API
// index of 1, where the spec keeps 0 and this codebase takes 1. That divergence lives
// in the resolver (it decides what is emitted); it is pre-existing, out of scope here,
// and deliberately not re-litigated in a third place - matching the resolver is what
// keeps validation checking what was actually built.
Int colorIndex = 0;
if (const auto explicitIndex = in.explicitFragDataIndex.find(outputName);
explicitIndex != in.explicitFragDataIndex.end()) {
colorIndex = static_cast<Int>(explicitIndex->second);
} else if (const glslang::TType* outputType = output.getType();
outputType != nullptr && outputType->getQualifier().hasIndex()) {
colorIndex = static_cast<Int>(outputType->getQualifier().layoutIndex);
}
if (colorIndex == 0) {
if (const glslang::TType* outputType = output.getType();
outputType != nullptr && outputType->getQualifier().hasIndex()) {
colorIndex = static_cast<Int>(outputType->getQualifier().layoutIndex);
}
}
const Int span = std::max<Int>(output.size, 1);
+51
View File
@@ -4097,6 +4097,57 @@ void main() { fragColorA = vec4(1.0); fragColorB = vec4(0.5); }
}
}
// An API colour index of ZERO is "no override", not "index 0". glBindFragDataLocation is
// glBindFragDataLocationIndexed with index 0 (GL_Program.cpp), so the blanket-bind pattern -
// portable code that binds every output name it knows about, without caring about dual-source -
// writes a real 0 into the frag-data index map for an output whose shader qualifier says 1.
// Reading that 0 as an override collapsed both outputs onto slot (0,0) and failed the link as an
// alias, while the IO resolver had left the qualifier at 1 and the emitted SPIR-V still carried
// Index 1 - validation rejecting a program the backend had already built correctly.
//
// The rule pinned here is the codebase's (non-zero API index wins, zero falls back to the shader
// qualifier), which is also what GL 4.6 core 15.2.3 gives for THIS shape: a shader layout
// qualifier is used and the bound value ignored.
TEST_F(ProgramTest, AnApiColorIndexOfZeroDoesNotOverrideTheShaderIndexQualifier) {
constexpr const char* dualSourceFs = R"(#version 460 core
layout(location = 0, index = 0) out vec4 fragColor0;
layout(location = 0, index = 1) out vec4 fragColor1;
void main() { fragColor0 = vec4(1.0); fragColor1 = vec4(0.5); }
)";
const GLuint program = CreateProgram();
const GLuint vs = CreateShader(GL_VERTEX_SHADER);
ShaderSource(vs, 1, &kPassthroughVs, nullptr);
CompileShader(vs);
AttachShader(program, vs);
DeleteShader(vs);
const GLuint fs = CreateShader(GL_FRAGMENT_SHADER);
ShaderSource(fs, 1, &dualSourceFs, nullptr);
CompileShader(fs);
AttachShader(program, fs);
DeleteShader(fs);
// The blanket bind: colour number 0, index 0, on the output the shader put at index 1.
BindFragDataLocation(program, 0, "fragColor0");
BindFragDataLocation(program, 0, "fragColor1");
LinkProgram(program);
GLint linkStatus = GL_FALSE;
GetProgramiv(program, GL_LINK_STATUS, &linkStatus);
EXPECT_EQ(linkStatus, GL_TRUE) << [&] {
char log[512] = "";
GetProgramInfoLog(program, sizeof(log), nullptr, log);
return std::string(log);
}();
// The explicit indexed form with a NON-zero index is still an override, and still links.
BindFragDataLocationIndexed(program, 0, 1, "fragColor1");
LinkProgram(program);
GetProgramiv(program, GL_LINK_STATUS, &linkStatus);
EXPECT_EQ(linkStatus, GL_TRUE);
EXPECT_EQ(GetFragDataIndex(program, "fragColor1"), 1);
for (int i = 0; i < 32 && GetError() != GL_NO_ERROR; ++i) {
}
}
TEST_F(ProgramTest, GetProgramivReportsTheGeometryStageLinkProperties) {
constexpr const char* gs = R"(#version 460 core
layout(triangles, invocations = 3) in;