Optimize texture glyph cache

To avoid calling the bounding box function for the same non-printable
characters in every call, we cache them as empty in the coords table
and bail out before painting them. This patch also fixes some possible
reads of uninitialized memory (reference to return value) and reference
to a default constructed object consisting of uninitialized pods.

Reviewed-by: Samuel

4 files changed, 28 insertions, 12 deletions

diff --git a/src/gui/painting/qpaintengine_raster.cpp b/src/gui/painting/qpaintengine_raster.cpp
index 97dfddf..ec5cef0 100644
--- a/src/gui/painting/qpaintengine_raster.cpp
+++ b/src/gui/painting/qpaintengine_raster.cpp
@@ -3111,7 +3111,9 @@ void QRasterPaintEngine::drawCachedGlyphs(int numGlyphs, const glyph_t *glyphs,
         if (supportsSubPixelPositions)
             subPixelPosition = cache->subPixelPositionForX(positions[i].x);
         QTextureGlyphCache::GlyphAndSubPixelPosition glyph(glyphs[i], subPixelPosition);
-        const QTextureGlyphCache::Coord &c = cache->coords.value(glyph);
+        const QTextureGlyphCache::Coord &c = cache->coords[glyph];
+        if (c.isNull())
+            continue;
 
         int x = qFloor(positions[i].x) + c.baseLineX - margin;
         int y = qFloor(positions[i].y) - c.baseLineY - margin;
diff --git a/src/gui/painting/qtextureglyphcache.cpp b/src/gui/painting/qtextureglyphcache.cpp
index 60300c3..670ca95 100644
--- a/src/gui/painting/qtextureglyphcache.cpp
+++ b/src/gui/painting/qtextureglyphcache.cpp
@@ -175,11 +175,16 @@ bool QTextureGlyphCache::populate(QFontEngine *fontEngine, int numGlyphs, const
                metrics.yoff.toReal(),
                metrics.x.toReal(),
                metrics.y.toReal());
-#endif
+#endif        
+        GlyphAndSubPixelPosition key(glyph, subPixelPosition);
         int glyph_width = metrics.width.ceil().toInt();
         int glyph_height = metrics.height.ceil().toInt();
-        if (glyph_height == 0 || glyph_width == 0)
+        if (glyph_height == 0 || glyph_width == 0) {
+            // Avoid multiple calls to boundingBox() for non-printable characters
+            Coord c = { 0, 0, 0, 0, 0, 0 };
+            coords.insert(key, c);
             continue;
+        }
         glyph_width += margin * 2 + 4;
         glyph_height += margin * 2 + 4;
         // align to 8-bit boundary
@@ -192,7 +197,7 @@ bool QTextureGlyphCache::populate(QFontEngine *fontEngine, int numGlyphs, const
                     metrics.x.round().truncate(),
                     -metrics.y.truncate() }; // baseline for horizontal scripts
 
-        listItemCoordinates.insert(GlyphAndSubPixelPosition(glyph, subPixelPosition), c);
+        listItemCoordinates.insert(key, c);
         rowHeight = qMax(rowHeight, glyph_height);
     }
     if (listItemCoordinates.isEmpty())
diff --git a/src/gui/painting/qtextureglyphcache_p.h b/src/gui/painting/qtextureglyphcache_p.h
index 4227e9a..7a0bdfd 100644
--- a/src/gui/painting/qtextureglyphcache_p.h
+++ b/src/gui/painting/qtextureglyphcache_p.h
@@ -107,6 +107,11 @@ public:
 
         int baseLineX;
         int baseLineY;
+
+        bool isNull() const
+        {
+            return w == 0 || h == 0;
+        }
     };
 
     bool populate(QFontEngine *fontEngine, int numGlyphs, const glyph_t *glyphs,
diff --git a/src/opengl/gl2paintengineex/qpaintengineex_opengl2.cpp b/src/opengl/gl2paintengineex/qpaintengineex_opengl2.cpp
index cdd785a..771d33f 100644
--- a/src/opengl/gl2paintengineex/qpaintengineex_opengl2.cpp
+++ b/src/opengl/gl2paintengineex/qpaintengineex_opengl2.cpp
@@ -1556,7 +1556,6 @@ void QGL2PaintEngineExPrivate::drawCachedGlyphs(QFontEngineGlyphCache::Type glyp
         }
     }
 
