Reduce QGlyphRun memory usage and speed up large Text instances

To render the plain text of _War and Peace_ took 6.4GB of memory before
(Linux 64-bit debug developer build); and now it's 1.1GB initially.
More optimization is possible: sizeof(TexturedPoint2D) = 16, and we
store a 16-bit index, so 3158176 glyphs ought to take about 60MB (but
takes 200MB in practice); but we also store QGlyphRuns, which are
actually redundant after the SG is populated. We need them in case the
text can be edited, restyled, re-wrapped etc.

QSGDistanceFieldGlyphNode::updateGeometry() enforces a limit of 65532
vertices per node (0xFFFF is not allowed as an index value, because it's
reserved to indicate primitive restart; and glyphs are quads). But it
was making copies of the remaining QPointF positions and quint32 glyph
indices and giving those recursively to the subnodes, so every subnode
was storing all of those for itself and all of _its_ subnodes, even
though it only needs max 16383 of them. Now, in the RootGlyphNode,
m_glyphs stores the glyphs that node will render (as before); but in
each subnode, we use QGlyphRun::setRawData() to populate m_glyphs with a
range from the index and position arrays from the root node.
setRawData() just sets the pointers to those arrays; so to avoid any
chance of dangling pointers, we need to keep those vectors, which live
in the GlyphInfo structs in m_glyphsInOtherTextures. That's the reason
the glyphsInOtherTextures hash, which was temporary before, is now kept
(although the dangling pointers somehow didn't cause any crash).

In the first loop over indices in updateGeometry(), each SubGlyphNode
will break out of the loop as soon as it has created enough
TexturedPoint2D and index instances for itself, because it no longer
needs to redo the rest of the loop over indices that was done in the
RootGlyphNode; this speeds up the rendering of large text.

Creation of subnodes is now iterative rather than recursive, which
saves stack space, and also looks better in the QSG_RENDERER_DEBUG=dump
output: you can see that each RootGlyphNode has one level of direct
children (161 of them for _War and Peace_). In a future change, perhaps
we could create those dynamically when the user flicks down to them,
and reclaim memory from children that are no longer shown; but that
would not be possible if they were all nested inside each other.

Amends aeb1d48c9938241b1ffcf8e42e3864595e90b168

Task-number: QTBUG-60491
Task-number: QTBUG-90734
Change-Id: Iff981b9ba86acc01775fd72e3ce79ea9e33d9061
Reviewed-by: Eskil Abrahamsen Blomfeldt <eskil.abrahamsen-blomfeldt@qt.io>
(cherry picked from commit 75a9b09d93b9462b77347d1992371a189037bc62)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>

2 files changed, 58 insertions, 28 deletions

diff --git a/src/quick/scenegraph/qsgdistancefieldglyphnode.cpp b/src/quick/scenegraph/qsgdistancefieldglyphnode.cpp
index 8ac112b106..5028370bd4 100644
--- a/src/quick/scenegraph/qsgdistancefieldglyphnode.cpp
+++ b/src/quick/scenegraph/qsgdistancefieldglyphnode.cpp
@@ -43,6 +43,10 @@
 
 QT_BEGIN_NAMESPACE
 
+Q_LOGGING_CATEGORY(lcSgText, "qt.scenegraph.text")
+
+qint64 QSGDistanceFieldGlyphNode::m_totalAllocation = 0;
+
 QSGDistanceFieldGlyphNode::QSGDistanceFieldGlyphNode(QSGRenderContext *context)
     : m_glyphNodeType(RootGlyphNode)
     , m_context(context)
@@ -135,6 +139,7 @@ void QSGDistanceFieldGlyphNode::setGlyphs(const QPointF &position, const QGlyphR
     const QVector<quint32> glyphIndexes = m_glyphs.glyphIndexes();
     for (int i = 0; i < glyphIndexes.count(); ++i)
         m_allGlyphIndexesLookup.insert(glyphIndexes.at(i));
+    qCDebug(lcSgText, "inserting %lld glyphs, %lld unique", glyphIndexes.count(), m_allGlyphIndexesLookup.count());
 }
 
 void QSGDistanceFieldGlyphNode::setStyle(QQuickText::TextStyle style)
@@ -198,8 +203,7 @@ void QSGDistanceFieldGlyphNode::updateGeometry()
     QSGGeometry *g = geometry();
 
     Q_ASSERT(g->indexType() == QSGGeometry::UnsignedShortType);
-
-    QHash<const QSGDistanceFieldGlyphCache::Texture *, GlyphInfo> glyphsInOtherTextures;
+    m_glyphsInOtherTextures.clear();
 
     const QVector<quint32> indexes = m_glyphs.glyphIndexes();
     const QVector<QPointF> positions = m_glyphs.positions();
@@ -208,9 +212,12 @@ void QSGDistanceFieldGlyphNode::updateGeometry()
     // The template parameters here are assuming that most strings are short, 64
     // characters or less.
     QVarLengthArray<QSGGeometry::TexturedPoint2D, 256> vp;
-    vp.reserve(indexes.size() * 4);
     QVarLengthArray<ushort, 384> ip;
-    ip.reserve(indexes.size() * 6);
+    const qsizetype maxIndexCount = (std::numeric_limits<quint16>::max() - 1) / 4; // 16383 (see below: 0xFFFF is not allowed)
+    const qsizetype maxVertexCount = maxIndexCount * 4; // 65532
+    const auto likelyGlyphCount = qMin(indexes.size(), maxIndexCount);
+    vp.reserve(likelyGlyphCount * 4);
+    ip.reserve(likelyGlyphCount * 6);
 
     qreal maxTexMargin = m_glyph_cache->distanceFieldRadius();
     qreal fontScale = m_glyph_cache->fontScale(fontPixelSize);
@@ -236,15 +243,20 @@ void QSGDistanceFieldGlyphNode::updateGeometry()
 
         // As we use UNSIGNED_SHORT indexing in the geometry, we overload the
         // "glyphsInOtherTextures" concept as overflow for if there are more
-        // than 65535 vertices to render which would otherwise exceed the
+        // than 65532 vertices to render, which would otherwise exceed the
         // maximum index size. (leave 0xFFFF unused in order not to clash with
-        // primitive restart) This will cause sub-nodes to be recursively
-        // created to handle any number of glyphs.
-        if (m_texture != texture || vp.size() >= 65535) {
-            if (texture->texture) {
-                GlyphInfo &glyphInfo = glyphsInOtherTextures[texture];
+        // primitive restart) This will cause sub-nodes to be
+        // created to handle any number of glyphs. But only the RootGlyphNode
+        // needs to do this classification; from the perspective of a SubGlyphNode,
+        // it's already done, and m_glyphs contains only pointers to ranges of
+        // indices and positions that the RootGlyphNode is storing.
+        if (m_texture != texture || vp.size() >= maxVertexCount) {
+            if (m_glyphNodeType == RootGlyphNode && texture->texture) {
+                GlyphInfo &glyphInfo = m_glyphsInOtherTextures[texture];
                 glyphInfo.indexes.append(glyphIndex);
                 glyphInfo.positions.append(position);
+            } else if (vp.size() >= maxVertexCount && m_glyphNodeType == SubGlyphNode) {
+                break; // out of this loop over indices, because we won't add any more vertices
             }
             continue;
         }
@@ -303,26 +315,40 @@ void QSGDistanceFieldGlyphNode::updateGeometry()
         ip.append(o + 0);
     }
 
-    QHash<const QSGDistanceFieldGlyphCache::Texture *, GlyphInfo>::const_iterator ite = glyphsInOtherTextures.constBegin();
-    while (ite != glyphsInOtherTextures.constEnd()) {
-        QGlyphRun subNodeGlyphRun(m_glyphs);
-        subNodeGlyphRun.setGlyphIndexes(ite->indexes);
-        subNodeGlyphRun.setPositions(ite->positions);
-
-        QSGDistanceFieldGlyphNode *subNode = new QSGDistanceFieldGlyphNode(m_context);
-        subNode->setGlyphNodeType(SubGlyphNode);
-        subNode->setColor(m_color);
-        subNode->setStyle(m_style);
-        subNode->setStyleColor(m_styleColor);
-        subNode->setPreferredAntialiasingMode(m_antialiasingMode);
-        subNode->setGlyphs(m_originalPosition, subNodeGlyphRun);
-        subNode->update();
-        subNode->updateGeometry(); // we have to explicitly call this now as preprocess won't be called before it's rendered
-        appendChildNode(subNode);
-
-        ++ite;
+    int subnodeCount = 0;
+    if (m_glyphNodeType == SubGlyphNode) {
+        Q_ASSERT(m_glyphsInOtherTextures.isEmpty());
+    } else {
+        if (!m_glyphsInOtherTextures.isEmpty())
+            qCDebug(lcSgText, "%lld 'other' textures", m_glyphsInOtherTextures.count());
+        QHash<const QSGDistanceFieldGlyphCache::Texture *, GlyphInfo>::const_iterator ite = m_glyphsInOtherTextures.constBegin();
+        while (ite != m_glyphsInOtherTextures.constEnd()) {
+            QGlyphRun subNodeGlyphRun(m_glyphs);
+            for (int i = 0; i < ite->indexes.count(); i += maxIndexCount) {
+                int len = qMin(maxIndexCount, ite->indexes.count() - i);
+                subNodeGlyphRun.setRawData(ite->indexes.constData() + i, ite->positions.constData() + i, len);
+                qCDebug(lcSgText) << "subNodeGlyphRun has" << len << "positions:"
+                                  << *(ite->positions.constData() + i) << "->" << *(ite->positions.constData() + i + len - 1);
+
+                QSGDistanceFieldGlyphNode *subNode = new QSGDistanceFieldGlyphNode(m_context);
+                subNode->setGlyphNodeType(SubGlyphNode);
+                subNode->setColor(m_color);
+                subNode->setStyle(m_style);
+                subNode->setStyleColor(m_styleColor);
+                subNode->setPreferredAntialiasingMode(m_antialiasingMode);
+                subNode->setGlyphs(m_originalPosition, subNodeGlyphRun);
+                subNode->update();
+                subNode->updateGeometry(); // we have to explicitly call this now as preprocess won't be called before it's rendered
+                appendChildNode(subNode);
+                ++subnodeCount;
+            }
+            ++ite;
+        }
     }
 
+    m_totalAllocation += vp.size() * sizeof(QSGGeometry::TexturedPoint2D) + ip.size() * sizeof(quint16);
+    qCDebug(lcSgText) << "allocating for" << vp.size() << "vtx (reserved" << likelyGlyphCount * 4 << "):" << vp.size() * sizeof(QSGGeometry::TexturedPoint2D)
+            << "bytes;" << ip.size() << "idx:" << ip.size() * sizeof(quint16) << "bytes; total bytes so far" << m_totalAllocation;
     g->allocate(vp.size(), ip.size());
     memcpy(g->vertexDataAsTexturedPoint2D(), vp.constData(), vp.size() * sizeof(QSGGeometry::TexturedPoint2D));
     memcpy(g->indexDataAsUShort(), ip.constData(), ip.size() * sizeof(quint16));
diff --git a/src/quick/scenegraph/qsgdistancefieldglyphnode_p.h b/src/quick/scenegraph/qsgdistancefieldglyphnode_p.h
index 2b2975ccb3..e7655f502d 100644
--- a/src/quick/scenegraph/qsgdistancefieldglyphnode_p.h
+++ b/src/quick/scenegraph/qsgdistancefieldglyphnode_p.h
@@ -115,9 +115,13 @@ private:
         QVector<QPointF> positions;
     };
     QSet<quint32> m_allGlyphIndexesLookup;
+    // m_glyphs holds pointers to the GlyphInfo.indexes and positions arrays, so we need to hold on to them
+    QHash<const QSGDistanceFieldGlyphCache::Texture *, GlyphInfo> m_glyphsInOtherTextures;
 
     uint m_dirtyGeometry: 1;
     uint m_dirtyMaterial: 1;
+
+    static qint64 m_totalAllocation; // all SG glyph vertices and indices; only for qCDebug metrics
 };
 
 QT_END_NAMESPACE