[Fix, Test] (MG_State, MG_Backend/DirectVulkan): resolve transform-feedback captures that name a member of an output interface block

This commit is contained in:
2026-08-11 08:39:39 -04:00
parent 5a7bd9942d
commit 964a7fcc92
5 changed files with 433 additions and 4 deletions
@@ -12,7 +12,10 @@
#include "MG_Util/ShaderTranspiler/ShaderCompiler.h"
#include "MG_Util/ShaderTranspiler/SpvcSession.h"
#include "MG_Util/ShaderTranspiler/Types.h"
#include <algorithm>
#include <cstring>
#include <map>
#include <utility>
#include <spirv-tools/libspirv.h>
#include <spirv-tools/optimizer.hpp>
#include <source/opt/build_module.h>
@@ -1131,6 +1134,15 @@ namespace MobileGL::MG_Backend::DirectVulkan {
std::string name;
Uint32 bufferIndex = 0;
Uint32 offsetBytes = 0;
// Set when the capture names a member of an output interface block
// ("Block.member"): the decoration target is then the block's struct TYPE,
// decorated per member, not the variable. `name` keeps the GL spelling and
// is useless for the id lookup, so the instance name is carried separately.
std::string blockInstanceName;
std::string blockName;
Int blockMemberIndex = -1;
Int blockMemberElement = -1; // array element of that member, -1 = the whole member
Uint32 byteSize = 0;
};
const char* name() const override { return "mobilegl-xfb-capture-decorate"; }
XfbCaptureDecoratePass(Vector<CapturedVarying> varyings, Vector<Uint32> strides)
@@ -1162,6 +1174,33 @@ namespace MobileGL::MG_Backend::DirectVulkan {
decorationManager->AddDecorationVal(targetId, static_cast<Uint32>(spv::Decoration::Offset),
offsetBytes);
};
// SPIR-V puts XfbBuffer/XfbStride/Offset on the struct MEMBER when the
// captured varying lives in an interface block (SPIR-V 1.6 §3.20 lists all
// three as member-decoratable); Offset in particular is illegal on the block
// variable once the type is decorated Block.
const auto decorateMemberForXfb = [&](Uint32 structTypeId, Uint32 memberIndex, Uint32 bufferIndex,
Uint32 offsetBytes) {
const Uint32 stride = bufferIndex < m_strides.size() ? m_strides[bufferIndex] : 0;
decorationManager->AddMemberDecoration(structTypeId, memberIndex,
static_cast<Uint32>(spv::Decoration::XfbBuffer),
bufferIndex);
decorationManager->AddMemberDecoration(structTypeId, memberIndex,
static_cast<Uint32>(spv::Decoration::XfbStride), stride);
decorationManager->AddMemberDecoration(structTypeId, memberIndex,
static_cast<Uint32>(spv::Decoration::Offset), offsetBytes);
};
// A member array captured element by element ("Block.attrib[0]" .. "[15]")
// is one SPIR-V member, so its captures collapse into a single decoration
// placed at the first element's offset - the rest follow from the member's
// own layout. Collected first so the group is complete before it decorates.
struct MemberGroup {
Uint32 bufferIndex = 0;
Uint32 minOffset = 0;
Uint32 elementBytes = 0;
Vector<Uint32> offsets;
};
std::map<std::pair<Uint32, Uint32>, MemberGroup> memberGroups;
Bool modified = false;
Bool needsPositionMirror = false;
@@ -1174,6 +1213,41 @@ namespace MobileGL::MG_Backend::DirectVulkan {
positionOffset = varying.offsetBytes;
continue;
}
if (varying.blockMemberIndex >= 0) {
// glslang names the block's instance variable and its struct type
// separately; an anonymous instance leaves only the type named, so
// both spellings are tried before giving up.
Uint32 structTypeId = 0;
if (const auto it = idsByName.find(varying.blockInstanceName); it != idsByName.end()) {
structTypeId = BlockStructTypeOf(it->second);
}
if (structTypeId == 0) {
if (const auto it = idsByName.find(varying.blockName); it != idsByName.end()) {
const spvtools::opt::Instruction* def = context()->get_def_use_mgr()->GetDef(it->second);
if (def != nullptr && def->opcode() == spv::Op::OpTypeStruct) {
structTypeId = it->second;
} else if (def != nullptr && def->opcode() == spv::Op::OpVariable) {
structTypeId = BlockStructTypeOf(it->second);
}
}
}
if (structTypeId == 0) {
MGLOG_E("XfbCaptureDecoratePass: no SPIR-V interface block '%s' (instance '%s') for "
"capture '%s'",
varying.blockName.c_str(), varying.blockInstanceName.c_str(),
varying.name.c_str());
continue;
}
auto& group =
memberGroups[{structTypeId, static_cast<Uint32>(varying.blockMemberIndex)}];
if (group.offsets.empty() || varying.offsetBytes < group.minOffset) {
group.minOffset = varying.offsetBytes;
}
group.bufferIndex = varying.bufferIndex;
group.elementBytes = varying.byteSize;
group.offsets.push_back(varying.offsetBytes);
continue;
}
const auto idIt = idsByName.find(varying.name);
if (idIt == idsByName.end()) {
MGLOG_E("XfbCaptureDecoratePass: no SPIR-V variable named '%s'", varying.name.c_str());
@@ -1183,6 +1257,25 @@ namespace MobileGL::MG_Backend::DirectVulkan {
modified = true;
}
for (auto& [key, group] : memberGroups) {
// The single Offset can only stand for the whole group when the group's
// captures are a gap-free ascending run - that is what SPIR-V lays the
// member's elements out as. Anything else still gets a best-effort
// decoration, but say so, because the capture layout will not match GL.
std::sort(group.offsets.begin(), group.offsets.end());
for (SizeT i = 1; i < group.offsets.size(); ++i) {
if (group.elementBytes == 0 ||
group.offsets[i] != group.offsets[i - 1] + group.elementBytes) {
MGLOG_I("XfbCaptureDecoratePass: block member %u of type %%%u is captured with a "
"non-contiguous element set; the capture layout will differ from GL's",
key.second, key.first);
break;
}
}
decorateMemberForXfb(key.first, key.second, group.bufferIndex, group.minOffset);
modified = true;
}
if (needsPositionMirror) {
modified |= MirrorPositionForCapture(entryFunctionId, *entryPoint, positionBufferIndex,
positionOffset, decorateForXfb);
@@ -1204,6 +1297,27 @@ namespace MobileGL::MG_Backend::DirectVulkan {
}
private:
// The struct type an interface-block variable points at, peeling an array of
// block instances on the way. 0 when the id is not a block variable at all.
Uint32 BlockStructTypeOf(Uint32 variableId) {
auto* defUse = context()->get_def_use_mgr();
const spvtools::opt::Instruction* variable = defUse->GetDef(variableId);
if (variable == nullptr || variable->opcode() != spv::Op::OpVariable) return 0;
const spvtools::opt::Instruction* pointer = defUse->GetDef(variable->type_id());
if (pointer == nullptr || pointer->opcode() != spv::Op::OpTypePointer) return 0;
Uint32 pointeeId = pointer->GetSingleWordInOperand(1);
for (const spvtools::opt::Instruction* pointee = defUse->GetDef(pointeeId); pointee != nullptr;
pointee = defUse->GetDef(pointeeId)) {
if (pointee->opcode() == spv::Op::OpTypeStruct) return pointeeId;
if (pointee->opcode() != spv::Op::OpTypeArray &&
pointee->opcode() != spv::Op::OpTypeRuntimeArray) {
return 0;
}
pointeeId = pointee->GetSingleWordInOperand(0);
}
return 0;
}
template <typename DecorateFn>
Bool MirrorPositionForCapture(Uint32 entryFunctionId, spvtools::opt::Instruction& entryPoint,
Uint32 bufferIndex, Uint32 offsetBytes, const DecorateFn& decorateForXfb) {
@@ -1522,7 +1636,9 @@ namespace MobileGL::MG_Backend::DirectVulkan {
Vector<XfbCaptureDecoratePass::CapturedVarying> varyings;
varyings.reserve(program.GetTransformFeedbackVaryingCount());
for (const auto& varying : program.GetTransformFeedbackVaryings()) {
varyings.push_back({varying.name, varying.bufferIndex, varying.offsetBytes});
varyings.push_back({varying.name, varying.bufferIndex, varying.offsetBytes,
varying.blockInstanceName, varying.blockName, varying.blockMemberIndex,
varying.blockMemberElement, varying.byteSize});
}
Vector<Uint32> strides;
strides.reserve(program.GetTransformFeedbackBufferCount());
@@ -1030,21 +1030,80 @@ namespace MobileGL::MG_State::GLState {
}
}
}
// GL 4.6 core 11.1.2.1 (and the resource-name rule of 7.3.1.1): a member of
// an output interface block is named "<BLOCK name>.<member>" - the block's
// TYPE name, never the instance name, and that holds for an anonymous
// instance too. glslang's linker object for such a block is the *instance*
// symbol ("vs_out", or "anon@N" when there is none), so the head of the
// dotted path has to be matched against getType().getTypeName() instead of
// getName(). Without this every capture of a block member resolved to
// nothing and the link failed with "is not an output of the vertex stage".
String blockName;
String memberName;
if (const SizeT dot = declaredName.find('.'); dot != String::npos) {
blockName = declaredName.substr(0, dot);
memberName = declaredName.substr(dot + 1);
// An array of block instances is spelled "<block>[i].<member>"; every
// instance shares one member list, so the subscript only has to go.
if (!blockName.empty() && blockName.back() == ']') {
const SizeT bracket = blockName.rfind('[');
if (bracket != String::npos) blockName.resize(bracket);
}
}
for (const auto* node : linkerObjects->getSequence()) {
const glslang::TIntermSymbol* symbol = node->getAsSymbolNode();
if (symbol == nullptr || symbol->getType().getQualifier().storage != glslang::EvqVaryingOut) {
continue;
}
if (symbol->getName() != declaredName.c_str()) {
continue;
const glslang::TType& symbolType = symbol->getType();
const glslang::TType* capturedType = nullptr;
if (memberName.empty()) {
if (symbol->getName() != declaredName.c_str()) {
continue;
}
capturedType = &symbolType;
} else {
if (symbolType.getBasicType() != glslang::EbtBlock) {
continue;
}
// The spec spelling is the block name; the instance name is accepted
// as a fallback so a request written the (common, non-conformant)
// instance-qualified way resolves instead of failing the whole link.
if (symbolType.getTypeName() != blockName.c_str() &&
symbol->getName() != blockName.c_str()) {
continue;
}
const glslang::TTypeList* members = symbolType.getStruct();
if (members == nullptr) {
continue;
}
for (SizeT m = 0; m < members->size(); ++m) {
const glslang::TType* memberType = (*members)[m].type;
if (memberType == nullptr || memberType->getFieldName() != memberName.c_str()) {
continue;
}
capturedType = memberType;
varying.blockMemberIndex = static_cast<Int>(m);
break;
}
if (capturedType == nullptr) {
// Right block, wrong member: no other linker object can match.
break;
}
varying.blockName = symbolType.getTypeName().c_str();
varying.blockInstanceName = symbol->getName().c_str();
}
resolved = ResolveXfbSymbolType(symbol->getType(), varying.type, varying.size, bytesPerElement);
resolved = ResolveXfbSymbolType(*capturedType, varying.type, varying.size, bytesPerElement);
if (resolved && singleElement) {
if (static_cast<Int>(element) >= varying.size) {
resolved = false;
break;
}
varying.size = 1;
if (varying.blockMemberIndex >= 0) {
varying.blockMemberElement = static_cast<Int>(element);
}
}
break;
}
@@ -672,6 +672,21 @@ namespace MobileGL::MG_State::GLState {
// Offset within the gap-free record a backend that cannot express the GL
// layout captures into; see NeedsScatteredTransformFeedbackCapture.
Uint32 packedOffsetBytes = 0;
// GL 4.6 core 11.1.2.1 / 7.3.1.1: a member of an output interface block is
// captured under "<block name>.<member>". `name` keeps that GL spelling (it is
// what the interface queries and the ESSL backend's driver-side capture list
// need, since SPIRV-Cross re-emits the block under its own type name), while
// the three fields below carry what a SPIR-V backend needs instead: the
// decoration target is the block's *instance* variable and the member index
// inside it. blockMemberIndex < 0 means "not a block member".
String blockInstanceName;
String blockName;
Int blockMemberIndex = -1;
// Which element of an arrayed block member this capture names, -1 for "the
// member as a whole". SPIR-V cannot decorate a single array element, so a
// backend needs the element index to tell a full run from a partial one.
Int blockMemberElement = -1;
};
// ---- P1: everything a link PRODUCES, in one movable block ----
+17
View File
@@ -164,6 +164,22 @@ target_link_libraries(
${LINK_LIBRARIES}
)
add_executable(
XfbBlockVaryingTest
XfbBlockVaryingTest.cpp
)
target_include_directories(XfbBlockVaryingTest PRIVATE
${MGL_ROOT}/include
${MGL_ROOT}/MobileGL
)
target_link_libraries(
XfbBlockVaryingTest PRIVATE
GTest::gtest_main
${LINK_LIBRARIES}
)
add_executable(
ProgramInterfaceTest
ProgramInterfaceTest.cpp
@@ -195,6 +211,7 @@ include(GoogleTest)
gtest_discover_tests(ProgramUtilTest DISCOVERY_TIMEOUT 30 PROPERTIES LABELS unit)
gtest_discover_tests(ProgramTest DISCOVERY_TIMEOUT 30 PROPERTIES LABELS unit)
gtest_discover_tests(ProgramInterfaceTest DISCOVERY_TIMEOUT 30 PROPERTIES LABELS unit)
gtest_discover_tests(XfbBlockVaryingTest DISCOVERY_TIMEOUT 30 PROPERTIES LABELS unit)
# Heavier than the rest of the unit suite by design: several cases deliberately saturate the
# compile pool so there is something in flight to race against.
gtest_discover_tests(AsyncCompileTest DISCOVERY_TIMEOUT 60 PROPERTIES LABELS unit TIMEOUT 300)
@@ -0,0 +1,222 @@
// MobileGL - MobileGL/MG_Test/Program/XfbBlockVaryingTest.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
// Transform-feedback capture of a member of an output interface block.
//
// GL 4.6 core 11.1.2.1 names such a varying "<BLOCK name>.<member>" - the block's TYPE
// name, never the instance name - which is exactly what KHR-GL4x.vertex_attrib_binding
// (gl4cVertexAttribBindingTests.cpp:419-437, `out StageData { vec4 attrib[16]; } vs_out;`
// captured as "StageData.attrib[0]".."[15]") relies on. The resolver used to match the
// requested name against glslang's linker-object symbol name, which for a block is the
// INSTANCE ("vs_out"), so every one of those captures came back unresolved and the link
// failed with "is not an output of the vertex stage" + GL_INVALID_VALUE.
//
// GPU-free: everything asserted here is a property of the link, not of any driver.
#include <gtest/gtest.h>
#include <string>
#include <vector>
#include "Includes.h"
#include "Init.h"
#include "MG_Impl/GLImpl/Getter/GL_Getter.h"
#include "MG_Impl/GLImpl/Program/GL_Program.h"
#include "MG_State/GLState/Core.h"
using namespace MobileGL;
using namespace MobileGL::MG_Impl::GLImpl;
namespace {
class XfbBlockVaryingTest : public ::testing::Test {
protected:
void SetUp() override { MobileGL::Initialize(); }
};
GLuint MakeVsOnlyProgram(const char* vs) {
const GLuint program = CreateProgram();
const GLuint shader = CreateShader(GL_VERTEX_SHADER);
ShaderSource(shader, 1, &vs, nullptr);
CompileShader(shader);
GLint compiled = GL_FALSE;
GetShaderiv(shader, GL_COMPILE_STATUS, &compiled);
EXPECT_EQ(compiled, GL_TRUE) << [&] {
char log[4096] = "";
GetShaderInfoLog(shader, sizeof(log), nullptr, log);
return std::string(log);
}();
AttachShader(program, shader);
return program;
}
std::string LinkLog(GLuint program) {
char log[4096] = "";
GetProgramInfoLog(program, sizeof(log), nullptr, log);
return std::string(log);
}
GLint Programiv(GLuint program, GLenum pname) {
GLint value = -1;
GetProgramiv(program, pname, &value);
return value;
}
struct VaryingRecord {
std::string name;
GLsizei size = 0;
GLenum type = 0;
};
VaryingRecord Varying(GLuint program, GLuint index) {
VaryingRecord record;
GLchar buffer[256] = {'\0'};
GLsizei length = 0;
GetTransformFeedbackVarying(program, index, sizeof(buffer), &length, &record.size, &record.type, buffer);
record.name.assign(buffer, buffer + (length < 0 ? 0 : length));
return record;
}
void ClearErrors() {
for (int i = 0; i < 32 && GetError() != GL_NO_ERROR; ++i) {
}
}
// The CTS shader, narrowed to two elements so the expectations stay readable.
const char* kNamedBlockVs = R"(#version 430 core
layout(location = 0) in vec4 vs_in_attrib[2];
out StageData {
vec4 attrib[2];
} vs_out;
void main() {
for (int i = 0; i < vs_in_attrib.length(); ++i) {
vs_out.attrib[i] = vs_in_attrib[i];
}
}
)";
TEST_F(XfbBlockVaryingTest, CapturesBlockMemberElementsByBlockTypeName) {
ClearErrors();
const GLuint program = MakeVsOnlyProgram(kNamedBlockVs);
const GLchar* const varyings[2] = {"StageData.attrib[0]", "StageData.attrib[1]"};
TransformFeedbackVaryings(program, 2, varyings, GL_INTERLEAVED_ATTRIBS);
LinkProgram(program);
ASSERT_EQ(Programiv(program, GL_LINK_STATUS), GL_TRUE) << LinkLog(program);
EXPECT_EQ(GetError(), GL_NO_ERROR);
EXPECT_EQ(Programiv(program, GL_TRANSFORM_FEEDBACK_VARYINGS), 2);
EXPECT_EQ(Programiv(program, GL_TRANSFORM_FEEDBACK_BUFFER_MODE), GL_INTERLEAVED_ATTRIBS);
for (GLuint i = 0; i < 2; ++i) {
const VaryingRecord record = Varying(program, i);
EXPECT_EQ(record.name, std::string("StageData.attrib[") + std::to_string(i) + "]");
// One element of the member array, not the whole array.
EXPECT_EQ(record.size, 1) << "index " << i;
EXPECT_EQ(record.type, static_cast<GLenum>(GL_FLOAT_VEC4)) << "index " << i;
}
}
// The whole member, no subscript: the array size has to survive.
TEST_F(XfbBlockVaryingTest, CapturesAWholeBlockMemberArray) {
ClearErrors();
const GLuint program = MakeVsOnlyProgram(kNamedBlockVs);
const GLchar* const varyings[1] = {"StageData.attrib"};
TransformFeedbackVaryings(program, 1, varyings, GL_INTERLEAVED_ATTRIBS);
LinkProgram(program);
ASSERT_EQ(Programiv(program, GL_LINK_STATUS), GL_TRUE) << LinkLog(program);
const VaryingRecord record = Varying(program, 0);
EXPECT_EQ(record.name, "StageData.attrib");
EXPECT_EQ(record.size, 2);
EXPECT_EQ(record.type, static_cast<GLenum>(GL_FLOAT_VEC4));
}
// Members of an anonymous instance are named the same way - the block name is still
// what identifies them, and there is no instance name to fall back on.
TEST_F(XfbBlockVaryingTest, CapturesAnonymousInstanceBlockMember) {
ClearErrors();
const GLuint program = MakeVsOnlyProgram(R"(#version 430 core
layout(location = 0) in vec4 vs_in_attrib;
out StageData {
vec4 color;
vec2 uv;
};
void main() {
color = vs_in_attrib;
uv = vs_in_attrib.xy;
}
)");
const GLchar* const varyings[2] = {"StageData.color", "StageData.uv"};
TransformFeedbackVaryings(program, 2, varyings, GL_INTERLEAVED_ATTRIBS);
LinkProgram(program);
ASSERT_EQ(Programiv(program, GL_LINK_STATUS), GL_TRUE) << LinkLog(program);
EXPECT_EQ(Varying(program, 0).type, static_cast<GLenum>(GL_FLOAT_VEC4));
EXPECT_EQ(Varying(program, 1).type, static_cast<GLenum>(GL_FLOAT_VEC2));
}
// The instance-qualified spelling is not what the spec asks for, but it is what a lot of
// application code writes; resolving it too costs nothing and keeps those links alive.
TEST_F(XfbBlockVaryingTest, AlsoAcceptsTheInstanceQualifiedSpelling) {
ClearErrors();
const GLuint program = MakeVsOnlyProgram(kNamedBlockVs);
const GLchar* const varyings[1] = {"vs_out.attrib[1]"};
TransformFeedbackVaryings(program, 1, varyings, GL_INTERLEAVED_ATTRIBS);
LinkProgram(program);
ASSERT_EQ(Programiv(program, GL_LINK_STATUS), GL_TRUE) << LinkLog(program);
EXPECT_EQ(Varying(program, 0).size, 1);
EXPECT_EQ(Varying(program, 0).type, static_cast<GLenum>(GL_FLOAT_VEC4));
}
// A dotted path that resolves to nothing must still fail the link, and say so - the
// fix must not turn "unknown member" into a silently dropped capture.
TEST_F(XfbBlockVaryingTest, RejectsAnUnknownBlockMember) {
ClearErrors();
const GLuint program = MakeVsOnlyProgram(kNamedBlockVs);
const GLchar* const varyings[1] = {"StageData.missing"};
TransformFeedbackVaryings(program, 1, varyings, GL_INTERLEAVED_ATTRIBS);
LinkProgram(program);
EXPECT_EQ(Programiv(program, GL_LINK_STATUS), GL_FALSE);
EXPECT_NE(LinkLog(program).find("StageData.missing"), std::string::npos) << LinkLog(program);
}
TEST_F(XfbBlockVaryingTest, RejectsAnUnknownBlock) {
ClearErrors();
const GLuint program = MakeVsOnlyProgram(kNamedBlockVs);
const GLchar* const varyings[1] = {"NoSuchBlock.attrib[0]"};
TransformFeedbackVaryings(program, 1, varyings, GL_INTERLEAVED_ATTRIBS);
LinkProgram(program);
EXPECT_EQ(Programiv(program, GL_LINK_STATUS), GL_FALSE);
}
// Plain (non-block) outputs must keep resolving exactly as before.
TEST_F(XfbBlockVaryingTest, StillResolvesPlainOutputs) {
ClearErrors();
const GLuint program = MakeVsOnlyProgram(R"(#version 430 core
layout(location = 0) in vec4 vs_in_attrib;
out vec4 plain[2];
out vec3 single;
void main() {
plain[0] = vs_in_attrib;
plain[1] = vs_in_attrib;
single = vs_in_attrib.xyz;
}
)");
const GLchar* const varyings[3] = {"plain[1]", "single", "gl_Position"};
TransformFeedbackVaryings(program, 3, varyings, GL_INTERLEAVED_ATTRIBS);
LinkProgram(program);
ASSERT_EQ(Programiv(program, GL_LINK_STATUS), GL_TRUE) << LinkLog(program);
EXPECT_EQ(Varying(program, 0).size, 1);
EXPECT_EQ(Varying(program, 0).type, static_cast<GLenum>(GL_FLOAT_VEC4));
EXPECT_EQ(Varying(program, 1).type, static_cast<GLenum>(GL_FLOAT_VEC3));
EXPECT_EQ(Varying(program, 2).type, static_cast<GLenum>(GL_FLOAT_VEC4));
}
} // namespace