-
     if (recreateVertexArrays) {
         vertexCoordinates->clear();
         textureCoordinates->clear();
@@ -1569,7 +1568,10 @@ void QGL2PaintEngineExPrivate::drawCachedGlyphs(QFontEngineGlyphCache::Type glyp
 
             QTextureGlyphCache::GlyphAndSubPixelPosition glyph(staticTextItem->glyphs[i], subPixelPosition);
 
-            const QTextureGlyphCache::Coord &c = cache->coords.value(glyph);
+            const QTextureGlyphCache::Coord &c = cache->coords[glyph];
+            if (c.isNull())
+                continue;
+
             int x = staticTextItem->glyphPositions[i].x.toInt() + c.baseLineX - margin;
             int y = staticTextItem->glyphPositions[i].y.toInt() - c.baseLineY - margin;
 
@@ -1580,10 +1582,12 @@ void QGL2PaintEngineExPrivate::drawCachedGlyphs(QFontEngineGlyphCache::Type glyp
         staticTextItem->userDataNeedsUpdate = false;
     }
 
-    if (elementIndices.size() < staticTextItem->numGlyphs*6) {
+    int numGlyphs = vertexCoordinates->vertexCount() / 4;
+
+    if (elementIndices.size() < numGlyphs*6) {
         Q_ASSERT(elementIndices.size() % 6 == 0);
         int j = elementIndices.size() / 6 * 4;
-        while (j < staticTextItem->numGlyphs*4) {
+        while (j < numGlyphs*4) {
             elementIndices.append(j + 0);
             elementIndices.append(j + 0);
             elementIndices.append(j + 1);
@@ -1672,9 +1676,9 @@ void QGL2PaintEngineExPrivate::drawCachedGlyphs(QFontEngineGlyphCache::Type glyp
             updateTextureFilter(GL_TEXTURE_2D, GL_REPEAT, false);
 
 #if defined(QT_OPENGL_DRAWCACHEDGLYPHS_INDEX_ARRAY_VBO)
-            glDrawElements(GL_TRIANGLE_STRIP, 6 * staticTextItem->numGlyphs, GL_UNSIGNED_SHORT, 0);
+            glDrawElements(GL_TRIANGLE_STRIP, 6 * numGlyphs, GL_UNSIGNED_SHORT, 0);
 #else
-            glDrawElements(GL_TRIANGLE_STRIP, 6 * staticTextItem->numGlyphs, GL_UNSIGNED_SHORT, elementIndices.data());
+            glDrawElements(GL_TRIANGLE_STRIP, 6 * numGlyphs, GL_UNSIGNED_SHORT, elementIndices.data());
 #endif
 
             shaderManager->setMaskType(QGLEngineShaderManager::SubPixelMaskPass2);
@@ -1722,10 +1726,10 @@ void QGL2PaintEngineExPrivate::drawCachedGlyphs(QFontEngineGlyphCache::Type glyp
     }
 
 #if defined(QT_OPENGL_DRAWCACHEDGLYPHS_INDEX_ARRAY_VBO)
-    glDrawElements(GL_TRIANGLE_STRIP, 6 * staticTextItem->numGlyphs, GL_UNSIGNED_SHORT, 0);
+    glDrawElements(GL_TRIANGLE_STRIP, 6 * numGlyphs, GL_UNSIGNED_SHORT, 0);
     glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, 0);
 #else
-    glDrawElements(GL_TRIANGLE_STRIP, 6 * staticTextItem->numGlyphs, GL_UNSIGNED_SHORT, elementIndices.data());
+    glDrawElements(GL_TRIANGLE_STRIP, 6 * numGlyphs, GL_UNSIGNED_SHORT, elementIndices.data());
 #endif
 }