Fix font height metrics with embedded bitmaps

For fonts with embedded bitmaps, we cannot trust the HHEA and OS/2 tables, since there are a different set of font metrics in the EBLC/CBLC tables for each of the predefined bitmap sizes. In this case, we can safely fall back to the metrics returned by the system, as the inconsistency we were originally fixing was only between OS/2 and HHEA and will not matter for the bitmap fonts. This patch also simplifies the code path through the font engines a bit. Instead of setting the system metrics in the processHheaTable() function when the table cannot be found, we instead always fetch the system metrics at the very start of initializeHeightMetrics() and then override if there are no embedded bitmaps, and if the HHEA and OS/2 tables are available. This also reduces the number of virtual functions needed to sort out the height metrics. Fixes: QTBUG-83754 Change-Id: Ib9dc6fc6cf972e48209a4a272469d2b4bd1ebffe Reviewed-by: Lars Knoll <[email protected]> Reviewed-by: Konstantin Ritt <[email protected]>
goompean · May 4, 2020 · 7a18b7e · 7a18b7e
1 parent b7e3a9e
commit 7a18b7e
Show file tree

Hide file tree

Showing 10 changed files with 33 additions and 46 deletions.
diff --git a/src/gui/text/qfontengine.cpp b/src/gui/text/qfontengine.cpp
@@ -450,13 +450,14 @@ bool QFontEngine::processHheaTable() const
 
 void QFontEngine::initializeHeightMetrics() const
 {
-    if (!processHheaTable()) {
-        qWarning() << "Cannot determine metrics for font" << fontDef.family;
-        m_ascent = m_descent = m_leading = 1;
-    }
+    bool hasEmbeddedBitmaps = !getSfntTable(MAKE_TAG('E', 'B', 'L', 'C')).isEmpty() || !getSfntTable(MAKE_TAG('C', 'B', 'L', 'C')).isEmpty();
+    if (!hasEmbeddedBitmaps) {
+        // Get HHEA table values if available
+        processHheaTable();
 
-    // Allow OS/2 metrics to override if present
-    processOS2Table();
+        // Allow OS/2 metrics to override if present
+        processOS2Table();
+    }
 
     m_heightMetricsQueried = true;
 }

diff --git a/src/gui/text/qfontengine_p.h b/src/gui/text/qfontengine_p.h
@@ -375,8 +375,8 @@ class Q_GUI_EXPORT QFontEngine
     mutable bool m_heightMetricsQueried;
 
     virtual void initializeHeightMetrics() const;
