Fix advance of zero-width spaces in fonts that omits the glyph

Some fonts appear to include zero-width space in the CMAP table, but
not include an actual definitions of the glyph they point to. The
missing glyph causes a warning, but isn't handled making the character
end up being giving the same metrics as whatever character it came after.

This patch adds explicit handling of missing glyphs, and also caches
their missing state when caching is enabled.

Task-number: QTBUG-40912
Change-Id: I06fba9c01df59548e750e36babfdd5a6bafd6bd0
Reviewed-by: Konstantin Ritt <ritt.ks@gmail.com>
Reviewed-by: Pierre Rossi <pierre.rossi@gmail.com>

2 files changed, 21 insertions, 7 deletions

diff --git a/src/gui/text/qfontengine_ft.cpp b/src/gui/text/qfontengine_ft.cpp
index cb3cb34d27..0a22038bb4 100644
--- a/src/gui/text/qfontengine_ft.cpp
+++ b/src/gui/text/qfontengine_ft.cpp
@@ -132,6 +132,7 @@ static bool ft_getSfntTable(void *user_data, uint tag, uchar *buffer, uint *leng
     return result;
 }
 
+static QFontEngineFT::Glyph emptyGlyph = {0, 0, 0, 0, 0, 0, 0, 0};
 
 // -------------------------- Freetype support ------------------------------
 
@@ -858,6 +859,9 @@ QFontEngineFT::Glyph *QFontEngineFT::loadGlyph(QGlyphSet *set, uint glyph,
     if (g && g->format == format && (fetchMetricsOnly || g->data))
         return g;
 
+    if (!g && set && set->isGlyphMissing(glyph))
+        return &emptyGlyph;
+
     QFontEngineFT::GlyphInfo info;
 
     Q_ASSERT(format != Format_None);
@@ -894,8 +898,12 @@ QFontEngineFT::Glyph *QFontEngineFT::loadGlyph(QGlyphSet *set, uint glyph,
         load_flags |= FT_LOAD_FORCE_AUTOHINT;
         err = FT_Load_Glyph(face, glyph, load_flags);
     }
-    if (err != FT_Err_Ok)
+    if (err != FT_Err_Ok) {
         qWarning("load glyph failed err=%x face=%p, glyph=%d", err, face, glyph);
+        if (set)
+            set->setGlyphMissing(glyph);
+        return &emptyGlyph;
+    }
 
     FT_GlyphSlot slot = face->glyph;
 
@@ -1633,9 +1641,12 @@ void QFontEngineFT::recalcAdvances(QGlyphLayout *glyphs, QFontEngine::ShaperFlag
             if (!face)
                 face = lockFace();
             g = loadGlyph(cacheEnabled ? &defaultGlyphSet : 0, glyphs->glyphs[i], 0, Format_None, true);
-            glyphs->advances[i] = design ? QFixed::fromFixed(face->glyph->linearHoriAdvance >> 10)
-                                         : QFixed::fromFixed(face->glyph->metrics.horiAdvance).round();
-            if (!cacheEnabled)
+            if (g)
+                glyphs->advances[i] = design ? QFixed::fromFixed(g->linearAdvance) : QFixed(g->advance);
+            else
+                glyphs->advances[i] = design ? QFixed::fromFixed(face->glyph->linearHoriAdvance >> 10)
+                                             : QFixed::fromFixed(face->glyph->metrics.horiAdvance).round();
+            if (!cacheEnabled && g != &emptyGlyph)
                 delete g;
         }
     }
@@ -1674,7 +1685,7 @@ glyph_metrics_t QFontEngineFT::boundingBox(const QGlyphLayout &glyphs)
             xmax = qMax(xmax, x + g->width);
             ymax = qMax(ymax, y + g->height);
             overall.xoff += g->advance;
-            if (!cacheEnabled)
+            if (!cacheEnabled && g != &emptyGlyph)
                 delete g;
         } else {
             int left  = FLOOR(face->glyph->metrics.horiBearingX);
@@ -1717,7 +1728,7 @@ glyph_metrics_t QFontEngineFT::boundingBox(glyph_t glyph)
         overall.xoff = g->advance;
         if (fontDef.styleStrategy & QFont::ForceIntegerMetrics)
             overall.xoff = overall.xoff.round();
-        if (!cacheEnabled)
+        if (!cacheEnabled && g != &emptyGlyph)
             delete g;
     } else {
         int left  = FLOOR(face->glyph->metrics.horiBearingX);
@@ -1808,7 +1819,7 @@ glyph_metrics_t QFontEngineFT::alphaMapBoundingBox(glyph_t glyph, QFixed subPixe
         overall.width = g->width;
         overall.height = g->height;
         overall.xoff = g->advance;
-        if (!glyphSet)
+        if (!glyphSet && g != &emptyGlyph)
             delete g;
     } else {
         int left  = FLOOR(face->glyph->metrics.horiBearingX);
diff --git a/src/gui/text/qfontengine_ft_p.h b/src/gui/text/qfontengine_ft_p.h
index 09c70c5083..e23024e472 100644
--- a/src/gui/text/qfontengine_ft_p.h
+++ b/src/gui/text/qfontengine_ft_p.h
@@ -187,8 +187,11 @@ public:
         inline Glyph *getGlyph(glyph_t index, QFixed subPixelPosition = 0) const;
         void setGlyph(glyph_t index, QFixed spp, Glyph *glyph);
 
+        inline bool isGlyphMissing(glyph_t index) const { return missing_glyphs.contains(index); }
+        inline void setGlyphMissing(glyph_t index) const { missing_glyphs.insert(index); }
 private:
         mutable QHash<GlyphAndSubPixelPosition, Glyph *> glyph_data; // maps from glyph index to glyph data
+        mutable QSet<glyph_t> missing_glyphs;
         mutable Glyph *fast_glyph_data[256]; // for fast lookup of glyphs < 256
         mutable int fast_glyph_count;
     };