From ea4819a21d4dbb5b0d2db4fd3edee04a7ed4eeea Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Wed, 12 Aug 2026 07:36:12 -0400 Subject: [PATCH] [Fix, Test] (MG_State, MG_IntegrationTest): an array vertex input occupies one location per element, not one location in total --- .../Scenarios/VertexAttribBindingScenario.cpp | 68 +++++++++++++++++++ .../GLState/ProgramState/ProgramLinkTask.cpp | 26 ++++++- 2 files changed, 92 insertions(+), 2 deletions(-) diff --git a/MobileGL/MG_IntegrationTest/Scenarios/VertexAttribBindingScenario.cpp b/MobileGL/MG_IntegrationTest/Scenarios/VertexAttribBindingScenario.cpp index cd2ae3c3..c67b5796 100644 --- a/MobileGL/MG_IntegrationTest/Scenarios/VertexAttribBindingScenario.cpp +++ b/MobileGL/MG_IntegrationTest/Scenarios/VertexAttribBindingScenario.cpp @@ -462,4 +462,72 @@ void main() { glDeleteProgram(program); } + // Same program, but every one of the 16 elements is asked for a DIFFERENT current value. + // + // An input array occupies one location per element (GL 4.6 core 11.1.1), so `in vec4 a[16]` + // at location 0 is active on 0..15 - and the whole location span is what a backend reads to + // decide which attributes need their current value pushed. Reflection used to record the + // span of the ELEMENT type only, so a 16-element array claimed exactly one location: every + // element above the first silently read the (0,0,0,1) an unwritten input defaults to instead + // of the value glVertexAttrib4f had set. The test above could not see it, because the only + // element it reads a current value from is element 0 - the one location the array did claim. + TEST_F(VertexAttribBindingScenario, EveryInputArrayElementGetsItsOwnCurrentValue) { + if (!Ready()) GTEST_SKIP(); + + const std::string vs = R"(#version 430 core +layout(location = 0) in vec4 vs_in_attrib[16]; +out StageData { + vec4 attrib[16]; +} vs_out; +void main() { + for (int i = 0; i < vs_in_attrib.length(); ++i) { + vs_out.attrib[i] = vs_in_attrib[i]; + } +} +)"; + std::vector names; + for (int i = 0; i < 16; ++i) names.push_back("StageData.attrib[" + std::to_string(i) + "]"); + std::vector varyings; + for (const auto& n : names) varyings.push_back(n.c_str()); + + std::string log; + const GLuint program = BuildCaptureProgram(vs, varyings, &log); + ASSERT_NE(program, 0u) << "capture program did not link: " << log; + + // Distinct in every component, and never (0,0,0,1): the value an element that was + // skipped would report has to be distinguishable from every value that was asked for. + for (GLuint i = 0; i < 16; ++i) { + const float base = static_cast(i) + 1.0f; + glVertexAttrib4f(i, base, base + 100.0f, base + 200.0f, base + 300.0f); + } + + constexpr std::size_t kFloatsPerPoint = 64; + std::vector poison(kFloatsPerPoint, kPoison); + glBindBufferBase(GL_TRANSFORM_FEEDBACK_BUFFER, 0, m_xfbo); + glBufferData(GL_TRANSFORM_FEEDBACK_BUFFER, static_cast(poison.size() * sizeof(float)), + poison.data(), GL_DYNAMIC_DRAW); + glEnable(GL_RASTERIZER_DISCARD); + glUseProgram(program); + glBeginTransformFeedback(GL_POINTS); + glDrawArrays(GL_POINTS, 0, 1); + glEndTransformFeedback(); + glDisable(GL_RASTERIZER_DISCARD); + + std::vector data(poison.size(), kPoison); + glGetBufferSubData(GL_TRANSFORM_FEEDBACK_BUFFER, 0, + static_cast(data.size() * sizeof(float)), data.data()); + glUseProgram(0); + + for (int element = 0; element < 16; ++element) { + const float base = static_cast(element) + 1.0f; + EXPECT_FLOAT_EQ(data[element * 4 + 0], base) << "element " << element; + EXPECT_FLOAT_EQ(data[element * 4 + 1], base + 100.0f) << "element " << element; + EXPECT_FLOAT_EQ(data[element * 4 + 2], base + 200.0f) << "element " << element; + EXPECT_FLOAT_EQ(data[element * 4 + 3], base + 300.0f) << "element " << element; + } + + for (GLuint i = 0; i < 16; ++i) glVertexAttrib4f(i, 0.0f, 0.0f, 0.0f, 0.0f); + glDeleteProgram(program); + } + } // namespace MGITest diff --git a/MobileGL/MG_State/GLState/ProgramState/ProgramLinkTask.cpp b/MobileGL/MG_State/GLState/ProgramState/ProgramLinkTask.cpp index 63ad6ccb..9194d3ea 100644 --- a/MobileGL/MG_State/GLState/ProgramState/ProgramLinkTask.cpp +++ b/MobileGL/MG_State/GLState/ProgramState/ProgramLinkTask.cpp @@ -68,6 +68,8 @@ namespace { return type && type->getQualifier().builtIn != glslang::EbvNone; } + // Locations one ELEMENT of a vertex input occupies (GL 4.6 core 11.1.1): a matrix + // takes one per column, everything else this backend can feed takes one. static int GetVertexInputLocationSpan(GLenum glType) { switch (glType) { case GL_FLOAT_MAT2: @@ -87,6 +89,26 @@ namespace { } } + // How many elements an ARRAY vertex input has. glslang reflects such an input as ONE + // record spelled "name[0]" carrying the ELEMENT's glDefineType and the array length, + // so the type alone cannot say how many locations the declaration covers: GL 4.6 core + // 11.1.1 gives an array one location per element (times the element's own span), and + // `in vec4 a[16]` at location 0 therefore occupies 0..15, not 0. Missing that left + // every location above the base with no recorded name or type, which is what the + // backends read to decide whether an attribute is active at all. + static MobileGL::Int GetVertexInputArrayElements(const glslang::TObjectReflection& input) { + const glslang::TType* type = input.getType(); + if (type == nullptr || !type->isArray()) return 1; + // An unsized input array has no span to compute; treat it as one element rather + // than guessing, so it can only ever under-claim locations. + if (!type->isSizedArray()) return 1; + return std::max(1, type->getCumulativeArraySize()); + } + + static MobileGL::Int GetVertexInputTotalLocationSpan(const glslang::TObjectReflection& input) { + return GetVertexInputLocationSpan(input.glDefineType) * GetVertexInputArrayElements(input); + } + static GLenum GetVertexInputLocationType(GLenum glType) { switch (glType) { case GL_FLOAT_MAT2: @@ -878,7 +900,7 @@ namespace MobileGL::MG_State::GLState { for (int i = 0; i < inCount; ++i) { Int loc = (Int)artifacts.program->getPipeInput(i).layoutLocation(); if (loc >= 0 && loc != glslang::TQualifier::layoutLocationEnd) { - const Int locationSpan = GetVertexInputLocationSpan(artifacts.program->getPipeInput(i).glDefineType); + const Int locationSpan = GetVertexInputTotalLocationSpan(artifacts.program->getPipeInput(i)); maxLoc = std::max(maxLoc, loc + locationSpan - 1); } MGLOG_D("ProgramObject %u: Reflection - pipe input[%d] name='%s' layoutLocation=%d glType=%u", @@ -914,7 +936,7 @@ namespace MobileGL::MG_State::GLState { (Int)ProgramObject::NormalizeBuiltinPipeInputName(inVar.name).length()); if (location >= 0 && location < (int)artifacts.attribs.size()) { - const Int locationSpan = GetVertexInputLocationSpan(inVar.glDefineType); + const Int locationSpan = GetVertexInputTotalLocationSpan(inVar); const GLenum locationType = GetVertexInputLocationType(inVar.glDefineType); for (Int locationOffset = 0; locationOffset < locationSpan; ++locationOffset) { const Int expandedLocation = location + locationOffset;