Merge pull request #1494 from billhollings/fix-subpass-mtltex-mem-leak

Fix memory leak of dummy MTLTexture in render subpasses that use no attachments.

Docs/Whats_New.md

@@ -22,6 +22,7 @@ Released TBD
 - Do not use `MTLEvent` for `VkSemaphore` under *Rosetta2*.
 - Support compiling *MSL 2.4* in runtime pipelines and `MoltenVKShaderConverterTool`.
 - Fix issue where *MSL 2.3* only available on *Apple Silicon*, even on *macOS*.
+- Fix memory leak of dummy `MTLTexture` in render subpasses that use no attachments.
 - Fix Metal object retain-release errors in assignment operators.
 - Update to latest SPIRV-Cross:
 	- MSL: Add 64 bit support for `OpSwitch`.

MoltenVK/MoltenVK/Commands/MVKCmdRenderPass.mm

@@ -64,8 +64,7 @@ void MVKCmdBeginRenderPass<N_CV, N_A>::encode(MVKCommandEncoder* cmdEncoder) {
 	cmdEncoder->beginRenderpass(this,
 								_contents,
 								_renderPass,
-								_framebuffer->getExtent2D(),
+								_framebuffer,
-								_framebuffer->getLayerCount(),
 								_renderArea,
 								_clearValues.contents(),
 								_attachments.contents());

MoltenVK/MoltenVK/Commands/MVKCmdTransfer.mm

@@ -1234,7 +1234,7 @@ void MVKCmdClearAttachments<N>::encode(MVKCommandEncoder* cmdEncoder) {
 	simd::float4 vertices[vtxCnt];
 	simd::float4 clearColors[kMVKClearAttachmentCount];
 
-	VkExtent2D fbExtent = cmdEncoder->_framebufferExtent;
+	VkExtent2D fbExtent = cmdEncoder->getFramebufferExtent();
 #if MVK_MACOS_OR_IOS
 	// I need to know if the 'renderTargetWidth' and 'renderTargetHeight' properties
 	// actually do something, but [MTLRenderPassDescriptor instancesRespondToSelector: @selector(renderTargetWidth)]
@@ -1255,7 +1255,7 @@ void MVKCmdClearAttachments<N>::encode(MVKCommandEncoder* cmdEncoder) {
     // Populate the render pipeline state attachment key with info from the subpass and framebuffer.
 	_rpsKey.mtlSampleCount = mvkSampleCountFromVkSampleCountFlagBits(subpass->getSampleCount());
 	if (cmdEncoder->_canUseLayeredRendering &&
-		(cmdEncoder->_framebufferLayerCount > 1 || cmdEncoder->getSubpass()->isMultiview())) {
+		(cmdEncoder->getFramebufferLayerCount() > 1 || cmdEncoder->getSubpass()->isMultiview())) {
 		_rpsKey.enableLayeredRendering();
 	}
 

MoltenVK/MoltenVK/Commands/MVKCommandBuffer.h

@@ -283,8 +283,7 @@ public:
 	void beginRenderpass(MVKCommand* passCmd,
 						 VkSubpassContents subpassContents,
 						 MVKRenderPass* renderPass,
-						 VkExtent2D framebufferExtent,
+						 MVKFramebuffer* framebuffer,
-						 uint32_t framebufferLayerCount,
 						 VkRect2D& renderArea,
 						 MVKArrayRef<VkClearValue> clearValues,
 						 MVKArrayRef<MVKImageView*> attachments);
@@ -307,6 +306,12 @@ public:
 	/** Returns the render subpass that is currently active. */
 	MVKRenderSubpass* getSubpass();
 
+	/** The extent of current framebuffer.*/
+	VkExtent2D getFramebufferExtent();
+
+	/** The layer count of current framebuffer.*/
+	uint32_t getFramebufferLayerCount();
+
 	/** Returns the index of the currently active multiview subpass, or zero if the current render pass is not multiview. */
 	uint32_t getMultiviewPassIndex();
 
@@ -483,12 +488,6 @@ public:
 	/** Indicates whether the current draw is an indexed draw. */
 	bool _isIndexedDraw;
 
-	/** The extent of current framebuffer.*/
-	VkExtent2D _framebufferExtent;
-
-	/** The layer count of current framebuffer.*/
-	uint32_t _framebufferLayerCount;
-
 #pragma mark Construction
 
 	MVKCommandEncoder(MVKCommandBuffer* cmdBuffer);
@@ -513,6 +512,7 @@ protected:
 
 	VkSubpassContents _subpassContents;
 	MVKRenderPass* _renderPass;
+	MVKFramebuffer* _framebuffer;
 	MVKCommand* _lastMultiviewPassCmd;
 	uint32_t _renderSubpassIndex;
 	uint32_t _multiviewPassIndex;

MoltenVK/MoltenVK/Commands/MVKCommandBuffer.mm

@@ -17,6 +17,7 @@
  */
 
 #include "MVKCommandBuffer.h"
+#include "MVKFramebuffer.h"
 #include "MVKCommandPool.h"
 #include "MVKQueue.h"
 #include "MVKPipeline.h"
@@ -250,6 +251,7 @@ MVKRenderSubpass* MVKCommandBuffer::getLastMultiviewSubpass() {
 
 void MVKCommandEncoder::encode(id<MTLCommandBuffer> mtlCmdBuff,
 							   MVKCommandEncodingContext* pEncodingContext) {
+	_framebuffer = nullptr;
 	_renderPass = nullptr;
 	_subpassContents = VK_SUBPASS_CONTENTS_INLINE;
 	_renderSubpassIndex = 0;
@@ -289,17 +291,15 @@ void MVKCommandEncoder::encodeSecondary(MVKCommandBuffer* secondaryCmdBuffer) {
 void MVKCommandEncoder::beginRenderpass(MVKCommand* passCmd,
 										VkSubpassContents subpassContents,
 										MVKRenderPass* renderPass,
-										VkExtent2D framebufferExtent,
+										MVKFramebuffer* framebuffer,
-										uint32_t framebufferLayerCount,
 										VkRect2D& renderArea,
 										MVKArrayRef<VkClearValue> clearValues,
 										MVKArrayRef<MVKImageView*> attachments) {
 	_renderPass = renderPass;
-	_framebufferExtent = framebufferExtent;
+	_framebuffer = framebuffer;
-	_framebufferLayerCount = framebufferLayerCount;
 	_renderArea = renderArea;
 	_isRenderingEntireAttachment = (mvkVkOffset2DsAreEqual(_renderArea.offset, {0,0}) &&
-									mvkVkExtent2DsAreEqual(_renderArea.extent, _framebufferExtent));
+									mvkVkExtent2DsAreEqual(_renderArea.extent, getFramebufferExtent()));
 	_clearValues.assign(clearValues.begin(), clearValues.end());
 	_attachments.assign(attachments.begin(), attachments.end());
 	setSubpass(passCmd, subpassContents, 0);
@@ -346,8 +346,7 @@ void MVKCommandEncoder::beginMetalRenderPass(MVKCommandUse cmdUse) {
     MTLRenderPassDescriptor* mtlRPDesc = [MTLRenderPassDescriptor renderPassDescriptor];
 	getSubpass()->populateMTLRenderPassDescriptor(mtlRPDesc,
 												  _multiviewPassIndex,
-												  _framebufferExtent,
+												  _framebuffer,
-												  _framebufferLayerCount,
 												  _attachments.contents(),
 												  _clearValues.contents(),
 												  _isRenderingEntireAttachment,
@@ -359,7 +358,7 @@ void MVKCommandEncoder::beginMetalRenderPass(MVKCommandUse cmdUse) {
 		mtlRPDesc.visibilityResultBuffer = _pEncodingContext->visibilityResultBuffer->_mtlBuffer;
 	}
 
-	VkExtent2D fbExtent = _framebufferExtent;
+	VkExtent2D fbExtent = getFramebufferExtent();
     mtlRPDesc.renderTargetWidthMVK = max(min(_renderArea.offset.x + _renderArea.extent.width, fbExtent.width), 1u);
     mtlRPDesc.renderTargetHeightMVK = max(min(_renderArea.offset.y + _renderArea.extent.height, fbExtent.height), 1u);
     if (_canUseLayeredRendering) {
@@ -381,7 +380,7 @@ void MVKCommandEncoder::beginMetalRenderPass(MVKCommandUse cmdUse) {
             // We need to use the view count for this multiview pass.
 			renderTargetArrayLength = getSubpass()->getViewCountInMetalPass(_multiviewPassIndex);
         } else {
-			renderTargetArrayLength = _framebufferLayerCount;
+			renderTargetArrayLength = getFramebufferLayerCount();
         }
         // Metal does not allow layered render passes where some RTs are 3D and others are 2D.
         if (!(found3D && found2D) || renderTargetArrayLength > 1) {
@@ -434,6 +433,10 @@ NSString* MVKCommandEncoder::getMTLRenderCommandEncoderName(MVKCommandUse cmdUse
 	return mvkMTLRenderCommandEncoderLabel(cmdUse);
 }
 
+VkExtent2D MVKCommandEncoder::getFramebufferExtent() { return _framebuffer ? _framebuffer->getExtent2D() : VkExtent2D{0,0}; }
+
+uint32_t MVKCommandEncoder::getFramebufferLayerCount() { return _framebuffer ? _framebuffer->getLayerCount() : 0; }
+
 void MVKCommandEncoder::bindPipeline(VkPipelineBindPoint pipelineBindPoint, MVKPipeline* pipeline) {
     switch (pipelineBindPoint) {
         case VK_PIPELINE_BIND_POINT_GRAPHICS:
@@ -530,7 +533,7 @@ void MVKCommandEncoder::clearRenderArea() {
 		VkClearRect clearRect;
 		clearRect.rect = _renderArea;
 		clearRect.baseArrayLayer = 0;
-		clearRect.layerCount = _framebufferLayerCount;
+		clearRect.layerCount = getFramebufferLayerCount();
 
 		// Create and execute a temporary clear attachments command.
 		// To be threadsafe...do NOT acquire and return the command from the pool.
@@ -577,8 +580,7 @@ void MVKCommandEncoder::endRenderpass() {
 	endMetalRenderEncoding();
 
 	_renderPass = nullptr;
-	_framebufferExtent = {};
+	_framebuffer = nullptr;
-	_framebufferLayerCount = 0;
 	_attachments.clear();
 	_renderSubpassIndex = 0;
 }

MoltenVK/MoltenVK/GPUObjects/MVKFramebuffer.h

@@ -21,6 +21,9 @@
 #include "MVKDevice.h"
 #include "MVKImage.h"
 #include "MVKSmallVector.h"
+#include <mutex>
+
+class MVKRenderSubpass;
 
 
 #pragma mark MVKFramebuffer
@@ -45,16 +48,25 @@ public:
 	/** Returns the attachments.  */
 	MVKArrayRef<MVKImageView*> getAttachments() { return _attachments.contents(); }
 
+	/**
+	 * Returns a MTLTexture for use as a dummy texture when a render subpass,
+	 * that is compatible with the specified subpass, has no attachments.
+	 */
+	id<MTLTexture> getDummyAttachmentMTLTexture(MVKRenderSubpass* subpass, uint32_t passIdx);
+
 #pragma mark Construction
 
-	/** Constructs an instance for the specified device. */
 	MVKFramebuffer(MVKDevice* device, const VkFramebufferCreateInfo* pCreateInfo);
 
+	~MVKFramebuffer() override;
+
 protected:
 	void propagateDebugName() override {}
 
+	MVKSmallVector<MVKImageView*, 4> _attachments;
+	id<MTLTexture> _mtlDummyTex = nil;
+	std::mutex _lock;
 	VkExtent2D _extent;
 	uint32_t _layerCount;
-	MVKSmallVector<MVKImageView*, 4> _attachments;
 };
 

MoltenVK/MoltenVK/GPUObjects/MVKFramebuffer.mm

@@ -17,11 +17,68 @@
  */
 
 #include "MVKFramebuffer.h"
+#include "MVKRenderPass.h"
+
+using namespace std;
 
 
 #pragma mark MVKFramebuffer
 
-#pragma mark Construction
+id<MTLTexture> MVKFramebuffer::getDummyAttachmentMTLTexture(MVKRenderSubpass* subpass, uint32_t passIdx) {
+	if (_mtlDummyTex) { return _mtlDummyTex; }
+
+	// Lock and check again in case another thread has created the texture.
+	lock_guard<mutex> lock(_lock);
+	if (_mtlDummyTex) { return _mtlDummyTex; }
+
+	VkExtent2D fbExtent = getExtent2D();
+	uint32_t fbLayerCount = getLayerCount();
+	uint32_t sampleCount = mvkSampleCountFromVkSampleCountFlagBits(subpass->getDefaultSampleCount());
+	MTLTextureDescriptor* mtlTexDesc = [MTLTextureDescriptor texture2DDescriptorWithPixelFormat: MTLPixelFormatR8Unorm width: fbExtent.width height: fbExtent.height mipmapped: NO];
+	if (subpass->isMultiview()) {
+#if MVK_MACOS_OR_IOS
+		if (sampleCount > 1 && getDevice()->_pMetalFeatures->multisampleLayeredRendering) {
+			mtlTexDesc.textureType = MTLTextureType2DMultisampleArray;
+			mtlTexDesc.sampleCount = sampleCount;
+		} else {
+			mtlTexDesc.textureType = MTLTextureType2DArray;
+		}
+#else
+		mtlTexDesc.textureType = MTLTextureType2DArray;
+#endif
+		mtlTexDesc.arrayLength = subpass->getViewCountInMetalPass(passIdx);
+	} else if (fbLayerCount > 1) {
+#if MVK_MACOS
+		if (sampleCount > 1 && getDevice()->_pMetalFeatures->multisampleLayeredRendering) {
+			mtlTexDesc.textureType = MTLTextureType2DMultisampleArray;
+			mtlTexDesc.sampleCount = sampleCount;
+		} else {
+			mtlTexDesc.textureType = MTLTextureType2DArray;
+		}
+#else
+		mtlTexDesc.textureType = MTLTextureType2DArray;
+#endif
+		mtlTexDesc.arrayLength = fbLayerCount;
+	} else if (sampleCount > 1) {
+		mtlTexDesc.textureType = MTLTextureType2DMultisample;
+		mtlTexDesc.sampleCount = sampleCount;
+	}
+#if MVK_IOS
+	if ([getMTLDevice() supportsFeatureSet: MTLFeatureSet_iOS_GPUFamily1_v3]) {
+		mtlTexDesc.storageMode = MTLStorageModeMemoryless;
+	} else {
+		mtlTexDesc.storageMode = MTLStorageModePrivate;
+	}
+#else
+	mtlTexDesc.storageMode = MTLStorageModePrivate;
+#endif
+	mtlTexDesc.usage = MTLTextureUsageRenderTarget;
+
+	_mtlDummyTex = [getMTLDevice() newTextureWithDescriptor: mtlTexDesc];	// retained
+	[_mtlDummyTex setPurgeableState: MTLPurgeableStateVolatile];
+
+	return _mtlDummyTex;
+}
 
 MVKFramebuffer::MVKFramebuffer(MVKDevice* device,
 							   const VkFramebufferCreateInfo* pCreateInfo) : MVKVulkanAPIDeviceObject(device) {
@@ -36,3 +93,8 @@ MVKFramebuffer::MVKFramebuffer(MVKDevice* device,
 		}
 	}
 }
+
+MVKFramebuffer::~MVKFramebuffer() {
+	[_mtlDummyTex release];
+}
+

MoltenVK/MoltenVK/GPUObjects/MVKRenderPass.h

@@ -76,6 +76,9 @@ public:
 	/** Returns the Vulkan sample count of the attachments used in this subpass. */
 	VkSampleCountFlagBits getSampleCount();
 
+	/** Returns the default sample count for when there are no attachments used in this subpass. */
+	VkSampleCountFlagBits getDefaultSampleCount() { return _defaultSampleCount; }
+
 	/** Sets the default sample count for when there are no attachments used in this subpass. */
 	void setDefaultSampleCount(VkSampleCountFlagBits count) { _defaultSampleCount = count; }
 
@@ -104,8 +107,7 @@ public:
 	 */
 	void populateMTLRenderPassDescriptor(MTLRenderPassDescriptor* mtlRPDesc,
 										 uint32_t passIdx,
-										 VkExtent2D framebufferExtent,
+										 MVKFramebuffer* framebuffer,
-										 uint32_t framebufferLayerCount,
 										 const MVKArrayRef<MVKImageView*> attachments,
 										 const MVKArrayRef<VkClearValue> clearValues,
 										 bool isRenderingEntireAttachment,
@@ -161,7 +163,6 @@ private:
 	VkAttachmentReference2 _depthStencilResolveAttachment;
 	VkResolveModeFlagBits _depthResolveMode = VK_RESOLVE_MODE_NONE;
 	VkResolveModeFlagBits _stencilResolveMode = VK_RESOLVE_MODE_NONE;
-	id<MTLTexture> _mtlDummyTex = nil;
 	VkSampleCountFlagBits _defaultSampleCount = VK_SAMPLE_COUNT_1_BIT;
 };
 

MoltenVK/MoltenVK/GPUObjects/MVKRenderPass.mm

@@ -196,8 +196,7 @@ uint32_t MVKRenderSubpass::getViewCountUpToMetalPass(uint32_t passIdx) const {
 
 void MVKRenderSubpass::populateMTLRenderPassDescriptor(MTLRenderPassDescriptor* mtlRPDesc,
 													   uint32_t passIdx,
-													   VkExtent2D framebufferExtent,
+													   MVKFramebuffer* framebuffer,
-													   uint32_t framebufferLayerCount,
 													   const MVKArrayRef<MVKImageView*> attachments,
 													   const MVKArrayRef<VkClearValue> clearValues,
 													   bool isRenderingEntireAttachment,
@@ -326,68 +325,23 @@ void MVKRenderSubpass::populateMTLRenderPassDescriptor(MTLRenderPassDescriptor*
 		}
 	}
 
-	_mtlDummyTex = nil;
+	// Vulkan supports rendering without attachments, but older Metal does not.
+	// If Metal does not support rendering without attachments, create a dummy attachment to pass Metal validation.
 	if (caUsedCnt == 0 && dsRPAttIdx == VK_ATTACHMENT_UNUSED) {
-		uint32_t sampleCount = mvkSampleCountFromVkSampleCountFlagBits(_defaultSampleCount);
         if (_renderPass->getDevice()->_pMetalFeatures->renderWithoutAttachments) {
-            // We support having no attachments.
 #if MVK_MACOS_OR_IOS
-            mtlRPDesc.defaultRasterSampleCount = sampleCount;
+            mtlRPDesc.defaultRasterSampleCount = mvkSampleCountFromVkSampleCountFlagBits(_defaultSampleCount);
 #endif
-            return;
-        }
-
-		// Add a dummy attachment so this passes validation.
-		VkExtent2D fbExtent = framebufferExtent;
-		MTLTextureDescriptor* mtlTexDesc = [MTLTextureDescriptor texture2DDescriptorWithPixelFormat: MTLPixelFormatR8Unorm width: fbExtent.width height: fbExtent.height mipmapped: NO];
-		if (isMultiview()) {
-#if MVK_MACOS_OR_IOS
-			if (sampleCount > 1 && _renderPass->getDevice()->_pMetalFeatures->multisampleLayeredRendering) {
-				mtlTexDesc.textureType = MTLTextureType2DMultisampleArray;
-				mtlTexDesc.sampleCount = sampleCount;
 		} else {
-				mtlTexDesc.textureType = MTLTextureType2DArray;
-			}
-#else
-			mtlTexDesc.textureType = MTLTextureType2DArray;
-#endif
-			mtlTexDesc.arrayLength = getViewCountInMetalPass(passIdx);
-		} else if (framebufferLayerCount > 1) {
-#if MVK_MACOS
-			if (sampleCount > 1 && _renderPass->getDevice()->_pMetalFeatures->multisampleLayeredRendering) {
-				mtlTexDesc.textureType = MTLTextureType2DMultisampleArray;
-				mtlTexDesc.sampleCount = sampleCount;
-			} else {
-				mtlTexDesc.textureType = MTLTextureType2DArray;
-			}
-#else
-			mtlTexDesc.textureType = MTLTextureType2DArray;
-#endif
-			mtlTexDesc.arrayLength = framebufferLayerCount;
-		} else if (sampleCount > 1) {
-			mtlTexDesc.textureType = MTLTextureType2DMultisample;
-			mtlTexDesc.sampleCount = sampleCount;
-		}
-#if MVK_IOS
-		if ([_renderPass->getMTLDevice() supportsFeatureSet: MTLFeatureSet_iOS_GPUFamily1_v3]) {
-			mtlTexDesc.storageMode = MTLStorageModeMemoryless;
-		} else {
-			mtlTexDesc.storageMode = MTLStorageModePrivate;
-		}
-#else
-		mtlTexDesc.storageMode = MTLStorageModePrivate;
-#endif
-		mtlTexDesc.usage = MTLTextureUsageRenderTarget;
-		_mtlDummyTex = [_renderPass->getMTLDevice() newTextureWithDescriptor: mtlTexDesc];  // not retained
-		[_mtlDummyTex setPurgeableState: MTLPurgeableStateVolatile];
 			MTLRenderPassColorAttachmentDescriptor* mtlColorAttDesc = mtlRPDesc.colorAttachments[0];
-		mtlColorAttDesc.texture = _mtlDummyTex;
+			mtlColorAttDesc.texture = framebuffer->getDummyAttachmentMTLTexture(this, passIdx);
 			mtlColorAttDesc.level = 0;
 			mtlColorAttDesc.slice = 0;
 			mtlColorAttDesc.depthPlane = 0;
 			mtlColorAttDesc.loadAction = MTLLoadActionDontCare;
 			mtlColorAttDesc.storeAction = MTLStoreActionDontCare;
 		}
+	}
 }
 
 void MVKRenderSubpass::encodeStoreActions(MVKCommandEncoder* cmdEncoder,