[Fix] (Shader): an inactive fragment output no longer takes location 0 from the active one

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;

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<SizeT>(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<SpvReflectInterfaceVariable*> 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] = "";
 

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);
     }
 

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