[Fix, Test] (GLImpl, ProgramState): answer the classic uniform queries for atomic counters at GL level

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;
             }

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.

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<GLenum>(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<GLint> 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