[Fix, Test] (MG_Backend/DirectVulkan, MG_IntegrationTest): an image uniform array is one binding with many descriptors, not one - Magma wrote only element zero and left the rest undefined

This commit is contained in:
2026-08-11 23:49:31 -04:00
parent c4e6ea1f23
commit f6849fc0b3
6 changed files with 698 additions and 12 deletions
@@ -2600,6 +2600,18 @@ namespace MobileGL::MG_Backend::DirectVulkan {
const GLenum uniformType = program.GetUniformType(static_cast<Uint>(location));
if (descriptorKind == DescriptorBindingKind::StorageImage) {
// An ARRAY of image uniforms is ONE binding carrying `count` descriptors,
// and the layout has to say so. Leaving it at the default 1 declared
// `uniform image2D g_image[4]` as a single-descriptor binding while the
// shader indexed descriptors 1..3 of it - an out-of-bounds descriptor
// access that lavapipe SIGSEGVs inside the JIT-ed shader thread rather than
// reporting (KHR-GL42.shader_image_load_store.advanced-sso-simple). Unlike
// a storage BLOCK array, whose elements take consecutive GL binding points
// from the declared one, each element of an image array carries its own
// independently assigned image unit - see ResolveStorageImageDescriptor.
entry.bindingDescriptorCounts[binding] =
static_cast<Uint16>(std::max<Uint32>(1u, sampler->count));
const VkFormat reflectedFormat =
ConvertSpirvImageFormatToVkFormat(sampler->image.image_format);
VkFormat& existingFormat = entry.storageImageFormatByBinding[binding];
@@ -745,7 +745,7 @@ namespace MobileGL::MG_Backend::DirectVulkan {
Bool UniformManager::ResolveStorageImageDescriptor(VkCommandBuffer commandBuffer,
const MG_State::GLState::ProgramObject& program,
const ProgramFactory::VkProgramObject& programObj,
Uint32 binding,
Uint32 binding, Uint32 element,
VkDescriptorImageInfo& outImageInfo) const {
outImageInfo = {};
MOBILEGL_ASSERT(m_textureManager != nullptr, "ResolveStorageImageDescriptor: texture manager is null");
@@ -753,11 +753,24 @@ namespace MobileGL::MG_Backend::DirectVulkan {
MOBILEGL_ASSERT(binding < programObj.samplerUniformLocationByBinding.size(),
"ResolveStorageImageDescriptor: binding %u out of range", binding);
const Int location = programObj.samplerUniformLocationByBinding[binding];
if (location < 0) {
const Int baseLocation = programObj.samplerUniformLocationByBinding[binding];
if (baseLocation < 0) {
MGLOG_E("ResolveStorageImageDescriptor: storage image binding %u has no uniform location", binding);
return false;
}
// Per ELEMENT, and this is where an image array differs from a storage-block array: GL
// gives every element of `uniform image2D g_image[4]` its own glUniform1i-assigned image
// unit, and the four units need not be consecutive or even ordered (the conformance case
// uses 0, 2, 4, 6). DoReflection reserves one uniform location per array element, so the
// element's location is the base plus its index - checked against the array's real
// extent so a descriptorCount that outran the reflection cannot walk onto the next
// uniform.
const Int location = baseLocation + static_cast<Int>(element);
if (!program.UniformLocationsAliasSameUniform(baseLocation, location)) {
MGLOG_E("ResolveStorageImageDescriptor: binding %u element %u is past the end of its image array",
binding, element);
return false;
}
const Int imageUnit = program.GetUniformSamplerOrImageUnitIndex(static_cast<Uint>(location));
if (imageUnit < 0 || imageUnit >= MG_State::GLState::TextureState::MAX_TEXTURE_IMAGE_UNITS) {
MGLOG_E("ResolveStorageImageDescriptor: image unit %d out of range for binding %u",
@@ -1527,17 +1540,33 @@ namespace MobileGL::MG_Backend::DirectVulkan {
write.pBufferInfo = &bufferInfos[firstBufferInfoIndex];
writes.push_back(write);
} else if (kind == ProgramFactory::DescriptorBindingKind::StorageImage) {
VkDescriptorImageInfo imageInfo{};
if (!ResolveStorageImageDescriptor(commandBuffer, program, programObj, binding, imageInfo)) {
MGLOG_E(
"UniformDescriptorBinder::BindProgramUniformBuffers failed: storage image binding %u has no valid descriptor",
binding);
return false;
// One write per binding, but `descriptorCount` image infos: an ARRAY of image
// uniforms is a single binding whose elements each carry their own image unit.
// Writing only element 0 - which is all this used to do - left elements 1..N
// never written at all, and a shader that indexes them reads an undefined
// descriptor (lavapipe faults inside the shader; a real driver is free to do
// anything).
const Uint32 descriptorCount =
binding < programObj.bindingDescriptorCounts.size()
? std::max<Uint32>(1, programObj.bindingDescriptorCounts[binding])
: 1u;
const SizeT firstImageInfoIndex = imageInfos.size();
for (Uint32 element = 0; element < descriptorCount; ++element) {
VkDescriptorImageInfo imageInfo{};
if (!ResolveStorageImageDescriptor(commandBuffer, program, programObj, binding, element,
imageInfo)) {
MGLOG_E(
"UniformDescriptorBinder::BindProgramUniformBuffers failed: storage image binding %u "
"element %u has no valid descriptor",
binding, element);
return false;
}
imageInfos.push_back(imageInfo);
}
imageInfos.push_back(imageInfo);
fastRebindKindsEligible = false;
write.descriptorType = VK_DESCRIPTOR_TYPE_STORAGE_IMAGE;
write.pImageInfo = &imageInfos.back();
write.descriptorCount = descriptorCount;
write.pImageInfo = &imageInfos[firstImageInfoIndex];
writes.push_back(write);
} else {
VkDescriptorImageInfo imageInfo{};
@@ -172,10 +172,12 @@ namespace MobileGL::MG_Backend::DirectVulkan {
Bool ResolveStorageBufferDescriptor(const MG_State::GLState::ProgramObject& program,
const ProgramFactory::VkProgramObject& programObj, Uint32 binding,
Uint32 element, VkDescriptorBufferInfo& outBufferInfo) const;
// `element` indexes an image ARRAY inside one binding; each element carries its own
// independently assigned GL image unit.
Bool ResolveStorageImageDescriptor(VkCommandBuffer commandBuffer,
const MG_State::GLState::ProgramObject& program,
const ProgramFactory::VkProgramObject& programObj, Uint32 binding,
VkDescriptorImageInfo& outImageInfo) const;
Uint32 element, VkDescriptorImageInfo& outImageInfo) const;
// Result of resolving a UBO binding: either a zero-copy direct bind to the app's resident
// VkBuffer (the GLES backend's approach - no per-draw copy) or the CPU payload to upload.
struct UboBindResult {