Use consistent vertical metrics on all platforms

Qt assumes that ascent+descent is the bounding height of the
text, but for historical reasons, the ascent for some fonts
will not contain the diacritics. On Windows, the preference
is to use data from the OS/2 table which were explicitly
invented to work around this, but on other platforms we
are not respecting this table. This causes a text layout
that looks fine on Windows to have overlapping characters
on e.g. macOS.

To make vertical metrics (ascent, descent, leading) consistent
across all platforms, we don't blindly trust the values we get
from the underlying font system, but apply in the following order:

1. If OS/2 table exists and USE_TYPO_METRICS flag is set, we
use the typo metrics from OS/2 table
2. If OS/2 table exists and USE_TYPO_METRICS flag is not set,
we use winAscent/winDescent from OS/2 and the line gap from
HHEA table.
3. If no OS/2 table exists, we try to get ascent, descent and
line gap from the HHEA table.
4. If the HHEA table does not exist (not an SFNT), we fall back
to the system-provided metrics. (on macOS, we know the
system-provided metrics will match the data in HHEA, so we
skip parsing that table and use the data from CoreText if
there is no OS/2 table).

Task-number: QTBUG-80554
Change-Id: I41e6561a99513698c8e42451b4ec98bd5eb6892f
Reviewed-by: Lars Knoll <lars.knoll@qt.io>
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>

8 files changed, 61 insertions, 115 deletions

diff --git a/src/platformsupport/fontdatabases/freetype/qfontengine_ft.cpp b/src/platformsupport/fontdatabases/freetype/qfontengine_ft.cpp
index d38fa299f9..910a16bf95 100644
--- a/src/platformsupport/fontdatabases/freetype/qfontengine_ft.cpp
+++ b/src/platformsupport/fontdatabases/freetype/qfontengine_ft.cpp
@@ -1241,12 +1241,26 @@ int QFontEngineFT::synthesized() const
     return s;
 }
 
-QFixed QFontEngineFT::ascent() const
+void QFontEngineFT::initializeHeightMetrics() const
 {
-    QFixed v = QFixed::fromFixed(metrics.ascender);
-    if (scalableBitmapScaleFactor != 1)
-        v *= scalableBitmapScaleFactor;
-    return v;
+    QFontEngine::initializeHeightMetrics();
+
+    if (scalableBitmapScaleFactor != 1) {
+        m_ascent *= scalableBitmapScaleFactor;
+        m_descent *= scalableBitmapScaleFactor;
+        m_leading *= scalableBitmapScaleFactor;
+    }
+}
+
+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
@@ -1261,22 +1275,6 @@ QFixed QFontEngineFT::capHeight() const
     return calculatedCapHeight();
 }
 
-QFixed QFontEngineFT::descent() const
-{
-    QFixed v = QFixed::fromFixed(-metrics.descender);
-    if (scalableBitmapScaleFactor != 1)
-        v *= scalableBitmapScaleFactor;
-    return v;
-}
-
-QFixed QFontEngineFT::leading() const
-{
-    QFixed v = QFixed::fromFixed(metrics.height - metrics.ascender + metrics.descender);
-    if (scalableBitmapScaleFactor != 1)
-        v *= scalableBitmapScaleFactor;
-    return v;
-}
-
 QFixed QFontEngineFT::xHeight() 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
index 0e719c7457..f8eca7e267 100644
--- a/src/platformsupport/fontdatabases/freetype/qfontengine_ft_p.h
+++ b/src/platformsupport/fontdatabases/freetype/qfontengine_ft_p.h
@@ -191,10 +191,9 @@ private:
     bool getSfntTableData(uint tag, uchar *buffer, uint *length) const override;
     int synthesized() const override;
 
-    QFixed ascent() const override;
+    void initializeHeightMetrics() const override;
+    bool processHheaTable() const override;
     QFixed capHeight() const override;
-    QFixed descent() const override;
-    QFixed leading() 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
index 2437051274..7a6dfe266f 100644
--- a/src/platformsupport/fontdatabases/mac/qfontengine_coretext.mm
+++ b/src/platformsupport/fontdatabases/mac/qfontengine_coretext.mm
@@ -381,14 +381,15 @@ QT_WARNING_POP
     return ret;
 }
 
