From 6e2a3b349644f050d089dcdd60a917f934a76b46 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Fri, 21 Aug 2026 11:55:29 -0400 Subject: [PATCH] [Fix, Test] (GLState): stop enumerating buffer variables as GL uniforms --- .../GLImpl/Program/ProgramInterface.cpp | 27 +++++--- .../GLState/ProgramState/ProgramLinkTask.cpp | 20 ++++++ MobileGL/MG_Test/Program/ProgramTest.cpp | 63 +++++++++++++++++++ 3 files changed, 102 insertions(+), 8 deletions(-) diff --git a/MobileGL/MG_Impl/GLImpl/Program/ProgramInterface.cpp b/MobileGL/MG_Impl/GLImpl/Program/ProgramInterface.cpp index a6834dd6..3ecf1e5f 100644 --- a/MobileGL/MG_Impl/GLImpl/Program/ProgramInterface.cpp +++ b/MobileGL/MG_Impl/GLImpl/Program/ProgramInterface.cpp @@ -328,15 +328,25 @@ namespace MobileGL::MG_Impl::GLImpl::ProgramInterface { const ProgramObject::LinkArtifacts& reflection, Model& model, const Vector& blockKind, const Vector& blockInterfaceIndex) { - const Uint uniformCount = program.GetUniformCount(); - for (Uint glIndex = 0; glIndex < uniformCount; ++glIndex) { - const Int tIndex = program.TProgramUniformIndex(glIndex); + // Walks the TPROGRAM uniform space, not the GL one. A buffer variable is not a GL + // uniform (GL 4.6 core 7.3.1) and DoReflection therefore keeps it out of the GL + // active-uniform index space - but GL_BUFFER_VARIABLE still has to enumerate it, and + // this is the only place that does. GL uniforms keep their GL index as their + // GL_UNIFORM resource index: the GL space is a subsequence of this one, so pushing + // the GL-visible entries in this order preserves the correspondence. + const Int tUniformCount = static_cast(reflection.uniformReflection.size()); + for (Int tIndex = 0; tIndex < tUniformCount; ++tIndex) { const auto& refl = ProgramObject::UniformAtIn(reflection, tIndex); const auto& type = refl.type; const Int owner = refl.index; const BlockKind kind = (owner >= 0 && owner < static_cast(blockKind.size())) ? blockKind[owner] : BlockKind::GlobalUbo; + const Int glIndex = program.GlUniformIndexFromTProgram(tIndex); + // Everything except a buffer variable is enumerated through the GL space, so a + // uniform the relaxed parse swept out of it (a declared-but-dead default-block + // one) stays out of GL_UNIFORM too. + if (kind != BlockKind::Storage && glIndex < 0) continue; Resource resource; resource.name = refl.name; @@ -369,11 +379,12 @@ namespace MobileGL::MG_Impl::GLImpl::ProgramInterface { resource.atomicCounterBufferIndex = blockInterfaceIndex[owner]; resource.location = -1; } else { - resource.blockIndex = program.GetActiveUniformBlockIndex(glIndex); - resource.offset = program.GetActiveUniformOffset(glIndex); - resource.arrayStride = program.GetActiveUniformArrayStride(glIndex); - resource.matrixStride = program.GetActiveUniformMatrixStride(glIndex); - resource.isRowMajor = program.GetActiveUniformIsRowMajor(glIndex); + const Uint glUniformIndex = static_cast(glIndex); + resource.blockIndex = program.GetActiveUniformBlockIndex(glUniformIndex); + resource.offset = program.GetActiveUniformOffset(glUniformIndex); + resource.arrayStride = program.GetActiveUniformArrayStride(glUniformIndex); + resource.matrixStride = program.GetActiveUniformMatrixStride(glUniformIndex); + resource.isRowMajor = program.GetActiveUniformIsRowMajor(glUniformIndex); // A member of a named uniform block has no location, whatever the // frontend's own location table says (it hands one out to every uniform // so glUniform* can address block members through the global UBO). diff --git a/MobileGL/MG_State/GLState/ProgramState/ProgramLinkTask.cpp b/MobileGL/MG_State/GLState/ProgramState/ProgramLinkTask.cpp index 7189ed0e..23d8a66f 100644 --- a/MobileGL/MG_State/GLState/ProgramState/ProgramLinkTask.cpp +++ b/MobileGL/MG_State/GLState/ProgramState/ProgramLinkTask.cpp @@ -1081,6 +1081,20 @@ namespace MobileGL::MG_State::GLState { const auto isNamedBlockMember = [&isGlobalUboMember](const glslang::TObjectReflection& uniform) { return uniform.index >= 0 && !isGlobalUboMember(uniform); }; + // A member of a BUFFER block is a buffer variable, not a uniform: GL 4.6 core 7.3.1 + // gives it the GL_BUFFER_VARIABLE interface and 7.6 keeps it out of GL_ACTIVE_UNIFORMS, + // glGetActiveUniform, glGetUniformIndices and glGetActiveUniformsiv. The relaxed parse + // reflects it as a uniform anyway (no EShReflectionSeparateBuffers), so drop it from the + // GL index space here - the same place the dead default-block uniforms are dropped, and + // the counterpart of the location half already handled by isNamedBlockMember below. + // + // Atomic counters are NOT in this set even though their synthesized owner is a buffer + // block: an atomic_uint IS a uniform (of type GL_UNSIGNED_INT_ATOMIC_COUNTER), and + // KHR-GL43.shader_atomic_counters.basic-program-query enumerates it as one. + const auto isBufferVariable = [this](const glslang::TObjectReflection& uniform) { + if (uniform.index < 0 || uniform.index >= artifacts.program->getNumUniformBlocks()) return false; + return IsStorageBlock(artifacts.program->getUniformBlock(uniform.index)); + }; for (Int i = 0; i < tProgramUniformCount; i++) { const auto& uniform = artifacts.program->getUniform(i); if (isGlobalUboMember(uniform) && uniform.stages == 0) { @@ -1089,6 +1103,12 @@ namespace MobileGL::MG_State::GLState { in.externalIndex, uniform.name.c_str()); continue; } + if (isBufferVariable(uniform)) { + MGLOG_D("ProgramObject %u: Reflection - buffer variable '%s' filtered from the GL uniform " + "surface", + in.externalIndex, uniform.name.c_str()); + continue; + } artifacts.tProgramUniformIndexToGl[i] = static_cast(artifacts.glUniformIndexToTProgram.size()); artifacts.glUniformIndexToTProgram.push_back(i); } diff --git a/MobileGL/MG_Test/Program/ProgramTest.cpp b/MobileGL/MG_Test/Program/ProgramTest.cpp index df93cf2b..0827ab6e 100644 --- a/MobileGL/MG_Test/Program/ProgramTest.cpp +++ b/MobileGL/MG_Test/Program/ProgramTest.cpp @@ -2556,6 +2556,9 @@ TEST_F(ProgramTest, UniformBlockProgramInterfaceMatchesTheUniformBlockList) { EXPECT_EQ(GetProgramResourceIndex(program, GL_UNIFORM_BLOCK, "AVeryLongStorageBlockName"), GL_INVALID_INDEX); EXPECT_NE(GetProgramResourceIndex(program, GL_SHADER_STORAGE_BLOCK, "AVeryLongStorageBlockName"), GL_INVALID_INDEX); + // ... and the buffer variable by GL_BUFFER_VARIABLE, not GL_UNIFORM. + EXPECT_NE(GetProgramResourceIndex(program, GL_BUFFER_VARIABLE, "storageVec"), GL_INVALID_INDEX); + EXPECT_EQ(GetProgramResourceIndex(program, GL_UNIFORM, "storageVec"), GL_INVALID_INDEX); for (const char* blockName : {"Blk", "Blk2"}) { const GLuint interfaceIndex = GetProgramResourceIndex(program, GL_UNIFORM_BLOCK, blockName); @@ -2579,6 +2582,66 @@ TEST_F(ProgramTest, UniformBlockProgramInterfaceMatchesTheUniformBlockList) { EXPECT_EQ(GetError(), GL_NO_ERROR); } +// GL 4.6 core 7.3.1 / 7.6: a buffer variable is not a uniform - it lives in the +// GL_BUFFER_VARIABLE interface - so it must not appear in GL_ACTIVE_UNIFORMS, +// glGetActiveUniform, glGetUniformIndices or glGetActiveUniformsiv. An ATOMIC COUNTER, by +// contrast, IS a uniform (of type GL_UNSIGNED_INT_ATOMIC_COUNTER) and must stay enumerated. +TEST_F(ProgramTest, ActiveUniformsExcludeBufferVariablesButKeepAtomicCounters) { + GLuint program = LinkVsFsProgram(kMixedBlockKindsVs, kMixedBlockKindsFs); + + GLint activeUniforms = -1; + GetProgramiv(program, GL_ACTIVE_UNIFORMS, &activeUniforms); + ASSERT_EQ(activeUniforms, 4) + << "uboVec, uboVec2[0], uScale and counter - storageVec is a buffer variable"; + + // Neither spelling of the buffer variable is a uniform index. + EXPECT_EQ(UniformIndexByName(program, "storageVec"), GL_INVALID_INDEX); + EXPECT_EQ(UniformIndexByName(program, "AVeryLongStorageBlockName.storageVec"), GL_INVALID_INDEX); + // The location half of the same rule (already landed) must stay consistent with it. + EXPECT_EQ(GetUniformLocation(program, "storageVec"), -1); + + char nameBuf[128] = ""; + for (GLint i = 0; i < activeUniforms; ++i) { + GLsizei nameLen = 0; + GLint size = 0; + GLenum type = 0; + GetActiveUniform(program, static_cast(i), sizeof(nameBuf), &nameLen, &size, &type, nameBuf); + EXPECT_EQ(std::string(nameBuf).find("storageVec"), std::string::npos) + << "buffer variable enumerated as active uniform " << i << ": " << nameBuf; + } + + // The counter is still a uniform, still reports the atomic-counter type, has no owning + // uniform block, and still points at its atomic counter BUFFER. + const GLuint counter = UniformIndexByName(program, "counter"); + ASSERT_NE(counter, GL_INVALID_INDEX); + EXPECT_EQ(QueryUniformiv(program, counter, GL_UNIFORM_TYPE), + static_cast(GL_UNSIGNED_INT_ATOMIC_COUNTER)); + EXPECT_EQ(QueryUniformiv(program, counter, GL_UNIFORM_BLOCK_INDEX), -1); + EXPECT_EQ(QueryUniformiv(program, counter, GL_UNIFORM_ATOMIC_COUNTER_BUFFER_INDEX), 0); + EXPECT_EQ(QueryUniformiv(program, counter, GL_UNIFORM_OFFSET), 0); + EXPECT_EQ(GetUniformLocation(program, "counter"), -1); + + // GL_ACTIVE_ATOMIC_COUNTER_BUFFERS indexes into the GL uniform index space, so the + // counter index it reports has to be the one glGetUniformIndices just handed out. + GLint counterBuffers = -1; + GetProgramiv(program, GL_ACTIVE_ATOMIC_COUNTER_BUFFERS, &counterBuffers); + ASSERT_EQ(counterBuffers, 1); + GLint counterCount = -1; + GetActiveAtomicCounterBufferiv(program, 0, GL_ATOMIC_COUNTER_BUFFER_ACTIVE_ATOMIC_COUNTERS, &counterCount); + ASSERT_EQ(counterCount, 1); + GLint counterIndex = -1; + GetActiveAtomicCounterBufferiv(program, 0, GL_ATOMIC_COUNTER_BUFFER_ACTIVE_ATOMIC_COUNTER_INDICES, + &counterIndex); + EXPECT_EQ(static_cast(counterIndex), counter); + GLint counterBinding = -1; + GetActiveAtomicCounterBufferiv(program, 0, GL_ATOMIC_COUNTER_BUFFER_BINDING, &counterBinding); + EXPECT_EQ(counterBinding, 1); + + // The default-block uniform is untouched by either filter. + EXPECT_NE(GetUniformLocation(program, "uScale"), -1); + EXPECT_EQ(GetError(), GL_NO_ERROR); +} + TEST_F(ProgramTest, DeleteShaderWhileAttachedKeepsNameUsableUntilDetach) { // GL CTS compiles through exactly this sequence (create, attach, DELETE, source, // compile): glDeleteShader on an attached shader only flags it, and the name must