diff --git a/MobileGL/MG_IntegrationTest/Scenarios/DualSourceBlendScenario.cpp b/MobileGL/MG_IntegrationTest/Scenarios/DualSourceBlendScenario.cpp index 7777bd70..6f130857 100644 --- a/MobileGL/MG_IntegrationTest/Scenarios/DualSourceBlendScenario.cpp +++ b/MobileGL/MG_IntegrationTest/Scenarios/DualSourceBlendScenario.cpp @@ -104,6 +104,22 @@ void main() glViewport(0, 0, kExtent, kExtent); glDisable(GL_SCISSOR_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(probe.At(kExtent / 2, kExtent / 2).r) > 245; + } for (int i = 0; i < 16 && glGetError() != GL_NO_ERROR; ++i) { } } @@ -129,10 +145,23 @@ void main() 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_fbo = 0; GLuint m_vao = 0; unsigned int m_program = 0; + bool m_programRenders = false; std::string m_programError; }; @@ -147,11 +176,10 @@ void main() // Anything else means the factors were mistranslated rather than either honoured or declined. TEST_F(DualSourceBlendScenario, DualSourceBlendDrawProducesOneOfTheTwoDefinedResults) { if (!Ready()) GTEST_SKIP(); - if (m_program == 0) { - GTEST_SKIP() << "this driver cannot build a dual-source fragment shader: " << m_programError; - } + SkipUnlessTheProgramRuns(); glDisable(GL_BLEND); + glBlendFunc(GL_ONE, GL_ZERO); Draw(/*src0=*/0.0f, /*src1=*/0.0f); glEnable(GL_BLEND); @@ -180,11 +208,10 @@ void main() // source either way, so this case is really "no crash, no error, no surprise". TEST_F(DualSourceBlendScenario, DualSourceFactorsWithBlendingDisabledJustWriteTheSource) { if (!Ready()) GTEST_SKIP(); - if (m_program == 0) { - GTEST_SKIP() << "this driver cannot build a dual-source fragment shader: " << m_programError; - } + SkipUnlessTheProgramRuns(); glDisable(GL_BLEND); + glBlendFunc(GL_ONE, GL_ZERO); Draw(/*src0=*/0.0f, /*src1=*/0.0f); glBlendFunc(GL_SRC1_ALPHA, GL_ONE_MINUS_SRC1_ALPHA); diff --git a/MobileGL/MG_State/GLState/ProgramState/ProgramLinkTask.cpp b/MobileGL/MG_State/GLState/ProgramState/ProgramLinkTask.cpp index 0e75f882..0df2f5f9 100644 --- a/MobileGL/MG_State/GLState/ProgramState/ProgramLinkTask.cpp +++ b/MobileGL/MG_State/GLState/ProgramState/ProgramLinkTask.cpp @@ -1856,7 +1856,13 @@ namespace MobileGL::MG_State::GLState { // them to the draw-buffer range fails the link of every such program. if (artifacts.program->getIntermediate(EShLangFragment) == nullptr) return true; - UnorderedMap 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 colorSlotOwners; const Int outputCount = artifacts.program->getNumPipeOutputs(); for (Int index = 0; index < outputCount; ++index) { const auto& output = artifacts.program->getPipeOutput(index); @@ -1869,6 +1875,16 @@ namespace MobileGL::MG_State::GLState { const Int location = explicitLocation != in.explicitFragDataLocation.end() ? static_cast(explicitLocation->second) : static_cast(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(explicitIndex->second); + } else if (const glslang::TType* outputType = output.getType(); + outputType != nullptr && outputType->getQualifier().hasIndex()) { + colorIndex = static_cast(outputType->getQualifier().layoutIndex); + } const Int span = std::max(output.size, 1); if (location < 0 || location + span > in.maxFragmentOutputColorNumber) { @@ -1881,10 +1897,16 @@ namespace MobileGL::MG_State::GLState { } for (Int colorNumber = location; colorNumber < location + span; ++colorNumber) { - auto [owner, inserted] = colorNumberOwners.emplace(colorNumber, outputName); + const Int64 slot = (static_cast(colorIndex) << 32) | + static_cast(static_cast(colorNumber)); + auto [owner, inserted] = colorSlotOwners.emplace(slot, outputName); if (!inserted) { - artifacts.infoLog = std::format("Fragment outputs '{}' and '{}' alias color number {}.", - owner->second, outputName, colorNumber); + artifacts.infoLog = + 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)); ProgramObject::ResetLinkArtifacts(artifacts); return false; diff --git a/MobileGL/MG_Test/Program/ProgramTest.cpp b/MobileGL/MG_Test/Program/ProgramTest.cpp index ff3c38aa..107062cc 100644 --- a/MobileGL/MG_Test/Program/ProgramTest.cpp +++ b/MobileGL/MG_Test/Program/ProgramTest.cpp @@ -4023,6 +4023,80 @@ void main() { mgColor = vec4(1.0); } )"; } // 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) { constexpr const char* gs = R"(#version 460 core layout(triangles, invocations = 3) in;