From c3412ca394bac1152c75ea7a2286ad2ca4c0713f Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Mon, 10 Aug 2026 09:27:01 -0400 Subject: [PATCH] [Fix, Test] (MG_Util): give inactive vertex inputs a Location - undecorated inputs are invalid SPIR-V and Adreno rejects the whole pipeline --- MobileGL/MG_Test/Program/ProgramUtilTest.cpp | 110 ++++++++++++++++++ .../glslang/TMglGlslIoResolver.cpp | 42 ++++++- .../glslang/TMglGlslIoResolver.h | 8 ++ 3 files changed, 158 insertions(+), 2 deletions(-) diff --git a/MobileGL/MG_Test/Program/ProgramUtilTest.cpp b/MobileGL/MG_Test/Program/ProgramUtilTest.cpp index 98c09e94..edac6f91 100644 --- a/MobileGL/MG_Test/Program/ProgramUtilTest.cpp +++ b/MobileGL/MG_Test/Program/ProgramUtilTest.cpp @@ -9,8 +9,11 @@ #include #include +#include +#include #include #include +#include #include "Includes.h" #include "Init.h" @@ -2649,3 +2652,110 @@ TEST_F(ProgramUtilTest, ShaderPreprocessCacheHonorsByteBudget) { EXPECT_EQ(cache.GetEntryCount(), before); EXPECT_EQ(cache.Find(ShaderStage::Vertex, ShaderPreprocessCache::HashSource(oversized), oversized, kEnvA), nullptr); } + +// Every vertex input that reaches SPIR-V must carry a Location decoration - including the +// declarations glslang's io-mapper considers INACTIVE. +// +// The shape is Iris's: seven attributes, only some of them bound through +// glBindAttribLocation (ProgramAttrib::explicitVertexInLocations), and at least one neither +// bound nor referenced. GL says only active inputs get generic attribute locations, so the +// resolver deliberately does not RESERVE a slot for a dead one - but it must still RESOLVE a +// location for it, because glslang emits an OpVariable for every declared global (the entry +// point's interface comes from the linker objects) and SPIR-V requires every non-built-in +// Input to be decorated (VUID-StandaloneSpirv-Location-04916). +// +// This test drives the FRONTEND rather than the GL entry points on purpose: it checks the RAW +// GlslangToSpv output, before SanitizeAndOptimizeBinary. A GL-level test cannot see the defect +// for an unreferenced attribute, because AggressiveDCE deletes the offending variable on its +// way to the backend - and yet the real victim (Iris' mc_midTexCoord, Adreno 830, +// programHash 0x4a7e9a37fb49caa1) survived DCE and killed the pipeline with VK_ERROR_UNKNOWN. +TEST_F(ProgramUtilTest, PartiallyBoundVertexInputsAllReceiveALocation) { + using namespace MG_Util::ShaderTranspiler; + + const String vertexSource = R"(#version 460 core +in vec3 a_Position; +in vec4 a_Color; +in vec2 a_TexCoord; +in vec2 mc_midTexCoord; +in vec4 mc_Entity; +in vec3 iris_Normal; +in vec4 a_Unreferenced; +out vec4 v_Color; +void main() { + v_Color = a_Color + vec4(a_TexCoord, 0.0, 0.0) + vec4(mc_midTexCoord, 0.0, 0.0) + mc_Entity + + vec4(iris_Normal, 0.0); + gl_Position = vec4(a_Position, 1.0); +} +)"; + + ShaderAttrib shaderAttrib{.shaderType = GL_VERTEX_SHADER, .sourceStr = vertexSource}; + auto shaderResult = ShaderCompiler::CompileShader(shaderAttrib); + ASSERT_TRUE(shaderResult) << shaderResult.error().log; + + // PARTIALLY bound, and deliberately not a dense 0..N run - exactly what Iris does. + // mc_midTexCoord and a_Unreferenced are left unbound (FastSTL's map has no + // initializer-list constructor, hence the explicit inserts). + UnorderedMap explicitVertexIns; + explicitVertexIns["a_Position"] = 0; + explicitVertexIns["a_Color"] = 1; + explicitVertexIns["a_TexCoord"] = 2; + explicitVertexIns["iris_Normal"] = 10; + explicitVertexIns["mc_Entity"] = 11; + ProgramAttrib programAttrib{.shaders = {shaderResult.value()}, + .explicitVertexInLocations = explicitVertexIns}; + auto programResult = ShaderCompiler::LinkProgram(programAttrib); + ASSERT_TRUE(programResult) << programResult.error().log; + + ProgramBinaryAttrib binaryAttrib{.shaderTypes = {GL_VERTEX_SHADER}, .program = *programResult.value()}; + auto binaryResult = ShaderCompiler::GetSpirvBinaryFromProgram(binaryAttrib); + ASSERT_TRUE(binaryResult) << binaryResult.error().log; + ASSERT_EQ(binaryResult->size(), 1u); + const auto& vertexBinary = binaryResult->front(); + + // The authoritative check - this is the same validator whose VUID the driver enforces. + spvtools::SpirvTools tools(SPV_ENV_VULKAN_1_1); + String validatorMessages; + tools.SetMessageConsumer([&validatorMessages](spv_message_level_t, const char*, const spv_position_t&, + const char* message) { + if (message != nullptr) validatorMessages += String(message) + "\n"; + }); + EXPECT_TRUE(tools.Validate(vertexBinary)) + << "the raw vertex module is not valid SPIR-V; Adreno rejects the whole pipeline for this " + << "while lavapipe tolerates it:\n" + << validatorMessages; + + // ...and, independently of the validator, every non-built-in Input carries a UNIQUE location. + constexpr unsigned kOpDecorate = 71, kOpVariable = 59; + constexpr unsigned kDecorationBuiltIn = 11, kDecorationLocation = 30; + constexpr unsigned kStorageClassInput = 1; + std::map locationById; + std::set builtInIds; + std::vector inputIds; + for (SizeT i = 5; i < vertexBinary.size();) { // 5-word header + const unsigned wordCount = vertexBinary[i] >> 16; + const unsigned opcode = vertexBinary[i] & 0xFFFFu; + ASSERT_GT(wordCount, 0u) << "malformed SPIR-V instruction stream"; + if (i + wordCount > vertexBinary.size()) break; + if (opcode == kOpDecorate && wordCount >= 4 && vertexBinary[i + 2] == kDecorationLocation) { + locationById[vertexBinary[i + 1]] = vertexBinary[i + 3]; + } else if (opcode == kOpDecorate && wordCount >= 3 && vertexBinary[i + 2] == kDecorationBuiltIn) { + builtInIds.insert(vertexBinary[i + 1]); + } else if (opcode == kOpVariable && wordCount >= 4 && vertexBinary[i + 3] == kStorageClassInput) { + inputIds.push_back(vertexBinary[i + 2]); + } + i += wordCount; + } + + std::set usedLocations; + SizeT checked = 0; + for (const unsigned id : inputIds) { + if (builtInIds.count(id) != 0) continue; + const auto it = locationById.find(id); + ASSERT_NE(it, locationById.end()) + << "vertex input id " << id << " reached SPIR-V with no Location decoration"; + EXPECT_TRUE(usedLocations.insert(it->second).second) + << "two vertex inputs were assigned location " << it->second; + ++checked; + } + EXPECT_GE(checked, 7u) << "expected all seven declared inputs to be present in the raw module"; +} diff --git a/MobileGL/MG_Util/ShaderTranspiler/glslang/TMglGlslIoResolver.cpp b/MobileGL/MG_Util/ShaderTranspiler/glslang/TMglGlslIoResolver.cpp index f78e207a..4aef3a06 100644 --- a/MobileGL/MG_Util/ShaderTranspiler/glslang/TMglGlslIoResolver.cpp +++ b/MobileGL/MG_Util/ShaderTranspiler/glslang/TMglGlslIoResolver.cpp @@ -105,8 +105,46 @@ namespace MobileGL { } int TMglGlslIoResolver::resolveInOutLocation(EShLanguage stage, glslang::TVarEntryInfo& ent) { - if (!ent.live && stage == EShLangVertex && ent.symbol->getType().getQualifier().isPipeInput()) { - return ent.newLocation = -1; + // NO dead-vertex-input early-out here, deliberately - the skip belongs in + // reserverStorageSlot() and ONLY there. + // + // Skipping RESERVATION is the GL semantic: only active inputs get generic attribute + // locations, so a dead declaration must not consume a slot an active input should + // have. Skipping RESOLUTION as well used to look like the same statement, but it is a + // different one: it leaves the variable with no layoutLocation, and glslang still + // EMITS it - a declared input is in the shader's linker objects and therefore in the + // entry point's interface. The result is an OpVariable of storage class Input with no + // Location decoration, which SPIR-V forbids + // (VUID-StandaloneSpirv-Location-04916). lavapipe tolerates it; Adreno rejects the + // whole pipeline with VK_ERROR_UNKNOWN, which is how this shipped undetected - every + // desktop gate, retrace corpus included, is blind to it. + // + // Found 2026-08-11 on an Adreno 830: the Iris weather program (mc_midTexCoord among + // seven attributes, only some of them glBindAttribLocation-bound) died at the first + // rainy-world draw, 100% reproducible, programHash 0x4a7e9a37fb49caa1. + // + // They cannot simply be handed to the base resolver either. Auto-assignment for inputs + // WITHOUT an explicit binding happens entirely in the resolve pass, in sort order, so a + // dead declaration reaching the free-slot search first would take location 0 and push + // the active input up - which is precisely the GL violation the reservation skip + // exists to prevent (ProgramTest.InactiveExplicitVertexBindingsDoNotReserveLocations + // pins it: Iris injects Position/UV0 into packs that actually read vaPosition). + // + // So dead inputs get their locations from the TOP of the attribute range downward, + // while the base resolver hands active ones out from 0 upward. Both properties hold at + // once: every emitted input carries a Location, and no active input is displaced. The + // two allocators can only meet if live + dead exceed the attribute limit, which is an + // over-subscribed program GL would reject anyway; if that happens we leave the + // variable to the base resolver rather than hand out a colliding location. + const glslang::TType& type = ent.symbol->getType(); + if (!ent.live && stage == EShLangVertex && type.getQualifier().isPipeInput() && + !type.getQualifier().hasLocation() && !type.isBuiltIn()) { + const int size = std::max(1, glslang::TIntermediate::computeTypeLocationSize(type, stage)); + if (m_nextInactiveVertexInLocation - (size - 1) >= 0) { + m_nextInactiveVertexInLocation -= (size - 1); + ent.symbol->getWritableType().getQualifier().layoutLocation = m_nextInactiveVertexInLocation; + --m_nextInactiveVertexInLocation; + } } return TDefaultGlslIoResolver::resolveInOutLocation(stage, ent); } diff --git a/MobileGL/MG_Util/ShaderTranspiler/glslang/TMglGlslIoResolver.h b/MobileGL/MG_Util/ShaderTranspiler/glslang/TMglGlslIoResolver.h index 86e9068a..d1c22683 100644 --- a/MobileGL/MG_Util/ShaderTranspiler/glslang/TMglGlslIoResolver.h +++ b/MobileGL/MG_Util/ShaderTranspiler/glslang/TMglGlslIoResolver.h @@ -51,5 +51,13 @@ namespace MobileGL { std::map m_plainUniformLocationSizeByName; std::map m_plainUniformLocationByName; bool m_plainUniformLocationsAssigned = false; + // Descending allocator for INACTIVE vertex inputs (see resolveInOutLocation): they + // still have to carry a Location because glslang emits them, but they must not take a + // slot an active input would get. 15, not 31: the location survives into the ESSL + // SPIRV-Cross emits for DirectGLES, and GL/ES only guarantee GL_MAX_VERTEX_ATTRIBS + // >= 16 - a location of 31 makes the generated shader fail to compile on a real ES + // driver (caught by the super-duper-vanilla and chocapic retrace fixtures). + static constexpr int kInactiveVertexInLocationTop = 15; + int m_nextInactiveVertexInLocation = kInactiveVertexInLocationTop; }; } // namespace MobileGL