From 4983d26e0aa678653bac6981b00f7448a220e1b1 Mon Sep 17 00:00:00 2001 From: Laszlo Agocs Date: Tue, 21 Mar 2023 20:52:22 +0100 Subject: Fix destroying glyph cache textures with hidden text items Move the management of the to-be-released textures up to the rendercontext, so that the glyph cache objects do not need to care about this. Issuing a delete(Later) on the QRhiTexture in the glyph cache is too early if there is no visible text item in the scene that's going to commit the resource updates collected on the rendercontext's QRhiResourceUpdateBatch. Instead, it is logical to move also the list of pending-release textures to the rendercontext, since it already has the update batch object. Fixes: QTBUG-111995 Change-Id: Ic4309166f265950a7061ae510567ce677c4943df Reviewed-by: Qt CI Bot Reviewed-by: Andy Nichols (cherry picked from commit e9c0b15060e0b70f70857d02029d6ebed95c0abf) Reviewed-by: Qt Cherry-pick Bot --- src/quick/scenegraph/qsgdefaultrendercontext.cpp | 15 ++++++++++++-- src/quick/scenegraph/qsgdefaultrendercontext_p.h | 5 ++++- .../scenegraph/qsgrhidistancefieldglyphcache.cpp | 23 ++++------------------ src/quick/scenegraph/qsgrhitextureglyphcache.cpp | 20 +++---------------- src/quick/scenegraph/qsgrhitextureglyphcache_p.h | 1 - 5 files changed, 24 insertions(+), 40 deletions(-) diff --git a/src/quick/scenegraph/qsgdefaultrendercontext.cpp b/src/quick/scenegraph/qsgdefaultrendercontext.cpp index 3fee9d9faf..cd0a5b6d28 100644 --- a/src/quick/scenegraph/qsgdefaultrendercontext.cpp +++ b/src/quick/scenegraph/qsgdefaultrendercontext.cpp @@ -119,7 +119,7 @@ void QSGDefaultRenderContext::invalidate() qDeleteAll(m_glyphCaches); m_glyphCaches.clear(); - releaseGlyphCacheResourceUpdates(); + resetGlyphCacheResources(); m_rhi = nullptr; @@ -264,12 +264,23 @@ QRhiResourceUpdateBatch *QSGDefaultRenderContext::glyphCacheResourceUpdates() return m_glyphCacheResourceUpdates; } -void QSGDefaultRenderContext::releaseGlyphCacheResourceUpdates() +void QSGDefaultRenderContext::deferredReleaseGlyphCacheTexture(QRhiTexture *texture) +{ + if (texture) + m_pendingGlyphCacheTextures.insert(texture); +} + +void QSGDefaultRenderContext::resetGlyphCacheResources() { if (m_glyphCacheResourceUpdates) { m_glyphCacheResourceUpdates->release(); m_glyphCacheResourceUpdates = nullptr; } + + for (QRhiTexture *t : std::as_const(m_pendingGlyphCacheTextures)) + t->deleteLater(); // the QRhiTexture object stays valid for the current frame + + m_pendingGlyphCacheTextures.clear(); } QT_END_NAMESPACE diff --git a/src/quick/scenegraph/qsgdefaultrendercontext_p.h b/src/quick/scenegraph/qsgdefaultrendercontext_p.h index 448255a594..580a973e85 100644 --- a/src/quick/scenegraph/qsgdefaultrendercontext_p.h +++ b/src/quick/scenegraph/qsgdefaultrendercontext_p.h @@ -24,6 +24,7 @@ class QRhi; class QRhiCommandBuffer; class QRhiRenderPassDescriptor; class QRhiResourceUpdateBatch; +class QRhiTexture; class QSGMaterialShader; class QSurface; @@ -102,7 +103,8 @@ public: QRhiResourceUpdateBatch *maybeGlyphCacheResourceUpdates(); QRhiResourceUpdateBatch *glyphCacheResourceUpdates(); - void releaseGlyphCacheResourceUpdates(); + void deferredReleaseGlyphCacheTexture(QRhiTexture *texture); + void resetGlyphCacheResources(); protected: static QString fontKey(const QRawFont &font, int renderTypeQuality); @@ -116,6 +118,7 @@ protected: qreal m_currentDevicePixelRatio; bool m_useDepthBufferFor2D; QRhiResourceUpdateBatch *m_glyphCacheResourceUpdates; + QSet m_pendingGlyphCacheTextures; }; QT_END_NAMESPACE diff --git a/src/quick/scenegraph/qsgrhidistancefieldglyphcache.cpp b/src/quick/scenegraph/qsgrhidistancefieldglyphcache.cpp index a68842577d..54cf298943 100644 --- a/src/quick/scenegraph/qsgrhidistancefieldglyphcache.cpp +++ b/src/quick/scenegraph/qsgrhidistancefieldglyphcache.cpp @@ -32,19 +32,10 @@ QSGRhiDistanceFieldGlyphCache::QSGRhiDistanceFieldGlyphCache(QSGDefaultRenderCon QSGRhiDistanceFieldGlyphCache::~QSGRhiDistanceFieldGlyphCache() { - // A plain delete should work, but just in case commitResourceUpdates was - // not called and something is enqueued on the update batch for a texture, - // defer until the end of the frame. - for (int i = 0; i < m_textures.size(); ++i) { - if (m_textures[i].texture) - m_textures[i].texture->deleteLater(); - } + for (const TextureInfo &t : std::as_const(m_textures)) + m_rc->deferredReleaseGlyphCacheTexture(t.texture); delete m_areaAllocator; - - // should be empty, but just in case - for (QRhiTexture *t : std::as_const(m_pendingDispose)) - t->deleteLater(); } void QSGRhiDistanceFieldGlyphCache::requestGlyphs(const QSet &glyphs) @@ -243,7 +234,7 @@ void QSGRhiDistanceFieldGlyphCache::resizeTexture(TextureInfo *texInfo, int widt resourceUpdates->copyTexture(texInfo->texture, oldTexture); } - m_pendingDispose.insert(oldTexture); + m_rc->deferredReleaseGlyphCacheTexture(oldTexture); } bool QSGRhiDistanceFieldGlyphCache::useTextureResizeWorkaround() const @@ -522,14 +513,8 @@ void QSGRhiDistanceFieldGlyphCache::commitResourceUpdates(QRhiResourceUpdateBatc { if (QRhiResourceUpdateBatch *resourceUpdates = m_rc->maybeGlyphCacheResourceUpdates()) { mergeInto->merge(resourceUpdates); - m_rc->releaseGlyphCacheResourceUpdates(); + m_rc->resetGlyphCacheResources(); } - - // now let's assume the resource updates will be committed in this frame - for (QRhiTexture *t : std::as_const(m_pendingDispose)) - t->deleteLater(); // will be deleted after the frame is submitted -> safe - - m_pendingDispose.clear(); } bool QSGRhiDistanceFieldGlyphCache::eightBitFormatIsAlphaSwizzled() const diff --git a/src/quick/scenegraph/qsgrhitextureglyphcache.cpp b/src/quick/scenegraph/qsgrhitextureglyphcache.cpp index 2324db5762..9fe1b2ad01 100644 --- a/src/quick/scenegraph/qsgrhitextureglyphcache.cpp +++ b/src/quick/scenegraph/qsgrhitextureglyphcache.cpp @@ -23,15 +23,7 @@ QSGRhiTextureGlyphCache::QSGRhiTextureGlyphCache(QSGDefaultRenderContext *rc, QSGRhiTextureGlyphCache::~QSGRhiTextureGlyphCache() { - // A plain delete should work, but just in case commitResourceUpdates was - // not called and something is enqueued on the update batch for m_texture, - // defer until the end of the frame. - if (m_texture) - m_texture->deleteLater(); - - // should be empty, but just in case - for (QRhiTexture *t : std::as_const(m_pendingDispose)) - t->deleteLater(); + m_rc->deferredReleaseGlyphCacheTexture(m_texture); } QRhiTexture *QSGRhiTextureGlyphCache::createEmptyTexture(QRhiTexture::Format format) @@ -97,7 +89,7 @@ void QSGRhiTextureGlyphCache::resizeTextureData(int width, int height) resourceUpdates->uploadTexture(t, QRhiTextureUploadEntry(0, 0, subresDesc)); } - m_pendingDispose.insert(m_texture); + m_rc->deferredReleaseGlyphCacheTexture(m_texture); m_texture = t; } } @@ -220,14 +212,8 @@ void QSGRhiTextureGlyphCache::commitResourceUpdates(QRhiResourceUpdateBatch *mer { if (QRhiResourceUpdateBatch *resourceUpdates = m_rc->maybeGlyphCacheResourceUpdates()) { mergeInto->merge(resourceUpdates); - m_rc->releaseGlyphCacheResourceUpdates(); + m_rc->resetGlyphCacheResources(); } - - // now let's assume the resource updates will be committed in this frame - for (QRhiTexture *t : std::as_const(m_pendingDispose)) - t->deleteLater(); // will be deleted after the frame is submitted -> safe - - m_pendingDispose.clear(); } bool QSGRhiTextureGlyphCache::eightBitFormatIsAlphaSwizzled() const diff --git a/src/quick/scenegraph/qsgrhitextureglyphcache_p.h b/src/quick/scenegraph/qsgrhitextureglyphcache_p.h index 2960c91b01..3c6b99bc9f 100644 --- a/src/quick/scenegraph/qsgrhitextureglyphcache_p.h +++ b/src/quick/scenegraph/qsgrhitextureglyphcache_p.h @@ -60,7 +60,6 @@ private: QSize m_size; bool m_bgra = false; QVarLengthArray m_uploads; - QSet m_pendingDispose; }; QT_END_NAMESPACE -- cgit v1.2.3