diff --git a/MobileGL/MG_Backend/DirectGLES/Utils.cpp b/MobileGL/MG_Backend/DirectGLES/Utils.cpp index fbc91690..2bf0a766 100644 --- a/MobileGL/MG_Backend/DirectGLES/Utils.cpp +++ b/MobileGL/MG_Backend/DirectGLES/Utils.cpp @@ -572,7 +572,10 @@ namespace MobileGL::MG_Backend::DirectGLES { ZoneScopedC(TRACY_ZONECOLOR_BACKEND); #endif // The name is the marker: ShaderSourceProcessor only emits it when the source - // wrote gl_FragColor, and such a shader can have no other output. + // names gl_FragColor, and a shader that writes it can have no other output. One that + // names it only in an inactive preprocessor arm keeps the declaration unwritten, and + // the IO resolver moves that inactive output off location 0, so the match below + // (which wants location 0) does not take it. static const char* const kLoweredName = "mg_FragColor"; if (shaderType != GL_FRAGMENT_SHADER || drawBufferCount <= 1) { return glslCode; diff --git a/MobileGL/MG_Test/Program/ProgramTest.cpp b/MobileGL/MG_Test/Program/ProgramTest.cpp index 0c776a8f..649e8e4e 100644 --- a/MobileGL/MG_Test/Program/ProgramTest.cpp +++ b/MobileGL/MG_Test/Program/ProgramTest.cpp @@ -1613,6 +1613,83 @@ void main() { EXPECT_EQ(locationMask, 0x1u); } +// An X server's 2D shaders name gl_FragColor only in the arm of a version guard that a 1.30 +// compile does not take, and write their own output instead. The legacy lowering still declares +// mg_FragColor (it cannot see the guard), ahead of the shader's output: that declaration is +// inactive and must not take location 0, or every copy and fill writes to a draw buffer nobody +// enabled and the window keeps whatever it held before. +TEST_F(ProgramTest, InactiveLoweredFragColorDoesNotDisplaceTheActiveOutput) { + const char* vertexSource = R"(#version 130 +in vec2 primitive; +out vec2 fill_pos; +uniform vec4 v_matrix; +void main() { + gl_Position = vec4(primitive * v_matrix.xz + v_matrix.yw, 0.0, 1.0); + fill_pos = primitive; +} +)"; + const char* fragmentSource = R"(#version 130 +#if __VERSION__ >= 130 +out vec4 frag_color; +#else +#define frag_color gl_FragColor +#endif +in vec2 fill_pos; +uniform sampler2D sampler; +void main() { + frag_color = texture(sampler, fill_pos); +} +)"; + const auto compile = [](GLenum stage, const char* source) { + const GLuint shader = CreateShader(stage); + ShaderSource(shader, 1, &source, nullptr); + CompileShader(shader); + GLint status = GL_FALSE; + GetShaderiv(shader, GL_COMPILE_STATUS, &status); + char log[1024] = ""; + GetShaderInfoLog(shader, sizeof log, nullptr, log); + EXPECT_EQ(status, GL_TRUE) << log; + return shader; + }; + const GLuint program = CreateProgram(); + AttachShader(program, compile(GL_VERTEX_SHADER, vertexSource)); + AttachShader(program, compile(GL_FRAGMENT_SHADER, fragmentSource)); + BindAttribLocation(program, 0, "primitive"); + LinkProgram(program); + GLint linkStatus = GL_FALSE; + GetProgramiv(program, GL_LINK_STATUS, &linkStatus); + ASSERT_EQ(linkStatus, GL_TRUE); + + EXPECT_EQ(GetFragDataLocation(program, "frag_color"), 0); + + auto programObject = MG_State::pGLContext->GetProgramObject(program); + ASSERT_NE(programObject, nullptr); + const Int fragmentIndex = programObject->GetShaderIndexByStage(ShaderStage::Fragment); + ASSERT_GE(fragmentIndex, 0); + const auto& spirvs = programObject->GetGeneratedSpirv(); + ASSERT_LT(static_cast(fragmentIndex), spirvs.size()); + const auto& fragmentSpirv = spirvs[fragmentIndex]; + spv_reflect::ShaderModule reflection(fragmentSpirv.size() * sizeof(Uint), fragmentSpirv.data()); + ASSERT_EQ(reflection.GetResult(), SPV_REFLECT_RESULT_SUCCESS); + uint32_t outputCount = 0; + ASSERT_EQ(reflection.EnumerateOutputVariables(&outputCount, nullptr), SPV_REFLECT_RESULT_SUCCESS); + Vector outputs(outputCount); + ASSERT_EQ(reflection.EnumerateOutputVariables(&outputCount, outputs.data()), SPV_REFLECT_RESULT_SUCCESS); + Bool sawActive = false; + for (const auto* output : outputs) { + if (output == nullptr || output->name == nullptr) continue; + const String name = output->name; + if (name == "frag_color") { + sawActive = true; + EXPECT_EQ(output->location, 0u); + } else { + // The lowered declaration, if it survives at all, sits clear of the active output. + EXPECT_NE(output->location, 0u) << name; + } + } + EXPECT_TRUE(sawActive); +} + TEST_F(ProgramTest, CompileAndLinkWithExplicitFragmentOut) { char infoLog[1024] = ""; diff --git a/MobileGL/MG_Util/ShaderTranspiler/glslang/TMglGlslIoResolver.cpp b/MobileGL/MG_Util/ShaderTranspiler/glslang/TMglGlslIoResolver.cpp index 686bd80b..d68be20e 100644 --- a/MobileGL/MG_Util/ShaderTranspiler/glslang/TMglGlslIoResolver.cpp +++ b/MobileGL/MG_Util/ShaderTranspiler/glslang/TMglGlslIoResolver.cpp @@ -152,6 +152,29 @@ namespace MobileGL { --m_nextInactiveVertexInLocation; } } + // INACTIVE FRAGMENT OUTPUTS go to the top of the draw-buffer range for the same reason: + // GL gives locations to active outputs only, and resolution runs in declaration order, so + // a dead output declared first would take location 0 and push the real one to 1, where no + // draw buffer reads it. The legacy lowering is where that happens: a source whose + // gl_FragColor sits in an inactive preprocessor arm (`#if __VERSION__ >= 130 / out vec4 + // frag_color; / #else / #define frag_color gl_FragColor`) still gets the injected + // mg_FragColor declaration ahead of its own output. The slot is reserved, so no active + // output auto-assigned later can land on it. + if (!ent.live && stage == EShLangFragment && type.getQualifier().isPipeOutput() && + !type.getQualifier().hasLocation() && !type.isBuiltIn()) { + const int size = std::max(1, glslang::TIntermediate::computeTypeLocationSize(type, stage)); + const int drawBuffers = referenceIntermediate.getLimits().maxDrawBuffers; + const int top = (drawBuffers > 0 ? drawBuffers : kMinimumDrawBuffers) - size; + const int key = buildStorageKey(EShLangFragment, glslang::EvqInOut); + for (int location = top; location >= 0; --location) { + bool free = true; + for (int i = 0; i < size && free; ++i) free = checkEmpty(key, location + i); + if (!free) continue; + reserveSlot(key, location, size); + ent.symbol->getWritableType().getQualifier().layoutLocation = location; + break; + } + } return TDefaultGlslIoResolver::resolveInOutLocation(stage, ent); } diff --git a/MobileGL/MG_Util/ShaderTranspiler/glslang/TMglGlslIoResolver.h b/MobileGL/MG_Util/ShaderTranspiler/glslang/TMglGlslIoResolver.h index f2fd0e35..3a871923 100644 --- a/MobileGL/MG_Util/ShaderTranspiler/glslang/TMglGlslIoResolver.h +++ b/MobileGL/MG_Util/ShaderTranspiler/glslang/TMglGlslIoResolver.h @@ -95,5 +95,8 @@ namespace MobileGL { // driver (caught by the super-duper-vanilla and chocapic retrace fixtures). static constexpr int kInactiveVertexInLocationTop = 15; int m_nextInactiveVertexInLocation = kInactiveVertexInLocationTop; + // The draw-buffer count every GL/ES implementation has, for an inactive fragment output + // when the compile carried no limit. + static constexpr int kMinimumDrawBuffers = 4; }; } // namespace MobileGL