Fix string type data inputs and setAttribute leaking memory

Two leaks were fixed:
1 - String table now uses separate storage for dynamic strings
2 - Glyph cache is deleted if not used for current frame

Task-number: QT3DS-3686
Change-Id: Ib4cf2c61c5301a030039fef51b6d00b06d988c45
Reviewed-by: Jari Karppinen <jari.karppinen@qt.io>
Reviewed-by: Antti Määttä <antti.maatta@qt.io>
Reviewed-by: Janne Koskinen <janne.p.koskinen@qt.io>
 

6 files changed, 93 insertions, 10 deletions

diff --git a/src/foundation/StringTable.cpp b/src/foundation/StringTable.cpp
index 47130dc..f4dc574 100644
--- a/src/foundation/StringTable.cpp
+++ b/src/foundation/StringTable.cpp
@@ -38,6 +38,8 @@
 #include "foundation/SerializationTypes.h"
 #include "foundation/Qt3DSMutex.h"
 
+#include <QtCore/qhash.h>
+
 using namespace qt3ds;
 using namespace qt3ds::foundation;
 using namespace eastl;
@@ -527,6 +529,8 @@ struct SStringTableMutexScope
 
 #define STRING_TABLE_MULTITHREADED_METHOD SStringTableMutexScope __locker(m_MultithreadMutex)
 
+static const QT3DSU32 dynHandleBase = 0x80000000;
+
 class StringTable : public IStringTable
 {
     typedef nvhash_map<SCharAndHash, QT3DSU32> TMapType;
@@ -539,6 +543,18 @@ class StringTable : public IStringTable
     TWideStr m_WideConvertBuffer;
     Mutex m_MultithreadMutexBacker;
     Mutex *m_MultithreadMutex;
+
+    struct DynamicString
+    {
+        QByteArray str;
+        int refCount = 0;
+    };
+
+    QHash<QT3DSU32, DynamicString *> m_dynamicFreeHandlesMap;
+    QHash<QT3DSU32, DynamicString *> m_dynamicUsedHandlesMap;
+    QHash<QByteArray, QT3DSU32> m_dynamicStringToHandleMap;
+    QT3DSU32 m_nextFreeDynamicHandle = dynHandleBase;
+
     // Data that will be written out to the file.
 
 public:
@@ -554,7 +570,11 @@ public:
     {
     }
 
-    virtual ~StringTable() override {}
+    virtual ~StringTable() override
+    {
+        qDeleteAll(m_dynamicFreeHandlesMap);
+        qDeleteAll(m_dynamicUsedHandlesMap);
+    }
 
     QT3DS_IMPLEMENT_REF_COUNT_ADDREF_RELEASE_OVERRIDE(m_FileData.m_Allocator.m_Allocator)
 
@@ -630,10 +650,54 @@ public:
         return m_FileData.FindStrHandle(str);
     }
 
+    CStringHandle getDynamicHandle(const QByteArray &str) override
+    {
+        STRING_TABLE_MULTITHREADED_METHOD;
+        QT3DSU32 handle = m_dynamicStringToHandleMap.value(str, 0);
+        DynamicString *theStr = nullptr;
+
+        if (!handle) {
+            if (m_dynamicFreeHandlesMap.isEmpty()) {
+                handle = ++m_nextFreeDynamicHandle;
+                theStr = new DynamicString;
+            } else {
+                auto first = m_dynamicFreeHandlesMap.begin();
+                handle = first.key();
+                theStr = first.value();
+                m_dynamicFreeHandlesMap.erase(first);
+            }
+            theStr->str = str;
+            m_dynamicStringToHandleMap.insert(str, handle);
+            m_dynamicUsedHandlesMap.insert(handle, theStr);
+        } else {
+            theStr = m_dynamicUsedHandlesMap.value(handle);
+        }
+        ++theStr->refCount;
+        return CStringHandle::ISwearThisHasBeenRegistered(handle);
+    }
+
+    void releaseDynamicHandle(QT3DSU32 strHandle) override
+    {
+        DynamicString *str = m_dynamicUsedHandlesMap.value(strHandle);
+        if (str) {
+            --str->refCount;
+            if (str->refCount == 0) {
+                m_dynamicUsedHandlesMap.remove(strHandle);
+                m_dynamicFreeHandlesMap.insert(strHandle, str);
+                m_dynamicStringToHandleMap.remove(str->str);
+            }
+        }
+    }
+
     CRegisteredString HandleToStr(QT3DSU32 strHandle) override
     {
         STRING_TABLE_MULTITHREADED_METHOD;
-        return m_FileData.FindStrByHandle(strHandle);
+        if (strHandle >= dynHandleBase) {
+            DynamicString *str = m_dynamicUsedHandlesMap.value(strHandle);
+            return CRegisteredString::ISwearThisHasBeenRegistered(str->str.constData());
+        } else {
+            return m_FileData.FindStrByHandle(strHandle);
+        }
     }
 
     const wchar_t *GetWideStr(CRegisteredString theStr)
