From 7a7340ebe2625103f2769bfdf068894f1f904cae Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Fri, 21 Aug 2026 00:07:07 -0400 Subject: [PATCH] [Fix, Test] (GLImpl, ProgramState): answer the classic uniform queries for atomic counters at GL level --- .../MG_Impl/GLImpl/Program/GL_Program.cpp | 9 +++ .../GLState/ProgramState/ProgramObject.h | 42 ++++++++++ .../MG_Test/Program/ProgramInterfaceTest.cpp | 80 +++++++++++++++++++ 3 files changed, 131 insertions(+) diff --git a/MobileGL/MG_Impl/GLImpl/Program/GL_Program.cpp b/MobileGL/MG_Impl/GLImpl/Program/GL_Program.cpp index da1f5732..4a151591 100644 --- a/MobileGL/MG_Impl/GLImpl/Program/GL_Program.cpp +++ b/MobileGL/MG_Impl/GLImpl/Program/GL_Program.cpp @@ -525,6 +525,10 @@ namespace MobileGL::MG_Impl::GLImpl { case GL_UNIFORM_ARRAY_STRIDE: case GL_UNIFORM_MATRIX_STRIDE: case GL_UNIFORM_IS_ROW_MAJOR: + // GL 4.2 / ARB_shader_atomic_counters adds this one to the accepted set. Leaving it + // out did not merely lose the answer: the leftover GL_INVALID_ENUM is what made + // KHR-GL43.shader_atomic_counters.basic-program-query force a FAIL. + case GL_UNIFORM_ATOMIC_COUNTER_BUFFER_INDEX: break; default: MG_State::pGLContext->RecordError( @@ -580,6 +584,11 @@ namespace MobileGL::MG_Impl::GLImpl { case GL_UNIFORM_IS_ROW_MAJOR: params[i] = programObject->GetActiveUniformIsRowMajor(idx); break; + case GL_UNIFORM_ATOMIC_COUNTER_BUFFER_INDEX: + // Index into the GL_ACTIVE_ATOMIC_COUNTER_BUFFERS list, -1 for every uniform + // that is not an atomic counter (GL 4.6 core table 7.6). + params[i] = programObject->GetActiveUniformAtomicCounterBufferIndex(idx); + break; default: break; } diff --git a/MobileGL/MG_State/GLState/ProgramState/ProgramObject.h b/MobileGL/MG_State/GLState/ProgramState/ProgramObject.h index 2271b8b4..0febf712 100644 --- a/MobileGL/MG_State/GLState/ProgramState/ProgramObject.h +++ b/MobileGL/MG_State/GLState/ProgramState/ProgramObject.h @@ -225,6 +225,9 @@ namespace MobileGL::MG_State::GLState { } GLenum GetActiveUniformType(Uint index) const { + // The lowered counter is a plain uint inside a synthesized block; what the GL + // client declared - and what glGetActiveUniform must report - is an atomic_uint. + if (IsActiveUniformAtomicCounter(index)) return GL_UNSIGNED_INT_ATOMIC_COUNTER; auto& uniform = Artifacts().program->getUniform(TProgramUniformIndex(index)); return uniform.glDefineType; } @@ -244,10 +247,49 @@ namespace MobileGL::MG_State::GLState { Int GetActiveUniformBlockIndex(Uint index) const { auto& uniform = Artifacts().program->getUniform(TProgramUniformIndex(index)); + // An atomic counter is a DEFAULT-BLOCK uniform to GL, whatever block the + // transpiler lowered it onto (GL 4.6 core 7.6, table 7.6): -1. + if (IsActiveUniformAtomicCounter(index)) return -1; // Members of the synthesized global UBO are default-block uniforms to GL: -1. return GlBlockIndexFromTProgram(uniform.index); } + // The transpiler lowers every atomic_uint onto a synthesized gl_AtomicCounterBlock_N + // block, but GL keeps seeing an atomic counter as a default-block uniform of type + // GL_UNSIGNED_INT_ATOMIC_COUNTER that points at an atomic-counter BUFFER. These two + // answer for that GL-level declaration; without them the query surface reports the + // lowering instead (GL_UNSIGNED_INT, block index 0) and + // KHR-GL43.shader_atomic_counters.basic-program-query fails on both. + // + // The returned value is an index into the GL_ACTIVE_ATOMIC_COUNTER_BUFFERS list, i.e. + // the RANK of the owning counter block among the counter blocks in glslang's block + // order - exactly how ProgramInterface numbers the GL_ATOMIC_COUNTER_BUFFER + // resources glGetActiveAtomicCounterBufferiv answers from. -1 when this uniform is + // not an atomic counter. + Int GetActiveUniformAtomicCounterBufferIndex(Uint index) const { + const Int tIndex = TProgramUniformIndex(index); + if (tIndex < 0) return -1; + const Int owner = Artifacts().program->getUniform(tIndex).index; + if (owner < 0) return -1; + const Int blockCount = Artifacts().program->getNumUniformBlocks(); + if (owner >= blockCount) return -1; + const SizeT prefixLength = StringView(MG_Util::ShaderTranspiler::ATOMIC_COUNTER_BLOCK_PREFIX).size(); + Int counterBufferIndex = 0; + for (Int i = 0; i < blockCount; ++i) { + const auto& blockName = Artifacts().program->getUniformBlock(i).name; + if (blockName.compare(0, prefixLength, MG_Util::ShaderTranspiler::ATOMIC_COUNTER_BLOCK_PREFIX) != 0) { + continue; + } + if (i == owner) return counterBufferIndex; + ++counterBufferIndex; + } + return -1; + } + + Bool IsActiveUniformAtomicCounter(Uint index) const { + return GetActiveUniformAtomicCounterBufferIndex(index) >= 0; + } + // GL_UNIFORM_OFFSET: byte offset within the owning named block; -1 for a default-block // uniform. The relaxed parse gives global-UBO members real byte offsets, but GL must keep // seeing them as default-block uniforms, so gate on the GL-visible block index. diff --git a/MobileGL/MG_Test/Program/ProgramInterfaceTest.cpp b/MobileGL/MG_Test/Program/ProgramInterfaceTest.cpp index 16430327..8f6e1c86 100644 --- a/MobileGL/MG_Test/Program/ProgramInterfaceTest.cpp +++ b/MobileGL/MG_Test/Program/ProgramInterfaceTest.cpp @@ -716,6 +716,86 @@ void main() { EXPECT_EQ(TakeError(), GL_INVALID_ENUM); } + // The CLASSIC query surface has to agree with the interface query above. MobileGL lowers + // every atomic_uint onto a synthesized gl_AtomicCounterBlock_N, and glGetActiveUniform / + // glGetActiveUniformsiv used to report that lowering: GL_UNSIGNED_INT instead of + // GL_UNSIGNED_INT_ATOMIC_COUNTER, the synthesized block's index instead of the -1 a + // default-block uniform owes, and GL_INVALID_ENUM for + // GL_UNIFORM_ATOMIC_COUNTER_BUFFER_INDEX - the last of which is what made + // KHR-GL43.shader_atomic_counters.basic-program-query a forced FAIL. + TEST_F(ProgramInterfaceTest, AtomicCounterClassicUniformQueries) { + const char* fs = R"(#version 430 +out vec4 color; +layout (binding = 0, offset = 0) uniform atomic_uint ac_counter0; +layout (binding = 1, offset = 0) uniform atomic_uint ac_counter1; +uniform float plain; +void main() { + color = vec4(float(atomicCounterIncrement(ac_counter0) + atomicCounterIncrement(ac_counter1)) + plain); +} +)"; + const GLuint p = MakeProgram(kSimpleVs, fs); + LinkProgram(p); + ExpectLinked(p); + ClearErrors(); + + const auto indexOf = [p](const char* name) { + const GLchar* names[1] = {name}; + GLuint index = GL_INVALID_INDEX; + GetUniformIndices(p, 1, names, &index); + return index; + }; + const auto uniformiv = [p](GLuint index, GLenum pname) { + GLint value = -12345; + const GLuint indices[1] = {index}; + GetActiveUniformsiv(p, 1, indices, pname, &value); + return value; + }; + + const GLuint counter0 = indexOf("ac_counter0"); + const GLuint counter1 = indexOf("ac_counter1"); + const GLuint plain = indexOf("plain"); + ASSERT_NE(counter0, GL_INVALID_INDEX); + ASSERT_NE(counter1, GL_INVALID_INDEX); + ASSERT_NE(plain, GL_INVALID_INDEX); + + // (a) glGetActiveUniform and glGetActiveUniformsiv(GL_UNIFORM_TYPE) both report the + // GL-level type. + GLint size = 0; + GLenum type = 0; + GLchar nameBuffer[64] = {'\0'}; + GetActiveUniform(p, counter0, sizeof(nameBuffer), nullptr, &size, &type, nameBuffer); + EXPECT_EQ(type, static_cast(GL_UNSIGNED_INT_ATOMIC_COUNTER)); + EXPECT_EQ(std::string(nameBuffer), "ac_counter0"); + EXPECT_EQ(uniformiv(counter0, GL_UNIFORM_TYPE), GL_UNSIGNED_INT_ATOMIC_COUNTER); + EXPECT_EQ(uniformiv(counter1, GL_UNIFORM_TYPE), GL_UNSIGNED_INT_ATOMIC_COUNTER); + EXPECT_EQ(uniformiv(plain, GL_UNIFORM_TYPE), GL_FLOAT); + + // (b) an atomic counter is a DEFAULT-BLOCK uniform, whatever it was lowered onto. + EXPECT_EQ(uniformiv(counter0, GL_UNIFORM_BLOCK_INDEX), -1); + EXPECT_EQ(uniformiv(counter1, GL_UNIFORM_BLOCK_INDEX), -1); + EXPECT_EQ(uniformiv(plain, GL_UNIFORM_BLOCK_INDEX), -1); + + // (c) the pname is accepted, answers with the buffer's index, and reports -1 for a + // uniform that is not a counter. Two bindings mean two distinct buffers. + const GLint buffer0 = uniformiv(counter0, GL_UNIFORM_ATOMIC_COUNTER_BUFFER_INDEX); + const GLint buffer1 = uniformiv(counter1, GL_UNIFORM_ATOMIC_COUNTER_BUFFER_INDEX); + EXPECT_GE(buffer0, 0); + EXPECT_GE(buffer1, 0); + EXPECT_NE(buffer0, buffer1); + EXPECT_LT(buffer0, Interfaceiv(p, GL_ATOMIC_COUNTER_BUFFER, GL_ACTIVE_RESOURCES)); + EXPECT_LT(buffer1, Interfaceiv(p, GL_ATOMIC_COUNTER_BUFFER, GL_ACTIVE_RESOURCES)); + EXPECT_EQ(uniformiv(plain, GL_UNIFORM_ATOMIC_COUNTER_BUFFER_INDEX), -1); + // No leftover error: the CTS harness fails the subcase on one. + EXPECT_EQ(TakeError(), GL_NO_ERROR); + + // The classic surface and the interface query name the same buffer. + const std::vector interfaceBuffer = + PropsOf(p, GL_UNIFORM, "ac_counter0", {GL_ATOMIC_COUNTER_BUFFER_INDEX}); + ASSERT_EQ(interfaceBuffer.size(), 1u); + EXPECT_EQ(interfaceBuffer[0], buffer0); + EXPECT_EQ(TakeError(), GL_NO_ERROR); + } + // Two counters that share a binding AND an offset must fail to link. glslang's own check // lives in fixOffset(), which the Vulkan-relaxed parse never reaches - it folds the // atomic_uint into a storage block and returns from declareVariable() first - so the pair