From fe60711c249629f1222a73dc8d4d16944c37f9c5 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Sun, 18 Oct 2026 14:40:04 -0400 Subject: [PATCH] [Fix, Test] (MG_Backend/DirectVulkan, MG_Test/ShaderTranspiler): fix up geometry emits in helpers - clip-space fix-up and XFB gl_Position mirror go before every EmitVertex and EmitStreamVertex in the entry point's call tree, not only main's - helpers are not inlined, so a helper-emitted vertex skipped the depth remap (clipped below GL z=0) and was never captured by XFB - TransformSpirvForVulkanPositionFix exported for the unit cases --- .../DirectVulkan/Renderer/ProgramFactory.cpp | 159 +++++----- .../DirectVulkan/Renderer/ProgramFactory.h | 6 + .../MG_Test/ShaderTranspiler/CMakeLists.txt | 1 + .../PositionFixupSitesTest.cpp | 283 ++++++++++++++++++ 4 files changed, 374 insertions(+), 75 deletions(-) create mode 100644 MobileGL/MG_Test/ShaderTranspiler/PositionFixupSitesTest.cpp diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/ProgramFactory.cpp b/MobileGL/MG_Backend/DirectVulkan/Renderer/ProgramFactory.cpp index 88eb43e9..c285209c 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/ProgramFactory.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/ProgramFactory.cpp @@ -20,6 +20,7 @@ #include #include #include +#include #include #include #include @@ -914,6 +915,36 @@ namespace MobileGL::MG_Backend::DirectVulkan { return true; } + // Where a per-vertex fix-up of `entryPoint`'s outputs goes: in front of every emit a + // geometry shader can execute - EmitVertex and EmitStreamVertex alike, in the entry + // function or in any function it calls, since the front end does not inline helpers - and, + // for the other stages, in front of each return from the entry function itself, as a + // helper's return does not end the invocation. Collected before anything is inserted. + Vector CollectPerVertexFixupSites(spvtools::opt::IRContext* context, + const spvtools::opt::Instruction& entryPoint) { + Vector sites; + const Uint32 entryFunctionId = entryPoint.GetSingleWordInOperand(1); + if (context->GetFunction(entryFunctionId) == nullptr) return sites; + const auto collect = [&sites](spvtools::opt::Function& function, auto isSite) { + function.ForEachInst([&](spvtools::opt::Instruction* inst) { + if (isSite(inst->opcode())) sites.push_back(inst); + }); + }; + if (static_cast(entryPoint.GetSingleWordInOperand(0)) != + spv::ExecutionModel::Geometry) { + collect(*context->GetFunction(entryFunctionId), [](spv::Op op) { return op == spv::Op::OpReturn; }); + return sites; + } + std::unordered_set callTree; + context->CollectCallTreeFromRoots(entryFunctionId, &callTree); + for (auto& function : *context->module()) { + if (callTree.count(function.result_id()) == 0) continue; + collect(function, + [](spv::Op op) { return op == spv::Op::OpEmitVertex || op == spv::Op::OpEmitStreamVertex; }); + } + return sites; + } + // Is `pointerId` the position target itself, or an access chain rooted at it? Bool PointerReachesPositionTarget(spvtools::opt::IRContext* context, Uint32 pointerId, const PositionTargetInfo& target) { @@ -1095,22 +1126,11 @@ namespace MobileGL::MG_Backend::DirectVulkan { continue; } - auto* function = context()->GetFunction(entryPoint.GetSingleWordInOperand(1)); - if (!function) continue; - Bool modifiedThisEntryPoint = false; - for (auto& bb : *function) { - for (auto instIter = bb.begin(); instIter != bb.end(); ++instIter) { - auto* inst = &*instIter; - const Bool needsFixup = - (model == spv::ExecutionModel::Geometry && inst->opcode() == spv::Op::OpEmitVertex) || - (model != spv::ExecutionModel::Geometry && inst->opcode() == spv::Op::OpReturn); - if (!needsFixup) continue; - - modifiedThisEntryPoint |= - InsertPositionFixup(context(), inst, target, halfConstId, doYFlip, doZRemap, - doSurfaceRotate90, doSurfaceRotate180, doSurfaceRotate270); - } + for (auto* site : CollectPerVertexFixupSites(context(), entryPoint)) { + modifiedThisEntryPoint |= + InsertPositionFixup(context(), site, target, halfConstId, doYFlip, doZRemap, + doSurfaceRotate90, doSurfaceRotate180, doSurfaceRotate270); } // Per entry point, and only for one this pass actually injected into: the // injected load/store is a static use of the position variable, so the @@ -1364,8 +1384,8 @@ namespace MobileGL::MG_Backend::DirectVulkan { // Decorates the module's captured varyings for VK_EXT_transform_feedback: // user outputs get XfbBuffer/XfbStride/Offset directly; a captured // gl_Position (a gl_PerVertex member) is mirrored into a dedicated output - // variable copied before every OpReturn, BEFORE the position fixup runs, - // so the captured value is the shader's own (pre-remap) gl_Position. + // variable copied at every CollectPerVertexFixupSites site, BEFORE the position + // fixup runs, so the captured value is the shader's own (pre-remap) gl_Position. class XfbCaptureDecoratePass final : public spvtools::opt::Pass { public: struct CapturedVarying { @@ -1393,7 +1413,6 @@ namespace MobileGL::MG_Backend::DirectVulkan { auto entryPointIter = get_module()->entry_points().begin(); if (entryPointIter == get_module()->entry_points().end()) return Status::SuccessWithoutChange; spvtools::opt::Instruction* entryPoint = &*entryPointIter; - const Uint32 entryFunctionId = entryPoint->GetSingleWordInOperand(1); // Name -> result id map from the debug section. std::unordered_map idsByName; @@ -1542,14 +1561,14 @@ namespace MobileGL::MG_Backend::DirectVulkan { } if (needsPositionMirror) { - modified |= MirrorPerVertexBuiltInForCapture(entryFunctionId, *entryPoint, spv::BuiltIn::Position, - IsVec4Float32, "gl_Position", positionBufferIndex, - positionOffset, decorateForXfb); + modified |= MirrorPerVertexBuiltInForCapture(*entryPoint, spv::BuiltIn::Position, IsVec4Float32, + "gl_Position", positionBufferIndex, positionOffset, + decorateForXfb); } if (needsPointSizeMirror) { - modified |= MirrorPerVertexBuiltInForCapture(entryFunctionId, *entryPoint, spv::BuiltIn::PointSize, - IsFloat32Scalar, "gl_PointSize", pointSizeBufferIndex, - pointSizeOffset, decorateForXfb); + modified |= MirrorPerVertexBuiltInForCapture(*entryPoint, spv::BuiltIn::PointSize, IsFloat32Scalar, + "gl_PointSize", pointSizeBufferIndex, pointSizeOffset, + decorateForXfb); } if (!modified) return Status::SuccessWithoutChange; @@ -1592,11 +1611,10 @@ namespace MobileGL::MG_Backend::DirectVulkan { // built-in is looked up and what type it has, so one injector serves both. Anything // else in gl_PerVertex would need its own type check before it could be added here. template - Bool MirrorPerVertexBuiltInForCapture(Uint32 entryFunctionId, spvtools::opt::Instruction& entryPoint, - spv::BuiltIn builtIn, BuiltInTypeCheckFn typeCheck, - const char* glslName, Uint32 bufferIndex, Uint32 offsetBytes, + Bool MirrorPerVertexBuiltInForCapture(spvtools::opt::Instruction& entryPoint, spv::BuiltIn builtIn, + BuiltInTypeCheckFn typeCheck, const char* glslName, + Uint32 bufferIndex, Uint32 offsetBytes, const DecorateFn& decorateForXfb) { - const Uint32 entryPointModel = entryPoint.GetSingleWordInOperand(0); using namespace spvtools::opt; PositionTargetInfo target{}; if (!FindBuiltInTarget(context(), builtIn, typeCheck, &target)) { @@ -1640,27 +1658,17 @@ namespace MobileGL::MG_Backend::DirectVulkan { decorateForXfb(mirrorVariableId, bufferIndex, offsetBytes); entryPoint.AddOperand({SPV_OPERAND_TYPE_ID, {mirrorVariableId}}); - auto* function = context()->GetFunction(entryFunctionId); - if (function == nullptr) return false; - const auto model = static_cast(entryPointModel); + // Geometry stages capture per emitted vertex; other stages at return. Bool injected = false; - for (auto& block : *function) { - for (auto instIter = block.begin(); instIter != block.end(); ++instIter) { - // Geometry stages capture per emitted vertex; other stages at return. - const Bool isInjectionSite = model == spv::ExecutionModel::Geometry - ? instIter->opcode() == spv::Op::OpEmitVertex - : instIter->opcode() == spv::Op::OpReturn; - if (!isInjectionSite) continue; - InstructionBuilder builder(context(), &*instIter, IRContext::kAnalysisNone); - const Uint32 memberIndexId = builder.GetUintConstantId(target.memberIndex); - auto* access = - builder.AddAccessChain(target.vectorPtrTypeId, target.variableId, {memberIndexId}); - if (access == nullptr) return injected; - auto* value = builder.AddLoad(target.vectorTypeId, access->result_id()); - if (value == nullptr) return injected; - builder.AddStore(mirrorVariableId, value->result_id()); - injected = true; - } + for (auto* site : CollectPerVertexFixupSites(context(), entryPoint)) { + InstructionBuilder builder(context(), site, IRContext::kAnalysisNone); + const Uint32 memberIndexId = builder.GetUintConstantId(target.memberIndex); + auto* access = builder.AddAccessChain(target.vectorPtrTypeId, target.variableId, {memberIndexId}); + if (access == nullptr) return injected; + auto* value = builder.AddLoad(target.vectorTypeId, access->result_id()); + if (value == nullptr) return injected; + builder.AddStore(mirrorVariableId, value->result_id()); + injected = true; } // The mirror was listed on the entry point above, but the loop just added a READ // of the SOURCE block through an access chain, and the interface rule covers @@ -1951,36 +1959,37 @@ namespace MobileGL::MG_Backend::DirectVulkan { } return success; } + } // namespace - Bool TransformSpirvForVulkanPositionFix(const Vector& input, Vector& output, - ProgramFactory::CompileOptionFlags transformFlags) { - if (input.empty()) { - output.clear(); - return true; - } - - if (!transformFlags) { - output = input; - return true; - } - - spvtools::Optimizer optimizer(SPV_ENV_VULKAN_1_3); - spvtools::OptimizerOptions options; - options.set_run_validator(false); // see TransformSpirvForExplicitLod0Sampling - optimizer.SetMessageConsumer( - [](spv_message_level_t, const char*, const spv_position_t&, const char* message) { - MGLOG_E("Vulkan: position fix pass: %s", message != nullptr ? message : ""); - }); - optimizer.RegisterPass(CreateGlToVulkanPositionFixPass(transformFlags)); - - const Bool success = optimizer.Run(input.data(), input.size(), &output, options); - if (!success) { - MGLOG_E("Vulkan: failed to run GL->Vulkan position fix pass"); - output = input; - } - return success; + Bool TransformSpirvForVulkanPositionFix(const Vector& input, Vector& output, + ProgramFactory::CompileOptionFlags transformFlags) { + if (input.empty()) { + output.clear(); + return true; } + if (!transformFlags) { + output = input; + return true; + } + + spvtools::Optimizer optimizer(SPV_ENV_VULKAN_1_3); + spvtools::OptimizerOptions options; + options.set_run_validator(false); // see TransformSpirvForExplicitLod0Sampling + optimizer.SetMessageConsumer([](spv_message_level_t, const char*, const spv_position_t&, const char* message) { + MGLOG_E("Vulkan: position fix pass: %s", message != nullptr ? message : ""); + }); + optimizer.RegisterPass(CreateGlToVulkanPositionFixPass(transformFlags)); + + const Bool success = optimizer.Run(input.data(), input.size(), &output, options); + if (!success) { + MGLOG_E("Vulkan: failed to run GL->Vulkan position fix pass"); + output = input; + } + return success; + } + + namespace { ShaderStage PickClipFixupStage(const Vector& stages) { Bool hasGeometry = false; Bool hasTessEval = false; diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/ProgramFactory.h b/MobileGL/MG_Backend/DirectVulkan/Renderer/ProgramFactory.h index 11e557f9..a1178993 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/ProgramFactory.h +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/ProgramFactory.h @@ -726,4 +726,10 @@ namespace MobileGL::MG_Backend::DirectVulkan { // Private copy whose y is (framebufferHeight - y). Fails open (output = input, false). Exposed // so a unit case can validate the bytes it hands the driver. Bool TransformSpirvForFragCoordYFlip(const Vector& input, Vector& output, Uint32 framebufferHeight); + + // The clip-space fix-up of the last pre-rasterization stage: gl_Position is rewritten in place + // per `transformFlags` before every vertex the stage produces. Fails open (output = input, + // false). Exposed, like the transform above, so a unit case can validate its bytes. + Bool TransformSpirvForVulkanPositionFix(const Vector& input, Vector& output, + ProgramFactory::CompileOptionFlags transformFlags); } // namespace MobileGL::MG_Backend::DirectVulkan diff --git a/MobileGL/MG_Test/ShaderTranspiler/CMakeLists.txt b/MobileGL/MG_Test/ShaderTranspiler/CMakeLists.txt index 4e87aa76..541a0e50 100644 --- a/MobileGL/MG_Test/ShaderTranspiler/CMakeLists.txt +++ b/MobileGL/MG_Test/ShaderTranspiler/CMakeLists.txt @@ -24,6 +24,7 @@ add_executable( LowerViewportIndexTest.cpp DeferGeometryEmitsTest.cpp RetainGeometryOutputsTest.cpp + PositionFixupSitesTest.cpp ClampMultisampleFetchTest.cpp LegalizeResourceArrayIndexTest.cpp FlattenAtomicCounterBlockTest.cpp diff --git a/MobileGL/MG_Test/ShaderTranspiler/PositionFixupSitesTest.cpp b/MobileGL/MG_Test/ShaderTranspiler/PositionFixupSitesTest.cpp new file mode 100644 index 00000000..7ec0004a --- /dev/null +++ b/MobileGL/MG_Test/ShaderTranspiler/PositionFixupSitesTest.cpp @@ -0,0 +1,283 @@ +// MobileGL - MobileGL/MG_Test/ShaderTranspiler/PositionFixupSitesTest.cpp +// Copyright (c) 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 +// +// Magma's clip-space fix-up rewrites gl_Position in front of every vertex the last +// pre-rasterization stage produces. A geometry shader produces one at every emit it can execute, +// and the front end leaves helper functions in place, so an EmitVertex() inside a helper needs the +// fix-up too: without it those vertices reach the rasterizer un-remapped, and a GL depth below +// zero is clipped away. A vertex shader's helper return, on the other hand, does not end the +// invocation and must stay untouched. + +#include + +#include + +#include "Includes.h" +#include "Init.h" +#include +#include +#include + +#include + +#include "source/opt/build_module.h" +#include "source/opt/ir_context.h" +#include "spirv.hpp" + +using namespace MobileGL; +using MobileGL::MG_Util::ShaderTranspiler::ShaderCompiler; +using spvtools::opt::Instruction; +using spvtools::opt::IRContext; + +namespace { + using Bit = MG_Backend::DirectVulkan::ProgramFactory::CompileOptionBit; + using CompileOptionFlags = MG_Backend::DirectVulkan::ProgramFactory::CompileOptionFlags; + + // One stage, linked and sanitized the way production hands it to the backend. + Vector CompileStage(GLenum stage, const String& source) { + using namespace MG_Util::ShaderTranspiler; + MobileGL::Initialize(); + 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 {}; + Vector module = binaryResult->front(); + EXPECT_TRUE(ShaderCompiler::SanitizeAndOptimizeBinary(module, module, true, true)); + return module; + } + + Vector FixUp(const Vector& spirv) { + Vector out; + EXPECT_TRUE(MG_Backend::DirectVulkan::TransformSpirvForVulkanPositionFix( + spirv, out, CompileOptionFlags(Bit::PositionYFlip) | Bit::PositionZRemap)); + return out; + } + + String ValidationError(const Vector& spirv) { + spvtools::SpirvTools tools(SPV_ENV_VULKAN_1_1); + String error; + tools.SetMessageConsumer([&](spv_message_level_t, const char*, const spv_position_t&, const char* message) { + error += message != nullptr ? message : ""; + }); + return tools.Validate(spirv) ? String{} : (error.empty() ? String{"invalid"} : error); + } + + String Disassemble(const Vector& spirv) { + spvtools::SpirvTools tools(SPV_ENV_VULKAN_1_1); + String text; + EXPECT_TRUE(tools.Disassemble(spirv, &text)); + return text; + } + + // Whether `pointerId` is an access chain to gl_PerVertex's Position member. + Bool IsPositionPointer(IRContext* context, Uint32 pointerId) { + auto* defUse = context->get_def_use_mgr(); + const Instruction* chain = defUse->GetDef(pointerId); + if (chain == nullptr || chain->opcode() != spv::Op::OpAccessChain || chain->NumInOperands() != 2) return false; + const Instruction* base = defUse->GetDef(chain->GetSingleWordInOperand(0)); + const Instruction* index = defUse->GetDef(chain->GetSingleWordInOperand(1)); + if (base == nullptr || base->opcode() != spv::Op::OpVariable || index == nullptr || + index->opcode() != spv::Op::OpConstant) + return false; + const Uint32 structId = defUse->GetDef(base->type_id())->GetSingleWordInOperand(1); + const Uint32 member = index->GetSingleWordInOperand(0); + for (const Instruction& annotation : context->annotations()) { + if (annotation.opcode() == spv::Op::OpMemberDecorate && annotation.GetSingleWordInOperand(0) == structId && + annotation.GetSingleWordInOperand(1) == member && + annotation.GetSingleWordInOperand(2) == Uint32(spv::Decoration::BuiltIn) && + annotation.GetSingleWordInOperand(3) == Uint32(spv::BuiltIn::Position)) + return true; + } + return false; + } + + // Whether `inst` is the fix-up's closing store: Position stored back with its y negated, the + // y taken from a load of Position itself. The shader's own stores never have that shape. + Bool IsFixupStore(IRContext* context, const Instruction& inst) { + if (inst.opcode() != spv::Op::OpStore || !IsPositionPointer(context, inst.GetSingleWordInOperand(0))) + return false; + auto* defUse = context->get_def_use_mgr(); + const Instruction* value = defUse->GetDef(inst.GetSingleWordInOperand(1)); + if (value == nullptr || value->opcode() != spv::Op::OpCompositeConstruct || value->NumInOperands() != 4) + return false; + const Instruction* negatedY = defUse->GetDef(value->GetSingleWordInOperand(1)); + if (negatedY == nullptr || negatedY->opcode() != spv::Op::OpFNegate) return false; + const Instruction* y = defUse->GetDef(negatedY->GetSingleWordInOperand(0)); + if (y == nullptr || y->opcode() != spv::Op::OpCompositeExtract) return false; + const Instruction* loaded = defUse->GetDef(y->GetSingleWordInOperand(0)); + return loaded != nullptr && loaded->opcode() == spv::Op::OpLoad && + IsPositionPointer(context, loaded->GetSingleWordInOperand(0)); + } + + struct Sites { + SizeT sites = 0; + SizeT sitesInHelpers = 0; + // Sites whose immediately preceding instruction is the fix-up's store. + SizeT fixedSites = 0; + SizeT fixupStores = 0; + SizeT fixupStoresInHelpers = 0; + }; + + // Emits for a geometry shader, returns for any other stage. + Sites Inspect(const Vector& spirv) { + Sites sites; + std::unique_ptr context = + spvtools::BuildModule(SPV_ENV_VULKAN_1_1, nullptr, spirv.data(), spirv.size()); + EXPECT_NE(context, nullptr); + if (context == nullptr) return sites; + const Instruction& entryPoint = *context->module()->entry_points().begin(); + const Bool geometry = + static_cast(entryPoint.GetSingleWordInOperand(0)) == spv::ExecutionModel::Geometry; + const Uint32 entryFunctionId = entryPoint.GetSingleWordInOperand(1); + for (auto& function : *context->module()) { + const Bool helper = function.result_id() != entryFunctionId; + for (auto& block : function) { + const Instruction* previous = nullptr; + for (const Instruction& inst : block) { + const Bool isSite = geometry ? (inst.opcode() == spv::Op::OpEmitVertex || + inst.opcode() == spv::Op::OpEmitStreamVertex) + : inst.opcode() == spv::Op::OpReturn; + if (isSite) { + ++sites.sites; + if (helper) ++sites.sitesInHelpers; + if (previous != nullptr && IsFixupStore(context.get(), *previous)) ++sites.fixedSites; + } + if (IsFixupStore(context.get(), inst)) { + ++sites.fixupStores; + if (helper) ++sites.fixupStoresInHelpers; + } + previous = &inst; + } + } + } + return sites; + } +} // namespace + +// gsprobe's helper case: each corner is written and emitted inside the helper. +TEST(PositionFixupSitesTest, AnEmitInsideAHelperGetsTheFixup) { + const auto spirv = CompileStage(GL_GEOMETRY_SHADER, R"(#version 410 core +layout(triangles) in; +layout(triangle_strip, max_vertices = 3) out; +void corner(int i) { gl_Position = vec4(gl_in[i].gl_Position.xy, -0.5, 1.0); EmitVertex(); } +void main() { corner(0); corner(1); corner(2); EndPrimitive(); } +)"); + ASSERT_FALSE(spirv.empty()); + const Sites before = Inspect(spirv); + ASSERT_EQ(before.sites, 1u) << "the helper is expected to survive to the backend"; + ASSERT_EQ(before.sitesInHelpers, 1u); + ASSERT_EQ(before.fixupStores, 0u); + + const auto fixed = FixUp(spirv); + EXPECT_EQ(ValidationError(fixed), String{}) << Disassemble(fixed); + const Sites after = Inspect(fixed); + EXPECT_EQ(after.sites, 1u); + EXPECT_EQ(after.fixedSites, 1u) << Disassemble(fixed); + EXPECT_EQ(after.fixupStores, 1u); +} + +// The value is written in main and only the emit sits in a helper. +TEST(PositionFixupSitesTest, AHelperThatOnlyEmitsGetsTheFixup) { + const auto spirv = CompileStage(GL_GEOMETRY_SHADER, R"(#version 410 core +layout(triangles) in; +layout(triangle_strip, max_vertices = 3) out; +void emitIt() { EmitVertex(); } +void main() { + for (int i = 0; i < 3; i++) { gl_Position = vec4(gl_in[i].gl_Position.xy, -0.5, 1.0); emitIt(); } + EndPrimitive(); +} +)"); + ASSERT_FALSE(spirv.empty()); + ASSERT_EQ(Inspect(spirv).sitesInHelpers, 1u); + + const auto fixed = FixUp(spirv); + EXPECT_EQ(ValidationError(fixed), String{}) << Disassemble(fixed); + const Sites after = Inspect(fixed); + EXPECT_EQ(after.sites, 1u); + EXPECT_EQ(after.fixedSites, 1u) << Disassemble(fixed); + EXPECT_EQ(after.fixupStores, 1u); +} + +// Emits in main and in a helper side by side: each one is fixed exactly once. +TEST(PositionFixupSitesTest, EmitsInMainAndInAHelperAreEachFixedOnce) { + const auto spirv = CompileStage(GL_GEOMETRY_SHADER, R"(#version 410 core +layout(triangles) in; +layout(triangle_strip, max_vertices = 3) out; +void corner(int i) { gl_Position = gl_in[i].gl_Position; EmitVertex(); } +void main() { + gl_Position = gl_in[0].gl_Position; + EmitVertex(); + corner(1); + corner(2); + EndPrimitive(); +} +)"); + ASSERT_FALSE(spirv.empty()); + const Sites before = Inspect(spirv); + ASSERT_EQ(before.sites, 2u); + ASSERT_EQ(before.sitesInHelpers, 1u); + + const auto fixed = FixUp(spirv); + EXPECT_EQ(ValidationError(fixed), String{}) << Disassemble(fixed); + const Sites after = Inspect(fixed); + EXPECT_EQ(after.fixedSites, 2u) << Disassemble(fixed); + EXPECT_EQ(after.fixupStores, 2u); + EXPECT_EQ(after.fixupStoresInHelpers, 1u); +} + +// The shape that always worked: every emit in main, each one still fixed. +TEST(PositionFixupSitesTest, EmitsInMainKeepTheirFixup) { + const auto spirv = CompileStage(GL_GEOMETRY_SHADER, R"(#version 410 core +layout(triangles) in; +layout(triangle_strip, max_vertices = 4) out; +void main() { + for (int i = 0; i < 3; i++) { gl_Position = gl_in[i].gl_Position; EmitVertex(); } + gl_Position = gl_in[0].gl_Position; + EmitVertex(); + EndPrimitive(); +} +)"); + ASSERT_FALSE(spirv.empty()); + const Sites before = Inspect(spirv); + ASSERT_EQ(before.sites, 2u); + ASSERT_EQ(before.sitesInHelpers, 0u); + + const auto fixed = FixUp(spirv); + EXPECT_EQ(ValidationError(fixed), String{}) << Disassemble(fixed); + const Sites after = Inspect(fixed); + EXPECT_EQ(after.fixedSites, 2u) << Disassemble(fixed); + EXPECT_EQ(after.fixupStores, 2u); +} + +// A vertex shader that writes gl_Position in a helper: only main's return ends the invocation, +// so that is the only place the fix-up goes. +TEST(PositionFixupSitesTest, AVertexShaderHelperReturnIsNotAFixupSite) { + const auto spirv = CompileStage(GL_VERTEX_SHADER, R"(#version 410 core +layout(location = 0) in vec2 pos; +void place(vec2 p) { gl_Position = vec4(p, -0.5, 1.0); } +void main() { place(pos); } +)"); + ASSERT_FALSE(spirv.empty()); + const Sites before = Inspect(spirv); + ASSERT_EQ(before.sitesInHelpers, 1u) << "the helper is expected to survive to the backend"; + + const auto fixed = FixUp(spirv); + EXPECT_EQ(ValidationError(fixed), String{}) << Disassemble(fixed); + const Sites after = Inspect(fixed); + EXPECT_EQ(after.fixupStores, 1u) << Disassemble(fixed); + EXPECT_EQ(after.fixupStoresInHelpers, 0u); + EXPECT_EQ(after.fixedSites, 1u); +}