diff --git a/CMakeLists.txt b/CMakeLists.txt index 4bc03c14..dd109a77 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -285,6 +285,7 @@ set(SOURCE_FILES MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/PackDoubleVertexInputsPass.cpp MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/FlattenXfbInterfaceBlocksPass.cpp MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/UniquifyIoBlockNamesPass.cpp + MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/StripIoBlockLocationsPass.cpp MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/SplitArrayVertexInputsPass.cpp MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/RebaseInstanceIndexPass.cpp MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/ZeroBaseVertexPass.cpp diff --git a/MobileGL/MG_Backend/DirectGLES/Managers.cpp b/MobileGL/MG_Backend/DirectGLES/Managers.cpp index 3f5a863b..42bd91d0 100644 --- a/MobileGL/MG_Backend/DirectGLES/Managers.cpp +++ b/MobileGL/MG_Backend/DirectGLES/Managers.cpp @@ -6301,7 +6301,8 @@ namespace MobileGL::MG_Backend::DirectGLES { const std::set& xfbCaptureBlockNames, const ImageFormatBakeInputs& imageFormatBake, const UnorderedMap& storageBlockBindingOverrides, const std::map& inputBlockRenames, - const std::map& outputBlockRenames, + const std::map& outputBlockRenames, const Bool stripInputBlockLocations, + const Bool stripOutputBlockLocations, const Int atomicCounterEsslBindingTop, const Bool enableSpirvValidation, String& outSource, std::set& outFlattenedXfbBlockNames, Vector& outAtomicCounterGlBindings, String& outError) const { @@ -6487,6 +6488,35 @@ 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 @@ -7087,6 +7117,18 @@ namespace MobileGL::MG_Backend::DirectGLES { stagePipelineIndices[index] = InterStagePipelineIndex(stage); if (CanDeclareBlocksInBothDirections(stage)) anyStageCanDeclareBlocksInBothDirections = true; } + // A SECOND, INDEPENDENT interface-block repair riding the same gate, because it + // needs the same question answered: "does this program have a stage where an + // inter-stage block can go wrong?". CanDeclareBlocksInBothDirections is true for + // exactly the tessellation and geometry stages, which is also exactly the set of + // stages whose presence makes this driver drop a LOCATED block's payload (a + // vertex-to-fragment located block is fine on the same driver, measured). The two + // repairs are otherwise unrelated: the rename fixes a name collision inside ONE + // stage, this drops a qualifier from EVERY block of the program - so it does not + // wait for the collision probe to find anything. + const Bool ioBlockLocationStripArmed = + !g_GLESCapabilities.SupportsLocatedInterStageIoBlocks && + anyStageCanDeclareBlocksInBothDirections; if (anyStageCanDeclareBlocksInBothDirections) { for (SizeT index = 0; index < shaderSpirvs.size(); ++index) { MG_Util::ShaderTranspiler::ShaderCompiler::ProbeIoBlockNamesForEssl( @@ -7281,6 +7323,31 @@ namespace MobileGL::MG_Backend::DirectGLES { } esslKeyInputs.inputBlockRenames = &inputBlockRenames; esslKeyInputs.outputBlockRenames = &outputBlockRenames; + + // ...and THIS STAGE's share of the interface-block LOCATION strip, planned the + // same way and for the same reason. The gate has three parts, all of which have + // to hold before a single block loses its qualifier: + // * the driver POST measured the defect (never a renderer-string quirk list); + // * this program has a stage that can hit it - a located block between a + // vertex and a fragment stage works on the affected driver, so a program + // with neither tessellation nor geometry keeps its ESSL byte for byte; + // * for THIS stage and THIS direction, the other end of the interface is in + // this same program. In a separate-shader-objects pipeline it is not, and + // the location is the only thing matching the two programs across - the + // identical reason the rename plan above tests producer/consumer presence. + // The direction tests reuse that plan's answers rather than recomputing them. + Bool stripInputBlockLocations = false; + Bool stripOutputBlockLocations = false; + if (ioBlockLocationStripArmed && stagePipelineIndices[index] >= 0) { + const Int myPipelineIndex = stagePipelineIndices[index]; + for (const Int otherPipelineIndex : stagePipelineIndices) { + if (otherPipelineIndex < 0) continue; + if (otherPipelineIndex < myPipelineIndex) stripInputBlockLocations = true; + if (otherPipelineIndex > myPipelineIndex) stripOutputBlockLocations = true; + } + } + esslKeyInputs.stripInputBlockLocations = stripInputBlockLocations; + esslKeyInputs.stripOutputBlockLocations = stripOutputBlockLocations; esslKeyInputs.enableSpirvValidation = enableSpirvValidation; auto& esslCache = MG_Util::ShaderTranspiler::GetEsslTranslationCache(); @@ -7307,6 +7374,7 @@ namespace MobileGL::MG_Backend::DirectGLES { if (!TranspileSpirvToEssl(spirvCode, glShaderType, xfbCaptureBlockNames, imageFormatBake, storageBlockBindingOverrides, inputBlockRenames, outputBlockRenames, + stripInputBlockLocations, stripOutputBlockLocations, m_atomicCounterEsslBindingTop, enableSpirvValidation, source, stageFlattenedXfbBlockNames, diff --git a/MobileGL/MG_Backend/DirectGLES/Managers.h b/MobileGL/MG_Backend/DirectGLES/Managers.h index 36d24936..8e50d929 100644 --- a/MobileGL/MG_Backend/DirectGLES/Managers.h +++ b/MobileGL/MG_Backend/DirectGLES/Managers.h @@ -1633,6 +1633,7 @@ namespace MobileGL::MG_Backend::DirectGLES { const UnorderedMap& storageBlockBindingOverrides, const std::map& inputBlockRenames, const std::map& outputBlockRenames, + Bool stripInputBlockLocations, Bool stripOutputBlockLocations, Int atomicCounterEsslBindingTop, Bool enableSpirvValidation, String& outSource, std::set& outFlattenedXfbBlockNames, diff --git a/MobileGL/MG_Util/BackendLoaders/OpenGL/Loader.cpp b/MobileGL/MG_Util/BackendLoaders/OpenGL/Loader.cpp index 8499681e..42c88547 100644 --- a/MobileGL/MG_Util/BackendLoaders/OpenGL/Loader.cpp +++ b/MobileGL/MG_Util/BackendLoaders/OpenGL/Loader.cpp @@ -836,6 +836,196 @@ namespace MobileGL::MG_Util::BackendLoader { return includesBase; } + // Whether an inter-stage interface BLOCK that carries an explicit layout(location=) + // actually transports its payload across a geometry (or tessellation) boundary. + // + // The Mali-G1-Ultra ES driver (r54p1) links such a program with an empty info log, runs + // the draw, and delivers ZEROES to the consuming stage; the identical program with the + // qualifier removed from the blocks carries the payload correctly. That is not a MobileGL + // artefact - a bare EGL/GLES 3.2 program with no MobileGL in the process reproduces it - + // and it is the whole of the KHR-GLxx.shading_language_420pack interface-block group's + // failures on this device. A located block between a VERTEX and a FRAGMENT stage is fine + // on the same driver, so the probe deliberately spans a geometry stage: that is the + // shape that breaks, and answering the narrower question would report a healthy driver. + // + // Probed rather than matched on the renderer string, because "which drivers do this" is + // not knowable and a quirk list is wrong the moment a driver is fixed. A driver that + // cannot run the probe at all (pre-ES 3.2, no geometry stage, missing entry point) is + // reported as HEALTHY: that leaves its ESSL exactly as it is today, which is the answer + // with no behaviour change in it. + Bool ProbeLocatedInterStageIoBlocksTransportPayload(const MG_External::GLESCapabilities& caps, + const MG_External::GLESFunctionsTable& f) { + const Bool esVersionOk = + caps.GLESVersion.Major > 3 || (caps.GLESVersion.Major == 3 && caps.GLESVersion.Minor >= 2); + if (!esVersionOk || !f.glCreateShader || !f.glCreateProgram || !f.glGenVertexArrays || + !f.glGenRenderbuffers || !f.glGenFramebuffers || !f.glReadPixels || !f.glDrawArrays) { + MGLOG_I("located-IO-block probe skipped: needs an ES 3.2 context with a geometry stage"); + return true; + } + while (f.glGetError() != GL_NO_ERROR) { + } + + // Position comes from gl_VertexID so the probe needs no vertex buffer, and the block + // payload is two values that survive an 8-bit target exactly (0.25 -> 64, 0.5 -> 128). + // TWO different block names, because the two boundaries this crosses (VS->GS and + // GS->FS) are separate interfaces; a single name would also trip the in/out collision + // UniquifyIoBlockNamesPass exists for and confuse one defect with the other. + const char* vsSource = "#version 320 es\n" + "precision highp float;\n" + "layout(location = 0) out MgProbeBlock { vec2 mg_probeValue; } mg_probeOut;\n" + "void main() {\n" + " vec2 p = vec2((gl_VertexID == 1) ? 3.0 : -1.0, (gl_VertexID == 2) ? 3.0 : -1.0);\n" + " gl_Position = vec4(p, 0.0, 1.0);\n" + " mg_probeOut.mg_probeValue = vec2(0.25, 0.5);\n" + "}\n"; + const char* gsSource = "#version 320 es\n" + "precision highp float;\n" + "layout(triangles) in;\n" + "layout(triangle_strip, max_vertices = 3) out;\n" + "layout(location = 0) in MgProbeBlock { vec2 mg_probeValue; } mg_probeIn[];\n" + "layout(location = 0) out MgProbeBlock2 { vec2 mg_probeValue; } mg_probeOut;\n" + "void main() {\n" + " for (int i = 0; i < 3; ++i) {\n" + " gl_Position = gl_in[i].gl_Position;\n" + " mg_probeOut.mg_probeValue = mg_probeIn[i].mg_probeValue;\n" + " EmitVertex();\n" + " }\n" + "}\n"; + const char* fsSource = "#version 320 es\n" + "precision highp float;\n" + "layout(location = 0) in MgProbeBlock2 { vec2 mg_probeValue; } mg_probeIn;\n" + "layout(location = 0) out vec4 mg_probeColor;\n" + "void main() { mg_probeColor = vec4(mg_probeIn.mg_probeValue, 0.0, 1.0); }\n"; + + const auto compileShader = [&f](GLenum type, const char* src) -> GLuint { + const GLuint shader = f.glCreateShader(type); + if (shader == 0) return 0; + f.glShaderSource(shader, 1, &src, nullptr); + f.glCompileShader(shader); + GLint status = GL_FALSE; + f.glGetShaderiv(shader, GL_COMPILE_STATUS, &status); + if (status != GL_TRUE) { + f.glDeleteShader(shader); + return 0; + } + return shader; + }; + const GLuint vs = compileShader(GL_VERTEX_SHADER, vsSource); + const GLuint gs = compileShader(GL_GEOMETRY_SHADER, gsSource); + const GLuint fs = compileShader(GL_FRAGMENT_SHADER, fsSource); + GLuint program = 0; + if (vs != 0 && gs != 0 && fs != 0) { + program = f.glCreateProgram(); + if (program != 0) { + f.glAttachShader(program, vs); + f.glAttachShader(program, gs); + f.glAttachShader(program, fs); + f.glLinkProgram(program); + GLint status = GL_FALSE; + f.glGetProgramiv(program, GL_LINK_STATUS, &status); + if (status != GL_TRUE) { + f.glDeleteProgram(program); + program = 0; + } + } + } + if (vs != 0) f.glDeleteShader(vs); + if (gs != 0) f.glDeleteShader(gs); + if (fs != 0) f.glDeleteShader(fs); + if (program == 0) { + MGLOG_I("located-IO-block probe skipped: probe program failed to build (vs=%u gs=%u fs=%u)", vs, + gs, fs); + while (f.glGetError() != GL_NO_ERROR) { + } + return true; + } + + // Everything this probe changes is read back first and put back afterwards. The + // capability run owns a context of its own, but a probe that leaves a 1x1 viewport or + // a bound scratch framebuffer behind would be found by whatever draws next rather + // than here, and that is not a bug anyone should have to chase twice. + GLint savedProgram = 0, savedVao = 0, savedDrawFbo = 0, savedReadFbo = 0, savedRenderbuffer = 0; + GLint savedViewport[4] = {0, 0, 0, 0}; + GLfloat savedClearColor[4] = {0.0f, 0.0f, 0.0f, 0.0f}; + GLboolean savedColorMask[4] = {GL_TRUE, GL_TRUE, GL_TRUE, GL_TRUE}; + f.glGetIntegerv(GL_CURRENT_PROGRAM, &savedProgram); + f.glGetIntegerv(GL_VERTEX_ARRAY_BINDING, &savedVao); + f.glGetIntegerv(GL_DRAW_FRAMEBUFFER_BINDING, &savedDrawFbo); + f.glGetIntegerv(GL_READ_FRAMEBUFFER_BINDING, &savedReadFbo); + f.glGetIntegerv(GL_RENDERBUFFER_BINDING, &savedRenderbuffer); + f.glGetIntegerv(GL_VIEWPORT, savedViewport); + f.glGetFloatv(GL_COLOR_CLEAR_VALUE, savedClearColor); + f.glGetBooleanv(GL_COLOR_WRITEMASK, savedColorMask); + // The five raster states that can void the draw and turn a healthy driver into a + // "broken" verdict. Saved, forced off, and put back. + constexpr GLenum kQuietedStates[] = {GL_SCISSOR_TEST, GL_RASTERIZER_DISCARD, GL_CULL_FACE, + GL_DEPTH_TEST, GL_BLEND}; + GLboolean savedStates[5] = {GL_FALSE, GL_FALSE, GL_FALSE, GL_FALSE, GL_FALSE}; + for (SizeT i = 0; i < std::size(kQuietedStates); ++i) { + savedStates[i] = f.glIsEnabled(kQuietedStates[i]); + if (savedStates[i] == GL_TRUE) f.glDisable(kQuietedStates[i]); + } + f.glColorMask(GL_TRUE, GL_TRUE, GL_TRUE, GL_TRUE); + + // ES makes a draw on the default vertex array object invalid in a core-profile sense, + // and the default framebuffer may be incomplete (surfaceless contexts), so the probe + // brings both of its own. + GLuint vao = 0, framebuffer = 0, renderbuffer = 0; + f.glGenVertexArrays(1, &vao); + f.glBindVertexArray(vao); + f.glGenRenderbuffers(1, &renderbuffer); + f.glBindRenderbuffer(GL_RENDERBUFFER, renderbuffer); + f.glRenderbufferStorage(GL_RENDERBUFFER, GL_RGBA8, 1, 1); + f.glGenFramebuffers(1, &framebuffer); + f.glBindFramebuffer(GL_FRAMEBUFFER, framebuffer); + f.glFramebufferRenderbuffer(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_RENDERBUFFER, renderbuffer); + f.glViewport(0, 0, 1, 1); + f.glUseProgram(program); + f.glClearColor(0.0f, 0.0f, 0.0f, 0.0f); + f.glClear(GL_COLOR_BUFFER_BIT); + f.glDrawArrays(GL_TRIANGLES, 0, 3); + + Bool transports = true; + const GLenum drawError = f.glGetError(); + if (drawError == GL_NO_ERROR) { + GLubyte pixel[4] = {0, 0, 0, 0}; + f.glReadPixels(0, 0, 1, 1, GL_RGBA, GL_UNSIGNED_BYTE, pixel); + if (f.glGetError() == GL_NO_ERROR) { + // Exactly the two values the vertex stage wrote, with one bit of slack for a + // driver that rounds the 8-bit conversion the other way. A stage that received + // nothing reads 0/0, which is nowhere near either. + transports = pixel[0] >= 0x3f && pixel[0] <= 0x41 && pixel[1] >= 0x7f && pixel[1] <= 0x81; + MGLOG_I("located-IO-block probe: fragment stage received (%u, %u), expected (64, 128)", + pixel[0], pixel[1]); + } else { + MGLOG_I("located-IO-block probe inconclusive: readback failed"); + } + } else { + MGLOG_I("located-IO-block probe inconclusive: draw raised GL error 0x%x", drawError); + } + + f.glUseProgram(static_cast(savedProgram)); + f.glBindFramebuffer(GL_FRAMEBUFFER, 0); + f.glDeleteFramebuffers(1, &framebuffer); + f.glDeleteRenderbuffers(1, &renderbuffer); + f.glBindVertexArray(0); + f.glDeleteVertexArrays(1, &vao); + f.glBindVertexArray(static_cast(savedVao)); + f.glBindRenderbuffer(GL_RENDERBUFFER, static_cast(savedRenderbuffer)); + f.glBindFramebuffer(GL_DRAW_FRAMEBUFFER, static_cast(savedDrawFbo)); + f.glBindFramebuffer(GL_READ_FRAMEBUFFER, static_cast(savedReadFbo)); + f.glViewport(savedViewport[0], savedViewport[1], savedViewport[2], savedViewport[3]); + f.glClearColor(savedClearColor[0], savedClearColor[1], savedClearColor[2], savedClearColor[3]); + f.glColorMask(savedColorMask[0], savedColorMask[1], savedColorMask[2], savedColorMask[3]); + for (SizeT i = 0; i < std::size(kQuietedStates); ++i) { + if (savedStates[i] == GL_TRUE) f.glEnable(kQuietedStates[i]); + } + f.glDeleteProgram(program); + while (f.glGetError() != GL_NO_ERROR) { + } + return transports; + } + // GL 4.6 table 23.65 admits exactly four answers for GL_LAYER_PROVOKING_VERTEX and // GL_VIEWPORT_INDEX_PROVOKING_VERTEX. Anything else means the driver wrote something MobileGL // cannot forward as a convention, and GL_UNDEFINED_VERTEX - a legal answer, not a placeholder @@ -1724,6 +1914,14 @@ namespace MobileGL::MG_Util::BackendLoader { MGLOG_I(" Indirect draw gl_InstanceID includes baseInstance: %s", caps.IndirectDrawInstanceIdIncludesBaseInstance ? "true" : "false"); + caps.SupportsLocatedInterStageIoBlocks = + ProbeLocatedInterStageIoBlocksTransportPayload(caps, glesFuncs); + MGLOG_I(" Located inter-stage interface blocks transport their payload: %s", + caps.SupportsLocatedInterStageIoBlocks + ? "true" + : "false (DirectGLES will emit tessellation/geometry programs' interface " + "blocks without a location qualifier)"); + caps.IsAngleRenderer = caps.GLESRendererString.find("ANGLE") != String::npos; caps.IsAngleLlvmpipeRenderer = caps.IsAngleRenderer && caps.GLESRendererString.find("llvmpipe") != String::npos; diff --git a/MobileGL/MG_Util/BackendLoaders/OpenGL/Loader.h b/MobileGL/MG_Util/BackendLoaders/OpenGL/Loader.h index 71b18dda..defd5564 100644 --- a/MobileGL/MG_Util/BackendLoaders/OpenGL/Loader.h +++ b/MobileGL/MG_Util/BackendLoaders/OpenGL/Loader.h @@ -1247,6 +1247,17 @@ namespace MobileGL { // straight to vkCmdDraw*Indirect and compiles gl_InstanceID to SPIR-V // InstanceIndex, which includes firstInstance. Bool IndirectDrawInstanceIdIncludesBaseInstance = false; + // True when an inter-stage interface BLOCK carrying an explicit layout(location=) + // actually delivers its payload across a tessellation or geometry boundary. The + // Mali-G1-Ultra ES driver links such a program with an empty info log and then + // hands the consuming stage zeroes; DirectGLES answers by emitting those blocks + // with no location qualifier at all (StripIoBlockLocationsPass), which ES matches + // by block name and member sequence instead. + // + // Defaults TRUE and stays true when the probe cannot run, because that is the + // behaviour every driver had before the probe existed - a capability like this + // must never be assumed broken on a driver nobody measured. + Bool SupportsLocatedInterStageIoBlocks = true; Int UniformBufferOffsetAlignment = 256; // Its storage-buffer counterpart, queried separately because it is a separate limit: // Adreno 830 answers 32 for GL_UNIFORM_BUFFER_OFFSET_ALIGNMENT and 64 for @@ -1349,6 +1360,12 @@ namespace MobileGL { // so MG_Test can drive it against a fake GLES functions table. Bool ProbeIndirectInstanceIdIncludesBaseInstance(const MG_External::GLESCapabilities& caps, const MG_External::GLESFunctionsTable& glesFuncs); + // Detects whether an inter-stage interface block that carries an explicit + // layout(location=) actually transports its payload across a geometry boundary (see + // Loader.cpp). Called by FillInGLESCapabilities; exposed so MG_Test can drive it + // against a fake GLES functions table. + Bool ProbeLocatedInterStageIoBlocksTransportPayload(const MG_External::GLESCapabilities& caps, + const MG_External::GLESFunctionsTable& glesFuncs); } // namespace MG_Util::BackendLoader } // namespace MobileGL #undef MOBILEGL_EXTERNAL_GLES diff --git a/MobileGL/MG_Util/ShaderTranspiler/ShaderCompiler.cpp b/MobileGL/MG_Util/ShaderTranspiler/ShaderCompiler.cpp index 4eb22f2c..8a4de5f1 100644 --- a/MobileGL/MG_Util/ShaderTranspiler/ShaderCompiler.cpp +++ b/MobileGL/MG_Util/ShaderTranspiler/ShaderCompiler.cpp @@ -45,6 +45,7 @@ #include "SpirvPasses/ClampMultisampleFetchPass.h" #include "SpirvPasses/PrivateToEntryLocalPass.h" #include "SpirvPasses/StripUniformLocationsPass.h" +#include "SpirvPasses/StripIoBlockLocationsPass.h" #include "SpirvPasses/StripUboMemberRelaxedPrecisionPass.h" #include "SpirvPasses/StripNoPerspectivePass.h" #include "SpirvPasses/EmulateNoPerspectivePass.h" @@ -1227,6 +1228,23 @@ namespace MobileGL { outputBinary, true, enableSpirvValidation); } + bool ShaderCompiler::StripIoBlockLocationsForEssl(const Vector& inputBinary, + const bool stripInputBlocks, + const bool stripOutputBlocks, + bool& strippedAny, + Vector& outputBinary, + const bool enableSpirvValidation) { + using namespace spvtools; + strippedAny = false; + if (!stripInputBlocks && !stripOutputBlocks) return false; + Optimizer optimizer(SPV_ENV_VULKAN_1_1); + optimizer.RegisterPass(StripIoBlockLocationsPass::CreateStripIoBlockLocationsPass( + stripInputBlocks, stripOutputBlocks, &strippedAny)); + + return RunOptimizerChecked("StripIoBlockLocationsForEssl", optimizer, inputBinary, + outputBinary, true, enableSpirvValidation); + } + bool ShaderCompiler::PackDoubleVertexInputsForVulkan(const Vector& inputBinary, Vector& outputBinary, const bool enableSpirvValidation) { diff --git a/MobileGL/MG_Util/ShaderTranspiler/ShaderCompiler.h b/MobileGL/MG_Util/ShaderTranspiler/ShaderCompiler.h index 3eb21723..ba7b87a7 100644 --- a/MobileGL/MG_Util/ShaderTranspiler/ShaderCompiler.h +++ b/MobileGL/MG_Util/ShaderTranspiler/ShaderCompiler.h @@ -154,6 +154,20 @@ namespace MobileGL { std::set& renamedBlockNames, Vector& outputBinary, bool enableSpirvValidation = false); + // Drops the Location (and Component) decoration from inter-stage interface + // BLOCK variables, so SPIRV-Cross emits them unqualified and ES matches them + // by block name plus member sequence. `stripInputBlocks` covers the blocks + // this stage consumes and `stripOutputBlocks` the ones it produces - armed + // separately because an interface whose other end is in a DIFFERENT program + // must keep the location that matches it there. `strippedAny` reports whether + // this stage actually had one. The Mali ES driver loses the payload of a + // located block across any tessellation or geometry boundary; only for the + // DirectGLES transpile path, and only when the driver POST says so. See + // StripIoBlockLocationsPass. + static bool StripIoBlockLocationsForEssl(const Vector& inputBinary, + bool stripInputBlocks, bool stripOutputBlocks, + bool& strippedAny, Vector& outputBinary, + bool enableSpirvValidation = false); // Drops RelaxedPrecision member decorations from uniform-block structs so // SPIRV-Cross prints the same (highp) member precision in every stage; ES // drivers reject cross-stage uniform blocks whose member precisions differ. diff --git a/MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/StripIoBlockLocationsPass.cpp b/MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/StripIoBlockLocationsPass.cpp new file mode 100644 index 00000000..4cb7e681 --- /dev/null +++ b/MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/StripIoBlockLocationsPass.cpp @@ -0,0 +1,145 @@ +// MobileGL - MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/StripIoBlockLocationsPass.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 "StripIoBlockLocationsPass.h" + +#include "spirv.hpp" +#include "source/opt/def_use_manager.h" +#include "source/opt/instruction.h" +#include "source/opt/ir_context.h" +#include "source/opt/module.h" +#include "source/util/make_unique.h" + +#include +#include + +namespace MobileGL { + namespace MG_Util { + namespace ShaderTranspiler { + namespace { + using spvtools::opt::Instruction; + using spvtools::opt::IRContext; + + // Every struct type carrying the Block decoration, minus the ones with a builtin + // member (gl_PerVertex and friends): those are spelled by the language, carry no + // user Location, and are not what this pass is about. Same shape as + // UniquifyIoBlockNamesPass::CollectUserBlockStructIds, and deliberately kept + // beside its own pass rather than shared - the two ask the same question of the + // module but are armed by different gates, and one growing a special case must + // not silently move the other. + std::unordered_set CollectUserBlockStructIds(IRContext* irContext) { + std::unordered_set blockStructIds; + std::unordered_set builtinStructIds; + for (Instruction& annotation : irContext->module()->annotations()) { + if (annotation.opcode() == spv::Op::OpDecorate) { + if (static_cast(annotation.GetSingleWordInOperand(1)) == + spv::Decoration::Block) { + blockStructIds.insert(annotation.GetSingleWordInOperand(0)); + } + } else if (annotation.opcode() == spv::Op::OpMemberDecorate) { + if (static_cast(annotation.GetSingleWordInOperand(2)) == + spv::Decoration::BuiltIn) { + builtinStructIds.insert(annotation.GetSingleWordInOperand(0)); + } + } + } + for (const uint32_t builtinStructId : builtinStructIds) { + blockStructIds.erase(builtinStructId); + } + return blockStructIds; + } + + // True when `variable` is an Input/Output interface block of the direction the + // caller armed. Tessellation and geometry interfaces are arrays of the block + // struct, so array levels are unwrapped before the struct is recognised. + Bool IsArmedInterfaceBlock(IRContext* irContext, Instruction& variable, + const std::unordered_set& blockStructIds, + Bool stripInputBlocks, Bool stripOutputBlocks) { + if (variable.opcode() != spv::Op::OpVariable) return false; + const auto storageClass = + static_cast(variable.GetSingleWordInOperand(0)); + if (storageClass == spv::StorageClass::Input) { + if (!stripInputBlocks) return false; + } else if (storageClass == spv::StorageClass::Output) { + if (!stripOutputBlocks) return false; + } else { + return false; + } + + auto* defUseMgr = irContext->get_def_use_mgr(); + Instruction* pointerType = defUseMgr->GetDef(variable.type_id()); + if (pointerType == nullptr || pointerType->opcode() != spv::Op::OpTypePointer) { + return false; + } + uint32_t pointeeId = pointerType->GetSingleWordInOperand(1); + Instruction* pointee = defUseMgr->GetDef(pointeeId); + while (pointee != nullptr && (pointee->opcode() == spv::Op::OpTypeArray || + pointee->opcode() == spv::Op::OpTypeRuntimeArray)) { + pointeeId = pointee->GetSingleWordInOperand(0); + pointee = defUseMgr->GetDef(pointeeId); + } + if (pointee == nullptr || pointee->opcode() != spv::Op::OpTypeStruct) return false; + return blockStructIds.find(pointeeId) != blockStructIds.end(); + } + } // namespace + + spvtools::opt::Pass::Status StripIoBlockLocationsPass::Process() { + if (m_strippedAny != nullptr) *m_strippedAny = false; + if (!m_stripInputBlocks && !m_stripOutputBlocks) return Status::SuccessWithoutChange; + + auto* irContext = context(); + const std::unordered_set blockStructIds = CollectUserBlockStructIds(irContext); + if (blockStructIds.empty()) return Status::SuccessWithoutChange; + + // The variable ids to strip, resolved BEFORE anything is killed: the walk below + // deletes annotations, and deciding what to delete while deleting reads a list + // that is being mutated underneath it. + std::unordered_set armedVariableIds; + for (Instruction& variable : irContext->module()->types_values()) { + if (IsArmedInterfaceBlock(irContext, variable, blockStructIds, m_stripInputBlocks, + m_stripOutputBlocks)) { + armedVariableIds.insert(variable.result_id()); + } + } + if (armedVariableIds.empty()) return Status::SuccessWithoutChange; + + std::vector toKill; + for (Instruction& annotation : irContext->module()->annotations()) { + if (annotation.opcode() != spv::Op::OpDecorate) continue; + const auto decoration = + static_cast(annotation.GetSingleWordInOperand(1)); + // Component travels with Location and is meaningless without it; a block + // whose Location is gone and whose Component survives would be a shader + // SPIRV-Cross prints `layout(component = N)` for on its own, which ESSL has + // no spelling for at all. + if (decoration != spv::Decoration::Location && + decoration != spv::Decoration::Component) { + continue; + } + if (armedVariableIds.find(annotation.GetSingleWordInOperand(0)) == + armedVariableIds.end()) { + continue; + } + toKill.push_back(&annotation); + } + + for (Instruction* inst : toKill) { + irContext->KillInst(inst); + } + if (m_strippedAny != nullptr) *m_strippedAny = !toKill.empty(); + return toKill.empty() ? Status::SuccessWithoutChange : Status::SuccessWithChange; + } + + spvtools::Optimizer::PassToken StripIoBlockLocationsPass::CreateStripIoBlockLocationsPass( + Bool stripInputBlocks, Bool stripOutputBlocks, Bool* strippedAny) { + return spvtools::Optimizer::PassToken(spvtools::MakeUnique( + stripInputBlocks, stripOutputBlocks, strippedAny)); + } + } // namespace ShaderTranspiler + } // namespace MG_Util +} // namespace MobileGL diff --git a/MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/StripIoBlockLocationsPass.h b/MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/StripIoBlockLocationsPass.h new file mode 100644 index 00000000..14789373 --- /dev/null +++ b/MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/StripIoBlockLocationsPass.h @@ -0,0 +1,81 @@ +// MobileGL - MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/StripIoBlockLocationsPass.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 + +namespace MobileGL { + namespace MG_Util { + namespace ShaderTranspiler { + // Drops the Location (and Component) decoration from an inter-stage interface + // BLOCK variable, so SPIRV-Cross emits `out FOO { ... } x;` instead of + // `layout(location = N) out FOO { ... } x;`. + // + // WHY. On the Mali-G1-Ultra ES driver (r54p1), an interface block that carries an + // explicit layout(location=) transports NOTHING across any boundary that involves + // a tessellation or geometry stage. The stages compile, the program links with an + // empty info log, the draw runs - and the consuming stage reads zeroes. The same + // program with the qualifier removed from the blocks (and nothing else changed) + // carries the payload correctly. Measured with no MobileGL in the process at all: + // a bare EGL/GLES 3.2 program built from the five ESSL stages MobileGL emits for + // KHR-GLxx.shading_language_420pack.length_of_vector_and_matrix_* reproduces it, + // and a three-stage VS->GS->FS reduction isolates it to + // (block carries a location) AND (a tessellation or geometry stage is present). + // A located block between a vertex and a fragment stage is fine on the same + // driver, which is why the caller only arms this for programs that have one of + // those stages. + // + // The locations are not the application's: these blocks carry no location in the + // GLSL source at all (the 420pack cases declare none). glslang's cross-stage IO + // resolver invents them, SPIRV-Cross prints them because ESSL >= 310 allows a + // location on a block, and nothing downstream needs them - ES matches inter-stage + // blocks by block name plus member sequence, which is exactly what + // UniquifyIoBlockNamesPass keeps consistent across the program. + // + // WHAT. Only variables in Input/Output storage whose (array-unwrapped) pointee is + // a Block-decorated struct. Plain varyings keep their locations - they work on + // this driver and are how the fragment stage's inputs and outputs are matched - + // and so do vertex attributes and fragment outputs, which are never blocks. + // Builtin blocks (gl_PerVertex) are skipped; they carry no Location anyway. + // + // The two directions are armed SEPARATELY by the caller, because an interface + // whose other end lives in a DIFFERENT program (a separable program pipeline) + // must keep its location: that is the only thing matching it there, and the other + // program never saw this decision. In a monolithic program both ends are present + // and both flags are set. + // + // DirectGLES only: DirectVulkan hands the module to the driver as SPIR-V, where + // Location is how interfaces are matched and removing it would be a miscompile. + class StripIoBlockLocationsPass final : public spvtools::opt::Pass { + public: + // `stripInputBlocks` covers the blocks this stage CONSUMES and + // `stripOutputBlocks` the ones it PRODUCES. `strippedAny`, when non-null, + // receives whether this stage actually had one, so the caller can decline the + // re-serialised module when there was nothing to strip. + StripIoBlockLocationsPass(Bool stripInputBlocks, Bool stripOutputBlocks, + Bool* strippedAny = nullptr) + : m_stripInputBlocks(stripInputBlocks), m_stripOutputBlocks(stripOutputBlocks), + m_strippedAny(strippedAny) {} + + const char* name() const override { return "mobilegl-strip-io-block-locations"; } + Status Process() override; + + static spvtools::Optimizer::PassToken CreateStripIoBlockLocationsPass( + Bool stripInputBlocks, Bool stripOutputBlocks, Bool* strippedAny); + + private: + Bool m_stripInputBlocks = false; + Bool m_stripOutputBlocks = false; + Bool* m_strippedAny = nullptr; + }; + } // namespace ShaderTranspiler + } // namespace MG_Util +} // namespace MobileGL diff --git a/MobileGL/MG_Util/ShaderTranspiler/TranslationCache.cpp b/MobileGL/MG_Util/ShaderTranspiler/TranslationCache.cpp index 3f7f43ab..72c3bf97 100644 --- a/MobileGL/MG_Util/ShaderTranspiler/TranslationCache.cpp +++ b/MobileGL/MG_Util/ShaderTranspiler/TranslationCache.cpp @@ -194,6 +194,8 @@ namespace MobileGL::MG_Util::ShaderTranspiler { static const std::map kEmptyRenames; builder.StringMap(inputs.inputBlockRenames ? *inputs.inputBlockRenames : kEmptyRenames); builder.StringMap(inputs.outputBlockRenames ? *inputs.outputBlockRenames : kEmptyRenames); + builder.Value(static_cast(inputs.stripInputBlockLocations)); + builder.Value(static_cast(inputs.stripOutputBlockLocations)); static const Vector kEmptyWords; builder.Words(inputs.spirv ? *inputs.spirv : kEmptyWords); return MakeTranslationCacheKey(builder); diff --git a/MobileGL/MG_Util/ShaderTranspiler/TranslationCache.h b/MobileGL/MG_Util/ShaderTranspiler/TranslationCache.h index e3713945..e39ad4e7 100644 --- a/MobileGL/MG_Util/ShaderTranspiler/TranslationCache.h +++ b/MobileGL/MG_Util/ShaderTranspiler/TranslationCache.h @@ -658,6 +658,14 @@ namespace MobileGL::MG_Util::ShaderTranspiler { // arguments, so they are exactly as fine as its behaviour and no finer. const std::map* inputBlockRenames = nullptr; const std::map* outputBlockRenames = nullptr; + // THIS STAGE's share of the interface-block LOCATION strip - the two arguments + // StripIoBlockLocationsForEssl is called with, which decide whether the emitted ESSL + // prints `layout(location = N)` on a block at all. False for every program on a + // driver whose POST said located blocks work, and for every program without a + // tessellation or geometry stage. Armed per direction because an interface whose + // other end is in a different program must keep its location. + Bool stripInputBlockLocations = false; + Bool stripOutputBlockLocations = false; // The top of the reserved storage-block window atomic-counter blocks are moved into // (`top - N` for GL binding N). Derived from the driver's