diff --git a/src/foundation/StringTable.h b/src/foundation/StringTable.h
index d46c111..8f7fa84 100644
--- a/src/foundation/StringTable.h
+++ b/src/foundation/StringTable.h
@@ -248,6 +248,8 @@ namespace foundation {
         virtual CRegisteredString RegisterStr(const char32_t *str) = 0;
 
         virtual CStringHandle GetHandle(Qt3DSBCharPtr str) = 0;
+        virtual CStringHandle getDynamicHandle(const QByteArray &str) = 0;
+        virtual void releaseDynamicHandle(QT3DSU32 strHandle) = 0;
         virtual CRegisteredString HandleToStr(QT3DSU32 strHandle) = 0;
 
         virtual CRegisteredString RegisterStr(const wchar_t *str) = 0;
diff --git a/src/runtime/Qt3DSElementSystem.cpp b/src/runtime/Qt3DSElementSystem.cpp
index b4d3fbc..f77bac5 100644
--- a/src/runtime/Qt3DSElementSystem.cpp
+++ b/src/runtime/Qt3DSElementSystem.cpp
@@ -673,6 +673,8 @@ void SElement::SetAttribute(TPropertyDescAndValuePtr inKey, const Q3DStudio::UVa
         case Q3DStudio::ATTRIBUTETYPE_STRING:
             if (currentValue->m_StringHandle == inValue.m_StringHandle)
                 return;
+            m_BelongedPresentation->GetStringTable().releaseDynamicHandle(
+                        currentValue->m_StringHandle);
             break;
         default: // Early return
             if (Q3DStudio::ATTRIBUTE_EYEBALL != attHash && currentValue->m_INT32 == inValue.m_INT32)
diff --git a/src/runtime/Qt3DSQmlElementHelper.cpp b/src/runtime/Qt3DSQmlElementHelper.cpp
index 81649f2..0a13a1f 100644
--- a/src/runtime/Qt3DSQmlElementHelper.cpp
+++ b/src/runtime/Qt3DSQmlElementHelper.cpp
@@ -210,8 +210,8 @@ bool CQmlElementHelper::SetAttribute(TElement *theElement, const char *theAttNam
 
         case ATTRIBUTETYPE_STRING:
             theNewValue.m_StringHandle =
-                theElement->GetBelongedPresentation()->GetStringTable().GetHandle(
-                    (const char *)value);
+                theElement->GetBelongedPresentation()->GetStringTable().getDynamicHandle(
+                        QByteArray(static_cast<const char *>(value)));
             break;
 
         case ATTRIBUTETYPE_POINTER:
diff --git a/src/runtimerender/Qt3DSDistanceFieldRenderer.cpp b/src/runtimerender/Qt3DSDistanceFieldRenderer.cpp
index d445a3f..f28e545 100644
--- a/src/runtimerender/Qt3DSDistanceFieldRenderer.cpp
+++ b/src/runtimerender/Qt3DSDistanceFieldRenderer.cpp
@@ -91,20 +91,32 @@ void Q3DSDistanceFieldRenderer::EndFrame()
     // Remove meshes for glyphs that weren't rendered last frame
     NVAllocatorCallback &alloc = m_context->GetAllocator();
 
-    QHash<size_t, Q3DSDistanceFieldMesh>::const_iterator it = m_meshCache.constBegin();
-    while (it != m_meshCache.constEnd()) {
-        const size_t glyphHash = it.key();
-        const Q3DSDistanceFieldMesh &mesh = it.value();
+    QHash<size_t, QHash<Q3DSDistanceFieldGlyphCache::TextureInfo *, GlyphInfo>>::const_iterator
+            glyphIt = m_glyphCache.constBegin();
+    while (glyphIt != m_glyphCache.constEnd()) {
+        const size_t textHash = glyphIt.key();
+        if (!m_renderedTexts.contains(textHash))
+            m_glyphCache.erase(glyphIt++);
+        else
+            glyphIt++;
+    }
+
+    QHash<size_t, Q3DSDistanceFieldMesh>::const_iterator meshIt = m_meshCache.constBegin();
+    while (meshIt != m_meshCache.constEnd()) {
+        const size_t glyphHash = meshIt.key();
+        const Q3DSDistanceFieldMesh &mesh = meshIt.value();
         if (!m_renderedGlyphs.contains(glyphHash)) {
             NVDelete(alloc, mesh.vertexBuffer);
             NVDelete(alloc, mesh.indexBuffer);
             NVDelete(alloc, mesh.inputAssembler);
-            m_meshCache.erase(it++);
+            m_meshCache.erase(meshIt++);
         } else {
-            it++;
+            meshIt++;
         }
     }
+
     m_renderedGlyphs.clear();
+    m_renderedTexts.clear();
 }
 
 QHash<Q3DSDistanceFieldGlyphCache::TextureInfo *, GlyphInfo>
@@ -950,6 +962,8 @@ void Q3DSDistanceFieldRenderer::renderText(SText &text, const QT3DSMat44 &mvp)
         m_renderedGlyphs += glyphHashValue;
     }
 
+    m_renderedTexts += textHashValue;
+
     text.m_Bounds = NVBounds3(minimum, maximum);
 }
 
diff --git a/src/runtimerender/Qt3DSDistanceFieldRenderer.h b/src/runtimerender/Qt3DSDistanceFieldRenderer.h
index 3af19a9..a64fddd 100644
--- a/src/runtimerender/Qt3DSDistanceFieldRenderer.h
+++ b/src/runtimerender/Qt3DSDistanceFieldRenderer.h
@@ -153,6 +153,7 @@ private:
     Q3DSDistanceFieldShader m_shader;
     Q3DSDistanceFieldDropShadowShader m_dropShadowShader;
     QVector<size_t> m_renderedGlyphs;
+    QVector<size_t> m_renderedTexts;
 
     QStringList m_systemDirs;
     QStringList m_projectDirs;