[Fix] (Program): let two fragment outputs share a colour number when their colour index differs

This commit is contained in:
2026-08-27 10:48:22 -04:00
parent 06605ed0ea
commit 532b5e9cc5
3 changed files with 133 additions and 10 deletions
@@ -104,6 +104,22 @@ void main()
glViewport(0, 0, kExtent, kExtent); glViewport(0, 0, kExtent, kExtent);
glDisable(GL_SCISSOR_TEST); glDisable(GL_SCISSOR_TEST);
glDisable(GL_DEPTH_TEST); glDisable(GL_DEPTH_TEST);
// Capability probe, not an assertion. A GL link that succeeded is not proof that
// the BACKEND can run the program: DirectGLES transpiles to ESSL lazily at first
// use, and GLSL ES has no `index` layout qualifier outside
// GL_EXT_blend_func_extended, so on a driver without it the stage never compiles
// and the draw renders nothing. One unblended white draw tells the two apart, and
// the cases skip rather than measure a picture the driver never produced.
if (m_program != 0) {
glDisable(GL_BLEND);
glBlendFunc(GL_ONE, GL_ZERO);
Draw(/*src0=*/1.0f, /*src1=*/1.0f);
glFinish();
const Image probe = ReadPixels(kExtent, kExtent);
m_programRenders =
!probe.Empty() && static_cast<int>(probe.At(kExtent / 2, kExtent / 2).r) > 245;
}
for (int i = 0; i < 16 && glGetError() != GL_NO_ERROR; ++i) { for (int i = 0; i < 16 && glGetError() != GL_NO_ERROR; ++i) {
} }
} }
@@ -129,10 +145,23 @@ void main()
glUseProgram(0); glUseProgram(0);
} }
// Both cases share this gate: nothing below can be measured on a backend that cannot
// run a dual-source fragment program at all.
void SkipUnlessTheProgramRuns() {
if (m_program == 0) {
GTEST_SKIP() << "this driver cannot build a dual-source fragment shader: " << m_programError;
}
if (!m_programRenders) {
GTEST_SKIP() << "this backend links a dual-source fragment program but renders nothing with "
"it (GLSL ES needs GL_EXT_blend_func_extended for the `index` qualifier)";
}
}
GLuint m_renderbuffer = 0; GLuint m_renderbuffer = 0;
GLuint m_fbo = 0; GLuint m_fbo = 0;
GLuint m_vao = 0; GLuint m_vao = 0;
unsigned int m_program = 0; unsigned int m_program = 0;
bool m_programRenders = false;
std::string m_programError; std::string m_programError;
}; };
@@ -147,11 +176,10 @@ void main()
// Anything else means the factors were mistranslated rather than either honoured or declined. // Anything else means the factors were mistranslated rather than either honoured or declined.
TEST_F(DualSourceBlendScenario, DualSourceBlendDrawProducesOneOfTheTwoDefinedResults) { TEST_F(DualSourceBlendScenario, DualSourceBlendDrawProducesOneOfTheTwoDefinedResults) {
if (!Ready()) GTEST_SKIP(); if (!Ready()) GTEST_SKIP();
if (m_program == 0) { SkipUnlessTheProgramRuns();
GTEST_SKIP() << "this driver cannot build a dual-source fragment shader: " << m_programError;
}
glDisable(GL_BLEND); glDisable(GL_BLEND);
glBlendFunc(GL_ONE, GL_ZERO);
Draw(/*src0=*/0.0f, /*src1=*/0.0f); Draw(/*src0=*/0.0f, /*src1=*/0.0f);
glEnable(GL_BLEND); glEnable(GL_BLEND);
@@ -180,11 +208,10 @@ void main()
// source either way, so this case is really "no crash, no error, no surprise". // source either way, so this case is really "no crash, no error, no surprise".
TEST_F(DualSourceBlendScenario, DualSourceFactorsWithBlendingDisabledJustWriteTheSource) { TEST_F(DualSourceBlendScenario, DualSourceFactorsWithBlendingDisabledJustWriteTheSource) {
if (!Ready()) GTEST_SKIP(); if (!Ready()) GTEST_SKIP();
if (m_program == 0) { SkipUnlessTheProgramRuns();
GTEST_SKIP() << "this driver cannot build a dual-source fragment shader: " << m_programError;
}
glDisable(GL_BLEND); glDisable(GL_BLEND);
glBlendFunc(GL_ONE, GL_ZERO);
Draw(/*src0=*/0.0f, /*src1=*/0.0f); Draw(/*src0=*/0.0f, /*src1=*/0.0f);
glBlendFunc(GL_SRC1_ALPHA, GL_ONE_MINUS_SRC1_ALPHA); glBlendFunc(GL_SRC1_ALPHA, GL_ONE_MINUS_SRC1_ALPHA);
@@ -1856,7 +1856,13 @@ namespace MobileGL::MG_State::GLState {
// them to the draw-buffer range fails the link of every such program. // them to the draw-buffer range fails the link of every such program.
if (artifacts.program->getIntermediate(EShLangFragment) == nullptr) return true; if (artifacts.program->getIntermediate(EShLangFragment) == nullptr) return true;
UnorderedMap<Int, String> colorNumberOwners; // Keyed on (colour number, COLOUR INDEX), not on the colour number alone. Two fragment
// outputs may share a location as long as their index differs - that pair IS dual-source
// blending (GL 4.6 core 11.1.3 / ARB_blend_func_extended, core since 3.3), spelled either
// `layout(location = 0, index = 0)` + `layout(location = 0, index = 1)` in the shader or
// through two glBindFragDataLocationIndexed calls. Aliasing on the number alone made every
// such program fail to link with "alias color number 0", which is the whole feature.
UnorderedMap<Int64, String> colorSlotOwners;
const Int outputCount = artifacts.program->getNumPipeOutputs(); const Int outputCount = artifacts.program->getNumPipeOutputs();
for (Int index = 0; index < outputCount; ++index) { for (Int index = 0; index < outputCount; ++index) {
const auto& output = artifacts.program->getPipeOutput(index); const auto& output = artifacts.program->getPipeOutput(index);
@@ -1869,6 +1875,16 @@ namespace MobileGL::MG_State::GLState {
const Int location = explicitLocation != in.explicitFragDataLocation.end() const Int location = explicitLocation != in.explicitFragDataLocation.end()
? static_cast<Int>(explicitLocation->second) ? static_cast<Int>(explicitLocation->second)
: static_cast<Int>(output.layoutLocation()); : 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.
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);
}
const Int span = std::max<Int>(output.size, 1); const Int span = std::max<Int>(output.size, 1);
if (location < 0 || location + span > in.maxFragmentOutputColorNumber) { if (location < 0 || location + span > in.maxFragmentOutputColorNumber) {
@@ -1881,10 +1897,16 @@ namespace MobileGL::MG_State::GLState {
} }
for (Int colorNumber = location; colorNumber < location + span; ++colorNumber) { for (Int colorNumber = location; colorNumber < location + span; ++colorNumber) {
auto [owner, inserted] = colorNumberOwners.emplace(colorNumber, outputName); const Int64 slot = (static_cast<Int64>(colorIndex) << 32) |
static_cast<Int64>(static_cast<Uint32>(colorNumber));
auto [owner, inserted] = colorSlotOwners.emplace(slot, outputName);
if (!inserted) { if (!inserted) {
artifacts.infoLog = std::format("Fragment outputs '{}' and '{}' alias color number {}.", artifacts.infoLog =
owner->second, outputName, colorNumber); colorIndex == 0
? std::format("Fragment outputs '{}' and '{}' alias color number {}.", owner->second,
outputName, colorNumber)
: std::format("Fragment outputs '{}' and '{}' alias color number {} at index {}.",
owner->second, outputName, colorNumber, colorIndex);
DeferLog(std::format("ProgramObject {}: Link failed - {}", in.externalIndex, artifacts.infoLog)); DeferLog(std::format("ProgramObject {}: Link failed - {}", in.externalIndex, artifacts.infoLog));
ProgramObject::ResetLinkArtifacts(artifacts); ProgramObject::ResetLinkArtifacts(artifacts);
return false; return false;
+74
View File
@@ -4023,6 +4023,80 @@ void main() { mgColor = vec4(1.0); }
)"; )";
} // namespace } // namespace
// Two fragment outputs on ONE location with DIFFERENT colour indices is not an aliasing error -
// it is dual-source blending (GL 4.6 core 11.1.3 / ARB_blend_func_extended, core since 3.3), and
// the GL_SRC1_* blend factors have nothing to read without it. The link-time aliasing check keyed
// on the colour number alone, so every such program failed to link with "alias color number 0"
// and the whole feature was unreachable from shader-side GLSL.
TEST_F(ProgramTest, FragmentOutputsMayShareALocationWhenTheirColorIndexDiffers) {
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 = LinkStages({{GL_VERTEX_SHADER, kPassthroughVs}, {GL_FRAGMENT_SHADER, dualSourceFs}});
GLint linkStatus = GL_FALSE;
GetProgramiv(program, GL_LINK_STATUS, &linkStatus);
ASSERT_EQ(linkStatus, GL_TRUE) << [&] {
char log[512] = "";
GetProgramInfoLog(program, sizeof(log), nullptr, log);
return std::string(log);
}();
// Both outputs are active and both sit on colour number 0 - which is the shape that used to be
// refused. (glGetFragDataIndex still answers 0 for the index-1 output: it reports only what
// glBindFragDataLocationIndexed bound, and reflecting the shader-side qualifier is a separate
// gap, so it is deliberately not asserted here.)
EXPECT_EQ(GetFragDataLocation(program, "fragColor0"), 0);
EXPECT_EQ(GetFragDataLocation(program, "fragColor1"), 0);
EXPECT_EQ(GetError(), GL_NO_ERROR);
}
// The check it must NOT stop making: two outputs on the same colour number AND the same index
// really do alias, and that link has to fail. Aliased through glBindFragDataLocation rather than
// through two `layout(location = 0)` qualifiers on purpose - the qualifier form is caught by
// glslang at COMPILE time, so it would never reach the link-time rule this pins.
TEST_F(ProgramTest, FragmentOutputsSharingAColorNumberAtTheSameIndexStillFailToLink) {
constexpr const char* twoOutputFs = R"(#version 460 core
out vec4 fragColorA;
out vec4 fragColorB;
void main() { fragColorA = vec4(1.0); fragColorB = 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, &twoOutputFs, nullptr);
CompileShader(fs);
AttachShader(program, fs);
DeleteShader(fs);
BindFragDataLocation(program, 0, "fragColorA");
BindFragDataLocation(program, 0, "fragColorB");
LinkProgram(program);
GLint linkStatus = GL_TRUE;
GetProgramiv(program, GL_LINK_STATUS, &linkStatus);
EXPECT_EQ(linkStatus, GL_FALSE);
char infoLog[512] = "";
GetProgramInfoLog(program, sizeof(infoLog), nullptr, infoLog);
EXPECT_NE(std::string(infoLog).find("alias color number"), std::string::npos) << infoLog;
// ...and the same pair separated by the colour INDEX links, which is the whole point of the
// key being a pair.
BindFragDataLocationIndexed(program, 0, 1, "fragColorB");
LinkProgram(program);
GetProgramiv(program, GL_LINK_STATUS, &linkStatus);
EXPECT_EQ(linkStatus, GL_TRUE) << [&] {
char log[512] = "";
GetProgramInfoLog(program, sizeof(log), nullptr, log);
return std::string(log);
}();
for (int i = 0; i < 32 && GetError() != GL_NO_ERROR; ++i) {
}
}
TEST_F(ProgramTest, GetProgramivReportsTheGeometryStageLinkProperties) { TEST_F(ProgramTest, GetProgramivReportsTheGeometryStageLinkProperties) {
constexpr const char* gs = R"(#version 460 core constexpr const char* gs = R"(#version 460 core
layout(triangles, invocations = 3) in; layout(triangles, invocations = 3) in;