From d66674bf53a572c1788ae35649539c3d381de875 Mon Sep 17 00:00:00 2001 From: Miikka Heikkinen Date: Wed, 19 Jun 2019 13:57:02 +0300 Subject: Fix string type data inputs and setAttribute leaking memory MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Reviewed-by: Antti Määttä Reviewed-by: Janne Koskinen --- src/foundation/StringTable.cpp | 68 +++++++++++++++++++++++- src/foundation/StringTable.h | 2 + src/runtime/Qt3DSElementSystem.cpp | 2 + src/runtime/Qt3DSQmlElementHelper.cpp | 4 +- src/runtimerender/Qt3DSDistanceFieldRenderer.cpp | 26 ++++++--- src/runtimerender/Qt3DSDistanceFieldRenderer.h | 1 + 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 + 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 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 m_dynamicFreeHandlesMap; + QHash m_dynamicUsedHandlesMap; + QHash 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(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::const_iterator it = m_meshCache.constBegin(); - while (it != m_meshCache.constEnd()) { - const size_t glyphHash = it.key(); - const Q3DSDistanceFieldMesh &mesh = it.value(); + QHash>::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::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 @@ -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 m_renderedGlyphs; + QVector m_renderedTexts; QStringList m_systemDirs; QStringList m_projectDirs; -- cgit v1.2.3