From 8961682137e3db280100bb7b2b2645153610e560 Mon Sep 17 00:00:00 2001 From: osy Date: Wed, 26 Nov 2025 14:31:02 -0800 Subject: [PATCH] Fix buffer and heap out-of-sync in initExternalMemory() If a buffer is created before a heap in MVKDeviceMemory, the buffer would be used in some places (for example `MVKDeviceMemory::map` and the heap would be used in other places (for example `MVKBuffer::getMTLBuffer()`). This leads to inconsistent memory mappings. Specifically, there are two cases when this happens: 1. When `vkAllocateMemory` is called with an imported buffer. 2. When `vkAllocateMemory` expects `HANDLE_TYPE_MTLBUFFER` to be exported. In either case, `ensureMTLBuffer()` gets called before `ensureMTLHeap()` which sets `_mtlBuffer` to a buffer not allocated from the placement heap. This change fixes this by making sure that if a placement heap is needed, we will always create it prior to allocating the buffer. In cases where the heap is not used at all (imported buffer), we will return error if the user tries to specify a `HANDLE_TYPE_MTLHEAP` for export. --- .../MoltenVK/GPUObjects/MVKDeviceMemory.h | 2 +- .../MoltenVK/GPUObjects/MVKDeviceMemory.mm | 27 ++++++++++++++----- 2 files changed, 22 insertions(+), 7 deletions(-) diff --git a/MoltenVK/MoltenVK/GPUObjects/MVKDeviceMemory.h b/MoltenVK/MoltenVK/GPUObjects/MVKDeviceMemory.h index 88f29fcad..7451cc633 100644 --- a/MoltenVK/MoltenVK/GPUObjects/MVKDeviceMemory.h +++ b/MoltenVK/MoltenVK/GPUObjects/MVKDeviceMemory.h @@ -160,7 +160,7 @@ class MVKDeviceMemory : public MVKVulkanAPIDeviceObject { bool ensureHostMemory(); void freeHostMemory(); MVKResource* getDedicatedResource(); - void initExternalMemory(MVKImage* dedicatedImage); + void initExternalMemory(MVKImage* dedicatedImage, bool wantsHeap); MVKSmallVector _buffers; MVKSmallVector _imageMemoryBindings; diff --git a/MoltenVK/MoltenVK/GPUObjects/MVKDeviceMemory.mm b/MoltenVK/MoltenVK/GPUObjects/MVKDeviceMemory.mm index 27d634fbe..72497fa34 100644 --- a/MoltenVK/MoltenVK/GPUObjects/MVKDeviceMemory.mm +++ b/MoltenVK/MoltenVK/GPUObjects/MVKDeviceMemory.mm @@ -196,6 +196,9 @@ // Can't create a MTLHeap on imported memory if (_isHostMemImported) { return true; } + // Can't create a MTLHeap if we already have a _mtlBuffer + if (_mtlBuffer) { return true; } + // Don't bother if we don't have placement heaps. if (!getMetalFeatures().placementHeaps) { return true; } @@ -311,6 +314,7 @@ _allocationSize = pAllocateInfo->allocationSize; bool willExportMTLBuffer = false; + bool wantsHeap = true; MVKImage* dedicatedImage = nullptr; VkBuffer dedicatedBuffer = VK_NULL_HANDLE; for (const auto* next = (const VkBaseInStructure*)pAllocateInfo->pNext; next; next = next->pNext) { @@ -356,6 +360,7 @@ _mtlStorageMode = _mtlBuffer.storageMode; _mtlCPUCacheMode = _mtlBuffer.cpuCacheMode; _allocationSize = _mtlBuffer.length; + wantsHeap = false; break; } case VK_STRUCTURE_TYPE_EXPORT_METAL_OBJECT_CREATE_INFO_EXT: { @@ -388,6 +393,7 @@ _mtlCPUCacheMode = _mtlBuffer.cpuCacheMode; _allocationSize = _mtlBuffer.length; _pMemory = isMemoryHostAccessible() ? _mtlBuffer.contents : nullptr; + wantsHeap = false; } else if (pImportInfo->handleType & VK_EXTERNAL_MEMORY_HANDLE_TYPE_MTLTEXTURE_BIT_EXT) { [_mtlTexture release]; _mtlTexture = [((id)pImportInfo->handle) retain]; @@ -398,7 +404,7 @@ } } - initExternalMemory(dedicatedImage); // After setting _isDedicated + initExternalMemory(dedicatedImage, wantsHeap); // After setting _isDedicated // "Dedicated" means this memory can only be used for this image or buffer. if (dedicatedImage) { @@ -421,7 +427,7 @@ } // If we can, create a MTLHeap. This should happen before creating the buffer, allowing us to map its contents. - if (!ensureMTLHeap()) { + if (wantsHeap && !ensureMTLHeap()) { setConfigurationResult(reportError(VK_ERROR_OUT_OF_DEVICE_MEMORY, "vkAllocateMemory(): Could not allocate VkDeviceMemory of size %llu bytes.", _allocationSize)); return; } @@ -434,7 +440,7 @@ } } -void MVKDeviceMemory::initExternalMemory(MVKImage* dedicatedImage) { +void MVKDeviceMemory::initExternalMemory(MVKImage* dedicatedImage, bool wantsHeap) { if ( !_externalMemoryHandleType ) { return; } if ( !mvkIsOnlyAnyFlagEnabled(_externalMemoryHandleType, VK_EXTERNAL_MEMORY_HANDLE_TYPE_MTLBUFFER_BIT_EXT | VK_EXTERNAL_MEMORY_HANDLE_TYPE_MTLTEXTURE_BIT_EXT | VK_EXTERNAL_MEMORY_HANDLE_TYPE_MTLHEAP_BIT_EXT) ) { @@ -445,14 +451,23 @@ if (mvkIsAnyFlagEnabled(_externalMemoryHandleType, VK_EXTERNAL_MEMORY_HANDLE_TYPE_MTLHEAP_BIT_EXT)) { auto& xmProps = getPhysicalDevice()->getExternalBufferProperties(VK_EXTERNAL_MEMORY_HANDLE_TYPE_MTLHEAP_BIT_EXT); requiresDedicated = requiresDedicated || mvkIsAnyFlagEnabled(xmProps.externalMemoryFeatures, VK_EXTERNAL_MEMORY_FEATURE_DEDICATED_ONLY_BIT); - - // Make sure allocation happens at creation time since we may need to export the memory before usage - ensureMTLHeap(); + + if (wantsHeap) { + // Make sure allocation happens at creation time since we may need to export the memory before usage + ensureMTLHeap(); + } else { + setConfigurationResult(reportError(VK_ERROR_INITIALIZATION_FAILED, "vkAllocateMemory(): Cannot export MTLHeap from an imported MTLBuffer.")); + } } if (mvkIsAnyFlagEnabled(_externalMemoryHandleType, VK_EXTERNAL_MEMORY_HANDLE_TYPE_MTLBUFFER_BIT_EXT)) { auto& xmProps = getPhysicalDevice()->getExternalBufferProperties(VK_EXTERNAL_MEMORY_HANDLE_TYPE_MTLBUFFER_BIT_EXT); requiresDedicated = requiresDedicated || mvkIsAnyFlagEnabled(xmProps.externalMemoryFeatures, VK_EXTERNAL_MEMORY_FEATURE_DEDICATED_ONLY_BIT); + if (wantsHeap) { + // Make sure to initialize heap before buffer if we need one + ensureMTLHeap(); + } + // Make sure allocation happens at creation time since we may need to export the memory before usage ensureMTLBuffer(); }