From f20b20e64334706d38eb3dda850437cc4807d723 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Thu, 13 Aug 2026 01:28:35 -0400 Subject: [PATCH 1/2] [Fix, Feat, Test] (MG_Backend/DirectVulkan, MG_Test): synthesize the pass-through tessellation control stage GL gives an evaluation-only program, and refuse the half-tessellated pipeline Mali dereferences null inside --- .../DirectVulkan/Renderer/PipelineFactory.cpp | 50 +++- .../DirectVulkan/Renderer/PipelineFactory.h | 11 + .../DirectVulkan/Renderer/ProgramFactory.cpp | 232 ++++++++++++++++ .../DirectVulkan/Renderer/ProgramFactory.h | 61 ++++- .../DirectVulkan/Renderer/VulkanRenderer.cpp | 20 ++ MobileGL/MG_Test/Pipeline/CMakeLists.txt | 1 + .../Pipeline/PassthroughTessControlTest.cpp | 247 ++++++++++++++++++ 7 files changed, 619 insertions(+), 3 deletions(-) create mode 100644 MobileGL/MG_Test/Pipeline/PassthroughTessControlTest.cpp diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/PipelineFactory.cpp b/MobileGL/MG_Backend/DirectVulkan/Renderer/PipelineFactory.cpp index 85e3d2d1..ef1f235a 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/PipelineFactory.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/PipelineFactory.cpp @@ -471,9 +471,55 @@ namespace MobileGL::MG_Backend::DirectVulkan { blend.attachmentCount = payload.colorAttachmentCount; blend.pAttachments = colorAttachments.empty() ? nullptr : colorAttachments.data(); + // A GL program may have a tessellation EVALUATION stage and no CONTROL stage: GL 4.6 core + // 11.2.2 gives it a fixed-function pass-through instead. Vulkan has no such stage, and + // VUID-VkGraphicsPipelineCreateInfo-pStages-00730 requires both tessellation stages or + // neither - so the renderer synthesizes the pass-through GL describes and hands it in + // here (see ProgramFactory::GetOrCreatePassthroughTessControlStage). + // + // The refusal below is what keeps the half-tessellated shape away from the driver when + // there is no synthesized stage to add - because Mali does not reject it, it dereferences + // null INSIDE vkCreateGraphicsPipelines and takes the process down (SIGSEGV, fault addr + // 0x34, on Mali-G715/r54p2 and Mali-G925/r49p1 alike; Adreno and lavapipe merely render + // wrong). Returning VK_NULL_HANDLE routes this through the same path a driver rejection + // takes: the draw is skipped, nothing is memoised, and the process survives. + const Vector* effectiveStages = payload.stages; + Vector stagesWithPassthrough; + if (payload.passthroughTessControlStage.module != VK_NULL_HANDLE) { + stagesWithPassthrough = *payload.stages; + stagesWithPassthrough.push_back(payload.passthroughTessControlStage); + effectiveStages = &stagesWithPassthrough; + } + { + VkShaderStageFlags stagesPresent = 0; + for (const auto& stageInfo : *effectiveStages) { + stagesPresent |= stageInfo.stage; + } + const Bool hasTessControl = (stagesPresent & VK_SHADER_STAGE_TESSELLATION_CONTROL_BIT) != 0; + const Bool hasTessEval = (stagesPresent & VK_SHADER_STAGE_TESSELLATION_EVALUATION_BIT) != 0; + if (hasTessControl != hasTessEval) { + // MGLOG_I, and latched: _E is compiled out of the INFO-level builds CTS and the + // shipping app run, which is exactly where this refusal is the only explanation + // for a missing draw. Latched because failures are deliberately not memoised - a + // program in this state re-enters here once per draw, every frame. + static Bool s_warnedHalfTessellatedPipeline = false; + if (!s_warnedHalfTessellatedPipeline) { + s_warnedHalfTessellatedPipeline = true; + MGLOG_I("PipelineFactory::CreatePipeline: refusing a pipeline with %s tessellation stage and " + "no %s stage (VUID-VkGraphicsPipelineCreateInfo-pStages-00730). programHash=0x%llx " + "patchControlPoints=%u. Its draws are skipped; logged once.", + hasTessEval ? "an evaluation" : "a control", + hasTessEval ? "control" : "evaluation", + static_cast(payload.programHash), + payload.patchControlPoints); + } + return VK_NULL_HANDLE; + } + } + VkGraphicsPipelineCreateInfo gpi{VK_STRUCTURE_TYPE_GRAPHICS_PIPELINE_CREATE_INFO}; - gpi.stageCount = static_cast(payload.stages->size()); - gpi.pStages = payload.stages->data(); + gpi.stageCount = static_cast(effectiveStages->size()); + gpi.pStages = effectiveStages->data(); gpi.pVertexInputState = payload.vertexInputState; gpi.pInputAssemblyState = &ia; gpi.pTessellationState = diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/PipelineFactory.h b/MobileGL/MG_Backend/DirectVulkan/Renderer/PipelineFactory.h index 77948c4c..3feb7cd0 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/PipelineFactory.h +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/PipelineFactory.h @@ -71,6 +71,17 @@ namespace MobileGL::MG_Backend::DirectVulkan { Bool fragmentReplacesDepth = false; Array colorBlendAttachments{}; const Vector* stages = nullptr; + // The tessellation control stage this renderer synthesized for a program that has + // an evaluation stage and none of its own (GL 4.6 core 11.2.2 gives such a program a + // fixed-function pass-through; Vulkan has no such thing and + // VUID-VkGraphicsPipelineCreateInfo-pStages-00730 forbids the half-tessellated + // pipeline outright). Appended to `stages` at creation. A null module means the + // renderer could not build one, and CreatePipeline refuses the pipeline - the same + // refusal it applies when `stages` itself is half-tessellated. + // + // NOT hashed: it is a pure function of the program and of patchControlPoints, both + // of which ComputeHash already mixes in. + VkPipelineShaderStageCreateInfo passthroughTessControlStage{}; const VkPipelineVertexInputStateCreateInfo* vertexInputState = nullptr; // Diagnostic only; may be null. Read solely from the pipeline-creation failure path. const Vector* stageSpirvDigests = nullptr; diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/ProgramFactory.cpp b/MobileGL/MG_Backend/DirectVulkan/Renderer/ProgramFactory.cpp index 0806f93e..ebf09c36 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/ProgramFactory.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/ProgramFactory.cpp @@ -3190,6 +3190,7 @@ namespace MobileGL::MG_Backend::DirectVulkan { #endif ReflectVertexInputs(shaders, moduleSpirvs, entry); ReflectFragmentOutputs(shaders, moduleSpirvs, entry); + ReflectPassthroughTessControlNeed(shaders, moduleSpirvs, entry); ReflectLayout(program, moduleSpirvs, entry); // A failed remap means the modules kept glslang's per-stage auto-mapped binding numbers - // no cross-stage unification, no set->0 normalisation - so the bindings this layout @@ -3247,4 +3248,235 @@ namespace MobileGL::MG_Backend::DirectVulkan { } } } + + ProgramFactory::~ProgramFactory() { + for (auto& entry : m_passthroughTessControlStages) { + if (entry.second.module != VK_NULL_HANDLE) { + vkDestroyShaderModule(m_device, entry.second.module, nullptr); + } + } + } + + String ProgramFactory::BuildPassthroughTessControlSource(Uint32 patchVertices) { + // The stage GL 4.6 core 11.2.2 describes when a program has an evaluation shader and no + // control shader: "the input patch is passed through unmodified", the output patch has + // as many vertices as the input one (PATCH_VERTICES), and the levels come from the + // PATCH_DEFAULT_OUTER_LEVEL / PATCH_DEFAULT_INNER_LEVEL state. + // + // Those two levels default to 1.0 and are baked here as literals because + // glPatchParameterfv - their only setter - is not implemented in this frontend (it is a + // stub in MG_Impl/GLImpl/Exporting/Definitions.cpp). Implementing that entry point means + // making the levels a parameter of this source AND of the cache key in + // GetOrCreatePassthroughTessControlStage; the two must move together, so they are named + // together here. + // + // gl_out carries gl_Position and nothing else on purpose. The evaluation stage that + // reads it was linked against the VERTEX stage directly, so its input gl_PerVertex holds + // exactly the built-ins that stage used, and its user-defined inputs (if any) come + // straight off the vertex stage's outputs - which a control stage sitting in between + // would leave unwritten. ReflectPassthroughTessControlNeed refuses those programs rather + // than let this write a partial interface. + // + // All four outer levels and both inner levels are written unconditionally: writing a + // level the evaluation stage's domain does not use is legal and ignored, and it saves + // this from having to know the domain. + String source = "#version 450 core\n"; + source += "layout(vertices = " + std::to_string(patchVertices) + ") out;\n"; + // gl_in and gl_out are redeclared to the exact gl_PerVertex the FRONTEND's linked programs + // carry - gl_Position, gl_PointSize, gl_ClipDistance[1], in that order - because Vulkan + // matches built-in interface blocks by their whole shape, and the two obvious spellings + // are both wrong: + // * narrowing the block to gl_Position alone makes the evaluation stage read a patch of + // zeroes (degenerate triangles, nothing rasterized), and + // * taking glslang's DEFAULT block for a standalone control stage yields FOUR members - + // it appends gl_CullDistance - where a linked vertex+evaluation program has three. + // PassthroughTessControlTest.MatchesTheFrontendPerVertexBlock is the latch: it links a + // vertex+evaluation program through this same compiler and fails if the two shapes ever + // stop agreeing, rather than letting the mismatch show up as a black frame. + // + // Only gl_Position is written. gl_PointSize is declared but left alone deliberately: + // writing it from a tessellation stage requires the shaderTessellationAndGeometryPointSize + // feature, which this renderer does not enable, so a program whose evaluation stage reads + // gl_in[].gl_PointSize gets an undefined point size instead of the vertex stage's - a gap + // this trades for not making every tessellated pipeline depend on an optional feature. + source += "in gl_PerVertex {\n" + " vec4 gl_Position;\n" + " float gl_PointSize;\n" + " float gl_ClipDistance[1];\n" + "} gl_in[gl_MaxPatchVertices];\n"; + source += "out gl_PerVertex {\n" + " vec4 gl_Position;\n" + " float gl_PointSize;\n" + " float gl_ClipDistance[1];\n" + "} gl_out[];\n"; + source += "void main() {\n"; + source += " gl_out[gl_InvocationID].gl_Position = gl_in[gl_InvocationID].gl_Position;\n"; + source += " gl_TessLevelOuter[0] = 1.0;\n"; + source += " gl_TessLevelOuter[1] = 1.0;\n"; + source += " gl_TessLevelOuter[2] = 1.0;\n"; + source += " gl_TessLevelOuter[3] = 1.0;\n"; + source += " gl_TessLevelInner[0] = 1.0;\n"; + source += " gl_TessLevelInner[1] = 1.0;\n"; + source += "}\n"; + return source; + } + + VkPipelineShaderStageCreateInfo ProgramFactory::GetOrCreatePassthroughTessControlStage(Uint32 patchVertices) { + // A cached VK_NULL_HANDLE is a remembered failure, not a miss: returning it keeps a + // generator that cannot compile from re-running glslang on every draw. + const auto cached = m_passthroughTessControlStages.find(patchVertices); + if (cached != m_passthroughTessControlStages.end()) { + return cached->second; + } + + VkPipelineShaderStageCreateInfo stage{VK_STRUCTURE_TYPE_PIPELINE_SHADER_STAGE_CREATE_INFO}; + stage.stage = VK_SHADER_STAGE_TESSELLATION_CONTROL_BIT; + stage.module = VK_NULL_HANDLE; + stage.pName = "main"; + + using namespace MG_Util::ShaderTranspiler; + const String source = BuildPassthroughTessControlSource(patchVertices); + // Same compile configuration as every other stage of every other program: this runs on + // the GL thread (the draw path), so the live compile env is the right one, and flags=0 + // is the Vulkan-targeting form (CompileForOpenGL is what the GLES backend adds). + const SharedPtr& env = GetCurrentCompileEnv(); + ShaderAttrib shaderAttrib{.shaderType = GL_TESS_CONTROL_SHADER, + .sourceStr = source, + .flags = 0, + .env = env.get()}; + auto compiled = ShaderCompiler::CompileShader(shaderAttrib); + if (!compiled) { + MGLOG_I("ProgramFactory: could not compile the pass-through tessellation control stage for " + "patchVertices=%u; a program with an evaluation stage and no control stage cannot draw. %s", + patchVertices, compiled.error().log.c_str()); + m_passthroughTessControlStages.emplace(patchVertices, stage); + return stage; + } + + ProgramAttrib programAttrib{}; + programAttrib.shaders.push_back(compiled.value()); + auto linked = ShaderCompiler::LinkProgram(programAttrib); + if (!linked) { + MGLOG_I("ProgramFactory: could not link the pass-through tessellation control stage for " + "patchVertices=%u. %s", patchVertices, linked.error().log.c_str()); + m_passthroughTessControlStages.emplace(patchVertices, stage); + return stage; + } + + ProgramBinaryAttrib binaryAttrib{.shaderTypes = {GL_TESS_CONTROL_SHADER}, .program = *linked.value()}; + auto binary = ShaderCompiler::GetSpirvBinaryFromProgram(binaryAttrib); + if (!binary || binary.value().empty() || binary.value().front().empty()) { + MGLOG_I("ProgramFactory: could not generate SPIR-V for the pass-through tessellation control stage " + "for patchVertices=%u", patchVertices); + m_passthroughTessControlStages.emplace(patchVertices, stage); + return stage; + } + + const Vector& spirv = binary.value().front(); +#if MOBILEGL_LOG_ACTIVE_LEVEL <= MOBILEGL_LOG_LEVEL_DEBUG + ValidateTransformedSpirv(spirv, ShaderStage::TessControl, 0); +#else + if (MG_Util::ShaderTranspiler::ShaderCompiler::SpirvValidationEnabled()) { + ValidateTransformedSpirv(spirv, ShaderStage::TessControl, 0); + } +#endif + + VkShaderModuleCreateInfo smci{VK_STRUCTURE_TYPE_SHADER_MODULE_CREATE_INFO}; + smci.codeSize = spirv.size() * sizeof(Uint); + smci.pCode = spirv.data(); + VkShaderModule module = VK_NULL_HANDLE; + const VkResult result = vkCreateShaderModule(m_device, &smci, nullptr, &module); + if (result != VK_SUCCESS) { + MGLOG_I("ProgramFactory: vkCreateShaderModule failed (%d) for the pass-through tessellation control " + "stage for patchVertices=%u", static_cast(result), patchVertices); + m_passthroughTessControlStages.emplace(patchVertices, stage); + return stage; + } + + stage.module = module; + MGLOG_I("ProgramFactory: built the pass-through tessellation control stage for patchVertices=%u " + "(GL 4.6 11.2.2; Vulkan has no fixed-function equivalent)", patchVertices); + m_passthroughTessControlStages.emplace(patchVertices, stage); + return stage; + } + + void ProgramFactory::ReflectPassthroughTessControlNeed( + const Vector>& shaders, + const Vector>& spirv, + VkProgramObject& entry) const { + entry.needsPassthroughTessControl = false; + entry.passthroughTessControlEmulatable = false; + + Bool hasTessEval = false; + Bool hasTessControl = false; + SizeT tessEvalModuleIndex = 0; + for (SizeT i = 0; i < shaders.size(); ++i) { + if (!shaders[i]) continue; + const auto stage = shaders[i]->GetShaderStage(); + if (stage == ShaderStage::TessControl) hasTessControl = true; + if (stage == ShaderStage::TessEval) { + hasTessEval = true; + tessEvalModuleIndex = i; + } + } + if (!hasTessEval || hasTessControl) return; + + entry.needsPassthroughTessControl = true; + + if (tessEvalModuleIndex >= spirv.size() || spirv[tessEvalModuleIndex].empty()) return; + const auto& module = spirv[tessEvalModuleIndex]; + + SpvReflectShaderModule reflectModule{}; + const SpvReflectResult createResult = + spvReflectCreateShaderModule(module.size() * sizeof(Uint), module.data(), &reflectModule); + if (createResult != SPV_REFLECT_RESULT_SUCCESS) { + MGLOG_I("ProgramFactory::ReflectPassthroughTessControlNeed: reflection failed (result=%d); the " + "evaluation stage's inputs are unknown, so the pass-through is not offered", + static_cast(createResult)); + return; + } + + uint32_t inputCount = 0; + SpvReflectResult reflectResult = spvReflectEnumerateInputVariables(&reflectModule, &inputCount, nullptr); + Vector inputs(inputCount); + if (reflectResult == SPV_REFLECT_RESULT_SUCCESS && inputCount > 0) { + reflectResult = spvReflectEnumerateInputVariables(&reflectModule, &inputCount, inputs.data()); + } + if (reflectResult != SPV_REFLECT_RESULT_SUCCESS) { + spvReflectDestroyShaderModule(&reflectModule); + return; + } + + // The question is only ever "does this stage read anything a control stage would have to + // forward", and the answer is: does it have a LOCATION. A located input is a user-defined + // varying (or a per-patch input), which the vertex stage writes today and would stop + // reaching once a control stage sits in between - the pass-through carries gl_Position and + // nothing else, so such a program is declined instead of being handed undefined values. + // Everything without a location is a built-in: gl_in, gl_TessCoord, gl_PatchVerticesIn, + // gl_PrimitiveID, gl_TessLevel*, all either forwarded or generated for the evaluation + // stage by the tessellator itself. + // + // This deliberately does NOT judge on SpvReflectInterfaceVariable::built_in. gl_in is an + // array of interface blocks, and for those SPIRV-Reflect reports built_in == -1 on the + // block AND leaves every member's built_in at 0 - which is SpvBuiltInPosition, so a + // member walk reads "Position, Position, Position" for a {Position, PointSize, + // ClipDistance} block and would accept anything on the strength of parse garbage. The + // location, by contrast, is decorated on the OpVariable and is what SPIRV-Reflect reads + // straight through. + constexpr Uint32 kNoLocation = 0xFFFFFFFFu; + Bool emulatable = true; + for (auto* input : inputs) { + if (input == nullptr) continue; + if (input->location == kNoLocation) continue; + MGLOG_I("ProgramFactory: a tessellation evaluation stage with no control stage reads the " + "user-defined input '%s' at location=%u; a synthesized control stage cannot forward it, so " + "this program's draws are declined rather than fed an undefined varying", + input->name != nullptr ? input->name : "", input->location); + emulatable = false; + break; + } + + spvReflectDestroyShaderModule(&reflectModule); + entry.passthroughTessControlEmulatable = emulatable; + } } // namespace MobileGL::MG_Backend::DirectVulkan diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/ProgramFactory.h b/MobileGL/MG_Backend/DirectVulkan/Renderer/ProgramFactory.h index 2dcca478..d37f1661 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/ProgramFactory.h +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/ProgramFactory.h @@ -151,6 +151,22 @@ namespace MobileGL::MG_Backend::DirectVulkan { // PROGRAM rather than of the variant: the zeroed variant leaves the variable // declared, so both variants answer the same and the draw path can ask either. Bool readsBaseVertexBuiltin = false; + // This program has a tessellation EVALUATION stage and no tessellation CONTROL + // stage. GL allows that (4.6 core 11.2.2: with no control shader the input patch + // is passed through unmodified, the output patch size is PATCH_VERTICES, and the + // levels come from the PATCH_DEFAULT_*_LEVEL state); Vulkan does not - either both + // tessellation stages are present or neither + // (VUID-VkGraphicsPipelineCreateInfo-pStages-00730). So the draw path has to supply + // the pass-through stage GL describes; see GetOrCreatePassthroughTessControlStage. + Bool needsPassthroughTessControl = false; + // ...and the pass-through this renderer can synthesize carries gl_Position and + // nothing else, so it is only correct when the evaluation stage's inputs are + // built-ins. A user-defined varying would arrive at the evaluation stage + // UNWRITTEN once a control stage sits between it and the vertex stage, which is + // silently wrong pixels rather than a crash - so those programs are declined + // instead (PipelineFactory::CreatePipeline refuses the pipeline and the draw is + // skipped). See ReflectPassthroughTessControlNeed. + Bool passthroughTessControlEmulatable = false; // Frame-boundary counter value of the last GetOrCreateProgram hit; drives // cache eviction (see OnFrameBoundary). Mutable: the draw snapshot's memoised // entry pointer re-stamps use through a const reference (StampProgramUse). @@ -202,6 +218,8 @@ namespace MobileGL::MG_Backend::DirectVulkan { fragmentInputComponentCount = other.fragmentInputComponentCount; fragmentReplacesDepth = other.fragmentReplacesDepth; readsBaseVertexBuiltin = other.readsBaseVertexBuiltin; + needsPassthroughTessControl = other.needsPassthroughTessControl; + passthroughTessControlEmulatable = other.passthroughTessControlEmulatable; lastUsedFrame = other.lastUsedFrame; other.hash = 0; other.descriptorSetLayout = VK_NULL_HANDLE; @@ -216,6 +234,8 @@ namespace MobileGL::MG_Backend::DirectVulkan { other.fragmentInputComponentCount = 0; other.fragmentReplacesDepth = false; other.readsBaseVertexBuiltin = false; + other.needsPassthroughTessControl = false; + other.passthroughTessControlEmulatable = false; other.lastUsedFrame = 0; } VkProgramObject& operator=(VkProgramObject&& other) noexcept { @@ -256,6 +276,8 @@ namespace MobileGL::MG_Backend::DirectVulkan { fragmentInputComponentCount = other.fragmentInputComponentCount; fragmentReplacesDepth = other.fragmentReplacesDepth; readsBaseVertexBuiltin = other.readsBaseVertexBuiltin; + needsPassthroughTessControl = other.needsPassthroughTessControl; + passthroughTessControlEmulatable = other.passthroughTessControlEmulatable; lastUsedFrame = other.lastUsedFrame; other.hash = 0; other.descriptorSetLayout = VK_NULL_HANDLE; @@ -270,6 +292,8 @@ namespace MobileGL::MG_Backend::DirectVulkan { other.fragmentInputComponentCount = 0; other.fragmentReplacesDepth = false; other.readsBaseVertexBuiltin = false; + other.needsPassthroughTessControl = false; + other.passthroughTessControlEmulatable = false; other.lastUsedFrame = 0; return *this; } @@ -321,7 +345,10 @@ namespace MobileGL::MG_Backend::DirectVulkan { m_unformattedFloatStorageImagesEnabled(unformattedFloatStorageImagesEnabled) { VkProgramObject::s_device = device; } - ~ProgramFactory() = default; + // Destroys the pass-through tessellation control modules. Runs while the device is + // still alive for the same reason ~VkProgramObject's does: this factory outlives + // nothing that owns the device. + ~ProgramFactory(); ProgramFactory(const ProgramFactory&) = delete; HashType ComputeHash(const MG_State::GLState::ProgramObject& program, CompileOptionFlags flags) const; @@ -374,6 +401,27 @@ namespace MobileGL::MG_Backend::DirectVulkan { // this builtin? static Bool ReflectedDeclaresInputBuiltin(const SpvReflectShaderModule& reflectModule, SpvBuiltIn builtin); + // The pass-through tessellation control stage GL 4.6 core 11.2.2 describes for a + // program that has an evaluation stage and no control stage, for an input patch of + // `patchVertices` control points. Returned BY VALUE (a stage description is a POD, and + // the cache below is a rehashing map, so a pointer into it would not survive the next + // distinct patch size). `.module == VK_NULL_HANDLE` means the stage could not be built: + // the caller then has no control stage to inject, and CreatePipeline refuses the + // pipeline rather than handing the driver a half-tessellated one. + // + // Keyed on the patch size because GL takes the output patch size from PATCH_VERTICES, + // which is draw state, not link state - the CTS case that motivated this links at the + // default 3 and draws at 4. The pipeline cache already re-keys on patchControlPoints, + // so the module a pipeline was built with is part of that pipeline's identity. + // Compiling is bounded by the number of distinct patch sizes a program draws with + // (MAX_PATCH_VERTICES = 32 in the worst case, one or two in practice) and only ever + // happens for the rare program that has no control stage at all. + VkPipelineShaderStageCreateInfo GetOrCreatePassthroughTessControlStage(Uint32 patchVertices); + + // Source of the module above. Exposed for tests: the generated GLSL is the whole + // contract with the evaluation stage, so it is worth pinning independently of a device. + static String BuildPassthroughTessControlSource(Uint32 patchVertices); + private: struct ProgramLookupCache { const MG_State::GLState::ProgramObject* program = nullptr; @@ -391,6 +439,12 @@ namespace MobileGL::MG_Backend::DirectVulkan { VkProgramObject& entry) const; void ReflectLayout(const MG_State::GLState::ProgramObject& program, const Vector>& spirv, VkProgramObject& entry) const; + // Fills needsPassthroughTessControl / passthroughTessControlEmulatable off the linked + // modules. Const and reflection-only: it decides nothing about the pipeline, it only + // records what the evaluation stage's input interface is made of. + void ReflectPassthroughTessControlNeed(const Vector>& shaders, + const Vector>& spirv, + VkProgramObject& entry) const; VkDevice m_device = VK_NULL_HANDLE; Uint32 m_maxBindings = 0; @@ -411,6 +465,11 @@ namespace MobileGL::MG_Backend::DirectVulkan { // See GetCacheStructureEpoch(). Starts at 1 so a zero-initialized memo can never match. Uint64 m_cacheStructureEpoch = 1; IEvictionObserver* m_evictionObserver = nullptr; + // Pass-through tessellation control stages by input patch size. Never evicted: at most + // MAX_PATCH_VERTICES entries exist for the lifetime of the device, and every pipeline + // ever built from one keeps referencing its module. A failed build is cached as + // VK_NULL_HANDLE so a broken generator costs one compile, not one per draw. + UnorderedMap m_passthroughTessControlStages; static inline XXH64_state_t* m_hashState = XXH64_createState(); }; } // namespace MobileGL::MG_Backend::DirectVulkan diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp index 00394629..14ccdbcb 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp @@ -4883,6 +4883,26 @@ void main() { .vertexInputState = pipelineVertexInputState, .stageSpirvDigests = &programObj.stageSpirvDigests }; + // A program with a tessellation evaluation stage and no control stage relies on GL's + // fixed-function pass-through (GL 4.6 core 11.2.2), which Vulkan does not have. Build the + // stage GL describes for THIS draw's patch size - PATCH_VERTICES is draw state, not link + // state, so it is only knowable here - and hand it to the pipeline. Where the + // pass-through cannot stand in for what the evaluation stage actually reads, nothing is + // attached and CreatePipeline refuses the pipeline, which skips the draw. + // + // Gated on the PATCH topology as well, and that gate is load-bearing rather than an + // optimisation: patchControlPoints is only meaningful for a patch draw, and a pipeline + // that carries tessellation stages while its topology is anything else violates + // VUID-VkGraphicsPipelineCreateInfo-topology-00737 - the same class of invalid input as + // the missing control stage, on the same driver. Such a draw is illegal in GL too (a + // program with a tessellation stage may only be drawn with GL_PATCHES), so nothing legal + // loses its pass-through here; what it does lose is the pipeline, because the refusal + // below then sees an evaluation stage with no control stage and declines. + if (programObj.needsPassthroughTessControl && programObj.passthroughTessControlEmulatable && + vkTopology == VK_PRIMITIVE_TOPOLOGY_PATCH_LIST) { + payload.passthroughTessControlStage = + m_programFactory->GetOrCreatePassthroughTessControlStage(payload.patchControlPoints); + } if (!payload.stencilTestEnable) { payload.frontStencilFailOp = VK_STENCIL_OP_KEEP; payload.frontStencilPassOp = VK_STENCIL_OP_KEEP; diff --git a/MobileGL/MG_Test/Pipeline/CMakeLists.txt b/MobileGL/MG_Test/Pipeline/CMakeLists.txt index f3fe1ff9..0b9cc576 100644 --- a/MobileGL/MG_Test/Pipeline/CMakeLists.txt +++ b/MobileGL/MG_Test/Pipeline/CMakeLists.txt @@ -3,6 +3,7 @@ cmake_minimum_required(VERSION 3.14) add_executable( PipelineQuirkTest PipelineQuirkTest.cpp + PassthroughTessControlTest.cpp ) target_include_directories(PipelineQuirkTest PRIVATE diff --git a/MobileGL/MG_Test/Pipeline/PassthroughTessControlTest.cpp b/MobileGL/MG_Test/Pipeline/PassthroughTessControlTest.cpp new file mode 100644 index 00000000..ac3b023d --- /dev/null +++ b/MobileGL/MG_Test/Pipeline/PassthroughTessControlTest.cpp @@ -0,0 +1,247 @@ +// MobileGL - MobileGL/MG_Test/Pipeline/PassthroughTessControlTest.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 "Includes.h" +#include "Init.h" + +#include +#include +#include +#include +#include + +using namespace MobileGL; +using MobileGL::MG_Backend::DirectVulkan::ProgramFactory; +using MobileGL::MG_Util::ShaderTranspiler::ShaderCompiler; + +namespace { + // A test-side SPIR-V walker, deliberately independent of the production reflection: the + // generator's contract with the evaluation stage is "declare this many output vertices and + // write these built-ins", and that has to be readable off the module itself. + constexpr Uint32 kSpirvHeaderWordCount = 5; + constexpr Uint32 kOpExecutionMode = 16; + constexpr Uint32 kOpDecorate = 71; + constexpr Uint32 kOpMemberDecorate = 72; + constexpr Uint32 kExecutionModeOutputVertices = 26; + constexpr Uint32 kDecorationBuiltIn = 11; + + // SpvBuiltIn values used below. + constexpr Uint32 kBuiltInPosition = 0; + constexpr Uint32 kBuiltInInvocationId = 8; + constexpr Uint32 kBuiltInTessLevelOuter = 11; + constexpr Uint32 kBuiltInTessLevelInner = 12; + + template + void ForEachInstruction(const Vector& spirv, Visitor&& visit) { + for (SizeT i = kSpirvHeaderWordCount; i < spirv.size();) { + const Uint32 wordCount = spirv[i] >> 16; + const Uint32 opcode = spirv[i] & 0xFFFFu; + if (wordCount == 0 || i + wordCount > spirv.size()) break; + visit(opcode, &spirv[i], wordCount); + i += wordCount; + } + } + + // -1 when the module declares no OutputVertices mode at all, which is itself a failure the + // tests want to see named rather than silently compared against a wrong number. + Int DeclaredOutputVertices(const Vector& spirv) { + Int declared = -1; + ForEachInstruction(spirv, [&](Uint32 opcode, const Uint32* words, Uint32 wordCount) { + if (opcode == kOpExecutionMode && wordCount >= 4 && words[2] == kExecutionModeOutputVertices) { + declared = static_cast(words[3]); + } + }); + return declared; + } + + // The built-in members of every block in the module, keyed by the struct's result id, in + // member order. A gl_PerVertex is exactly such a struct, and its member list IS the shape the + // neighbouring stage has to agree with. + constexpr Uint32 kOpTypeStruct = 30; + + std::map> BuiltInBlockShapes(const Vector& spirv) { + std::map> shapes; + ForEachInstruction(spirv, [&](Uint32 opcode, const Uint32* words, Uint32 wordCount) { + if (opcode == kOpMemberDecorate && wordCount >= 5 && words[3] == kDecorationBuiltIn) { + shapes[words[1]].push_back(words[4]); + } + }); + return shapes; + } + + // Member count of a struct type, so a shape comparison can also catch a block that grew a + // NON-built-in member (which the decoration walk above would not see). + Uint32 StructMemberCount(const Vector& spirv, Uint32 structId) { + Uint32 count = 0; + ForEachInstruction(spirv, [&](Uint32 opcode, const Uint32* words, Uint32 wordCount) { + if (opcode == kOpTypeStruct && wordCount >= 2 && words[1] == structId) { + count = wordCount - 2; + } + }); + return count; + } + + std::set DeclaredBuiltIns(const Vector& spirv) { + std::set builtIns; + ForEachInstruction(spirv, [&](Uint32 opcode, const Uint32* words, Uint32 wordCount) { + if (opcode == kOpDecorate && wordCount >= 4 && words[2] == kDecorationBuiltIn) { + builtIns.insert(words[3]); + } + if (opcode == kOpMemberDecorate && wordCount >= 5 && words[3] == kDecorationBuiltIn) { + builtIns.insert(words[4]); + } + }); + return builtIns; + } + + Vector CompileGeneratedSource(Uint32 patchVertices) { + using namespace MG_Util::ShaderTranspiler; + const String source = ProgramFactory::BuildPassthroughTessControlSource(patchVertices); + + ShaderAttrib shaderAttrib{.shaderType = GL_TESS_CONTROL_SHADER, .sourceStr = source}; + auto shaderResult = ShaderCompiler::CompileShader(shaderAttrib); + EXPECT_TRUE(shaderResult) << (shaderResult ? String{} : shaderResult.error().log) << "\n" << source; + 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 = {GL_TESS_CONTROL_SHADER}, + .program = *programResult.value()}; + auto binaryResult = ShaderCompiler::GetSpirvBinaryFromProgram(binaryAttrib); + EXPECT_TRUE(binaryResult) << (binaryResult ? String{} : binaryResult.error().log); + if (!binaryResult || binaryResult->empty()) return {}; + return binaryResult->front(); + } +} // namespace + +class PassthroughTessControlTest : public ::testing::Test { +protected: + void SetUp() override { MobileGL::Initialize(); } +}; + +// The whole reason this stage is generated per patch size rather than once: GL takes the output +// patch size from PATCH_VERTICES, which is draw state. A program that links at the default 3 and +// draws at 4 - which is exactly what +// KHR-GL43.shader_storage_buffer_object.advanced-write-tessellation does - must get a stage built +// for 4, or its evaluation stage reads gl_in[3] out of a three-element array. +TEST_F(PassthroughTessControlTest, DeclaresTheRequestedPatchSize) { + for (const Uint32 patchVertices : {1u, 2u, 3u, 4u, 16u, 32u}) { + const Vector spirv = CompileGeneratedSource(patchVertices); + ASSERT_FALSE(spirv.empty()) << "patchVertices=" << patchVertices; + EXPECT_EQ(DeclaredOutputVertices(spirv), static_cast(patchVertices)) + << "patchVertices=" << patchVertices; + } +} + +// gl_Position in, gl_Position out, and both tessellation level arrays written: the four facts the +// evaluation stage downstream of this depends on. Position appearing at all is what makes the +// pass-through a pass-through; the levels are what GL's PATCH_DEFAULT_*_LEVEL state supplies when +// there is no control shader, and without them the tessellator produces nothing. +TEST_F(PassthroughTessControlTest, ForwardsPositionAndWritesBothLevelArrays) { + const Vector spirv = CompileGeneratedSource(4); + ASSERT_FALSE(spirv.empty()); + + const std::set builtIns = DeclaredBuiltIns(spirv); + EXPECT_TRUE(builtIns.contains(kBuiltInPosition)); + EXPECT_TRUE(builtIns.contains(kBuiltInInvocationId)); + EXPECT_TRUE(builtIns.contains(kBuiltInTessLevelOuter)); + EXPECT_TRUE(builtIns.contains(kBuiltInTessLevelInner)); +} + +// The generated source carries nothing but gl_Position across the interface. If that ever grows a +// user-defined varying, ReflectPassthroughTessControlNeed's "built-ins only" refusal stops being +// the right gate and both have to move together. +TEST_F(PassthroughTessControlTest, InterfaceIsBuiltInsOnly) { + const String source = ProgramFactory::BuildPassthroughTessControlSource(4); + EXPECT_EQ(source.find("layout(location"), String::npos) << source; + EXPECT_NE(source.find("layout(vertices = 4) out;"), String::npos) << source; +} + +// THE load-bearing test. Vulkan matches built-in interface blocks by their whole shape, and this +// stage is compiled ON ITS OWN - it never goes through the glslang link that gives a real program +// its gl_PerVertex. So the shape it declares has to equal the shape a linked vertex+evaluation +// program carries, and nothing at runtime says otherwise: a mismatch renders a black frame, no +// error, no validation message. That is exactly how the first cut of this shipped-and-failed +// (gl_Position only, three members short), and how the second did (glslang's default block for a +// standalone control stage, which appends gl_CullDistance where a linked program has no such +// member). This links the shader pair the motivating CTS case uses and compares the two shapes +// directly. +TEST_F(PassthroughTessControlTest, MatchesTheFrontendPerVertexBlock) { + using namespace MG_Util::ShaderTranspiler; + + // Deliberately the shape of KHR-GL43.shader_storage_buffer_object.advanced-write-tessellation: + // a vertex stage feeding an evaluation stage with no control stage in between. + static const char* kVs = R"(#version 430 core +layout(location = 0) in vec4 g_in_position; +void main() { gl_Position = g_in_position; } +)"; + static const char* kTes = R"(#version 430 core +layout(quads) in; +void main() { + vec4 p0 = mix(gl_in[0].gl_Position, gl_in[1].gl_Position, gl_TessCoord.x); + vec4 p1 = mix(gl_in[3].gl_Position, gl_in[2].gl_Position, gl_TessCoord.x); + gl_Position = mix(p0, p1, gl_TessCoord.y); +} +)"; + static const char* kFs = R"(#version 430 core +layout(location = 0) out vec4 g_fs_out; +void main() { g_fs_out = vec4(0, 1, 0, 1); } +)"; + + const Vector types{GL_VERTEX_SHADER, GL_TESS_EVALUATION_SHADER, GL_FRAGMENT_SHADER}; + const Vector sources{kVs, kTes, kFs}; + Vector> shaders; + for (SizeT i = 0; i < types.size(); ++i) { + ShaderAttrib attrib{.shaderType = types[i], .sourceStr = sources[i]}; + auto compiled = ShaderCompiler::CompileShader(attrib); + ASSERT_TRUE(compiled) << compiled.error().log; + shaders.push_back(compiled.value()); + } + ProgramAttrib programAttrib{.shaders = shaders}; + auto linked = ShaderCompiler::LinkProgram(programAttrib); + ASSERT_TRUE(linked) << linked.error().log; + ProgramBinaryAttrib binaryAttrib{.shaderTypes = types, .program = *linked.value()}; + auto binary = ShaderCompiler::GetSpirvBinaryFromProgram(binaryAttrib); + ASSERT_TRUE(binary); + ASSERT_EQ(binary->size(), types.size()); + + // The evaluation stage's gl_in is the block the pass-through has to feed. It is the only + // built-in block that stage declares as an input, so the module holds exactly one such shape + // besides its own gl_PerVertex output - and both are the same shape, which is the point. + const auto tesShapes = BuiltInBlockShapes((*binary)[1]); + ASSERT_FALSE(tesShapes.empty()); + const Vector frontendShape = tesShapes.begin()->second; + const Uint32 frontendMembers = StructMemberCount((*binary)[1], tesShapes.begin()->first); + for (const auto& [structId, shape] : tesShapes) { + EXPECT_EQ(shape, frontendShape) << "the evaluation stage's own built-in blocks disagree"; + EXPECT_EQ(StructMemberCount((*binary)[1], structId), frontendMembers); + } + + const Vector passthrough = CompileGeneratedSource(4); + ASSERT_FALSE(passthrough.empty()); + const auto passthroughShapes = BuiltInBlockShapes(passthrough); + ASSERT_FALSE(passthroughShapes.empty()); + + Uint32 perVertexBlocksChecked = 0; + for (const auto& [structId, shape] : passthroughShapes) { + // gl_TessLevelOuter/Inner are decorated on plain variables, not on a block, so every + // struct that reaches here is a gl_PerVertex - gl_in's and gl_out's. + EXPECT_EQ(shape, frontendShape) + << "the pass-through control stage's gl_PerVertex no longer matches the one the " + "frontend gives a linked vertex+evaluation program"; + EXPECT_EQ(StructMemberCount(passthrough, structId), frontendMembers) + << "the pass-through control stage's gl_PerVertex has a different member count"; + ++perVertexBlocksChecked; + } + EXPECT_EQ(perVertexBlocksChecked, 2u) << "expected both gl_in and gl_out to be gl_PerVertex blocks"; +} From 96646df12e00a9dff6dfc94e537f95189ced65ed Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Thu, 13 Aug 2026 01:34:57 -0400 Subject: [PATCH 2/2] [Fix] (MG_Backend/DirectVulkan): review round - failure diagnostics take MGLOG_E, the once-per-patch-size success note takes MGLOG_D, and the per-draw refusal stays latched --- .../DirectVulkan/Renderer/PipelineFactory.cpp | 11 ++++++----- .../DirectVulkan/Renderer/ProgramFactory.cpp | 14 +++++++------- 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/PipelineFactory.cpp b/MobileGL/MG_Backend/DirectVulkan/Renderer/PipelineFactory.cpp index ef1f235a..de69915f 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/PipelineFactory.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/PipelineFactory.cpp @@ -498,14 +498,15 @@ namespace MobileGL::MG_Backend::DirectVulkan { const Bool hasTessControl = (stagesPresent & VK_SHADER_STAGE_TESSELLATION_CONTROL_BIT) != 0; const Bool hasTessEval = (stagesPresent & VK_SHADER_STAGE_TESSELLATION_EVALUATION_BIT) != 0; if (hasTessControl != hasTessEval) { - // MGLOG_I, and latched: _E is compiled out of the INFO-level builds CTS and the - // shipping app run, which is exactly where this refusal is the only explanation - // for a missing draw. Latched because failures are deliberately not memoised - a - // program in this state re-enters here once per draw, every frame. + // Latched, and the latch is the point: a failed creation is deliberately never + // memoised (see GetOrCreatePipeline), so a program in this state re-enters here + // once per draw, every frame - and a refusal diagnostic that repeats per draw is + // noise, not a diagnostic. One line names the program; the draws it explains are + // all the same draw. static Bool s_warnedHalfTessellatedPipeline = false; if (!s_warnedHalfTessellatedPipeline) { s_warnedHalfTessellatedPipeline = true; - MGLOG_I("PipelineFactory::CreatePipeline: refusing a pipeline with %s tessellation stage and " + MGLOG_E("PipelineFactory::CreatePipeline: refusing a pipeline with %s tessellation stage and " "no %s stage (VUID-VkGraphicsPipelineCreateInfo-pStages-00730). programHash=0x%llx " "patchControlPoints=%u. Its draws are skipped; logged once.", hasTessEval ? "an evaluation" : "a control", diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/ProgramFactory.cpp b/MobileGL/MG_Backend/DirectVulkan/Renderer/ProgramFactory.cpp index ebf09c36..cfddc618 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/ProgramFactory.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/ProgramFactory.cpp @@ -3346,7 +3346,7 @@ namespace MobileGL::MG_Backend::DirectVulkan { .env = env.get()}; auto compiled = ShaderCompiler::CompileShader(shaderAttrib); if (!compiled) { - MGLOG_I("ProgramFactory: could not compile the pass-through tessellation control stage for " + MGLOG_E("ProgramFactory: could not compile the pass-through tessellation control stage for " "patchVertices=%u; a program with an evaluation stage and no control stage cannot draw. %s", patchVertices, compiled.error().log.c_str()); m_passthroughTessControlStages.emplace(patchVertices, stage); @@ -3357,7 +3357,7 @@ namespace MobileGL::MG_Backend::DirectVulkan { programAttrib.shaders.push_back(compiled.value()); auto linked = ShaderCompiler::LinkProgram(programAttrib); if (!linked) { - MGLOG_I("ProgramFactory: could not link the pass-through tessellation control stage for " + MGLOG_E("ProgramFactory: could not link the pass-through tessellation control stage for " "patchVertices=%u. %s", patchVertices, linked.error().log.c_str()); m_passthroughTessControlStages.emplace(patchVertices, stage); return stage; @@ -3366,7 +3366,7 @@ namespace MobileGL::MG_Backend::DirectVulkan { ProgramBinaryAttrib binaryAttrib{.shaderTypes = {GL_TESS_CONTROL_SHADER}, .program = *linked.value()}; auto binary = ShaderCompiler::GetSpirvBinaryFromProgram(binaryAttrib); if (!binary || binary.value().empty() || binary.value().front().empty()) { - MGLOG_I("ProgramFactory: could not generate SPIR-V for the pass-through tessellation control stage " + MGLOG_E("ProgramFactory: could not generate SPIR-V for the pass-through tessellation control stage " "for patchVertices=%u", patchVertices); m_passthroughTessControlStages.emplace(patchVertices, stage); return stage; @@ -3387,14 +3387,14 @@ namespace MobileGL::MG_Backend::DirectVulkan { VkShaderModule module = VK_NULL_HANDLE; const VkResult result = vkCreateShaderModule(m_device, &smci, nullptr, &module); if (result != VK_SUCCESS) { - MGLOG_I("ProgramFactory: vkCreateShaderModule failed (%d) for the pass-through tessellation control " + MGLOG_E("ProgramFactory: vkCreateShaderModule failed (%d) for the pass-through tessellation control " "stage for patchVertices=%u", static_cast(result), patchVertices); m_passthroughTessControlStages.emplace(patchVertices, stage); return stage; } stage.module = module; - MGLOG_I("ProgramFactory: built the pass-through tessellation control stage for patchVertices=%u " + MGLOG_D("ProgramFactory: built the pass-through tessellation control stage for patchVertices=%u " "(GL 4.6 11.2.2; Vulkan has no fixed-function equivalent)", patchVertices); m_passthroughTessControlStages.emplace(patchVertices, stage); return stage; @@ -3430,7 +3430,7 @@ namespace MobileGL::MG_Backend::DirectVulkan { const SpvReflectResult createResult = spvReflectCreateShaderModule(module.size() * sizeof(Uint), module.data(), &reflectModule); if (createResult != SPV_REFLECT_RESULT_SUCCESS) { - MGLOG_I("ProgramFactory::ReflectPassthroughTessControlNeed: reflection failed (result=%d); the " + MGLOG_E("ProgramFactory::ReflectPassthroughTessControlNeed: reflection failed (result=%d); the " "evaluation stage's inputs are unknown, so the pass-through is not offered", static_cast(createResult)); return; @@ -3468,7 +3468,7 @@ namespace MobileGL::MG_Backend::DirectVulkan { for (auto* input : inputs) { if (input == nullptr) continue; if (input->location == kNoLocation) continue; - MGLOG_I("ProgramFactory: a tessellation evaluation stage with no control stage reads the " + MGLOG_E("ProgramFactory: a tessellation evaluation stage with no control stage reads the " "user-defined input '%s' at location=%u; a synthesized control stage cannot forward it, so " "this program's draws are declined rather than fed an undefined varying", input->name != nullptr ? input->name : "", input->location);