From 6aa161fee7b192682fed077584d937ada935f5cf Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Thu, 20 Aug 2026 16:56:58 -0400 Subject: [PATCH] [Fix, Test] (DirectGLES, ShaderTranspiler, MG_IntegrationTest): spell an interface block declared in both directions once per producing stage --- CMakeLists.txt | 1 + MobileGL/MG_Backend/DirectGLES/Managers.cpp | 138 +++++++ MobileGL/MG_IntegrationTest/CMakeLists.txt | 1 + .../IoBlockNameCollisionScenario.cpp | 389 ++++++++++++++++++ .../MG_Test/ShaderTranspiler/CMakeLists.txt | 1 + .../UniquifyIoBlockNamesTest.cpp | 262 ++++++++++++ .../ShaderTranspiler/ShaderCompiler.cpp | 37 ++ .../MG_Util/ShaderTranspiler/ShaderCompiler.h | 24 ++ .../SpirvPasses/UniquifyIoBlockNamesPass.cpp | 231 +++++++++++ .../SpirvPasses/UniquifyIoBlockNamesPass.h | 90 ++++ 10 files changed, 1174 insertions(+) create mode 100644 MobileGL/MG_IntegrationTest/Scenarios/IoBlockNameCollisionScenario.cpp create mode 100644 MobileGL/MG_Test/ShaderTranspiler/UniquifyIoBlockNamesTest.cpp create mode 100644 MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/UniquifyIoBlockNamesPass.cpp create mode 100644 MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/UniquifyIoBlockNamesPass.h diff --git a/CMakeLists.txt b/CMakeLists.txt index 01abae32..33068f02 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -282,6 +282,7 @@ set(SOURCE_FILES MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/LowerViewportIndexPass.cpp 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/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 a71dcc35..f07b97c3 100644 --- a/MobileGL/MG_Backend/DirectGLES/Managers.cpp +++ b/MobileGL/MG_Backend/DirectGLES/Managers.cpp @@ -29,6 +29,7 @@ #include #include #include +#include #include #include #include @@ -4715,6 +4716,34 @@ namespace MobileGL::MG_Backend::DirectGLES { return signature + entry; } + // Pipeline position of a shader stage. Names the PRODUCER of an inter-stage + // interface block: a block one stage consumes was written by the stage before it. + // ShaderStage is declared in pipeline order, so the enum value IS the position; + // compute has no inter-stage interface at all and is reported as -1. + Int InterStagePipelineIndex(ShaderStage stage) { + switch (stage) { + case ShaderStage::Vertex: + case ShaderStage::TessControl: + case ShaderStage::TessEval: + case ShaderStage::Geometry: + case ShaderStage::Fragment: + return static_cast(stage); + default: + return -1; + } + } + + // Whether a stage can declare interface blocks in BOTH directions at once, i.e. + // whether one block name can name two different blocks inside it. A vertex INPUT + // and a fragment OUTPUT cannot be blocks and compute has neither, so only these + // three can. This is what keeps the module probe off every program without + // tessellation or geometry - which is every program Minecraft and its shader packs + // build. + Bool CanDeclareBlocksInBothDirections(ShaderStage stage) { + return stage == ShaderStage::TessControl || stage == ShaderStage::TessEval || + stage == ShaderStage::Geometry; + } + // Reflection names an array uniform after its first element ("g_image[0]") at every // location it spans; SPIR-V names the variable once, without the subscript. This is // the name both sides agree on. @@ -4998,6 +5027,61 @@ namespace MobileGL::MG_Backend::DirectGLES { } std::set flattenedXfbBlockNames; + // Desktop GLSL keeps SEPARATE name namespaces for input and output interface + // blocks, so ONE stage may legally declare `in FOO {...}` and `out FOO {...}` at + // the same time - which the tessellation evaluation stage of both interface-block + // tests in KHR-GL42/43.shading_language_420pack does ("in TCSOutputBlock ... out + // TCSOutputBlock"). SPIRV-Cross keeps the same split (block_input_names vs + // block_output_names) and re-emits BOTH under the name FOO, so the generated ESSL + // declares two different blocks called FOO in one shader. Adreno's ES compiler + // keeps them apart; Mali's does not - the stage compiles, the program links, and + // the output block's payload never reaches the next stage. All 22 of that group's + // Mali failures are exactly the two tests that write this shape, and every one of + // them passes on Adreno and on DirectVulkan. + // + // The repair is a rename keyed on the PRODUCING stage, planned here and applied + // per stage below so a producer and its consumer keep naming the same block. + // Gated twice over, because a re-serialised module is not free (it cost the + // create-indirect retrace 0.15 SSIM the first time the array-input split missed + // its gate): only a tessellation or geometry stage can declare blocks in both + // directions at all, and even then the probe has to FIND a collision before any + // stage is rewritten. + std::set collidingIoBlockNames; + std::set declaredIoBlockNames; + Vector stagePipelineIndices(attachedShaders.size(), -1); + Bool anyStageCanDeclareBlocksInBothDirections = false; + for (SizeT index = 0; index < attachedShaders.size(); ++index) { + const ShaderStage stage = attachedShaders[index]->GetShaderStage(); + stagePipelineIndices[index] = InterStagePipelineIndex(stage); + if (CanDeclareBlocksInBothDirections(stage)) anyStageCanDeclareBlocksInBothDirections = true; + } + if (anyStageCanDeclareBlocksInBothDirections) { + for (SizeT index = 0; index < attachedShaders.size() && index < shaderSpirvs.size(); ++index) { + MG_Util::ShaderTranspiler::ShaderCompiler::ProbeIoBlockNamesForEssl( + shaderSpirvs[index], collidingIoBlockNames, declaredIoBlockNames); + } + // A block a capture request names is resolved BY NAME at + // glTransformFeedbackVaryings time - and flattened away entirely by the pass + // below - so renaming one would ask the driver for a block the request does + // not spell. + for (const auto& xfbCaptureBlockName : xfbCaptureBlockNames) { + collidingIoBlockNames.erase(xfbCaptureBlockName); + } + } + // The one spelling every stage of THIS program agrees on for `blockName` as written + // by pipeline stage `producerPipelineIndex`. "__" is reserved in GLSL, so a name + // already ending in '_' does not get another one, and the digit-suffix loop steps + // off any name the program already spells. + const auto uniqueIoBlockName = [&declaredIoBlockNames](const String& blockName, + Int producerPipelineIndex) { + const char* separator = (!blockName.empty() && blockName.back() == '_') ? "" : "_"; + String candidate = blockName + separator + "mgio" + std::to_string(producerPipelineIndex); + while (declaredIoBlockNames.find(candidate) != declaredIoBlockNames.end()) { + candidate += "0"; + } + return candidate; + }; + for (int index = 0; index < attachedShaders.size(); ++index) { auto& shader = attachedShaders[index]; GLenum glShaderType = MG_Util::ConvertShaderStageToGLEnum(shader->GetShaderStage()); @@ -5146,6 +5230,60 @@ namespace MobileGL::MG_Backend::DirectGLES { stageFlattenedXfbBlockNames.end()); } + // The producer-keyed rename planned above the loop, applied to this stage: the + // blocks it CONSUMES are spelled after the previous stage present in the + // program and the ones it PRODUCES after itself, so a tessellation evaluation + // stage's two TCSOutputBlocks stop being one name and every other stage still + // agrees with it. Same adopt-only-if-rewritten gate as the flatten above. + // + // A block whose other end is NOT in this program is deliberately left alone: + // in a separate-shader-objects pipeline the interface it matches across lives + // in another program that never saw this plan, and renaming one side of THAT + // would break a program pipeline to repair a driver quirk. That is what the + // producer/consumer presence tests below are for - in a monolithic program + // both are trivially satisfied for every interface the collision can touch. + Vector uniquifiedIoBlockSpirv; + if (!collidingIoBlockNames.empty() && stagePipelineIndices[index] >= 0) { + const Int myPipelineIndex = stagePipelineIndices[index]; + Int producerPipelineIndex = -1; + Bool hasConsumerStage = false; + for (const Int otherPipelineIndex : stagePipelineIndices) { + if (otherPipelineIndex < 0) continue; + if (otherPipelineIndex < myPipelineIndex && + otherPipelineIndex > producerPipelineIndex) { + producerPipelineIndex = otherPipelineIndex; + } + if (otherPipelineIndex > myPipelineIndex) hasConsumerStage = true; + } + + std::map inputBlockRenames; + std::map outputBlockRenames; + for (const auto& collidingBlockName : collidingIoBlockNames) { + if (producerPipelineIndex >= 0) { + inputBlockRenames[collidingBlockName] = + uniqueIoBlockName(collidingBlockName, producerPipelineIndex); + } + if (hasConsumerStage) { + outputBlockRenames[collidingBlockName] = + uniqueIoBlockName(collidingBlockName, myPipelineIndex); + } + } + + std::set stageRenamedIoBlockNames; + if (MG_Util::ShaderTranspiler::ShaderCompiler::UniquifyIoBlockNamesForEssl( + *effectiveSpirv, inputBlockRenames, outputBlockRenames, + stageRenamedIoBlockNames, uniquifiedIoBlockSpirv, enableSpirvValidation) && + !uniquifiedIoBlockSpirv.empty() && !stageRenamedIoBlockNames.empty()) { + effectiveSpirv = &uniquifiedIoBlockSpirv; + MGLOG_D("Program %u stage %s: %zu inter-stage interface block(s) renamed per " + "producing stage, because some stage of this program declares the same " + "block name in both directions and the ES driver may alias the two.", + m_backendProgramId, + MG_Util::ConvertGLEnumToString(glShaderType).c_str(), + stageRenamedIoBlockNames.size()); + } + } + // 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 diff --git a/MobileGL/MG_IntegrationTest/CMakeLists.txt b/MobileGL/MG_IntegrationTest/CMakeLists.txt index 4616a9f8..d7d201fd 100644 --- a/MobileGL/MG_IntegrationTest/CMakeLists.txt +++ b/MobileGL/MG_IntegrationTest/CMakeLists.txt @@ -83,6 +83,7 @@ add_executable(MobileGLIntegrationTest Scenarios/ImageSizeAfterRespecScenario.cpp Scenarios/SsboDeclarationFormScenario.cpp Scenarios/Glsl420DeclarationScenario.cpp + Scenarios/IoBlockNameCollisionScenario.cpp Scenarios/FragmentOutputArrayIndexScenario.cpp Scenarios/BufferTextureScenario.cpp Scenarios/VertexAttribBindingScenario.cpp diff --git a/MobileGL/MG_IntegrationTest/Scenarios/IoBlockNameCollisionScenario.cpp b/MobileGL/MG_IntegrationTest/Scenarios/IoBlockNameCollisionScenario.cpp new file mode 100644 index 00000000..050a5d8e --- /dev/null +++ b/MobileGL/MG_IntegrationTest/Scenarios/IoBlockNameCollisionScenario.cpp @@ -0,0 +1,389 @@ +// MobileGL - MobileGL/MG_IntegrationTest/Scenarios/IoBlockNameCollisionScenario.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 - ONE BLOCK NAME USED IN BOTH DIRECTIONS BY ONE STAGE STILL CARRIES ITS PAYLOAD. +// +// Desktop GLSL keeps SEPARATE name namespaces for input and output interface blocks, so a +// single stage may legally write +// +// in TcsData { ... } tes_in[]; +// out TcsData { ... } tes_out; +// +// The tessellation evaluation stage of both interface-block tests in +// KHR-GL42/43.shading_language_420pack does exactly that, and MobileGL's backend used to +// hand the shape straight through: SPIRV-Cross splits the namespace the same way glslang +// does (block_input_names vs block_output_names) and re-emits BOTH blocks under the name +// TcsData, so the generated ESSL declares two different blocks of one name in one shader. +// Adreno's ES compiler keeps them apart. Mali's does not - the stage compiles, the program +// links, and the evaluation stage's writes never reach the geometry stage, which is all 22 +// of that group's Mali failures and none of Adreno's or DirectVulkan's. +// +// Both cases below drive the SAME five-stage pipeline (vertex -> tessellation control -> +// tessellation evaluation -> geometry -> fragment) and differ only in whether the +// evaluation stage reuses one name. The distinct-name case is the negative control: it is +// what says a red pixel in the colliding case is about the name and not about this machine's +// tessellation, its geometry stage, or the block mechanism in general. +// +// Colour code, so a failure names its own cause: +// green - the payload crossed all four stage boundaries, which is the pass. +// blue - the clear colour: nothing was drawn at all (the program did not link, or the +// backend program was rejected and every draw became a no-op). +// red - the pipeline ran but the plain (non-block) varying did not arrive, i.e. the +// failure is not about interface blocks. +// black - the pipeline ran, the plain varying arrived, and the BLOCK payload came back +// zeroed or garbage. That is the defect this scenario exists for. +// +// llvmpipe and lavapipe run this faithfully but do NOT reproduce the original defect - the +// aliasing is a Mali ES compiler behaviour. Read a green run here as "the rename did not +// break the ordinary path"; the claim it pins on the device is the CTS group above. + +#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 { + + // The payload starts here and is copied, unmodified, through every block below. + const char* const kVertexSource = R"(#version 420 core +out VsData { + vec4 payload; +} vs_out; +void main() +{ + vs_out.payload = vec4(0.0, 1.0, 0.0, 1.0); + gl_Position = vec4(0.0, 0.0, 0.0, 1.0); +} +)"; + + const char* const kTessControlSource = R"(#version 420 core +layout(vertices = 1) out; +in VsData { + vec4 payload; +} tcs_in[]; +out TcsData { + vec4 payload; +} tcs_out[]; +void main() +{ + tcs_out[gl_InvocationID].payload = tcs_in[gl_InvocationID].payload; + gl_TessLevelOuter[0] = 1.0; + gl_TessLevelOuter[1] = 1.0; + gl_TessLevelOuter[2] = 1.0; + gl_TessLevelOuter[3] = 1.0; + gl_TessLevelInner[0] = 1.0; + gl_TessLevelInner[1] = 1.0; +} +)"; + + // THE CASE UNDER TEST: one name, both directions, in one stage. + const char* const kCollidingTessEvalSource = R"(#version 420 core +layout(isolines, point_mode) in; +in TcsData { + vec4 payload; +} tes_in[]; +out TcsData { + vec4 payload; +} tes_out; +out float tes_gs_alive; +void main() +{ + tes_out.payload = tes_in[0].payload; + tes_gs_alive = 1.0; +} +)"; + + // The negative control: byte-identical but for the output block's name. + const char* const kDistinctTessEvalSource = R"(#version 420 core +layout(isolines, point_mode) in; +in TcsData { + vec4 payload; +} tes_in[]; +out TesData { + vec4 payload; +} tes_out; +out float tes_gs_alive; +void main() +{ + tes_out.payload = tes_in[0].payload; + tes_gs_alive = 1.0; +} +)"; + + // One geometry source per evaluation stage, because the block it consumes is named + // after the block the evaluation stage produced. + const char* const kCollidingGeometrySource = R"(#version 420 core +layout(points) in; +layout(triangle_strip, max_vertices = 4) out; +in TcsData { + vec4 payload; +} gs_in[]; +in float tes_gs_alive[]; +out GsData { + vec4 payload; +} gs_out; +out float gs_fs_alive; +void EmitCorner(vec2 corner) +{ + gs_out.payload = gs_in[0].payload; + gs_fs_alive = tes_gs_alive[0]; + gl_Position = vec4(corner, 0.0, 1.0); + EmitVertex(); +} +void main() +{ + EmitCorner(vec2(-1.0, -1.0)); + EmitCorner(vec2(-1.0, 1.0)); + EmitCorner(vec2( 1.0, -1.0)); + EmitCorner(vec2( 1.0, 1.0)); +} +)"; + + const char* const kDistinctGeometrySource = R"(#version 420 core +layout(points) in; +layout(triangle_strip, max_vertices = 4) out; +in TesData { + vec4 payload; +} gs_in[]; +in float tes_gs_alive[]; +out GsData { + vec4 payload; +} gs_out; +out float gs_fs_alive; +void EmitCorner(vec2 corner) +{ + gs_out.payload = gs_in[0].payload; + gs_fs_alive = tes_gs_alive[0]; + gl_Position = vec4(corner, 0.0, 1.0); + EmitVertex(); +} +void main() +{ + EmitCorner(vec2(-1.0, -1.0)); + EmitCorner(vec2(-1.0, 1.0)); + EmitCorner(vec2( 1.0, -1.0)); + EmitCorner(vec2( 1.0, 1.0)); +} +)"; + + // Red when the PLAIN varying did not arrive, so "the pipeline is broken" and "the + // block payload is broken" cannot be confused for one another. + const char* const kFragmentSource = R"(#version 420 core +in GsData { + vec4 payload; +} fs_in; +in float gs_fs_alive; +out vec4 fragColor; +void main() +{ + fragColor = gs_fs_alive > 0.5 ? fs_in.payload : vec4(1.0, 0.0, 0.0, 1.0); +} +)"; + + class IoBlockNameCollisionScenario : public ScenarioTest { + protected: + void SetUp() override { + ScenarioTest::SetUp(); + if (!Ready()) return; + glGenVertexArrays(1, &m_vao); + glBindVertexArray(m_vao); + if (!BackendHostsTessellationAndGeometry()) { + GTEST_SKIP() << "no tessellation/geometry stages on " << Gl().BackendName() << " (" + << Gl().RendererString() << "); there is no five-stage pipeline to " + << "carry a block through"; + } + } + + void TearDown() override { + if (!Ready()) return; + glUseProgram(0); + for (const GLuint program : m_programs) { + glDeleteProgram(program); + } + m_programs.clear(); + glBindVertexArray(0); + if (m_vao != 0) glDeleteVertexArrays(1, &m_vao); + m_vao = 0; + } + + // GL_MAX_TESS_GEN_LEVEL is a real backend answer, not a frontend constant: it + // reads 0 on a DirectGLES driver without GL_EXT_tessellation_shader and on a + // DirectVulkan device without the tessellationShader feature. There is no + // five-stage pipeline to assert about on such a stack. + static bool BackendHostsTessellationAndGeometry() { + GLint maxTessGenLevel = 0; + glGetIntegerv(GL_MAX_TESS_GEN_LEVEL, &maxTessGenLevel); + GLint maxGeometryOutputVertices = 0; + glGetIntegerv(GL_MAX_GEOMETRY_OUTPUT_VERTICES, &maxGeometryOutputVertices); + while (glGetError() != GL_NO_ERROR) { + } + return maxTessGenLevel >= 1 && maxGeometryOutputVertices >= 4; + } + + GLuint BuildPipeline(const char* tessEvalSource, const char* geometrySource) { + const GLenum stages[] = {GL_VERTEX_SHADER, GL_TESS_CONTROL_SHADER, + GL_TESS_EVALUATION_SHADER, GL_GEOMETRY_SHADER, + GL_FRAGMENT_SHADER}; + const char* const sources[] = {kVertexSource, kTessControlSource, tessEvalSource, + geometrySource, kFragmentSource}; + + GLuint shaders[5] = {0, 0, 0, 0, 0}; + bool ok = true; + for (int i = 0; i < 5; ++i) { + shaders[i] = glCreateShader(stages[i]); + glShaderSource(shaders[i], 1, &sources[i], nullptr); + glCompileShader(shaders[i]); + GLint compiled = 0; + glGetShaderiv(shaders[i], GL_COMPILE_STATUS, &compiled); + if (!compiled) { + m_buildLog = InfoLog(shaders[i], true); + ok = false; + break; + } + } + if (!ok) { + for (const GLuint shader : shaders) { + if (shader != 0) glDeleteShader(shader); + } + return 0; + } + + const GLuint program = glCreateProgram(); + for (const GLuint shader : shaders) { + glAttachShader(program, shader); + } + glLinkProgram(program); + GLint linked = 0; + glGetProgramiv(program, GL_LINK_STATUS, &linked); + for (const GLuint shader : shaders) { + glDeleteShader(shader); + } + if (!linked) { + m_buildLog = InfoLog(program, false); + glDeleteProgram(program); + return 0; + } + m_programs.push_back(program); + return program; + } + + // Clears to BLUE, so "the draw painted nothing" is a colour of its own rather + // than something that could be mistaken for a zeroed payload. + Rgba8 DrawAndReadCentre(GLuint program) const { + glViewport(0, 0, Gl().Width(), Gl().Height()); + glClearColor(0.0f, 0.0f, 1.0f, 1.0f); + glClear(GL_COLOR_BUFFER_BIT); + glUseProgram(program); + glPatchParameteri(GL_PATCH_VERTICES, 1); + glDrawArrays(GL_PATCHES, 0, 1); + + Rgba8 pixel{}; + glReadPixels(Gl().Width() / 2, Gl().Height() / 2, 1, 1, GL_RGBA, GL_UNSIGNED_BYTE, &pixel); + return pixel; + } + + static bool IsGreen(const Rgba8& pixel) { + return pixel.r < 64 && pixel.g > 192 && pixel.b < 64; + } + + const std::string& BuildLog() const { return m_buildLog; } + + static GLenum FirstGLError() { + const GLenum first = glGetError(); + while (glGetError() != GL_NO_ERROR) { + } + return first; + } + + private: + static std::string InfoLog(GLuint object, bool isShader) { + GLint length = 0; + if (isShader) { + glGetShaderiv(object, GL_INFO_LOG_LENGTH, &length); + } else { + glGetProgramiv(object, GL_INFO_LOG_LENGTH, &length); + } + std::vector log(static_cast(length > 1 ? length : 1), '\0'); + if (isShader) { + glGetShaderInfoLog(object, static_cast(log.size()), nullptr, log.data()); + } else { + glGetProgramInfoLog(object, static_cast(log.size()), nullptr, log.data()); + } + return std::string(log.data()); + } + + GLuint m_vao = 0; + std::vector m_programs; + std::string m_buildLog; + }; + + // The negative control, and it runs first on purpose: if this one is not green there + // is nothing to conclude from the case below it. + // + // It is also the CALIBRATION. GL_MAX_TESS_GEN_LEVEL answers for the tessellation + // stages honestly, but nothing MobileGL reports answers for the geometry stage the + // same way (GL_MAX_GEOMETRY_* are frontend constants and an ES driver may legitimately + // report zero geometry storage blocks while having geometry shaders), so a stack that + // cannot build a five-stage program at all is recognised here, by trying. + TEST_F(IoBlockNameCollisionScenario, DistinctlyNamedBlocksCarryThePayloadThroughFiveStages) { + if (!Ready()) return; + + const GLuint program = BuildPipeline(kDistinctTessEvalSource, kDistinctGeometrySource); + if (program == 0) { + GTEST_SKIP() << "this stack cannot build a five-stage tessellation+geometry program on " + << Gl().BackendName() << ", so there is no block to carry through: " + << BuildLog(); + } + + const Rgba8 centre = DrawAndReadCentre(program); + EXPECT_EQ(FirstGLError(), 0u); + EXPECT_TRUE(IsGreen(centre)) << "the control pipeline did not deliver its payload: " << centre; + } + + TEST_F(IoBlockNameCollisionScenario, OneBlockNameInBothDirectionsStillCarriesThePayload) { + if (!Ready()) return; + + // Same calibration as the case above, and for the same reason: a five-stage program + // this stack cannot build at all is not evidence about block names. Only once the + // DISTINCT-name build succeeds does a failure of the colliding one mean something. + if (BuildPipeline(kDistinctTessEvalSource, kDistinctGeometrySource) == 0) { + GTEST_SKIP() << "this stack cannot build a five-stage tessellation+geometry program on " + << Gl().BackendName() << ", so there is no block to carry through: " + << BuildLog(); + } + + // Legal desktop GLSL: input and output block names live in separate namespaces, so + // the evaluation stage below declares TcsData twice and must still compile. The + // control above having built is what makes this assertion about the NAME. + const GLuint program = BuildPipeline(kCollidingTessEvalSource, kCollidingGeometrySource); + ASSERT_NE(program, 0u) + << "an interface block name reused across the two directions of one stage is legal " + "desktop GLSL, but the program did not build: " + << BuildLog(); + + const Rgba8 centre = DrawAndReadCentre(program); + EXPECT_EQ(FirstGLError(), 0u); + EXPECT_TRUE(IsGreen(centre)) + << "the payload did not survive the stage that names its input and output block " + "the same: " + << centre << " (blue: nothing drew; red: the plain varying was lost too; black: " + "the block arrived empty)"; + } + + } // namespace +} // namespace MGITest diff --git a/MobileGL/MG_Test/ShaderTranspiler/CMakeLists.txt b/MobileGL/MG_Test/ShaderTranspiler/CMakeLists.txt index 29200a65..ea34ed21 100644 --- a/MobileGL/MG_Test/ShaderTranspiler/CMakeLists.txt +++ b/MobileGL/MG_Test/ShaderTranspiler/CMakeLists.txt @@ -9,6 +9,7 @@ add_executable( EmulateSubgroupsTest.cpp DemoteFloat64Test.cpp FlattenXfbInterfaceBlocksTest.cpp + UniquifyIoBlockNamesTest.cpp LowerViewportIndexTest.cpp ClampMultisampleFetchTest.cpp ) diff --git a/MobileGL/MG_Test/ShaderTranspiler/UniquifyIoBlockNamesTest.cpp b/MobileGL/MG_Test/ShaderTranspiler/UniquifyIoBlockNamesTest.cpp new file mode 100644 index 00000000..67cb654e --- /dev/null +++ b/MobileGL/MG_Test/ShaderTranspiler/UniquifyIoBlockNamesTest.cpp @@ -0,0 +1,262 @@ +// MobileGL - MobileGL/MG_Test/ShaderTranspiler/UniquifyIoBlockNamesTest.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 + +#include +#include +#include +#include + +#include "Includes.h" +#include "Init.h" +#include +#include +#include + +#include + +using namespace MobileGL; +using MobileGL::MG_Util::ShaderTranspiler::SessionUsageBit; +using MobileGL::MG_Util::ShaderTranspiler::ShaderCompiler; +using MobileGL::MG_Util::ShaderTranspiler::SpvcSession; + +namespace { + Vector CompileToSpirv(GLenum stage, const String& source) { + using namespace MG_Util::ShaderTranspiler; + ShaderAttrib shaderAttrib{.shaderType = stage, .sourceStr = source}; + auto shaderResult = ShaderCompiler::CompileShader(shaderAttrib); + EXPECT_TRUE(shaderResult) << (shaderResult ? String{} : shaderResult.error().log); + if (!shaderResult) return {}; + + ProgramAttrib programAttrib{.shaders = {shaderResult.value()}}; + auto programResult = ShaderCompiler::LinkProgram(programAttrib); + EXPECT_TRUE(programResult) << (programResult ? String{} : programResult.error().log); + if (!programResult) return {}; + + ProgramBinaryAttrib binaryAttrib{.shaderTypes = {stage}, .program = *programResult.value()}; + auto binaryResult = ShaderCompiler::GetSpirvBinaryFromProgram(binaryAttrib); + EXPECT_TRUE(binaryResult) << (binaryResult ? String{} : binaryResult.error().log); + if (!binaryResult || binaryResult->empty()) return {}; + return binaryResult->front(); + } + + String Disassemble(const Vector& spirv) { + spvtools::SpirvTools tools(SPV_ENV_VULKAN_1_1); + String text; + tools.Disassemble(spirv, &text); + return text; + } + + String Transpile(const Vector& spirv) { + SpvcSession session(spirv, SessionUsageBit::Transpile); + auto essl = ShaderCompiler::DecompileShader(session); + EXPECT_TRUE(essl) << (essl ? String{} : essl.error().log); + return essl ? essl.value() : String{}; + } + + // The tessellation evaluation stage of + // KHR-GL42/43.shading_language_420pack.length_of_vector_and_matrix_* and + // .qualifier_order_block_*, reduced to the shape that matters: ONE block name used for + // both the block this stage consumes and the block it produces. Legal desktop GLSL - the + // input and output block namespaces are separate - and something SPIRV-Cross re-emits + // verbatim, so the ESSL it produces declares two different blocks called TCSOutputBlock. + const char* kCollidingTessEvalSource = R"(#version 420 core +layout(isolines, point_mode) in; + +in vec4 tcs_tes_result[]; +out vec4 tes_gs_result; + +in TCSOutputBlock { + vec4 tcs_tes_variable; +} input_block[]; +out TCSOutputBlock { + vec4 tes_gs_variable; +} output_block; + +void main() +{ + tes_gs_result = tcs_tes_result[0]; + output_block.tes_gs_variable = input_block[0].tcs_tes_variable; +} +)"; + + // The same stage with the two blocks already named apart, which is the overwhelmingly + // common shape and the one that must go through untouched. + const char* kDistinctTessEvalSource = R"(#version 420 core +layout(isolines, point_mode) in; + +in vec4 tcs_tes_result[]; +out vec4 tes_gs_result; + +in TCSOutputBlock { + vec4 tcs_tes_variable; +} input_block[]; +out TESOutputBlock { + vec4 tes_gs_variable; +} output_block; + +void main() +{ + tes_gs_result = tcs_tes_result[0]; + output_block.tes_gs_variable = input_block[0].tcs_tes_variable; +} +)"; + + // gl_PerVertex is an Input block AND an Output block of one name in every tessellation + // and geometry stage. It is the language's block, not the shader's, so it must never be + // reported and never be renamed. + const char* kBuiltinBlockOnlyTessEvalSource = R"(#version 420 core +layout(isolines, point_mode) in; + +void main() +{ + gl_Position = gl_in[0].gl_Position; +} +)"; +} // namespace + +class UniquifyIoBlockNamesTest : public ::testing::Test { +protected: + void SetUp() override { + MobileGL::Initialize(); + m_validationFailuresAtStart = ShaderCompiler::SpirvValidationFailureCount(); + } + + void TearDown() override { + EXPECT_EQ(ShaderCompiler::SpirvValidationFailureCount(), m_validationFailuresAtStart) + << "the renamed module did not survive spirv-val"; + } + + Uint64 m_validationFailuresAtStart = 0; +}; + +TEST_F(UniquifyIoBlockNamesTest, ProbeReportsABlockNameUsedInBothDirections) { + const Vector input = CompileToSpirv(GL_TESS_EVALUATION_SHADER, kCollidingTessEvalSource); + ASSERT_FALSE(input.empty()); + + std::set colliding; + std::set declared; + ShaderCompiler::ProbeIoBlockNamesForEssl(input, colliding, declared); + + EXPECT_EQ(colliding, (std::set{"TCSOutputBlock"})); + // The name set the caller picks a replacement out of has to contain what the module + // already spells, or the replacement could land on top of an existing declaration. + EXPECT_NE(declared.find("TCSOutputBlock"), declared.end()); + EXPECT_NE(declared.find("input_block"), declared.end()); + EXPECT_NE(declared.find("output_block"), declared.end()); +} + +TEST_F(UniquifyIoBlockNamesTest, ProbeIgnoresAStageWhoseBlocksAlreadyHaveDistinctNames) { + const Vector input = CompileToSpirv(GL_TESS_EVALUATION_SHADER, kDistinctTessEvalSource); + ASSERT_FALSE(input.empty()); + + std::set colliding; + std::set declared; + ShaderCompiler::ProbeIoBlockNamesForEssl(input, colliding, declared); + + EXPECT_TRUE(colliding.empty()); + EXPECT_NE(declared.find("TCSOutputBlock"), declared.end()); +} + +TEST_F(UniquifyIoBlockNamesTest, ProbeNeverReportsTheBuiltinBlock) { + const Vector input = + CompileToSpirv(GL_TESS_EVALUATION_SHADER, kBuiltinBlockOnlyTessEvalSource); + ASSERT_FALSE(input.empty()); + + std::set colliding; + std::set declared; + ShaderCompiler::ProbeIoBlockNamesForEssl(input, colliding, declared); + + // gl_PerVertex is read through gl_in and written through gl_Position, i.e. it is exactly + // the in-and-out-under-one-name shape - and renaming it would invent a block no driver + // knows. + EXPECT_TRUE(colliding.empty()) << "gl_PerVertex must never enter the rename plan"; +} + +TEST_F(UniquifyIoBlockNamesTest, RenamesTheTwoBlocksApartInTheEmittedEssl) { + const Vector input = CompileToSpirv(GL_TESS_EVALUATION_SHADER, kCollidingTessEvalSource); + ASSERT_FALSE(input.empty()); + // The generated ESSL really does declare the block twice under one name before the fix - + // pinning the defect, not just the repair. + const String before = Transpile(input); + EXPECT_NE(before.find("in TCSOutputBlock"), String::npos) << before; + EXPECT_NE(before.find("out TCSOutputBlock"), String::npos) << before; + + // The plan the DirectGLES program build makes for a five-stage program: what this stage + // consumes is spelled after the tessellation control stage (pipeline index 1) and what it + // produces after itself (pipeline index 2). + const std::map inputRenames{{"TCSOutputBlock", "TCSOutputBlock_mgio1"}}; + const std::map outputRenames{{"TCSOutputBlock", "TCSOutputBlock_mgio2"}}; + + std::set renamed; + Vector output; + ASSERT_TRUE(ShaderCompiler::UniquifyIoBlockNamesForEssl(input, inputRenames, outputRenames, renamed, + output, true)); + ASSERT_FALSE(output.empty()); + EXPECT_EQ(renamed, (std::set{"TCSOutputBlock"})); + + const String dis = Disassemble(output); + spvtools::SpirvTools tools(SPV_ENV_VULKAN_1_1); + ASSERT_TRUE(tools.Validate(output)) << dis; + EXPECT_EQ(dis.find("\"TCSOutputBlock\""), String::npos) + << "the colliding name is still on a block struct:\n" + << dis; + EXPECT_NE(dis.find("\"TCSOutputBlock_mgio1\""), String::npos) << dis; + EXPECT_NE(dis.find("\"TCSOutputBlock_mgio2\""), String::npos) << dis; + + const String after = Transpile(output); + EXPECT_NE(after.find("TCSOutputBlock_mgio1"), String::npos) << after; + EXPECT_NE(after.find("TCSOutputBlock_mgio2"), String::npos) << after; + // Only the block TYPE name moves: the instance names are what the body reads and writes + // through, and the member names are half of what ES matches the interface by. + EXPECT_NE(after.find("input_block"), String::npos) << after; + EXPECT_NE(after.find("output_block"), String::npos) << after; + EXPECT_NE(after.find("tcs_tes_variable"), String::npos) << after; + EXPECT_NE(after.find("tes_gs_variable"), String::npos) << after; +} + +TEST_F(UniquifyIoBlockNamesTest, RenamesOnlyTheDirectionTheCallerPlanned) { + const Vector input = CompileToSpirv(GL_TESS_EVALUATION_SHADER, kCollidingTessEvalSource); + ASSERT_FALSE(input.empty()); + + // A separate-shader-objects program that ends at this stage plans no output rename, + // because the block's consumer lives in another program that never saw the plan. + const std::map inputRenames{{"TCSOutputBlock", "TCSOutputBlock_mgio1"}}; + + std::set renamed; + Vector output; + ASSERT_TRUE( + ShaderCompiler::UniquifyIoBlockNamesForEssl(input, inputRenames, {}, renamed, output, true)); + ASSERT_FALSE(output.empty()); + EXPECT_EQ(renamed, (std::set{"TCSOutputBlock"})); + + const String dis = Disassemble(output); + EXPECT_NE(dis.find("\"TCSOutputBlock_mgio1\""), String::npos) << dis; + // The output block keeps the name the other program still spells. + EXPECT_NE(dis.find("\"TCSOutputBlock\""), String::npos) << dis; + EXPECT_EQ(dis.find("\"TCSOutputBlock_mgio2\""), String::npos) << dis; +} + +TEST_F(UniquifyIoBlockNamesTest, ReportsNothingWhenThePlanNamesNoBlockThisStageDeclares) { + const Vector input = CompileToSpirv(GL_TESS_EVALUATION_SHADER, kDistinctTessEvalSource); + ASSERT_FALSE(input.empty()); + + const std::map renames{{"SomeOtherBlock", "SomeOtherBlock_mgio2"}}; + + std::set renamed; + Vector output; + ASSERT_TRUE(ShaderCompiler::UniquifyIoBlockNamesForEssl(input, renames, renames, renamed, output, true)); + // Empty is what tells the DirectGLES program build to keep the module it already had + // instead of adopting the optimizer's re-serialised copy. + EXPECT_TRUE(renamed.empty()); + + const String dis = Disassemble(output); + EXPECT_NE(dis.find("\"TCSOutputBlock\""), String::npos) << dis; + EXPECT_NE(dis.find("\"TESOutputBlock\""), String::npos) << dis; +} diff --git a/MobileGL/MG_Util/ShaderTranspiler/ShaderCompiler.cpp b/MobileGL/MG_Util/ShaderTranspiler/ShaderCompiler.cpp index 506c0931..5f64bf34 100644 --- a/MobileGL/MG_Util/ShaderTranspiler/ShaderCompiler.cpp +++ b/MobileGL/MG_Util/ShaderTranspiler/ShaderCompiler.cpp @@ -23,6 +23,7 @@ #include "SpirvPasses/LowerViewportIndexPass.h" #include "SpirvPasses/PackDoubleVertexInputsPass.h" #include "SpirvPasses/FlattenXfbInterfaceBlocksPass.h" +#include "SpirvPasses/UniquifyIoBlockNamesPass.h" #include "SpirvPasses/SplitArrayVertexInputsPass.h" #include "SpirvPasses/RebaseInstanceIndexPass.h" #include "SpirvPasses/ZeroBaseVertexPass.h" @@ -781,6 +782,42 @@ namespace MobileGL { outName); } + void ShaderCompiler::ProbeIoBlockNamesForEssl(const Vector& binary, + std::set& collidingBlockNames, + std::set& declaredNames) { + if (binary.empty()) { + // Same reasoning as ModuleDeclaresBufferTextureSampler: a stage that produced + // no SPIR-V has no block names to report, and parsing it would push a + // spurious diagnostic through the message consumer. + return; + } + std::unique_ptr context = spvtools::BuildModule( + SPV_ENV_VULKAN_1_1, MakeSpirvMessageConsumer("ProbeIoBlockNamesForEssl"), binary.data(), + binary.size()); + if (!context) { + // Unparseable here means unusable downstream too; let the ordinary transpile + // path produce the error rather than inventing a rename plan from it. + return; + } + UniquifyIoBlockNamesPass::ProbeIoBlockNames(context.get(), collidingBlockNames, declaredNames); + } + + bool ShaderCompiler::UniquifyIoBlockNamesForEssl(const Vector& inputBinary, + const std::map& inputBlockRenames, + const std::map& outputBlockRenames, + std::set& renamedBlockNames, + Vector& outputBinary, + const bool enableSpirvValidation) { + using namespace spvtools; + if (inputBlockRenames.empty() && outputBlockRenames.empty()) return false; + Optimizer optimizer(SPV_ENV_VULKAN_1_1); + optimizer.RegisterPass(UniquifyIoBlockNamesPass::CreateUniquifyIoBlockNamesPass( + inputBlockRenames, outputBlockRenames, &renamedBlockNames)); + + return RunOptimizerChecked("UniquifyIoBlockNamesForEssl", 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 94cc8ddd..ca27281c 100644 --- a/MobileGL/MG_Util/ShaderTranspiler/ShaderCompiler.h +++ b/MobileGL/MG_Util/ShaderTranspiler/ShaderCompiler.h @@ -12,6 +12,7 @@ #include "glslang/TVarEntryInfo.h" #include "glslang/TMglGlslIoResolver.h" +#include #include namespace MobileGL { @@ -101,6 +102,29 @@ namespace MobileGL { static bool RewriteXfbCaptureNameForFlattenedBlock(const String& captureName, const std::set& flattenedBlockNames, String& outName); + // Adds to `collidingBlockNames` every inter-stage interface block this stage + // declares in BOTH directions at once (`in FOO {...}; out FOO {...}`, which + // desktop GLSL allows because its input and output block namespaces are + // separate), and to `declaredNames` every name the module spells. The gate for + // UniquifyIoBlockNamesForEssl below, and the source of the name set a + // replacement has to avoid. Reads the module; never rewrites it. + static void ProbeIoBlockNamesForEssl(const Vector& binary, + std::set& collidingBlockNames, + std::set& declaredNames); + // Renames inter-stage interface BLOCK types so the collision the probe above + // found gets one spelling per producing stage. `inputBlockRenames` applies to + // blocks this stage consumes and `outputBlockRenames` to blocks it produces, + // both planned program-wide by the caller so a producer and its consumer keep + // matching; `renamedBlockNames` reports the original names this stage actually + // rewrote. SPIRV-Cross re-emits two same-named blocks verbatim and the Mali ES + // driver then loses the output block's payload. Only for the DirectGLES + // transpile path. See UniquifyIoBlockNamesPass. + static bool UniquifyIoBlockNamesForEssl(const Vector& inputBinary, + const std::map& inputBlockRenames, + const std::map& outputBlockRenames, + std::set& renamedBlockNames, + 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/UniquifyIoBlockNamesPass.cpp b/MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/UniquifyIoBlockNamesPass.cpp new file mode 100644 index 00000000..666c0613 --- /dev/null +++ b/MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/UniquifyIoBlockNamesPass.cpp @@ -0,0 +1,231 @@ +// MobileGL - MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/UniquifyIoBlockNamesPass.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 "UniquifyIoBlockNamesPass.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 "source/util/string_utils.h" + +#include +#include + +namespace MobileGL { + namespace MG_Util { + namespace ShaderTranspiler { + namespace { + using spvtools::opt::Instruction; + using spvtools::opt::IRContext; + + // Which storage classes a block struct is reachable from. A struct seen in both + // directions inside ONE module cannot be renamed per direction (there is only + // one name to change), so it is skipped rather than guessed at. + constexpr Uint32 kSeenAsInput = 1u; + constexpr Uint32 kSeenAsOutput = 2u; + + // Every struct type carrying the Block decoration, minus the ones with a builtin + // member (gl_PerVertex): those are named by the language, not by the shader, and + // renaming one would invent a block no driver knows. + 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 (uint32_t builtinStructId : builtinStructIds) { + blockStructIds.erase(builtinStructId); + } + return blockStructIds; + } + + // The block struct an Input/Output variable declares, or 0 when the variable is + // not an interface block of the kind this pass renames. Tessellation and geometry + // interfaces are arrays of the block struct, so one array level is unwrapped - + // the same shape StripUboMemberRelaxedPrecisionPass unwraps for instance-arrayed + // uniform blocks. + uint32_t GetInterfaceBlockStructId(IRContext* irContext, Instruction& variable, + const std::unordered_set& blockStructIds, + spv::StorageClass& outStorageClass) { + if (variable.opcode() != spv::Op::OpVariable) return 0; + const auto storageClass = + static_cast(variable.GetSingleWordInOperand(0)); + if (storageClass != spv::StorageClass::Input && + storageClass != spv::StorageClass::Output) { + return 0; + } + + auto* defUseMgr = irContext->get_def_use_mgr(); + Instruction* pointerType = defUseMgr->GetDef(variable.type_id()); + if (pointerType == nullptr || pointerType->opcode() != spv::Op::OpTypePointer) return 0; + 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 0; + if (blockStructIds.find(pointeeId) == blockStructIds.end()) return 0; + + outStorageClass = storageClass; + return pointeeId; + } + + String FindName(IRContext* irContext, uint32_t id) { + for (Instruction& debugInst : irContext->debugs2()) { + if (debugInst.opcode() != spv::Op::OpName) continue; + if (debugInst.GetSingleWordInOperand(0) != id) continue; + return debugInst.GetInOperand(1).AsString(); + } + return String(); + } + + // Replaces an EXISTING OpName only. A block struct with no name of its own is + // one SPIRV-Cross would spell from a fallback, which the consuming stage would + // not agree with anyway - leave it alone rather than invent a name for it. + Bool ReplaceExistingName(IRContext* irContext, uint32_t id, const String& newName) { + for (Instruction& debugInst : irContext->debugs2()) { + if (debugInst.opcode() != spv::Op::OpName) continue; + if (debugInst.GetSingleWordInOperand(0) != id) continue; + debugInst.SetInOperand( + 1, spvtools::utils::MakeVector(newName)); + return true; + } + return false; + } + + // Not a real id: "this name reached two different struct types in the same + // direction", which is already an illegal shader (glslang refuses to reuse a + // block name inside one interface) and which no rename could repair - two + // structs would come out with one new name. Both the probe and the rewrite + // decline it. + constexpr uint32_t kAmbiguousStructId = 0xffffffffu; + + // The module's interface blocks indexed the way both halves of this pass need + // them: by name within each direction, plus which directions each struct type + // is reached from. + struct IoBlockIndex { + std::map inputStructByName; + std::map outputStructByName; + std::unordered_map storageMaskByStructId; + }; + + IoBlockIndex IndexIoBlocks(IRContext* irContext, + const std::unordered_set& blockStructIds) { + IoBlockIndex index; + for (Instruction& variable : irContext->module()->types_values()) { + spv::StorageClass storageClass = spv::StorageClass::Input; + const uint32_t structId = + GetInterfaceBlockStructId(irContext, variable, blockStructIds, storageClass); + if (structId == 0) continue; + const Bool isInput = storageClass == spv::StorageClass::Input; + index.storageMaskByStructId[structId] |= isInput ? kSeenAsInput : kSeenAsOutput; + + const String blockName = FindName(irContext, structId); + if (blockName.empty()) continue; + std::map& byName = + isInput ? index.inputStructByName : index.outputStructByName; + const auto inserted = byName.emplace(blockName, structId); + if (!inserted.second && inserted.first->second != structId) { + inserted.first->second = kAmbiguousStructId; + } + } + return index; + } + } // namespace + + void UniquifyIoBlockNamesPass::ProbeIoBlockNames(spvtools::opt::IRContext* irContext, + std::set& outCollidingBlockNames, + std::set& outDeclaredNames) { + if (irContext == nullptr) return; + + for (Instruction& debugInst : irContext->debugs2()) { + if (debugInst.opcode() != spv::Op::OpName) continue; + outDeclaredNames.insert(debugInst.GetInOperand(1).AsString()); + } + + const std::unordered_set blockStructIds = CollectUserBlockStructIds(irContext); + if (blockStructIds.empty()) return; + + const IoBlockIndex index = IndexIoBlocks(irContext, blockStructIds); + for (const auto& input : index.inputStructByName) { + const auto output = index.outputStructByName.find(input.first); + if (output == index.outputStructByName.end()) continue; + if (input.second == kAmbiguousStructId || output->second == kAmbiguousStructId) continue; + // Same struct type on both sides: there is one name to rename and two + // directions wanting different ones, so the collision cannot be repaired. + if (input.second == output->second) continue; + outCollidingBlockNames.insert(input.first); + } + } + + spvtools::opt::Pass::Status UniquifyIoBlockNamesPass::Process() { + if (m_inputBlockRenames.empty() && m_outputBlockRenames.empty()) { + return Status::SuccessWithoutChange; + } + + auto* irContext = context(); + const std::unordered_set blockStructIds = CollectUserBlockStructIds(irContext); + if (blockStructIds.empty()) return Status::SuccessWithoutChange; + + // Indexed BEFORE anything is renamed, so every decline below is decided against + // the names the module arrived with rather than against a half-renamed one. + const IoBlockIndex index = IndexIoBlocks(irContext, blockStructIds); + + Bool modified = false; + for (int direction = 0; direction < 2; ++direction) { + const Bool isInput = direction == 0; + const std::map& byName = + isInput ? index.inputStructByName : index.outputStructByName; + const std::map& renames = + isInput ? m_inputBlockRenames : m_outputBlockRenames; + const Uint32 wantedMask = isInput ? kSeenAsInput : kSeenAsOutput; + + for (const auto& block : byName) { + if (block.second == kAmbiguousStructId) continue; + const auto rename = renames.find(block.first); + if (rename == renames.end()) continue; + if (rename->second.empty() || rename->second == block.first) continue; + // A struct type reached from BOTH directions carries one name for two + // interfaces, so renaming it for this direction would rename it for the + // other one too. Leave the module as it was. + const auto mask = index.storageMaskByStructId.find(block.second); + if (mask == index.storageMaskByStructId.end() || mask->second != wantedMask) continue; + if (!ReplaceExistingName(irContext, block.second, rename->second)) continue; + + if (m_renamedBlockNames != nullptr) m_renamedBlockNames->insert(block.first); + modified = true; + } + } + + return modified ? Status::SuccessWithChange : Status::SuccessWithoutChange; + } + + spvtools::Optimizer::PassToken UniquifyIoBlockNamesPass::CreateUniquifyIoBlockNamesPass( + const std::map& inputBlockRenames, + const std::map& outputBlockRenames, std::set* renamedBlockNames) { + return spvtools::Optimizer::PassToken(MakeUnique( + inputBlockRenames, outputBlockRenames, renamedBlockNames)); + } + } // namespace ShaderTranspiler + } // namespace MG_Util +} // namespace MobileGL diff --git a/MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/UniquifyIoBlockNamesPass.h b/MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/UniquifyIoBlockNamesPass.h new file mode 100644 index 00000000..11e0594e --- /dev/null +++ b/MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/UniquifyIoBlockNamesPass.h @@ -0,0 +1,90 @@ +// MobileGL - MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/UniquifyIoBlockNamesPass.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 +#include + +namespace MobileGL { + namespace MG_Util { + namespace ShaderTranspiler { + // Renames the STRUCT of an inter-stage interface block, so a block name a stage + // declares in both directions at once gets one spelling per producing stage. + // + // WHY. Desktop GLSL keeps SEPARATE name namespaces for input and output interface + // blocks, so a single stage may legally write + // + // in TCSOutputBlock { ... } input_block[]; + // out TCSOutputBlock { ... } output_block; + // + // which is exactly what the tessellation evaluation stage of + // KHR-GL42/43.shading_language_420pack.length_of_vector_and_matrix_* and + // .qualifier_order_block_* does. glslang accepts it deliberately (ParseHelper + // errors only when the two share a storage qualifier) and SPIRV-Cross re-emits + // BOTH under the name TCSOutputBlock, because it too splits the namespace + // (block_input_names vs block_output_names). The generated ESSL 3.20 then declares + // two different blocks called TCSOutputBlock in one shader. Adreno's ES compiler + // keeps them apart; Mali's does not - the stage compiles, the program links, and + // the output block's payload never reaches the next stage, which is all 22 of + // that group's Mali failures and none of Adreno's or DirectVulkan's. + // + // WHAT. The rename is planned program-wide by the CALLER and keyed on the + // PRODUCING stage, so a producer and its consumer keep naming the same block: + // the tessellation control stage's `out TCSOutputBlock` and the evaluation + // stage's `in TCSOutputBlock` both become _mgio, while the evaluation + // stage's own `out TCSOutputBlock` and the geometry stage's `in TCSOutputBlock` + // both become _mgio. Only the block TYPE name changes; instance names, + // member names, locations and every decoration are left exactly as they were, and + // ES matches inter-stage blocks by block name plus member sequence. + // + // DirectGLES only: DirectVulkan hands the module to the driver as SPIR-V, where + // the two blocks are distinct type ids and the debug names carry no meaning. + class UniquifyIoBlockNamesPass : public spvtools::opt::Pass { + public: + // `inputBlockRenames` applies to blocks this stage CONSUMES and + // `outputBlockRenames` to blocks it PRODUCES, both keyed by the block's + // current name. `renamedBlockNames` receives the ORIGINAL names this stage + // actually rewrote, so the caller can adopt the re-serialised module only + // when there was something to rewrite. + UniquifyIoBlockNamesPass(const std::map& inputBlockRenames, + const std::map& outputBlockRenames, + std::set* renamedBlockNames) + : m_inputBlockRenames(inputBlockRenames), m_outputBlockRenames(outputBlockRenames), + m_renamedBlockNames(renamedBlockNames) {} + + const char* name() const override { return "mobilegl-uniquify-io-block-names"; } + Status Process() override; + + // Reads a module WITHOUT rewriting it, for the caller's gate. Adds to + // `outCollidingBlockNames` every block name this module declares in BOTH Input + // and Output storage under two DIFFERENT struct types - the only shape the + // rename above can repair - and to `outDeclaredNames` every name the module + // spells, so the caller can pick a replacement that collides with none of them. + // Builtin blocks (gl_PerVertex and friends) are never reported. + static void ProbeIoBlockNames(spvtools::opt::IRContext* irContext, + std::set& outCollidingBlockNames, + std::set& outDeclaredNames); + + static spvtools::Optimizer::PassToken CreateUniquifyIoBlockNamesPass( + const std::map& inputBlockRenames, + const std::map& outputBlockRenames, + std::set* renamedBlockNames); + + private: + std::map m_inputBlockRenames; + std::map m_outputBlockRenames; + std::set* m_renamedBlockNames = nullptr; + }; + } // namespace ShaderTranspiler + } // namespace MG_Util +} // namespace MobileGL