Merge pull request #2125 from billhollings/fix-img-mem-bind-mapping

Fix potential crash when using multi-planar images.
KhronosGroup · Jan 11, 2024 · 41ed2be · 41ed2be
2 parents 66f6ff1 + 94e86a3
commit 41ed2be
Show file tree

Hide file tree

Showing 4 changed files with 32 additions and 9 deletions.
diff --git 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
@@ -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
@@ -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<MVKImageMemoryBinding*, 3> _memoryBindings;
     MVKSmallVector<MVKImagePlane*, 3> _planes;

diff --git 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<uint8_t>(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) {