CoreText: Simplify and share code for loading glyph advances

The function doesn't need the flags argument, nor does it need the
ctfont or fontdef arguments if it's a normal const member function.

It can also be used from QCoreTextFontEngine::stringToCMap(), instead
of duplicating the code. This was originally the case before b4aa5d97
which improved surrogate pair handling, but for some reason the change
introduced the duplicate code instead of just changing the arguments
in the function call slightly.

The use of 0xff000000 to skip certain glyphs looks dubious, and is
probably related to QFontEngineMulti's use of the high byte to
indicate which engine the glyph came from, but the multi engine
strips this away before calling out to the concrete engine so
it could potentially be removed in a later patch.

Change-Id: I6c693595616da1b69fdbe3d7a31e392a8443369d
Reviewed-by: Simon Hausmann <simon.hausmann@qt.io>

2 files changed, 27 insertions, 40 deletions

diff --git a/src/platformsupport/fontdatabases/mac/qfontengine_coretext.mm b/src/platformsupport/fontdatabases/mac/qfontengine_coretext.mm
index c00b9c3d46..a58ea71b19 100644
--- a/src/platformsupport/fontdatabases/mac/qfontengine_coretext.mm
+++ b/src/platformsupport/fontdatabases/mac/qfontengine_coretext.mm
@@ -133,28 +133,6 @@ QFont::Weight QCoreTextFontEngine::qtWeightFromCFWeight(float value)
     return ret;
 }
 
-static void loadAdvancesForGlyphs(CTFontRef ctfont,
-                                  QVarLengthArray<CGGlyph> &cgGlyphs,
-                                  QGlyphLayout *glyphs, int len,
-                                  QFontEngine::ShaperFlags flags,
-                                  const QFontDef &fontDef)
-{
-    Q_UNUSED(flags);
-    QVarLengthArray<CGSize> advances(len);
-    CTFontGetAdvancesForGlyphs(ctfont, kCTFontOrientationHorizontal, cgGlyphs.data(), advances.data(), len);
-
-    for (int i = 0; i < len; ++i) {
-        if (glyphs->glyphs[i] & 0xff000000)
-            continue;
-        glyphs->advances[i] = QFixed::fromReal(advances[i].width);
-    }
-
-    if (fontDef.styleStrategy & QFont::ForceIntegerMetrics) {
-        for (int i = 0; i < len; ++i)
-            glyphs->advances[i] = glyphs->advances[i].round();
-    }
-}
-
 int QCoreTextFontEngine::antialiasingThreshold = 0;
 QFontEngine::GlyphFormat QCoreTextFontEngine::defaultGlyphFormat = QFontEngine::Format_A32;
 
@@ -360,22 +338,9 @@ bool QCoreTextFontEngine::stringToCMap(const QChar *str, int len, QGlyphLayout *
     *nglyphs = glyph_pos;
     glyphs->numGlyphs = glyph_pos;
 
-    if (flags & GlyphIndicesOnly)
-        return true;
-
-    QVarLengthArray<CGSize> advances(glyph_pos);
-    CTFontGetAdvancesForGlyphs(ctfont, kCTFontOrientationHorizontal, cgGlyphs.data(), advances.data(), glyph_pos);
-
-    for (int i = 0; i < glyph_pos; ++i) {
-        if (glyphs->glyphs[i] & 0xff000000)
-            continue;
-        glyphs->advances[i] = QFixed::fromReal(advances[i].width);
-    }
+    if (!(flags & GlyphIndicesOnly))
+        loadAdvancesForGlyphs(cgGlyphs, glyphs);
 
-    if (fontDef.styleStrategy & QFont::ForceIntegerMetrics) {
-        for (int i = 0; i < glyph_pos; ++i)
-            glyphs->advances[i] = glyphs->advances[i].round();
-    }
     return true;
 }
 
@@ -801,17 +766,37 @@ QImage QCoreTextFontEngine::bitmapForGlyph(glyph_t glyph, QFixed subPixelPositio
 
 void QCoreTextFontEngine::recalcAdvances(QGlyphLayout *glyphs, QFontEngine::ShaperFlags flags) const
 {
-    int i, numGlyphs = glyphs->numGlyphs;
+    Q_UNUSED(flags);
+
+    const int numGlyphs = glyphs->numGlyphs;
     QVarLengthArray<CGGlyph> cgGlyphs(numGlyphs);
 
-    for (i = 0; i < numGlyphs; ++i) {
+    for (int i = 0; i < numGlyphs; ++i) {
         if (glyphs->glyphs[i] & 0xff000000)
             cgGlyphs[i] = 0;
         else
             cgGlyphs[i] = glyphs->glyphs[i];
     }
 
-    loadAdvancesForGlyphs(ctfont, cgGlyphs, glyphs, numGlyphs, flags, fontDef);
+    loadAdvancesForGlyphs(cgGlyphs, glyphs);
+}
+
+void QCoreTextFontEngine::loadAdvancesForGlyphs(QVarLengthArray<CGGlyph> &cgGlyphs, QGlyphLayout *glyphs) const
+{
+    const int numGlyphs = glyphs->numGlyphs;
+    QVarLengthArray<CGSize> advances(numGlyphs);
+    CTFontGetAdvancesForGlyphs(ctfont, kCTFontOrientationHorizontal, cgGlyphs.data(), advances.data(), numGlyphs);
+
+    for (int i = 0; i < numGlyphs; ++i) {
+        if (glyphs->glyphs[i] & 0xff000000)
+            continue;
+        glyphs->advances[i] = QFixed::fromReal(advances[i].width);
+    }
+
+    if (fontDef.styleStrategy & QFont::ForceIntegerMetrics) {
+        for (int i = 0; i < numGlyphs; ++i)
+            glyphs->advances[i] = glyphs->advances[i].round();
+    }
 }
 
 QFontEngine::FaceId QCoreTextFontEngine::faceId() const
diff --git a/src/platformsupport/fontdatabases/mac/qfontengine_coretext_p.h b/src/platformsupport/fontdatabases/mac/qfontengine_coretext_p.h
index b77aaa27c1..f4213a2ffa 100644
--- a/src/platformsupport/fontdatabases/mac/qfontengine_coretext_p.h
+++ b/src/platformsupport/fontdatabases/mac/qfontengine_coretext_p.h
@@ -128,6 +128,8 @@ public:
 protected:
     void init();
     QImage imageForGlyph(glyph_t glyph, QFixed subPixelPosition, bool colorful, const QTransform &m);
+    void loadAdvancesForGlyphs(QVarLengthArray<CGGlyph> &cgGlyphs, QGlyphLayout *glyphs) const;
+
     CTFontRef ctfont;
     CGFontRef cgFont;
     int synthesisFlags;