diff --git a/MobileGL/MG_Impl/GLImpl/Buffer/Validators.cpp b/MobileGL/MG_Impl/GLImpl/Buffer/Validators.cpp index 50df5b1e..a93da564 100644 --- a/MobileGL/MG_Impl/GLImpl/Buffer/Validators.cpp +++ b/MobileGL/MG_Impl/GLImpl/Buffer/Validators.cpp @@ -13,6 +13,7 @@ #include #include #include +#include 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(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( + pointCount, static_cast(MG_Util::ShaderTranspiler::MAX_ATOMIC_COUNTER_BUFFER_BINDINGS)); + } return pointCount; } } // namespace diff --git a/MobileGL/MG_Impl/GLImpl/Getter/GL_Getter.cpp b/MobileGL/MG_Impl/GLImpl/Getter/GL_Getter.cpp index 18d4d046..22d5fc7b 100644 --- a/MobileGL/MG_Impl/GLImpl/Getter/GL_Getter.cpp +++ b/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: - // the binding count is how many counter buffers a backend can actually address (each one - // costs a shader-storage binding point once glslang has lowered the counters onto a - // storage block), and the size is the byte offset ceiling a counter may be declared at. - constexpr GLint kFrontendMaxAtomicCounterBufferBindings = - static_cast(MG_Util::ShaderTranspiler::MAX_ATOMIC_COUNTER_BUFFER_BINDINGS); + // GL_MAX_ATOMIC_COUNTER_BUFFER_SIZE: the byte offset ceiling a counter may be declared + // at. The matching binding count is applied in GetIndexedBufferQueryPointCount, so that + // the getter, the indexed queries and glBindBufferBase all share one ceiling. constexpr GLint kFrontendMaxAtomicCounterBufferSize = static_cast(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(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(MG_Util::ShaderTranspiler::MAX_ATOMIC_COUNTER_BUFFER_BINDINGS)); + } return frontendCount; } @@ -2261,12 +2268,11 @@ namespace MobileGL::MG_Impl::GLImpl { static_cast(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 - // shader as a lowered storage block, so the number an implementation can serve is - // the reserved shader-storage range, and it has to be the same number glslang - // compiles a layout(binding = N) atomic_uint against. - *params = std::min(kFrontendMaxAtomicCounterBufferBindings, - static_cast(GetIndexedBufferQueryPointCount(BufferTarget::AtomicCounter))); + // NOT the frontend's binding-point array size: GetIndexedBufferQueryPointCount + // clamps this family to the range a lowered counter block can actually be served + // from, which is the same number glslang compiles a layout(binding = N) atomic_uint + // against and the same one glBindBufferBase validates an index against. + *params = static_cast(GetIndexedBufferQueryPointCount(BufferTarget::AtomicCounter)); break; case GL_MAX_ATOMIC_COUNTER_BUFFER_SIZE: // The conformance suite splits this evenly across every advertised binding point and diff --git a/MobileGL/MG_Test/SanityTest.cpp b/MobileGL/MG_Test/SanityTest.cpp index 8e9ab567..dff11265 100644 --- a/MobileGL/MG_Test/SanityTest.cpp +++ b/MobileGL/MG_Test/SanityTest.cpp @@ -958,6 +958,15 @@ TEST(GetterSanity, AtomicCounterQueriesMatchShaderCompilerLimits) { EXPECT_EQ(static_cast(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(Transpiler::MAX_ATOMIC_COUNTER_BUFFER_BINDINGS); + MG_Impl::GLImpl::BindBufferBase(GL_ATOMIC_COUNTER_BUFFER, pastLastBinding, buffer); + EXPECT_EQ(MG_Impl::GLImpl::GetError(), static_cast(GL_INVALID_VALUE)); + MG_Impl::GLImpl::GetIntegeri_v(GL_ATOMIC_COUNTER_BUFFER_BINDING, pastLastBinding, &reported); + EXPECT_EQ(MG_Impl::GLImpl::GetError(), static_cast(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