diff --git a/MobileGL/MG_Backend/DirectGLES/Managers.cpp b/MobileGL/MG_Backend/DirectGLES/Managers.cpp index 42bd91d0..894a0103 100644 --- a/MobileGL/MG_Backend/DirectGLES/Managers.cpp +++ b/MobileGL/MG_Backend/DirectGLES/Managers.cpp @@ -6488,35 +6488,6 @@ namespace MobileGL::MG_Backend::DirectGLES { } } - // The second half of the inter-stage interface-block repair, and the one that - // actually closes the 420pack group: this driver drops the payload of a block that - // carries an explicit layout(location=) whenever a tessellation or geometry stage - // is in the pipeline, so the qualifier comes off and ES matches the block by name - // and member sequence instead. The names those two sides agree on are the ones the - // rename above just fixed, which is why this runs AFTER it and not before. - // - // The caller arms the two directions; both are false unless the driver POST - // measured the defect AND this program has a stage that can hit it. Adopted only - // when this stage really had a located block, for the reason the array-input split - // documents: the optimizer hands back a re-serialised copy either way. - Vector strippedIoBlockLocationSpirv; - if (stripInputBlockLocations || stripOutputBlockLocations) { - Bool strippedAny = false; - if (MG_Util::ShaderTranspiler::ShaderCompiler::StripIoBlockLocationsForEssl( - *effectiveSpirv, stripInputBlockLocations, stripOutputBlockLocations, - strippedAny, strippedIoBlockLocationSpirv, enableSpirvValidation) && - !strippedIoBlockLocationSpirv.empty() && strippedAny) { - effectiveSpirv = &strippedIoBlockLocationSpirv; - MGLOG_D("Program %u stage %s: interface-block location qualifiers dropped " - "(%s), because this driver loses a located block's payload across a " - "tessellation or geometry boundary.", - m_backendProgramId, MG_Util::ConvertGLEnumToString(glShaderType).c_str(), - stripInputBlockLocations - ? (stripOutputBlockLocations ? "consumed and produced" : "consumed") - : "produced"); - } - } - // ESSL stage-matches uniform blocks by member precision, but SPIRV-Cross prints // a RelaxedPrecision member as explicit "mediump" in the vertex stage and as // UNQUALIFIED (mediump-by-default) in the fragment stage; after @@ -6722,6 +6693,45 @@ namespace MobileGL::MG_Backend::DirectGLES { effectiveSpirv = &atomicCounterSpirv; } + // The second half of the inter-stage interface-block repair, and the one that + // actually closes the 420pack group: this driver drops the payload of a block that + // carries an explicit layout(location=) whenever a tessellation or geometry stage + // is in the pipeline, so the qualifier comes off and ES matches the block by name + // and member sequence instead. The names those two sides agree on are the ones the + // rename above just fixed, which is why this runs AFTER it and not before. + // + // The caller arms the two directions; both are false unless the driver POST + // measured the defect AND this program has a stage that can hit it. Adopted only + // when this stage really had a located block, for the reason the array-input split + // documents: the optimizer hands back a re-serialised copy either way. + // + // LAST IN THE CHAIN, and that position is load-bearing. Vulkan SPIR-V REQUIRES a + // Location on every user-defined Input/Output variable + // ([VUID-StandaloneSpirv-Location-04915]), so the module this produces is + // deliberately no longer valid Vulkan SPIR-V - it is an ESSL-emission intermediate + // that goes straight into SPIRV-Cross and reaches no driver as SPIR-V. Running it + // here means no later pass validates what it produced; the pass itself skips + // validation for the same reason (see StripIoBlockLocationsForEssl). Anywhere + // earlier and every remaining pass would latch a validation failure on a module + // that is doing exactly what it was asked to. + Vector strippedIoBlockLocationSpirv; + if (stripInputBlockLocations || stripOutputBlockLocations) { + Bool strippedAny = false; + if (MG_Util::ShaderTranspiler::ShaderCompiler::StripIoBlockLocationsForEssl( + *effectiveSpirv, stripInputBlockLocations, stripOutputBlockLocations, + strippedAny, strippedIoBlockLocationSpirv, enableSpirvValidation) && + !strippedIoBlockLocationSpirv.empty() && strippedAny) { + effectiveSpirv = &strippedIoBlockLocationSpirv; + MGLOG_D("Program %u stage %s: interface-block location qualifiers dropped " + "(%s), because this driver loses a located block's payload across a " + "tessellation or geometry boundary.", + m_backendProgramId, MG_Util::ConvertGLEnumToString(glShaderType).c_str(), + stripInputBlockLocations + ? (stripOutputBlockLocations ? "consumed and produced" : "consumed") + : "produced"); + } + } + MG_Util::ShaderTranspiler::SpvcSession spvcSession(*effectiveSpirv, MG_Util::ShaderTranspiler::SessionUsageBit::Transpile); diff --git a/MobileGL/MG_Test/ShaderTranspiler/StripIoBlockLocationsTest.cpp b/MobileGL/MG_Test/ShaderTranspiler/StripIoBlockLocationsTest.cpp index b9353cc0..3bae499b 100644 --- a/MobileGL/MG_Test/ShaderTranspiler/StripIoBlockLocationsTest.cpp +++ b/MobileGL/MG_Test/ShaderTranspiler/StripIoBlockLocationsTest.cpp @@ -17,8 +17,6 @@ #include #include -#include - using namespace MobileGL; using MobileGL::MG_Util::ShaderTranspiler::SessionUsageBit; using MobileGL::MG_Util::ShaderTranspiler::ShaderCompiler; @@ -100,19 +98,17 @@ void main() )"; } // namespace +// NOTE ON spirv-val, because its absence here is deliberate and every sibling pass test +// asserts the opposite. Vulkan SPIR-V REQUIRES a Location decoration on every user-defined +// Input/Output variable ([VUID-StandaloneSpirv-Location-04915]), so a module whose interface +// blocks have had theirs removed is INVALID Vulkan SPIR-V by construction - that is what the +// pass was asked to produce. It never reaches a driver as SPIR-V: DirectGLES runs this last +// in its chain and hands the result straight to SPIRV-Cross, which needs no location to print +// a block. What the cases below assert instead is the thing that actually matters - that +// SPIRV-Cross still emits a complete, matchable interface from it. class StripIoBlockLocationsTest : public ::testing::Test { protected: - void SetUp() override { - MobileGL::Initialize(); - m_validationFailuresAtStart = ShaderCompiler::SpirvValidationFailureCount(); - } - - void TearDown() override { - EXPECT_EQ(ShaderCompiler::SpirvValidationFailureCount(), m_validationFailuresAtStart) - << "the stripped module did not survive spirv-val"; - } - - Uint64 m_validationFailuresAtStart = 0; + void SetUp() override { MobileGL::Initialize(); } }; TEST_F(StripIoBlockLocationsTest, DropsTheQualifierFromBothBlocksAndLeavesVaryingsAlone) { @@ -131,9 +127,6 @@ TEST_F(StripIoBlockLocationsTest, DropsTheQualifierFromBothBlocksAndLeavesVaryin ASSERT_FALSE(output.empty()); EXPECT_TRUE(strippedAny); - spvtools::SpirvTools tools(SPV_ENV_VULKAN_1_1); - ASSERT_TRUE(tools.Validate(output)); - const String after = Transpile(output); // The blocks come out bare... EXPECT_NE(after.find("in TCSOutputBlock"), String::npos) << after; diff --git a/MobileGL/MG_Util/ShaderTranspiler/ShaderCompiler.cpp b/MobileGL/MG_Util/ShaderTranspiler/ShaderCompiler.cpp index 8a4de5f1..05cb18e9 100644 --- a/MobileGL/MG_Util/ShaderTranspiler/ShaderCompiler.cpp +++ b/MobileGL/MG_Util/ShaderTranspiler/ShaderCompiler.cpp @@ -1241,8 +1241,16 @@ namespace MobileGL { optimizer.RegisterPass(StripIoBlockLocationsPass::CreateStripIoBlockLocationsPass( stripInputBlocks, stripOutputBlocks, &strippedAny)); + // NOT VALIDATED, and that is the point of the pass rather than an oversight. + // Vulkan SPIR-V requires a Location on every user-defined Input/Output variable + // ([VUID-StandaloneSpirv-Location-04915]), so a module whose interface blocks + // have deliberately lost theirs fails spirv-val by construction. It never + // reaches a driver as SPIR-V: the caller runs this last in the DirectGLES chain + // and hands the result straight to SPIRV-Cross, which needs no location to + // print a block. Validating here would latch a failure on every affected + // program and teach the counter to cry wolf. return RunOptimizerChecked("StripIoBlockLocationsForEssl", optimizer, inputBinary, - outputBinary, true, enableSpirvValidation); + outputBinary, false, enableSpirvValidation); } bool ShaderCompiler::PackDoubleVertexInputsForVulkan(const Vector& inputBinary,