[Fix, Test] (MG_Util, MG_Backend/DirectGLES, MG_Backend/DirectVulkan): the two image target kinds a compute dispatch could not read - 1D-array on ES, imageBuffer on Vulkan

This commit is contained in:
2026-08-12 16:11:10 -04:00
parent 0b36621069
commit 2b46a3db96
15 changed files with 1286 additions and 9 deletions
@@ -1721,6 +1721,8 @@ namespace MobileGL::MG_Backend::DirectVulkan {
return ProgramFactory::DescriptorBindingKind::CombinedImageSampler;
case SPV_REFLECT_DESCRIPTOR_TYPE_UNIFORM_TEXEL_BUFFER:
return ProgramFactory::DescriptorBindingKind::UniformTexelBuffer;
case SPV_REFLECT_DESCRIPTOR_TYPE_STORAGE_TEXEL_BUFFER:
return ProgramFactory::DescriptorBindingKind::StorageTexelBuffer;
case SPV_REFLECT_DESCRIPTOR_TYPE_STORAGE_BUFFER:
return ProgramFactory::DescriptorBindingKind::StorageBuffer;
case SPV_REFLECT_DESCRIPTOR_TYPE_STORAGE_IMAGE:
@@ -1750,6 +1752,7 @@ namespace MobileGL::MG_Backend::DirectVulkan {
}
if (kind == ProgramFactory::DescriptorBindingKind::CombinedImageSampler ||
kind == ProgramFactory::DescriptorBindingKind::UniformTexelBuffer ||
kind == ProgramFactory::DescriptorBindingKind::StorageTexelBuffer ||
kind == ProgramFactory::DescriptorBindingKind::StorageImage) {
const auto arraySuffix = name.find("[0]");
if (arraySuffix != String::npos) {
@@ -1839,8 +1842,11 @@ namespace MobileGL::MG_Backend::DirectVulkan {
// UniformManager::BindProgramUniformBuffers: UBO instance arrays
// (uniform Block {...} b[N];), storage-block instance arrays, image uniform
// arrays, and combined-image-sampler arrays (uniform sampler2D s[N];).
// Anything else - a uniform TEXEL buffer array is the one remaining kind -
// must fail program creation cleanly rather than continue with corrupt state.
// Anything else - the two TEXEL buffer kinds are what remain, samplerBuffer[N]
// and imageBuffer[N] - must fail program creation cleanly rather than continue
// with corrupt state. Their per-draw path writes pTexelBufferView as the
// address of a vector element sized for one descriptor per binding, so an
// array would not merely be unresolved, it would dangle.
//
// Getting listed here is not cosmetic: a kind that is rejected leaves
// GetOrCreateProgram's MOBILEGL_ASSERT(remapOk) as the only complaint, and
@@ -2661,6 +2667,7 @@ namespace MobileGL::MG_Backend::DirectVulkan {
const auto descriptorKind = ReflectDescriptorTypeToBindingKind(sampler->descriptor_type);
if (descriptorKind != DescriptorBindingKind::CombinedImageSampler &&
descriptorKind != DescriptorBindingKind::UniformTexelBuffer &&
descriptorKind != DescriptorBindingKind::StorageTexelBuffer &&
descriptorKind != DescriptorBindingKind::StorageImage &&
descriptorKind != DescriptorBindingKind::StorageBuffer) {
continue;
@@ -2790,6 +2797,29 @@ namespace MobileGL::MG_Backend::DirectVulkan {
}
}
if (descriptorKind == DescriptorBindingKind::StorageTexelBuffer) {
// Only the declared format is recorded, and only so the per-draw resolve can
// prefer it over the one glBindImageTexture named. Everything the StorageImage
// branch above does about ARRAYS is deliberately absent: an imageBuffer array
// is refused outright by the array gate in RemapDescriptorBindingsForVulkan,
// exactly as a samplerBuffer array is, so bindingDescriptorCounts stays at the
// default 1 and the descriptor write below may take the address of a vector
// element without reserving room for extra elements.
const VkFormat reflectedFormat =
ConvertSpirvImageFormatToVkFormat(sampler->image.image_format);
VkFormat& existingFormat = entry.storageImageFormatByBinding[binding];
MOBILEGL_ASSERT(existingFormat == VK_FORMAT_UNDEFINED ||
reflectedFormat == VK_FORMAT_UNDEFINED ||
existingFormat == reflectedFormat,
"ProgramFactory::ReflectLayout: storage texel buffer binding %u ('%s') "
"has conflicting reflected formats (%d vs %d)",
binding, uniformName.c_str(), static_cast<Int>(existingFormat),
static_cast<Int>(reflectedFormat));
if (existingFormat == VK_FORMAT_UNDEFINED) {
existingFormat = reflectedFormat;
}
}
const TextureTarget target = UniformTypeToTextureTarget(uniformType);
MOBILEGL_ASSERT(target != TextureTarget::Unknown,
"ProgramFactory::ReflectLayout: failed to resolve texture target for '%s'",
@@ -2867,6 +2897,8 @@ namespace MobileGL::MG_Backend::DirectVulkan {
entry.dynamicBindings.push_back(binding);
} else if (kind == DescriptorBindingKind::UniformTexelBuffer) {
layoutBinding.descriptorType = VK_DESCRIPTOR_TYPE_UNIFORM_TEXEL_BUFFER;
} else if (kind == DescriptorBindingKind::StorageTexelBuffer) {
layoutBinding.descriptorType = VK_DESCRIPTOR_TYPE_STORAGE_TEXEL_BUFFER;
} else if (kind == DescriptorBindingKind::StorageBuffer) {
layoutBinding.descriptorType = VK_DESCRIPTOR_TYPE_STORAGE_BUFFER;
} else if (kind == DescriptorBindingKind::StorageImage) {
@@ -33,7 +33,13 @@ namespace MobileGL::MG_Backend::DirectVulkan {
CombinedImageSampler,
UniformTexelBuffer,
StorageBuffer,
StorageImage
StorageImage,
// GLSL `imageBuffer` - a buffer texture reached through an IMAGE unit rather than a
// texture unit. Vulkan spells it VK_DESCRIPTOR_TYPE_STORAGE_TEXEL_BUFFER, which is a
// VkBufferView like UniformTexelBuffer and not a VkImageView like StorageImage: it is
// the one image uniform whose descriptor is a buffer. Appended, never inserted -
// DescriptorKeyHash mixes the enumerator's value.
StorageTexelBuffer
};
enum class CompileOptionBit : Uint {
@@ -103,6 +109,11 @@ namespace MobileGL::MG_Backend::DirectVulkan {
Vector<Int> samplerUniformLocationByBinding;
Vector<TextureTarget> samplerTextureTargetByBinding;
Vector<SamplerNumericDomain> samplerNumericDomainByBinding;
// Shared by StorageImage and StorageTexelBuffer bindings: a binding is one kind or
// the other, never both, and both need exactly the same thing - the format the
// shader declared, so the per-draw resolve can tell a typed declaration from a
// formatless one. Kept as one pair rather than two so the move operations below
// cannot drift out of sync with a field that only one kind populates.
Vector<VkFormat> storageImageFormatByBinding;
Vector<Bool> storageImageUsesBindingFormatByBinding;
Vector<String> storageBlockNameByBinding;
@@ -743,6 +743,142 @@ namespace MobileGL::MG_Backend::DirectVulkan {
return true;
}
// GLSL `imageBuffer`. The one image uniform whose Vulkan descriptor is a VkBufferView rather
// than a VkImageView, so it is half ResolveStorageImageDescriptor (the resource comes from an
// IMAGE unit, i.e. from glBindImageTexture, not from a texture unit) and half
// ResolveTexelBufferDescriptor (the descriptor is a buffer view over the GL buffer the
// texture is attached to).
//
// Before this existed the descriptor kind reflected as SPV_REFLECT_DESCRIPTOR_TYPE_STORAGE_-
// TEXEL_BUFFER and fell into ReflectDescriptorTypeToBindingKind's `default:`, whose only
// complaint is an assert that compiles out above DEBUG - so a release build declared no
// binding at all for a uniform the shader still read, and lavapipe segfaulted inside pipeline
// creation on the JIT worker thread. KHR-GL44.multi_bind.dispatch_bind_image_textures is the
// case that carries it.
Bool UniformManager::ResolveStorageTexelBufferDescriptor(const MG_State::GLState::ProgramObject& program,
const ProgramFactory::VkProgramObject& programObj,
Uint32 binding, Uint32 frameIndex,
VkBufferView& outBufferView) {
outBufferView = VK_NULL_HANDLE;
MOBILEGL_ASSERT(m_bufferManager != nullptr, "ResolveStorageTexelBufferDescriptor: buffer manager is null");
MOBILEGL_ASSERT(MG_State::pGLContext != nullptr, "ResolveStorageTexelBufferDescriptor: GL context is null");
MOBILEGL_ASSERT(frameIndex < m_frames.size(),
"ResolveStorageTexelBufferDescriptor: frame index out of range");
MOBILEGL_ASSERT(binding < programObj.samplerUniformLocationByBinding.size(),
"ResolveStorageTexelBufferDescriptor: binding %u out of range", binding);
const Int location = programObj.samplerUniformLocationByBinding[binding];
if (location < 0) {
MGLOG_E("ResolveStorageTexelBufferDescriptor: binding %u ('%s') has no uniform location", binding,
programObj.samplerNameByBinding[binding].c_str());
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("ResolveStorageTexelBufferDescriptor: image unit %d out of range for binding %u", imageUnit,
binding);
return false;
}
auto& imageBinding = MG_State::pGLContext->GetImageTextureBinding(imageUnit);
const auto& texture = imageBinding.Texture;
if (texture == nullptr) {
MGLOG_E("ResolveStorageTexelBufferDescriptor: image unit %d is unbound for binding %u", imageUnit,
binding);
return false;
}
if (texture->GetStorageType() != TextureStorageType::Buffer ||
texture->GetTarget() != TextureTarget::TextureBuffer) {
MGLOG_E("ResolveStorageTexelBufferDescriptor: binding %u ('%s') expected a texture buffer on image "
"unit %d, got textureId=%u target=%d storage=%d",
binding, programObj.samplerNameByBinding[binding].c_str(), imageUnit,
texture->GetExternalIndex(), static_cast<Int>(texture->GetTarget()),
static_cast<Int>(texture->GetStorageType()));
return false;
}
auto* textureBuffer = static_cast<MG_State::GLState::TextureObjectBuffer*>(texture.get());
const auto& bufferObject = textureBuffer->GetBufferBindingSlot().GetBoundObject();
if (bufferObject == nullptr) {
MGLOG_E("ResolveStorageTexelBufferDescriptor: texture buffer on image unit %d has no GL buffer bound",
imageUnit);
return false;
}
// Unlike the sampled texel buffer, the shader may WRITE this one, and those writes land in
// GPU memory behind the frontend's CPU shadow - which is what MapBuffer and
// GetBufferSubData read. Same two calls, and for the same reason, as the storage-block
// path above.
bufferObject->EnsureGpuResidentStorage();
bufferObject->MarkGpuWritten();
BufferSlice slice{};
if (!m_bufferManager->AcquireResidentSlice(BufferKind::TextureBuffer, bufferObject, slice) ||
!slice.IsValid()) {
MGLOG_E("ResolveStorageTexelBufferDescriptor: failed to sync GL buffer %u for texture buffer %u",
bufferObject->GetExternalIndex(), texture->GetExternalIndex());
return false;
}
// The format the SHADER declared wins over the one glBindImageTexture named, on the same
// policy as a storage image: a typed `layout(r32ui) uniform uimageBuffer` must be read as
// r32ui whatever the texture's own attachment format says. Falling back, in order:
// reflected format, then the bind format, then the texture's attached format.
MOBILEGL_ASSERT(binding < programObj.storageImageFormatByBinding.size(),
"ResolveStorageTexelBufferDescriptor: binding %u has no reflected format slot", binding);
const auto internalFormat = textureBuffer->GetFormat();
const VkFormat resourceFormat = MG_Util::ConvertTextureInternalFormatToVkEnum(internalFormat);
const VkFormat reflectedFormat = programObj.storageImageFormatByBinding[binding];
VkFormat vkFormat = reflectedFormat;
if (vkFormat == VK_FORMAT_UNDEFINED && imageBinding.Format != 0) {
vkFormat = MG_Util::ConvertTextureInternalFormatToVkEnum(
MG_Util::ConvertGLEnumToTextureInternalFormat(imageBinding.Format));
}
if (vkFormat == VK_FORMAT_UNDEFINED) {
vkFormat = resourceFormat;
}
if (vkFormat == VK_FORMAT_UNDEFINED) {
MGLOG_E("ResolveStorageTexelBufferDescriptor: unsupported image buffer format (internal=%d bind=0x%x)",
static_cast<Int>(internalFormat), imageBinding.Format);
return false;
}
// Sized against the VIEW's format, not the texture's attached one - they may differ by the
// paragraph above, and a range that is not a whole number of the view's texels is invalid.
const VkDeviceSize texelSize =
static_cast<VkDeviceSize>(MG_Util::GetSizedInternalFormatSizeInBytes(internalFormat));
const VkDeviceSize rangeOffset = static_cast<VkDeviceSize>(textureBuffer->GetBufferRangeOffset());
const VkDeviceSize rangeSize = static_cast<VkDeviceSize>(textureBuffer->GetBufferRangeSizeInBytes());
VkDeviceSize viewRange = std::min(rangeSize, slice.size > rangeOffset ? slice.size - rangeOffset : 0);
if (texelSize > 0) {
viewRange = (viewRange / texelSize) * texelSize;
}
if (viewRange == 0) {
MGLOG_E("ResolveStorageTexelBufferDescriptor: texture buffer %u has empty view range",
texture->GetExternalIndex());
return false;
}
VkBufferViewCreateInfo viewInfo{};
viewInfo.sType = VK_STRUCTURE_TYPE_BUFFER_VIEW_CREATE_INFO;
viewInfo.buffer = slice.buffer;
viewInfo.format = vkFormat;
viewInfo.offset = slice.offset + rangeOffset;
viewInfo.range = viewRange;
VkBufferView bufferView = VK_NULL_HANDLE;
const VkResult result = vkCreateBufferView(m_device, &viewInfo, nullptr, &bufferView);
if (result != VK_SUCCESS || bufferView == VK_NULL_HANDLE) {
MGLOG_E("ResolveStorageTexelBufferDescriptor: vkCreateBufferView failed result=%d format=%d range=%zu",
result, static_cast<Int>(vkFormat), static_cast<SizeT>(viewRange));
return false;
}
m_frames[frameIndex].texelBufferViews.push_back(bufferView);
outBufferView = bufferView;
return true;
}
Bool UniformManager::ResolveStorageBufferDescriptor(const MG_State::GLState::ProgramObject& program,
const ProgramFactory::VkProgramObject& programObj,
Uint32 binding, Uint32 element,
@@ -1267,7 +1403,7 @@ namespace MobileGL::MG_Backend::DirectVulkan {
}
const Uint32 descriptorCount = static_cast<Uint32>(descriptorCount64);
VkDescriptorPoolSize poolSizes[5]{};
VkDescriptorPoolSize poolSizes[6]{};
poolSizes[0].type = VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER_DYNAMIC;
poolSizes[0].descriptorCount = descriptorCount;
poolSizes[1].type = VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER;
@@ -1278,6 +1414,8 @@ namespace MobileGL::MG_Backend::DirectVulkan {
poolSizes[3].descriptorCount = descriptorCount;
poolSizes[4].type = VK_DESCRIPTOR_TYPE_STORAGE_IMAGE;
poolSizes[4].descriptorCount = descriptorCount;
poolSizes[5].type = VK_DESCRIPTOR_TYPE_STORAGE_TEXEL_BUFFER;
poolSizes[5].descriptorCount = descriptorCount;
VkDescriptorPoolCreateInfo poolInfo{};
poolInfo.sType = VK_STRUCTURE_TYPE_DESCRIPTOR_POOL_CREATE_INFO;
@@ -1644,6 +1782,25 @@ namespace MobileGL::MG_Backend::DirectVulkan {
write.descriptorType = VK_DESCRIPTOR_TYPE_UNIFORM_TEXEL_BUFFER;
write.pTexelBufferView = &texelBufferViews.back();
writes.push_back(write);
} else if (kind == ProgramFactory::DescriptorBindingKind::StorageTexelBuffer) {
// Shares texelBufferViews with the sampled kind above, and may do so safely for
// the same reason: neither kind can be an array, so each contributes exactly one
// element and the reserve of m_maxBindings cannot be outrun - which is what keeps
// the &back() below from dangling when a later binding pushes.
VkBufferView bufferView = VK_NULL_HANDLE;
if (!ResolveStorageTexelBufferDescriptor(program, programObj, binding, frameIndex, bufferView) ||
bufferView == VK_NULL_HANDLE) {
MGLOG_E("UniformDescriptorBinder::BindProgramUniformBuffers failed: image buffer binding %u "
"has no valid descriptor",
binding);
return false;
}
texelBufferViews.push_back(bufferView);
fastRebindKindsEligible = false;
write.descriptorType = VK_DESCRIPTOR_TYPE_STORAGE_TEXEL_BUFFER;
write.pTexelBufferView = &texelBufferViews.back();
writes.push_back(write);
} else if (kind == ProgramFactory::DescriptorBindingKind::StorageBuffer) {
// One write per binding, but `descriptorCount` buffer infos: a GLSL block
// instance array occupies a single binding whose elements each come from their
@@ -175,6 +175,14 @@ namespace MobileGL::MG_Backend::DirectVulkan {
Bool ResolveTexelBufferDescriptor(const MG_State::GLState::ProgramObject& program,
const ProgramFactory::VkProgramObject& programObj, Uint32 binding,
Uint32 frameIndex, VkBufferView& outBufferView);
// GLSL `imageBuffer`: the same VkBufferView descriptor as the sampled texel buffer above,
// but resolved from an IMAGE unit (glBindImageTexture) rather than a texture unit, and
// made GPU-resident-writable because the shader may store to it. No `element` parameter:
// an imageBuffer ARRAY is refused at program creation, so a binding is always one
// descriptor (see the array gate in RemapDescriptorBindingsForVulkan).
Bool ResolveStorageTexelBufferDescriptor(const MG_State::GLState::ProgramObject& program,
const ProgramFactory::VkProgramObject& programObj, Uint32 binding,
Uint32 frameIndex, VkBufferView& outBufferView);
// `element` indexes a block INSTANCE array's descriptors; it is 0 for every ordinary
// block. Each element resolves through its own GL storage block, and so its own GL
// binding point, buffer and glBindBufferRange window.
@@ -23,7 +23,11 @@ namespace MobileGL::MG_Backend::DirectVulkan {
VK_BUFFER_USAGE_VERTEX_BUFFER_BIT | VK_BUFFER_USAGE_INDEX_BUFFER_BIT |
VK_BUFFER_USAGE_UNIFORM_BUFFER_BIT | VK_BUFFER_USAGE_STORAGE_BUFFER_BIT |
VK_BUFFER_USAGE_INDIRECT_BUFFER_BIT | VK_BUFFER_USAGE_UNIFORM_TEXEL_BUFFER_BIT |
VK_BUFFER_USAGE_TRANSFER_SRC_BIT;
// "Every usage" has to mean every usage: a buffer texture reached through an IMAGE
// unit takes a VK_DESCRIPTOR_TYPE_STORAGE_TEXEL_BUFFER descriptor, and the write is
// invalid unless the buffer was created with this bit. Nothing asked for it until
// imageBuffer support existed, so the omission was invisible.
VK_BUFFER_USAGE_STORAGE_TEXEL_BUFFER_BIT | VK_BUFFER_USAGE_TRANSFER_SRC_BIT;
// Appended to kPersistentBackedUsage when VK_EXT_transform_feedback is enabled
// (see VkBufferManagerInitInfo::transformFeedbackUsageEnabled).
constexpr VkBufferUsageFlags kTransformFeedbackUsage =
@@ -714,7 +718,13 @@ namespace MobileGL::MG_Backend::DirectVulkan {
case BufferKind::Uniform:
return VK_BUFFER_USAGE_UNIFORM_BUFFER_BIT;
case BufferKind::TextureBuffer:
return VK_BUFFER_USAGE_UNIFORM_TEXEL_BUFFER_BIT;
// Both texel roles, for the same reason vertex/index carry both bits: one GL buffer
// texture can be read as a samplerBuffer and written as an imageBuffer, and which of
// the two it is only becomes known when a shader that uses it is bound - long after
// the resident buffer was created. A VkBufferView for a storage-texel descriptor is
// invalid unless the buffer was created with the storage bit, so a buffer that
// acquired only the uniform bit could never be given one.
return VK_BUFFER_USAGE_UNIFORM_TEXEL_BUFFER_BIT | VK_BUFFER_USAGE_STORAGE_TEXEL_BUFFER_BIT;
case BufferKind::ShaderStorage:
return VK_BUFFER_USAGE_STORAGE_BUFFER_BIT | VK_BUFFER_USAGE_INDIRECT_BUFFER_BIT;
case BufferKind::Indirect: