From 0cba6a27e27b160e2805c5cc29b108bf6e7fde20 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tor=20Arne=20Vestb=C3=B8?= Date: Tue, 2 Feb 2021 17:07:49 +0100 Subject: [PATCH] rhi: metal: Manually manage drawable lifetime The drawable returned from nextDrawable is autoreleased, which means unless we explicitly retain it we have no guarantee that it will stay alive long enough to be used again in endFrame(). For example, if the user had an autorelease-pool in their custom render code, where the call to beginPass() happens, the drawable would be released when that pool went out of scope. The only reason we didn't hit this yet is due to the default autorelease-pool being at the level of the runloop, so the drawable happened to outlive the rendering of a single frame. An advantage of manually managing the lifetime of the drawable is also that we can release it immediately after being done with it, instead of waiting until the autorelease-pool drains. Change-Id: Ie6fd271adbe1121eb947bb7075142f1a6c81175d Reviewed-by: Timur Pocheptsov Reviewed-by: Laszlo Agocs --- src/gui/rhi/qrhimetal.mm | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/gui/rhi/qrhimetal.mm b/src/gui/rhi/qrhimetal.mm index 9eb0ba75b5..7151741f2e 100644 --- a/src/gui/rhi/qrhimetal.mm +++ b/src/gui/rhi/qrhimetal.mm @@ -322,7 +322,7 @@ struct QMetalSwapChainData #else CAMetalLayer *layer = nullptr; #endif - id curDrawable; + id curDrawable = nil; dispatch_semaphore_t sem[QMTL_FRAMES_IN_FLIGHT]; MTLRenderPassDescriptor *rp = nullptr; id msaaTex[QMTL_FRAMES_IN_FLIGHT]; @@ -1457,7 +1457,7 @@ QRhi::FrameOpResult QRhiMetal::endFrame(QRhiSwapChain *swapChain, QRhi::EndFrame [swapChainD->cbWrapper.d->cb presentDrawable: swapChainD->d->curDrawable]; // Must not hold on to the drawable, regardless of needsPresent. - // (internally it is autoreleased or something, it seems) + [swapChainD->d->curDrawable release]; swapChainD->d->curDrawable = nil; __block int thisFrameSlot = currentFrameSlot; @@ -1964,10 +1964,11 @@ void QRhiMetal::beginPass(QRhiCommandBuffer *cb, Q_ASSERT(currentSwapChain); QMetalSwapChain *swapChainD = QRHI_RES(QMetalSwapChain, currentSwapChain); if (!swapChainD->d->curDrawable) { + QMacAutoReleasePool pool; #ifdef TARGET_IPHONE_SIMULATOR if (@available(ios 13.0, *)) #endif - swapChainD->d->curDrawable = [swapChainD->d->layer nextDrawable]; + swapChainD->d->curDrawable = [[swapChainD->d->layer nextDrawable] retain]; } if (!swapChainD->d->curDrawable) { qWarning("No drawable"); @@ -3807,6 +3808,9 @@ void QMetalSwapChain::destroy() d->layer = nullptr; + [d->curDrawable release]; + d->curDrawable = nil; + QRHI_RES_RHI(QRhiMetal); rhiD->swapchains.remove(this); @@ -3940,6 +3944,7 @@ bool QMetalSwapChain::createOrResize() [d->layer setDevice: rhiD->d->dev]; + [d->curDrawable release]; d->curDrawable = nil; for (int i = 0; i < QMTL_FRAMES_IN_FLIGHT; ++i) {