libqt5-qt3d/0003-QText2DEntity-fix-QTextureAtlas-parenting-that-could-lead-to.patch

From 9f2d212416c3c718a5661a09438fb413f9cc53b6 Mon Sep 17 00:00:00 2001
From: Paul Lemire <paul.lemire@kdab.com>
Date: Fri, 10 Feb 2023 09:47:50 +0100
Subject: [PATCH] QText2DEntity: fix QTextureAtlas parenting that could lead to
 crashes

We rely on a DistanceFieldFont object to manage QTextureAtlas that hold
the glyphs. The DistanceFieldFont/QTextureAtlas are supposed to be parented
by the scene root to ensure that a QTextureAtlas lives as long as possible.

DistanceFieldFont/QTextureAtlas are stored in a cache global to the scene
to minimize the use of resources.

When adding text elements, we can reuse atlases since the cache is global to
the scene and only destroy an atlas (and remove it from the cache) when we
know no more glyphs are referencing it.

However we were mistakenly passing a null parenty to DistanceFieldFont instace
of the scene root. This resulted on the QTextureAtlas not being parented by
the scene root but rather by the first DistanceFieldRenderer to use the atlas.
This meants that if the DistanceFieldRenderer were to be destroyed, so would
the atlas (yet it would still be referenced by the glyph cache leading to
crashes).

Change-Id: Id84f6a651b162a4bb3c571b11388fd2429b231de
Reviewed-by: Mike Krus <mike.krus@kdab.com>
(cherry picked from commit b1a135c547f38db0b2ce6b7bc4c4cccc43ef87d3)
Reviewed-by: Paul Lemire <paul.lemire@kdab.com>
---
 src/extras/text/qdistancefieldglyphcache.cpp  | 23 ++++++++++++-------
 src/extras/text/qtext2dentity.cpp             |  3 ++-
 .../qtext2dentity/tst_qtext2dentity.cpp       |  2 +-
 3 files changed, 18 insertions(+), 10 deletions(-)

diff --git a/src/extras/text/qdistancefieldglyphcache.cpp b/src/extras/text/qdistancefieldglyphcache.cpp
index a65e7fc63a..85591e6807 100644
--- a/src/extras/text/qdistancefieldglyphcache.cpp
+++ b/src/extras/text/qdistancefieldglyphcache.cpp
@@ -161,6 +161,7 @@ DistanceFieldFont::DistanceFieldFont(const QRawFont &font, bool doubleRes, Qt3DC
     , m_doubleGlyphResolution(doubleRes)
     , m_parentNode(parent)
 {
+    Q_ASSERT(m_parentNode);
 }
 
 DistanceFieldFont::~DistanceFieldFont()
@@ -197,13 +198,14 @@ StoredGlyph DistanceFieldFont::refGlyph(quint32 glyph)
         // scenarios
         const int size = m_doubleGlyphResolution ? 512 : 256;
 
-        QTextureAtlas *atlas = new QTextureAtlas(m_parentNode);
+        QTextureAtlas *atlas = new QTextureAtlas();
         atlas->setWidth(size);
         atlas->setHeight(size);
         atlas->setFormat(Qt3DRender::QAbstractTexture::R8_UNorm);
         atlas->setPixelFormat(QOpenGLTexture::Red);
         atlas->setMinificationFilter(Qt3DRender::QAbstractTexture::Linear);
         atlas->setMagnificationFilter(Qt3DRender::QAbstractTexture::Linear);
+        atlas->setParent(m_parentNode);
         m_atlasses << atlas;
 
         if (!storedGlyph.addToTextureAtlas(atlas))
@@ -236,7 +238,12 @@ void DistanceFieldFont::derefGlyph(quint32 glyph)
             Q_ASSERT(m_atlasses.contains(atlas));
 
             m_atlasses.removeAll(atlas);
-            delete atlas;
+
+            // This function might have been called as a result of destroying
+            // the scene root which traverses the entire scene tree. Calling
+            // delete on the atlas here could lead to dangling pointers in the
+            // least of children being traversed for destruction.
+            atlas->deleteLater();
         }
 
         m_glyphs.erase(it);
@@ -287,7 +294,8 @@ DistanceFieldFont* QDistanceFieldGlyphCache::getOrCreateDistanceFieldFont(const
     // create new font cache
     // we set the parent node to nullptr, since the parent node of QTextureAtlasses
     // will be set when we pass them to QText2DMaterial later
-    DistanceFieldFont *dff = new DistanceFieldFont(actualFont, useDoubleRes, nullptr);
+    Q_ASSERT(m_rootNode);
+    DistanceFieldFont *dff = new DistanceFieldFont(actualFont, useDoubleRes, m_rootNode);
     m_fonts.insert(key, dff);
     return dff;
 }
@@ -324,11 +332,10 @@ QDistanceFieldGlyphCache::Glyph refAndGetGlyph(DistanceFieldFont *dff, quint32 g
     if (dff) {
         const auto entry = dff->refGlyph(glyph);
 
-        if (entry.atlas()) {
-            ret.glyphPathBoundingRect = entry.glyphPathBoundingRect();
-            ret.texCoords = entry.texCoords();
-            ret.texture = entry.atlas();
-        }
+        Q_ASSERT(entry.atlas());
+        ret.glyphPathBoundingRect = entry.glyphPathBoundingRect();
+        ret.texCoords = entry.texCoords();
+        ret.texture = entry.atlas();
     }
 
     return ret;
diff --git a/src/extras/text/qtext2dentity.cpp b/src/extras/text/qtext2dentity.cpp
index 59e8284e10..e3d3dad2e2 100644
--- a/src/extras/text/qtext2dentity.cpp
+++ b/src/extras/text/qtext2dentity.cpp
@@ -304,8 +304,9 @@ void QText2DEntityPrivate::setCurrentGlyphRuns(const QVector<QGlyphRun> &runs)
         delete m_renderers.takeLast();
 
     while (m_renderers.size() < renderData.size()) {
-        DistanceFieldTextRenderer *renderer = new DistanceFieldTextRenderer(q_func());
+        DistanceFieldTextRenderer *renderer = new DistanceFieldTextRenderer();
         renderer->setColor(m_color);
+        renderer->setParent(q_func());
         m_renderers << renderer;
     }
 
diff --git a/tests/auto/extras/qtext2dentity/tst_qtext2dentity.cpp b/tests/auto/extras/qtext2dentity/tst_qtext2dentity.cpp
index 6fcc2e6370..35e241839b 100644
--- a/tests/auto/extras/qtext2dentity/tst_qtext2dentity.cpp
+++ b/tests/auto/extras/qtext2dentity/tst_qtext2dentity.cpp
@@ -90,7 +90,7 @@ void tst_qtext2dentity::checkChangeArbiter()
     auto atlases = lookupNodeByClassName(rootEntity.data(), "Qt3DExtras::QTextureAtlas");
     QVERIFY(atlases.length() == 1);
     auto atlas = atlases[0];
-    QTRY_VERIFY(Qt3DCore::QNodePrivate::get(atlas)->m_changeArbiter);
+    QVERIFY(Qt3DCore::QNodePrivate::get(atlas)->m_changeArbiter);
 #endif
 }
 
-- 
GitLab