From 3f55ac90fc2ed5f46cffc24ca5af190bdd9e4fc8 Mon Sep 17 00:00:00 2001 From: Laszlo Agocs Date: Fri, 4 Apr 2025 16:29:28 +0200 Subject: [PATCH] rhi: Remove runCleanup function The rest of the framework uses cleanup callbacks in a way that implies that manually triggering the callbacks will lead to problems: components such as QQuickWidget register cleanup callbacks to get notified about the destruction of the QRhi. These callbacks should not be invoked at arbitrary times. There is no usage of this function anywhere in Qt. Remove it. Slightly extend the autotest by exercising the keyed registration functions as well. Change-Id: I88f1a1e9bc5a642b8e8b6238fe198f123bd55978 Reviewed-by: Andy Nichols --- src/gui/rhi/qrhi.cpp | 36 ++++++++++------------------ src/gui/rhi/qrhi.h | 1 - src/gui/rhi/qrhi_p.h | 2 ++ tests/auto/gui/rhi/qrhi/tst_qrhi.cpp | 8 +++---- 4 files changed, 19 insertions(+), 28 deletions(-) diff --git a/src/gui/rhi/qrhi.cpp b/src/gui/rhi/qrhi.cpp index 192128b5ee8..519cbc3ac55 100644 --- a/src/gui/rhi/qrhi.cpp +++ b/src/gui/rhi/qrhi.cpp @@ -8808,7 +8808,7 @@ QRhi::~QRhi() if (!d) return; - runCleanup(); + d->runCleanup(); qDeleteAll(d->pendingDeleteResources); d->pendingDeleteResources.clear(); @@ -9337,8 +9337,7 @@ QThread *QRhi::thread() const } /*! - Registers a \a callback that is invoked either when the QRhi is destroyed, - or when runCleanup() is called. + Registers a \a callback that is invoked when the QRhi is destroyed. The callback will run with the graphics resource still available, so this provides an opportunity for the application to cleanly release QRhiResource @@ -9346,7 +9345,7 @@ QThread *QRhi::thread() const the lifetime of resources stored in \c cache type of objects, where the cache holds QRhiResources or objects containing QRhiResources. - \sa runCleanup(), ~QRhi() + \sa ~QRhi() */ void QRhi::addCleanupCallback(const CleanupCallback &callback) { @@ -9356,10 +9355,9 @@ void QRhi::addCleanupCallback(const CleanupCallback &callback) /*! \overload - Registers \a callback to be invoked either when the QRhi is destroyed or - when runCleanup() is called. This overload takes an opaque pointer, \a key, - that is used to ensure that a given callback is registered (and so called) - only once. + Registers \a callback to be invoked when the QRhi is destroyed. This + overload takes an opaque pointer, \a key, that is used to ensure that a + given callback is registered (and so called) only once. \sa removeCleanupCallback() */ @@ -9380,25 +9378,17 @@ void QRhi::removeCleanupCallback(const void *key) d->removeCleanupCallback(key); } -/*! - Invokes all registered cleanup functions. The list of cleanup callbacks it - then cleared. Normally destroying the QRhi does this automatically, but - sometimes it can be useful to trigger cleanup in order to release all - cached, non-essential resources. - - \sa addCleanupCallback() - */ -void QRhi::runCleanup() +void QRhiImplementation::runCleanup() { - for (const CleanupCallback &f : std::as_const(d->cleanupCallbacks)) - f(this); + for (const QRhi::CleanupCallback &f : std::as_const(cleanupCallbacks)) + f(q); - d->cleanupCallbacks.clear(); + cleanupCallbacks.clear(); - for (auto it = d->keyedCleanupCallbacks.cbegin(), end = d->keyedCleanupCallbacks.cend(); it != end; ++it) - it.value()(this); + for (auto it = keyedCleanupCallbacks.cbegin(), end = keyedCleanupCallbacks.cend(); it != end; ++it) + it.value()(q); - d->keyedCleanupCallbacks.clear(); + keyedCleanupCallbacks.clear(); } /*! diff --git a/src/gui/rhi/qrhi.h b/src/gui/rhi/qrhi.h index 576defe469f..5776554916a 100644 --- a/src/gui/rhi/qrhi.h +++ b/src/gui/rhi/qrhi.h @@ -1984,7 +1984,6 @@ public: void addCleanupCallback(const CleanupCallback &callback); void addCleanupCallback(const void *key, const CleanupCallback &callback); void removeCleanupCallback(const void *key); - void runCleanup(); QRhiGraphicsPipeline *newGraphicsPipeline(); QRhiComputePipeline *newComputePipeline(); diff --git a/src/gui/rhi/qrhi_p.h b/src/gui/rhi/qrhi_p.h index c62d5b6448f..a873eb8bb60 100644 --- a/src/gui/rhi/qrhi_p.h +++ b/src/gui/rhi/qrhi_p.h @@ -242,6 +242,8 @@ public: int effectiveSampleCount(int sampleCount) const; + void runCleanup(); + QRhi *q; static const int MAX_SHADER_CACHE_ENTRIES = 128; diff --git a/tests/auto/gui/rhi/qrhi/tst_qrhi.cpp b/tests/auto/gui/rhi/qrhi/tst_qrhi.cpp index 195100456a7..8bbced5fad3 100644 --- a/tests/auto/gui/rhi/qrhi/tst_qrhi.cpp +++ b/tests/auto/gui/rhi/qrhi/tst_qrhi.cpp @@ -371,10 +371,10 @@ void tst_QRhi::create() cleanupOk += 1; }; rhi->addCleanupCallback(cleanupFunc); - rhi->runCleanup(); - QCOMPARE(cleanupOk, 1); - cleanupOk = 0; rhi->addCleanupCallback(cleanupFunc); + rhi->addCleanupCallback(reinterpret_cast(quintptr(1234)), cleanupFunc); + rhi->addCleanupCallback(reinterpret_cast(quintptr(12345)), cleanupFunc); + rhi->removeCleanupCallback(reinterpret_cast(quintptr(1234))); QRhiResourceUpdateBatch *resUpd = rhi->nextResourceUpdateBatch(); QVERIFY(resUpd); @@ -512,7 +512,7 @@ void tst_QRhi::create() QVERIFY(!rhi->isDeviceLost()); rhi.reset(); - QCOMPARE(cleanupOk, 1); + QCOMPARE(cleanupOk, 3); } }