-    virtual bool processHheaTable() const;
-    virtual bool processOS2Table() const;
+    bool processHheaTable() const;
+    bool processOS2Table() const;
 
 private:
     struct GlyphCacheEntry {

diff --git a/src/platformsupport/fontdatabases/freetype/qfontengine_ft.cpp b/src/platformsupport/fontdatabases/freetype/qfontengine_ft.cpp
@@ -1243,6 +1243,10 @@ int QFontEngineFT::synthesized() const
 
 void QFontEngineFT::initializeHeightMetrics() const
 {
+    m_ascent = QFixed::fromFixed(metrics.ascender);
+    m_descent = QFixed::fromFixed(-metrics.descender);
+    m_leading = QFixed::fromFixed(metrics.height - metrics.ascender + metrics.descender);
+
     QFontEngine::initializeHeightMetrics();
 
     if (scalableBitmapScaleFactor != 1) {
@@ -1252,17 +1256,6 @@ void QFontEngineFT::initializeHeightMetrics() const
     }
 }
 
-bool QFontEngineFT::processHheaTable() const
-{
-    if (!QFontEngine::processHheaTable()) {
-        m_ascent = QFixed::fromFixed(metrics.ascender);
-        m_descent = QFixed::fromFixed(-metrics.descender);
-        m_leading = QFixed::fromFixed(metrics.height - metrics.ascender + metrics.descender);
-    }
-
-    return true;
-}
-
 QFixed QFontEngineFT::capHeight() const
 {
     TT_OS2 *os2 = (TT_OS2 *)FT_Get_Sfnt_Table(freetype->face, ft_sfnt_os2);

diff --git a/src/platformsupport/fontdatabases/freetype/qfontengine_ft_p.h b/src/platformsupport/fontdatabases/freetype/qfontengine_ft_p.h
@@ -192,7 +192,6 @@ class QFontEngineFT : public QFontEngine
     int synthesized() const override;
 
     void initializeHeightMetrics() const override;
-    bool processHheaTable() const override;
     QFixed capHeight() const override;
     QFixed xHeight() const override;
     QFixed averageCharWidth() const override;

diff --git a/src/platformsupport/fontdatabases/mac/qfontengine_coretext.mm b/src/platformsupport/fontdatabases/mac/qfontengine_coretext.mm
@@ -381,15 +381,13 @@ CGAffineTransform qt_transform_from_fontdef(const QFontDef &fontDef)
     return ret;
 }
 
-bool QCoreTextFontEngine::processHheaTable() const
+void QCoreTextFontEngine::initializeHeightMetrics() const
 {
-    if (!QFontEngine::processHheaTable()) {
-        m_ascent = QFixed::fromReal(CTFontGetAscent(ctfont));
-        m_descent = QFixed::fromReal(CTFontGetDescent(ctfont));
-        m_leading = QFixed::fromReal(CTFontGetLeading(ctfont));
-    }
+    m_ascent = QFixed::fromReal(CTFontGetAscent(ctfont));
+    m_descent = QFixed::fromReal(CTFontGetDescent(ctfont));
+    m_leading = QFixed::fromReal(CTFontGetLeading(ctfont));
 
-    return true;
+    QFontEngine::initializeHeightMetrics();
 }
 
 QFixed QCoreTextFontEngine::capHeight() const

diff --git a/src/platformsupport/fontdatabases/mac/qfontengine_coretext_p.h b/src/platformsupport/fontdatabases/mac/qfontengine_coretext_p.h
@@ -139,7 +139,7 @@ class QCoreTextFontEngine : public QFontEngine
     bool hasColorGlyphs() const;
     bool shouldAntialias() const;
     bool shouldSmoothFont() const;
-    bool processHheaTable() const override;
+    void initializeHeightMetrics() const override;
 
     QCFType<CTFontRef> ctfont;
     QCFType<CGFontRef> cgFont;

diff --git a/src/platformsupport/fontdatabases/windows/qwindowsfontengine.cpp b/src/platformsupport/fontdatabases/windows/qwindowsfontengine.cpp
@@ -610,15 +610,13 @@ void QWindowsFontEngine::getGlyphBearings(glyph_t glyph, qreal *leftBearing, qre
 }
 #endif // Q_CC_MINGW
 
-bool QWindowsFontEngine::processHheaTable() const
+void QWindowsFontEngine::initializeHeightMetrics() const
 {
-    if (!QFontEngine::processHheaTable()) {
-        m_ascent = tm.tmAscent;
-        m_descent = tm.tmDescent;
-        m_leading = tm.tmExternalLeading;
-    }
+    m_ascent = tm.tmAscent;
+    m_descent = tm.tmDescent;
+    m_leading = tm.tmExternalLeading;
 
-    return true;
+    QFontEngine::initializeHeightMetrics();
 }
 
 bool QWindowsFontEngine::hasUnreliableGlyphOutline() const

diff --git a/src/platformsupport/fontdatabases/windows/qwindowsfontengine_p.h b/src/platformsupport/fontdatabases/windows/qwindowsfontengine_p.h
@@ -130,7 +130,7 @@ class QWindowsFontEngine : public QFontEngine
     void setUniqueFamilyName(const QString &newName) { uniqueFamilyName = newName; }
 
 protected:
-    bool processHheaTable() const override;
+    void initializeHeightMetrics() const override;
 
 private:
     QWindowsNativeImage *drawGDIGlyph(HFONT font, glyph_t, int margin, const QTransform &xform,

diff --git a/src/platformsupport/fontdatabases/windows/qwindowsfontenginedirectwrite.cpp b/src/platformsupport/fontdatabases/windows/qwindowsfontenginedirectwrite.cpp
@@ -343,18 +343,16 @@ QString QWindowsFontEngineDirectWrite::filenameFromFontFile(IDWriteFontFile *fon
     return ret;
 }
 
-bool QWindowsFontEngineDirectWrite::processHheaTable() const
+void QWindowsFontEngineDirectWrite::initializeHeightMetrics() const
 {
-    if (!QFontEngine::processHheaTable()) {
-        DWRITE_FONT_METRICS metrics;
-        m_directWriteFontFace->GetMetrics(&metrics);
+    DWRITE_FONT_METRICS metrics;
+    m_directWriteFontFace->GetMetrics(&metrics);
 
-        m_ascent = DESIGN_TO_LOGICAL(metrics.ascent);
-        m_descent = DESIGN_TO_LOGICAL(metrics.descent);
-        m_leading = DESIGN_TO_LOGICAL(metrics.lineGap);
-    }
+    m_ascent = DESIGN_TO_LOGICAL(metrics.ascent);
+    m_descent = DESIGN_TO_LOGICAL(metrics.descent);
+    m_leading = DESIGN_TO_LOGICAL(metrics.lineGap);
 
-    return true;
+    QFontEngine::initializeHeightMetrics();
 }
 
 void QWindowsFontEngineDirectWrite::collectMetrics()

diff --git a/src/platformsupport/fontdatabases/windows/qwindowsfontenginedirectwrite_p.h b/src/platformsupport/fontdatabases/windows/qwindowsfontenginedirectwrite_p.h
@@ -122,7 +122,7 @@ class QWindowsFontEngineDirectWrite : public QFontEngine
 
     void setUniqueFamilyName(const QString &newName) { m_uniqueFamilyName = newName; }
 
-    bool processHheaTable() const override;
+    void initializeHeightMetrics() const override;
 
 private:
     QImage imageForGlyph(glyph_t t, QFixed subPixelPosition, int margin, const QTransform &xform, const QColor &color = QColor());