From cff959b2e8b3e8c7b2a5543717f192dcdbaf7ff7 Mon Sep 17 00:00:00 2001 From: BZLZHH Date: Tue, 4 Aug 2026 19:25:32 -0400 Subject: [PATCH] [Feat] (DirectVulkan, MG_Util): honour a glVertexAttribDivisor other than 1 Vulkan's VK_VERTEX_INPUT_RATE_INSTANCE advances an attribute once per instance and has no way to say anything else, so every non-zero divisor collapsed to 1: an attribute the application asked to change every three instances changed every one, and KHR-GL40.draw_indirect.basic-drawArrays-instancing and its elements sibling drew the wrong colours from instance one onward. VK_EXT_vertex_attribute_divisor is exactly this state, so it is enabled when the device has it and the per-binding divisors ride into the pipeline through VkPipelineVertexInputDivisorStateCreateInfoEXT. Only divisors other than 1 are listed - 1 is what the plain input rate already means - and they join the layout hash, so two layouts that differ only in a divisor no longer share a pipeline. POST reports the feature either way, because without it the failure is silent and looks like a shader bug: the attribute is fetched, just from the wrong instance. The GLES side gains the two checks this session's other work made load-bearing for the same reason - glPatchParameteri (without it GL_PATCH_VERTICES stays at the driver's 3 and a patch draw of any other size renders nothing) and the transform feedback object entry points (without them a second object cannot open a capture while the first is paused). KHR-GL40.draw_indirect on Magma: 70/70 but for the arbitrary primitive-restart index, which Vulkan cannot express at all. --- .../Renderer/VertexInputStateFactory.cpp | 18 +++++++++ .../Renderer/VertexInputStateFactory.h | 8 ++++ .../DirectVulkan/Renderer/VulkanRenderer.cpp | 30 ++++++++++++++ .../DirectVulkan/Renderer/VulkanRenderer.h | 3 ++ MobileGL/MG_Util/SelfTest/DriverPost.cpp | 40 +++++++++++++++++++ 5 files changed, 99 insertions(+) diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/VertexInputStateFactory.cpp b/MobileGL/MG_Backend/DirectVulkan/Renderer/VertexInputStateFactory.cpp index d5b54cb8..b2a9ba35 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VertexInputStateFactory.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/VertexInputStateFactory.cpp @@ -87,6 +87,7 @@ namespace MobileGL::MG_Backend::DirectVulkan { Vector bindingAttributeLocations; Vector bindingUsesClientMemory; Vector bindingConversions; + Vector bindingDivisors; Uint32 unsupportedAttribMask = 0; for (Uint32 location = 0; location < MG_State::GLState::VertexArrayObject::MAX_VERTEX_ATTRIBS; ++location) { @@ -180,6 +181,11 @@ namespace MobileGL::MG_Backend::DirectVulkan { bindingConversions.push_back(conversion); builder.AddBinding(binding, stride, inputRate); builder.AddAttribute(location, binding, vkFormat, 0); + // Divisor 1 is what VK_VERTEX_INPUT_RATE_INSTANCE already means; only anything + // else needs the extension to say it. + if (inputRate == VK_VERTEX_INPUT_RATE_INSTANCE && attr.Divisor != 1) { + bindingDivisors.push_back({binding, static_cast(attr.Divisor)}); + } } const auto& state = builder.Build(); @@ -191,6 +197,7 @@ namespace MobileGL::MG_Backend::DirectVulkan { BackendVertexInputState& entry = *slot; entry.hash = hash; entry.lastUsedFrameBoundary = m_frameBoundaryCounter; + entry.bindingDivisors = Move(bindingDivisors); entry.bindings = builder.GetBindings(); entry.attributes = builder.GetAttributes(); // See the layoutHash declaration: hash only the resolved layout, never @@ -207,6 +214,10 @@ namespace MobileGL::MG_Backend::DirectVulkan { XXHASH_VERIFY(XXH64_update(m_hashState, &attribute.format, sizeof(attribute.format))); XXHASH_VERIFY(XXH64_update(m_hashState, &attribute.offset, sizeof(attribute.offset))); } + for (const auto& divisor : entry.bindingDivisors) { + XXHASH_VERIFY(XXH64_update(m_hashState, &divisor.binding, sizeof(divisor.binding))); + XXHASH_VERIFY(XXH64_update(m_hashState, &divisor.divisor, sizeof(divisor.divisor))); + } XXHASH_VERIFY(XXH64_update(m_hashState, &unsupportedAttribMask, sizeof(unsupportedAttribMask))); entry.layoutHash = XXH64_digest(m_hashState); entry.attributeLocationMask = 0; @@ -224,6 +235,13 @@ namespace MobileGL::MG_Backend::DirectVulkan { entry.state = state; entry.state.pVertexBindingDescriptions = entry.bindings.empty() ? nullptr : entry.bindings.data(); entry.state.pVertexAttributeDescriptions = entry.attributes.empty() ? nullptr : entry.attributes.data(); + if (!entry.bindingDivisors.empty()) { + entry.divisorState.vertexBindingDivisorCount = static_cast(entry.bindingDivisors.size()); + entry.divisorState.pVertexBindingDivisors = entry.bindingDivisors.data(); + entry.state.pNext = &entry.divisorState; + } else { + entry.state.pNext = nullptr; + } return entry; } diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/VertexInputStateFactory.h b/MobileGL/MG_Backend/DirectVulkan/Renderer/VertexInputStateFactory.h index 54fe9333..8714019b 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VertexInputStateFactory.h +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/VertexInputStateFactory.h @@ -53,6 +53,14 @@ namespace MobileGL::MG_Backend::DirectVulkan { // Bitmask of `attributes[i].location` - the draw path needs it up to // three times per draw, so it is baked once at build time. Uint32 attributeLocationMask = 0; + // Per-binding glVertexAttribDivisor values other than 1. Vulkan's instance input + // rate advances once per instance and nothing else, so anything else has to be + // stated through VK_EXT_vertex_attribute_divisor. Empty when every instanced + // binding uses divisor 1, which is what the plain input rate already means. + Vector bindingDivisors; + VkPipelineVertexInputDivisorStateCreateInfoEXT divisorState{ + VK_STRUCTURE_TYPE_PIPELINE_VERTEX_INPUT_DIVISOR_STATE_CREATE_INFO_EXT + }; 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 8dea0e78..8d406426 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp @@ -9894,6 +9894,36 @@ void main() { MGLOG_W("VK_EXT_transform_feedback is unavailable; transform feedback capture will not work"); } + // VK_EXT_vertex_attribute_divisor. Vulkan's instance input rate advances an attribute + // once per instance and nothing else, so without this every glVertexAttribDivisor value + // collapses to 1 and an attribute meant to change every N instances changes every one. + m_vertexAttributeDivisorEnabled = false; + VkPhysicalDeviceVertexAttributeDivisorFeaturesEXT vertexAttributeDivisorFeatures{}; + vertexAttributeDivisorFeatures.sType = + VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_VERTEX_ATTRIBUTE_DIVISOR_FEATURES_EXT; + if (IsExtensionSupported(availableExtensions, VK_EXT_VERTEX_ATTRIBUTE_DIVISOR_EXTENSION_NAME) && + getPhysicalDeviceFeatures2 != nullptr) { + VkPhysicalDeviceFeatures2 featureQuery{}; + featureQuery.sType = VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_FEATURES_2; + featureQuery.pNext = &vertexAttributeDivisorFeatures; + getPhysicalDeviceFeatures2(m_physicalDevice.handle, &featureQuery); + if (vertexAttributeDivisorFeatures.vertexAttributeInstanceRateDivisor == VK_TRUE) { + if (!IsExtensionAlreadyEnabled(enabledDeviceExtensions, + VK_EXT_VERTEX_ATTRIBUTE_DIVISOR_EXTENSION_NAME)) { + enabledDeviceExtensions.push_back(VK_EXT_VERTEX_ATTRIBUTE_DIVISOR_EXTENSION_NAME); + } + vertexAttributeDivisorFeatures.vertexAttributeInstanceRateZeroDivisor = VK_FALSE; + vertexAttributeDivisorFeatures.pNext = const_cast(deviceCreateInfo.pNext); + deviceCreateInfo.pNext = &vertexAttributeDivisorFeatures; + m_vertexAttributeDivisorEnabled = true; + MGLOG_I("Enabled optional device extension: %s", VK_EXT_VERTEX_ATTRIBUTE_DIVISOR_EXTENSION_NAME); + } + } + if (!m_vertexAttributeDivisorEnabled) { + MGLOG_W("VK_EXT_vertex_attribute_divisor is unavailable; a glVertexAttribDivisor other " + "than 1 will advance its attribute once per instance"); + } + // Host query reset lets the occlusion-query ring recycle slots without a // command-buffer round trip. m_hostQueryResetEnabled = false; diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.h b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.h index a6a80df3..307a9cae 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.h +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.h @@ -494,6 +494,9 @@ namespace MobileGL::MG_Backend::DirectVulkan { // VK_EXT_transform_feedback (GL transform feedback capture) Bool m_transformFeedbackFeatureEnabled = false; + // VK_EXT_vertex_attribute_divisor: without it every non-zero glVertexAttribDivisor + // behaves as 1, because that is all Vulkan's instance input rate can express. + Bool m_vertexAttributeDivisorEnabled = false; static inline PFN_vkCmdBindTransformFeedbackBuffersEXT s_vkCmdBindTransformFeedbackBuffersEXT = nullptr; static inline PFN_vkCmdBeginTransformFeedbackEXT s_vkCmdBeginTransformFeedbackEXT = nullptr; static inline PFN_vkCmdEndTransformFeedbackEXT s_vkCmdEndTransformFeedbackEXT = nullptr; diff --git a/MobileGL/MG_Util/SelfTest/DriverPost.cpp b/MobileGL/MG_Util/SelfTest/DriverPost.cpp index 24de3c41..f9871f57 100644 --- a/MobileGL/MG_Util/SelfTest/DriverPost.cpp +++ b/MobileGL/MG_Util/SelfTest/DriverPost.cpp @@ -298,6 +298,25 @@ namespace MobileGL::MG_Util::SelfTest { "not supported; no impact: the native indirect path deliberately does not " "rely on it (shader-side emulation handles baseInstance semantics)"); } + if (glesFuncs.glPatchParameteri != nullptr) { + builder.Pass("Tessellation patch parameters", + "glPatchParameteri present (GL_PATCH_VERTICES reaches the driver)"); + } else { + builder.Warn("Tessellation patch parameters", + "glPatchParameteri missing (pre-ES 3.2 without GL_EXT_tessellation_shader); " + "GL_PATCH_VERTICES stays at the driver default of 3 and a patch draw of any " + "other size renders nothing"); + } + if (glesFuncs.glGenTransformFeedbacks != nullptr && glesFuncs.glBindTransformFeedback != nullptr && + glesFuncs.glPauseTransformFeedback != nullptr && glesFuncs.glResumeTransformFeedback != nullptr) { + builder.Pass("Transform feedback objects", + "supported (each GL transform feedback object gets one of the driver's, so " + "several can hold a paused capture at once)"); + } else { + builder.Warn("Transform feedback objects", + "entry points missing; every GL transform feedback object shares the driver's " + "default one, so a second object cannot open a capture while the first is paused"); + } if (caps.SupportsNorm16Texture) { builder.Pass("GL_EXT_texture_norm16", "supported"); } else { @@ -1412,6 +1431,27 @@ namespace MobileGL::MG_Util::SelfTest { "hard-fails at draw"); } + Bool vertexAttributeInstanceRateDivisor = false; + if (vkGetPhysicalDeviceFeatures2Fn != nullptr && + HasVkExtension(deviceExtensions, VK_EXT_VERTEX_ATTRIBUTE_DIVISOR_EXTENSION_NAME)) { + VkPhysicalDeviceVertexAttributeDivisorFeaturesEXT divisorFeatures{}; + divisorFeatures.sType = VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_VERTEX_ATTRIBUTE_DIVISOR_FEATURES_EXT; + VkPhysicalDeviceFeatures2 features2{}; + features2.sType = VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_FEATURES_2; + features2.pNext = &divisorFeatures; + vkGetPhysicalDeviceFeatures2Fn(physicalDevice, &features2); + vertexAttributeInstanceRateDivisor = divisorFeatures.vertexAttributeInstanceRateDivisor == VK_TRUE; + } + if (vertexAttributeInstanceRateDivisor) { + builder.Pass("vertexAttributeInstanceRateDivisor", + "supported (glVertexAttribDivisor advances an attribute every N instances)"); + } else { + builder.Warn("vertexAttributeInstanceRateDivisor", + "unsupported; Vulkan's instance input rate can only advance once per instance, so " + "every non-zero glVertexAttribDivisor behaves as 1 and instanced attributes meant to " + "change every N instances change every one"); + } + if (vkGetPhysicalDeviceProperties2Fn != nullptr && properties.apiVersion >= VK_API_VERSION_1_1) { VkPhysicalDeviceSubgroupProperties subgroupProperties{}; subgroupProperties.sType = VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_SUBGROUP_PROPERTIES;