diff --git a/MobileGL/MG_Impl/GLImpl/Buffer/GL_Buffer.cpp b/MobileGL/MG_Impl/GLImpl/Buffer/GL_Buffer.cpp index 850d304f..5a58d640 100644 --- a/MobileGL/MG_Impl/GLImpl/Buffer/GL_Buffer.cpp +++ b/MobileGL/MG_Impl/GLImpl/Buffer/GL_Buffer.cpp @@ -1527,19 +1527,27 @@ namespace MobileGL::MG_Impl::GLImpl { return false; } } - // A transform feedback capture binding is addressed in 32-bit components, so BOTH the - // offset and the size must be multiples of 4. An atomic counter binding is addressed in - // 32-bit counters and constrains its offset the same way (GL 4.6 core 6.1.1) - that one - // has no queryable alignment pname, which is why it was missing here. - const Bool isFourByteAddressed = - target == GL_TRANSFORM_FEEDBACK_BUFFER || target == GL_ATOMIC_COUNTER_BUFFER; - if (isFourByteAddressed && ((offset % 4) != 0 || (hasBuffer && (size % 4) != 0))) { + // GL 4.6 core 6.1.1 constrains the OFFSET to a multiple of four for both + // TRANSFORM_FEEDBACK_BUFFER and ATOMIC_COUNTER_BUFFER (the atomic-counter one has no + // queryable alignment pname, which is why it was missing here), and the SIZE only for + // transform feedback, whose capture is written in whole 32-bit components. Extending the + // size rule to atomic counters as well breaks a legal bind: the conformance suite splits + // MAX_ATOMIC_COUNTER_BUFFER_SIZE evenly across the binding points and that quotient is + // not required to land on four. + if ((target == GL_TRANSFORM_FEEDBACK_BUFFER || target == GL_ATOMIC_COUNTER_BUFFER) && (offset % 4) != 0) { + MG_State::pGLContext->RecordError( + ErrorCode::InvalidValue, + MakeUnique("MG_Impl/GLImpl", funcName, + std::format("offset ({}) must be a multiple of 4 for {}.", offset, + MG_Util::ConvertGLEnumToString(target)))); + return false; + } + if (target == GL_TRANSFORM_FEEDBACK_BUFFER && hasBuffer && (size % 4) != 0) { MG_State::pGLContext->RecordError( ErrorCode::InvalidValue, MakeUnique( "MG_Impl/GLImpl", funcName, - std::format("offset ({}) and size ({}) must both be multiples of 4 for {}.", offset, size, - MG_Util::ConvertGLEnumToString(target)))); + std::format("size ({}) must be a multiple of 4 for GL_TRANSFORM_FEEDBACK_BUFFER.", size))); return false; } return true; @@ -1742,29 +1750,28 @@ namespace MobileGL::MG_Impl::GLImpl { // ARB_multi_bind states the equivalence to a loop of single binds "except that ... buffers // will not be created if they do not exist": glBindBuffer instantiates a name glGenBuffers - // merely reserved, glBindBuffers* must refuse it instead. That is INVALID_OPERATION, and it - // is an all-or-nothing check - one bad name leaves every binding point in the range alone + // merely reserved, glBindBuffers* must refuse it and raise INVALID_OPERATION instead // (KHR-GL44.multi_bind.errors_bind_buffers). - static Bool ValidateMultiBindBufferNames(const GLuint* buffers, GLsizei count, const char* funcName) { - if (buffers == nullptr) return true; - for (GLsizei i = 0; i < count; ++i) { - if (buffers[i] == 0) continue; - if (MG_State::pGLContext->ValidateBufferObject(buffers[i])) continue; - MG_State::pGLContext->RecordError( - ErrorCode::InvalidOperation, - MakeUnique( - "MG_Impl/GLImpl", funcName, - std::format("buffers[{}] ({}) is not the name of an existing buffer object.", i, buffers[i]))); - return false; - } - return true; + // + // Deliberately PER ELEMENT, not all-or-nothing: the equivalence the extension defines is a + // loop, so a bad entry costs its own binding point and nothing else. Rejecting the whole + // call instead cost multi_bind.functional_bind_buffers_base its bindings. + static Bool IsExistingBufferForMultiBind(GLuint buffer, GLsizei index, const char* funcName) { + if (buffer == 0 || MG_State::pGLContext->ValidateBufferObject(buffer)) return true; + MG_State::pGLContext->RecordError( + ErrorCode::InvalidOperation, + MakeUnique( + "MG_Impl/GLImpl", funcName, + std::format("buffers[{}] ({}) is not the name of an existing buffer object.", index, buffer))); + return false; } void BindBuffersBase(GLenum target, GLuint first, GLsizei count, const GLuint* buffers) { if (!ValidateMultiBindBufferRange(target, first, count, __func__)) return; - if (!ValidateMultiBindBufferNames(buffers, count, __func__)) return; for (GLsizei i = 0; i < count; ++i) { - BindBufferBase_State(target, first + i, buffers ? buffers[i] : 0); + const GLuint buffer = buffers ? buffers[i] : 0; + if (!IsExistingBufferForMultiBind(buffer, i, __func__)) continue; + BindBufferBase_State(target, first + i, buffer); } } @@ -1777,8 +1784,8 @@ namespace MobileGL::MG_Impl::GLImpl { void BindBuffersRange(GLenum target, GLuint first, GLsizei count, const GLuint* buffers, const GLintptr* offsets, const GLsizeiptr* sizes) { if (!ValidateMultiBindBufferRange(target, first, count, __func__)) return; - if (!ValidateMultiBindBufferNames(buffers, count, __func__)) return; for (GLsizei i = 0; i < count; ++i) { + if (buffers && !IsExistingBufferForMultiBind(buffers[i], i, __func__)) continue; if (!buffers || buffers[i] == 0) { BindBufferBase_State(target, first + i, 0); } else { diff --git a/MobileGL/MG_Impl/GLImpl/Sampler/GL_Sampler.cpp b/MobileGL/MG_Impl/GLImpl/Sampler/GL_Sampler.cpp index 7f9c2ae8..0939d680 100644 --- a/MobileGL/MG_Impl/GLImpl/Sampler/GL_Sampler.cpp +++ b/MobileGL/MG_Impl/GLImpl/Sampler/GL_Sampler.cpp @@ -336,27 +336,22 @@ namespace MobileGL::MG_Impl::GLImpl { return; } - // ...and the names are checked up front for the same reason, with the extra rule that - // ARB_multi_bind spells out separately: "samplers will not be created if they do not - // exist". The single-bind path instantiates a name glGenSamplers merely reserved; here - // a name that is not an existing sampler OBJECT is INVALID_OPERATION and nothing binds - // (KHR-GL44.multi_bind.errors_bind_samplers). - if (samplers != nullptr) { - for (GLsizei i = 0; i < count; ++i) { - if (samplers[i] == 0) continue; - if (MG_State::pGLContext->ValidateSamplerObject(samplers[i])) continue; + // ARB_multi_bind adds one rule the single-bind path does not have: "samplers will not be + // created if they do not exist", so a name that is not an existing sampler OBJECT is + // INVALID_OPERATION here (KHR-GL44.multi_bind.errors_bind_samplers). Per element, not + // all-or-nothing - the extension defines glBindSamplers as a loop, so a bad entry costs + // its own texture unit and leaves the rest of the range bound. + for (GLsizei i = 0; i < count; ++i) { + const GLuint sampler = samplers ? samplers[i] : 0; + if (sampler != 0 && !MG_State::pGLContext->ValidateSamplerObject(sampler)) { MG_State::pGLContext->RecordError( ErrorCode::InvalidOperation, MakeUnique( "MG_Impl/GLImpl", "BindSamplers", - std::format("samplers[{}] ({}) is not the name of an existing sampler object.", i, - samplers[i]))); - return; + std::format("samplers[{}] ({}) is not the name of an existing sampler object.", i, sampler))); + continue; } - } - - for (GLsizei i = 0; i < count; ++i) { - BindSampler_State(first + i, samplers ? samplers[i] : 0); + BindSampler_State(first + i, sampler); } } diff --git a/MobileGL/MG_Test/State/NegativeApiErrorsTest.cpp b/MobileGL/MG_Test/State/NegativeApiErrorsTest.cpp index 732c6482..b3f55d6a 100644 --- a/MobileGL/MG_Test/State/NegativeApiErrorsTest.cpp +++ b/MobileGL/MG_Test/State/NegativeApiErrorsTest.cpp @@ -118,10 +118,13 @@ namespace { GL_INVALID_OPERATION}, }); - // The rejected call must have bound nothing at all. + // ARB_multi_bind defines these as a LOOP of single binds, so the bad entry costs its own + // binding point and the good one still binds - only the error is new. GLint bound = -1; GetIntegeri_v(GL_UNIFORM_BUFFER_BINDING, 0, &bound); - EXPECT_EQ(bound, 0); + EXPECT_EQ(static_cast(bound), buffer) << "a rejected element must not take the valid ones with it"; + GetIntegeri_v(GL_UNIFORM_BUFFER_BINDING, 1, &bound); + EXPECT_EQ(bound, 0) << "the rejected element must not have bound anything"; DrainErrors(); } @@ -140,8 +143,6 @@ namespace { // alignment pname, which is how its rule went missing. {"glBindBufferRange(ATOMIC_COUNTER_BUFFER, offset 3)", [&] { BindBufferRange(GL_ATOMIC_COUNTER_BUFFER, 0, atomicBuffer, 3, 16); }, GL_INVALID_VALUE}, - {"glBindBufferRange(ATOMIC_COUNTER_BUFFER, size 15)", - [&] { BindBufferRange(GL_ATOMIC_COUNTER_BUFFER, 0, atomicBuffer, 4, 15); }, GL_INVALID_VALUE}, }); // ...and the aligned form still works.