-QFixed QCoreTextFontEngine::ascent() const
+bool QCoreTextFontEngine::processHheaTable() const
 {
-QT_WARNING_PUSH
-QT_WARNING_DISABLE_DEPRECATED
-    return (fontDef.styleStrategy & QFont::ForceIntegerMetrics)
-            ? QFixed::fromReal(CTFontGetAscent(ctfont)).round()
-            : QFixed::fromReal(CTFontGetAscent(ctfont));
-QT_WARNING_POP
+    if (!QFontEngine::processHheaTable()) {
+        m_ascent = QFixed::fromReal(CTFontGetAscent(ctfont));
+        m_descent = QFixed::fromReal(CTFontGetDescent(ctfont));
+        m_leading = QFixed::fromReal(CTFontGetLeading(ctfont));
+    }
+
+    return true;
 }
 
 QFixed QCoreTextFontEngine::capHeight() const
@@ -406,26 +407,6 @@ QT_WARNING_POP
     return c;
 }
 
-QFixed QCoreTextFontEngine::descent() const
-{
-    QFixed d = QFixed::fromReal(CTFontGetDescent(ctfont));
-QT_WARNING_PUSH
-QT_WARNING_DISABLE_DEPRECATED
-    if (fontDef.styleStrategy & QFont::ForceIntegerMetrics)
-QT_WARNING_POP
-        d = d.round();
-
-    return d;
-}
-QFixed QCoreTextFontEngine::leading() const
-{
-QT_WARNING_PUSH
-QT_WARNING_DISABLE_DEPRECATED
-    return (fontDef.styleStrategy & QFont::ForceIntegerMetrics)
-            ? QFixed::fromReal(CTFontGetLeading(ctfont)).round()
-            : QFixed::fromReal(CTFontGetLeading(ctfont));
-QT_WARNING_POP
-}
 QFixed QCoreTextFontEngine::xHeight() const
 {
 QT_WARNING_PUSH
diff --git a/src/platformsupport/fontdatabases/mac/qfontengine_coretext_p.h b/src/platformsupport/fontdatabases/mac/qfontengine_coretext_p.h
index fe5be960b3..5111e2ce2e 100644
--- a/src/platformsupport/fontdatabases/mac/qfontengine_coretext_p.h
+++ b/src/platformsupport/fontdatabases/mac/qfontengine_coretext_p.h
@@ -82,10 +82,7 @@ public:
     glyph_metrics_t boundingBox(const QGlyphLayout &glyphs) override;
     glyph_metrics_t boundingBox(glyph_t glyph) override;
 
-    QFixed ascent() const override;
     QFixed capHeight() const override;
-    QFixed descent() const override;
-    QFixed leading() const override;
     QFixed xHeight() const override;
     qreal maxCharWidth() const override;
     QFixed averageCharWidth() const override;
@@ -142,6 +139,7 @@ protected:
     bool hasColorGlyphs() const;
     bool shouldAntialias() const;
     bool shouldSmoothFont() const;
+    bool processHheaTable() const override;
 
     QCFType<CTFontRef> ctfont;
     QCFType<CGFontRef> cgFont;
diff --git a/src/platformsupport/fontdatabases/windows/qwindowsfontengine.cpp b/src/platformsupport/fontdatabases/windows/qwindowsfontengine.cpp
index 8cf224337b..6c1088c019 100644
--- a/src/platformsupport/fontdatabases/windows/qwindowsfontengine.cpp
+++ b/src/platformsupport/fontdatabases/windows/qwindowsfontengine.cpp
@@ -492,21 +492,6 @@ glyph_metrics_t QWindowsFontEngine::boundingBox(glyph_t glyph, const QTransform
     return glyphMetrics;
 }
 
-QFixed QWindowsFontEngine::ascent() const
-{
-    return tm.tmAscent;
-}
-
-QFixed QWindowsFontEngine::descent() const
-{
-    return tm.tmDescent;
-}
-
-QFixed QWindowsFontEngine::leading() const
-{
-    return tm.tmExternalLeading;
-}
-
 namespace {
 #   pragma pack(1)
 
@@ -625,6 +610,17 @@ void QWindowsFontEngine::getGlyphBearings(glyph_t glyph, qreal *leftBearing, qre
 }
 #endif // Q_CC_MINGW
 
+bool QWindowsFontEngine::processHheaTable() const
+{
+    if (!QFontEngine::processHheaTable()) {
+        m_ascent = tm.tmAscent;
+        m_descent = tm.tmDescent;
+        m_leading = tm.tmExternalLeading;
+    }
+
+    return true;
+}
+
 bool QWindowsFontEngine::hasUnreliableGlyphOutline() const
 {
     return hasUnreliableOutline || QFontEngine::hasUnreliableGlyphOutline();
diff --git a/src/platformsupport/fontdatabases/windows/qwindowsfontengine_p.h b/src/platformsupport/fontdatabases/windows/qwindowsfontengine_p.h
index b1b9d828ac..4f5629dd0f 100644
--- a/src/platformsupport/fontdatabases/windows/qwindowsfontengine_p.h
+++ b/src/platformsupport/fontdatabases/windows/qwindowsfontengine_p.h
@@ -98,9 +98,6 @@ public:
     glyph_metrics_t boundingBox(glyph_t g, const QTransform &t) override;
 
 
-    QFixed ascent() const override;
-    QFixed descent() const override;
-    QFixed leading() const override;
     QFixed xHeight() const override;
     QFixed capHeight() const override;
     QFixed averageCharWidth() const override;
@@ -132,6 +129,9 @@ public:
 
     void setUniqueFamilyName(const QString &newName) { uniqueFamilyName = newName; }
 
+protected:
+    bool processHheaTable() const override;
+
 private:
     QWindowsNativeImage *drawGDIGlyph(HFONT font, glyph_t, int margin, const QTransform &xform,
                                       QImage::Format mask_format);
diff --git a/src/platformsupport/fontdatabases/windows/qwindowsfontenginedirectwrite.cpp b/src/platformsupport/fontdatabases/windows/qwindowsfontenginedirectwrite.cpp
index 04b6c6888d..7e5c1a2a9e 100644
--- a/src/platformsupport/fontdatabases/windows/qwindowsfontenginedirectwrite.cpp
+++ b/src/platformsupport/fontdatabases/windows/qwindowsfontenginedirectwrite.cpp
@@ -236,11 +236,8 @@ QWindowsFontEngineDirectWrite::QWindowsFontEngineDirectWrite(IDWriteFontFace *di
     , m_directWriteBitmapRenderTarget(0)
     , m_lineThickness(-1)
     , m_unitsPerEm(-1)
-    , m_ascent(-1)
     , m_capHeight(-1)
-    , m_descent(-1)
     , m_xHeight(-1)
-    , m_lineGap(-1)
 {
     qCDebug(lcQpaFonts) << __FUNCTION__ << pixelSize;
 
@@ -346,6 +343,20 @@ QString QWindowsFontEngineDirectWrite::filenameFromFontFile(IDWriteFontFile *fon
     return ret;
 }
 
+bool QWindowsFontEngineDirectWrite::processHheaTable() const
+{
+    if (!QFontEngine::processHheaTable()) {
+        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);
+    }
+
+    return true;
+}
+
 void QWindowsFontEngineDirectWrite::collectMetrics()
 {
     DWRITE_FONT_METRICS metrics;
@@ -354,11 +365,8 @@ void QWindowsFontEngineDirectWrite::collectMetrics()
     m_unitsPerEm = metrics.designUnitsPerEm;
 
     m_lineThickness = DESIGN_TO_LOGICAL(metrics.underlineThickness);
-    m_ascent = DESIGN_TO_LOGICAL(metrics.ascent);
     m_capHeight = DESIGN_TO_LOGICAL(metrics.capHeight);
-    m_descent = DESIGN_TO_LOGICAL(metrics.descent);
     m_xHeight = DESIGN_TO_LOGICAL(metrics.xHeight);
-    m_lineGap = DESIGN_TO_LOGICAL(metrics.lineGap);
     m_underlinePosition = DESIGN_TO_LOGICAL(metrics.underlinePosition);
 
     IDWriteFontFile *fontFile = nullptr;
@@ -567,7 +575,7 @@ QT_WARNING_POP
 
     }
 
-    return glyph_metrics_t(0, -m_ascent, w - lastRightBearing(glyphs), m_ascent + m_descent, w, 0);
+    return glyph_metrics_t(0, -ascent(), w - lastRightBearing(glyphs), ascent() + descent(), w, 0);
 }
 
 glyph_metrics_t QWindowsFontEngineDirectWrite::boundingBox(glyph_t g)
@@ -608,16 +616,6 @@ QT_WARNING_POP
     return glyph_metrics_t();
 }
 
-QFixed QWindowsFontEngineDirectWrite::ascent() const
-{
-QT_WARNING_PUSH
-QT_WARNING_DISABLE_DEPRECATED
-    return fontDef.styleStrategy & QFont::ForceIntegerMetrics
-            ? m_ascent.round()
-            : m_ascent;
-QT_WARNING_POP
-}
-
 QFixed QWindowsFontEngineDirectWrite::capHeight() const
 {
     if (m_capHeight <= 0)
@@ -631,26 +629,6 @@ QT_WARNING_DISABLE_DEPRECATED
 QT_WARNING_POP
 }
 
-QFixed QWindowsFontEngineDirectWrite::descent() const
-{
-QT_WARNING_PUSH
-QT_WARNING_DISABLE_DEPRECATED
-    return fontDef.styleStrategy & QFont::ForceIntegerMetrics
-           ? m_descent.round()
-           : m_descent;
-QT_WARNING_POP
-}
-
-QFixed QWindowsFontEngineDirectWrite::leading() const
-{
-QT_WARNING_PUSH
-QT_WARNING_DISABLE_DEPRECATED
-    return fontDef.styleStrategy & QFont::ForceIntegerMetrics
-           ? m_lineGap.round()
-           : m_lineGap;
-QT_WARNING_POP
-}
-
 QFixed QWindowsFontEngineDirectWrite::xHeight() const
 {
 QT_WARNING_PUSH
diff --git a/src/platformsupport/fontdatabases/windows/qwindowsfontenginedirectwrite_p.h b/src/platformsupport/fontdatabases/windows/qwindowsfontenginedirectwrite_p.h
index c8c6b83bf9..ccef808486 100644
--- a/src/platformsupport/fontdatabases/windows/qwindowsfontenginedirectwrite_p.h
+++ b/src/platformsupport/fontdatabases/windows/qwindowsfontenginedirectwrite_p.h
@@ -99,10 +99,7 @@ public:
     glyph_metrics_t alphaMapBoundingBox(glyph_t glyph, QFixed,
                                         const QTransform &matrix, GlyphFormat) override;
 
-    QFixed ascent() const override;
     QFixed capHeight() const override;
-    QFixed descent() const override;
-    QFixed leading() const override;
     QFixed xHeight() const override;
     qreal maxCharWidth() const override;
     FaceId faceId() const override;
@@ -125,6 +122,8 @@ public:
 
     void setUniqueFamilyName(const QString &newName) { m_uniqueFamilyName = newName; }
 
+    bool processHheaTable() const override;
+
 private:
     QImage imageForGlyph(glyph_t t, QFixed subPixelPosition, int margin, const QTransform &xform, const QColor &color = QColor());
     void collectMetrics();
@@ -139,11 +138,8 @@ private:
     QFixed m_lineThickness;
     QFixed m_underlinePosition;
     int m_unitsPerEm;
-    QFixed m_ascent;
     QFixed m_capHeight;
-    QFixed m_descent;
     QFixed m_xHeight;
-    QFixed m_lineGap;
     QFixed m_maxAdvanceWidth;
     FaceId m_faceId;
     QString m_uniqueFamilyName;