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 <qt_ci_bot@qt-project.org>
Reviewed-by: Andy Nichols <andy.nichols@qt.io>
(cherry picked from commit e9c0b15060e0b70f70857d02029d6ebed95c0abf)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>

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<QRhiTexture *> 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<glyph_t> &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<QRhiTextureUploadEntry, 16> m_uploads;
-    QSet<QRhiTexture *> m_pendingDispose;
 };
 
 QT_END_NAMESPACE