From 63f0756644f635aae840b7db4d52235888ec621d Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Sun, 26 Jul 2026 12:25:24 -0400 Subject: [PATCH] [Fix] (DirectVulkan): support UBO instance arrays as arrayed descriptors - uniform Block{...}b[N] reflected as one binding with descriptorCount=N, per-element GL block mapping, per-element buffer infos and dynamic offsets; non-UBO descriptor arrays now fail program creation cleanly instead of continuing corrupt --- .../DirectVulkan/Renderer/ProgramFactory.cpp | 116 +++++++++++++++--- .../DirectVulkan/Renderer/ProgramFactory.h | 10 ++ .../DirectVulkan/Renderer/UniformManager.cpp | 93 +++++++++----- .../DirectVulkan/Renderer/UniformManager.h | 2 +- 4 files changed, 173 insertions(+), 48 deletions(-) diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/ProgramFactory.cpp b/MobileGL/MG_Backend/DirectVulkan/Renderer/ProgramFactory.cpp index d6967c74..fb758973 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/ProgramFactory.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/ProgramFactory.cpp @@ -1094,9 +1094,17 @@ namespace MobileGL::MG_Backend::DirectVulkan { for (auto* binding : bindings) { MOBILEGL_ASSERT(binding != nullptr, "ProgramFactory: null descriptor binding reflection record"); const auto kind = ReflectDescriptorTypeToBindingKind(binding->descriptor_type); - MOBILEGL_ASSERT(binding->count == 1, - "ProgramFactory: descriptor arrays are unsupported (name='%s' count=%u)", - binding->name ? binding->name : "", binding->count); + // UBO instance arrays (uniform Block {...} b[N];) occupy one binding with + // descriptorCount = N; other descriptor arrays stay unsupported and must + // fail program creation cleanly rather than continue with corrupt state. + if (binding->count != 1 && kind != ProgramFactory::DescriptorBindingKind::UniformBufferDynamic) { + MGLOG_E("ProgramFactory: descriptor arrays are unsupported for this descriptor " + "kind (name='%s' count=%u type=%d)", + binding->name ? binding->name : "", binding->count, + static_cast(binding->descriptor_type)); + destroyReflectModules(); + return false; + } DescriptorKey key{}; key.kind = kind; @@ -1613,6 +1621,8 @@ namespace MobileGL::MG_Backend::DirectVulkan { entry.storageBlockIndexByBinding.assign(m_maxBindings, -1); entry.globalUboBinding = -1; entry.dynamicBindings.clear(); + entry.bindingDescriptorCounts.assign(m_maxBindings, 1); + entry.arrayedUniformBlockIndicesByBinding.clear(); // Use SpvcSession (Reflection mode) to reflect all SPIR-V modules in a single pass per module for (const auto& module : spirv) { @@ -1628,6 +1638,27 @@ namespace MobileGL::MG_Backend::DirectVulkan { "ProgramFactory::ReflectLayout: failed to create reflection module (result=%d)", static_cast(createReflectResult)); + // Descriptor counts per binding (UBO instance arrays reflect count > 1). + UnorderedMap descriptorCountByBinding; + { + uint32_t countProbe = 0; + if (spvReflectEnumerateDescriptorBindings(&reflectModule, &countProbe, nullptr) == + SPV_REFLECT_RESULT_SUCCESS && + countProbe > 0) { + Vector probeBindings(countProbe); + if (spvReflectEnumerateDescriptorBindings(&reflectModule, &countProbe, + probeBindings.data()) == + SPV_REFLECT_RESULT_SUCCESS) { + for (const auto* probeBinding : probeBindings) { + if (probeBinding != nullptr) { + descriptorCountByBinding[probeBinding->binding] = + std::max(1, probeBinding->count); + } + } + } + } + } + // Reflect uniform buffers auto ubos = session.GetShaderInterface(SPVC_RESOURCE_TYPE_UNIFORM_BUFFER); for (const auto& ubo : ubos) { @@ -1653,9 +1684,69 @@ namespace MobileGL::MG_Backend::DirectVulkan { continue; } - const Uint blockIndex = program.GetUniformBlockIndex(ubo.name.c_str()); - if (blockIndex == 0xFFFFFFFFu) { - MGLOG_D("ProgramFactory::ReflectLayout: skipping inactive UBO '%s' at binding %u", + const auto countIt = descriptorCountByBinding.find(binding); + const Uint32 descriptorCount = + countIt != descriptorCountByBinding.end() ? countIt->second : 1u; + + if (descriptorCount <= 1) { + const Uint blockIndex = program.GetUniformBlockIndex(ubo.name.c_str()); + if (blockIndex == 0xFFFFFFFFu) { + MGLOG_D("ProgramFactory::ReflectLayout: skipping inactive UBO '%s' at binding %u", + ubo.name.c_str(), binding); + continue; + } + + MOBILEGL_ASSERT(entry.bindingKinds[binding] == DescriptorBindingKind::None || + entry.bindingKinds[binding] == DescriptorBindingKind::UniformBufferDynamic, + "ProgramFactory::ReflectLayout: descriptor binding %u has conflicting kinds for UBO '%s'", + binding, ubo.name.c_str()); + entry.bindingKinds[binding] = DescriptorBindingKind::UniformBufferDynamic; + MOBILEGL_ASSERT(entry.globalUboBinding != static_cast(binding), + "ProgramFactory::ReflectLayout: regular UBO '%s' collides with global UBO binding %u", + ubo.name.c_str(), binding); + MOBILEGL_ASSERT(entry.uniformBlockIndexByBinding[binding] < 0 || + entry.uniformBlockIndexByBinding[binding] == static_cast(blockIndex), + "ProgramFactory::ReflectLayout: descriptor binding %u maps to conflicting UBO blocks (%d vs %u)", + binding, entry.uniformBlockIndexByBinding[binding], blockIndex); + entry.uniformBlockIndexByBinding[binding] = static_cast(blockIndex); + continue; + } + + // UBO instance array: one binding, descriptorCount elements. GL exposes each + // element as its own active block named "Name[i]"; map every element to its + // GL block index so the descriptor write can gather per-element buffer ranges. + if (descriptorCount > m_maxBindings) { + MGLOG_E("ProgramFactory::ReflectLayout: UBO array '%s' count %u exceeds maxBindings=%u; " + "leaving binding %u unmapped", + ubo.name.c_str(), descriptorCount, m_maxBindings, binding); + continue; + } + Vector elementBlockIndices; + elementBlockIndices.reserve(descriptorCount); + for (Uint32 element = 0; element < descriptorCount; ++element) { + String elementName = ubo.name + "[" + std::to_string(element) + "]"; + Uint elementBlockIndex = program.GetUniformBlockIndex(elementName.c_str()); + if (elementBlockIndex == 0xFFFFFFFFu && element == 0) { + // Some frontends report the first element under the bare block name. + elementBlockIndex = program.GetUniformBlockIndex(ubo.name.c_str()); + } + if (elementBlockIndex == 0xFFFFFFFFu) { + // Degrade rather than corrupt: reuse element 0's block if we have one, + // otherwise give up on the binding (same observable behavior as an + // inactive block: wrong values, but no crash). + MGLOG_E("ProgramFactory::ReflectLayout: UBO array '%s' element %u has no active " + "GL uniform block", + ubo.name.c_str(), element); + if (!elementBlockIndices.empty()) { + elementBlockIndex = static_cast(elementBlockIndices.front()); + } else { + break; + } + } + elementBlockIndices.push_back(static_cast(elementBlockIndex)); + } + if (elementBlockIndices.size() != descriptorCount) { + MGLOG_E("ProgramFactory::ReflectLayout: skipping unresolved UBO array '%s' at binding %u", ubo.name.c_str(), binding); continue; } @@ -1665,14 +1756,9 @@ namespace MobileGL::MG_Backend::DirectVulkan { "ProgramFactory::ReflectLayout: descriptor binding %u has conflicting kinds for UBO '%s'", binding, ubo.name.c_str()); entry.bindingKinds[binding] = DescriptorBindingKind::UniformBufferDynamic; - MOBILEGL_ASSERT(entry.globalUboBinding != static_cast(binding), - "ProgramFactory::ReflectLayout: regular UBO '%s' collides with global UBO binding %u", - ubo.name.c_str(), binding); - MOBILEGL_ASSERT(entry.uniformBlockIndexByBinding[binding] < 0 || - entry.uniformBlockIndexByBinding[binding] == static_cast(blockIndex), - "ProgramFactory::ReflectLayout: descriptor binding %u maps to conflicting UBO blocks (%d vs %u)", - binding, entry.uniformBlockIndexByBinding[binding], blockIndex); - entry.uniformBlockIndexByBinding[binding] = static_cast(blockIndex); + entry.bindingDescriptorCounts[binding] = static_cast(descriptorCount); + entry.uniformBlockIndexByBinding[binding] = elementBlockIndices[0]; + entry.arrayedUniformBlockIndicesByBinding[binding] = Move(elementBlockIndices); } // Reflect sampled images, storage images, samplerBuffer uniforms, and SSBOs. @@ -1819,7 +1905,7 @@ namespace MobileGL::MG_Backend::DirectVulkan { VkDescriptorSetLayoutBinding layoutBinding{}; layoutBinding.binding = binding; - layoutBinding.descriptorCount = 1; + layoutBinding.descriptorCount = entry.bindingDescriptorCounts[binding]; layoutBinding.stageFlags = VK_SHADER_STAGE_ALL; layoutBinding.pImmutableSamplers = nullptr; if (kind == DescriptorBindingKind::UniformBufferDynamic) { diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/ProgramFactory.h b/MobileGL/MG_Backend/DirectVulkan/Renderer/ProgramFactory.h index fa8f5cca..4d3d49b1 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/ProgramFactory.h +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/ProgramFactory.h @@ -59,6 +59,12 @@ namespace MobileGL::MG_Backend::DirectVulkan { Vector bindingKinds; Vector dynamicBindings; Vector uniformBlockIndexByBinding; + // Descriptor count per binding (1 except for UBO instance arrays, which occupy one + // binding with descriptorCount = N). + Vector bindingDescriptorCounts; + // Per-element GL uniform block indices for arrayed UBO bindings (count > 1); + // element 0 of a non-arrayed binding stays in uniformBlockIndexByBinding. + UnorderedMap> arrayedUniformBlockIndicesByBinding; Vector samplerNameByBinding; Vector samplerUniformLocationByBinding; Vector samplerTextureTargetByBinding; @@ -97,6 +103,8 @@ namespace MobileGL::MG_Backend::DirectVulkan { bindingKinds = std::move(other.bindingKinds); dynamicBindings = std::move(other.dynamicBindings); uniformBlockIndexByBinding = std::move(other.uniformBlockIndexByBinding); + bindingDescriptorCounts = std::move(other.bindingDescriptorCounts); + arrayedUniformBlockIndicesByBinding = std::move(other.arrayedUniformBlockIndicesByBinding); samplerNameByBinding = std::move(other.samplerNameByBinding); samplerUniformLocationByBinding = std::move(other.samplerUniformLocationByBinding); samplerTextureTargetByBinding = std::move(other.samplerTextureTargetByBinding); @@ -141,6 +149,8 @@ namespace MobileGL::MG_Backend::DirectVulkan { bindingKinds = std::move(other.bindingKinds); dynamicBindings = std::move(other.dynamicBindings); uniformBlockIndexByBinding = std::move(other.uniformBlockIndexByBinding); + bindingDescriptorCounts = std::move(other.bindingDescriptorCounts); + arrayedUniformBlockIndicesByBinding = std::move(other.arrayedUniformBlockIndicesByBinding); samplerNameByBinding = std::move(other.samplerNameByBinding); samplerUniformLocationByBinding = std::move(other.samplerUniformLocationByBinding); samplerTextureTargetByBinding = std::move(other.samplerTextureTargetByBinding); diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/UniformManager.cpp b/MobileGL/MG_Backend/DirectVulkan/Renderer/UniformManager.cpp index f54aa155..50838e99 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/UniformManager.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/UniformManager.cpp @@ -765,7 +765,7 @@ namespace MobileGL::MG_Backend::DirectVulkan { Bool UniformManager::ResolveUniformBufferPayload(const MG_State::GLState::ProgramObject& program, const ProgramFactory::VkProgramObject& programObj, Uint32 binding, - UboBindResult& out) const { + Uint32 arrayElement, UboBindResult& out) const { const void* outData = nullptr; VkDeviceSize outSize = 0; @@ -791,7 +791,19 @@ namespace MobileGL::MG_Backend::DirectVulkan { MOBILEGL_ASSERT(binding < programObj.uniformBlockIndexByBinding.size(), "ResolveUniformBufferPayload: UBO mapping binding %u out of range", binding); - const Int blockIndex = programObj.uniformBlockIndexByBinding[binding]; + Int blockIndex = programObj.uniformBlockIndexByBinding[binding]; + if (arrayElement > 0) { + const auto arrayIt = programObj.arrayedUniformBlockIndicesByBinding.find(binding); + const Bool elementValid = arrayIt != programObj.arrayedUniformBlockIndicesByBinding.end() && + arrayElement < arrayIt->second.size(); + MOBILEGL_ASSERT(elementValid, + "ResolveUniformBufferPayload: UBO binding %u has no array element %u", binding, + arrayElement); + if (!elementValid) { + return false; + } + blockIndex = arrayIt->second[arrayElement]; + } MOBILEGL_ASSERT(blockIndex >= 0, "ResolveUniformBufferPayload: no uniform block mapped to descriptor binding %u", binding); @@ -1038,11 +1050,17 @@ namespace MobileGL::MG_Backend::DirectVulkan { imageInfos.clear(); texelBufferViews.clear(); dynamicOffsets.clear(); + // Arrayed UBO bindings contribute extra buffer infos and dynamic offsets; reserve for + // the worst case so the pBufferInfo pointers taken below never dangle on reallocation. + Uint32 uboArrayExtra = 0; + for (const auto& arrayEntry : programObj.arrayedUniformBlockIndicesByBinding) { + uboArrayExtra += static_cast(arrayEntry.second.size()) - 1u; + } writes.reserve(m_maxBindings); - bufferInfos.reserve(m_maxBindings); + bufferInfos.reserve(m_maxBindings + uboArrayExtra); imageInfos.reserve(m_maxBindings); texelBufferViews.reserve(m_maxBindings); - dynamicOffsets.reserve(programObj.dynamicBindings.size()); + dynamicOffsets.reserve(programObj.dynamicBindings.size() + uboArrayExtra); const Uint32 bindingCount = std::min(m_maxBindings, static_cast(programObj.bindingKinds.size())); @@ -1060,40 +1078,51 @@ namespace MobileGL::MG_Backend::DirectVulkan { write.descriptorCount = 1; if (kind == ProgramFactory::DescriptorBindingKind::UniformBufferDynamic) { - UboBindResult ubo{}; - const Bool hasPayload = ResolveUniformBufferPayload(program, programObj, binding, ubo); - MOBILEGL_ASSERT(hasPayload && ubo.payload != nullptr && ubo.payloadSize > 0, - "UniformDescriptorBinder::BindProgramUniformBuffers failed: missing UBO payload on binding %u", - binding); + const Uint32 descriptorCount = + binding < programObj.bindingDescriptorCounts.size() + ? std::max(1, programObj.bindingDescriptorCounts[binding]) + : 1u; + const SizeT firstBufferInfoIndex = bufferInfos.size(); + for (Uint32 element = 0; element < descriptorCount; ++element) { + UboBindResult ubo{}; + const Bool hasPayload = + ResolveUniformBufferPayload(program, programObj, binding, element, ubo); + MOBILEGL_ASSERT(hasPayload && ubo.payload != nullptr && ubo.payloadSize > 0, + "UniformDescriptorBinder::BindProgramUniformBuffers failed: missing UBO payload on binding %u element %u", + binding, element); - VkDescriptorBufferInfo bufferInfo{}; - // Keep offset 0 (sub-range selected via the dynamic offset) so the hashed bufferInfo - // is stable across draws and the descriptor-set reuse cache keeps hitting. - bufferInfo.offset = 0; - Uint32 dynOffset; - if (ubo.directBindable) { - // Zero-copy: bind the app's resident VkBuffer directly, no per-draw memcpy. - bufferInfo.buffer = ubo.buffer; - bufferInfo.range = ubo.range; - dynOffset = static_cast(ubo.dynamicOffset); - } else { - BufferSlice slice{}; - if (!m_bufferManager->UploadTransient(BufferKind::Uniform, frameIndex, ubo.payload, - ubo.payloadSize, m_minDynamicOffsetAlignment, slice)) { - MOBILEGL_ASSERT(false, "UniformDescriptorBinder::BindProgramUniformBuffers failed: UBO upload failed on binding %u", - binding); - return false; + VkDescriptorBufferInfo bufferInfo{}; + // Keep offset 0 (sub-range selected via the dynamic offset) so the hashed bufferInfo + // is stable across draws and the descriptor-set reuse cache keeps hitting. + bufferInfo.offset = 0; + Uint32 dynOffset; + if (ubo.directBindable) { + // Zero-copy: bind the app's resident VkBuffer directly, no per-draw memcpy. + bufferInfo.buffer = ubo.buffer; + bufferInfo.range = ubo.range; + dynOffset = static_cast(ubo.dynamicOffset); + } else { + BufferSlice slice{}; + if (!m_bufferManager->UploadTransient(BufferKind::Uniform, frameIndex, ubo.payload, + ubo.payloadSize, m_minDynamicOffsetAlignment, slice)) { + MOBILEGL_ASSERT(false, "UniformDescriptorBinder::BindProgramUniformBuffers failed: UBO upload failed on binding %u element %u", + binding, element); + return false; + } + bufferInfo.buffer = slice.buffer; + bufferInfo.range = ubo.payloadSize; + dynOffset = static_cast(slice.offset); } - bufferInfo.buffer = slice.buffer; - bufferInfo.range = ubo.payloadSize; - dynOffset = static_cast(slice.offset); + bufferInfos.push_back(bufferInfo); + // Dynamic offsets are consumed in binding order, then array element order, + // matching Vulkan's dynamic-offset consumption rules. + dynamicOffsets.push_back(dynOffset); } - bufferInfos.push_back(bufferInfo); write.descriptorType = VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER_DYNAMIC; - write.pBufferInfo = &bufferInfos.back(); + write.descriptorCount = descriptorCount; + write.pBufferInfo = &bufferInfos[firstBufferInfoIndex]; writes.push_back(write); - dynamicOffsets.push_back(dynOffset); } else if (kind == ProgramFactory::DescriptorBindingKind::UniformTexelBuffer) { VkBufferView bufferView = VK_NULL_HANDLE; if (!ResolveTexelBufferDescriptor(program, programObj, binding, frameIndex, bufferView) || diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/UniformManager.h b/MobileGL/MG_Backend/DirectVulkan/Renderer/UniformManager.h index d7afe8c2..6f495f00 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/UniformManager.h +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/UniformManager.h @@ -116,7 +116,7 @@ namespace MobileGL::MG_Backend::DirectVulkan { }; Bool ResolveUniformBufferPayload(const MG_State::GLState::ProgramObject& program, const ProgramFactory::VkProgramObject& programObj, Uint32 binding, - UboBindResult& out) const; + Uint32 arrayElement, UboBindResult& out) const; Bool CreateDescriptorPool(Uint32 maxSets, VkDescriptorPool& outPool) const; Bool GrowFrameDescriptorPool(FrameResources& frame, Uint32 frameIndex); VkResult AllocateDescriptorSetsFromActivePool(