From cef81df73f3c9510c2aadd60dfa8b6c7e938a04e Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Tue, 11 Aug 2026 23:28:43 -0400 Subject: [PATCH 1/3] [Fix, Test] (ShaderTranspiler, DirectGLES): fold or lower dynamically indexed fragment outputs before ESSL emission - GLSL ES requires constant integral indices, so the OIT coefficient shader linked nothing on ANGLE and every translucent draw was a silent no-op --- CMakeLists.txt | 1 + MobileGL/MG_IntegrationTest/CMakeLists.txt | 1 + .../FragmentOutputArrayIndexScenario.cpp | 227 +++++++ MobileGL/MG_Test/Program/ProgramUtilTest.cpp | 246 ++++++++ .../ShaderTranspiler/ShaderCompiler.cpp | 70 +++ .../MG_Util/ShaderTranspiler/ShaderCompiler.h | 11 + .../LegalizeFragmentOutputIndexPass.cpp | 580 ++++++++++++++++++ .../LegalizeFragmentOutputIndexPass.h | 111 ++++ 8 files changed, 1247 insertions(+) create mode 100644 MobileGL/MG_IntegrationTest/Scenarios/FragmentOutputArrayIndexScenario.cpp create mode 100644 MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/LegalizeFragmentOutputIndexPass.cpp create mode 100644 MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/LegalizeFragmentOutputIndexPass.h diff --git a/CMakeLists.txt b/CMakeLists.txt index d288bab1..7a84f51d 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -285,6 +285,7 @@ set(SOURCE_FILES MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/StripUboMemberRelaxedPrecisionPass.cpp MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/StripNoPerspectivePass.cpp MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/EmulateNoPerspectivePass.cpp + MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/LegalizeFragmentOutputIndexPass.cpp MobileGL/MG_Util/BackendLoaders/OpenGL/Loader.cpp MobileGL/MG_Util/BackendLoaders/Vulkan/Loader.cpp diff --git a/MobileGL/MG_IntegrationTest/CMakeLists.txt b/MobileGL/MG_IntegrationTest/CMakeLists.txt index 48963737..cdd13421 100644 --- a/MobileGL/MG_IntegrationTest/CMakeLists.txt +++ b/MobileGL/MG_IntegrationTest/CMakeLists.txt @@ -64,6 +64,7 @@ add_executable(MobileGLIntegrationTest Scenarios/SwizzleAccessRoutineScenario.cpp Scenarios/ProgramPipelineScenario.cpp Scenarios/Glsl420DeclarationScenario.cpp + Scenarios/FragmentOutputArrayIndexScenario.cpp ) target_include_directories(MobileGLIntegrationTest PRIVATE diff --git a/MobileGL/MG_IntegrationTest/Scenarios/FragmentOutputArrayIndexScenario.cpp b/MobileGL/MG_IntegrationTest/Scenarios/FragmentOutputArrayIndexScenario.cpp new file mode 100644 index 00000000..80e2f1ab --- /dev/null +++ b/MobileGL/MG_IntegrationTest/Scenarios/FragmentOutputArrayIndexScenario.cpp @@ -0,0 +1,227 @@ +// MobileGL - MobileGL/MG_IntegrationTest/Scenarios/FragmentOutputArrayIndexScenario.cpp +// Copyright (c) 2025-2026 MobileGL-Dev +// Licensed under the GNU Lesser General Public License v3.0: +// https://www.gnu.org/licenses/gpl-3.0.txt +// https://www.gnu.org/licenses/lgpl-3.0.txt +// SPDX-License-Identifier: LGPL-3.0-only +// End of Source File Header +// +// Scenario - DYNAMICALLY INDEXED FRAGMENT OUTPUT ARRAYS, on a live driver. +// +// The bug: GLSL ES requires a *constant integral expression* to index a fragment output array +// (GLSL ES 3.00 4.3.6); SPIR-V has no such rule. A shader that writes `coeff[i]` from a loop +// therefore travels through glslang and SPIRV-Cross intact and lands on the ES driver as ESSL it +// refuses outright - "array indexes for fragment outputs must be constant integral expressions". +// The program links nothing and every draw that uses it becomes a silent no-op. That is the whole +// of improved-transparency-minecraft-26.3 on the Android DirectGLES lane: Minecraft 26.3's OIT +// coefficient shader has exactly this shape, and losing it empties the entire translucent layer +// (clouds and water) while the opaque geometry stays pixel-exact. +// +// WHY THIS SCENARIO EXISTS RATHER THAN A UNIT TEST. The unit tests in MG_Test/Program (see +// ProgramUtilTest, LoopDerivedFragmentOutputIndexFoldsToConstantIndices and its +// genuinely-dynamic sibling) prove the SPIR-V comes out with constant indices, validates, and +// decompiles to ESSL with only literal indices. What they cannot prove is that a real driver +// then ACCEPTS and RUNS it - and acceptance is the whole failure mode, because Mesa accepts the +// illegal form too. Only a live glCompileShader/glLinkProgram followed by a draw can tell the two +// apart, and only reading the pixels back can tell "linked" from "wrote the right attachment". +// +// Both backends run this: on DirectVulkan the original module is already legal (the legalization +// is DirectGLES-only, deliberately), so this doubles as the check that the two backends agree +// about what such a shader means. + +#include +#include +#include + +#include "../Harness/HeadlessGL.h" +#include "../Harness/ScenarioFixture.h" + +#ifdef GLAPI +#undef GLAPI +#endif +#define GL_GLEXT_PROTOTYPES +#include +#include +#undef GL_GLEXT_PROTOTYPES + +namespace MGITest { + namespace { + + constexpr const char* kVS = R"(#version 330 core +in vec2 aPos; +void main() { + gl_Position = vec4(aPos, 0.0, 1.0); +} +)"; + + // The Minecraft 26.3 OIT coefficient shape: both the attachment index and the component + // index come from loop counters, so nothing but the loop bounds decides where each value + // lands. Attachment 0 gets (0.0, 0.1, 0.2, 0.3) and attachment 1 gets (0.5, 0.6, 0.7, 0.8) - + // values that are only correct if the two indices were folded to the RIGHT constants, not + // merely to some constant. + constexpr const char* kLoopIndexedFS = R"(#version 330 core +out vec4 coeff[2]; +void main() { + for (int attachmentIndex = 0; attachmentIndex < 2; ++attachmentIndex) { + for (int i = 0; i < 4; ++i) { + coeff[attachmentIndex][i] = float(attachmentIndex) * 0.5 + float(i) * 0.1; + } + } +} +)"; + + // No loop can fold this one: the index arrives in a uniform. It exercises the fallback + // lowering (a switch over the array range for the write, constant-indexed loads and a + // select for the read) and it checks the untargeted attachment is left ALONE, which a + // lowering that wrote every element unconditionally would break. + constexpr const char* kUniformIndexedFS = R"(#version 330 core +uniform int uTarget; +out vec4 coeff[2]; +void main() { + coeff[0] = vec4(0.25, 0.25, 0.25, 1.0); + coeff[1] = vec4(0.75, 0.75, 0.75, 1.0); + coeff[uTarget] = coeff[uTarget] + vec4(0.25, 0.0, 0.0, 0.0); +} +)"; + + constexpr int kSize = 8; + + class FragmentOutputArrayIndexScenario : public ScenarioTest { + protected: + void SetUp() override { + ScenarioTest::SetUp(); + if (!Ready()) return; + + glGenFramebuffers(1, &m_fbo); + glBindFramebuffer(GL_FRAMEBUFFER, m_fbo); + for (int i = 0; i < 2; ++i) { + glGenTextures(1, &m_color[i]); + glBindTexture(GL_TEXTURE_2D, m_color[i]); + glTexStorage2D(GL_TEXTURE_2D, 1, GL_RGBA8, kSize, kSize); + glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST); + glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST); + glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0 + i, GL_TEXTURE_2D, + m_color[i], 0); + } + const GLenum drawBuffers[2] = {GL_COLOR_ATTACHMENT0, GL_COLOR_ATTACHMENT1}; + glDrawBuffers(2, drawBuffers); + ASSERT_EQ(glCheckFramebufferStatus(GL_FRAMEBUFFER), + static_cast(GL_FRAMEBUFFER_COMPLETE)); + + const float quad[] = {-1.0f, -1.0f, 1.0f, -1.0f, -1.0f, 1.0f, 1.0f, 1.0f}; + glGenVertexArrays(1, &m_vao); + glBindVertexArray(m_vao); + glGenBuffers(1, &m_vbo); + glBindBuffer(GL_ARRAY_BUFFER, m_vbo); + glBufferData(GL_ARRAY_BUFFER, sizeof(quad), quad, GL_STATIC_DRAW); + glEnableVertexAttribArray(0); + glVertexAttribPointer(0, 2, GL_FLOAT, GL_FALSE, 2 * sizeof(float), nullptr); + glViewport(0, 0, kSize, kSize); + } + + void TearDown() override { + if (Ready()) { + glBindFramebuffer(GL_FRAMEBUFFER, 0); + glDeleteFramebuffers(1, &m_fbo); + glDeleteTextures(2, m_color); + glDeleteBuffers(1, &m_vbo); + glDeleteVertexArrays(1, &m_vao); + } + ScenarioTest::TearDown(); + } + + // Clears both attachments to a colour no shader below writes, so an attachment that + // was never written reads back as the sentinel rather than as a plausible value. + void ClearToSentinel() { + glClearColor(0.0f, 0.0f, 1.0f, 1.0f); + glClear(GL_COLOR_BUFFER_BIT); + } + + std::vector ReadAttachment(int index) { + std::vector bytes(static_cast(kSize) * kSize * 4, 0); + glReadBuffer(GL_COLOR_ATTACHMENT0 + index); + glReadPixels(0, 0, kSize, kSize, GL_RGBA, GL_UNSIGNED_BYTE, bytes.data()); + std::vector centre(4, -1.0f); + // The middle pixel: the quad covers the whole target, so every pixel is the same, + // and the middle one cannot be a rasterization edge case. + const std::size_t offset = (static_cast(kSize / 2) * kSize + kSize / 2) * 4; + for (int i = 0; i < 4; ++i) { + centre[static_cast(i)] = static_cast(bytes[offset + i]) / 255.0f; + } + return centre; + } + + GLuint m_fbo = 0; + GLuint m_color[2] = {0, 0}; + GLuint m_vao = 0; + GLuint m_vbo = 0; + }; + + // The gate for the whole defect: before the legalization this program did not link on a + // strict ES driver (ANGLE), so the draw wrote nothing and BOTH attachments kept the + // sentinel. Now each attachment must carry the value its loop iteration produced. + TEST_F(FragmentOutputArrayIndexScenario, LoopIndexedOutputArrayWritesEveryAttachment) { + if (!Ready() || IsSkipped()) return; + + std::string error; + const GLuint program = CompileProgram(kVS, kLoopIndexedFS, &error); + ASSERT_NE(program, 0u) << "a loop-indexed fragment output array must compile and link: " + << error; + + ClearToSentinel(); + glUseProgram(program); + glDrawArrays(GL_TRIANGLE_STRIP, 0, 4); + + const std::vector first = ReadAttachment(0); + EXPECT_NEAR(first[0], 0.0f, 0.02f) << "attachment 0 red"; + EXPECT_NEAR(first[1], 0.1f, 0.02f) << "attachment 0 green"; + EXPECT_NEAR(first[2], 0.2f, 0.02f) + << "attachment 0 blue - a sentinel 1.0 here means the draw never ran"; + EXPECT_NEAR(first[3], 0.3f, 0.02f) << "attachment 0 alpha"; + + const std::vector second = ReadAttachment(1); + EXPECT_NEAR(second[0], 0.5f, 0.02f) + << "attachment 1 red - the second loop iteration must reach the second draw buffer"; + EXPECT_NEAR(second[1], 0.6f, 0.02f) << "attachment 1 green"; + EXPECT_NEAR(second[2], 0.7f, 0.02f) << "attachment 1 blue"; + EXPECT_NEAR(second[3], 0.8f, 0.02f) << "attachment 1 alpha"; + + glDeleteProgram(program); + EXPECT_EQ(FirstGLError(), 0u) << GLErrorName(FirstGLError()); + } + + // The fallback half, on a live driver, for both values of the uniform: the targeted + // attachment is read, incremented and written back; the other one keeps exactly what the + // constant-indexed store put there. + TEST_F(FragmentOutputArrayIndexScenario, UniformIndexedOutputArrayWritesOnlyTheSelectedAttachment) { + if (!Ready() || IsSkipped()) return; + + std::string error; + const GLuint program = CompileProgram(kVS, kUniformIndexedFS, &error); + ASSERT_NE(program, 0u) << "a uniform-indexed fragment output array must compile and link: " + << error; + const GLint targetLocation = glGetUniformLocation(program, "uTarget"); + ASSERT_GE(targetLocation, 0); + glUseProgram(program); + + for (int target = 0; target < 2; ++target) { + ClearToSentinel(); + glUniform1i(targetLocation, target); + glDrawArrays(GL_TRIANGLE_STRIP, 0, 4); + + const std::vector first = ReadAttachment(0); + const std::vector second = ReadAttachment(1); + EXPECT_NEAR(first[0], target == 0 ? 0.5f : 0.25f, 0.02f) + << "attachment 0 red with uTarget=" << target; + EXPECT_NEAR(first[1], 0.25f, 0.02f) << "attachment 0 green with uTarget=" << target; + EXPECT_NEAR(second[0], target == 1 ? 1.0f : 0.75f, 0.02f) + << "attachment 1 red with uTarget=" << target; + EXPECT_NEAR(second[1], 0.75f, 0.02f) << "attachment 1 green with uTarget=" << target; + } + + glDeleteProgram(program); + EXPECT_EQ(FirstGLError(), 0u) << GLErrorName(FirstGLError()); + } + + } // namespace +} // namespace MGITest diff --git a/MobileGL/MG_Test/Program/ProgramUtilTest.cpp b/MobileGL/MG_Test/Program/ProgramUtilTest.cpp index 0c2025ac..744fbf71 100644 --- a/MobileGL/MG_Test/Program/ProgramUtilTest.cpp +++ b/MobileGL/MG_Test/Program/ProgramUtilTest.cpp @@ -20,6 +20,7 @@ #include #include #include +#include #include #include #include @@ -3079,3 +3080,248 @@ void main() { EXPECT_EQ(ShaderCompiler::SpirvValidationFailureCount(), failuresBefore) << "the stripped module must validate clean"; } + +// --- Fragment-output array indexing (GLSL ES needs a constant integral expression) ------------- +// +// SPIR-V lets a fragment shader index an output array with any integer; GLSL ES does not +// (GLSL ES 3.00 4.3.6). SPIRV-Cross carries the dynamic index straight into the ESSL, a strict +// driver rejects the shader, the program links nothing, and every draw using it silently draws +// nothing - which is what empties the translucent layer of improved-transparency-minecraft-26.3 +// on the Android DirectGLES (ANGLE) lane while Mesa, being lenient, renders it correctly. +namespace { + Vector CompileFragmentToRawSpirv(const String& source) { + using namespace MG_Util::ShaderTranspiler; + ShaderAttrib shaderAttrib{.shaderType = GL_FRAGMENT_SHADER, .sourceStr = source}; + auto shaderResult = ShaderCompiler::CompileShader(shaderAttrib); + if (!shaderResult) { + ADD_FAILURE() << shaderResult.error().log; + return {}; + } + ProgramAttrib programAttrib{.shaders = {shaderResult.value()}}; + auto programResult = ShaderCompiler::LinkProgram(programAttrib); + if (!programResult) { + ADD_FAILURE() << programResult.error().log; + return {}; + } + ProgramBinaryAttrib binaryAttrib{.shaderTypes = {GL_FRAGMENT_SHADER}, + .program = *programResult.value()}; + auto binaryResult = ShaderCompiler::GetSpirvBinaryFromProgram(binaryAttrib); + if (!binaryResult || binaryResult->size() != 1u) { + ADD_FAILURE() << (binaryResult ? "unexpected module count" : binaryResult.error().log); + return {}; + } + return binaryResult->front(); + } + + String DisassembleSpirv(const Vector& binary) { + spvtools::SpirvTools tools(SPV_ENV_VULKAN_1_1); + String text; + tools.Disassemble(binary, &text); + return text; + } + + // Every `name[` in the emitted ESSL is followed by a digit. A surviving dynamic index reads + // `coeff[attachmentIndex]` or `coeff[_123]`, which is the exact construct ES compilers refuse. + bool AllArrayIndicesAreLiterals(const String& essl, const String& name) { + const String needle = name + "["; + SizeT offset = 0; + bool sawAny = false; + while ((offset = essl.find(needle, offset)) != String::npos) { + const SizeT indexStart = offset + needle.size(); + if (indexStart >= essl.size()) return false; + // A declaration (`out vec4 coeff[2];`) and a constant index both read as a digit. + if (std::isdigit(static_cast(essl[indexStart])) == 0) return false; + sawAny = true; + offset = indexStart; + } + return sawAny; + } + + String DecompileToEssl(const Vector& binary) { + using namespace MG_Util::ShaderTranspiler; + SpvcSession session(binary, SessionUsageBit::Transpile); + auto essl = ShaderCompiler::DecompileShader(session); + if (!essl) { + ADD_FAILURE() << "decompile errc: " << essl.error().errc << "\nlog: " << essl.error().log; + return {}; + } + return essl.value(); + } +} // namespace + +// The shape Minecraft 26.3's OIT coefficient shader has: the index comes from a loop counter, so +// the stock folding chain (loop-control hint, ssa-rewrite, loop-unroll, ccp, simplification, +// dead-branch-elim) turns every write into a constant-indexed one and the fallback never runs. +TEST_F(ProgramUtilTest, LoopDerivedFragmentOutputIndexFoldsToConstantIndices) { + using namespace MG_Util::ShaderTranspiler; + + const Vector raw = CompileFragmentToRawSpirv(R"(#version 330 core +out vec4 coeff[2]; +in vec4 vColor; +in float vDepth; +void main() { + for (int attachmentIndex = 0; attachmentIndex < 2; ++attachmentIndex) { + for (int i = 0; i < 4; ++i) { + coeff[attachmentIndex][i] = vColor[i] * float(attachmentIndex + i) * vDepth; + } + } +} +)"); + ASSERT_FALSE(raw.empty()); + ASSERT_TRUE(LegalizeFragmentOutputIndexPass::BinaryHasDynamicOutputIndexing(raw)) + << "the fixture must reproduce the defect before the fix is asked to remove it:\n" + << DisassembleSpirv(raw); + + SpirvValidationScope validationOn(true); + const Uint64 failuresBefore = ShaderCompiler::SpirvValidationFailureCount(); + + Vector legalized; + ASSERT_TRUE(ShaderCompiler::LegalizeFragmentOutputIndexingForEssl(raw, legalized)); + ASSERT_FALSE(legalized.empty()); + + const String disassembly = DisassembleSpirv(legalized); + EXPECT_FALSE(LegalizeFragmentOutputIndexPass::BinaryHasDynamicOutputIndexing(legalized)) + << "no fragment output may be left indexed by anything but a constant:\n" << disassembly; + EXPECT_EQ(disassembly.find("OpSwitch"), String::npos) + << "a loop-derived index must fold, not fall back to the switch lowering:\n" << disassembly; + EXPECT_EQ(ShaderCompiler::SpirvValidationFailureCount(), failuresBefore) + << "the legalized module must stay validator-clean"; + + const String essl = DecompileToEssl(legalized); + ASSERT_FALSE(essl.empty()); + EXPECT_TRUE(AllArrayIndicesAreLiterals(essl, "coeff")) + << "the generated ESSL still indexes a fragment output with a non-constant:\n" << essl; +} + +// The fallback half: an index computed from a uniform cannot be folded by any amount of +// unrolling, so the write becomes a switch over the array's range and the read becomes +// constant-indexed loads combined with selects. +TEST_F(ProgramUtilTest, GenuinelyDynamicFragmentOutputIndexLowersToConstantSwitch) { + using namespace MG_Util::ShaderTranspiler; + + const Vector raw = CompileFragmentToRawSpirv(R"(#version 330 core +uniform int uTarget; +out vec4 coeff[2]; +in vec4 vColor; +void main() { + coeff[0] = vColor; + coeff[1] = vColor * 0.5; + coeff[uTarget] = coeff[uTarget] * 2.0; +} +)"); + ASSERT_FALSE(raw.empty()); + ASSERT_TRUE(LegalizeFragmentOutputIndexPass::BinaryHasDynamicOutputIndexing(raw)) + << DisassembleSpirv(raw); + + SpirvValidationScope validationOn(true); + const Uint64 failuresBefore = ShaderCompiler::SpirvValidationFailureCount(); + + Vector legalized; + ASSERT_TRUE(ShaderCompiler::LegalizeFragmentOutputIndexingForEssl(raw, legalized)); + ASSERT_FALSE(legalized.empty()); + + const String disassembly = DisassembleSpirv(legalized); + EXPECT_FALSE(LegalizeFragmentOutputIndexPass::BinaryHasDynamicOutputIndexing(legalized)) + << "the uniform-driven index must be lowered away:\n" << disassembly; + EXPECT_NE(disassembly.find("OpSwitch"), String::npos) + << "the dynamic write must become a switch over the array range:\n" << disassembly; + EXPECT_NE(disassembly.find("OpSelect"), String::npos) + << "the dynamic read must become constant-indexed loads and a select:\n" << disassembly; + EXPECT_EQ(ShaderCompiler::SpirvValidationFailureCount(), failuresBefore) + << "the lowered module must stay validator-clean:\n" << disassembly; + + const String essl = DecompileToEssl(legalized); + ASSERT_FALSE(essl.empty()); + EXPECT_TRUE(AllArrayIndicesAreLiterals(essl, "coeff")) + << "the generated ESSL still indexes a fragment output with a non-constant:\n" << essl; +} + +// The bound on the folding half. The index here IS loop-derived, so unrolling would fold it - +// but the loop runs 512 times, and fully unrolling it would multiply the shader by 512 to save +// a switch with two cases. Past the trip-count cap the loop is left alone and the fallback takes +// it, which is cheap in the array length instead of the trip count. +TEST_F(ProgramUtilTest, ALoopTooLongToUnrollFallsBackToTheSwitchLowering) { + using namespace MG_Util::ShaderTranspiler; + + const Vector raw = CompileFragmentToRawSpirv(R"(#version 330 core +out vec4 coeff[2]; +in vec4 vColor; +void main() { + coeff[0] = vec4(0.0); + coeff[1] = vec4(0.0); + for (int i = 0; i < 512; ++i) { + coeff[i % 2] += vColor * 0.001; + } +} +)"); + ASSERT_FALSE(raw.empty()); + ASSERT_TRUE(LegalizeFragmentOutputIndexPass::BinaryHasDynamicOutputIndexing(raw)); + + SpirvValidationScope validationOn(true); + const Uint64 failuresBefore = ShaderCompiler::SpirvValidationFailureCount(); + + Vector legalized; + ASSERT_TRUE(ShaderCompiler::LegalizeFragmentOutputIndexingForEssl(raw, legalized)); + ASSERT_FALSE(legalized.empty()); + + const String disassembly = DisassembleSpirv(legalized); + EXPECT_FALSE(LegalizeFragmentOutputIndexPass::BinaryHasDynamicOutputIndexing(legalized)) + << "the index must be legalized even when the loop is left standing:\n" << disassembly; + EXPECT_NE(disassembly.find("OpLoopMerge"), String::npos) + << "a 512-trip loop must NOT be unrolled - that is the whole point of the cap:\n" + << disassembly; + EXPECT_NE(disassembly.find("OpSwitch"), String::npos) + << "with the loop standing, the write must go through the switch lowering:\n" << disassembly; + EXPECT_EQ(ShaderCompiler::SpirvValidationFailureCount(), failuresBefore) + << "lowering inside a loop body must stay validator-clean:\n" << disassembly; + + const String essl = DecompileToEssl(legalized); + ASSERT_FALSE(essl.empty()); + EXPECT_TRUE(AllArrayIndicesAreLiterals(essl, "coeff")) + << "the generated ESSL still indexes a fragment output with a non-constant:\n" << essl; +} + +// The gate: a fragment shader that never indexes an output array dynamically must come back byte +// for byte, so no shader that did not need this pays for it or is perturbed by it. +TEST_F(ProgramUtilTest, FragmentWithoutDynamicOutputIndexingIsPassedThroughUnchanged) { + using namespace MG_Util::ShaderTranspiler; + + const Vector raw = CompileFragmentToRawSpirv(R"(#version 330 core +out vec4 coeff[2]; +in vec4 vColor; +void main() { + for (int i = 0; i < 4; ++i) { + coeff[0][i] = vColor[i]; + } + coeff[1] = vColor; +} +)"); + ASSERT_FALSE(raw.empty()); + ASSERT_FALSE(LegalizeFragmentOutputIndexPass::BinaryHasDynamicOutputIndexing(raw)); + + Vector legalized; + ASSERT_TRUE(ShaderCompiler::LegalizeFragmentOutputIndexingForEssl(raw, legalized)); + EXPECT_EQ(legalized, raw) << "the module must not be rewritten - not even re-serialized - when " + "nothing indexes a fragment output dynamically"; +} + +// Stages other than fragment may index an output array dynamically in ESSL (the array here is a +// varying, not a draw buffer), so detection must not fire on them at all. +TEST_F(ProgramUtilTest, DynamicOutputIndexingOutsideTheFragmentStageIsNotDetected) { + using namespace MG_Util::ShaderTranspiler; + + const Vector raw = CompileVertexToRawSpirv(R"(#version 330 core +in vec3 a_Position; +out vec4 v_Values[2]; +uniform int uTarget; +void main() { + v_Values[0] = vec4(0.0); + v_Values[1] = vec4(1.0); + v_Values[uTarget] = vec4(a_Position, 1.0); + gl_Position = vec4(a_Position, 1.0); +} +)"); + ASSERT_FALSE(raw.empty()); + EXPECT_FALSE(LegalizeFragmentOutputIndexPass::BinaryHasDynamicOutputIndexing(raw)) + << "only fragment outputs carry the constant-index rule:\n" << DisassembleSpirv(raw); +} diff --git a/MobileGL/MG_Util/ShaderTranspiler/ShaderCompiler.cpp b/MobileGL/MG_Util/ShaderTranspiler/ShaderCompiler.cpp index a5392127..e9bd54e3 100644 --- a/MobileGL/MG_Util/ShaderTranspiler/ShaderCompiler.cpp +++ b/MobileGL/MG_Util/ShaderTranspiler/ShaderCompiler.cpp @@ -27,6 +27,7 @@ #include "SpirvPasses/StripUboMemberRelaxedPrecisionPass.h" #include "SpirvPasses/StripNoPerspectivePass.h" #include "SpirvPasses/EmulateNoPerspectivePass.h" +#include "SpirvPasses/LegalizeFragmentOutputIndexPass.h" #include "spirv-tools/libspirv.h" #include "spirv-tools/optimizer.hpp" @@ -631,6 +632,75 @@ namespace MobileGL { outputBinary); } + bool ShaderCompiler::LegalizeFragmentOutputIndexingForEssl(const Vector& inputBinary, + Vector& outputBinary) { + using namespace spvtools; + + // Detection gates everything: a module with no dynamically indexed fragment + // output - every shader but a handful - pays one BuildModule and is handed + // back byte for byte, so the folding chain can never perturb a shader that + // did not need it. + if (!LegalizeFragmentOutputIndexPass::BinaryHasDynamicOutputIndexing(inputBinary)) { + outputBinary = inputBinary; + return true; + } + + // Stock passes do the real work. The only bespoke member is the loop-control + // hint the stock unroller demands (see the pass header); with it set, an index + // derived from a loop counter - the shape of the Minecraft 26.3 OIT + // coefficient shader and of most real ones - folds to a literal here, and the + // fallback below never runs. + Optimizer folder(SPV_ENV_VULKAN_1_1); + // First, because both the unroller and the marking pass below read the + // induction variable as an OpPhi, and glslang emits it as loads and stores of + // a Function variable. + folder.RegisterPass(CreateLocalMultiStoreElimPass()); + folder.RegisterPass(LegalizeFragmentOutputIndexPass::CreateMarkLoopsForUnrollPass()); + folder.RegisterPass(CreateLoopUnrollPass(true)); + // Fold the unrolled induction values into the access chains, then clear out + // what constant conditions leave behind. + folder.RegisterPass(CreateCCPPass()); + folder.RegisterPass(CreateSimplificationPass()); + folder.RegisterPass(CreateDeadBranchElimPass()); + folder.RegisterPass(CreateBlockMergePass()); + + Vector folded; + if (!RunOptimizerChecked("LegalizeFragmentOutputIndexingForEssl.fold", folder, inputBinary, + folded) || + folded.empty()) { + // Fail open onto the fallback rather than onto the illegal module. + folded = inputBinary; + } + + if (!LegalizeFragmentOutputIndexPass::BinaryHasDynamicOutputIndexing(folded)) { + outputBinary = folded; + return true; + } + + // Genuinely dynamic (uniform-derived, non-constant trip count, ...): lower it. + Optimizer lowerer(SPV_ENV_VULKAN_1_1); + lowerer.RegisterPass(LegalizeFragmentOutputIndexPass::CreateLowerToConstantSwitchPass()); + // The chains the lowering replaced are dead now; remove_outputs must stay + // false here for the same reason it does in SanitizeAndOptimizeBinary. + lowerer.RegisterPass(CreateAggressiveDCEPass(false)); + + if (!RunOptimizerChecked("LegalizeFragmentOutputIndexingForEssl.lower", lowerer, folded, + outputBinary) || + outputBinary.empty()) { + outputBinary = folded; + return true; + } + + if (LegalizeFragmentOutputIndexPass::BinaryHasDynamicOutputIndexing(outputBinary)) { + // MGLOG_I, deliberately: MGLOG_E/W are compiled out at the INFO level every + // CI and retrace build uses, and this is precisely the diagnostic that has + // to survive to explain a shader the driver is about to reject. + MGLOG_I("[spirv] LegalizeFragmentOutputIndexingForEssl: a fragment output is still " + "indexed dynamically; a strict ES driver will reject this shader"); + } + return true; + } + bool ShaderCompiler::LowerRectImages(const Vector& inputBinary, Vector& outputBinary) { using namespace spvtools; diff --git a/MobileGL/MG_Util/ShaderTranspiler/ShaderCompiler.h b/MobileGL/MG_Util/ShaderTranspiler/ShaderCompiler.h index 895c20ce..85449154 100644 --- a/MobileGL/MG_Util/ShaderTranspiler/ShaderCompiler.h +++ b/MobileGL/MG_Util/ShaderTranspiler/ShaderCompiler.h @@ -43,6 +43,17 @@ namespace MobileGL { // devices lacking GL_NV_shader_noperspective_interpolation. See EmulateNoPerspectivePass. static bool EmulateNoPerspectiveForEssl(const Vector& inputBinary, Vector& outputBinary); + // Makes every index into a fragment-output array a constant integral + // expression, which is what GLSL ES requires and SPIR-V does not. Runs the + // stock folding chain first (loop unrolling folds the loop-derived indices + // real shaders use), and lowers whatever is left - a genuinely dynamic index - + // to a switch over the array's range. DirectGLES transpile path only: the + // original module is legal for Vulkan, and no other stage is constrained this + // way. Copies the input through untouched when no fragment output is indexed + // dynamically, which is every shader but a handful. + // See LegalizeFragmentOutputIndexPass. + static bool LegalizeFragmentOutputIndexingForEssl(const Vector& inputBinary, + Vector& outputBinary); // Rebases loads of the InstanceIndex builtin to (InstanceIndex - BaseInstance) so // shaders see GL's zero-based gl_InstanceID. Vertex shaders only; DirectVulkan // backend only (glslang's relaxed mode aliases gl_InstanceID to gl_InstanceIndex, diff --git a/MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/LegalizeFragmentOutputIndexPass.cpp b/MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/LegalizeFragmentOutputIndexPass.cpp new file mode 100644 index 00000000..ecfd89eb --- /dev/null +++ b/MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/LegalizeFragmentOutputIndexPass.cpp @@ -0,0 +1,580 @@ +// MobileGL - MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/LegalizeFragmentOutputIndexPass.cpp +// Copyright (c) 2025-2026 MobileGL-Dev +// Licensed under the GNU Lesser General Public License v3.0: +// https://www.gnu.org/licenses/gpl-3.0.txt +// https://www.gnu.org/licenses/lgpl-3.0.txt +// SPDX-License-Identifier: LGPL-3.0-only +// End of Source File Header + +#include "LegalizeFragmentOutputIndexPass.h" + +#include "spirv.hpp" +#include "source/opt/basic_block.h" +#include "source/opt/build_module.h" +#include "source/opt/constants.h" +#include "source/opt/def_use_manager.h" +#include "source/opt/function.h" +#include "source/opt/instruction.h" +#include "source/opt/ir_builder.h" +#include "source/opt/ir_context.h" +#include "source/opt/loop_descriptor.h" +#include "source/opt/module.h" +#include "source/opt/type_manager.h" +#include "source/util/make_unique.h" + +#include +#include +#include + +namespace MobileGL { + namespace MG_Util { + namespace ShaderTranspiler { + namespace { + using spvtools::MakeUnique; + using spvtools::opt::BasicBlock; + using spvtools::opt::Function; + using spvtools::opt::Instruction; + using spvtools::opt::InstructionBuilder; + using spvtools::opt::IRContext; + using spvtools::opt::Operand; + + // A fragment output array is at most GL_MAX_DRAW_BUFFERS elements (8 on ES + // 3.0, 16 in practice) and each lowered element costs one basic block, so a + // module claiming more than this is refused rather than exploded. + constexpr uint32_t kMaxLoweredArrayLength = 32; + // One CFG-changing rewrite per round (analyses are dropped after each), so + // the round budget bounds the work on a pathological module. + constexpr int kMaxLoweringRounds = 256; + // Full unrolling copies the body once per iteration, and nothing in the stock + // unroller bounds that. A shader whose output index comes from a 4096-trip + // loop would be legalized into a module orders of magnitude larger and slower + // to compile - so past this count the loop is left alone and the switch + // lowering, whose cost is the array length rather than the trip count, takes + // it instead. Real shaders of this shape (Minecraft 26.3's OIT coefficient + // writer included) iterate a handful of times. + constexpr size_t kMaxUnrolledIterations = 64; + + struct DynamicIndexUse { + Instruction* accessChain = nullptr; + uint32_t arrayLength = 0; + }; + + bool HasFragmentEntryPoint(IRContext* context) { + for (const Instruction& entryPoint : context->module()->entry_points()) { + if (static_cast(entryPoint.GetSingleWordInOperand(0)) == + spv::ExecutionModel::Fragment) { + return true; + } + } + return false; + } + + // Every Output-storage variable whose pointee is an array, mapped to that + // array's length. A length that is not a plain OpConstant (a spec constant) + // maps to 0: still detected as illegal ESSL, never lowered. + std::unordered_map CollectOutputArrays(IRContext* context) { + std::unordered_map outputArrays; + auto* defUseMgr = context->get_def_use_mgr(); + auto* constantMgr = context->get_constant_mgr(); + + for (Instruction& inst : context->module()->types_values()) { + if (inst.opcode() != spv::Op::OpVariable || + static_cast(inst.GetSingleWordInOperand(0)) != + spv::StorageClass::Output) { + continue; + } + + Instruction* pointerType = defUseMgr->GetDef(inst.type_id()); + if (pointerType == nullptr || pointerType->opcode() != spv::Op::OpTypePointer) { + continue; + } + Instruction* pointeeType = defUseMgr->GetDef(pointerType->GetSingleWordInOperand(1)); + if (pointeeType == nullptr || pointeeType->opcode() != spv::Op::OpTypeArray) { + continue; + } + + uint32_t arrayLength = 0; + const spvtools::opt::analysis::Constant* lengthConstant = + constantMgr->FindDeclaredConstant(pointeeType->GetSingleWordInOperand(1)); + if (lengthConstant != nullptr && lengthConstant->AsIntConstant() != nullptr) { + arrayLength = lengthConstant->AsIntConstant()->GetU32BitValue(); + } + outputArrays.emplace(inst.result_id(), arrayLength); + } + return outputArrays; + } + + // "Constant integral expression" in the ESSL sense: an OpConstant (or the + // zero an OpConstantNull stands for). A spec constant is deliberately NOT + // one - SPIRV-Cross prints it as an identifier, which is exactly what the + // driver rejects. + bool IsConstantIndex(IRContext* context, uint32_t indexId) { + Instruction* def = context->get_def_use_mgr()->GetDef(indexId); + return def != nullptr && (def->opcode() == spv::Op::OpConstant || + def->opcode() == spv::Op::OpConstantNull); + } + + // Access chains that index a fragment output array with a non-constant. + // Only the FIRST index is considered: it is the one that selects the array + // element, and it is the only one ESSL constrains. Chains rooted at another + // access chain (a component of an element) are indexing inside the element + // and are legal however they are computed. + std::vector CollectDynamicIndexUses(IRContext* context) { + std::vector uses; + if (!HasFragmentEntryPoint(context)) { + return uses; + } + + const std::unordered_map outputArrays = CollectOutputArrays(context); + if (outputArrays.empty()) { + return uses; + } + + for (Function& function : *context->module()) { + for (BasicBlock& block : function) { + for (Instruction& inst : block) { + if (inst.opcode() != spv::Op::OpAccessChain && + inst.opcode() != spv::Op::OpInBoundsAccessChain) { + continue; + } + if (inst.NumInOperands() < 2) { + continue; + } + const auto arrayIt = outputArrays.find(inst.GetSingleWordInOperand(0)); + if (arrayIt == outputArrays.end()) { + continue; + } + if (IsConstantIndex(context, inst.GetSingleWordInOperand(1))) { + continue; + } + uses.push_back({&inst, arrayIt->second}); + } + } + } + return uses; + } + + // The array index operand of |accessChain| replaced by the constant |element|, + // built at the builder's insertion point. Every later index is copied through + // unchanged: `coeff[idx][i]` keeps its (legal) dynamic component index. + Instruction* CloneChainWithConstantIndex(InstructionBuilder& builder, IRContext* context, + Instruction* accessChain, uint32_t constantIndexId) { + std::vector operands; + operands.reserve(accessChain->NumInOperands()); + for (uint32_t i = 0; i < accessChain->NumInOperands(); ++i) { + if (i == 1) { + operands.push_back({SPV_OPERAND_TYPE_ID, {constantIndexId}}); + } else { + operands.push_back(accessChain->GetInOperand(i)); + } + } + return builder.AddInstruction(MakeUnique(context, accessChain->opcode(), + accessChain->type_id(), + context->TakeNextId(), operands)); + } + + // The id of |element| as a constant of the same integer type as |indexId|. + uint32_t ConstantLikeIndex(IRContext* context, uint32_t indexId, uint32_t element) { + Instruction* indexDef = context->get_def_use_mgr()->GetDef(indexId); + const spvtools::opt::analysis::Type* indexType = + context->get_type_mgr()->GetType(indexDef->type_id()); + const spvtools::opt::analysis::Constant* constant = + context->get_constant_mgr()->GetConstant(indexType, {element}); + return context->get_constant_mgr()->GetDefiningInstruction(constant)->result_id(); + } + + // A 32-bit integer is the only index this pass lowers: OpSwitch matches its + // literals against the selector's width, and every ESSL fragment-output index + // is an int or uint. + bool IsLowerableIndexType(IRContext* context, uint32_t indexId) { + Instruction* indexDef = context->get_def_use_mgr()->GetDef(indexId); + if (indexDef == nullptr) { + return false; + } + const spvtools::opt::analysis::Type* type = + context->get_type_mgr()->GetType(indexDef->type_id()); + const spvtools::opt::analysis::Integer* integer = + type != nullptr ? type->AsInteger() : nullptr; + return integer != nullptr && integer->width() == 32; + } + + // The condition type OpSelect needs for |resultTypeId|. Before SPIR-V 1.4 a + // scalar bool may not select between vectors, so a vector result needs a bool + // vector of the same width - built by broadcasting the scalar comparison. + // Anything that is neither scalar nor vector (a matrix or struct element) is + // refused: pre-1.4 OpSelect cannot express it either. + bool TryGetSelectConditionType(IRContext* context, uint32_t resultTypeId, + uint32_t* conditionTypeId, uint32_t* dimension) { + auto* typeMgr = context->get_type_mgr(); + const spvtools::opt::analysis::Type* resultType = typeMgr->GetType(resultTypeId); + if (resultType == nullptr) { + return false; + } + + spvtools::opt::analysis::Bool boolType; + if (resultType->AsVector() != nullptr) { + const uint32_t count = resultType->AsVector()->element_count(); + spvtools::opt::analysis::Vector boolVector(&boolType, count); + *conditionTypeId = typeMgr->GetTypeInstruction(&boolVector); + *dimension = count; + return *conditionTypeId != 0; + } + if (resultType->AsInteger() != nullptr || resultType->AsFloat() != nullptr || + resultType->AsBool() != nullptr) { + *conditionTypeId = typeMgr->GetTypeInstruction(&boolType); + *dimension = 1; + return *conditionTypeId != 0; + } + return false; + } + + // Whether fully unrolling |loop| is bounded work. The trip count is read the + // same way the stock unroller reads it, so a loop this declines to measure is + // one CanPerformUnroll would refuse anyway - the hint would be inert on it, + // and the fallback lowering is what handles it. Requires the induction + // variable to already be an OpPhi, which is why this runs after ssa-rewrite. + bool IsBoundedUnrollCandidate(spvtools::opt::Loop* loop) { + const spvtools::opt::BasicBlock* condition = loop->FindConditionBlock(); + if (condition == nullptr) { + return false; + } + const Instruction* induction = loop->FindConditionVariable(condition); + if (induction == nullptr || induction->opcode() != spv::Op::OpPhi) { + return false; + } + size_t iterations = 0; + if (!loop->FindNumberOfIterations(induction, &*condition->ctail(), &iterations)) { + return false; + } + return iterations <= kMaxUnrolledIterations; + } + } // namespace + + bool LegalizeFragmentOutputIndexPass::BinaryHasDynamicOutputIndexing( + const std::vector& binary) { + if (binary.empty()) { + return false; + } + std::unique_ptr context = spvtools::BuildModule( + SPV_ENV_VULKAN_1_1, + [](spv_message_level_t, const char*, const spv_position_t&, const char*) {}, + binary.data(), binary.size()); + if (!context) { + return false; + } + return !CollectDynamicIndexUses(context.get()).empty(); + } + + spvtools::opt::Pass::Status LegalizeFragmentOutputIndexPass::Process() { + return m_mode == Mode::MarkLoopsForUnroll ? MarkLoopsForUnroll() : LowerToConstantSwitch(); + } + + spvtools::opt::Pass::Status LegalizeFragmentOutputIndexPass::MarkLoopsForUnroll() { + auto* irContext = context(); + const std::vector uses = CollectDynamicIndexUses(irContext); + if (uses.empty()) { + return Status::SuccessWithoutChange; + } + + bool modified = false; + for (const DynamicIndexUse& use : uses) { + BasicBlock* block = irContext->get_instr_block(use.accessChain); + if (block == nullptr) { + continue; + } + Function* function = block->GetParent(); + if (function == nullptr) { + continue; + } + + spvtools::opt::LoopDescriptor* loops = irContext->GetLoopDescriptor(function); + for (spvtools::opt::Loop* loop = (*loops)[block->id()]; loop != nullptr; + loop = loop->GetParent()) { + if (!IsBoundedUnrollCandidate(loop)) { + continue; + } + Instruction* mergeInst = loop->GetHeaderBlock()->GetLoopMergeInst(); + // Only a bare `None` control is promoted, and only when no extra + // literal (PartialCount, PeelCount, ...) follows it: the unroller + // tests the control word for equality with Unroll, so ORing the bit + // into a control that already carries something - DontUnroll above + // all - would neither unroll nor mean what it says. + if (mergeInst == nullptr || mergeInst->NumOperands() != 3 || + mergeInst->GetSingleWordOperand(2) != + static_cast(spv::LoopControlMask::MaskNone)) { + continue; + } + mergeInst->SetOperand( + 2, {static_cast(spv::LoopControlMask::Unroll)}); + modified = true; + } + } + + if (!modified) { + return Status::SuccessWithoutChange; + } + MGLOG_D("[spirv] fragment-output index: marked enclosing loops for full unrolling"); + return Status::SuccessWithChange; + } + + spvtools::opt::Pass::Status LegalizeFragmentOutputIndexPass::LowerToConstantSwitch() { + auto* irContext = context(); + if (!HasFragmentEntryPoint(irContext)) { + return Status::SuccessWithoutChange; + } + + bool modified = false; + // Access chains this pass has already refused, so a shape it cannot rewrite + // exactly cannot spin the round loop. + std::unordered_set declined; + + for (int round = 0; round < kMaxLoweringRounds; ++round) { + const std::vector uses = CollectDynamicIndexUses(irContext); + bool progressed = false; + + for (const DynamicIndexUse& use : uses) { + if (declined.count(use.accessChain->result_id()) != 0) { + continue; + } + const LoweringOutcome outcome = LowerOneChain(use.accessChain, use.arrayLength); + if (outcome == LoweringOutcome::Declined) { + declined.insert(use.accessChain->result_id()); + continue; + } + if (outcome == LoweringOutcome::Changed) { + modified = true; + progressed = true; + // A store rewrite splits the block it sat in; every cached + // analysis (and the instruction list this loop is walking) is + // stale from here on. Recollect from scratch. + break; + } + } + + if (!progressed) { + break; + } + } + + if (!modified) { + return Status::SuccessWithoutChange; + } + return Status::SuccessWithChange; + } + + LegalizeFragmentOutputIndexPass::LoweringOutcome LegalizeFragmentOutputIndexPass::LowerOneChain( + Instruction* accessChain, uint32_t arrayLength) { + auto* irContext = context(); + if (arrayLength == 0 || arrayLength > kMaxLoweredArrayLength) { + MGLOG_D("[spirv] fragment-output index: array length %u is not lowerable", arrayLength); + return LoweringOutcome::Declined; + } + if (!IsLowerableIndexType(irContext, accessChain->GetSingleWordInOperand(1))) { + return LoweringOutcome::Declined; + } + + std::vector stores; + std::vector loads; + bool unsupportedUse = false; + irContext->get_def_use_mgr()->ForEachUser(accessChain, [&](Instruction* user) { + switch (user->opcode()) { + case spv::Op::OpName: + case spv::Op::OpDecorate: + case spv::Op::OpDecorateId: + return; + case spv::Op::OpStore: + // Only as the pointer. A pointer stored as a *value* is not a + // fragment-output write and cannot be redirected element-wise. + if (user->GetSingleWordInOperand(0) == accessChain->result_id()) { + stores.push_back(user); + } else { + unsupportedUse = true; + } + return; + case spv::Op::OpLoad: + // Memory operands (Volatile, Aligned, ...) would be dropped by the + // per-element rebuild, so a load carrying any is refused instead. + if (user->NumInOperands() == 1) { + loads.push_back(user); + } else { + unsupportedUse = true; + } + return; + default: + // A pointer passed to a function, copied, or chained further cannot + // be resolved to one element here. + unsupportedUse = true; + return; + } + }); + + if (unsupportedUse) { + MGLOG_D("[spirv] fragment-output index: chain %%%u has a use this pass cannot rewrite", + accessChain->result_id()); + return LoweringOutcome::Declined; + } + + if (!loads.empty()) { + return LowerLoad(accessChain, arrayLength, loads.front()); + } + if (!stores.empty()) { + return LowerStore(accessChain, arrayLength, stores.front()); + } + + // No uses left: the chain itself is what detection is still seeing. + irContext->KillInst(accessChain); + irContext->InvalidateAnalysesExceptFor(IRContext::kAnalysisNone); + return LoweringOutcome::Changed; + } + + // switch (idx) { case 0: o[0] = v; break; case 1: o[1] = v; break; ... } + // + // The block holding the store is split at the store, and the tail becomes the + // switch's merge block, so whatever followed the store still runs exactly once + // on every path. An index outside [0, length) reaches the default target, which + // is the merge block: nothing is stored, which is what an out-of-range write to + // an output array already meant. + LegalizeFragmentOutputIndexPass::LoweringOutcome LegalizeFragmentOutputIndexPass::LowerStore( + Instruction* accessChain, uint32_t arrayLength, Instruction* store) { + auto* irContext = context(); + BasicBlock* block = irContext->get_instr_block(store); + if (block == nullptr) { + return LoweringOutcome::Declined; + } + // Splitting a loop header keeps the label - and so the back edge's target - + // on the first half while the OpLoopMerge moves to the second, which is not + // a loop any more. Refuse instead of producing that. + if (block->GetLoopMergeInst() != nullptr) { + MGLOG_D("[spirv] fragment-output index: store sits in a loop header, declining"); + return LoweringOutcome::Declined; + } + Function* function = block->GetParent(); + if (function == nullptr) { + return LoweringOutcome::Declined; + } + + const uint32_t indexId = accessChain->GetSingleWordInOperand(1); + const uint32_t valueId = store->GetSingleWordInOperand(1); + std::vector memoryOperands; + for (uint32_t i = 2; i < store->NumInOperands(); ++i) { + memoryOperands.push_back(store->GetInOperand(i)); + } + + const uint32_t mergeLabelId = irContext->TakeNextId(); + block->SplitBasicBlock(irContext, mergeLabelId, BasicBlock::iterator(store)); + // |store| now heads the merge block; the per-element stores replace it. + irContext->KillInst(store); + + std::vector> targets; + targets.reserve(arrayLength); + BasicBlock* insertAfter = block; + for (uint32_t element = 0; element < arrayLength; ++element) { + const uint32_t caseLabelId = irContext->TakeNextId(); + auto caseBlock = MakeUnique(MakeUnique( + irContext, spv::Op::OpLabel, 0, caseLabelId, std::initializer_list{})); + caseBlock->SetParent(function); + BasicBlock* casePtr = function->InsertBasicBlockAfter(std::move(caseBlock), insertAfter); + // The builders below register what they add, but this label was built by + // hand: without this the OpSwitch would name a target the def-use manager + // has never seen, which a consistency-checking build calls out. + irContext->AnalyzeDefUse(casePtr->GetLabelInst()); + irContext->set_instr_block(casePtr->GetLabelInst(), casePtr); + + InstructionBuilder caseBuilder( + irContext, casePtr, + IRContext::kAnalysisDefUse | IRContext::kAnalysisInstrToBlockMapping); + const uint32_t constantId = ConstantLikeIndex(irContext, indexId, element); + Instruction* elementChain = + CloneChainWithConstantIndex(caseBuilder, irContext, accessChain, constantId); + + std::vector storeOperands; + storeOperands.push_back({SPV_OPERAND_TYPE_ID, {elementChain->result_id()}}); + storeOperands.push_back({SPV_OPERAND_TYPE_ID, {valueId}}); + for (const Operand& memoryOperand : memoryOperands) { + storeOperands.push_back(memoryOperand); + } + caseBuilder.AddInstruction( + MakeUnique(irContext, spv::Op::OpStore, 0, 0, storeOperands)); + caseBuilder.AddBranch(mergeLabelId); + + targets.push_back({Operand::OperandData{element}, caseLabelId}); + insertAfter = casePtr; + } + + InstructionBuilder switchBuilder( + irContext, block, IRContext::kAnalysisDefUse | IRContext::kAnalysisInstrToBlockMapping); + switchBuilder.AddSwitch(indexId, mergeLabelId, targets, mergeLabelId); + + if (irContext->get_def_use_mgr()->NumUsers(accessChain) == 0) { + irContext->KillInst(accessChain); + } + irContext->InvalidateAnalysesExceptFor(IRContext::kAnalysisNone); + MGLOG_D("[spirv] fragment-output index: lowered a dynamic write to a %u-way switch", + arrayLength); + return LoweringOutcome::Changed; + } + + // A read needs no control flow: load every element through a constant index and + // pick with OpSelect. Reading an output array is rare, but it is legal SPIR-V and + // legal ESSL, and the elements this adds reads of were already readable here. + LegalizeFragmentOutputIndexPass::LoweringOutcome LegalizeFragmentOutputIndexPass::LowerLoad( + Instruction* accessChain, uint32_t arrayLength, Instruction* load) { + auto* irContext = context(); + uint32_t conditionTypeId = 0; + uint32_t dimension = 0; + if (!TryGetSelectConditionType(irContext, load->type_id(), &conditionTypeId, &dimension)) { + MGLOG_D("[spirv] fragment-output index: element type is not selectable, declining"); + return LoweringOutcome::Declined; + } + const uint32_t boolTypeId = irContext->get_type_mgr()->GetBoolTypeId(); + const uint32_t indexId = accessChain->GetSingleWordInOperand(1); + + InstructionBuilder builder( + irContext, load, IRContext::kAnalysisDefUse | IRContext::kAnalysisInstrToBlockMapping); + + uint32_t selectedId = 0; + for (uint32_t element = 0; element < arrayLength; ++element) { + const uint32_t constantId = ConstantLikeIndex(irContext, indexId, element); + Instruction* elementChain = + CloneChainWithConstantIndex(builder, irContext, accessChain, constantId); + Instruction* elementLoad = builder.AddLoad(load->type_id(), elementChain->result_id()); + if (element == 0) { + // Element 0 is the else-arm of the whole ladder, so an out-of-range + // index reads it - an undefined element for an undefined index. + selectedId = elementLoad->result_id(); + continue; + } + + Instruction* isElement = + builder.AddBinaryOp(boolTypeId, spv::Op::OpIEqual, indexId, constantId); + uint32_t conditionId = isElement->result_id(); + if (dimension > 1) { + std::vector components(dimension, conditionId); + conditionId = builder.AddCompositeConstruct(conditionTypeId, components)->result_id(); + } + selectedId = builder + .AddSelect(load->type_id(), conditionId, elementLoad->result_id(), + selectedId) + ->result_id(); + } + + irContext->ReplaceAllUsesWith(load->result_id(), selectedId); + irContext->KillInst(load); + irContext->InvalidateAnalysesExceptFor(IRContext::kAnalysisNone); + MGLOG_D("[spirv] fragment-output index: lowered a dynamic read to %u constant-indexed loads", + arrayLength); + return LoweringOutcome::Changed; + } + + spvtools::Optimizer::PassToken LegalizeFragmentOutputIndexPass::CreateMarkLoopsForUnrollPass() { + return spvtools::Optimizer::PassToken( + MakeUnique(Mode::MarkLoopsForUnroll)); + } + + spvtools::Optimizer::PassToken LegalizeFragmentOutputIndexPass::CreateLowerToConstantSwitchPass() { + return spvtools::Optimizer::PassToken( + MakeUnique(Mode::LowerToConstantSwitch)); + } + } // namespace ShaderTranspiler + } // namespace MG_Util +} // namespace MobileGL diff --git a/MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/LegalizeFragmentOutputIndexPass.h b/MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/LegalizeFragmentOutputIndexPass.h new file mode 100644 index 00000000..3c017e3d --- /dev/null +++ b/MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/LegalizeFragmentOutputIndexPass.h @@ -0,0 +1,111 @@ +// MobileGL - MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/LegalizeFragmentOutputIndexPass.h +// Copyright (c) 2025-2026 MobileGL-Dev +// Licensed under the GNU Lesser General Public License v3.0: +// https://www.gnu.org/licenses/gpl-3.0.txt +// https://www.gnu.org/licenses/lgpl-3.0.txt +// SPDX-License-Identifier: LGPL-3.0-only +// End of Source File Header + +#pragma once +#include "source/opt/pass.h" +#include "spirv-tools/optimizer.hpp" + +#include +#include + +namespace MobileGL { + namespace MG_Util { + namespace ShaderTranspiler { + // GLSL ES requires a *constant integral expression* to index a fragment output + // array (GLSL ES 3.00 4.3.6 / 3.20 4.4.2); SPIR-V has no such rule, so a shader + // that writes `coeff[i]` from a loop reaches SPIRV-Cross intact and comes out as + // ESSL a strict driver rejects outright: + // + // '[' : array indexes for fragment outputs must be constant integral expressions + // + // The program then links nothing and every draw that uses it is a silent no-op. + // Mesa accepts the same source, which is why this only ever showed on the ANGLE + // lane (see tools/trace_replay/README.md, improved-transparency-minecraft-26.3: + // the whole translucent layer disappears because the OIT coefficient shader is + // exactly this shape). + // + // Two modes, used as two halves of one legalization in + // ShaderCompiler::LegalizeFragmentOutputIndexingForEssl: + // + // MarkLoopsForUnroll - the companion the stock unroller needs. spirv-opt's + // CreateLoopUnrollPass only touches loops whose OpLoopMerge carries the + // Unroll loop control (LoopUtils::HasUnrollLoopControl), which glslang emits + // only for an explicit [[unroll]]. This mode sets that hint on the loops that + // actually enclose an offending access chain - and only those, so an + // unrelated long loop elsewhere in the same shader is never unrolled - and + // only when their trip count is known and small, so legalizing a shader can + // never explode it. With the hint set, the stock chain (ssa-rewrite, + // loop-unroll, ccp, simplification, dead-branch-elim) folds a loop-derived + // index to a literal, which is what the real-world shaders (the OIT one + // included) need. Must run AFTER ssa-rewrite: both the trip-count check and + // the unroller itself need the induction variable as an OpPhi. + // + // LowerToConstantSwitch - the fallback for an index that is *genuinely* + // dynamic (uniform-derived, a non-constant trip count, vertex data). It + // rewrites each write through such an access chain into an OpSwitch over the + // array's range with one constant-indexed store per case - the SPIR-V of + // `switch (i) { case 0: o[0] = v; break; case 1: o[1] = v; break; }` - and + // each read into per-element constant-indexed loads combined with OpSelect. + // An out-of-range index stores nothing, which is what indexing an output + // array out of range already meant. + // + // Fragment stage only: every other stage may index an output array dynamically + // in ESSL, and on DirectVulkan the original SPIR-V is legal as-is. The pass + // declines (leaving the module untouched) rather than half-transforming whenever + // it meets a shape it cannot rewrite exactly - a pointer handed to a function, a + // spec-constant array length, an index type that is not a 32-bit integer, or a + // store sitting in a loop header block, where splitting would move the + // OpLoopMerge away from the back edge's target. + class LegalizeFragmentOutputIndexPass final : public spvtools::opt::Pass { + public: + enum class Mode { + MarkLoopsForUnroll, + LowerToConstantSwitch, + }; + + explicit LegalizeFragmentOutputIndexPass(Mode mode) : m_mode(mode) {} + + const char* name() const override { + return m_mode == Mode::MarkLoopsForUnroll ? "mobilegl-mark-fragment-output-index-loops" + : "mobilegl-lower-fragment-output-index"; + } + + Status Process() override; + + static spvtools::Optimizer::PassToken CreateMarkLoopsForUnrollPass(); + static spvtools::Optimizer::PassToken CreateLowerToConstantSwitchPass(); + + // The detection half, on a serialized module: true when a fragment entry + // point indexes an Output-storage array with anything but an OpConstant. + // Cheap enough to gate the whole legalization on (one BuildModule, no + // serialization) and used again after the folding chain to decide whether + // the fallback has to run at all. + static bool BinaryHasDynamicOutputIndexing(const std::vector& binary); + + private: + enum class LoweringOutcome { + // The shape is not one this pass can rewrite exactly; the module keeps + // the illegal chain rather than a half-transform of it. + Declined, + Changed, + }; + + Status MarkLoopsForUnroll(); + Status LowerToConstantSwitch(); + + LoweringOutcome LowerOneChain(spvtools::opt::Instruction* accessChain, uint32_t arrayLength); + LoweringOutcome LowerStore(spvtools::opt::Instruction* accessChain, uint32_t arrayLength, + spvtools::opt::Instruction* store); + LoweringOutcome LowerLoad(spvtools::opt::Instruction* accessChain, uint32_t arrayLength, + spvtools::opt::Instruction* load); + + Mode m_mode; + }; + } // namespace ShaderTranspiler + } // namespace MG_Util +} // namespace MobileGL From 4d1d4f6225f03ae1535fef7479ab8a4cd29a5b62 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Tue, 11 Aug 2026 23:28:43 -0400 Subject: [PATCH 2/3] [Fix] (DirectGLES): report a rejected shader and a failed link at MGLOG_I - MGLOG_E is compiled out of every INFO build, which is why the retrace artifact could not say which shader the driver refused --- MobileGL/MG_Backend/DirectGLES/Managers.cpp | 36 +++++++++++++++++++-- 1 file changed, 33 insertions(+), 3 deletions(-) diff --git a/MobileGL/MG_Backend/DirectGLES/Managers.cpp b/MobileGL/MG_Backend/DirectGLES/Managers.cpp index 2daf716d..08eb95ac 100644 --- a/MobileGL/MG_Backend/DirectGLES/Managers.cpp +++ b/MobileGL/MG_Backend/DirectGLES/Managers.cpp @@ -4346,6 +4346,22 @@ namespace MobileGL::MG_Backend::DirectGLES { effectiveSpirv = &rectLoweredSpirv; } + // GLSL ES demands a constant integral expression to index a fragment output + // array; SPIR-V does not, so a shader that writes coeff[i] from a loop + // reaches SPIRV-Cross intact and comes out as ESSL a strict driver rejects + // outright ("array indexes for fragment outputs must be constant integral + // expressions"), linking no program and silently no-oping every draw that + // uses it. Mesa accepts it, ANGLE does not - which is the whole of the + // improved-transparency-minecraft-26.3 failure. Fold or lower the index here, + // on the ESSL path only: the same module is legal for DirectVulkan. + Vector outputIndexSpirv; + if (glShaderType == GL_FRAGMENT_SHADER && + MG_Util::ShaderTranspiler::ShaderCompiler::LegalizeFragmentOutputIndexingForEssl( + *effectiveSpirv, outputIndexSpirv) && + !outputIndexSpirv.empty()) { + effectiveSpirv = &outputIndexSpirv; + } + MG_Util::ShaderTranspiler::SpvcSession spvcSession(*effectiveSpirv, MG_Util::ShaderTranspiler::SessionUsageBit::Transpile); @@ -4435,7 +4451,18 @@ namespace MobileGL::MG_Backend::DirectGLES { Vector log(static_cast(logLength) + 1, '\0'); g_GLESFuncs.glGetShaderInfoLog(backendShaderId, logLength, nullptr, log.data()); log.back() = '\0'; - MGLOG_E("Shader compilation failed for backend ID %u: %s", backendShaderId, log.data()); + // MGLOG_I, deliberately. Every CI, retrace and release build compiles at + // MOBILEGL_LOG_LEVEL_INFO, where MGLOG_E and MGLOG_W expand to nothing + // (Log.h orders DEBUG < WARN < ERROR < INFO), so this diagnostic used to + // exist only in debug builds: the Android retrace artifact carried 294 + // INFO lines and zero ERROR lines while two generated shaders were being + // rejected outright, and the lane could not say why it was rendering an + // empty translucent layer. A shader the driver refuses is never noise. + MGLOG_I("Shader compilation failed. State program ID: %u, stage: %s, backend shader ID: " + "%u, driver log: %s", + stateProgramObject->GetExternalIndex(), + MG_Util::ConvertGLEnumToString(glShaderType).c_str(), backendShaderId, + log.data()); m_backendProgramUsable = false; // Nothing will ever attach this one, so nothing else can free it. g_GLESFuncs.glDeleteShader(backendShaderId); @@ -4497,8 +4524,11 @@ namespace MobileGL::MG_Backend::DirectGLES { Vector log(static_cast(logLength) + 1, '\0'); g_GLESFuncs.glGetProgramInfoLog(m_backendProgramId, logLength, nullptr, log.data()); log.back() = '\0'; - MGLOG_E("Program %u linking failed for %u: %s", stateProgramObject->GetExternalIndex(), - m_backendProgramId, log.data()); + // MGLOG_I for the same reason as the compile failure above: a program that + // links nothing no-ops every draw that uses it, and that has to be readable + // in an INFO-level artifact. + MGLOG_I("Program linking failed. State program ID: %u, backend program ID: %u, driver log: %s", + stateProgramObject->GetExternalIndex(), m_backendProgramId, log.data()); } else { MGLOG_D("Program linked successfully. ID: %u", m_backendProgramId); } From faa7b17da374de9e68f406a30629874767372e72 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Wed, 12 Aug 2026 00:03:03 -0400 Subject: [PATCH 3/3] [Test] (tools/trace_replay): drop the minecraft-26.2-in-world fixture --- tools/trace_replay/README.md | 2 -- .../fixtures/minecraft-26.2-in-world.0000519370.png | 3 --- tools/trace_replay/fixtures/minecraft-26.2-in-world.tgz | 3 --- tools/trace_replay/trace_cases.json | 7 ------- 4 files changed, 15 deletions(-) delete mode 100644 tools/trace_replay/fixtures/minecraft-26.2-in-world.0000519370.png delete mode 100644 tools/trace_replay/fixtures/minecraft-26.2-in-world.tgz diff --git a/tools/trace_replay/README.md b/tools/trace_replay/README.md index 3c5f2550..9441f207 100644 --- a/tools/trace_replay/README.md +++ b/tools/trace_replay/README.md @@ -20,8 +20,6 @@ The bundled fixtures cover: ![Minecraft 1.21.4 Fabric Sodium in-world golden](fixtures/minecraft-1.21.4-fabric-sodium-in-world.0000923340.png) - minecraft-26.2-main-menu: captured from Minecraft 26.2's main menu. ![Minecraft 26.2 main menu golden](fixtures/minecraft-26.2-main-menu.0000101926.png) -- minecraft-26.2-in-world: captured from Minecraft 26.2 after entering a normal singleplayer world. - ![Minecraft 26.2 in-world golden](fixtures/minecraft-26.2-in-world.0000519370.png) - improved-transparency-minecraft-26.3: captured from the Minecraft 26.3 improved-transparency scene. ![Minecraft 26.3 improved-transparency golden](fixtures/improved-transparency-minecraft-26.3.0002667619.png) - minecraft-1.21.4-fabric-common-mods-in-world: captured from Minecraft 1.21.4 Fabric with Sodium, Iris, REI, diff --git a/tools/trace_replay/fixtures/minecraft-26.2-in-world.0000519370.png b/tools/trace_replay/fixtures/minecraft-26.2-in-world.0000519370.png deleted file mode 100644 index b3201e02..00000000 --- a/tools/trace_replay/fixtures/minecraft-26.2-in-world.0000519370.png +++ /dev/null @@ -1,3 +0,0 @@ -version https://git-lfs.github.com/spec/v1 -oid sha256:591f1672c81ad364451b33aafe240ade2f992f6228dcb92d4b4466f564dd713d -size 101648 diff --git a/tools/trace_replay/fixtures/minecraft-26.2-in-world.tgz b/tools/trace_replay/fixtures/minecraft-26.2-in-world.tgz deleted file mode 100644 index 21c7d4c7..00000000 --- a/tools/trace_replay/fixtures/minecraft-26.2-in-world.tgz +++ /dev/null @@ -1,3 +0,0 @@ -version https://git-lfs.github.com/spec/v1 -oid sha256:54a155a5e8fe51143cf936b84fb47e054d4e177f8ca19a8c1347f99901a41360 -size 13244500 diff --git a/tools/trace_replay/trace_cases.json b/tools/trace_replay/trace_cases.json index 35ef03e0..eeab5dc5 100644 --- a/tools/trace_replay/trace_cases.json +++ b/tools/trace_replay/trace_cases.json @@ -68,13 +68,6 @@ "target_call": 101926, "timeout_seconds": 180 }, - { - "name": "minecraft-26.2-in-world", - "ci": false, - "trace_archive": "minecraft-26.2-in-world.tgz", - "golden": "minecraft-26.2-in-world.0000519370.png", - "target_call": 519370 - }, { "name": "minecraft-1.21.4-fabric-common-mods-in-world", "trace_archive": "minecraft-1.21.4-fabric-common-mods-in-world.tgz",