From a9fb7ef0af0425e52374b88461fa75eb700f3fca Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Thu, 20 Aug 2026 11:23:33 -0400 Subject: [PATCH] [Fix, Test] (GLImpl, GLState): answer GL_ACTIVE_ATOMIC_COUNTER_BUFFERS and implement glGetActiveAtomicCounterBufferiv --- .../MG_Impl/GLImpl/Exporting/Definitions.cpp | 2 +- .../MG_Impl/GLImpl/Program/GL_Program.cpp | 75 ++++++++++++++++++- MobileGL/MG_Impl/GLImpl/Program/GL_Program.h | 1 + .../GLImpl/Program/ProgramInterface.cpp | 2 +- .../GLState/ProgramState/ProgramObject.h | 4 - .../MG_Test/Program/ProgramInterfaceTest.cpp | 62 +++++++++++++++ MobileGL/MG_Util/ShaderTranspiler/Types.h | 2 +- 7 files changed, 140 insertions(+), 8 deletions(-) diff --git a/MobileGL/MG_Impl/GLImpl/Exporting/Definitions.cpp b/MobileGL/MG_Impl/GLImpl/Exporting/Definitions.cpp index 1be2e4b2..0df3254c 100644 --- a/MobileGL/MG_Impl/GLImpl/Exporting/Definitions.cpp +++ b/MobileGL/MG_Impl/GLImpl/Exporting/Definitions.cpp @@ -982,7 +982,7 @@ DECLARE_GL_FUNCTION_HEAD(void, GetDoublei_v, GLenum target, GLuint index, GLdoub DECLARE_GL_FUNCTION_HEAD(void, DrawArraysInstancedBaseInstance, GLenum mode, GLint first, GLsizei count, GLsizei instancecount, GLuint baseinstance) DECLARE_GL_FUNCTION_END_NO_RETURN(void, DrawArraysInstancedBaseInstance, mode, first, count, instancecount, baseinstance) DECLARE_GL_FUNCTION_HEAD(void, DrawElementsInstancedBaseInstance, GLenum mode, GLsizei count, GLenum type, const void* indices, GLsizei instancecount, GLuint baseinstance) DECLARE_GL_FUNCTION_END_NO_RETURN(void, DrawElementsInstancedBaseInstance, mode, count, type, indices, instancecount, baseinstance) DECLARE_GL_FUNCTION_HEAD(void, DrawElementsInstancedBaseVertexBaseInstance, GLenum mode, GLsizei count, GLenum type, const void* indices, GLsizei instancecount, GLint basevertex, GLuint baseinstance) DECLARE_GL_FUNCTION_END_NO_RETURN(void, DrawElementsInstancedBaseVertexBaseInstance, mode, count, type, indices, instancecount, basevertex, baseinstance) -DECLARE_GL_FUNCTION_STUB_HEAD(void, GetActiveAtomicCounterBufferiv, GLuint program, GLuint bufferIndex, GLenum pname, GLint* params) DECLARE_GL_FUNCTION_STUB_END_NO_RETURN(void, GetActiveAtomicCounterBufferiv, program, bufferIndex, pname, params) +DECLARE_GL_FUNCTION_HEAD(void, GetActiveAtomicCounterBufferiv, GLuint program, GLuint bufferIndex, GLenum pname, GLint* params) DECLARE_GL_FUNCTION_END_NO_RETURN(void, GetActiveAtomicCounterBufferiv, program, bufferIndex, pname, params) DECLARE_GL_FUNCTION_HEAD(void, DrawTransformFeedbackInstanced, GLenum mode, GLuint id, GLsizei instancecount) DECLARE_GL_FUNCTION_END_NO_RETURN(void, DrawTransformFeedbackInstanced, mode, id, instancecount) DECLARE_GL_FUNCTION_HEAD(void, DrawTransformFeedbackStreamInstanced, GLenum mode, GLuint id, GLuint stream, GLsizei instancecount) DECLARE_GL_FUNCTION_END_NO_RETURN(void, DrawTransformFeedbackStreamInstanced, mode, id, stream, instancecount) DECLARE_GL_FUNCTION_HEAD(void, ClearBufferData, GLenum target, GLenum internalformat, GLenum format, GLenum type, const void* data) DECLARE_GL_FUNCTION_END_NO_RETURN(void, ClearBufferData, target, internalformat, format, type, data) diff --git a/MobileGL/MG_Impl/GLImpl/Program/GL_Program.cpp b/MobileGL/MG_Impl/GLImpl/Program/GL_Program.cpp index 29090d8c..da1f5732 100644 --- a/MobileGL/MG_Impl/GLImpl/Program/GL_Program.cpp +++ b/MobileGL/MG_Impl/GLImpl/Program/GL_Program.cpp @@ -642,7 +642,13 @@ namespace MobileGL::MG_Impl::GLImpl { break; } case GL_ACTIVE_ATOMIC_COUNTER_BUFFERS: - *params = programObject->GetActiveAtomicCounterCount(); + // Counter BUFFERS, not counters, and glslang's own getNumAtomicCounters() answers + // neither: the relaxed parse has already turned every atomic_uint into a plain uint + // member of a synthesized storage block by the time it builds its reflection, so it + // reports zero. The interface-query model recovers the buffers from those blocks and + // is what glGetProgramInterfaceiv(GL_ATOMIC_COUNTER_BUFFER, GL_ACTIVE_RESOURCES) + // already answers - the two queries are required to agree. + *params = ProgramInterface::GetActiveResourceCount(*programObject, GL_ATOMIC_COUNTER_BUFFER); MGLOG_D("%s: %s = %d", __func__, MG_Util::ConvertGLEnumToString(pname).c_str(), *params); break; case GL_ACTIVE_ATTRIBUTES: @@ -2835,6 +2841,73 @@ namespace MobileGL::MG_Impl::GLImpl { return ProgramInterface::GetResourceLocationIndex(*programObject, programInterface, name); } + // GL 4.6 §7.7. Every property this reports is one the GL_ATOMIC_COUNTER_BUFFER interface + // already carries, so this is a rename of glGetProgramResourceiv's props onto the older + // entry point's - and the two are required to agree, which is only true while both read the + // same model. It was a silent stub: it wrote nothing, raised nothing, and left every probe + // reading its own uninitialised output. + static Bool TryMapActiveAtomicCounterBufferProp(GLenum pname, GLenum& outProp) { + switch (pname) { + case GL_ATOMIC_COUNTER_BUFFER_BINDING: + outProp = GL_BUFFER_BINDING; + return true; + case GL_ATOMIC_COUNTER_BUFFER_DATA_SIZE: + outProp = GL_BUFFER_DATA_SIZE; + return true; + case GL_ATOMIC_COUNTER_BUFFER_ACTIVE_ATOMIC_COUNTERS: + outProp = GL_NUM_ACTIVE_VARIABLES; + return true; + case GL_ATOMIC_COUNTER_BUFFER_ACTIVE_ATOMIC_COUNTER_INDICES: + outProp = GL_ACTIVE_VARIABLES; + return true; + case GL_ATOMIC_COUNTER_BUFFER_REFERENCED_BY_VERTEX_SHADER: + outProp = GL_REFERENCED_BY_VERTEX_SHADER; + return true; + case GL_ATOMIC_COUNTER_BUFFER_REFERENCED_BY_TESS_CONTROL_SHADER: + outProp = GL_REFERENCED_BY_TESS_CONTROL_SHADER; + return true; + case GL_ATOMIC_COUNTER_BUFFER_REFERENCED_BY_TESS_EVALUATION_SHADER: + outProp = GL_REFERENCED_BY_TESS_EVALUATION_SHADER; + return true; + case GL_ATOMIC_COUNTER_BUFFER_REFERENCED_BY_GEOMETRY_SHADER: + outProp = GL_REFERENCED_BY_GEOMETRY_SHADER; + return true; + case GL_ATOMIC_COUNTER_BUFFER_REFERENCED_BY_FRAGMENT_SHADER: + outProp = GL_REFERENCED_BY_FRAGMENT_SHADER; + return true; + case GL_ATOMIC_COUNTER_BUFFER_REFERENCED_BY_COMPUTE_SHADER: + outProp = GL_REFERENCED_BY_COMPUTE_SHADER; + return true; + default: + return false; + } + } + + void GetActiveAtomicCounterBufferiv(GLuint program, GLuint bufferIndex, GLenum pname, GLint* params) { + auto& programObject = TryToGetProgramForInterfaceQuery(program, __func__); + if (!programObject) return; + GLenum prop = GL_NONE; + if (!TryMapActiveAtomicCounterBufferProp(pname, prop)) { + MG_State::pGLContext->RecordError( + ErrorCode::InvalidEnum, + MakeUnique("MG_Impl/GLImpl", __func__, + "pname is not an active atomic counter buffer property.")); + return; + } + Vector values; + if (!ProgramInterface::GetResourceProp(*programObject, GL_ATOMIC_COUNTER_BUFFER, bufferIndex, prop, values)) { + MG_State::pGLContext->RecordError( + ErrorCode::InvalidValue, + MakeUnique("MG_Impl/GLImpl", __func__, + "bufferIndex is not an active atomic counter buffer index.")); + return; + } + if (params == nullptr) return; + // GL_ATOMIC_COUNTER_BUFFER_ACTIVE_ATOMIC_COUNTER_INDICES is the only multi-value property + // here, and the caller sized its array from _ACTIVE_ATOMIC_COUNTERS. + for (SizeT i = 0; i < values.size(); ++i) params[i] = values[i]; + } + // GL 4.6 §7.6.2: is an active shader storage block index of // - that is, exactly what glGetProgramResourceIndex(GL_SHADER_STORAGE_BLOCK) returned. // Since wave 2 that index is the interface-query layer's, so this is where the one index diff --git a/MobileGL/MG_Impl/GLImpl/Program/GL_Program.h b/MobileGL/MG_Impl/GLImpl/Program/GL_Program.h index 437db49c..cdebc445 100644 --- a/MobileGL/MG_Impl/GLImpl/Program/GL_Program.h +++ b/MobileGL/MG_Impl/GLImpl/Program/GL_Program.h @@ -140,6 +140,7 @@ namespace MobileGL::MG_Impl::GLImpl { const GLenum* props, GLsizei bufSize, GLsizei* length, GLint* params); GLint GetProgramResourceLocation(GLuint program, GLenum programInterface, const GLchar* name); GLint GetProgramResourceLocationIndex(GLuint program, GLenum programInterface, const GLchar* name); + void GetActiveAtomicCounterBufferiv(GLuint program, GLuint bufferIndex, GLenum pname, GLint* params); void ShaderStorageBlockBinding(GLuint program, GLuint storageBlockIndex, GLuint storageBlockBinding); void Uniform1d(GLint location, GLdouble v0); void Uniform1dv(GLint location, GLsizei count, const GLdouble* value); diff --git a/MobileGL/MG_Impl/GLImpl/Program/ProgramInterface.cpp b/MobileGL/MG_Impl/GLImpl/Program/ProgramInterface.cpp index 1137ad5f..6883e733 100644 --- a/MobileGL/MG_Impl/GLImpl/Program/ProgramInterface.cpp +++ b/MobileGL/MG_Impl/GLImpl/Program/ProgramInterface.cpp @@ -19,7 +19,7 @@ namespace MobileGL::MG_Impl::GLImpl::ProgramInterface { // "_" (ParseContextBase.cpp), one per GL // atomic-counter binding point. That block IS the GL_ATOMIC_COUNTER_BUFFER resource // and its trailing number IS GL_BUFFER_BINDING; its members stay GL_UNIFORMs. - constexpr const char* kAtomicCounterBlockPrefix = "gl_AtomicCounterBlock"; + constexpr const char* kAtomicCounterBlockPrefix = MG_Util::ShaderTranspiler::ATOMIC_COUNTER_BLOCK_PREFIX; enum class BlockKind { Uniform, // a real GL uniform block diff --git a/MobileGL/MG_State/GLState/ProgramState/ProgramObject.h b/MobileGL/MG_State/GLState/ProgramState/ProgramObject.h index a6cec261..b8c2bdca 100644 --- a/MobileGL/MG_State/GLState/ProgramState/ProgramObject.h +++ b/MobileGL/MG_State/GLState/ProgramState/ProgramObject.h @@ -702,10 +702,6 @@ namespace MobileGL::MG_State::GLState { // SIGSEGV inside glslang::TProgram::getNumPipeInputs - KHR-GL30.api.coverage does exactly // this after a failed glGetAttribLocation, and reached it as soon as the CopyTexImage2D // throw ahead of it stopped killing the run first. - Int GetActiveAtomicCounterCount() const { - const auto& program = Artifacts().program; - return program ? program->getNumAtomicCounters() : 0; - } Int GetActiveAttributesCount() const { const auto& program = Artifacts().program; return program ? program->getNumPipeInputs() : 0; diff --git a/MobileGL/MG_Test/Program/ProgramInterfaceTest.cpp b/MobileGL/MG_Test/Program/ProgramInterfaceTest.cpp index 01e0a1f5..e36a75f6 100644 --- a/MobileGL/MG_Test/Program/ProgramInterfaceTest.cpp +++ b/MobileGL/MG_Test/Program/ProgramInterfaceTest.cpp @@ -716,6 +716,68 @@ void main() { EXPECT_EQ(TakeError(), GL_INVALID_ENUM); } + // glGetProgramiv(GL_ACTIVE_ATOMIC_COUNTER_BUFFERS) and glGetActiveAtomicCounterBufferiv are + // the pre-4.3 spelling of the interface above, and the spec requires the two to agree. + // Neither did: the first counted glslang's atomic counter UNIFORMS - zero, because the + // relaxed parse folds every atomic_uint into a storage block before reflection runs - and + // the second was a stub that wrote nothing and raised nothing. + TEST_F(ProgramInterfaceTest, ActiveAtomicCounterBufferQueriesMatchTheInterface) { + const char* fs = R"(#version 430 +out vec4 color; +layout (binding = 1, offset = 0) uniform atomic_uint a; +layout (binding = 2, offset = 0) uniform atomic_uint b; +layout (binding = 2, offset = 4) uniform atomic_uint c; +void main() { + color = vec4(float(atomicCounterIncrement(a) + atomicCounterIncrement(b) + atomicCounterIncrement(c))); +} +)"; + const GLuint p = MakeProgram(kSimpleVs, fs); + LinkProgram(p); + ExpectLinked(p); + ClearErrors(); + + GLint bufferCount = -12345; + GetProgramiv(p, GL_ACTIVE_ATOMIC_COUNTER_BUFFERS, &bufferCount); + EXPECT_EQ(bufferCount, Interfaceiv(p, GL_ATOMIC_COUNTER_BUFFER, GL_ACTIVE_RESOURCES)); + ASSERT_EQ(bufferCount, 2); + + const auto activeBufferiv = [p](GLuint index, GLenum pname) { + GLint value = -12345; + GetActiveAtomicCounterBufferiv(p, index, pname, &value); + return value; + }; + for (GLuint index = 0; index < static_cast(bufferCount); ++index) { + const std::vector viaInterface = + Props(p, GL_ATOMIC_COUNTER_BUFFER, index, + {GL_BUFFER_BINDING, GL_BUFFER_DATA_SIZE, GL_NUM_ACTIVE_VARIABLES, + GL_REFERENCED_BY_VERTEX_SHADER, GL_REFERENCED_BY_FRAGMENT_SHADER}); + ASSERT_EQ(viaInterface.size(), 5u); + EXPECT_EQ(activeBufferiv(index, GL_ATOMIC_COUNTER_BUFFER_BINDING), viaInterface[0]); + EXPECT_EQ(activeBufferiv(index, GL_ATOMIC_COUNTER_BUFFER_DATA_SIZE), viaInterface[1]); + EXPECT_EQ(activeBufferiv(index, GL_ATOMIC_COUNTER_BUFFER_ACTIVE_ATOMIC_COUNTERS), viaInterface[2]); + EXPECT_EQ(activeBufferiv(index, GL_ATOMIC_COUNTER_BUFFER_REFERENCED_BY_VERTEX_SHADER), viaInterface[3]); + EXPECT_EQ(activeBufferiv(index, GL_ATOMIC_COUNTER_BUFFER_REFERENCED_BY_FRAGMENT_SHADER), viaInterface[4]); + + // The counter indices are the GL_UNIFORM indices, in the same order. + const std::vector expectedIndices = Props(p, GL_ATOMIC_COUNTER_BUFFER, index, {GL_ACTIVE_VARIABLES}); + ASSERT_FALSE(expectedIndices.empty()); + std::vector indices(expectedIndices.size(), -12345); + GetActiveAtomicCounterBufferiv(p, index, GL_ATOMIC_COUNTER_BUFFER_ACTIVE_ATOMIC_COUNTER_INDICES, + indices.data()); + EXPECT_EQ(indices, expectedIndices); + } + EXPECT_EQ(TakeError(), GL_NO_ERROR); + + GLint sink = -12345; + GetActiveAtomicCounterBufferiv(p, static_cast(bufferCount), GL_ATOMIC_COUNTER_BUFFER_BINDING, &sink); + EXPECT_EQ(TakeError(), GL_INVALID_VALUE); + EXPECT_EQ(sink, -12345) << "a rejected query must not write the caller's output"; + // The interface-query spelling of the same property is NOT accepted here. + GetActiveAtomicCounterBufferiv(p, 0, GL_BUFFER_BINDING, &sink); + EXPECT_EQ(TakeError(), GL_INVALID_ENUM); + EXPECT_EQ(sink, -12345); + } + // --------------------------------------------------------- transform-feedback ------ TEST_F(ProgramInterfaceTest, TransformFeedbackVaryingTypes) { const char* vs = R"(#version 430 diff --git a/MobileGL/MG_Util/ShaderTranspiler/Types.h b/MobileGL/MG_Util/ShaderTranspiler/Types.h index 64c0b061..86a7e637 100644 --- a/MobileGL/MG_Util/ShaderTranspiler/Types.h +++ b/MobileGL/MG_Util/ShaderTranspiler/Types.h @@ -18,7 +18,7 @@ namespace MobileGL { // synthesized storage block named "_" // (ParseContextBase::growAtomicCounterBlock). That block IS the GL atomic counter // buffer, and the trailing number is the only place the GL binding survives. - inline const char* ATOMIC_COUNTER_BLOCK_PREFIX = "gl_AtomicCounterBlock"; + inline constexpr const char* ATOMIC_COUNTER_BLOCK_PREFIX = "gl_AtomicCounterBlock"; // Atomic-counter limits, in ONE place because GL 4.6 requires glGetIntegerv and the // shading language's gl_MaxAtomicCounter* constants to report the same numbers