diff --git a/MobileGL/MG_State/GLState/ProgramState/ProgramLinkTask.cpp b/MobileGL/MG_State/GLState/ProgramState/ProgramLinkTask.cpp index 69dbfc2a..3860b355 100644 --- a/MobileGL/MG_State/GLState/ProgramState/ProgramLinkTask.cpp +++ b/MobileGL/MG_State/GLState/ProgramState/ProgramLinkTask.cpp @@ -1023,6 +1023,12 @@ namespace MobileGL::MG_State::GLState { return uniform.index >= 0 && uniform.index < static_cast(artifacts.tProgramBlockIndexToGl.size()) && artifacts.tProgramBlockIndexToGl[uniform.index] < 0; }; + // Member of a block GL can see - a named uniform block, a buffer block, or the + // synthesized atomic-counter block. GL locations are a property of the DEFAULT uniform + // block alone (GL 4.6 core 7.6.1), so these take none. + const auto isNamedBlockMember = [&isGlobalUboMember](const glslang::TObjectReflection& uniform) { + return uniform.index >= 0 && !isGlobalUboMember(uniform); + }; for (Int i = 0; i < tProgramUniformCount; i++) { const auto& uniform = artifacts.program->getUniform(i); if (isGlobalUboMember(uniform) && uniform.stages == 0) { @@ -1069,8 +1075,7 @@ namespace MobileGL::MG_State::GLState { for (const Int i : artifacts.glUniformIndexToTProgram) { const auto& uniform = artifacts.program->getUniform(i); const glslang::TType* type = uniform.getType(); - const Bool inNamedBlock = uniform.index >= 0 && !isGlobalUboMember(uniform); - if (inNamedBlock) continue; // block members never take glUniform locations + if (isNamedBlockMember(uniform)) continue; // block members never take glUniform locations if (const Int* explicitLocation = findExplicitLocation(uniform.name)) { effectiveLocation[i] = static_cast(*explicitLocation); @@ -1145,20 +1150,23 @@ namespace MobileGL::MG_State::GLState { in.externalIndex, uniform.name.c_str(), location, location + locationSpan - 1); } + // Counts ONLY default-block uniforms, which is the whole of what a GL uniform location + // is and the whole of what GL_MAX_UNIFORM_LOCATIONS bounds (GL 4.6 core 7.6.1). A + // named-block member used to be counted here too and used to be handed a location by the + // first-fit pass below, which is a spec violation twice over: glGetUniformLocation must + // answer -1 for it (glGetProgramResourceLocation already did), and every slot it took + // pushed a real default-block uniform one location further up. On a program with a + // buffer block that is exactly how a location EQUAL to the advertised maximum got minted + // - the table's ceiling is raised to hold this count, so one extra block member raised it + // to MAX and the first-fit pass then filled the last slot + // (KHR-GL43.explicit_uniform_location.uniform-loc-mix-with-implicit-max, whose compute + // program carries an SSBO; its -max-array sibling ran the pool out and failed to link). Int requiredUniformLocations = deadReservedLocationCount; - // The same count restricted to DEFAULT-BLOCK uniforms, which is the only thing - // GL_MAX_UNIFORM_LOCATIONS bounds. requiredUniformLocations cannot serve: it also carries - // named-block members, which take a slot in this allocator's table (an implementation - // detail) but consume no GL uniform location at all, so a big UBO array would otherwise - // fail a link the spec allows. - Int defaultBlockLocationDemand = deadReservedLocationCount; for (const Int i : artifacts.glUniformIndexToTProgram) { auto& uniform = artifacts.program->getUniform(i); const Uint location = effectiveLocation[i]; const Int locationSpan = GetUniformLocationSpan(uniform); - requiredUniformLocations += locationSpan; - const Bool inNamedBlock = uniform.index >= 0 && !isGlobalUboMember(uniform); - if (!inNamedBlock) defaultBlockLocationDemand += locationSpan; + if (!isNamedBlockMember(uniform)) requiredUniformLocations += locationSpan; if (location != kNoLocation) { artifacts.maxUniformLocation = std::max(artifacts.maxUniformLocation, location + locationSpan - 1); } @@ -1177,11 +1185,11 @@ namespace MobileGL::MG_State::GLState { // (KHR-GL43.explicit_uniform_location.uniform-loc-negative-link-max-num-of-locations). // A single uniform whose own span passes the ceiling was already rejected above; this is // the aggregate half of the same rule. - if (defaultBlockLocationDemand > static_cast(kMaxUniformLocations)) { + if (requiredUniformLocations > static_cast(kMaxUniformLocations)) { artifacts.infoLog = std::format("Uniform locations exhausted: the default-block uniforms need {} locations but " "GL_MAX_UNIFORM_LOCATIONS is {}.", - defaultBlockLocationDemand, kMaxUniformLocations); + requiredUniformLocations, kMaxUniformLocations); DeferLog(std::format("ProgramObject {}: Link failed - {}", in.externalIndex, artifacts.infoLog)); ProgramObject::ResetLinkArtifacts(artifacts); return false; @@ -1254,6 +1262,10 @@ namespace MobileGL::MG_State::GLState { // is demoted to the first-fit pass below instead of failing the link. for (const Int i : artifacts.glUniformIndexToTProgram) { auto& uniform = artifacts.program->getUniform(i); + // Same rule the effective-location loop applies: a block member has no GL location, + // so it must not reach the first-fit pass either. Its uniformLocations entry stays + // at kNoLocation, which glGetUniformLocation reads back as the -1 the spec wants. + if (isNamedBlockMember(uniform)) continue; if (locationIsSourceExplicit[i]) continue; const Uint location = effectiveLocation[i]; if (location == kNoLocation) { diff --git a/MobileGL/MG_Test/Program/ProgramTest.cpp b/MobileGL/MG_Test/Program/ProgramTest.cpp index 924b8f83..da9c2e1b 100644 --- a/MobileGL/MG_Test/Program/ProgramTest.cpp +++ b/MobileGL/MG_Test/Program/ProgramTest.cpp @@ -3479,3 +3479,104 @@ void main() { } EXPECT_EQ(GetError(), GL_NO_ERROR); } + +// GL 4.6 core 7.6.1: a uniform LOCATION is a property of the default uniform block. A member of +// a named uniform block or a buffer block has none, and glGetUniformLocation must answer -1 for +// it - which is what glGetProgramResourceLocation(GL_UNIFORM, ...) already did, so the two used +// to disagree. The location such a member was handed was not merely reported, it was CONSUMED: +// it came out of the same first-fit table the default-block uniforms draw from. +TEST_F(ProgramTest, BlockMembersConsumeNoUniformLocation) { + const char* csSource = R"(#version 430 core +layout(local_size_x = 1) in; +layout(std430, binding = 1) buffer ResultBuffer { vec4 bufferMember; }; +layout(std140, binding = 2) uniform SettingsBlock { vec4 blockMember; }; +layout(location = 0) uniform float uDead[3]; +uniform float uImplicit; +void main() { bufferMember = blockMember * uImplicit; } +)"; + const GLuint cs = CompileShaderChecked(GL_COMPUTE_SHADER, csSource); + const GLuint program = CreateProgram(); + AttachShader(program, cs); + LinkProgram(program); + GLint linkStatus = GL_FALSE; + GetProgramiv(program, GL_LINK_STATUS, &linkStatus); + char infoLog[1024] = ""; + GetProgramInfoLog(program, sizeof(infoLog), nullptr, infoLog); + ASSERT_EQ(linkStatus, GL_TRUE) << infoLog; + + for (const char* member : {"bufferMember", "blockMember"}) { + EXPECT_EQ(GetUniformLocation(program, member), -1) << member << " is a block member, not a GL uniform"; + EXPECT_EQ(GetProgramResourceLocation(program, GL_UNIFORM, member), -1) + << member << ": the two location queries must agree"; + } + + // uDead[3] reserves 0..2 without becoming visible, so the first location left for the one + // default-block uniform is 3. It used to be 4, because a block member took 3 first. + EXPECT_EQ(GetUniformLocation(program, "uImplicit"), 3) + << "a block member consumed a location the default-block uniform was entitled to"; + EXPECT_EQ(GetUniformLocation(program, "uDead"), -1); + EXPECT_EQ(GetError(), GL_NO_ERROR); +} + +// The same defect at the boundary, which is where the conformance suite catches it. The location +// table's ceiling is raised to hold every uniform it must place; counting block members into that +// raise pushed the ceiling to GL_MAX_UNIFORM_LOCATIONS itself, and the first-fit pass then handed +// out the one location past the legal 0..MAX-1 range +// (KHR-GL43.explicit_uniform_location.uniform-loc-mix-with-implicit-max, whose compute program +// carries an SSBO: "Uniform u2 returned location (4095) is greater than implementation dependent +// limit (4095)"). Its -array sibling shares the root cause and failed one step further along, with +// the pool reported exhausted and no link at all. +TEST_F(ProgramTest, ImplicitLocationStaysInRangeWhenABufferBlockSharesTheProgram) { + GLint maxLocations = 0; + GetIntegerv(GL_MAX_UNIFORM_LOCATIONS, &maxLocations); + ASSERT_GE(maxLocations, 1024) << "GL 4.3 requires at least 1024 uniform locations"; + + // The CTS shape: explicit unused arrays fill the pool except for a hole of `implicitCount` + // locations at `holeBase`, and the one implicit uniform must land exactly in that hole. + const auto runCase = [&](int holeBase, int implicitCount) { + String decls; + int nextName = 0; + if (holeBase > 0) { + decls += "layout(location = 0) uniform float u" + std::to_string(nextName++) + "[" + + std::to_string(holeBase) + "];\n"; + } + const int tailBase = holeBase + implicitCount; + if (tailBase < maxLocations) { + decls += "layout(location = " + std::to_string(tailBase) + ") uniform float u" + + std::to_string(nextName++) + "[" + std::to_string(maxLocations - tailBase) + "];\n"; + } + const String implicitName = "u" + std::to_string(nextName); + decls += "uniform float " + implicitName + "[" + std::to_string(implicitCount) + "];\n"; + + // The buffer block is the whole point: it is one more uniform the table has to seat, and + // seating it inside the location space is what used to push the implicit uniform out. + const String csSource = "#version 430 core\n" + "layout(local_size_x = 1) in;\n" + "layout(std430, binding = 1) buffer ResultBuffer { vec4 cs_result; };\n" + + decls + "void main() { cs_result = vec4(" + implicitName + "[0]); }\n"; + const GLuint cs = CompileShaderChecked(GL_COMPUTE_SHADER, csSource.c_str()); + const GLuint program = CreateProgram(); + AttachShader(program, cs); + LinkProgram(program); + GLint linkStatus = GL_FALSE; + GetProgramiv(program, GL_LINK_STATUS, &linkStatus); + char infoLog[1024] = ""; + GetProgramInfoLog(program, sizeof(infoLog), nullptr, infoLog); + ASSERT_EQ(linkStatus, GL_TRUE) << "hole at " << holeBase << " x" << implicitCount << ": " << infoLog; + + const GLint location = GetUniformLocation(program, implicitName.c_str()); + EXPECT_EQ(location, holeBase) << "the implicit uniform must take the one free span left"; + EXPECT_LT(location + implicitCount, maxLocations + 1) + << "locations " << location << ".." << (location + implicitCount - 1) + << " must stay inside 0.." << (maxLocations - 1); + EXPECT_EQ(GetUniformLocation(program, "cs_result"), -1); + }; + + // The three holes the CTS walks, for its single-uniform and its 3-element-array subcase. + for (const int implicitCount : {1, 3}) { + runCase(0, implicitCount); + runCase(3, implicitCount); + runCase(maxLocations - implicitCount, implicitCount); + } + EXPECT_EQ(GetError(), GL_NO_ERROR); +}