[Fix, Test] (GLImpl): enforce GL_MAX_ATOMIC_COUNTER_BUFFER_BINDINGS on the bind and indexed-query paths

MobileGL/MG_Impl/GLImpl/Buffer/Validators.cpp

@@ -13,6 +13,7 @@
 #include <MG_Util/Converters/GLToStr/GLEnumConverter.h>
 #include <MG_Util/Converters/MGToGL/BufferEnumConverter.h>
 #include <MG_Util/Converters/MGToStr/BufferEnumConverter.h>
+#include <MG_Util/ShaderTranspiler/Types.h>
 
 namespace MobileGL::MG_Impl::GLImpl::BufferImpl {
     Bool ValidateBufferTarget(BufferTarget target) {
@@ -67,6 +68,13 @@ namespace MobileGL::MG_Impl::GLImpl::BufferImpl {
                 // binding points in GL 3.3 (no ARB_transform_feedback3).
                 pointCount = std::min<SizeT>(pointCount, 4);
             }
+            if (target == BufferTarget::AtomicCounter) {
+                // GL_MAX_ATOMIC_COUNTER_BUFFER_BINDINGS, which is NOT the state layer's array
+                // size: a counter buffer reaches a shader only as a lowered storage block, so the
+                // reserved range is the ceiling, and glGetIntegerv advertises the same number.
+                pointCount = std::min<SizeT>(
+                    pointCount, static_cast<SizeT>(MG_Util::ShaderTranspiler::MAX_ATOMIC_COUNTER_BUFFER_BINDINGS));
+            }
             return pointCount;
         }
     } // namespace

MobileGL/MG_Impl/GLImpl/Getter/GL_Getter.cpp

@@ -80,12 +80,9 @@ namespace MobileGL::MG_Impl::GLImpl {
         constexpr GLint kFrontendMaxTessControlAtomicCounterBuffers = 0;
         constexpr GLint kFrontendMaxTessEvaluationAtomicCounterBuffers = 0;
         constexpr GLint kFrontendMaxVertexAtomicCounterBuffers = 0;
-        // GL_MAX_ATOMIC_COUNTER_BUFFER_BINDINGS / _SIZE. Both come from the shared table above:
+        // GL_MAX_ATOMIC_COUNTER_BUFFER_SIZE: the byte offset ceiling a counter may be declared
-        // the binding count is how many counter buffers a backend can actually address (each one
+        // at. The matching binding count is applied in GetIndexedBufferQueryPointCount, so that
-        // costs a shader-storage binding point once glslang has lowered the counters onto a
+        // the getter, the indexed queries and glBindBufferBase all share one ceiling.
-        // storage block), and the size is the byte offset ceiling a counter may be declared at.
-        constexpr GLint kFrontendMaxAtomicCounterBufferBindings =
-            static_cast<GLint>(MG_Util::ShaderTranspiler::MAX_ATOMIC_COUNTER_BUFFER_BINDINGS);
         constexpr GLint kFrontendMaxAtomicCounterBufferSize =
             static_cast<GLint>(MG_Util::ShaderTranspiler::MAX_ATOMIC_COUNTER_BUFFER_SIZE);
         // KHR_debug minima (GL 4.6 table 23.66); the debug entry points are stubs, but the
@@ -204,6 +201,16 @@ namespace MobileGL::MG_Impl::GLImpl {
                     MG_Backend::pActiveBackendObject->GetDynamicParameters().MaxShaderStorageBufferBindings;
                 return std::min(frontendCount, static_cast<SizeT>(std::max(backendCount, 0)));
             }
+            if (bufferTarget == BufferTarget::AtomicCounter) {
+                // The counter family's binding count is NOT the state layer's array size: a
+                // counter buffer only reaches a shader as a lowered storage block, so what an
+                // implementation can serve is the reserved range, and that number is also what
+                // glslang compiles a layout(binding = N) atomic_uint against. Clamped here so
+                // GL_MAX_ATOMIC_COUNTER_BUFFER_BINDINGS, the indexed getters' index check and
+                // glBindBufferBase's all report the same ceiling.
+                return std::min(frontendCount,
+                                static_cast<SizeT>(MG_Util::ShaderTranspiler::MAX_ATOMIC_COUNTER_BUFFER_BINDINGS));
+            }
             return frontendCount;
         }
 
@@ -2261,12 +2268,11 @@ namespace MobileGL::MG_Impl::GLImpl {
                                                           static_cast<Uint64>(INT32_MAX)));
             break;
         case GL_MAX_ATOMIC_COUNTER_BUFFER_BINDINGS:
-            // NOT the frontend's binding-point array size (36). A counter buffer only reaches a
+            // NOT the frontend's binding-point array size: GetIndexedBufferQueryPointCount
-            // shader as a lowered storage block, so the number an implementation can serve is
+            // clamps this family to the range a lowered counter block can actually be served
-            // the reserved shader-storage range, and it has to be the same number glslang
+            // from, which is the same number glslang compiles a layout(binding = N) atomic_uint
-            // compiles a layout(binding = N) atomic_uint against.
+            // against and the same one glBindBufferBase validates an index against.
-            *params = std::min<GLint>(kFrontendMaxAtomicCounterBufferBindings,
+            *params = static_cast<GLint>(GetIndexedBufferQueryPointCount(BufferTarget::AtomicCounter));
-                                      static_cast<GLint>(GetIndexedBufferQueryPointCount(BufferTarget::AtomicCounter)));
             break;
         case GL_MAX_ATOMIC_COUNTER_BUFFER_SIZE:
             // The conformance suite splits this evenly across every advertised binding point and

MobileGL/MG_Test/SanityTest.cpp

@@ -958,6 +958,15 @@ TEST(GetterSanity, AtomicCounterQueriesMatchShaderCompilerLimits) {
     EXPECT_EQ(static_cast<GLuint>(reported), buffer);
     EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR);
 
+    // The advertised ceiling is also the one glBindBufferBase and the indexed getter enforce.
+    // A limit nothing validates against is how these tables drifted apart in the first place:
+    // the binding-point ARRAY is 36 deep, and it used to be that number an application saw.
+    constexpr GLuint pastLastBinding = static_cast<GLuint>(Transpiler::MAX_ATOMIC_COUNTER_BUFFER_BINDINGS);
+    MG_Impl::GLImpl::BindBufferBase(GL_ATOMIC_COUNTER_BUFFER, pastLastBinding, buffer);
+    EXPECT_EQ(MG_Impl::GLImpl::GetError(), static_cast<GLenum>(GL_INVALID_VALUE));
+    MG_Impl::GLImpl::GetIntegeri_v(GL_ATOMIC_COUNTER_BUFFER_BINDING, pastLastBinding, &reported);
+    EXPECT_EQ(MG_Impl::GLImpl::GetError(), static_cast<GLenum>(GL_INVALID_VALUE));
+
     // ...and the shading language has to expand the same numbers. Each array is sized by a
     // built-in constant and indexed at its last element with a literal, so the stage only
     // compiles when that constant is at least what glGetIntegerv just reported - which it was