From 59f7059bf4bfe94f82d22e16b6af68911b8791c5 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Mon, 24 Aug 2026 05:31:15 -0400 Subject: [PATCH] [Fix, Test] (DirectVulkan): keep the draw when a program declares an image-backed resource the application left unbound --- .../DirectVulkan/Renderer/ProgramFactory.cpp | 71 +++ .../DirectVulkan/Renderer/ProgramFactory.h | 8 + .../DirectVulkan/Renderer/UniformManager.cpp | 427 +++++++++++++++++- .../DirectVulkan/Renderer/UniformManager.h | 41 +- .../DirectVulkan/Renderer/VkBufferManager.cpp | 39 ++ .../DirectVulkan/Renderer/VkBufferManager.h | 12 + .../DirectVulkan/Renderer/VulkanRenderer.cpp | 2 +- MobileGL/MG_IntegrationTest/CMakeLists.txt | 1 + .../UnboundImageDescriptorScenario.cpp | 380 ++++++++++++++++ 9 files changed, 959 insertions(+), 22 deletions(-) create mode 100644 MobileGL/MG_IntegrationTest/Scenarios/UnboundImageDescriptorScenario.cpp diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/ProgramFactory.cpp b/MobileGL/MG_Backend/DirectVulkan/Renderer/ProgramFactory.cpp index 8b9eb13a..dfdcebcb 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/ProgramFactory.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/ProgramFactory.cpp @@ -2186,6 +2186,49 @@ namespace MobileGL::MG_Backend::DirectVulkan { } } + SamplerNumericDomain ProgramFactory::UniformTypeToImageNumericDomain(GLenum glType) { + switch (glType) { + case GL_INT_IMAGE_1D: + case GL_INT_IMAGE_2D: + case GL_INT_IMAGE_3D: + case GL_INT_IMAGE_2D_RECT: + case GL_INT_IMAGE_CUBE: + case GL_INT_IMAGE_BUFFER: + case GL_INT_IMAGE_1D_ARRAY: + case GL_INT_IMAGE_2D_ARRAY: + case GL_INT_IMAGE_CUBE_MAP_ARRAY: + case GL_INT_IMAGE_2D_MULTISAMPLE: + case GL_INT_IMAGE_2D_MULTISAMPLE_ARRAY: + return SamplerNumericDomain::SignedInteger; + case GL_UNSIGNED_INT_IMAGE_1D: + case GL_UNSIGNED_INT_IMAGE_2D: + case GL_UNSIGNED_INT_IMAGE_3D: + case GL_UNSIGNED_INT_IMAGE_2D_RECT: + case GL_UNSIGNED_INT_IMAGE_CUBE: + case GL_UNSIGNED_INT_IMAGE_BUFFER: + case GL_UNSIGNED_INT_IMAGE_1D_ARRAY: + case GL_UNSIGNED_INT_IMAGE_2D_ARRAY: + case GL_UNSIGNED_INT_IMAGE_CUBE_MAP_ARRAY: + case GL_UNSIGNED_INT_IMAGE_2D_MULTISAMPLE: + case GL_UNSIGNED_INT_IMAGE_2D_MULTISAMPLE_ARRAY: + return SamplerNumericDomain::UnsignedInteger; + case GL_IMAGE_1D: + case GL_IMAGE_2D: + case GL_IMAGE_3D: + case GL_IMAGE_2D_RECT: + case GL_IMAGE_CUBE: + case GL_IMAGE_BUFFER: + case GL_IMAGE_1D_ARRAY: + case GL_IMAGE_2D_ARRAY: + case GL_IMAGE_CUBE_MAP_ARRAY: + case GL_IMAGE_2D_MULTISAMPLE: + case GL_IMAGE_2D_MULTISAMPLE_ARRAY: + return SamplerNumericDomain::Float; + default: + return SamplerNumericDomain::Unknown; + } + } + ProgramFactory::HashType ProgramFactory::ComputeHash(const MG_State::GLState::ProgramObject& program, CompileOptionFlags flags) const { XXHASH_VERIFY(XXH64_reset(m_hashState, m_config.CacheVersion)); @@ -2956,6 +2999,34 @@ namespace MobileGL::MG_Backend::DirectVulkan { static_cast(numericDomain)); entry.samplerNumericDomainByBinding[binding] = numericDomain; } + // Every other opaque kind records its domain too. Only the combined-image-sampler + // path above needs it to pick a sampled view format; the three below need it to + // describe the descriptor a binding gets when its unit is UNBOUND, which is legal + // GL and must not lose the draw (see UniformManager's Resolve*Descriptor). Left + // Unknown, those placeholders would have no way to tell a `samplerBuffer` from a + // `usamplerBuffer` - and a texel buffer view whose numeric type disagrees with the + // shader's is invalid Vulkan, not merely wrong data. + if (descriptorKind == DescriptorBindingKind::UniformTexelBuffer || + descriptorKind == DescriptorBindingKind::StorageTexelBuffer || + descriptorKind == DescriptorBindingKind::StorageImage) { + const SamplerNumericDomain opaqueDomain = + descriptorKind == DescriptorBindingKind::UniformTexelBuffer + ? UniformTypeToSamplerNumericDomain(uniformType) + : UniformTypeToImageNumericDomain(uniformType); + MOBILEGL_ASSERT(opaqueDomain != SamplerNumericDomain::Unknown, + "ProgramFactory::ReflectLayout: failed to resolve numeric domain for '%s' " + "(uniformType=0x%x)", + uniformName.c_str(), uniformType); + MOBILEGL_ASSERT(entry.samplerNumericDomainByBinding[binding] == + SamplerNumericDomain::Unknown || + entry.samplerNumericDomainByBinding[binding] == opaqueDomain, + "ProgramFactory::ReflectLayout: binding %u ('%s') has conflicting numeric " + "domains (%d vs %d)", + binding, uniformName.c_str(), + static_cast(entry.samplerNumericDomainByBinding[binding]), + static_cast(opaqueDomain)); + entry.samplerNumericDomainByBinding[binding] = opaqueDomain; + } MOBILEGL_ASSERT(entry.samplerUniformLocationByBinding[binding] < 0 || location < 0 || entry.samplerUniformLocationByBinding[binding] == location, "ProgramFactory::ReflectLayout: texture binding %u maps to conflicting uniform locations (%d vs %d)", diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/ProgramFactory.h b/MobileGL/MG_Backend/DirectVulkan/Renderer/ProgramFactory.h index bb1eb53a..79700e38 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/ProgramFactory.h +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/ProgramFactory.h @@ -448,6 +448,14 @@ namespace MobileGL::MG_Backend::DirectVulkan { static VkShaderStageFlagBits ToVkStage(ShaderStage stage); static VkFormat ConvertSpirvImageFormatToVkFormat(SpvImageFormat format); static SamplerNumericDomain UniformTypeToSamplerNumericDomain(GLenum glType); + // The same question for an IMAGE uniform (`image2D`, `uimageBuffer`, ...), which the + // sampler form above deliberately does not answer. Kept separate rather than folded in + // because the two are asked in different places for different reasons: a sampler's domain + // decides a sampled VIEW format, an image's decides what a placeholder descriptor for an + // UNBOUND image unit must be (see UniformManager::AcquireUnboundTexelBufferView and + // GetUnboundStorageImageTexture) - a formatless `writeonly` declaration reflects no + // format at all, and the numeric domain is then the only thing that constrains it. + static SamplerNumericDomain UniformTypeToImageNumericDomain(GLenum glType); // True when any entry point declares the DepthReplacing execution mode, i.e. the // shader assigns gl_FragDepth. Exposed so the blended depth-write quirk's exemption // can be pinned by tests. A false negative loses the exemption, so such a shader is diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/UniformManager.cpp b/MobileGL/MG_Backend/DirectVulkan/Renderer/UniformManager.cpp index a2ad7cc6..f7577221 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/UniformManager.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/UniformManager.cpp @@ -11,14 +11,19 @@ #include "MG_Backend/DirectVulkan/DirectVulkanResourceState.h" #include "MG_State/GLState/Core.h" #include "MG_State/GLState/ProgramState/ProgramObject.h" +#include "MG_State/GLState/TextureState/TextureObject1D.h" #include "MG_State/GLState/TextureState/TextureObject2D.h" +#include "MG_State/GLState/TextureState/TextureObject2DCube.h" +#include "MG_State/GLState/TextureState/TextureObject3D.h" #include "MG_State/GLState/TextureState/TextureObjectBuffer.h" +#include "MG_State/GLState/TextureState/TextureObjectStubs.h" #include "MG_Util/Converters/GLToMG/TextureEnumConverter.h" #include "MG_Util/Converters/MGToStr/FramebufferEnumConverter.h" #include "MG_Util/Converters/MGToVk/TextureEnumConverter.h" #include "MG_Util/Metrics/TextureMetrics.h" #include "MG_Util/ShaderTranspiler/Types.h" #include +#include #include #include #include @@ -28,6 +33,136 @@ namespace MobileGL::MG_Backend::DirectVulkan { namespace { constexpr Uint kFallbackTexture2DExternalIndex = 0xFFFFFF00u; + // One id for every storage-image placeholder. They are never reachable through GL - no + // glGenTextures ever hands this out, and nothing looks a placeholder up by name - so the + // id only has to stay clear of the application's, exactly like the sampled fallback's. + constexpr Uint kUnboundStorageImageExternalIndex = 0xFFFFFF01u; + + // The R32 member of each numeric class. Every one of the three is a MANDATORY-support + // format for uniform texel buffers, storage texel buffers and storage images alike + // (Vulkan 1.0, "Required Format Support"), which is what makes them a fallback that + // cannot itself fail for want of device features. + VkFormat PlaceholderFormatForNumericDomain(SamplerNumericDomain numericDomain) { + switch (numericDomain) { + case SamplerNumericDomain::Float: + return VK_FORMAT_R32_SFLOAT; + case SamplerNumericDomain::SignedInteger: + return VK_FORMAT_R32_SINT; + case SamplerNumericDomain::UnsignedInteger: + return VK_FORMAT_R32_UINT; + case SamplerNumericDomain::Unknown: + break; + } + return VK_FORMAT_UNDEFINED; + } + + Bool BufferFormatSupportsFeature(VkPhysicalDevice physicalDevice, VkFormat format, + VkFormatFeatureFlags requiredFeature) { + if (physicalDevice == VK_NULL_HANDLE || format == VK_FORMAT_UNDEFINED) { + return false; + } + VkFormatProperties properties{}; + vkGetPhysicalDeviceFormatProperties(physicalDevice, format, &properties); + return (properties.bufferFeatures & requiredFeature) == requiredFeature; + } + + // Reverse of MG_Util::ConvertTextureInternalFormatToVkEnum. A placeholder texture is + // built through the ordinary frontend texture object (that is what gets it an image with + // STORAGE usage, a GENERAL transition and a view, for free), and that object is described + // by a GL internal format - while everything upstream of here speaks VkFormat. Scanned + // rather than tabulated: it runs once per (target, format) placeholder ever created, the + // enum is ~70 entries, and a second hand-written table is a second thing to drift. + // Ascending order matters: the sized formats precede the unsized aliases, so a scan + // answers with the sized one. + TextureInternalFormat InternalFormatForVkFormat(VkFormat format) { + if (format == VK_FORMAT_UNDEFINED) { + return TextureInternalFormat::Unknown; + } + for (Int index = 0; index < static_cast(TextureInternalFormat::TextureInternalFormatCount); + ++index) { + const auto candidate = static_cast(index); + if (MG_Util::ConvertTextureInternalFormatToVkEnum(candidate) == format) { + return candidate; + } + } + return TextureInternalFormat::Unknown; + } + + // What a 1x1 placeholder of a given target has to allocate for the backend to give it the + // Vulkan view type that target's image declaration demands (see + // VkTextureManager's TryResolveTextureShapeInfo, which reads exactly these two things). + struct PlaceholderShape { + Array uploadTargets{}; + Uint32 uploadTargetCount = 0; + // The GL depth of the single level: the array length for an array target, the depth + // for a 3D one, and 6 for a cube map array (one whole cube). + Int depth = 1; + Bool valid = false; + }; + + PlaceholderShape PlaceholderShapeForTarget(TextureTarget target) { + PlaceholderShape shape{}; + switch (target) { + case TextureTarget::Texture1D: + shape = {{TextureUploadTarget::Texture1D}, 1, 1, true}; + break; + case TextureTarget::Texture2D: + shape = {{TextureUploadTarget::Texture2D}, 1, 1, true}; + break; + case TextureTarget::TextureRectangle: + shape = {{TextureUploadTarget::TextureRectangle}, 1, 1, true}; + break; + case TextureTarget::Texture3D: + shape = {{TextureUploadTarget::Texture3D}, 1, 1, true}; + break; + case TextureTarget::Texture1DArray: + shape = {{TextureUploadTarget::Texture1DArray}, 1, 1, true}; + break; + case TextureTarget::Texture2DArray: + shape = {{TextureUploadTarget::Texture2DArray}, 1, 1, true}; + break; + case TextureTarget::TextureCubeMap: + shape = {{TextureUploadTarget::CubeMapPositiveX, TextureUploadTarget::CubeMapNegativeX, + TextureUploadTarget::CubeMapPositiveY, TextureUploadTarget::CubeMapNegativeY, + TextureUploadTarget::CubeMapPositiveZ, TextureUploadTarget::CubeMapNegativeZ}, + 6, 1, true}; + break; + case TextureTarget::TextureCubeMapArray: + // Layers are cube faces, so the count must be a whole number of cubes. + shape = {{TextureUploadTarget::CubeMapArray}, 1, 6, true}; + break; + default: + // Multisample targets above all: their descriptor needs a multisample view. + break; + } + return shape; + } + + // TextureObjectMipmap, not ITextureObject: AllocateStorage and MarkStorageDirty live + // there, and every placeholder shape above is one of its subclasses. + SharedPtr MakePlaceholderTextureObject(TextureTarget target, + Uint index) { + switch (target) { + case TextureTarget::Texture1D: + return MakeShared(index); + case TextureTarget::Texture2D: + return MakeShared(index); + case TextureTarget::TextureRectangle: + return MakeShared(index); + case TextureTarget::Texture3D: + return MakeShared(index); + case TextureTarget::Texture1DArray: + return MakeShared(index); + case TextureTarget::Texture2DArray: + return MakeShared(index); + case TextureTarget::TextureCubeMap: + return MakeShared(index); + case TextureTarget::TextureCubeMapArray: + return MakeShared(index); + default: + return nullptr; + } + } } static Bool FindFramebufferAttachmentForTexture(const MG_State::GLState::FramebufferObject& framebuffer, @@ -115,7 +250,8 @@ namespace MobileGL::MG_Backend::DirectVulkan { return reflectedFormat != VK_FORMAT_UNDEFINED ? reflectedFormat : resourceFormat; } - Bool UniformManager::Initialize(VkDevice device, VkBufferManager* bufferManager, + Bool UniformManager::Initialize(VkDevice device, VkPhysicalDevice physicalDevice, + VkBufferManager* bufferManager, ProgramFactory* programFactory, VkDeviceSize minUniformBufferOffsetAlignment, Uint32 frameCount, Uint32 maxBindings, Uint32 setsPerFrame, @@ -123,6 +259,8 @@ namespace MobileGL::MG_Backend::DirectVulkan { Shutdown(); MOBILEGL_ASSERT(device != VK_NULL_HANDLE, "UniformDescriptorBinder::Initialize requires valid VkDevice"); + MOBILEGL_ASSERT(physicalDevice != VK_NULL_HANDLE, + "UniformDescriptorBinder::Initialize requires valid VkPhysicalDevice"); MOBILEGL_ASSERT(bufferManager != nullptr, "UniformDescriptorBinder::Initialize requires valid buffer manager"); MOBILEGL_ASSERT(programFactory != nullptr, "UniformDescriptorBinder::Initialize requires valid program factory"); @@ -135,6 +273,7 @@ namespace MobileGL::MG_Backend::DirectVulkan { "UniformDescriptorBinder::Initialize requires valid sampler manager"); m_device = device; + m_physicalDevice = physicalDevice; m_bufferManager = bufferManager; m_programFactory = programFactory; m_minDynamicOffsetAlignment = std::max(1, minUniformBufferOffsetAlignment); @@ -173,6 +312,17 @@ namespace MobileGL::MG_Backend::DirectVulkan { } void UniformManager::Shutdown() { + // Before the per-frame loop, because these views are NOT owned by any frame slot (see + // m_unboundTexelBufferViews) and the loop below is what clears m_device. + if (m_device != VK_NULL_HANDLE) { + for (const auto& viewEntry : m_unboundTexelBufferViews) { + if (viewEntry.second != VK_NULL_HANDLE) { + vkDestroyBufferView(m_device, viewEntry.second, nullptr); + } + } + } + m_unboundTexelBufferViews.clear(); + m_unboundStorageImageTextures.clear(); for (auto& frame : m_frames) { if (m_device != VK_NULL_HANDLE) { for (auto& view : frame.texelBufferViews) { @@ -199,6 +349,7 @@ namespace MobileGL::MG_Backend::DirectVulkan { m_bufferManager = nullptr; m_programFactory = nullptr; m_device = VK_NULL_HANDLE; + m_physicalDevice = VK_NULL_HANDLE; m_minDynamicOffsetAlignment = 1; m_frameCount = 0; m_maxBindings = 0; @@ -693,11 +844,29 @@ namespace MobileGL::MG_Backend::DirectVulkan { MOBILEGL_ASSERT(m_bufferManager != nullptr, "ResolveTexelBufferDescriptor: buffer manager is null"); MOBILEGL_ASSERT(frameIndex < m_frames.size(), "ResolveTexelBufferDescriptor: frame index out of range"); + MOBILEGL_ASSERT(binding < programObj.samplerNumericDomainByBinding.size(), + "ResolveTexelBufferDescriptor: numeric domain binding %u out of range", binding); + const SamplerNumericDomain numericDomain = programObj.samplerNumericDomainByBinding[binding]; + SharedPtr texture; if (!ResolveSamplerTexture(program, programObj, binding, texture) || texture == nullptr) { - MGLOG_E_ONCE("ResolveTexelBufferDescriptor: texture buffer binding %u ('%s') is unbound", binding, - programObj.samplerNameByBinding[binding].c_str()); - return false; + // NOT an error, and not a reason to lose the draw. A texture unit with nothing on it + // is a legal GL state (4.6 core 8.24): the sampler is incomplete, so a fetch through + // it returns undefined values - the same answer the sampled path above gives with its + // fallback texture, which a buffer texture simply cannot use because its descriptor is + // a VkBufferView. A per-format placeholder view is the equivalent for this kind. + const VkBufferView placeholder = + AcquireUnboundTexelBufferView(VK_FORMAT_UNDEFINED, numericDomain, false); + if (placeholder == VK_NULL_HANDLE) { + MGLOG_E_ONCE("ResolveTexelBufferDescriptor: texture buffer binding %u ('%s') is unbound, and the " + "placeholder descriptor could not be created", binding, + programObj.samplerNameByBinding[binding].c_str()); + return false; + } + MGLOG_D("ResolveTexelBufferDescriptor: binding %u ('%s') is unbound; using the placeholder descriptor", + binding, programObj.samplerNameByBinding[binding].c_str()); + outBufferView = placeholder; + return true; } if (texture->GetStorageType() != TextureStorageType::Buffer || @@ -712,9 +881,21 @@ namespace MobileGL::MG_Backend::DirectVulkan { auto* textureBuffer = static_cast(texture.get()); const auto& bufferObject = textureBuffer->GetBufferBindingSlot().GetBoundObject(); if (bufferObject == nullptr) { - MGLOG_E_ONCE("ResolveTexelBufferDescriptor: texture buffer binding %u ('%s') has no GL buffer bound", - binding, programObj.samplerNameByBinding[binding].c_str()); - return false; + // A buffer texture with no buffer object attached is INCOMPLETE, not illegal (GL 4.6 + // core 8.9), and sampling an incomplete texture is undefined - so this too keeps the + // draw on a placeholder rather than dropping it. + const VkBufferView placeholder = + AcquireUnboundTexelBufferView(VK_FORMAT_UNDEFINED, numericDomain, false); + if (placeholder == VK_NULL_HANDLE) { + MGLOG_E_ONCE("ResolveTexelBufferDescriptor: texture buffer binding %u ('%s') has no GL buffer bound, " + "and the placeholder descriptor could not be created", binding, + programObj.samplerNameByBinding[binding].c_str()); + return false; + } + MGLOG_D("ResolveTexelBufferDescriptor: binding %u ('%s') has no attached GL buffer; using the " + "placeholder descriptor", binding, programObj.samplerNameByBinding[binding].c_str()); + outBufferView = placeholder; + return true; } BufferSlice slice{}; @@ -804,12 +985,31 @@ namespace MobileGL::MG_Backend::DirectVulkan { return false; } + MOBILEGL_ASSERT(binding < programObj.storageImageFormatByBinding.size(), + "ResolveStorageTexelBufferDescriptor: binding %u has no reflected format slot", binding); + MOBILEGL_ASSERT(binding < programObj.samplerNumericDomainByBinding.size(), + "ResolveStorageTexelBufferDescriptor: numeric domain binding %u out of range", binding); + auto& imageBinding = MG_State::pGLContext->GetImageTextureBinding(imageUnit); const auto& texture = imageBinding.Texture; if (texture == nullptr) { - MGLOG_E_ONCE("ResolveStorageTexelBufferDescriptor: image unit %d is unbound for binding %u", imageUnit, - binding); - return false; + // An image unit with no texture on it is legal GL (4.6 core 8.26): loads return zero + // and stores are discarded. Declining here took the whole draw or dispatch with it - + // the same shape as the unbound storage block fixed alongside this. A placeholder view + // in the shader's own declared format lets the work proceed with the stores landing + // nowhere anyone can observe, which is what GL asks for. + const VkBufferView placeholder = + AcquireUnboundTexelBufferView(programObj.storageImageFormatByBinding[binding], + programObj.samplerNumericDomainByBinding[binding], true); + if (placeholder == VK_NULL_HANDLE) { + MGLOG_E_ONCE("ResolveStorageTexelBufferDescriptor: image unit %d is unbound for binding %u, and the " + "placeholder descriptor could not be created", imageUnit, binding); + return false; + } + MGLOG_D("ResolveStorageTexelBufferDescriptor: image unit %d (binding %u) is unbound; using the " + "placeholder descriptor", imageUnit, binding); + outBufferView = placeholder; + return true; } if (texture->GetStorageType() != TextureStorageType::Buffer || texture->GetTarget() != TextureTarget::TextureBuffer) { @@ -824,9 +1024,20 @@ namespace MobileGL::MG_Backend::DirectVulkan { auto* textureBuffer = static_cast(texture.get()); const auto& bufferObject = textureBuffer->GetBufferBindingSlot().GetBoundObject(); if (bufferObject == nullptr) { - MGLOG_E_ONCE("ResolveStorageTexelBufferDescriptor: texture buffer on image unit %d has no GL buffer bound", - imageUnit); - return false; + // Incomplete buffer texture, same as the sampled path: legal state, undefined data, + // and no reason to drop the work. + const VkBufferView placeholder = + AcquireUnboundTexelBufferView(programObj.storageImageFormatByBinding[binding], + programObj.samplerNumericDomainByBinding[binding], true); + if (placeholder == VK_NULL_HANDLE) { + MGLOG_E_ONCE("ResolveStorageTexelBufferDescriptor: texture buffer on image unit %d has no GL buffer " + "bound, and the placeholder descriptor could not be created", imageUnit); + return false; + } + MGLOG_D("ResolveStorageTexelBufferDescriptor: texture buffer on image unit %d has no attached GL buffer; " + "using the placeholder descriptor", imageUnit); + outBufferView = placeholder; + return true; } // Unlike the sampled texel buffer, the shader MAY write this one, and those writes land @@ -852,8 +1063,6 @@ namespace MobileGL::MG_Backend::DirectVulkan { // 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]; @@ -1062,8 +1271,40 @@ namespace MobileGL::MG_Backend::DirectVulkan { auto& imageBinding = MG_State::pGLContext->GetImageTextureBinding(imageUnit); if (imageBinding.Texture == nullptr) { - MGLOG_E_ONCE("ResolveStorageImageDescriptor: image unit %d is unbound for binding %u", imageUnit, binding); - return false; + // Legal GL: an image unit with no texture bound makes loads return zero and discards + // stores (4.6 core 8.26). It is not a reason to lose the draw, which is what returning + // false here did - both SetupDraw and DispatchCompute skip everything on it. The + // placeholder is a 1x1 image of the target and format the shader's declaration asks + // for, so the descriptor is valid and the stores land where nobody can see them. + TextureTarget placeholderTarget = TextureTarget::Unknown; + VkFormat placeholderFormat = VK_FORMAT_UNDEFINED; + SharedPtr placeholder; + if (ResolveUnboundStorageImagePlaceholder(programObj, binding, placeholderTarget, placeholderFormat)) { + placeholder = GetUnboundStorageImageTexture(placeholderTarget, placeholderFormat); + } + VkImageView placeholderView = VK_NULL_HANDLE; + if (placeholder != nullptr && + m_textureManager->TransitionTextureForStorageImage(commandBuffer, *placeholder)) { + // layered=true, layer=0: the placeholder's own view type IS the one the shader's + // image declaration demands, and that is exactly what the layered form asks for + // (see GetOrCreateStorageImageView, which only narrows the view type when a + // non-layered binding names a single layer). + placeholderView = + m_textureManager->GetOrCreateStorageImageView(*placeholder, 0, placeholderFormat, true, 0); + } + if (placeholderView == VK_NULL_HANDLE) { + MGLOG_E_ONCE("ResolveStorageImageDescriptor: image unit %d is unbound for binding %u, and no " + "placeholder descriptor could be built (target=%d format=%d)", + imageUnit, binding, static_cast(placeholderTarget), + static_cast(placeholderFormat)); + return false; + } + MGLOG_D("ResolveStorageImageDescriptor: image unit %d (binding %u) is unbound; using the placeholder " + "descriptor", imageUnit, binding); + outImageInfo.sampler = VK_NULL_HANDLE; + outImageInfo.imageView = placeholderView; + outImageInfo.imageLayout = VK_IMAGE_LAYOUT_GENERAL; + return true; } const Bool ready = m_textureManager->TransitionTextureForStorageImage(commandBuffer, *imageBinding.Texture); @@ -1155,6 +1396,138 @@ namespace MobileGL::MG_Backend::DirectVulkan { return m_fallbackTexture2D; } + VkBufferView UniformManager::AcquireUnboundTexelBufferView(VkFormat declaredFormat, + SamplerNumericDomain numericDomain, Bool storage) { + MOBILEGL_ASSERT(m_bufferManager != nullptr, "AcquireUnboundTexelBufferView: buffer manager is null"); + const VkFormatFeatureFlags requiredFeature = storage ? VK_FORMAT_FEATURE_STORAGE_TEXEL_BUFFER_BIT + : VK_FORMAT_FEATURE_UNIFORM_TEXEL_BUFFER_BIT; + const VkFormat fallbackFormat = PlaceholderFormatForNumericDomain(numericDomain); + + VkFormat format = declaredFormat; + if (format == VK_FORMAT_UNDEFINED || !BufferFormatSupportsFeature(m_physicalDevice, format, requiredFeature)) { + // The declared format is what a shader that WRITES through this descriptor is + // validated against, so it is tried first and kept whenever the device can use it. + // Falling back is for the two cases where it cannot be: a sampled texel buffer, which + // declares no format at all, and a device that does not list the declared one as a + // texel buffer. The fallback stays inside the shader's numeric class, which is the + // part the descriptor is checked on for a formatless declaration - and the R32 + // members of the three classes are mandatory-support formats, so this cannot fail for + // want of device features. + format = fallbackFormat; + } + if (format == VK_FORMAT_UNDEFINED || !BufferFormatSupportsFeature(m_physicalDevice, format, requiredFeature)) { + MGLOG_E_ONCE("AcquireUnboundTexelBufferView: no usable placeholder format (declared=%d fallback=%d " + "storage=%s)", + static_cast(declaredFormat), static_cast(fallbackFormat), + storage ? "true" : "false"); + return VK_NULL_HANDLE; + } + + const Uint64 key = (static_cast(format) << 1) | (storage ? 1ull : 0ull); + const auto cached = m_unboundTexelBufferViews.find(key); + if (cached != m_unboundTexelBufferViews.end()) { + return cached->second; + } + + const BufferSlice placeholder = m_bufferManager->AcquireUnboundTexelBufferDescriptor(); + if (!placeholder.IsValid()) { + MGLOG_E_ONCE("AcquireUnboundTexelBufferView: placeholder buffer unavailable"); + return VK_NULL_HANDLE; + } + // A buffer view's range must be a whole number of texels of its own format, and the + // placeholder is sized for the largest of them - so floor rather than assume. + const VkDeviceSize texelSize = std::max(1, vkuFormatTexelBlockSize(format)); + const VkDeviceSize range = (placeholder.size / texelSize) * texelSize; + if (range == 0) { + MGLOG_E_ONCE("AcquireUnboundTexelBufferView: placeholder holds no whole texel of format=%d", + static_cast(format)); + return VK_NULL_HANDLE; + } + + VkBufferViewCreateInfo viewInfo{}; + viewInfo.sType = VK_STRUCTURE_TYPE_BUFFER_VIEW_CREATE_INFO; + viewInfo.buffer = placeholder.buffer; + viewInfo.format = format; + viewInfo.offset = placeholder.offset; + viewInfo.range = range; + + VkBufferView view = VK_NULL_HANDLE; + const VkResult result = vkCreateBufferView(m_device, &viewInfo, nullptr, &view); + if (result != VK_SUCCESS || view == VK_NULL_HANDLE) { + MGLOG_E_ONCE("AcquireUnboundTexelBufferView: vkCreateBufferView failed result=%d format=%d", result, + static_cast(format)); + return VK_NULL_HANDLE; + } + m_unboundTexelBufferViews.emplace(key, view); + MGLOG_D("AcquireUnboundTexelBufferView: created placeholder view format=%d storage=%s", + static_cast(format), storage ? "true" : "false"); + return view; + } + + Bool UniformManager::ResolveUnboundStorageImagePlaceholder(const ProgramFactory::VkProgramObject& programObj, + Uint32 binding, TextureTarget& outTarget, + VkFormat& outFormat) const { + MOBILEGL_ASSERT(binding < programObj.samplerTextureTargetByBinding.size(), + "ResolveUnboundStorageImagePlaceholder: binding %u out of range", binding); + MOBILEGL_ASSERT(binding < programObj.storageImageFormatByBinding.size(), + "ResolveUnboundStorageImagePlaceholder: format binding %u out of range", binding); + outTarget = programObj.samplerTextureTargetByBinding[binding]; + // The shader's own format qualifier, exactly as the bound path prefers it over the one + // glBindImageTexture named - there is no binding here to name one. A `writeonly` image + // may carry no qualifier at all; its numeric class is then the only constraint, and the + // R32 member of that class is what carries it (see AcquireUnboundTexelBufferView). + outFormat = programObj.storageImageFormatByBinding[binding]; + if (outFormat == VK_FORMAT_UNDEFINED) { + outFormat = PlaceholderFormatForNumericDomain(programObj.samplerNumericDomainByBinding[binding]); + } + return outFormat != VK_FORMAT_UNDEFINED && PlaceholderShapeForTarget(outTarget).valid; + } + + SharedPtr UniformManager::GetUnboundStorageImageTexture( + TextureTarget target, VkFormat format) const { + const Uint64 key = (static_cast(target) << 32) | static_cast(format); + const auto cached = m_unboundStorageImageTextures.find(key); + if (cached != m_unboundStorageImageTextures.end()) { + return cached->second; + } + + const PlaceholderShape shape = PlaceholderShapeForTarget(target); + if (!shape.valid) { + // A multisample image uniform is the case with no answer here: its descriptor demands + // a multisample view, and a single-sampled 1x1 image is invalid Vulkan in that slot, + // not a degraded picture. The caller declines the binding exactly as it did before. + MGLOG_D("GetUnboundStorageImageTexture: no placeholder shape for target=%d", static_cast(target)); + return nullptr; + } + const TextureInternalFormat internalFormat = InternalFormatForVkFormat(format); + if (internalFormat == TextureInternalFormat::Unknown) { + MGLOG_E_ONCE("GetUnboundStorageImageTexture: no GL internal format matches VkFormat=%d", + static_cast(format)); + return nullptr; + } + + auto texture = MakePlaceholderTextureObject(target, kUnboundStorageImageExternalIndex); + if (texture == nullptr) { + return nullptr; + } + texture->SetInternalFormat(internalFormat); + const SizeT texelBytes = MG_Util::GetSizedInternalFormatSizeInBytes(internalFormat); + for (Uint32 index = 0; index < shape.uploadTargetCount; ++index) { + texture->AllocateStorage(shape.uploadTargets[index], 0, + {.texelSize = {1, 1, shape.depth}, + .byteSize = texelBytes * static_cast(shape.depth)}); + // Not dirty: there is deliberately nothing to upload. The image is created and + // transitioned to GENERAL by the storage-image preparation pass like any other, and + // its contents are exactly as undefined as GL says a fetch through an unbound image + // unit is. + texture->MarkStorageDirty(shape.uploadTargets[index], 0, false); + } + m_unboundStorageImageTextures.emplace(key, texture); + MGLOG_D("GetUnboundStorageImageTexture: created placeholder target=%d format=%d", static_cast(target), + static_cast(format)); + return texture; + } + Bool UniformManager::ResolveSampledBinding(const MG_State::GLState::ProgramObject& program, const ProgramFactory::VkProgramObject& programObj, Uint32 binding, Uint32 element, @@ -1340,9 +1713,23 @@ namespace MobileGL::MG_Backend::DirectVulkan { auto* texture = MG_State::pGLContext->GetImageTextureBinding(imageUnit).Texture.get(); if (texture == nullptr) { - MGLOG_E_ONCE("CollectStorageImageTextures: image unit %d is unbound for binding %u element %u", - imageUnit, binding, element); - return false; + // ResolveStorageImageDescriptor will substitute the placeholder image for this + // binding; include it here for the same reason the sampled walk includes the + // fallback texture - this walk is what gets a storage image created, + // STORAGE-usage-marked and transitioned to GENERAL BEFORE the render pass + // opens, and all three of those are illegal once it has. A target with no + // placeholder shape (multisample) contributes nothing and is declined at + // resolve time exactly as it was. + TextureTarget placeholderTarget = TextureTarget::Unknown; + VkFormat placeholderFormat = VK_FORMAT_UNDEFINED; + if (!ResolveUnboundStorageImagePlaceholder(programObj, binding, placeholderTarget, + placeholderFormat)) { + continue; + } + texture = GetUnboundStorageImageTexture(placeholderTarget, placeholderFormat).get(); + if (texture == nullptr) { + continue; + } } if (std::find(outTextures.begin(), outTextures.end(), texture) == outTextures.end()) { outTextures.push_back(texture); diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/UniformManager.h b/MobileGL/MG_Backend/DirectVulkan/Renderer/UniformManager.h index d83c4e4f..55c494fa 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/UniformManager.h +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/UniformManager.h @@ -42,7 +42,10 @@ namespace MobileGL::MG_Backend::DirectVulkan { SamplerNumericDomain numericDomain = SamplerNumericDomain::Unknown; }; - Bool Initialize(VkDevice device, VkBufferManager* bufferManager, + // `physicalDevice` is only ever asked for format properties: a placeholder descriptor for + // an unbound texel-buffer binding has to be built from a format the DEVICE accepts as a + // texel buffer, and there is no other route to that answer from here. + Bool Initialize(VkDevice device, VkPhysicalDevice physicalDevice, VkBufferManager* bufferManager, ProgramFactory* programFactory, VkDeviceSize minUniformBufferOffsetAlignment, Uint32 frameCount, Uint32 maxBindings = 16, Uint32 setsPerFrame = 64, @@ -177,6 +180,32 @@ namespace MobileGL::MG_Backend::DirectVulkan { const MG_State::GLState::ProgramObject& program, const ProgramFactory::VkProgramObject& programObj, Uint32 binding, Uint32 element); SharedPtr GetFallbackTexture(TextureTarget target) const; + // ---- placeholders for UNBOUND image-backed descriptors ------------------------- + // GL lets a program declare `samplerBuffer`, `imageBuffer` or `image2D` and bind nothing + // to the unit it names: the fetch is then undefined (GL 4.6 core 8.9 for an incomplete + // buffer texture, 8.26 for an image unit with no texture) - undefined VALUES, not a + // dropped draw. Vulkan has no unwritten descriptor, so something valid has to sit in the + // set or the whole draw or dispatch is lost, which is what these two build. Same shape as + // VkBufferManager::AcquireUnboundStorageDescriptor, one level up: per FORMAT rather than + // one shared object, because a descriptor whose format disagrees with the shader's + // declaration is invalid Vulkan even when nothing ever reads it. + // + // `declaredFormat` is the format the SHADER declared (VK_FORMAT_UNDEFINED for a sampled + // texel buffer, which never carries one, or for a formatless `writeonly` image); + // `numericDomain` decides the format when there is no declaration and is the fallback + // class when the device cannot use the declared one as a texel buffer. + VkBufferView AcquireUnboundTexelBufferView(VkFormat declaredFormat, SamplerNumericDomain numericDomain, + Bool storage); + // A 1x1 (x1 layer, or 6 faces for a cube) texture of `format`, shaped for `target` so the + // view the descriptor gets has the view type the shader's image declaration demands. + // Null for a target with no single-sampled placeholder shape - multisample images, whose + // descriptor needs a multisample view that this cannot stand in for. + SharedPtr GetUnboundStorageImageTexture(TextureTarget target, + VkFormat format) const; + // The (target, format) pair a storage-image binding's placeholder is keyed by, resolved + // from reflection alone. False when the binding has no placeholder shape. + Bool ResolveUnboundStorageImagePlaceholder(const ProgramFactory::VkProgramObject& programObj, Uint32 binding, + TextureTarget& outTarget, VkFormat& outFormat) const; // `element` indexes a sampler ARRAY inside one binding; each element carries its own // independently assigned GL texture unit, so it selects the texture, the sampler // override and the fallback separately from its neighbours. @@ -251,6 +280,7 @@ namespace MobileGL::MG_Backend::DirectVulkan { VkDescriptorSet& outDescriptorSet); VkDevice m_device = VK_NULL_HANDLE; + VkPhysicalDevice m_physicalDevice = VK_NULL_HANDLE; VkBufferManager* m_bufferManager = nullptr; ProgramFactory* m_programFactory = nullptr; Vector m_frames; @@ -263,6 +293,15 @@ namespace MobileGL::MG_Backend::DirectVulkan { VkTextureManager* m_textureManager = nullptr; VkSamplerManager* m_samplerManager = nullptr; mutable SharedPtr m_fallbackTexture2D; + // See AcquireUnboundTexelBufferView / GetUnboundStorageImageTexture. Both are lazily + // populated, never evicted (a program's declared formats are a fixed, tiny set) and torn + // down with the manager. The texel views are keyed by format AND by storage-vs-sampled + // because the two descriptor kinds demand different format FEATURES of the device, so one + // format can be usable for one and not the other. Deliberately NOT the per-frame + // texelBufferViews list: those are destroyed at every frame boundary, and these must + // outlive it or the placeholder would be rebuilt for every unbound binding every frame. + UnorderedMap m_unboundTexelBufferViews; + mutable UnorderedMap> m_unboundStorageImageTextures; // Per-draw scratch buffers for BindProgramUniformBuffers: reused (clear keeps // capacity) so the descriptor-write path stops allocating on every draw. diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/VkBufferManager.cpp b/MobileGL/MG_Backend/DirectVulkan/Renderer/VkBufferManager.cpp index 6605b9c3..29f214d7 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VkBufferManager.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/VkBufferManager.cpp @@ -19,6 +19,11 @@ namespace MobileGL::MG_Backend::DirectVulkan { // See VkBufferManager::AcquireUnboundStorageDescriptor. 256 bytes: comfortably past // every minStorageBufferOffsetAlignment in the wild, and free. constexpr VkDeviceSize kUnboundStorageDescriptorBytes = 256; + // See VkBufferManager::AcquireUnboundTexelBufferDescriptor. The same 256 bytes, for the + // same reason plus one: a texel buffer view's range must be a whole number of texels of + // whatever format the placeholder is asked for, and 256 divides by every texel size in + // the GL image-format table (1, 2, 4, 8 and 16 bytes). + constexpr VkDeviceSize kUnboundTexelBufferDescriptorBytes = 256; // A zero-copy persistent buffer is created once and never recreated (the app holds // its mapped pointer), and may be bound to any role, so it carries every usage. @@ -135,6 +140,7 @@ namespace MobileGL::MG_Backend::DirectVulkan { } m_transientUploadArena.Shutdown(); m_unboundStorageBuffer.Destroy(); + m_unboundTexelBuffer.Destroy(); DestroyAllDeferredReleases(); ReleaseAllLiveResources(); m_copyProvider = nullptr; @@ -742,6 +748,39 @@ namespace MobileGL::MG_Backend::DirectVulkan { return m_unboundStorageBuffer.GetSlice(); } + BufferSlice VkBufferManager::AcquireUnboundTexelBufferDescriptor() { + if (!m_unboundTexelBuffer.IsValid()) { + if (m_initInfo.allocator == nullptr) { + return {}; + } + // A SECOND placeholder rather than more usage bits on the storage-block one. The two + // are independent failure domains: a device that refuses this allocation must not + // take the storage-block placeholder - and with it the fix this one is a sibling of - + // down with it. Host-visible and zero-filled for the same reason as that one: this is + // reached from descriptor resolution, inside an already-open recording, which must + // not start a copy of its own. + const Bool created = m_unboundTexelBuffer.Create({ + .allocator = m_initInfo.allocator, + .size = kUnboundTexelBufferDescriptorBytes, + .usage = VK_BUFFER_USAGE_UNIFORM_TEXEL_BUFFER_BIT | VK_BUFFER_USAGE_STORAGE_TEXEL_BUFFER_BIT | + VK_BUFFER_USAGE_TRANSFER_DST_BIT, + .memoryUsage = VMA_MEMORY_USAGE_AUTO, + .allocationFlags = VMA_ALLOCATION_CREATE_HOST_ACCESS_SEQUENTIAL_WRITE_BIT | + VMA_ALLOCATION_CREATE_MAPPED_BIT, + .requiredFlags = VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT | VK_MEMORY_PROPERTY_HOST_COHERENT_BIT, + }); + if (!created) { + MGLOG_E_ONCE("VkBufferManager::AcquireUnboundTexelBufferDescriptor: placeholder creation failed"); + m_unboundTexelBuffer.Destroy(); + return {}; + } + if (void* mapped = m_unboundTexelBuffer.GetMappedData()) { + Memset(mapped, 0, static_cast(kUnboundTexelBufferDescriptorBytes)); + } + } + return m_unboundTexelBuffer.GetSlice(); + } + VkBufferUsageFlags VkBufferManager::GetVkBufferUsage(BufferKind kind) { switch (kind) { case BufferKind::Vertex: diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/VkBufferManager.h b/MobileGL/MG_Backend/DirectVulkan/Renderer/VkBufferManager.h index 6ca02a01..3f08db64 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VkBufferManager.h +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/VkBufferManager.h @@ -131,6 +131,16 @@ namespace MobileGL::MG_Backend::DirectVulkan { // that indexes past it. BufferSlice AcquireUnboundStorageDescriptor(); + // The store a texel-buffer descriptor - `samplerBuffer` or `imageBuffer` - gets when the + // unit the program's uniform names has no buffer texture on it, or the buffer texture on + // it has no GL buffer attached. Both are legal GL states that make a fetch return + // undefined values (GL 4.6 core 8.9: a buffer texture with no attached buffer object is + // incomplete, and sampling an incomplete texture is undefined - not a lost draw), and both + // used to take the whole draw or dispatch with them. The VIEW over this - one per format, + // and the descriptor is a VkBufferView, not a buffer - is built by + // UniformManager::AcquireUnboundTexelBufferView. + BufferSlice AcquireUnboundTexelBufferDescriptor(); + // Draw-time acquire for resident (device-storage) buffers: ensures the // resource exists and is fully uploaded, marks it used this frame. Bool AcquireResidentSlice(BufferKind kind, const SharedPtr& bufferObject, @@ -194,6 +204,8 @@ namespace MobileGL::MG_Backend::DirectVulkan { // See AcquireUnboundStorageDescriptor. Lazily created, never re-created, torn down // with the manager. VkBufferObject m_unboundStorageBuffer; + // See AcquireUnboundTexelBufferDescriptor. Same lifetime rules. + VkBufferObject m_unboundTexelBuffer; IBufferCopyCommandProvider* m_copyProvider = nullptr; Vector> m_deferredBufferReleases; Vector>> m_deferredResourceReleases; diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp index c7c4fe58..12e499dd 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp @@ -3136,7 +3136,7 @@ void main() { m_uniformManager = MakeUnique(); MOBILEGL_ASSERT(m_uniformManager != nullptr, "UniformDescriptorBinder creation failed."); succeeded = m_uniformManager->Initialize( - m_device, &m_bufferManager, m_programFactory.get(), + m_device, m_physicalDevice.handle, &m_bufferManager, m_programFactory.get(), m_physicalDevice.properties.limits.minUniformBufferOffsetAlignment, m_config.MaxFramesInFlight, maxProgramBindings, kDescriptorSetsPerFrame, m_textureManager.get(), m_samplerManager.get()); MOBILEGL_ASSERT(succeeded, "UniformDescriptorBinder initialization failed."); diff --git a/MobileGL/MG_IntegrationTest/CMakeLists.txt b/MobileGL/MG_IntegrationTest/CMakeLists.txt index e47ba4d2..d40eec92 100644 --- a/MobileGL/MG_IntegrationTest/CMakeLists.txt +++ b/MobileGL/MG_IntegrationTest/CMakeLists.txt @@ -107,6 +107,7 @@ add_executable(MobileGLIntegrationTest Scenarios/StorageBufferRegrowScenario.cpp Scenarios/RelinkStageSetScenario.cpp Scenarios/GuiBatchScenario.cpp + Scenarios/UnboundImageDescriptorScenario.cpp ) target_include_directories(MobileGLIntegrationTest PRIVATE diff --git a/MobileGL/MG_IntegrationTest/Scenarios/UnboundImageDescriptorScenario.cpp b/MobileGL/MG_IntegrationTest/Scenarios/UnboundImageDescriptorScenario.cpp new file mode 100644 index 00000000..aa2fa4f9 --- /dev/null +++ b/MobileGL/MG_IntegrationTest/Scenarios/UnboundImageDescriptorScenario.cpp @@ -0,0 +1,380 @@ +// MobileGL - MobileGL/MG_IntegrationTest/Scenarios/UnboundImageDescriptorScenario.cpp +// Copyright (c) 2026 MobileGL-Dev +// Licensed under the GNU Lesser General Public License v3.0: +// https://www.gnu.org/licenses/gpl-3.0.txt +// https://www.gnu.org/licenses/lgpl-3.0.txt +// SPDX-License-Identifier: LGPL-3.0-only +// End of Source File Header +// +// Scenario - A PROGRAM DECLARES AN IMAGE-BACKED RESOURCE AND THE APPLICATION BINDS NOTHING. +// +// The sibling of GuiBatchScenario's MeshesBlockLeftUnbound, one descriptor kind further out. +// That one pinned an unbound shader storage BLOCK; the same "nothing is bound, so lose the +// whole draw" shape survived in the three image-backed kinds: +// +// * `samplerBuffer` - a texture unit with no buffer texture on it, and a buffer texture with +// no GL buffer attached to it. Both make the sampler INCOMPLETE (GL 4.6 +// core 8.9, 8.24), and sampling an incomplete texture returns undefined +// VALUES. It is not an error and it is not a lost draw. +// * `imageBuffer` - an image unit with nothing on it. GL 4.6 core 8.26 is explicit: loads +// return zero and stores are discarded. +// * `image2D` - the same rule, through a VkImageView rather than a VkBufferView. +// +// Vulkan has no such thing as an unwritten descriptor, so DirectVulkan's descriptor resolution +// used to answer "no valid descriptor" and both SetupDraw and DispatchCompute skip everything on +// that answer - the draw or dispatch simply never happened, silently. Every test below asserts +// on the OTHER work in the same shader: the pixels the fragment stage painted, or the buffer the +// dispatch filled. All of it is unrelated to the unbound resource and all of it disappeared. +// +// The unbound resource is STATICALLY USED in every case, because an unreferenced one is +// optimised out before it ever reaches a descriptor and would prove nothing. Where the use is a +// read it sits behind a uniform-controlled branch that is false at runtime - the descriptor is +// declared and must be written, but no undefined value reaches an assertion. Where it is a write +// (the `writeonly` cases, which is how the real workloads spell it) it is unconditional: GL says +// the store is discarded, so there is nothing to guard against. +// +// Reproduces on DirectVulkan only. DirectGLES forwards the unbound unit to the GLES driver, +// which does what GL says, so it is the control - every test here must stay green on both. + +#include +#include +#include + +#include "../Harness/HeadlessGL.h" +#include "../Harness/ScenarioFixture.h" + +#ifdef GLAPI +#undef GLAPI +#endif +#define GL_GLEXT_PROTOTYPES +#include +#include +#undef GL_GLEXT_PROTOTYPES + +namespace MGITest { + namespace { + + constexpr int kFboSize = 32; + constexpr int kElements = 4; + + // No vertex attributes: the quad's corners come from gl_VertexID, so nothing about the + // vertex fetch can be confused with the descriptor question under test. + constexpr const char* kQuadVertexSource = R"(#version 430 core +void main() { + vec2 corner = vec2((gl_VertexID & 1) == 0 ? -1.0 : 1.0, + (gl_VertexID & 2) == 0 ? -1.0 : 1.0); + gl_Position = vec4(corner, 0.0, 1.0); +} +)"; + + // The assertion in every draw case: opaque green everywhere. The unbound resource + // contributes nothing to it - u_readUnbound is 0, so the fetch never runs - but the + // descriptor for it still has to exist, which is the point. + constexpr const char* kSamplerBufferFragmentSource = R"(#version 430 core +uniform samplerBuffer u_unbound; +uniform int u_readUnbound; +out vec4 o_color; +void main() { + vec4 color = vec4(0.0, 1.0, 0.0, 1.0); + if (u_readUnbound != 0) { + color = texelFetch(u_unbound, 0); + } + o_color = color; +} +)"; + + constexpr const char* kSamplerBufferComputeSource = R"(#version 430 core +layout(local_size_x = 1) in; +layout(std430, binding = 0) buffer Output { uint g_data[]; }; +uniform samplerBuffer u_unbound; +uniform int u_readUnbound; +void main() { + uint index = gl_GlobalInvocationID.x; + uint value = index + 1u; + if (u_readUnbound != 0) { + value += uint(texelFetch(u_unbound, 0).r); + } + g_data[index] = value; +} +)"; + + // writeonly, and the store is unconditional: this is how AcceleratedRendering and the + // conformance cases spell an image the shader only produces into. GL discards the store + // when the unit is empty; nothing here reads it back. + constexpr const char* kImageBufferFragmentSource = R"(#version 430 core +layout(binding = 0, r32ui) uniform writeonly uimageBuffer u_unbound; +out vec4 o_color; +void main() { + imageStore(u_unbound, 0, uvec4(7u)); + o_color = vec4(0.0, 1.0, 0.0, 1.0); +} +)"; + + constexpr const char* kImageBufferComputeSource = R"(#version 430 core +layout(local_size_x = 1) in; +layout(std430, binding = 0) buffer Output { uint g_data[]; }; +layout(binding = 0, r32ui) uniform writeonly uimageBuffer u_unbound; +void main() { + uint index = gl_GlobalInvocationID.x; + imageStore(u_unbound, int(index), uvec4(7u)); + g_data[index] = index + 1u; +} +)"; + + constexpr const char* kImage2DFragmentSource = R"(#version 430 core +layout(binding = 0, rgba8) uniform writeonly image2D u_unbound; +out vec4 o_color; +void main() { + imageStore(u_unbound, ivec2(0, 0), vec4(1.0)); + o_color = vec4(0.0, 1.0, 0.0, 1.0); +} +)"; + + constexpr const char* kImage2DComputeSource = R"(#version 430 core +layout(local_size_x = 1) in; +layout(std430, binding = 0) buffer Output { uint g_data[]; }; +layout(binding = 0, rgba8) uniform writeonly image2D u_unbound; +void main() { + uint index = gl_GlobalInvocationID.x; + imageStore(u_unbound, ivec2(int(index), 0), vec4(1.0)); + g_data[index] = index + 1u; +} +)"; + + // No layout format at all, which GLSL 4.20 allows for a write-only image. The reflection + // then carries NO format for the binding, so the placeholder descriptor can only be + // constrained by the declaration's numeric class - a different route through the fix than + // every typed case above. + constexpr const char* kFormatlessImage2DComputeSource = R"(#version 430 core +layout(local_size_x = 1) in; +layout(std430, binding = 0) buffer Output { uint g_data[]; }; +layout(binding = 0) uniform writeonly image2D u_unbound; +void main() { + uint index = gl_GlobalInvocationID.x; + imageStore(u_unbound, ivec2(int(index), 0), vec4(1.0)); + g_data[index] = index + 1u; +} +)"; + + class UnboundImageDescriptorScenario : public ScenarioTest { + protected: + void SetUp() override { + ScenarioTest::SetUp(); + if (!Ready()) return; + m_target = MakeColorFbo(kFboSize, kFboSize); + ASSERT_NE(m_target.fbo, 0u) << "could not create the render target"; + glGenVertexArrays(1, &m_vao); + glGenBuffers(1, &m_storage); + // The harness shares one context across every scenario in the process, so an + // earlier one may well have left a texture on unit 0 or an image on unit 0. The + // whole subject here is that nothing is bound, so say so rather than assume it. + glActiveTexture(GL_TEXTURE0); + glBindTexture(GL_TEXTURE_BUFFER, 0); + glBindTexture(GL_TEXTURE_2D, 0); + glBindImageTexture(0, 0, 0, GL_FALSE, 0, GL_READ_WRITE, GL_RGBA8); + FirstGLError(); + } + + void TearDown() override { + if (!Ready()) return; + glUseProgram(0); + glBindBufferBase(GL_SHADER_STORAGE_BUFFER, 0, 0); + if (m_program != 0) glDeleteProgram(m_program); + if (m_storage != 0) glDeleteBuffers(1, &m_storage); + if (m_vao != 0) glDeleteVertexArrays(1, &m_vao); + BindDefaultFramebuffer(); + DestroyColorFbo(m_target); + glViewport(0, 0, Gl().Width(), Gl().Height()); + } + + // Every case needs image or buffer-texture uniforms in a particular stage, and a host + // that has none of them would report a failure that is about the host, not the fix. + bool StageSupports(GLenum imageUniformLimit, GLenum textureImageUnitLimit) const { + GLint images = 0; + GLint units = 0; + glGetIntegerv(imageUniformLimit, &images); + glGetIntegerv(textureImageUnitLimit, &units); + while (glGetError() != GL_NO_ERROR) { + } + return images >= 1 && units >= 1; + } + + unsigned int MakeComputeProgram(const char* source) { + const GLuint shader = glCreateShader(GL_COMPUTE_SHADER); + glShaderSource(shader, 1, &source, nullptr); + glCompileShader(shader); + GLint compiled = GL_FALSE; + glGetShaderiv(shader, GL_COMPILE_STATUS, &compiled); + if (compiled == GL_FALSE) { + char log[4096] = {}; + glGetShaderInfoLog(shader, sizeof(log) - 1, nullptr, log); + ADD_FAILURE() << "the compute shader did not compile: " << log; + glDeleteShader(shader); + return 0; + } + const GLuint program = glCreateProgram(); + glAttachShader(program, shader); + glLinkProgram(program); + glDeleteShader(shader); + GLint linked = GL_FALSE; + glGetProgramiv(program, GL_LINK_STATUS, &linked); + if (linked == GL_FALSE) { + char log[4096] = {}; + glGetProgramInfoLog(program, sizeof(log) - 1, nullptr, log); + ADD_FAILURE() << "the compute program did not link: " << log; + glDeleteProgram(program); + return 0; + } + return program; + } + + // Fills a four-element SSBO with 1..4 while the unbound resource is declared and + // statically used. Zeros everywhere mean the dispatch never ran. + void ExpectDispatchStillRuns(const char* source, const char* what) { + m_program = MakeComputeProgram(source); + ASSERT_NE(m_program, 0u); + + const std::vector zeros(static_cast(kElements), 0u); + glBindBuffer(GL_SHADER_STORAGE_BUFFER, m_storage); + glBufferData(GL_SHADER_STORAGE_BUFFER, + static_cast(zeros.size() * sizeof(unsigned int)), zeros.data(), + GL_DYNAMIC_COPY); + glBindBufferBase(GL_SHADER_STORAGE_BUFFER, 0, m_storage); + ASSERT_EQ(FirstGLError(), 0u) << "setting up the output buffer raised a GL error"; + + glUseProgram(m_program); + const GLint readUnbound = glGetUniformLocation(m_program, "u_readUnbound"); + if (readUnbound != -1) { + glUniform1i(readUnbound, 0); + } + glDispatchCompute(kElements, 1, 1); + glMemoryBarrier(GL_BUFFER_UPDATE_BARRIER_BIT); + EXPECT_EQ(FirstGLError(), 0u) << "the dispatch raised a GL error (" << what << ")"; + + std::vector values(static_cast(kElements), 0xDEADBEEFu); + glBindBuffer(GL_SHADER_STORAGE_BUFFER, m_storage); + glGetBufferSubData(GL_SHADER_STORAGE_BUFFER, 0, + static_cast(values.size() * sizeof(unsigned int)), values.data()); + for (int i = 0; i < kElements; ++i) { + EXPECT_EQ(values[static_cast(i)], static_cast(i + 1)) + << "element " << i << " came back as " << values[static_cast(i)] + << "; zero everywhere means the whole dispatch was dropped over the unbound " << what; + } + } + + // Paints the whole render target green while the unbound resource is declared and + // statically used. A black target means the draw never happened. + void ExpectDrawStillRuns(const char* fragmentSource, const char* what) { + std::string error; + m_program = CompileProgram(kQuadVertexSource, fragmentSource, &error); + ASSERT_NE(m_program, 0u) << error; + + BindFbo(m_target); + ClearTo(0.0f, 0.0f, 0.0f, 1.0f); + glBindVertexArray(m_vao); + glUseProgram(m_program); + const GLint readUnbound = glGetUniformLocation(m_program, "u_readUnbound"); + if (readUnbound != -1) { + glUniform1i(readUnbound, 0); + } + glDrawArrays(GL_TRIANGLE_STRIP, 0, 4); + glBindVertexArray(0); + EXPECT_EQ(FirstGLError(), 0u) << "the draw raised a GL error (" << what << ")"; + + const Image image = ReadPixels(kFboSize, kFboSize); + ASSERT_FALSE(image.Empty()) << "the readback came back empty"; + // Whole-region, not a centre pixel: the quad covers the target exactly, so + // anything short of all of it is a failure worth naming. + EXPECT_TRUE(RegionIsMostly(image, 0, kFboSize - 1, 0, kFboSize - 1, "green", 0.0, + std::string("the quad drawn with an unbound ") + what)) + << "an all-black target means the draw was dropped over the unbound " << what; + } + + ColorFbo m_target{}; + GLuint m_vao = 0; + GLuint m_storage = 0; + unsigned int m_program = 0; + }; + + } // namespace + + // ---- uniform samplerBuffer (VK_DESCRIPTOR_TYPE_UNIFORM_TEXEL_BUFFER) -------------------- + + TEST_F(UnboundImageDescriptorScenario, ADeclaredButUnboundSamplerBufferDoesNotLoseTheDispatch) { + if (!Ready() || IsSkipped()) return; + if (!StageSupports(GL_MAX_COMPUTE_IMAGE_UNIFORMS, GL_MAX_COMPUTE_TEXTURE_IMAGE_UNITS)) { + GTEST_SKIP() << "the compute stage has no texture image units"; + } + ExpectDispatchStillRuns(kSamplerBufferComputeSource, "samplerBuffer"); + } + + TEST_F(UnboundImageDescriptorScenario, ADeclaredButUnboundSamplerBufferDoesNotLoseTheDraw) { + if (!Ready() || IsSkipped()) return; + ExpectDrawStillRuns(kSamplerBufferFragmentSource, "samplerBuffer"); + } + + // The other way a texel-buffer descriptor comes out empty: the unit HAS a buffer texture, but + // no glTexBuffer ever attached a buffer object to it. GL calls that texture incomplete, which + // is undefined data and not a lost draw - a separate site in the resolve from the one above, + // and it used to return false too. + TEST_F(UnboundImageDescriptorScenario, ABufferTextureWithNoAttachedBufferDoesNotLoseTheDraw) { + if (!Ready() || IsSkipped()) return; + + GLuint texture = 0; + glGenTextures(1, &texture); + glActiveTexture(GL_TEXTURE0); + glBindTexture(GL_TEXTURE_BUFFER, texture); + // Deliberately no glTexBuffer: the texture exists and is bound, and has no store. + ASSERT_EQ(FirstGLError(), 0u) << "binding an empty buffer texture raised a GL error"; + + ExpectDrawStillRuns(kSamplerBufferFragmentSource, "buffer texture with no attached buffer"); + + glBindTexture(GL_TEXTURE_BUFFER, 0); + glDeleteTextures(1, &texture); + } + + // ---- writeonly imageBuffer (VK_DESCRIPTOR_TYPE_STORAGE_TEXEL_BUFFER) -------------------- + + TEST_F(UnboundImageDescriptorScenario, AWriteonlyImageBufferLeftUnboundDoesNotLoseTheDispatch) { + if (!Ready() || IsSkipped()) return; + if (!StageSupports(GL_MAX_COMPUTE_IMAGE_UNIFORMS, GL_MAX_COMPUTE_TEXTURE_IMAGE_UNITS)) { + GTEST_SKIP() << "the compute stage has no image uniforms"; + } + ExpectDispatchStillRuns(kImageBufferComputeSource, "imageBuffer"); + } + + TEST_F(UnboundImageDescriptorScenario, AWriteonlyImageBufferLeftUnboundDoesNotLoseTheDraw) { + if (!Ready() || IsSkipped()) return; + if (!StageSupports(GL_MAX_FRAGMENT_IMAGE_UNIFORMS, GL_MAX_TEXTURE_IMAGE_UNITS)) { + GTEST_SKIP() << "the fragment stage has no image uniforms"; + } + ExpectDrawStillRuns(kImageBufferFragmentSource, "imageBuffer"); + } + + // ---- writeonly image2D (VK_DESCRIPTOR_TYPE_STORAGE_IMAGE) ------------------------------- + + TEST_F(UnboundImageDescriptorScenario, AWriteonlyImage2DLeftUnboundDoesNotLoseTheDispatch) { + if (!Ready() || IsSkipped()) return; + if (!StageSupports(GL_MAX_COMPUTE_IMAGE_UNIFORMS, GL_MAX_COMPUTE_TEXTURE_IMAGE_UNITS)) { + GTEST_SKIP() << "the compute stage has no image uniforms"; + } + ExpectDispatchStillRuns(kImage2DComputeSource, "image2D"); + } + + TEST_F(UnboundImageDescriptorScenario, AWriteonlyImage2DLeftUnboundDoesNotLoseTheDraw) { + if (!Ready() || IsSkipped()) return; + if (!StageSupports(GL_MAX_FRAGMENT_IMAGE_UNIFORMS, GL_MAX_TEXTURE_IMAGE_UNITS)) { + GTEST_SKIP() << "the fragment stage has no image uniforms"; + } + ExpectDrawStillRuns(kImage2DFragmentSource, "image2D"); + } + + TEST_F(UnboundImageDescriptorScenario, AFormatlessWriteonlyImage2DLeftUnboundDoesNotLoseTheDispatch) { + if (!Ready() || IsSkipped()) return; + if (!StageSupports(GL_MAX_COMPUTE_IMAGE_UNIFORMS, GL_MAX_COMPUTE_TEXTURE_IMAGE_UNITS)) { + GTEST_SKIP() << "the compute stage has no image uniforms"; + } + ExpectDispatchStillRuns(kFormatlessImage2DComputeSource, "format-less image2D"); + } + +} // namespace MGITest