From 7efdf7ae39703afd96212bbbca8bb94f69f39dd4 Mon Sep 17 00:00:00 2001
From: Eskil Abrahamsen Blomfeldt <eskil.abrahamsen-blomfeldt@nokia.com>
Date: Mon, 5 Mar 2012 17:25:50 +0100
Subject: Fix missing glyphs when using shared distance-field cache

When using a shared distance field cache, signals such as
itemsMissing() and itemsUpdated() can be triggered in the
server process by external requests. This, in turn, can
lead to unnecessary storeGlyphs() calls (performance hit)
and, even worse, calls to setGlyphPositions() for glyphs
which have not previously been requested through populate().
The latter could cause missing glyphs when the same
characters were requested later, as they would then be
stored in the cache with the bounding rect of a default
constructed QPainterPath (in setGlyphPositions()).

To fix this, we ignore all glyphs which have no been requested
in this process, thus filtering out all events for glyphs which
have only been used externally so far. I've also added an
assert to the setGlyphPositions() to help us catch this case
early if it should return.

Change-Id: Ief333f612e4affea768d9c60409d43ce29fe3f60
Reviewed-by: Jiang Jiang <jiang.jiang@nokia.com>
---
 src/quick/scenegraph/qsgadaptationlayer.cpp        |  2 ++
 .../qsgshareddistancefieldglyphcache.cpp           | 35 +++++++++++++---------
 .../qsgshareddistancefieldglyphcache_p.h           |  1 +
 3 files changed, 24 insertions(+), 14 deletions(-)

(limited to 'src/quick')

diff --git a/src/quick/scenegraph/qsgadaptationlayer.cpp b/src/quick/scenegraph/qsgadaptationlayer.cpp
index 92045c55f4..9608ebe861 100644
--- a/src/quick/scenegraph/qsgadaptationlayer.cpp
+++ b/src/quick/scenegraph/qsgadaptationlayer.cpp
@@ -243,6 +243,8 @@ void QSGDistanceFieldGlyphCache::setGlyphsPosition(const QList<GlyphPosition> &g
     for (int i = 0; i < count; ++i) {
         GlyphPosition glyph = glyphs.at(i);
 
+        Q_ASSERT(m_cacheData->glyphPaths.contains(glyph.glyph));
+
         QPainterPath path = m_cacheData->glyphPaths.value(glyph.glyph);
         QRectF br = path.boundingRect();
         TexCoord c;
diff --git a/src/quick/scenegraph/qsgshareddistancefieldglyphcache.cpp b/src/quick/scenegraph/qsgshareddistancefieldglyphcache.cpp
index 46bd141909..76fb9036f4 100644
--- a/src/quick/scenegraph/qsgshareddistancefieldglyphcache.cpp
+++ b/src/quick/scenegraph/qsgshareddistancefieldglyphcache.cpp
@@ -126,6 +126,7 @@ void QSGSharedDistanceFieldGlyphCache::requestGlyphs(const QSet<glyph_t> &glyphs
 #endif
 
     m_requestedGlyphsThatHaveNotBeenReturned.unite(glyphs);
+    m_requestedGlyphs.unite(glyphs);
 
     QVector<quint32> glyphsVector;
     glyphsVector.reserve(glyphs.size());
@@ -198,6 +199,8 @@ void QSGSharedDistanceFieldGlyphCache::releaseGlyphs(const QSet<glyph_t> &glyphs
            m_cacheId.constData(), glyphs.size());
 #endif
 
+    m_requestedGlyphs.subtract(glyphs);
+
     QVector<quint32> glyphsVector;
     glyphsVector.reserve(glyphs.size());
 
@@ -588,19 +591,21 @@ void QSGSharedDistanceFieldGlyphCache::reportItemsUpdated(const QByteArray &cach
 #endif
 
         for (int i=0; i<itemIds.size(); ++i) {
-            PendingGlyph &pendingGlyph = m_pendingReadyGlyphs[itemIds.at(i)];
-            void *oldBuffer = pendingGlyph.buffer;
-            Q_ASSERT(bufferSize.height() >= pendingGlyph.bufferSize.height());
+            if (m_requestedGlyphs.contains(itemIds.at(i))) {
+                PendingGlyph &pendingGlyph = m_pendingReadyGlyphs[itemIds.at(i)];
+                void *oldBuffer = pendingGlyph.buffer;
+                Q_ASSERT(bufferSize.height() >= pendingGlyph.bufferSize.height());
 
-            pendingGlyph.buffer = bufferId;
-            pendingGlyph.position = positions.at(i);
-            pendingGlyph.bufferSize = bufferSize;
+                pendingGlyph.buffer = bufferId;
+                pendingGlyph.position = positions.at(i);
+                pendingGlyph.bufferSize = bufferSize;
 
-            m_sharedGraphicsCache->referenceBuffer(bufferId);
-            if (oldBuffer != 0)
-                m_sharedGraphicsCache->dereferenceBuffer(oldBuffer);
+                m_sharedGraphicsCache->referenceBuffer(bufferId);
+                if (oldBuffer != 0)
+                    m_sharedGraphicsCache->dereferenceBuffer(oldBuffer);
 
-            m_requestedGlyphsThatHaveNotBeenReturned.remove(itemIds.at(i));
+                m_requestedGlyphsThatHaveNotBeenReturned.remove(itemIds.at(i));
+            }
         }
     }
 
@@ -616,8 +621,10 @@ void QSGSharedDistanceFieldGlyphCache::reportItemsInvalidated(const QByteArray &
         if (m_cacheId != cacheId)
             return;
 
-        for (int i=0; i<itemIds.size(); ++i)
-            m_pendingInvalidatedGlyphs.insert(itemIds.at(i));
+        for (int i=0; i<itemIds.size(); ++i) {
+            if (m_requestedGlyphs.contains(itemIds.at(i)))
+                m_pendingInvalidatedGlyphs.insert(itemIds.at(i));
+        }
     }
 
     emit glyphsPending();
@@ -638,8 +645,8 @@ void QSGSharedDistanceFieldGlyphCache::reportItemsMissing(const QByteArray &cach
 #endif
 
         for (int i=0; i<itemIds.size(); ++i) {
-            m_pendingMissingGlyphs.insert(itemIds.at(i));
-            m_requestedGlyphsThatHaveNotBeenReturned.remove(itemIds.at(i));
+            if (m_requestedGlyphsThatHaveNotBeenReturned.remove(itemIds.at(i)))
+                m_pendingMissingGlyphs.insert(itemIds.at(i));
         }
     }
 
diff --git a/src/quick/scenegraph/qsgshareddistancefieldglyphcache_p.h b/src/quick/scenegraph/qsgshareddistancefieldglyphcache_p.h
index 3052f20813..4a91b4473a 100644
--- a/src/quick/scenegraph/qsgshareddistancefieldglyphcache_p.h
+++ b/src/quick/scenegraph/qsgshareddistancefieldglyphcache_p.h
@@ -87,6 +87,7 @@ private:
     void saveTexture(GLuint textureId, int width, int height);
 
     QSet<quint32> m_requestedGlyphsThatHaveNotBeenReturned;
+    QSet<quint32> m_requestedGlyphs;
     QWaitCondition m_pendingGlyphsCondition;
     QByteArray m_cacheId;
     QPlatformSharedGraphicsCache *m_sharedGraphicsCache;
-- 
cgit v1.2.3