Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Fix memory leak with the shared distance field glyph cache.
A new glyph node instance registers its owner element with its
glyph cache which in the case of the shared distance field glyph cache
connects a signal from the cache to a slot on the owner, changing the
text creates a new node but destroying the old node didn't remove the
connection causing them to accumulate over time.

In the glyph node; unregister the owner element from the cache on
destruction, and in the cache only connect an owner element the
first time it is registered and keep a reference count so the
items can be disconnected when registrations from all nodes have
been cancelled.

Change-Id: Ibf32a146e146dbbf4e0556d1bd756465bd8e45ba
Reviewed-by: Eskil Abrahamsen Blomfeldt <eskil.abrahamsen-blomfeldt@nokia.com>
  • Loading branch information
Andrew den Exter authored and Qt by Nokia committed Mar 14, 2012
1 parent d73e6d4 commit 4cd2f0b
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 4 deletions.
1 change: 1 addition & 0 deletions src/quick/scenegraph/qsgdistancefieldglyphnode.cpp
Expand Up @@ -76,6 +76,7 @@ QSGDistanceFieldGlyphNode::~QSGDistanceFieldGlyphNode()
glyphIndexes.append(m_allGlyphs.at(i).glyphIndex);
m_glyph_cache->release(glyphIndexes);
m_glyph_cache->unregisterGlyphNode(this);
m_glyph_cache->unregisterOwnerElement(ownerElement());
}

for (int i = 0; i < m_nodesToDelete.count(); ++i)
Expand Down
19 changes: 15 additions & 4 deletions src/quick/scenegraph/qsgshareddistancefieldglyphcache.cpp
Expand Up @@ -234,14 +234,25 @@ void QSGSharedDistanceFieldGlyphCache::releaseGlyphs(const QSet<glyph_t> &glyphs

void QSGSharedDistanceFieldGlyphCache::registerOwnerElement(QQuickItem *ownerElement)
{
bool ok = connect(this, SIGNAL(glyphsPending()), ownerElement, SLOT(triggerPreprocess()));
Q_ASSERT_X(ok, Q_FUNC_INFO, "QML element that owns a glyph node must have triggerPreprocess() slot");
Q_UNUSED(ok);
Owner &owner = m_registeredOwners[ownerElement];
if (owner.ref == 0) {
owner.item = ownerElement;

bool ok = connect(this, SIGNAL(glyphsPending()), ownerElement, SLOT(triggerPreprocess()));
Q_ASSERT_X(ok, Q_FUNC_INFO, "QML element that owns a glyph node must have triggerPreprocess() slot");
Q_UNUSED(ok);
}
++owner.ref;
}

void QSGSharedDistanceFieldGlyphCache::unregisterOwnerElement(QQuickItem *ownerElement)
{
disconnect(this, SIGNAL(glyphsPending()), ownerElement, SLOT(triggerPreprocess()));
QHash<QQuickItem *, Owner>::iterator it = m_registeredOwners.find(ownerElement);
if (it != m_registeredOwners.end() && --it->ref <= 0) {
if (it->item)
disconnect(this, SIGNAL(glyphsPending()), ownerElement, SLOT(triggerPreprocess()));
m_registeredOwners.erase(it);
}
}

#if defined(QSGSHAREDDISTANCEFIELDGLYPHCACHE_DEBUG_)
Expand Down
12 changes: 12 additions & 0 deletions src/quick/scenegraph/qsgshareddistancefieldglyphcache_p.h
Expand Up @@ -44,6 +44,7 @@

#include <QtCore/qwaitcondition.h>
#include <private/qsgadaptationlayer_p.h>
#include <private/qqmlguard_p.h>

QT_BEGIN_HEADER

Expand Down Expand Up @@ -105,8 +106,19 @@ private Q_SLOTS:
QPoint position;
};

struct Owner
{
Owner() : ref(0) {}
Owner(const Owner &o) : item(o.item), ref(o.ref) {}
Owner &operator =(const Owner &o) { item = o.item; ref = o.ref; return *this; }

QQmlGuard<QQuickItem> item;
int ref;
};

QHash<quint32, PendingGlyph> m_pendingReadyGlyphs;
QHash<glyph_t, void *> m_bufferForGlyph;
QHash<QQuickItem *, Owner> m_registeredOwners;
};

QT_END_NAMESPACE
Expand Down

0 comments on commit 4cd2f0b

Please sign in to comment.