From 92712e240a4ce7b372929ee38b3c8d72b60e43f2 Mon Sep 17 00:00:00 2001 From: Bill Hollings Date: Sat, 25 Dec 2021 16:18:18 -0500 Subject: [PATCH] Fix memory leak of dummy MTLTexture in render subpasses that use no attachments. Older Metal does not support rendering without subpass attachments. In this case, a dummy attachment with a dummy MTLTexture is created whenever the subpass begins, but was not being correctly used. Move the creation, retaining, and releasing of the dummy MTLTexture to MVKFramebuffer, where the extent and layer count is known and can be reused. Pass framebuffer to MVKCommandEncoder::beginRenderpass() and remember current framebuffer in MVKCommandEncoder. Add getFramebufferExtent() and getFramebufferLayerCount() to MVKCommandEncoder. Pass framebuffer to MVKRenderSubpass:populateMTLRenderPassDescriptor() and retrieve dummy MTLTexture from framebuffer. --- Docs/Whats_New.md | 1 + .../MoltenVK/Commands/MVKCmdRenderPass.mm | 3 +- MoltenVK/MoltenVK/Commands/MVKCmdTransfer.mm | 4 +- MoltenVK/MoltenVK/Commands/MVKCommandBuffer.h | 16 ++--- .../MoltenVK/Commands/MVKCommandBuffer.mm | 26 +++---- MoltenVK/MoltenVK/GPUObjects/MVKFramebuffer.h | 16 ++++- .../MoltenVK/GPUObjects/MVKFramebuffer.mm | 64 ++++++++++++++++- MoltenVK/MoltenVK/GPUObjects/MVKRenderPass.h | 7 +- MoltenVK/MoltenVK/GPUObjects/MVKRenderPass.mm | 68 +++---------------- 9 files changed, 118 insertions(+), 87 deletions(-) diff --git a/Docs/Whats_New.md b/Docs/Whats_New.md index 47a6ff26..27c61afb 100644 --- a/Docs/Whats_New.md +++ b/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. - Update to latest SPIRV-Cross: - MSL: Add 64 bit support for `OpSwitch`. - MSL: Don't output depth and stencil values with explicit early fragment tests. diff --git a/MoltenVK/MoltenVK/Commands/MVKCmdRenderPass.mm b/MoltenVK/MoltenVK/Commands/MVKCmdRenderPass.mm index 98ba2c11..2a98a876 100644 --- a/MoltenVK/MoltenVK/Commands/MVKCmdRenderPass.mm +++ b/MoltenVK/MoltenVK/Commands/MVKCmdRenderPass.mm @@ -64,8 +64,7 @@ void MVKCmdBeginRenderPass::encode(MVKCommandEncoder* cmdEncoder) { cmdEncoder->beginRenderpass(this, _contents, _renderPass, - _framebuffer->getExtent2D(), - _framebuffer->getLayerCount(), + _framebuffer, _renderArea, _clearValues.contents(), _attachments.contents()); diff --git a/MoltenVK/MoltenVK/Commands/MVKCmdTransfer.mm b/MoltenVK/MoltenVK/Commands/MVKCmdTransfer.mm index a4d81da8..72e36e5d 100644 --- a/MoltenVK/MoltenVK/Commands/MVKCmdTransfer.mm +++ b/MoltenVK/MoltenVK/Commands/MVKCmdTransfer.mm @@ -1234,7 +1234,7 @@ void MVKCmdClearAttachments::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::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(); } diff --git a/MoltenVK/MoltenVK/Commands/MVKCommandBuffer.h b/MoltenVK/MoltenVK/Commands/MVKCommandBuffer.h index 489f74ab..7803bf27 100644 --- a/MoltenVK/MoltenVK/Commands/MVKCommandBuffer.h +++ b/MoltenVK/MoltenVK/Commands/MVKCommandBuffer.h @@ -283,8 +283,7 @@ public: void beginRenderpass(MVKCommand* passCmd, VkSubpassContents subpassContents, MVKRenderPass* renderPass, - VkExtent2D framebufferExtent, - uint32_t framebufferLayerCount, + MVKFramebuffer* framebuffer, VkRect2D& renderArea, MVKArrayRef clearValues, MVKArrayRef 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; diff --git a/MoltenVK/MoltenVK/Commands/MVKCommandBuffer.mm b/MoltenVK/MoltenVK/Commands/MVKCommandBuffer.mm index c9a176c7..b363fe2c 100644 --- a/MoltenVK/MoltenVK/Commands/MVKCommandBuffer.mm +++ b/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 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, - uint32_t framebufferLayerCount, + MVKFramebuffer* framebuffer, VkRect2D& renderArea, MVKArrayRef clearValues, MVKArrayRef attachments) { _renderPass = renderPass; - _framebufferExtent = framebufferExtent; - _framebufferLayerCount = framebufferLayerCount; + _framebuffer = framebuffer; _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, - _framebufferLayerCount, + _framebuffer, _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 = {}; - _framebufferLayerCount = 0; + _framebuffer = nullptr; _attachments.clear(); _renderSubpassIndex = 0; } diff --git a/MoltenVK/MoltenVK/GPUObjects/MVKFramebuffer.h b/MoltenVK/MoltenVK/GPUObjects/MVKFramebuffer.h index 928da956..39beb73f 100644 --- a/MoltenVK/MoltenVK/GPUObjects/MVKFramebuffer.h +++ b/MoltenVK/MoltenVK/GPUObjects/MVKFramebuffer.h @@ -21,6 +21,9 @@ #include "MVKDevice.h" #include "MVKImage.h" #include "MVKSmallVector.h" +#include + +class MVKRenderSubpass; #pragma mark MVKFramebuffer @@ -45,16 +48,25 @@ public: /** Returns the attachments. */ MVKArrayRef 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 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 _attachments; + id _mtlDummyTex = nil; + std::mutex _lock; VkExtent2D _extent; uint32_t _layerCount; - MVKSmallVector _attachments; }; diff --git a/MoltenVK/MoltenVK/GPUObjects/MVKFramebuffer.mm b/MoltenVK/MoltenVK/GPUObjects/MVKFramebuffer.mm index 91d3bded..31daa65b 100644 --- a/MoltenVK/MoltenVK/GPUObjects/MVKFramebuffer.mm +++ b/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 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 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 ([_renderPass->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]; +} + diff --git a/MoltenVK/MoltenVK/GPUObjects/MVKRenderPass.h b/MoltenVK/MoltenVK/GPUObjects/MVKRenderPass.h index 93695ba4..6dfb2289 100644 --- a/MoltenVK/MoltenVK/GPUObjects/MVKRenderPass.h +++ b/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, - uint32_t framebufferLayerCount, + MVKFramebuffer* framebuffer, const MVKArrayRef attachments, const MVKArrayRef clearValues, bool isRenderingEntireAttachment, @@ -161,7 +163,6 @@ private: VkAttachmentReference2 _depthStencilResolveAttachment; VkResolveModeFlagBits _depthResolveMode = VK_RESOLVE_MODE_NONE; VkResolveModeFlagBits _stencilResolveMode = VK_RESOLVE_MODE_NONE; - id _mtlDummyTex = nil; VkSampleCountFlagBits _defaultSampleCount = VK_SAMPLE_COUNT_1_BIT; }; diff --git a/MoltenVK/MoltenVK/GPUObjects/MVKRenderPass.mm b/MoltenVK/MoltenVK/GPUObjects/MVKRenderPass.mm index 47dc9c36..7e1a9291 100644 --- a/MoltenVK/MoltenVK/GPUObjects/MVKRenderPass.mm +++ b/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, - uint32_t framebufferLayerCount, + MVKFramebuffer* framebuffer, const MVKArrayRef attachments, const MVKArrayRef clearValues, bool isRenderingEntireAttachment, @@ -326,67 +325,22 @@ 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; + MTLRenderPassColorAttachmentDescriptor* mtlColorAttDesc = mtlRPDesc.colorAttachments[0]; + mtlColorAttDesc.texture = framebuffer->getDummyAttachmentMTLTexture(this, passIdx); + mtlColorAttDesc.level = 0; + mtlColorAttDesc.slice = 0; + mtlColorAttDesc.depthPlane = 0; + mtlColorAttDesc.loadAction = MTLLoadActionDontCare; + mtlColorAttDesc.storeAction = MTLStoreActionDontCare; } -#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.level = 0; - mtlColorAttDesc.slice = 0; - mtlColorAttDesc.depthPlane = 0; - mtlColorAttDesc.loadAction = MTLLoadActionDontCare; - mtlColorAttDesc.storeAction = MTLStoreActionDontCare; } }