diff --git a/MobileGL/MG_Backend/DirectGLES/BackendObject_DirectGLES.cpp b/MobileGL/MG_Backend/DirectGLES/BackendObject_DirectGLES.cpp index 053de746..a81d8f7a 100644 --- a/MobileGL/MG_Backend/DirectGLES/BackendObject_DirectGLES.cpp +++ b/MobileGL/MG_Backend/DirectGLES/BackendObject_DirectGLES.cpp @@ -981,7 +981,11 @@ namespace MobileGL::MG_Backend::DirectGLES { m_dynamicParameters.MaxVertexTextureImageUnits = m_GLESCapabilities.MaxVertexTextureImageUnits; m_dynamicParameters.MaxComputeTextureImageUnits = m_GLESCapabilities.MaxComputeTextureImageUnits; m_dynamicParameters.MaxCombinedTextureImageUnits = m_GLESCapabilities.MaxCombinedTextureImageUnits; - m_dynamicParameters.MaxVertexAttribs = m_GLESCapabilities.MaxVertexAttribs; + // Never advertise more attributes than the state layer can store: the current-value array and + // the Uint32 attribute masks the draw path passes around are both bounded by MAX_VERTEX_ATTRIBS. + m_dynamicParameters.MaxVertexAttribs = + std::min(m_GLESCapabilities.MaxVertexAttribs, + static_cast(MG_State::GLState::VertexArrayObject::MAX_VERTEX_ATTRIBS)); m_dynamicParameters.MaxComputeShaderStorageBlocks = m_GLESCapabilities.MaxComputeShaderStorageBlocks; m_dynamicParameters.MaxCombinedShaderStorageBlocks = m_GLESCapabilities.MaxCombinedShaderStorageBlocks; m_dynamicParameters.MaxComputeUniformBlocks = m_GLESCapabilities.MaxComputeUniformBlocks; diff --git a/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp b/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp index cbdc1da0..1588ca28 100644 --- a/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp +++ b/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp @@ -321,6 +321,52 @@ namespace MobileGL::MG_Backend::DirectGLES { } backendObj->SyncToBackend(currentVAOObject); } + + // GL: a shader input whose generic attribute array is DISABLED reads that attribute's *current + // value* (context state set by glVertexAttrib*, default (0,0,0,1)) rather than any buffer. + // MobileGL stores those values in MG_State only, so without this step the ES driver would feed + // the shader its own current values, which MobileGL never writes -- i.e. always (0,0,0,1). + // SyncToBackend has already issued glDisableVertexAttribArray for these locations, so the ES + // current value is what the shader will actually read. + void SyncCurrentVertexAttributeValues() { +#ifdef TRACY_ENABLE + ZoneScopedC(TRACY_ZONECOLOR_BACKEND); +#endif + const auto& program = MG_State::pGLContext->GetCurrentProgram(); + if (!program) return; + + const auto& vao = MG_State::pGLContext->GetBoundVertexArray(); + if (!vao) return; + + const Uint32 activeAttribMask = program->GetActiveAttributeLocationMask(); + if (activeAttribMask == 0) return; + + constexpr Uint32 maxVertexAttribs = + static_cast(MG_State::GLState::VertexArrayObject::MAX_VERTEX_ATTRIBS); + for (Uint32 location = 0; location < maxVertexAttribs; ++location) { + if ((activeAttribMask & (1u << location)) == 0) continue; + if (vao->GetAttribute(location).Enabled) continue; + + const auto& currentValue = MG_State::pGLContext->GetCurrentVertexAttribute(location); + const auto typeInfo = MG_State::GLState::ClassifyVertexAttribType(program->GetAttribType(location)); + switch (typeInfo.baseType) { + case MG_State::GLState::VertexAttribBaseType::Float: + g_GLESFuncs.glVertexAttrib4fv(location, currentValue.floatValue.data()); + break; + case MG_State::GLState::VertexAttribBaseType::Int: + g_GLESFuncs.glVertexAttribI4iv(location, currentValue.intValue.data()); + break; + case MG_State::GLState::VertexAttribBaseType::Uint: + g_GLESFuncs.glVertexAttribI4uiv(location, currentValue.uintValue.data()); + break; + case MG_State::GLState::VertexAttribBaseType::Unsupported: + MGLOG_E("SyncCurrentVertexAttributeValues: program=%u location=%u has no enabled array and its " + "shader input type 0x%x is not supported as a current generic vertex attribute", + program->GetExternalIndex(), location, program->GetAttribType(location)); + break; + } + } + } } // namespace VertexArrayImpl namespace TextureImpl { @@ -864,6 +910,8 @@ namespace MobileGL::MG_Backend::DirectGLES { } } + VertexArrayImpl::SyncCurrentVertexAttributeValues(); + BindCurrentTextures(); BindCurrentProgramWithResources(); } diff --git a/MobileGL/MG_Backend/DirectVulkan/BackendObject_DirectVulkan.cpp b/MobileGL/MG_Backend/DirectVulkan/BackendObject_DirectVulkan.cpp index baca8ade..3585eb6b 100644 --- a/MobileGL/MG_Backend/DirectVulkan/BackendObject_DirectVulkan.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/BackendObject_DirectVulkan.cpp @@ -725,7 +725,11 @@ namespace MobileGL::MG_Backend::DirectVulkan { std::min(m_vulkanCaps.MaxComputeTextureImageUnits, maxSupportedTextureUnits); m_dynamicParameters.MaxCombinedTextureImageUnits = std::min(m_vulkanCaps.MaxCombinedTextureImageUnits, maxSupportedTextureUnits); - m_dynamicParameters.MaxVertexAttribs = m_vulkanCaps.MaxVertexAttribs; + // Never advertise more attributes than the state layer can store: the current-value array and + // the Uint32 attribute masks the draw path passes around are both bounded by MAX_VERTEX_ATTRIBS. + m_dynamicParameters.MaxVertexAttribs = + std::min(m_vulkanCaps.MaxVertexAttribs, + static_cast(MG_State::GLState::VertexArrayObject::MAX_VERTEX_ATTRIBS)); m_dynamicParameters.MaxComputeShaderStorageBlocks = m_vulkanCaps.MaxComputeShaderStorageBlocks; m_dynamicParameters.MaxCombinedShaderStorageBlocks = m_vulkanCaps.MaxCombinedShaderStorageBlocks; m_dynamicParameters.MaxComputeUniformBlocks = m_vulkanCaps.MaxComputeUniformBlocks; diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/VertexInputStateFactory.cpp b/MobileGL/MG_Backend/DirectVulkan/Renderer/VertexInputStateFactory.cpp index 7e83a95e..048d0731 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VertexInputStateFactory.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/VertexInputStateFactory.cpp @@ -65,6 +65,7 @@ namespace MobileGL::MG_Backend::DirectVulkan { Vector bindingBaseOffsets; Vector bindingAttributeLocations; Vector bindingUsesClientMemory; + Uint32 unsupportedAttribMask = 0; for (Uint32 location = 0; location < MG_State::GLState::VertexArrayObject::MAX_VERTEX_ATTRIBS; ++location) { const auto& attr = vao.GetAttribute(location); @@ -74,15 +75,19 @@ namespace MobileGL::MG_Backend::DirectVulkan { const auto vkFormat = ToVkVertexFormat(attr.Type, attr.Size, attr.Normalized, attr.IsInteger); if (vkFormat == VK_FORMAT_UNDEFINED) { - MGLOG_D("Skipping unsupported vertex attribute layout (location=%u, type=%s, size=%d)", + MGLOG_E("Unsupported vertex attribute layout (location=%u, type=%s, size=%d): the array is " + "enabled but cannot be mapped to a VkFormat", location, MG_Util::ConvertDataTypeToString(attr.Type).c_str(), attr.Size); + unsupportedAttribMask |= (1u << location); continue; } const SizeT componentSize = GetComponentSize(attr.Type); if (componentSize == 0) { - MGLOG_D("Skipping vertex attribute with unknown component size (location=%u, type=%s)", + MGLOG_E("Vertex attribute with unknown component size (location=%u, type=%s): the array is " + "enabled but cannot be sized", location, MG_Util::ConvertDataTypeToString(attr.Type).c_str()); + unsupportedAttribMask |= (1u << location); continue; } @@ -112,6 +117,7 @@ namespace MobileGL::MG_Backend::DirectVulkan { entry.bindingBaseOffsets = std::move(bindingBaseOffsets); entry.bindingAttributeLocations = std::move(bindingAttributeLocations); entry.bindingUsesClientMemory = std::move(bindingUsesClientMemory); + entry.unsupportedAttribMask = unsupportedAttribMask; entry.state = state; entry.state.pVertexBindingDescriptions = entry.bindings.empty() ? nullptr : entry.bindings.data(); entry.state.pVertexAttributeDescriptions = entry.attributes.empty() ? nullptr : entry.attributes.data(); @@ -128,6 +134,17 @@ namespace MobileGL::MG_Backend::DirectVulkan { case 4: return VK_FORMAT_R32G32B32A32_SFLOAT; default: return VK_FORMAT_UNDEFINED; } + case DataType::Float16: + // GL_HALF_FLOAT is a floating-point array type: it is never an integer attribute, and + // GL_TRUE for `normalized` is ignored for float types rather than selecting a *NORM format. + if (isInteger) return VK_FORMAT_UNDEFINED; + switch (size) { + case 1: return VK_FORMAT_R16_SFLOAT; + case 2: return VK_FORMAT_R16G16_SFLOAT; + case 3: return VK_FORMAT_R16G16B16_SFLOAT; + case 4: return VK_FORMAT_R16G16B16A16_SFLOAT; + default: return VK_FORMAT_UNDEFINED; + } case DataType::Int32: if (!isInteger || normalized) return VK_FORMAT_UNDEFINED; switch (size) { diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/VertexInputStateFactory.h b/MobileGL/MG_Backend/DirectVulkan/Renderer/VertexInputStateFactory.h index f1c108df..5661e8c4 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VertexInputStateFactory.h +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/VertexInputStateFactory.h @@ -27,6 +27,10 @@ namespace MobileGL::MG_Backend::DirectVulkan { Vector bindingBaseOffsets; Vector bindingAttributeLocations; Vector bindingUsesClientMemory; + // Locations whose array is ENABLED but whose GL format has no VkFormat mapping. They are + // absent from `attributes`, so without this mask the draw path cannot tell them apart from + // a genuinely disabled array and would silently feed the shader the current attribute value. + Uint32 unsupportedAttribMask = 0; VkPipelineVertexInputStateCreateInfo state{ VK_STRUCTURE_TYPE_PIPELINE_VERTEX_INPUT_STATE_CREATE_INFO }; diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp index 04a67702..db5465c5 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp @@ -607,10 +607,20 @@ namespace MobileGL::MG_Backend::DirectVulkan { } } + // Vertex attribute locations are tracked in Uint32 bitmasks, so MAX_VERTEX_ATTRIBS is both the + // state-layer storage bound and the width of every mask below. Keep them in lockstep. + static constexpr Uint32 kMaxVertexAttribs = + static_cast(MG_State::GLState::VertexArrayObject::MAX_VERTEX_ATTRIBS); + static_assert(kMaxVertexAttribs <= 32, "Vertex attribute masks are Uint32"); + // The loops below walk locations [0, kMaxVertexAttribs) and index programObj.vertexInputTypes with + // each one, so that array must be at least as wide. + static_assert(kMaxVertexAttribs <= ProgramFactory::VkProgramObject::kMaxVertexInputLocations, + "vertexInputTypes is indexed by vertex attribute location"); + static Uint32 BuildVertexInputAttributeMask(const Vector& attributes) { Uint32 attributeMask = 0; for (const auto& attribute : attributes) { - if (attribute.location < 32) { + if (attribute.location < kMaxVertexAttribs) { attributeMask |= (1u << attribute.location); } } @@ -2194,7 +2204,7 @@ void main() { } SizeT syntheticBinding = vertexInputState.bindings.size(); - for (Uint32 location = 0; location < 32; ++location) { + for (Uint32 location = 0; location < kMaxVertexAttribs; ++location) { if ((missingAttribMask & (1u << location)) == 0) { continue; } @@ -2206,9 +2216,13 @@ void main() { VkDeviceSize sourceSize = 0; const Bool supported = TryGetCurrentVertexAttributeUploadPayload(currentValue, glType, format, sourceData, sourceSize); - MOBILEGL_ASSERT(supported, - "DirectVulkan does not support current generic vertex attribute type yet: program=%u location=%u type=0x%x", - program.GetExternalIndex(), location, glType); + if (!supported) { + // SetupDraw's pre-flight should have rejected this already; never upload a null payload. + MGLOG_E("UploadAndBindVertexStreams skipped: unsupported current generic vertex attribute type: " + "program=%u location=%u type=0x%x", + program.GetExternalIndex(), location, glType); + return false; + } BufferSlice slice{}; if (!m_bufferManager.UploadTransient(BufferKind::Vertex, m_frameContext.GetCurrentFrameIndex(), @@ -2877,7 +2891,7 @@ void main() { patchedAttributes.assign(vis.attributes.begin(), vis.attributes.end()); Bool hasPatchedVertexAttributes = false; for (auto& attribute : patchedAttributes) { - if (attribute.location >= 32 || (activeAttribMask & (1u << attribute.location)) == 0) { + if (attribute.location >= kMaxVertexAttribs || (activeAttribMask & (1u << attribute.location)) == 0) { continue; } @@ -2920,7 +2934,7 @@ void main() { } Uint32 syntheticBinding = static_cast(vis.bindings.size()); - for (Uint32 location = 0; location < 32; ++location) { + for (Uint32 location = 0; location < kMaxVertexAttribs; ++location) { if ((missingAttribMask & (1u << location)) == 0) { continue; } @@ -3297,6 +3311,42 @@ void main() { return false; } + // Vertex-input pre-flight, run before pipeline creation so that a bad attribute can never be + // baked into a cached VkPipeline. + { + const auto& vertexInputState = m_vertexInputStateFactory->GetOrCreateVertexInputState(vao); + const Uint32 activeAttribMask = programObj.activeVertexInputLocationMask; + + // An enabled array whose GL type has no VkFormat mapping never reaches the vertex input + // state, which makes it indistinguishable from a disabled array: the draw would treat it as + // "missing" and silently feed the shader the current attribute value instead of the app's + // vertex data. Fail loudly rather than render wrong pixels. + const Uint32 brokenAttribMask = vertexInputState.unsupportedAttribMask & activeAttribMask; + if (brokenAttribMask != 0) { + MGLOG_E("SetupDraw skipped: program=%u reads vertex attribute location mask 0x%x whose enabled " + "array has no supported vertex format", + program.GetExternalIndex(), brokenAttribMask); + return false; + } + + // Every genuinely disabled attribute the shader reads must have a current-value type we can + // synthesize a binding for; otherwise the upload below would push a null payload. + const Uint32 missingAttribMask = + activeAttribMask & ~BuildVertexInputAttributeMask(vertexInputState.attributes); + for (Uint32 location = 0; location < kMaxVertexAttribs; ++location) { + if ((missingAttribMask & (1u << location)) == 0) continue; + + const GLenum glType = programObj.vertexInputTypes[location]; + if (MG_State::GLState::ClassifyVertexAttribType(glType).baseType == + MG_State::GLState::VertexAttribBaseType::Unsupported) { + MGLOG_E("SetupDraw skipped: program=%u location=%u has no enabled array and its shader input " + "type 0x%x is not supported as a current generic vertex attribute", + program.GetExternalIndex(), location, glType); + return false; + } + } + } + auto pipeline = GetOrCreatePipeline(mode, program, programObj, transformFlags, vao, *renderPassEntry); activeRenderPass = VkRenderPassManager::GetActiveRenderPass(); diff --git a/MobileGL/MG_Impl/GLImpl/Getter/GL_Getter.cpp b/MobileGL/MG_Impl/GLImpl/Getter/GL_Getter.cpp index 00e56e17..8057347a 100644 --- a/MobileGL/MG_Impl/GLImpl/Getter/GL_Getter.cpp +++ b/MobileGL/MG_Impl/GLImpl/Getter/GL_Getter.cpp @@ -9,6 +9,7 @@ #include "GL_Getter.h" #include #include +#include #include #include #include @@ -1845,7 +1846,10 @@ namespace MobileGL::MG_Impl::GLImpl { *params = dynamicParameters.MaxUniformBlockSize; break; case GL_MAX_VERTEX_ATTRIBS: - *params = dynamicParameters.MaxVertexAttribs; + // Single source of truth with the validators: the value reported here is exactly the bound + // glVertexAttrib*/glGetVertexAttrib*/glBindAttribLocation enforce, and it never exceeds the + // state layer's current-value storage capacity. + *params = static_cast(VertexArrayImpl::GetMaxVertexAttribs()); break; case GL_MAX_VERTEX_TEXTURE_IMAGE_UNITS: *params = dynamicParameters.MaxVertexTextureImageUnits; diff --git a/MobileGL/MG_Impl/GLImpl/Program/GL_Program.cpp b/MobileGL/MG_Impl/GLImpl/Program/GL_Program.cpp index dc693582..2191c177 100644 --- a/MobileGL/MG_Impl/GLImpl/Program/GL_Program.cpp +++ b/MobileGL/MG_Impl/GLImpl/Program/GL_Program.cpp @@ -8,6 +8,7 @@ #include "GL_Program.h" #include "Config.h" +#include #include #include #include @@ -270,7 +271,7 @@ namespace MobileGL::MG_Impl::GLImpl { } void BindAttribLocation_State(GLuint program, GLuint index, const GLchar* name) { - if (index >= MG_State::GLState::VertexArrayObject::MAX_VERTEX_ATTRIBS) { + if (index >= VertexArrayImpl::GetMaxVertexAttribs()) { MG_State::pGLContext->RecordError( ErrorCode::InvalidValue, MakeUnique("MG_Impl/GLImpl", __func__, diff --git a/MobileGL/MG_Impl/GLImpl/VertexArray/GL_VertexArray.cpp b/MobileGL/MG_Impl/GLImpl/VertexArray/GL_VertexArray.cpp index 35261711..63ac57d8 100644 --- a/MobileGL/MG_Impl/GLImpl/VertexArray/GL_VertexArray.cpp +++ b/MobileGL/MG_Impl/GLImpl/VertexArray/GL_VertexArray.cpp @@ -48,7 +48,11 @@ namespace MobileGL::MG_Impl::GLImpl { } static bool ValidateVertexBindingIndex(GLuint bindingindex, const char* funcName) { - if (bindingindex >= MG_State::GLState::VertexArrayObject::MAX_VERTEX_ATTRIB_BINDINGS) { + // Bound by the same dynamic limit as attribute indices: the default attribute -> binding + // mapping is the identity, so a binding point the backend cannot address as an attribute + // would resolve into an attribute the backend must then reject on every draw. Real drivers + // likewise report MAX_VERTEX_ATTRIB_BINDINGS == MAX_VERTEX_ATTRIBS. + if (bindingindex >= VertexArrayImpl::GetMaxVertexAttribs()) { MG_State::pGLContext->RecordError( ErrorCode::InvalidValue, MakeUnique("MG_Impl/GLImpl", funcName, @@ -503,6 +507,9 @@ namespace MobileGL::MG_Impl::GLImpl { MakeUnique("MG_Impl/GLImpl", __func__, "params pointer cannot be null.")); return; } + // GL_CURRENT_VERTEX_ATTRIB is context state and returns before TryGetVertexAttribute, so the + // index bound has to be enforced up front or an out-of-range index reads past the array. + if (!VertexArrayImpl::ValidateVertexAttributeIndex(index)) return; if (!ValidateVertexAttribPname(pname)) return; if (IsCurrentVertexAttribQuery(pname)) { @@ -558,6 +565,7 @@ namespace MobileGL::MG_Impl::GLImpl { MakeUnique("MG_Impl/GLImpl", __func__, "params pointer cannot be null.")); return; } + if (!VertexArrayImpl::ValidateVertexAttributeIndex(index)) return; if (!ValidateVertexAttribPname(pname)) return; if (IsCurrentVertexAttribQuery(pname)) { @@ -645,6 +653,7 @@ namespace MobileGL::MG_Impl::GLImpl { MakeUnique("MG_Impl/GLImpl", __func__, "params pointer cannot be null.")); return; } + if (!VertexArrayImpl::ValidateVertexAttributeIndex(index)) return; if (!ValidateVertexAttribPname(pname)) return; if (IsCurrentVertexAttribQuery(pname)) { diff --git a/MobileGL/MG_Impl/GLImpl/VertexArray/Validators.cpp b/MobileGL/MG_Impl/GLImpl/VertexArray/Validators.cpp index 9adf089f..4842b175 100644 --- a/MobileGL/MG_Impl/GLImpl/VertexArray/Validators.cpp +++ b/MobileGL/MG_Impl/GLImpl/VertexArray/Validators.cpp @@ -7,12 +7,22 @@ // End of Source File Header #include "Validators.h" +#include #include #include #include #include namespace MobileGL::MG_Impl::GLImpl::VertexArrayImpl { + Uint GetMaxVertexAttribs() { + constexpr Uint capacity = static_cast(MG_State::GLState::VertexArrayObject::MAX_VERTEX_ATTRIBS); + if (!MG_Backend::pActiveBackendObject) return capacity; + + const Int backendLimit = MG_Backend::pActiveBackendObject->GetDynamicParameters().MaxVertexAttribs; + if (backendLimit <= 0) return capacity; + return std::min(static_cast(backendLimit), capacity); + } + Bool ValidateVertexArrayName(Uint index) { Bool isValid = MG_State::pGLContext->ValidateVertexArrayName(index); if (!isValid) { @@ -37,13 +47,13 @@ namespace MobileGL::MG_Impl::GLImpl::VertexArrayImpl { } Bool ValidateVertexAttributeIndex(Uint index) { - if (index >= MG_State::GLState::VertexArrayObject::MAX_VERTEX_ATTRIBS) { + const Uint maxVertexAttribs = GetMaxVertexAttribs(); + if (index >= maxVertexAttribs) { MG_State::pGLContext->RecordError( ErrorCode::InvalidValue, MakeUnique( "MG_Impl/GLImpl", "ValidateVertexAttributeIndex", - std::format("Attribute index {} exceeds maximum of {}.", index, - MG_State::GLState::VertexArrayObject::MAX_VERTEX_ATTRIBS - 1))); + std::format("Attribute index {} exceeds maximum of {}.", index, maxVertexAttribs - 1))); return false; } return true; diff --git a/MobileGL/MG_Impl/GLImpl/VertexArray/Validators.h b/MobileGL/MG_Impl/GLImpl/VertexArray/Validators.h index 198c70b6..dcf1728c 100644 --- a/MobileGL/MG_Impl/GLImpl/VertexArray/Validators.h +++ b/MobileGL/MG_Impl/GLImpl/VertexArray/Validators.h @@ -11,6 +11,10 @@ #include namespace MobileGL::MG_Impl::GLImpl::VertexArrayImpl { + // The GL-visible GL_MAX_VERTEX_ATTRIBS: min(active backend limit, VertexArrayObject storage + // capacity). Falls back to the capacity when no backend is active (unit tests). + Uint GetMaxVertexAttribs(); + Bool ValidateVertexArrayName(Uint index); Bool ValidateVertexArrayObject(Uint index); Bool ValidateVertexAttributeIndex(Uint index); diff --git a/MobileGL/MG_State/GLState/Core.cpp b/MobileGL/MG_State/GLState/Core.cpp index 30a7851e..39cecbd5 100644 --- a/MobileGL/MG_State/GLState/Core.cpp +++ b/MobileGL/MG_State/GLState/Core.cpp @@ -145,9 +145,32 @@ namespace MobileGL::MG_State { return m_vertexArrayState.GetBoundVertexArray(); } + VertexAttribTypeInfo ClassifyVertexAttribType(GLenum glType) { + switch (glType) { + case GL_FLOAT: return {VertexAttribBaseType::Float, 1}; + case GL_FLOAT_VEC2: return {VertexAttribBaseType::Float, 2}; + case GL_FLOAT_VEC3: return {VertexAttribBaseType::Float, 3}; + case GL_FLOAT_VEC4: return {VertexAttribBaseType::Float, 4}; + case GL_INT: return {VertexAttribBaseType::Int, 1}; + case GL_INT_VEC2: return {VertexAttribBaseType::Int, 2}; + case GL_INT_VEC3: return {VertexAttribBaseType::Int, 3}; + case GL_INT_VEC4: return {VertexAttribBaseType::Int, 4}; + case GL_UNSIGNED_INT: return {VertexAttribBaseType::Uint, 1}; + case GL_UNSIGNED_INT_VEC2: return {VertexAttribBaseType::Uint, 2}; + case GL_UNSIGNED_INT_VEC3: return {VertexAttribBaseType::Uint, 3}; + case GL_UNSIGNED_INT_VEC4: return {VertexAttribBaseType::Uint, 4}; + default: return {}; + } + } + + // The three accessors below are reachable from backend draw paths with a location taken from + // shader reflection, so the bound must be enforced at runtime rather than by MOBILEGL_ASSERT + // (which expands to nothing outside debug builds). void GLContext::SetCurrentVertexAttributeFloat(Uint index, const Array& value) { - MOBILEGL_ASSERT(index < m_currentVertexAttributes.size(), - "SetCurrentVertexAttributeFloat: index %u is out of range", index); + if (index >= m_currentVertexAttributes.size()) { + MGLOG_E("SetCurrentVertexAttributeFloat: index %u is out of range", index); + return; + } auto& current = m_currentVertexAttributes[index]; current.floatValue = value; @@ -158,8 +181,10 @@ namespace MobileGL::MG_State { } void GLContext::SetCurrentVertexAttributeInt(Uint index, const Array& value) { - MOBILEGL_ASSERT(index < m_currentVertexAttributes.size(), - "SetCurrentVertexAttributeInt: index %u is out of range", index); + if (index >= m_currentVertexAttributes.size()) { + MGLOG_E("SetCurrentVertexAttributeInt: index %u is out of range", index); + return; + } auto& current = m_currentVertexAttributes[index]; current.intValue = value; @@ -170,8 +195,10 @@ namespace MobileGL::MG_State { } void GLContext::SetCurrentVertexAttributeUint(Uint index, const Array& value) { - MOBILEGL_ASSERT(index < m_currentVertexAttributes.size(), - "SetCurrentVertexAttributeUint: index %u is out of range", index); + if (index >= m_currentVertexAttributes.size()) { + MGLOG_E("SetCurrentVertexAttributeUint: index %u is out of range", index); + return; + } auto& current = m_currentVertexAttributes[index]; current.uintValue = value; @@ -182,8 +209,11 @@ namespace MobileGL::MG_State { } const CurrentVertexAttributeValue& GLContext::GetCurrentVertexAttribute(Uint index) const { - MOBILEGL_ASSERT(index < m_currentVertexAttributes.size(), - "GetCurrentVertexAttribute: index %u is out of range", index); + static const CurrentVertexAttributeValue defaultValue{}; + if (index >= m_currentVertexAttributes.size()) { + MGLOG_E("GetCurrentVertexAttribute: index %u is out of range", index); + return defaultValue; + } return m_currentVertexAttributes[index]; } diff --git a/MobileGL/MG_State/GLState/Core.h b/MobileGL/MG_State/GLState/Core.h index e6df3ea7..291f2a44 100644 --- a/MobileGL/MG_State/GLState/Core.h +++ b/MobileGL/MG_State/GLState/Core.h @@ -31,6 +31,20 @@ namespace MobileGL { Array uintValue{0u, 0u, 0u, 1u}; }; + // Which of the three views above a shader input of a given GLSL type consumes. + enum class VertexAttribBaseType { Unsupported, Float, Int, Uint }; + + struct VertexAttribTypeInfo { + VertexAttribBaseType baseType = VertexAttribBaseType::Unsupported; + Uint componentCount = 0; + }; + + // Maps a shader vertex-input type (GL_FLOAT_VEC3, GL_INT_VEC2, ...) onto the current-value + // view that feeds it. Shared by every backend so that "a disabled array reads the current + // value" resolves identically regardless of which backend is active; each backend only + // translates the result into its own API call. + VertexAttribTypeInfo ClassifyVertexAttribType(GLenum glType); + class GLContext { public: GLContext() = default; diff --git a/MobileGL/MG_State/GLState/ProgramState/ProgramObject.cpp b/MobileGL/MG_State/GLState/ProgramState/ProgramObject.cpp index 56abafb1..63644c3a 100644 --- a/MobileGL/MG_State/GLState/ProgramState/ProgramObject.cpp +++ b/MobileGL/MG_State/GLState/ProgramState/ProgramObject.cpp @@ -7,6 +7,8 @@ // End of Source File Header #include "ProgramObject.h" +#include +#include #include #include #include @@ -20,6 +22,22 @@ void main() {} )"; namespace { + // How many vertex input locations reflection may record. Backends consume this through + // GetActiveAttributeLocationMask()/GetAttribType(), so a value below the advertised + // GL_MAX_VERTEX_ATTRIBS would make a legal attribute location invisible to them -- DirectGLES would + // then never feed the shader that attribute's current value. Bounded by the state layer's storage + // capacity, which is also the width of the Uint32 masks backends build from it. + static MobileGL::Int GetReflectionVertexAttribLimit() { + constexpr MobileGL::Int capacity = + static_cast(MobileGL::MG_State::GLState::VertexArrayObject::MAX_VERTEX_ATTRIBS); + if (!MobileGL::MG_Backend::pActiveBackendObject) return capacity; + + const MobileGL::Int backendLimit = + MobileGL::MG_Backend::pActiveBackendObject->GetDynamicParameters().MaxVertexAttribs; + if (backendLimit <= 0) return capacity; + return std::min(backendLimit, capacity); + } + static MobileGL::String StripArrayElementSuffix(const MobileGL::String& name) { const MobileGL::SizeT bracket = name.find('['); return bracket == MobileGL::String::npos ? name : name.substr(0, bracket); @@ -460,7 +478,7 @@ namespace MobileGL::MG_State::GLState { maxLoc = std::max(0, inCount - 1); } - GLint maxAttribs = 16; // TODO: get from backend + const GLint maxAttribs = GetReflectionVertexAttribLimit(); MGLOG_D("ProgramObject %u: Reflection - computed maxLoc=%d, using maxAttribs=%d", m_externalIndex, maxLoc, maxAttribs); diff --git a/MobileGL/MG_State/GLState/VertexArrayState/VertexArrayObject.h b/MobileGL/MG_State/GLState/VertexArrayState/VertexArrayObject.h index 9497dd76..a93227a1 100644 --- a/MobileGL/MG_State/GLState/VertexArrayState/VertexArrayObject.h +++ b/MobileGL/MG_State/GLState/VertexArrayState/VertexArrayObject.h @@ -44,8 +44,12 @@ namespace MobileGL { class VertexArrayObject { public: - static constexpr int MAX_VERTEX_ATTRIBS = 16; - static constexpr int MAX_VERTEX_ATTRIB_BINDINGS = 16; + // Storage capacity, not the GL-visible limit. GL_MAX_VERTEX_ATTRIBS is reported as + // min(backend limit, MAX_VERTEX_ATTRIBS) and validated against that dynamic value; + // 32 is the width of the Uint32 attribute masks the backends pass around, so it is + // also the hard ceiling. + static constexpr int MAX_VERTEX_ATTRIBS = 32; + static constexpr int MAX_VERTEX_ATTRIB_BINDINGS = 32; VertexArrayObject(Uint externIndex); @@ -104,14 +108,24 @@ namespace MobileGL { void BumpAttributeSwitchVersion(Uint index); void ResolveAttributeFromBinding(Uint attribIndex); + // 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 + // tail mapped to binding point 0 whenever the limit grows. + static constexpr Array MakeIdentityAttributeBindings() { + Array mapping{}; + for (Uint index = 0; index < static_cast(MAX_VERTEX_ATTRIBS); ++index) { + mapping[index] = index; + } + return mapping; + } + const Uint m_externalIndex = 0; Array m_attributes; Array m_attributeVersions; BindingSlot m_indexBufferBindingSlot; Array m_bindingPoints; - Array m_attributeBindingIndex = {0, 1, 2, 3, 4, 5, 6, 7, - 8, 9, 10, 11, 12, 13, 14, 15}; + Array m_attributeBindingIndex = MakeIdentityAttributeBindings(); Array m_attributeRelativeOffset = {}; // Set once an attribute (or its binding point) is touched through the // ARB_vertex_attrib_binding API; only such attributes are re-resolved, so the diff --git a/MobileGL/MG_Test/SanityTest.cpp b/MobileGL/MG_Test/SanityTest.cpp index 498258f9..01fc4e45 100644 --- a/MobileGL/MG_Test/SanityTest.cpp +++ b/MobileGL/MG_Test/SanityTest.cpp @@ -19,6 +19,7 @@ #include #include #include +#include #include #include #include @@ -408,6 +409,55 @@ void main() { MG_Backend::pActiveBackendObject.reset(); } +// GL_MAX_VERTEX_ATTRIBS must follow the backend but never exceed the state layer's current-value +// storage: the DirectVulkan draw path indexes that array by shader input location, so advertising more +// than it can hold is an out-of-bounds read waiting to happen. +TEST(GetterSanity, ClampsMaxVertexAttribsToCurrentValueStorageCapacity) { + using namespace MobileGL; + constexpr GLint capacity = MG_State::GLState::VertexArrayObject::MAX_VERTEX_ATTRIBS; + + MG_State::pGLContext = MakeUnique(); + + // A driver reporting more attributes than MobileGL can store gets clamped. + { + MG_Backend::DynamicBackendParameters params; + params.MaxVertexAttribs = capacity * 2; + MG_Backend::pActiveBackendObject = MakeUnique(params); + + EXPECT_EQ(MG_Impl::GLImpl::VertexArrayImpl::GetMaxVertexAttribs(), static_cast(capacity)); + GLint reported = 0; + MG_Impl::GLImpl::GetIntegerv(GL_MAX_VERTEX_ATTRIBS, &reported); + EXPECT_EQ(reported, capacity); + MG_Backend::pActiveBackendObject.reset(); + } + + // A driver below the capacity is followed exactly, and validation enforces that same bound. + { + MG_Backend::DynamicBackendParameters params; + params.MaxVertexAttribs = 16; + MG_Backend::pActiveBackendObject = MakeUnique(params); + + EXPECT_EQ(MG_Impl::GLImpl::VertexArrayImpl::GetMaxVertexAttribs(), 16u); + GLint reported = 0; + MG_Impl::GLImpl::GetIntegerv(GL_MAX_VERTEX_ATTRIBS, &reported); + EXPECT_EQ(reported, 16); + + MG_State::pGLContext->ClearErrors(); + EXPECT_FALSE(MG_Impl::GLImpl::VertexArrayImpl::ValidateVertexAttributeIndex(16)); + EXPECT_TRUE(MG_State::pGLContext->HasGLError()); + MG_State::pGLContext->ClearErrors(); + EXPECT_TRUE(MG_Impl::GLImpl::VertexArrayImpl::ValidateVertexAttributeIndex(15)); + EXPECT_FALSE(MG_State::pGLContext->HasGLError()); + + MG_Backend::pActiveBackendObject.reset(); + } + + // With no active backend the storage capacity is the bound, and nothing dereferences a null backend. + EXPECT_EQ(MG_Impl::GLImpl::VertexArrayImpl::GetMaxVertexAttribs(), static_cast(capacity)); + + MG_State::pGLContext.reset(); +} + TEST(GetterSanity, ReportsKhrSubgroupDynamicParameters) { using namespace MobileGL; diff --git a/MobileGL/MG_Test/VertexArray/VertexArrayTest.cpp b/MobileGL/MG_Test/VertexArray/VertexArrayTest.cpp index 8f19fb2f..ea0e82b4 100644 --- a/MobileGL/MG_Test/VertexArray/VertexArrayTest.cpp +++ b/MobileGL/MG_Test/VertexArray/VertexArrayTest.cpp @@ -14,6 +14,7 @@ #include #include #include +#include #include using namespace MobileGL; @@ -225,6 +226,75 @@ TEST_F(VertexArrayTest, BoundVAOPreservesState) { ASSERT_FALSE(vao2->IsAttributeEnabled(0)); } +// The current-value array must cover the full attribute capacity. It used to be sized 16 while the +// DirectVulkan draw path indexed it with shader locations up to 31, reading past the end; the only +// guard was MOBILEGL_ASSERT, which expands to nothing outside debug builds. +TEST_F(VertexArrayTest, CurrentVertexAttributeStorageCoversFullCapacity) { + constexpr Uint capacity = static_cast(MG_State::GLState::VertexArrayObject::MAX_VERTEX_ATTRIBS); + const Uint highIndex = capacity - 1; + + MG_State::pGLContext->SetCurrentVertexAttributeFloat(highIndex, {1.0f, 2.0f, 3.0f, 4.0f}); + const auto& stored = MG_State::pGLContext->GetCurrentVertexAttribute(highIndex); + EXPECT_FLOAT_EQ(stored.floatValue[0], 1.0f); + EXPECT_FLOAT_EQ(stored.floatValue[3], 4.0f); + + // Neighbouring slots keep the GL default of (0, 0, 0, 1). + const auto& untouched = MG_State::pGLContext->GetCurrentVertexAttribute(highIndex - 1); + EXPECT_FLOAT_EQ(untouched.floatValue[0], 0.0f); + EXPECT_FLOAT_EQ(untouched.floatValue[3], 1.0f); + + // Out-of-range access must be bounded at runtime, not just asserted in debug builds. + const auto& outOfRange = MG_State::pGLContext->GetCurrentVertexAttribute(capacity); + EXPECT_FLOAT_EQ(outOfRange.floatValue[0], 0.0f); + EXPECT_FLOAT_EQ(outOfRange.floatValue[3], 1.0f); + + MG_State::pGLContext->SetCurrentVertexAttributeFloat(capacity, {9.0f, 9.0f, 9.0f, 9.0f}); + EXPECT_FLOAT_EQ(MG_State::pGLContext->GetCurrentVertexAttribute(highIndex).floatValue[0], 1.0f); +} + +// A binding point the backend cannot address as an attribute must be rejected: the default mapping +// is the identity, so accepting it would resolve into an attribute index the backend then rejects on +// every draw. +TEST_F(VertexArrayTest, VertexBindingIndexIsBoundedByTheAdvertisedAttribLimit) { + Vector vaoNames; + MG_State::pGLContext->GenVertexArrayNames(1, vaoNames); + MG_State::pGLContext->CreateVertexArrayObject(vaoNames[0]); + MG_State::pGLContext->BindVertexArray(vaoNames[0]); + MG_State::pGLContext->ClearErrors(); + + const GLuint outOfRange = MG_Impl::GLImpl::VertexArrayImpl::GetMaxVertexAttribs(); + MG_Impl::GLImpl::VertexAttribBinding(0, outOfRange); + EXPECT_TRUE(MG_State::pGLContext->HasGLError()); +} + +// The default attribute -> binding-point mapping is the identity. It used to be a 16-element literal +// list, so every attribute at or above 16 silently resolved against binding point 0 instead. +TEST_F(VertexArrayTest, DefaultAttributeBindingIsIdentityAcrossFullCapacity) { + Vector vaoNames; + MG_State::pGLContext->GenVertexArrayNames(1, vaoNames); + auto vao = MG_State::pGLContext->CreateVertexArrayObject(vaoNames[0]); + MG_State::pGLContext->BindVertexArray(vaoNames[0]); + + auto vbo = CreateTestVBO(); + + constexpr Uint kHighAttrib = 20; + static_assert(kHighAttrib >= 16, "must exceed the old 16-entry identity list"); + static_assert(kHighAttrib < MG_State::GLState::VertexArrayObject::MAX_VERTEX_ATTRIBS); + static_assert(kHighAttrib < MG_State::GLState::VertexArrayObject::MAX_VERTEX_ATTRIB_BINDINGS); + + // Binding point kHighAttrib must feed attribute kHighAttrib with no explicit SetAttributeBinding. + vao->SetAttributeFormatSeparate(kHighAttrib, 3, DataType::Float32, false, false, 12); + vao->SetBindingBuffer(kHighAttrib, vbo, 16, 24); + + const auto& attr = vao->GetAttribute(kHighAttrib); + EXPECT_EQ(attr.Buffer, vbo); + EXPECT_EQ(attr.Stride, 24); + EXPECT_EQ(attr.Offset, 28u); // binding offset (16) + attribute relative offset (12) + + // Attribute 0 must not have been dragged along by binding point kHighAttrib. + EXPECT_EQ(vao->GetAttribute(0).Buffer, nullptr); +} + using namespace MobileGL::MG_Impl::GLImpl; class GeneralVertexArrayTest : public ::testing::Test { @@ -674,3 +744,109 @@ TEST_F(GeneralVertexArrayTest, General_ElementBufferBindingPoint) { EXPECT_EQ(GetError(), GL_NO_ERROR); } + +// GL_CURRENT_VERTEX_ATTRIB is per-context state that exists for every index below +// GL_MAX_VERTEX_ATTRIBS, and defaults to (0, 0, 0, 1). +TEST_F(GeneralVertexArrayTest, General_CurrentVertexAttribRoundTripsAtHighestLegalIndex) { + CreateVAO(); + const GLuint highIndex = VertexArrayImpl::GetMaxVertexAttribs() - 1; + ASSERT_GT(highIndex, 0u); + + VertexAttrib4f(highIndex, 1.0f, 2.0f, 3.0f, 4.0f); + EXPECT_EQ(GetError(), GL_NO_ERROR); + + GLfloat values[4] = {-1.0f, -1.0f, -1.0f, -1.0f}; + GetVertexAttribfv(highIndex, GL_CURRENT_VERTEX_ATTRIB, values); + EXPECT_EQ(GetError(), GL_NO_ERROR); + EXPECT_FLOAT_EQ(values[0], 1.0f); + EXPECT_FLOAT_EQ(values[1], 2.0f); + EXPECT_FLOAT_EQ(values[2], 3.0f); + EXPECT_FLOAT_EQ(values[3], 4.0f); + + // Untouched attributes keep the GL default of (0, 0, 0, 1). + GLfloat defaults[4] = {-1.0f, -1.0f, -1.0f, -1.0f}; + GetVertexAttribfv(highIndex - 1, GL_CURRENT_VERTEX_ATTRIB, defaults); + EXPECT_EQ(GetError(), GL_NO_ERROR); + EXPECT_FLOAT_EQ(defaults[0], 0.0f); + EXPECT_FLOAT_EQ(defaults[1], 0.0f); + EXPECT_FLOAT_EQ(defaults[2], 0.0f); + EXPECT_FLOAT_EQ(defaults[3], 1.0f); +} + +// glVertexAttrib{1,2,3}f fill the components the caller omitted with (0, 0, 1). +TEST_F(GeneralVertexArrayTest, General_CurrentVertexAttribFillsOmittedComponents) { + CreateVAO(); + + VertexAttrib1f(1, 7.0f); + GLfloat one[4] = {}; + GetVertexAttribfv(1, GL_CURRENT_VERTEX_ATTRIB, one); + EXPECT_FLOAT_EQ(one[0], 7.0f); + EXPECT_FLOAT_EQ(one[1], 0.0f); + EXPECT_FLOAT_EQ(one[2], 0.0f); + EXPECT_FLOAT_EQ(one[3], 1.0f); + + VertexAttrib2f(2, 7.0f, 8.0f); + GLfloat two[4] = {}; + GetVertexAttribfv(2, GL_CURRENT_VERTEX_ATTRIB, two); + EXPECT_FLOAT_EQ(two[1], 8.0f); + EXPECT_FLOAT_EQ(two[2], 0.0f); + EXPECT_FLOAT_EQ(two[3], 1.0f); + + VertexAttrib3f(3, 7.0f, 8.0f, 9.0f); + GLfloat three[4] = {}; + GetVertexAttribfv(3, GL_CURRENT_VERTEX_ATTRIB, three); + EXPECT_FLOAT_EQ(three[2], 9.0f); + EXPECT_FLOAT_EQ(three[3], 1.0f); + + EXPECT_EQ(GetError(), GL_NO_ERROR); +} + +// The integer current-value views must survive a round trip without going through float. +TEST_F(GeneralVertexArrayTest, General_CurrentVertexAttribIntegerRoundTrip) { + CreateVAO(); + + VertexAttribI4i(1, -5, 6, -7, 8); + GLint signedValues[4] = {}; + GetVertexAttribIiv(1, GL_CURRENT_VERTEX_ATTRIB, signedValues); + EXPECT_EQ(signedValues[0], -5); + EXPECT_EQ(signedValues[1], 6); + EXPECT_EQ(signedValues[2], -7); + EXPECT_EQ(signedValues[3], 8); + + VertexAttribI4ui(2, 10u, 20u, 30u, 40u); + GLuint unsignedValues[4] = {}; + GetVertexAttribIuiv(2, GL_CURRENT_VERTEX_ATTRIB, unsignedValues); + EXPECT_EQ(unsignedValues[0], 10u); + EXPECT_EQ(unsignedValues[3], 40u); + + EXPECT_EQ(GetError(), GL_NO_ERROR); +} + +// The GL_CURRENT_VERTEX_ATTRIB branch returned before any index validation, so an out-of-range +// index silently read past the current-value array instead of raising GL_INVALID_VALUE. +TEST_F(GeneralVertexArrayTest, General_CurrentVertexAttribQueryRejectsOutOfRangeIndex) { + CreateVAO(); + const GLuint outOfRange = VertexArrayImpl::GetMaxVertexAttribs(); + + GLfloat floats[4] = {-1.0f, -2.0f, -3.0f, -4.0f}; + GetVertexAttribfv(outOfRange, GL_CURRENT_VERTEX_ATTRIB, floats); + EXPECT_EQ(GetError(), GL_INVALID_VALUE); + EXPECT_FLOAT_EQ(floats[0], -1.0f); + EXPECT_FLOAT_EQ(floats[3], -4.0f); + + GLint ints[4] = {-1, -2, -3, -4}; + GetVertexAttribiv(outOfRange, GL_CURRENT_VERTEX_ATTRIB, ints); + EXPECT_EQ(GetError(), GL_INVALID_VALUE); + EXPECT_EQ(ints[0], -1); + + GLint signedInts[4] = {-1, -2, -3, -4}; + GetVertexAttribIiv(outOfRange, GL_CURRENT_VERTEX_ATTRIB, signedInts); + EXPECT_EQ(GetError(), GL_INVALID_VALUE); + EXPECT_EQ(signedInts[0], -1); + + GLuint uints[4] = {1u, 2u, 3u, 4u}; + GetVertexAttribIuiv(outOfRange, GL_CURRENT_VERTEX_ATTRIB, uints); + EXPECT_EQ(GetError(), GL_INVALID_VALUE); + EXPECT_EQ(uints[0], 1u); +} + diff --git a/MobileGL/MG_Util/SelfTest/DriverPost.cpp b/MobileGL/MG_Util/SelfTest/DriverPost.cpp index 768eb83e..f42d6995 100644 --- a/MobileGL/MG_Util/SelfTest/DriverPost.cpp +++ b/MobileGL/MG_Util/SelfTest/DriverPost.cpp @@ -12,6 +12,9 @@ #include #include #include +// Only for the compile-time MAX_VERTEX_ATTRIBS constant asserted below. The POST still executes no +// MG_State code: it runs standalone, before MG_State::Init(). +#include #include #include #include @@ -167,6 +170,49 @@ namespace MobileGL::MG_Util::SelfTest { : ""; } + // ---- Vertex attribute limit -------------------------------------------- + // GL 3.3 Core mandates GL_MAX_VERTEX_ATTRIBS >= 16 (spec table 6.32); a driver below + // that cannot back a conformant core context at all. + constexpr Int kGL33MinVertexAttribs = 16; + + // The capacity of the per-context current-vertex-attribute array, which is also the width of + // the Uint32 attribute masks the backends pass around. Pinned to the state layer's constant so + // the two can never drift: a mismatch between them is precisely the defect this row guards. + constexpr Int kMobileGLMaxVertexAttribs = MG_State::GLState::VertexArrayObject::MAX_VERTEX_ATTRIBS; + static_assert(kMobileGLMaxVertexAttribs <= 32, "Vertex attribute masks are Uint32"); + static_assert(kMobileGLMaxVertexAttribs >= kGL33MinVertexAttribs, + "MobileGL cannot advertise a conformant GL 3.3 Core GL_MAX_VERTEX_ATTRIBS"); + + // Both backends index a fixed-size, per-context array of current generic vertex attribute + // values by shader input location, and both clamp the GL_MAX_VERTEX_ATTRIBS they advertise + // to that array's capacity. A driver reporting more attributes than the array can hold used + // to make the DirectVulkan draw path walk locations past the end of it -- an out-of-bounds + // read in release builds, and a MOBILEGL_ASSERT abort in debug builds -- as soon as a shader + // declared a vertex input at a high location whose array was disabled. The clamp closes that + // hole, so this row exists to make the underlying driver/host mismatch visible rather than + // silently swallowed. + void EvaluateVertexAttribLimit(ReportBuilder& builder, Int deviceLimit, const char* rowName, + const char* driverLimitName) { + if (deviceLimit < kGL33MinVertexAttribs) { + builder.Fail(rowName, + format("{} = {} (< {}); OpenGL 3.3 Core requires at least {} generic vertex " + "attributes, so this driver cannot back a conformant core context", + driverLimitName, deviceLimit, kGL33MinVertexAttribs, kGL33MinVertexAttribs)); + return; + } + if (deviceLimit > kMobileGLMaxVertexAttribs) { + builder.Warn(rowName, + format("{} = {} (> {}); MobileGL clamps GL_MAX_VERTEX_ATTRIBS to {} because its " + "current-vertex-attribute storage and its Uint32 attribute masks hold {} " + "locations, so the driver's extra attributes stay unusable", + driverLimitName, deviceLimit, kMobileGLMaxVertexAttribs, + kMobileGLMaxVertexAttribs, kMobileGLMaxVertexAttribs)); + return; + } + builder.Pass(rowName, format("{} = {}; MobileGL advertises GL_MAX_VERTEX_ATTRIBS = {}", + driverLimitName, deviceLimit, deviceLimit)); + } + void EvaluateGlesChecklist(ReportBuilder& builder, const MG_External::GLESCapabilities& caps, const MG_External::GLESFunctionsTable& glesFuncs) { const Int major = caps.GLESVersion.Major; @@ -185,6 +231,9 @@ namespace MobileGL::MG_Util::SelfTest { versionLabel + " (< 3.1: no compute shaders or native indirect draws)"); } + EvaluateVertexAttribLimit(builder, caps.MaxVertexAttribs, "Vertex attributes", + "GL_MAX_VERTEX_ATTRIBS"); + if (es31) { GLint maxVertexSsboBlocks = 0; glesFuncs.glGetIntegerv(GL_MAX_VERTEX_SHADER_STORAGE_BLOCKS, &maxVertexSsboBlocks); @@ -1006,6 +1055,9 @@ namespace MobileGL::MG_Util::SelfTest { VkApiVersionToString(properties.apiVersion))); } + EvaluateVertexAttribLimit(builder, static_cast(properties.limits.maxVertexInputAttributes), + "Vertex attributes", "maxVertexInputAttributes"); + Vector deviceExtensions; Uint32 deviceExtensionCount = 0; if (vkEnumerateDeviceExtensionPropertiesFn(physicalDevice, nullptr, &deviceExtensionCount, nullptr) ==