From 94e86a32bddf7bc35995287283ba4648b14e0c7c Mon Sep 17 00:00:00 2001 From: Bill Hollings Date: Thu, 11 Jan 2024 11:37:50 -0500 Subject: [PATCH] Fix potential crash when using multi-planar images. - MVKImage add functions to consolidate mapping plane indexes to memory bindings to add safety in situation where one memory binding supports all planes. - Update MoltenVK version to 1.2.8 (unrelated). --- Docs/Whats_New.md | 9 ++++++++ MoltenVK/MoltenVK/API/mvk_private_api.h | 2 +- MoltenVK/MoltenVK/GPUObjects/MVKImage.h | 3 +++ MoltenVK/MoltenVK/GPUObjects/MVKImage.mm | 27 +++++++++++++++++------- 4 files changed, 32 insertions(+), 9 deletions(-) diff --git a/Docs/Whats_New.md b/Docs/Whats_New.md index 06b098876..30b85f2f0 100644 --- a/Docs/Whats_New.md +++ b/Docs/Whats_New.md @@ -13,6 +13,15 @@ Copyright (c) 2015-2024 [The Brenwill Workshop Ltd.](http://www.brenwill.com) +MoltenVK 1.2.8 +-------------- + +Released TBD + +- Fix potential crash when using multi-planar images. + + + MoltenVK 1.2.7 -------------- diff --git a/MoltenVK/MoltenVK/API/mvk_private_api.h b/MoltenVK/MoltenVK/API/mvk_private_api.h index 522a41c29..38ec2f3d1 100644 --- a/MoltenVK/MoltenVK/API/mvk_private_api.h +++ b/MoltenVK/MoltenVK/API/mvk_private_api.h @@ -64,7 +64,7 @@ typedef unsigned long MTLArgumentBuffersTier; */ #define MVK_VERSION_MAJOR 1 #define MVK_VERSION_MINOR 2 -#define MVK_VERSION_PATCH 7 +#define MVK_VERSION_PATCH 8 #define MVK_MAKE_VERSION(major, minor, patch) (((major) * 10000) + ((minor) * 100) + (patch)) #define MVK_VERSION MVK_MAKE_VERSION(MVK_VERSION_MAJOR, MVK_VERSION_MINOR, MVK_VERSION_PATCH) diff --git a/MoltenVK/MoltenVK/GPUObjects/MVKImage.h b/MoltenVK/MoltenVK/GPUObjects/MVKImage.h index 3ae974d24..9cc6a2820 100644 --- a/MoltenVK/MoltenVK/GPUObjects/MVKImage.h +++ b/MoltenVK/MoltenVK/GPUObjects/MVKImage.h @@ -345,6 +345,9 @@ class MVKImage : public MVKVulkanAPIDeviceObject { bool getIsValidViewFormat(VkFormat viewFormat); VkImageUsageFlags getCombinedUsage() { return _usage | _stencilUsage; } MTLTextureUsage getMTLTextureUsage(MTLPixelFormat mtlPixFmt); + uint8_t getMemoryBindingCount() const { return (uint8_t)_memoryBindings.size(); } + uint8_t getMemoryBindingIndex(uint8_t planeIndex) const; + MVKImageMemoryBinding* getMemoryBinding(uint8_t planeIndex); MVKSmallVector _memoryBindings; MVKSmallVector _planes; diff --git a/MoltenVK/MoltenVK/GPUObjects/MVKImage.mm b/MoltenVK/MoltenVK/GPUObjects/MVKImage.mm index b1ad70b0b..6fb3be5bc 100644 --- a/MoltenVK/MoltenVK/GPUObjects/MVKImage.mm +++ b/MoltenVK/MoltenVK/GPUObjects/MVKImage.mm @@ -145,7 +145,7 @@ subRez.layoutState = pCreateInfo->initialLayout; VkDeviceSize offset = 0; - if (_planeIndex > 0 && _image->_memoryBindings.size() == 1) { + if (_planeIndex > 0 && _image->getMemoryBindingCount() == 1) { if (!_image->_isLinear && !_image->_isLinearForAtomics && _image->getDevice()->_pMetalFeatures->placementHeaps) { // For textures allocated directly on the heap, we need to obey the size and alignment // requirements reported by the device. @@ -303,7 +303,7 @@ } MVKImageMemoryBinding* MVKImagePlane::getMemoryBinding() const { - return (_image->_memoryBindings.size() > 1) ? _image->_memoryBindings[_planeIndex] : _image->_memoryBindings[0]; + return _image->getMemoryBinding(_planeIndex); } void MVKImagePlane::applyImageMemoryBarrier(MVKPipelineBarrier& barrier, @@ -522,12 +522,14 @@ return VK_SUCCESS; } +// If I am the only memory binding, I cover all planes. uint8_t MVKImageMemoryBinding::beginPlaneIndex() const { - return (_image->_memoryBindings.size() > 1) ? _planeIndex : 0; + return (_image->getMemoryBindingCount() > 1) ? _planeIndex : 0; } +// If I am the only memory binding, I cover all planes. uint8_t MVKImageMemoryBinding::endPlaneIndex() const { - return (_image->_memoryBindings.size() > 1) ? _planeIndex : (uint8_t)_image->_memoryBindings.size(); + return (_image->getMemoryBindingCount() > 1) ? _planeIndex + 1 : (uint8_t)_image->_planes.size(); } MVKImageMemoryBinding::MVKImageMemoryBinding(MVKDevice* device, MVKImage* image, uint8_t planeIndex) : MVKResource(device), _image(image), _planeIndex(planeIndex) { @@ -554,7 +556,7 @@ } void MVKImage::flushToDevice(VkDeviceSize offset, VkDeviceSize size) { - for (int bindingIndex = 0; bindingIndex < _memoryBindings.size(); bindingIndex++) { + for (int bindingIndex = 0; bindingIndex < getMemoryBindingCount(); bindingIndex++) { MVKImageMemoryBinding *binding = _memoryBindings[bindingIndex]; binding->flushToDevice(offset, size); } @@ -621,6 +623,15 @@ #pragma mark Resource memory +// There may be less memory bindings than planes, but there will always be at least one. +uint8_t MVKImage::getMemoryBindingIndex(uint8_t planeIndex) const { + return std::min(planeIndex, getMemoryBindingCount() - 1); +} + +MVKImageMemoryBinding* MVKImage::getMemoryBinding(uint8_t planeIndex) { + return _memoryBindings[getMemoryBindingIndex(planeIndex)]; +} + void MVKImage::applyImageMemoryBarrier(MVKPipelineBarrier& barrier, MVKCommandEncoder* cmdEncoder, MVKCommandUse cmdUse) { @@ -650,7 +661,7 @@ mvkDisableFlags(pMemoryRequirements->memoryTypeBits, getPhysicalDevice()->getLazilyAllocatedMemoryTypes()); } - return _memoryBindings[planeIndex]->getMemoryRequirements(pMemoryRequirements); + return getMemoryBinding(planeIndex)->getMemoryRequirements(pMemoryRequirements); } VkResult MVKImage::getMemoryRequirements(const void* pInfo, VkMemoryRequirements2* pMemoryRequirements) { @@ -669,11 +680,11 @@ } VkResult rslt = getMemoryRequirements(&pMemoryRequirements->memoryRequirements, planeIndex); if (rslt != VK_SUCCESS) { return rslt; } - return _memoryBindings[planeIndex]->getMemoryRequirements(pInfo, pMemoryRequirements); + return getMemoryBinding(planeIndex)->getMemoryRequirements(pInfo, pMemoryRequirements); } VkResult MVKImage::bindDeviceMemory(MVKDeviceMemory* mvkMem, VkDeviceSize memOffset, uint8_t planeIndex) { - return _memoryBindings[planeIndex]->bindDeviceMemory(mvkMem, memOffset); + return getMemoryBinding(planeIndex)->bindDeviceMemory(mvkMem, memOffset); } VkResult MVKImage::bindDeviceMemory2(const VkBindImageMemoryInfo* pBindInfo) {