[Fix] (DirectVulkan): retype gl_FragCoord access chains to Private when the y-flip pass redirects them to its copy

This commit is contained in:
2026-10-02 13:34:16 -04:00
parent 8da082f805
commit 239fc228fc
4 changed files with 195 additions and 26 deletions
@@ -1313,6 +1313,30 @@ namespace MobileGL::MG_Backend::DirectVulkan {
}
});
if (!ok) return false;
// An access chain into the builtin (gl_FragCoord.y is one) carries an Input
// pointer type. Based on the Private copy it has to carry a Private one, and so
// does every chain built on top of it - otherwise the module hands the driver a
// pointer whose storage class disagrees with its base, which is invalid SPIR-V
// and reads an undefined value. Plan every retype before touching anything, so a
// shape this pass does not model declines the whole rewrite.
Vector<Pair<Instruction*, Uint32>> retypes;
Vector<Instruction*> chains;
for (Instruction* user : users) {
if (IsAccessChain(user->opcode())) chains.push_back(user);
}
while (!chains.empty()) {
Instruction* chain = chains.back();
chains.pop_back();
const Uint32 privateType = PrivatePointerTypeFor(chain->type_id());
if (privateType == 0) return false;
retypes.emplace_back(chain, privateType);
context()->get_def_use_mgr()->ForEachUser(chain, [&](Instruction* next) {
if (IsAccessChain(next->opcode()) && next->NumInOperands() > 0 &&
next->GetSingleWordInOperand(0) == chain->result_id()) {
chains.push_back(next);
}
});
}
for (Instruction* user : users) {
for (Uint32 i = 0; i < user->NumInOperands(); ++i) {
auto& operand = user->GetInOperand(i);
@@ -1323,9 +1347,32 @@ namespace MobileGL::MG_Backend::DirectVulkan {
}
context()->AnalyzeUses(user);
}
for (auto& [chain, privateType] : retypes) {
chain->SetResultType(privateType);
context()->AnalyzeUses(chain);
}
return true;
}
static Bool IsAccessChain(spv::Op opcode) {
return opcode == spv::Op::OpAccessChain || opcode == spv::Op::OpInBoundsAccessChain ||
opcode == spv::Op::OpPtrAccessChain || opcode == spv::Op::OpInBoundsPtrAccessChain;
}
// The Private-storage twin of an Input pointer type, or 0 when `pointerTypeId` is not
// an Input pointer (then the chain is not the shape this pass rewrites).
Uint32 PrivatePointerTypeFor(Uint32 pointerTypeId) {
using namespace spvtools::opt;
Instruction* pointerType = context()->get_def_use_mgr()->GetDef(pointerTypeId);
if (!pointerType || pointerType->opcode() != spv::Op::OpTypePointer) return 0;
if (static_cast<spv::StorageClass>(pointerType->GetSingleWordInOperand(0)) !=
spv::StorageClass::Input) {
return 0;
}
return context()->get_type_mgr()->FindPointerToType(pointerType->GetSingleWordInOperand(1),
spv::StorageClass::Private);
}
Uint32 m_framebufferHeight = 0;
};
@@ -1861,35 +1908,38 @@ namespace MobileGL::MG_Backend::DirectVulkan {
return spvtools::Optimizer::PassToken(MakeUnique<GlToVulkanPositionFixPass>(transformFlags));
}
Bool TransformSpirvForFragCoordYFlip(const Vector<Uint>& input, Vector<Uint>& output,
Uint32 framebufferHeight) {
if (input.empty()) {
output.clear();
return true;
}
if (framebufferHeight == 0) {
output = input;
return true;
}
} // namespace
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: fragcoord y-flip pass: %s", message != nullptr ? message : "");
});
optimizer.RegisterPass(
spvtools::Optimizer::PassToken(MakeUnique<GlFragCoordYFlipPass>(framebufferHeight)));
const Bool success = optimizer.Run(input.data(), input.size(), &output, options);
if (!success) {
MGLOG_E("Vulkan: failed to run the gl_FragCoord y-flip pass; keeping the original module");
output = input;
}
return success;
Bool TransformSpirvForFragCoordYFlip(const Vector<Uint>& input, Vector<Uint>& output,
Uint32 framebufferHeight) {
if (input.empty()) {
output.clear();
return true;
}
if (framebufferHeight == 0) {
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: fragcoord y-flip pass: %s", message != nullptr ? message : "");
});
optimizer.RegisterPass(
spvtools::Optimizer::PassToken(MakeUnique<GlFragCoordYFlipPass>(framebufferHeight)));
const Bool success = optimizer.Run(input.data(), input.size(), &output, options);
if (!success) {
MGLOG_E("Vulkan: failed to run the gl_FragCoord y-flip pass; keeping the original module");
output = input;
}
return success;
}
namespace {
Bool TransformSpirvForXfbCapture(const Vector<Uint>& input, Vector<Uint>& output,
const MagmaProgramSource& program) {
if (input.empty()) {
@@ -658,4 +658,9 @@ namespace MobileGL::MG_Backend::DirectVulkan {
UnorderedMap<Uint64, VkPipelineShaderStageCreateInfo> m_passthroughTessControlStages;
static inline thread_local XXH64_state_t* m_hashState = XXH64_createState();
};
// The FragCoordYFlip variant's module transform: every read of gl_FragCoord goes through a
// 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<Uint>& input, Vector<Uint>& output, Uint32 framebufferHeight);
} // namespace MobileGL::MG_Backend::DirectVulkan
@@ -19,6 +19,7 @@ add_executable(
WidenImageFormatsTest.cpp
GlslangCaptureTest.cpp
DemotePointSizeTest.cpp
FragCoordYFlipTest.cpp
)
target_include_directories(SpirvPassTest PRIVATE
@@ -0,0 +1,113 @@
// MobileGL - MobileGL/MG_Test/ShaderTranspiler/FragCoordYFlipTest.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
// Magma's FragCoordYFlip variant redirects every read of gl_FragCoord to a Private copy. A
// single-component read (gl_FragCoord.y) is an access chain whose result type is an INPUT
// pointer; left that way on a Private base it is invalid SPIR-V that the driver still accepts and
// reads garbage through. The module must validate, and the chain must point at the copy.
#include <gtest/gtest.h>
#include "Includes.h"
#include "Init.h"
#include <MG_Backend/DirectVulkan/Renderer/ProgramFactory.h>
#include <MG_Util/ShaderTranspiler/ShaderCompiler.h>
#include <MG_Util/ShaderTranspiler/Types.h>
#include "spirv-tools/libspirv.hpp"
using namespace MobileGL;
using MobileGL::MG_Util::ShaderTranspiler::ShaderCompiler;
namespace {
Vector<Vector<Uint32>> CompileProgram(const char* vertex, const char* fragment) {
using namespace MG_Util::ShaderTranspiler;
Vector<SharedPtr<glslang::TShader>> shaders;
Vector<GLenum> types;
for (const auto& [stage, source] : Vector<Pair<GLenum, const char*>>{{GL_VERTEX_SHADER, vertex},
{GL_FRAGMENT_SHADER, fragment}}) {
ShaderAttrib attrib{.shaderType = stage, .sourceStr = source};
auto shader = ShaderCompiler::CompileShader(attrib);
EXPECT_TRUE(shader) << (shader ? String{} : shader.error().log);
if (!shader) return {};
shaders.push_back(shader.value());
types.push_back(stage);
}
ProgramAttrib programAttrib{.shaders = shaders};
auto program = ShaderCompiler::LinkProgram(programAttrib);
EXPECT_TRUE(program) << (program ? String{} : program.error().log);
if (!program) return {};
ProgramBinaryAttrib binaryAttrib{.shaderTypes = types, .program = *program.value()};
auto binary = ShaderCompiler::GetSpirvBinaryFromProgram(binaryAttrib);
EXPECT_TRUE(binary) << (binary ? String{} : binary.error().log);
if (!binary) return {};
Vector<Vector<Uint32>> modules = Move(binary.value());
for (auto& module : modules) {
EXPECT_TRUE(ShaderCompiler::SanitizeAndOptimizeBinary(module, module, true, true));
}
return modules;
}
String Disassemble(const Vector<Uint32>& spirv) {
spvtools::SpirvTools tools(SPV_ENV_VULKAN_1_1);
String text;
EXPECT_TRUE(tools.Disassemble(spirv, &text));
return text;
}
String ValidationError(const Vector<Uint32>& 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);
}
constexpr const char* kVertex = R"(#version 330 core
layout(location = 0) in vec2 position;
void main() { gl_Position = vec4(position, 0.0, 1.0); }
)";
void ExpectFlippedModuleValid(const char* fragment) {
MobileGL::Initialize();
auto modules = CompileProgram(kVertex, fragment);
ASSERT_EQ(modules.size(), 2u);
const Vector<Uint32>& input = modules[1];
ASSERT_EQ(ValidationError(input), String{});
Vector<Uint32> flipped;
ASSERT_TRUE(MG_Backend::DirectVulkan::TransformSpirvForFragCoordYFlip(input, flipped, 720));
EXPECT_EQ(ValidationError(flipped), String{}) << Disassemble(flipped);
// The flip really happened: the height is subtracted, and no chain still reads the
// builtin's Input storage.
const String text = Disassemble(flipped);
EXPECT_NE(text.find("OpFSub"), String::npos) << text;
EXPECT_EQ(text.find("OpAccessChain %_ptr_Input_float"), String::npos) << text;
}
} // namespace
TEST(FragCoordYFlipTest, SingleComponentReadStaysValid) {
ExpectFlippedModuleValid(R"(#version 330 core
out vec4 color;
void main() { color = vec4(gl_FragCoord.y / 720.0, 0.0, 0.0, 1.0); }
)");
}
TEST(FragCoordYFlipTest, ComponentAndWholeVectorReadsStayValid) {
ExpectFlippedModuleValid(R"(#version 330 core
out vec4 color;
uniform float limit;
void main() {
float acc = 0.0;
for (int i = 0; i < 4; ++i) {
if (gl_FragCoord.x > limit) acc += gl_FragCoord.y;
}
color = vec4(gl_FragCoord.xy, acc, 1.0);
}
)");
}