From d83b4dbbb5b309f409cbf7674ab16096f9c75e8b Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Tue, 11 Aug 2026 08:49:37 -0400 Subject: [PATCH] [Fix, Test] (MG_State, MG_Impl): ARB_vertex_attrib_binding state model - spec stride default, legacy stride/pointer shadows, divisor re-binds, core-profile VAO-0 rejection --- .../GLImpl/VertexArray/GL_VertexArray.cpp | 49 +- .../VertexArrayState/VertexArrayObject.cpp | 86 +++- .../VertexArrayState/VertexArrayObject.h | 17 +- MobileGL/MG_Test/VertexArray/CMakeLists.txt | 17 + .../VertexAttribBindingStateTest.cpp | 430 ++++++++++++++++++ 5 files changed, 562 insertions(+), 37 deletions(-) create mode 100644 MobileGL/MG_Test/VertexArray/VertexAttribBindingStateTest.cpp diff --git a/MobileGL/MG_Impl/GLImpl/VertexArray/GL_VertexArray.cpp b/MobileGL/MG_Impl/GLImpl/VertexArray/GL_VertexArray.cpp index e7677708..92a938fc 100644 --- a/MobileGL/MG_Impl/GLImpl/VertexArray/GL_VertexArray.cpp +++ b/MobileGL/MG_Impl/GLImpl/VertexArray/GL_VertexArray.cpp @@ -179,6 +179,28 @@ namespace MobileGL::MG_Impl::GLImpl { return vao; } + // The ARB_vertex_attrib_binding entry points that take no vertex array name modify the + // *bound* vertex array, and in a core profile the default vertex array (name 0) is not + // one: every one of them is INVALID_OPERATION there (GL 4.6 core 10.3.1, and the tail of + // each KHR-GL4x.vertex_attrib_binding.negative-* case checks exactly this). MobileGL + // keeps a real object at name 0 for the compatibility paths, so GetBoundVertexArray + // never returns null and the rule has to be spelled out - behind the same gate the VAO-0 + // draw rule already uses (MOBILEGL_RELAXED_SEMANTICS, plus "the context never asked for + // a core profile"), so applications that legitimately run relaxed keep working. + static SharedPtr GetBoundVertexArrayForBindingApi(const char* funcName) { + auto vao = GetBoundVertexArrayOrError(funcName); + if (!vao) return nullptr; + if (vao->GetExternalIndex() == 0 && !MG_State::IsRelaxedSemanticsActive()) { + MG_State::pGLContext->RecordError( + ErrorCode::InvalidOperation, + MakeUnique( + "MG_Impl/GLImpl", funcName, + "The default vertex array object cannot be modified in a core profile.")); + return nullptr; + } + return vao; + } + static bool ValidateVertexAttribPname(GLenum pname) { switch (pname) { case GL_VERTEX_ATTRIB_ARRAY_ENABLED: @@ -944,7 +966,7 @@ namespace MobileGL::MG_Impl::GLImpl { params[0] = static_cast(attr->Size); return; case GL_VERTEX_ATTRIB_ARRAY_STRIDE: - params[0] = static_cast(attr->Stride); + params[0] = static_cast(attr->LegacyStride); return; case GL_VERTEX_ATTRIB_ARRAY_TYPE: params[0] = static_cast(MG_Util::ConvertDataTypeToGLEnum(attr->Type)); @@ -1014,7 +1036,7 @@ namespace MobileGL::MG_Impl::GLImpl { params[0] = static_cast(attr->Size); return; case GL_VERTEX_ATTRIB_ARRAY_STRIDE: - params[0] = static_cast(attr->Stride); + params[0] = static_cast(attr->LegacyStride); return; case GL_VERTEX_ATTRIB_ARRAY_TYPE: params[0] = static_cast(MG_Util::ConvertDataTypeToGLEnum(attr->Type)); @@ -1079,8 +1101,11 @@ namespace MobileGL::MG_Impl::GLImpl { case GL_VERTEX_ATTRIB_ARRAY_SIZE: params[0] = attr->Size; return; + // The legacy shadow, not the resolved draw stride: GL 4.6 core table 23.3 defines this + // as the last glVertexAttrib*Pointer argument, which glBindVertexBuffer must not + // overwrite even though it does overwrite what the backend actually reads. case GL_VERTEX_ATTRIB_ARRAY_STRIDE: - params[0] = attr->Stride; + params[0] = attr->LegacyStride; return; case GL_VERTEX_ATTRIB_ARRAY_TYPE: params[0] = static_cast(MG_Util::ConvertDataTypeToGLEnum(attr->Type)); @@ -1138,7 +1163,7 @@ namespace MobileGL::MG_Impl::GLImpl { } const auto& attr = vao->GetAttribute(index); - *pointer = reinterpret_cast(attr.Offset); + *pointer = reinterpret_cast(attr.LegacyPointer); } void GetVertexAttribIiv(GLuint index, GLenum pname, GLint* params) { @@ -1222,7 +1247,7 @@ namespace MobileGL::MG_Impl::GLImpl { *param = static_cast(attr.Size); return; case GL_VERTEX_ATTRIB_ARRAY_STRIDE: - *param = static_cast(attr.Stride); + *param = static_cast(attr.LegacyStride); return; case GL_VERTEX_ATTRIB_ARRAY_TYPE: *param = static_cast(MG_Util::ConvertDataTypeToGLEnum(attr.Type)); @@ -1294,14 +1319,14 @@ namespace MobileGL::MG_Impl::GLImpl { } void BindVertexBuffer(GLuint bindingindex, GLuint buffer, GLintptr offset, GLsizei stride) { - auto vao = GetBoundVertexArrayOrError("BindVertexBuffer"); + auto vao = GetBoundVertexArrayForBindingApi("BindVertexBuffer"); if (!vao) return; VertexBufferBinding_State(vao, bindingindex, buffer, offset, stride, "BindVertexBuffer"); } void BindVertexBuffers(GLuint first, GLsizei count, const GLuint* buffers, const GLintptr* offsets, const GLsizei* strides) { - auto vao = GetBoundVertexArrayOrError("BindVertexBuffers"); + auto vao = GetBoundVertexArrayForBindingApi("BindVertexBuffers"); if (!vao) return; if (!ValidateVertexBindingRange(first, count, "BindVertexBuffers")) return; for (GLsizei i = 0; i < count; ++i) { @@ -1315,21 +1340,21 @@ namespace MobileGL::MG_Impl::GLImpl { } void VertexAttribFormat(GLuint attribindex, GLint size, GLenum type, GLboolean normalized, GLuint relativeoffset) { - auto vao = GetBoundVertexArrayOrError("VertexAttribFormat"); + auto vao = GetBoundVertexArrayForBindingApi("VertexAttribFormat"); if (!vao) return; VertexAttribFormatSeparate_State(vao, attribindex, size, type, normalized, relativeoffset, false, "VertexAttribFormat"); } void VertexAttribIFormat(GLuint attribindex, GLint size, GLenum type, GLuint relativeoffset) { - auto vao = GetBoundVertexArrayOrError("VertexAttribIFormat"); + auto vao = GetBoundVertexArrayForBindingApi("VertexAttribIFormat"); if (!vao) return; VertexAttribFormatSeparate_State(vao, attribindex, size, type, GL_FALSE, relativeoffset, true, "VertexAttribIFormat"); } void VertexAttribLFormat(GLuint attribindex, GLint size, GLenum type, GLuint relativeoffset) { - auto vao = GetBoundVertexArrayOrError("VertexAttribLFormat"); + auto vao = GetBoundVertexArrayForBindingApi("VertexAttribLFormat"); if (!vao) return; VertexAttribLFormatSeparate_State(vao, attribindex, size, type, relativeoffset); } @@ -1341,7 +1366,7 @@ namespace MobileGL::MG_Impl::GLImpl { } void VertexAttribBinding(GLuint attribindex, GLuint bindingindex) { - auto vao = GetBoundVertexArrayOrError("VertexAttribBinding"); + auto vao = GetBoundVertexArrayForBindingApi("VertexAttribBinding"); if (!vao) return; if (!VertexArrayImpl::ValidateVertexAttributeIndex(attribindex)) return; if (!ValidateVertexBindingIndex(bindingindex, "VertexAttribBinding")) return; @@ -1349,7 +1374,7 @@ namespace MobileGL::MG_Impl::GLImpl { } void VertexBindingDivisor(GLuint bindingindex, GLuint divisor) { - auto vao = GetBoundVertexArrayOrError("VertexBindingDivisor"); + auto vao = GetBoundVertexArrayForBindingApi("VertexBindingDivisor"); if (!vao) return; if (!ValidateVertexBindingIndex(bindingindex, "VertexBindingDivisor")) return; vao->SetBindingDivisor(bindingindex, divisor); diff --git a/MobileGL/MG_State/GLState/VertexArrayState/VertexArrayObject.cpp b/MobileGL/MG_State/GLState/VertexArrayState/VertexArrayObject.cpp index a8bbb0ef..e61fb6d5 100644 --- a/MobileGL/MG_State/GLState/VertexArrayState/VertexArrayObject.cpp +++ b/MobileGL/MG_State/GLState/VertexArrayState/VertexArrayObject.cpp @@ -29,6 +29,8 @@ namespace MobileGL::MG_State::GLState { attr.Normalized = false; attr.Stride = 0; attr.Offset = 0; + attr.LegacyStride = 0; + attr.LegacyPointer = 0; attr.Buffer = nullptr; BumpAttributeFormatVersion(index); @@ -61,10 +63,19 @@ namespace MobileGL::MG_State::GLState { void VertexArrayObject::SetAttributeFormat(Uint index, int size, DataType type, Bool normalized, int stride, SizeT offset, Bool isInteger, Bool isBgra) { if (index >= MAX_VERTEX_ATTRIBS) return; + if (size < 1 || size > 4) { + return; + } // The classic pointer-style API takes back full ownership of the resolved fields. m_attributeUsesBindingModel[index] = false; + // The legacy query shadows: written here and nowhere else, so a later binding-model + // mutation cannot leak into VERTEX_ATTRIB_ARRAY_STRIDE / _POINTER. They are pure + // query state, so they carry no version bump of their own. + m_attributes[index].LegacyStride = stride; + m_attributes[index].LegacyPointer = offset; + if (m_attributes[index].Size == size && m_attributes[index].Type == type && m_attributes[index].Normalized == normalized && m_attributes[index].Stride == stride && m_attributes[index].Offset == offset && m_attributes[index].IsInteger == isInteger && @@ -72,10 +83,6 @@ namespace MobileGL::MG_State::GLState { return; } - if (size < 1 || size > 4) { - return; - } - auto& attr = m_attributes[index]; attr.Size = size; attr.Type = type; @@ -111,6 +118,12 @@ namespace MobileGL::MG_State::GLState { binding.Offset = offset; binding.Stride = effectiveStride; binding.Divisor = m_attributes[index].Divisor; + + // Other attributes may already be pointed at this binding point through + // glVertexAttribBinding; they see the new buffer/offset/stride too (basic-state3 + // checks exactly that after a glVertexAttribPointer). They are not adopted into the + // binding model here - only the ones already in it re-resolve. + ResolveAttributesForBinding(index, /*adopt: */ false); } void VertexArrayObject::BindAttributeBuffer(Uint index, const SharedPtr& buffer) { @@ -147,10 +160,24 @@ namespace MobileGL::MG_State::GLState { void VertexArrayObject::SetAttributeDivisor(Uint index, Uint divisor) { if (index >= MAX_VERTEX_ATTRIBS) return; - // glVertexAttribDivisor is VertexBindingDivisor on the attribute's own binding point - // (GL 4.6 core 10.3.2), so the binding-point view has to follow the resolved attribute. - if (index < MAX_VERTEX_ATTRIB_BINDINGS && m_attributeBindingIndex[index] == index) { + // GL 4.6 core 10.3.2 defines VertexAttribDivisor(i, d) as + // VertexAttribBinding(i, i); VertexBindingDivisor(i, d) + // - the binding is RE-POINTED at i, it is not merely written through when it already + // happens to be i. Guarding the write on "binding == index" (which is what this did) + // left an attribute that glVertexAttribBinding had moved elsewhere pointing at the old + // binding, so the next resolve restored that binding's divisor and the new one was + // lost (KHR-GL4x.vertex_attrib_binding.basic-state4). + // + // What is deliberately NOT copied from VertexAttribBinding is the adoption into the + // binding model: an attribute configured the classic way keeps its pointer-resolved + // stride/offset, exactly as before. The binding point mirrors that state already + // (MirrorPointerIntoBinding), so nothing observable differs - and adopting it here + // would silently swap the raw pointer stride for the effective one under every + // application that calls glVertexAttribDivisor after glVertexAttribPointer. + if (index < MAX_VERTEX_ATTRIB_BINDINGS) { + m_attributeBindingIndex[index] = index; m_bindingPoints[index].Divisor = divisor; + ResolveAttributesForBinding(index, /*adopt: */ false); } if (m_attributes[index].Divisor == divisor) return; m_attributes[index].Divisor = divisor; @@ -164,7 +191,6 @@ namespace MobileGL::MG_State::GLState { void VertexArrayObject::ResolveAttributeFromBinding(Uint attribIndex) { if (attribIndex >= MAX_VERTEX_ATTRIBS) return; - if (!m_attributeUsesBindingModel[attribIndex]) return; const Uint bindingIndex = m_attributeBindingIndex[attribIndex]; if (bindingIndex >= MAX_VERTEX_ATTRIB_BINDINGS) return; @@ -172,11 +198,24 @@ namespace MobileGL::MG_State::GLState { auto& attr = m_attributes[attribIndex]; + // VERTEX_ATTRIB_ARRAY_DIVISOR is not independent per-attribute state: it IS the divisor + // of the binding point the attribute is attached to (GL 4.6 core 10.3.2), whichever API + // configured the attribute. glVertexBindingDivisor therefore has to reach a classic + // pointer-configured attribute as well - basic-state4 alternates the two spellings on + // the same attribute and expects each to win in turn. + if (attr.Divisor != binding.Divisor) { + attr.Divisor = binding.Divisor; + BumpAttributeFormatVersion(attribIndex); + } + + // Everything else stays owned by whichever API configured the attribute: a classic + // glVertexAttrib*Pointer attribute keeps its pointer-resolved stride and offset. + if (!m_attributeUsesBindingModel[attribIndex]) return; + const SizeT resolvedOffset = binding.Offset + m_attributeRelativeOffset[attribIndex]; - if (attr.Stride != binding.Stride || attr.Offset != resolvedOffset || attr.Divisor != binding.Divisor) { + if (attr.Stride != binding.Stride || attr.Offset != resolvedOffset) { attr.Stride = binding.Stride; attr.Offset = resolvedOffset; - attr.Divisor = binding.Divisor; BumpAttributeFormatVersion(attribIndex); } @@ -186,6 +225,14 @@ namespace MobileGL::MG_State::GLState { } } + void VertexArrayObject::ResolveAttributesForBinding(Uint bindingIndex, Bool adopt) { + for (Uint attribIndex = 0; attribIndex < MAX_VERTEX_ATTRIBS; ++attribIndex) { + if (m_attributeBindingIndex[attribIndex] != bindingIndex) continue; + if (adopt) m_attributeUsesBindingModel[attribIndex] = true; + ResolveAttributeFromBinding(attribIndex); + } + } + void VertexArrayObject::SetBindingBuffer(Uint bindingIndex, const SharedPtr& buffer, SizeT offset, int stride) { if (bindingIndex >= MAX_VERTEX_ATTRIB_BINDINGS) return; @@ -195,15 +242,10 @@ namespace MobileGL::MG_State::GLState { binding.Offset = offset; binding.Stride = stride; - for (Uint attribIndex = 0; attribIndex < MAX_VERTEX_ATTRIBS; ++attribIndex) { - if (m_attributeBindingIndex[attribIndex] == bindingIndex) { - // Binding a vertex buffer to a binding point adopts every attribute currently - // mapped to that binding point into the binding model (the default mapping is - // attribute i -> binding i, which matches the GL 4.3 rules for state mixing). - m_attributeUsesBindingModel[attribIndex] = true; - ResolveAttributeFromBinding(attribIndex); - } - } + // Binding a vertex buffer to a binding point adopts every attribute currently mapped to + // that binding point into the binding model (the default mapping is attribute i -> + // binding i, which matches the GL 4.3 rules for state mixing). + ResolveAttributesForBinding(bindingIndex, /*adopt: */ true); } void VertexArrayObject::SetBindingDivisor(Uint bindingIndex, Uint divisor) { @@ -211,11 +253,7 @@ namespace MobileGL::MG_State::GLState { m_bindingPoints[bindingIndex].Divisor = divisor; - for (Uint attribIndex = 0; attribIndex < MAX_VERTEX_ATTRIBS; ++attribIndex) { - if (m_attributeBindingIndex[attribIndex] == bindingIndex && m_attributeUsesBindingModel[attribIndex]) { - ResolveAttributeFromBinding(attribIndex); - } - } + ResolveAttributesForBinding(bindingIndex, /*adopt: */ false); } void VertexArrayObject::SetAttributeBinding(Uint attribIndex, Uint bindingIndex) { diff --git a/MobileGL/MG_State/GLState/VertexArrayState/VertexArrayObject.h b/MobileGL/MG_State/GLState/VertexArrayState/VertexArrayObject.h index 15af140c..34e6e75e 100644 --- a/MobileGL/MG_State/GLState/VertexArrayState/VertexArrayObject.h +++ b/MobileGL/MG_State/GLState/VertexArrayState/VertexArrayObject.h @@ -32,6 +32,16 @@ namespace MobileGL { Bool IsBgra = false; Uint Divisor = 0; SharedPtr Buffer; + + // GL 4.6 core table 23.3: VERTEX_ATTRIB_ARRAY_STRIDE and _POINTER are the + // arguments of the last glVertexAttrib*Pointer call on this attribute, + // reported verbatim, and NOTHING else writes them - not glVertexAttribFormat, + // not glBindVertexBuffer. Stride/Offset above are the *resolved* draw inputs + // and the binding model does overwrite those, so the two views have to be + // stored apart or the binding-model sequence reports a legacy state it never + // set (KHR-GL4x.vertex_attrib_binding.basic-state3). + int LegacyStride = 0; + SizeT LegacyPointer = 0; }; // ARB_vertex_attrib_binding separate binding point. Attributes configured through the @@ -40,7 +50,8 @@ namespace MobileGL { struct VertexBufferBindingPoint { SharedPtr Buffer; SizeT Offset = 0; - int Stride = 0; + // GL 4.6 core table 23.4: the initial VERTEX_BINDING_STRIDE is 16, not 0. + int Stride = 16; Uint Divisor = 0; }; @@ -185,6 +196,10 @@ namespace MobileGL { void BumpAttributeBufferVersion(Uint index); void BumpAttributeSwitchVersion(Uint index); void ResolveAttributeFromBinding(Uint attribIndex); + // Re-resolve every attribute currently pointed at `bindingIndex`. `adopt` turns + // the ones that are not in the binding model yet into binding-model attributes + // first (what glBindVertexBuffer does, GL 4.3 rules for state mixing). + void ResolveAttributesForBinding(Uint bindingIndex, Bool adopt); // The default mapping is attribute i -> binding point i. Keep it an iota over // MAX_VERTEX_ATTRIBS rather than a literal list: a literal list silently leaves the diff --git a/MobileGL/MG_Test/VertexArray/CMakeLists.txt b/MobileGL/MG_Test/VertexArray/CMakeLists.txt index bac2f850..8cb7a918 100644 --- a/MobileGL/MG_Test/VertexArray/CMakeLists.txt +++ b/MobileGL/MG_Test/VertexArray/CMakeLists.txt @@ -16,5 +16,22 @@ target_link_libraries( ${LINK_LIBRARIES} ) +add_executable( + VertexAttribBindingStateTest + VertexAttribBindingStateTest.cpp +) + +target_include_directories(VertexAttribBindingStateTest PRIVATE + ${MGL_ROOT}/include + ${MGL_ROOT}/MobileGL +) + +target_link_libraries( + VertexAttribBindingStateTest PRIVATE + GTest::gtest_main + ${LINK_LIBRARIES} +) + include(GoogleTest) gtest_discover_tests(VertexArrayTest DISCOVERY_TIMEOUT 30 PROPERTIES LABELS unit) +gtest_discover_tests(VertexAttribBindingStateTest DISCOVERY_TIMEOUT 30 PROPERTIES LABELS unit) diff --git a/MobileGL/MG_Test/VertexArray/VertexAttribBindingStateTest.cpp b/MobileGL/MG_Test/VertexArray/VertexAttribBindingStateTest.cpp new file mode 100644 index 00000000..b03202a7 --- /dev/null +++ b/MobileGL/MG_Test/VertexArray/VertexAttribBindingStateTest.cpp @@ -0,0 +1,430 @@ +// MobileGL - MobileGL/MG_Test/VertexArray/VertexAttribBindingStateTest.cpp +// Copyright (c) 2025-2026 MobileGL-Dev +// Licensed under the GNU Lesser General Public License v3.0: +// https://www.gnu.org/licenses/gpl-3.0.txt +// https://www.gnu.org/licenses/lgpl-3.0.txt +// SPDX-License-Identifier: LGPL-3.0-only +// End of Source File Header + +// The ARB_vertex_attrib_binding state model, replayed exactly as +// KHR-GL4x.vertex_attrib_binding.basic-state1/3/4 and .negative-* walk it +// (external/openglcts/modules/gl/gl4cVertexAttribBindingTests.cpp): after each mutation the +// ten per-attribute pnames and the four per-binding-point pnames are read back in full, which +// is what makes a single wrong field visible as itself instead of as a downstream render +// difference. +// +// Four defects are pinned here, all of them frontend-only (both backends reported them +// byte-identically): +// * VERTEX_BINDING_STRIDE defaulted to 0; the spec's initial value is 16. +// * The eager binding -> attribute resolve overwrote VERTEX_ATTRIB_ARRAY_STRIDE / _POINTER, +// which are legacy state only glVertexAttrib*Pointer may write. +// * glVertexAttribDivisor did not re-point the attribute at its own binding point, so a +// later resolve restored the old binding's divisor. +// * The binding entry points accepted the default vertex array (name 0) in a core profile. +// +// GPU-free: this is all GL object state, no backend is consulted. + +#include + +#include +#include + +#include "Includes.h" +#include "Init.h" +#include +#include +#include +#include +#include +#include + +using namespace MobileGL; +using namespace MobileGL::MG_Impl::GLImpl; + +namespace { + + // Mirrors the CTS's VertexAttribState: the initial per-attribute state, mutated field by + // field as the sequence proceeds, and verified in full after every call. + struct AttribState { + explicit AttribState(GLuint attribIndex) : index(attribIndex), binding(attribIndex) {} + + GLuint index = 0; + GLint enabled = 0; + GLint size = 4; + GLint stride = 0; + GLenum type = GL_FLOAT; + GLint normalized = 0; + GLint integer = 0; + GLint isLong = 0; + GLint divisor = 0; + GLuint pointer = 0; + GLuint bufferBinding = 0; + GLuint binding = 0; + GLint relativeOffset = 0; + + void Verify(const char* where) const { + GLint p = -1; + GetVertexAttribiv(index, GL_VERTEX_ATTRIB_ARRAY_ENABLED, &p); + EXPECT_EQ(p, enabled) << where << ": ENABLED(" << index << ")"; + GetVertexAttribiv(index, GL_VERTEX_ATTRIB_ARRAY_SIZE, &p); + EXPECT_EQ(p, size) << where << ": SIZE(" << index << ")"; + GetVertexAttribiv(index, GL_VERTEX_ATTRIB_ARRAY_STRIDE, &p); + EXPECT_EQ(p, stride) << where << ": STRIDE(" << index << ")"; + GetVertexAttribiv(index, GL_VERTEX_ATTRIB_ARRAY_TYPE, &p); + EXPECT_EQ(static_cast(p), type) << where << ": TYPE(" << index << ")"; + GetVertexAttribiv(index, GL_VERTEX_ATTRIB_ARRAY_NORMALIZED, &p); + EXPECT_EQ(p, normalized) << where << ": NORMALIZED(" << index << ")"; + GetVertexAttribiv(index, GL_VERTEX_ATTRIB_ARRAY_INTEGER, &p); + EXPECT_EQ(p, integer) << where << ": INTEGER(" << index << ")"; + GetVertexAttribiv(index, GL_VERTEX_ATTRIB_ARRAY_LONG, &p); + EXPECT_EQ(p, isLong) << where << ": LONG(" << index << ")"; + GetVertexAttribiv(index, GL_VERTEX_ATTRIB_ARRAY_DIVISOR, &p); + EXPECT_EQ(p, divisor) << where << ": DIVISOR(" << index << ")"; + void* pp = nullptr; + GetVertexAttribPointerv(index, GL_VERTEX_ATTRIB_ARRAY_POINTER, &pp); + EXPECT_EQ(reinterpret_cast(pp), static_cast(pointer)) + << where << ": POINTER(" << index << ")"; + GetVertexAttribiv(index, GL_VERTEX_ATTRIB_ARRAY_BUFFER_BINDING, &p); + EXPECT_EQ(static_cast(p), bufferBinding) << where << ": BUFFER_BINDING(" << index << ")"; + GetVertexAttribiv(index, GL_VERTEX_ATTRIB_BINDING, &p); + EXPECT_EQ(static_cast(p), binding) << where << ": BINDING(" << index << ")"; + GetVertexAttribiv(index, GL_VERTEX_ATTRIB_RELATIVE_OFFSET, &p); + EXPECT_EQ(p, relativeOffset) << where << ": RELATIVE_OFFSET(" << index << ")"; + } + }; + + // Mirrors the CTS's VertexBindingState, initial stride 16 included. + struct BindingState { + explicit BindingState(GLuint bindingIndex) : index(bindingIndex) {} + + GLuint index = 0; + GLuint buffer = 0; + GLint offset = 0; + GLint stride = 16; + GLint divisor = 0; + + void Verify(const char* where) const { + GLint p = -1; + GetIntegeri_v(GL_VERTEX_BINDING_BUFFER, index, &p); + EXPECT_EQ(static_cast(p), buffer) << where << ": VERTEX_BINDING_BUFFER(" << index << ")"; + // The CTS reads the offset through glGetInteger64i_v; that entry point's pname + // routing is a separate defect with its own regression (see the indexed-getter + // parity test), so the state model is pinned through the 32-bit view here. + GetIntegeri_v(GL_VERTEX_BINDING_OFFSET, index, &p); + EXPECT_EQ(p, offset) << where << ": VERTEX_BINDING_OFFSET(" << index << ")"; + GetIntegeri_v(GL_VERTEX_BINDING_STRIDE, index, &p); + EXPECT_EQ(p, stride) << where << ": VERTEX_BINDING_STRIDE(" << index << ")"; + GetIntegeri_v(GL_VERTEX_BINDING_DIVISOR, index, &p); + EXPECT_EQ(p, divisor) << where << ": VERTEX_BINDING_DIVISOR(" << index << ")"; + } + }; + + // Strict core rules only apply when the current EGL context explicitly asked for a core + // profile; the suite's default (no current context) is relaxed. RAII so a failed + // expectation cannot leave the context current for the rest of the binary. + struct ScopedCoreProfileContext { + ScopedCoreProfileContext() { + auto& egl = *MG_State::pEGLContext; + m_display = egl.GetDisplay(EGL_DEFAULT_DISPLAY); + EXPECT_NE(m_display, EGL_NO_DISPLAY); + EXPECT_TRUE(egl.InitializeDisplay(m_display, nullptr, nullptr)); + EGLint configCount = 0; + EXPECT_TRUE(egl.ChooseConfig(m_display, nullptr, &m_config, 1, &configCount)); + const EGLint surfaceAttribs[] = {EGL_WIDTH, 1, EGL_HEIGHT, 1, EGL_NONE}; + m_surface = egl.CreatePbufferSurface(m_display, m_config, surfaceAttribs); + EXPECT_NE(m_surface, EGL_NO_SURFACE); + const EGLint contextAttribs[] = {EGL_CONTEXT_MAJOR_VERSION, + 3, + EGL_CONTEXT_MINOR_VERSION, + 3, + EGL_CONTEXT_OPENGL_PROFILE_MASK, + EGL_CONTEXT_OPENGL_CORE_PROFILE_BIT, + EGL_NONE}; + m_context = egl.CreateContext(m_display, m_config, EGL_NO_CONTEXT, contextAttribs); + EXPECT_NE(m_context, EGL_NO_CONTEXT); + EXPECT_TRUE(egl.MakeCurrent(m_display, m_surface, m_surface, m_context)); + } + ~ScopedCoreProfileContext() { + auto& egl = *MG_State::pEGLContext; + egl.MakeCurrent(EGL_NO_DISPLAY, EGL_NO_SURFACE, EGL_NO_SURFACE, EGL_NO_CONTEXT); + if (m_context != EGL_NO_CONTEXT) egl.DestroyContext(m_display, m_context); + if (m_surface != EGL_NO_SURFACE) egl.DestroySurface(m_display, m_surface); + } + ScopedCoreProfileContext(const ScopedCoreProfileContext&) = delete; + ScopedCoreProfileContext& operator=(const ScopedCoreProfileContext&) = delete; + + private: + EGLDisplay m_display = EGL_NO_DISPLAY; + EGLConfig m_config = nullptr; + EGLSurface m_surface = EGL_NO_SURFACE; + MG_State::EGLState::EGLContext::EGLContextHandle m_context = EGL_NO_CONTEXT; + }; + + class VertexAttribBindingStateTest : public ::testing::Test { + protected: + void SetUp() override { + MobileGL::Initialize(); + // A fresh context per case: the state model under test is cumulative, so a leftover + // VAO binding from a neighbour would silently change what "default state" means. + MG_State::pGLContext = MakeUnique(); + GenVertexArrays(1, &m_vao); + BindVertexArray(m_vao); + } + + void TearDown() override { + EXPECT_EQ(GetError(), GL_NO_ERROR) << "test left an unconsumed GL error behind"; + } + + GLuint CreateVbo(GLsizeiptr size) { + GLuint vbo = 0; + GenBuffers(1, &vbo); + BindBuffer(GL_ARRAY_BUFFER, vbo); + BufferData(GL_ARRAY_BUFFER, size, nullptr, GL_DYNAMIC_COPY); + BindBuffer(GL_ARRAY_BUFFER, 0); + return vbo; + } + + static void DrainErrors() { + for (int i = 0; i < 16 && GetError() != GL_NO_ERROR; ++i) { + } + } + + GLuint m_vao = 0; + }; + + // basic-state1's opening block: the initial per-attribute mapping and the per-binding-point + // defaults, VERTEX_BINDING_STRIDE = 16 included. That check is the FIRST thing the CTS case + // does, so a wrong default masked everything the case would have found after it. + TEST_F(VertexAttribBindingStateTest, DefaultsMatchTheSpecInitialState) { + for (GLuint i = 0; i < 16; ++i) { + AttribState(i).Verify("defaults"); + BindingState(i).Verify("defaults"); + } + EXPECT_EQ(GetError(), GL_NO_ERROR); + } + + // basic-state3, verbatim: a full separate-format sequence, then a pointer call, then a + // binding update on top of it. The legacy STRIDE/POINTER pair must stay untouched by every + // step except the glVertexAttribPointer one, and must survive the binding update after it. + TEST_F(VertexAttribBindingStateTest, SeparateFormatSequenceKeepsLegacyStrideAndPointerAtZero) { + const GLuint vbo0 = CreateVbo(10000); + const GLuint vbo1 = CreateVbo(10000); + const GLuint vbo2 = CreateVbo(10000); + ASSERT_EQ(GetError(), GL_NO_ERROR); + + AttribState va0(0), va2(2), va15(15); + BindingState vb0(0), vb2(2), vb15(15); + + VertexAttribFormat(0, 2, GL_BYTE, GL_TRUE, 16); + va0.size = 2; + va0.type = GL_BYTE; + va0.normalized = 1; + va0.relativeOffset = 16; + va0.Verify("after glVertexAttribFormat"); + // The format call says nothing about a buffer, so binding point 0 keeps its defaults - + // stride 16 among them. + vb0.Verify("after glVertexAttribFormat"); + + VertexAttribIFormat(2, 3, GL_INT, 512); + va2.size = 3; + va2.type = GL_INT; + va2.integer = 1; + va2.relativeOffset = 512; + va2.Verify("after glVertexAttribIFormat"); + vb2.Verify("after glVertexAttribIFormat"); + + BindVertexBuffer(0, vbo0, 2048, 128); + va0.bufferBinding = vbo0; + vb0.buffer = vbo0; + vb0.offset = 2048; + vb0.stride = 128; + va0.Verify("after glBindVertexBuffer(0)"); + vb0.Verify("after glBindVertexBuffer(0)"); + + BindVertexBuffer(2, vbo2, 64, 256); + va2.bufferBinding = vbo2; + vb2.buffer = vbo2; + vb2.offset = 64; + vb2.stride = 256; + va2.Verify("after glBindVertexBuffer(2)"); + vb2.Verify("after glBindVertexBuffer(2)"); + + // Attribute 2 moves onto binding 0 and takes that binding point's buffer with it. + VertexAttribBinding(2, 0); + va2.binding = 0; + va2.bufferBinding = vbo0; + va0.Verify("after glVertexAttribBinding(2,0)"); + vb0.Verify("after glVertexAttribBinding(2,0)"); + va2.Verify("after glVertexAttribBinding(2,0)"); + vb2.Verify("after glVertexAttribBinding(2,0)"); + + VertexAttribBinding(0, 15); + va0.binding = 15; + va0.bufferBinding = 0; + va0.Verify("after glVertexAttribBinding(0,15)"); + vb0.Verify("after glVertexAttribBinding(0,15)"); + va15.Verify("after glVertexAttribBinding(0,15)"); + vb15.Verify("after glVertexAttribBinding(0,15)"); + + BindVertexBuffer(15, vbo1, 16, 32); + va0.bufferBinding = vbo1; + va15.bufferBinding = vbo1; + vb15.buffer = vbo1; + vb15.offset = 16; + vb15.stride = 32; + va0.Verify("after glBindVertexBuffer(15)"); + va15.Verify("after glBindVertexBuffer(15)"); + vb15.Verify("after glBindVertexBuffer(15)"); + + // The one call that IS allowed to write the legacy pair - and it also re-points the + // attribute at its own binding point and rewrites that binding point. + BindBuffer(GL_ARRAY_BUFFER, vbo2); + VertexAttribPointer(0, 4, GL_UNSIGNED_BYTE, GL_FALSE, 8, reinterpret_cast(640)); + BindBuffer(GL_ARRAY_BUFFER, 0); + va0.size = 4; + va0.type = GL_UNSIGNED_BYTE; + va0.stride = 8; + va0.pointer = 640; + va0.relativeOffset = 0; + va0.normalized = 0; + va0.binding = 0; + va0.bufferBinding = vbo2; + vb0.buffer = vbo2; + vb0.offset = 640; + vb0.stride = 8; + va2.bufferBinding = vbo2; + va0.Verify("after glVertexAttribPointer"); + vb0.Verify("after glVertexAttribPointer"); + va2.Verify("after glVertexAttribPointer"); + va15.Verify("after glVertexAttribPointer"); + vb15.Verify("after glVertexAttribPointer"); + + // ...and a binding update on top of it leaves the legacy pair exactly where the pointer + // call left it. This is the assertion the eager resolve used to fail. + BindVertexBuffer(0, vbo1, 80, 24); + vb0.buffer = vbo1; + vb0.offset = 80; + vb0.stride = 24; + va0.bufferBinding = vbo1; + va2.bufferBinding = vbo1; + va0.Verify("after the trailing glBindVertexBuffer(0)"); + vb0.Verify("after the trailing glBindVertexBuffer(0)"); + va2.Verify("after the trailing glBindVertexBuffer(0)"); + EXPECT_EQ(GetError(), GL_NO_ERROR); + } + + // basic-state4: glVertexAttribDivisor is VertexAttribBinding(i,i) + VertexBindingDivisor(i,d), + // and glVertexBindingDivisor reaches the attribute's own DIVISOR query either way. + TEST_F(VertexAttribBindingStateTest, DivisorGoesThroughTheBindingPoint) { + for (GLuint i = 0; i < 16; ++i) { + AttribState va(i); + BindingState vb(i); + VertexAttribDivisor(i, i + 7); + va.divisor = static_cast(i + 7); + vb.divisor = static_cast(i + 7); + va.Verify("after glVertexAttribDivisor"); + vb.Verify("after glVertexAttribDivisor"); + } + for (GLuint i = 0; i < 16; ++i) { + AttribState va(i); + BindingState vb(i); + VertexBindingDivisor(i, i); + va.divisor = static_cast(i); + vb.divisor = static_cast(i); + va.Verify("after glVertexBindingDivisor"); + vb.Verify("after glVertexBindingDivisor"); + } + + // Attribute 2 moves onto binding 5 and inherits binding 5's divisor; binding 2 keeps its + // own. + VertexAttribBinding(2, 5); + AttribState va5(5); + va5.divisor = 5; + BindingState vb5(5); + vb5.divisor = 5; + AttribState va2(2); + va2.divisor = 5; + va2.binding = 5; + BindingState vb2(2); + vb2.divisor = 2; + va5.Verify("after glVertexAttribBinding(2,5)"); + vb5.Verify("after glVertexAttribBinding(2,5)"); + va2.Verify("after glVertexAttribBinding(2,5)"); + vb2.Verify("after glVertexAttribBinding(2,5)"); + + // ...and glVertexAttribDivisor pulls it back onto binding 2. Guarding the write on + // "binding already == index" left the attribute on binding 5 and threw the divisor away. + VertexAttribDivisor(2, 23); + va2.binding = 2; + va2.divisor = 23; + vb2.divisor = 23; + va5.Verify("after glVertexAttribDivisor(2,23)"); + vb5.Verify("after glVertexAttribDivisor(2,23)"); + va2.Verify("after glVertexAttribDivisor(2,23)"); + vb2.Verify("after glVertexAttribDivisor(2,23)"); + EXPECT_EQ(GetError(), GL_NO_ERROR); + } + + // The tail of every negative-* case: with the default vertex array bound, a core profile + // rejects all four binding entry points. + TEST_F(VertexAttribBindingStateTest, BindingApiRejectsTheDefaultVertexArrayInCoreProfile) { + ScopedCoreProfileContext coreContext; + ASSERT_FALSE(MG_State::IsRelaxedSemanticsActive()); + DrainErrors(); + + BindVertexArray(0); + ASSERT_EQ(GetError(), GL_NO_ERROR); + + BindVertexBuffer(0, 7, 0, 12); + EXPECT_EQ(GetError(), GL_INVALID_OPERATION) << "glBindVertexBuffer"; + VertexAttribFormat(0, 4, GL_FLOAT, GL_FALSE, 0); + EXPECT_EQ(GetError(), GL_INVALID_OPERATION) << "glVertexAttribFormat"; + VertexAttribIFormat(0, 4, GL_INT, 0); + EXPECT_EQ(GetError(), GL_INVALID_OPERATION) << "glVertexAttribIFormat"; + VertexAttribBinding(0, 0); + EXPECT_EQ(GetError(), GL_INVALID_OPERATION) << "glVertexAttribBinding"; + VertexBindingDivisor(0, 1); + EXPECT_EQ(GetError(), GL_INVALID_OPERATION) << "glVertexBindingDivisor"; + + BindVertexArray(m_vao); + DrainErrors(); + } + + // ...and the relaxed default - which is what every context that never asked for a core + // profile gets - keeps accepting them, because applications depend on it. + TEST_F(VertexAttribBindingStateTest, BindingApiStillAcceptsTheDefaultVertexArrayWhenRelaxed) { + ASSERT_TRUE(MG_State::IsRelaxedSemanticsActive()); + const GLuint vbo = CreateVbo(1024); + DrainErrors(); + + BindVertexArray(0); + BindVertexBuffer(0, vbo, 0, 12); + EXPECT_EQ(GetError(), GL_NO_ERROR) << "glBindVertexBuffer under relaxed semantics"; + VertexAttribFormat(0, 4, GL_FLOAT, GL_FALSE, 0); + EXPECT_EQ(GetError(), GL_NO_ERROR) << "glVertexAttribFormat under relaxed semantics"; + VertexAttribBinding(0, 0); + EXPECT_EQ(GetError(), GL_NO_ERROR) << "glVertexAttribBinding under relaxed semantics"; + VertexBindingDivisor(0, 1); + EXPECT_EQ(GetError(), GL_NO_ERROR) << "glVertexBindingDivisor under relaxed semantics"; + + BindVertexArray(m_vao); + DrainErrors(); + } + + // MOBILEGL_RELAXED_SEMANTICS wins even on an explicit core-profile context. + TEST_F(VertexAttribBindingStateTest, RelaxedSemanticsOverrideReopensTheDefaultVertexArray) { + ScopedCoreProfileContext coreContext; + const Bool saved = MG_Config::Features.RelaxedSemantics; + MG_Config::Features.RelaxedSemantics = true; + const GLuint vbo = CreateVbo(1024); + DrainErrors(); + + BindVertexArray(0); + BindVertexBuffer(0, vbo, 0, 12); + EXPECT_EQ(GetError(), GL_NO_ERROR); + + BindVertexArray(m_vao); + MG_Config::Features.RelaxedSemantics = saved; + DrainErrors(); + } +} // namespace