Remove historical +1 from font height calculation

Historically, we've calculated font height as ascent+descent+1.
In Qt 4, a patch was added to work around this by subtracting
1 from the descent of the font engines. We now remove the +1 and
the work arounds.

Change-Id: I7e25d49b97ac892015d3328f32d70eb9a7c2d88f
Reviewed-by: Friedemann Kleint <Friedemann.Kleint@nokia.com>
Reviewed-by: Lars Knoll <lars.knoll@nokia.com>

diff --git a/src/gui/text/qabstracttextdocumentlayout.cpp b/src/gui/text/qabstracttextdocumentlayout.cpp
index 2c22b72..7bf2a63 100644 (file)
--- a/src/gui/text/qabstracttextdocumentlayout.cpp
+++ b/src/gui/text/qabstracttextdocumentlayout.cpp
@@ -469,7 +469,7 @@ void QAbstractTextDocumentLayout::resizeInlineObject(QTextInlineObject item, int
 
     QSizeF s = handler.iface->intrinsicSize(document(), posInDocument, format);
     item.setWidth(s.width());
-    item.setAscent(s.height() - 1);
+    item.setAscent(s.height());
     item.setDescent(0);
 }
 

diff --git a/src/gui/text/qfontengine_ft.cpp b/src/gui/text/qfontengine_ft.cpp
index c9eadd3..8880eb7 100644 (file)
--- a/src/gui/text/qfontengine_ft.cpp
+++ b/src/gui/text/qfontengine_ft.cpp
@@ -1173,8 +1173,7 @@ QFixed QFontEngineFT::ascent() const
 
 QFixed QFontEngineFT::descent() const
 {
-    // subtract a pixel to work around QFontMetrics's built-in + 1
-    return QFixed::fromFixed(-metrics.descender - 64);
+    return QFixed::fromFixed(-metrics.descender);
 }
 
 QFixed QFontEngineFT::leading() const
@@ -1589,7 +1588,7 @@ glyph_metrics_t QFontEngineFT::boundingBox(const QGlyphLayout &glyphs)
     glyph_metrics_t overall;
     // initialize with line height, we get the same behaviour on all platforms
     overall.y = -ascent();
-    overall.height = ascent() + descent() + 1;
+    overall.height = ascent() + descent();
 
     QFixed ymax = 0;
     QFixed xmax = 0;

diff --git a/src/gui/text/qfontenginedirectwrite.cpp b/src/gui/text/qfontenginedirectwrite.cpp
index afbc41d..0f21ae8 100644 (file)
--- a/src/gui/text/qfontenginedirectwrite.cpp
+++ b/src/gui/text/qfontenginedirectwrite.cpp
@@ -484,8 +484,8 @@ QFixed QFontEngineDirectWrite::ascent() const
 QFixed QFontEngineDirectWrite::descent() const
 {
     return fontDef.styleStrategy & QFont::ForceIntegerMetrics
-           ? (m_descent - 1).round()
-           : (m_descent - 1);
+           ? (m_descent).round()
+           : (m_descent);
 }
 
 QFixed QFontEngineDirectWrite::leading() const

diff --git a/src/gui/text/qfontmetrics.cpp b/src/gui/text/qfontmetrics.cpp
index a2f0dd7..283494e 100644 (file)
--- a/src/gui/text/qfontmetrics.cpp
+++ b/src/gui/text/qfontmetrics.cpp
@@ -288,7 +288,7 @@ int QFontMetrics::height() const
 {
     QFontEngine *engine = d->engineForScript(QUnicodeTables::Common);
     Q_ASSERT(engine != 0);
-    return qRound(engine->ascent()) + qRound(engine->descent()) + 1;
+    return qRound(engine->ascent()) + qRound(engine->descent());
 }
 
 /*!
@@ -316,7 +316,7 @@ int QFontMetrics::lineSpacing() const
 {
     QFontEngine *engine = d->engineForScript(QUnicodeTables::Common);
     Q_ASSERT(engine != 0);
-    return qRound(engine->leading()) + qRound(engine->ascent()) + qRound(engine->descent()) + 1;
+    return qRound(engine->leading()) + qRound(engine->ascent()) + qRound(engine->descent());
 }
 
 /*!
@@ -1147,7 +1147,7 @@ qreal QFontMetricsF::height() const
     QFontEngine *engine = d->engineForScript(QUnicodeTables::Common);
     Q_ASSERT(engine != 0);
 
-    return (engine->ascent() + engine->descent() + 1).toReal();
+    return (engine->ascent() + engine->descent()).toReal();
 }
 
 /*!
@@ -1175,7 +1175,7 @@ qreal QFontMetricsF::lineSpacing() const
 {
     QFontEngine *engine = d->engineForScript(QUnicodeTables::Common);
     Q_ASSERT(engine != 0);
-    return (engine->leading() + engine->ascent() + engine->descent() + 1).toReal();
+    return (engine->leading() + engine->ascent() + engine->descent()).toReal();
 }
 
 /*!

diff --git a/src/gui/text/qtextengine_p.h b/src/gui/text/qtextengine_p.h
index 53031cf..b29f626 100644 (file)
--- a/src/gui/text/qtextengine_p.h
+++ b/src/gui/text/qtextengine_p.h
@@ -366,7 +366,7 @@ struct Q_AUTOTEST_EXPORT QScriptItem
     QFixed leading;
     QFixed width;
     int glyph_data_offset;
-    QFixed height() const { return ascent + descent + 1; }
+    QFixed height() const { return ascent + descent; }
 };
 
 
@@ -396,7 +396,7 @@ struct Q_AUTOTEST_EXPORT QScriptLine
     mutable uint gridfitted : 1;
     uint hasTrailingSpaces : 1;
     uint leadingIncluded : 1;
-    QFixed height() const { return (ascent + descent).ceil() + 1
+    QFixed height() const { return (ascent + descent).ceil()
                             + (leadingIncluded?  qMax(QFixed(),leading) : QFixed()); }
     QFixed base() const { return ascent
                           + (leadingIncluded ? qMax(QFixed(),leading) : QFixed()); }

diff --git a/src/gui/text/qtextlayout.cpp b/src/gui/text/qtextlayout.cpp
index 84d3fce..943caea 100644 (file)
--- a/src/gui/text/qtextlayout.cpp
+++ b/src/gui/text/qtextlayout.cpp
@@ -1429,9 +1429,9 @@ qreal QTextLine::descent() const
 }
 
 /*!
-    Returns the line's height. This is equal to ascent() + descent() + 1
+    Returns the line's height. This is equal to ascent() + descent()
     if leading is not included. If leading is included, this equals to
-    ascent() + descent() + leading() + 1.
+    ascent() + descent() + leading().
 
     \sa ascent(), descent(), leading(), setLeadingIncluded()
 */

diff --git a/src/platformsupport/fontdatabases/mac/qfontengine_coretext.mm b/src/platformsupport/fontdatabases/mac/qfontengine_coretext.mm
index 489138a..a51ffb7 100644 (file)
--- a/src/platformsupport/fontdatabases/mac/qfontengine_coretext.mm
+++ b/src/platformsupport/fontdatabases/mac/qfontengine_coretext.mm
@@ -255,9 +255,7 @@ QFixed QCoreTextFontEngine::descent() const
     if (fontDef.styleStrategy & QFont::ForceIntegerMetrics)
         d = d.round();
 
-    // subtract a pixel to even out the historical +1 in QFontMetrics::height().
-    // Fix in Qt 5.
-    return d - 1;
+    return d;
 }
 QFixed QCoreTextFontEngine::leading() const
 {

diff --git a/src/plugins/platforms/windows/qwindowsfontengine.cpp b/src/plugins/platforms/windows/qwindowsfontengine.cpp
index ffa57ad..007f6d5 100644 (file)
--- a/src/plugins/platforms/windows/qwindowsfontengine.cpp
+++ b/src/plugins/platforms/windows/qwindowsfontengine.cpp
@@ -517,9 +517,7 @@ QFixed QWindowsFontEngine::ascent() const
 
 QFixed QWindowsFontEngine::descent() const
 {
-    // ### we subtract 1 to even out the historical +1 in QFontMetrics'
-    // ### height=asc+desc+1 equation. Fix in Qt5.
-    return tm.tmDescent - 1;
+    return tm.tmDescent;
 }
 
 QFixed QWindowsFontEngine::leading() const

diff --git a/tests/auto/gui/text/qfontmetrics/tst_qfontmetrics.cpp b/tests/auto/gui/text/qfontmetrics/tst_qfontmetrics.cpp
index 0b5486d..4dbdf9a 100644 (file)
--- a/tests/auto/gui/text/qfontmetrics/tst_qfontmetrics.cpp
+++ b/tests/auto/gui/text/qfontmetrics/tst_qfontmetrics.cpp
@@ -140,7 +140,7 @@ void tst_QFontMetrics::metrics()
                 font = fdb.font(family, style, 12);
 
                 QFontMetrics fontmetrics(font);
-                QCOMPARE(fontmetrics.ascent() + fontmetrics.descent() + 1,
+                QCOMPARE(fontmetrics.ascent() + fontmetrics.descent(),
                         fontmetrics.height());
 
                 QCOMPARE(fontmetrics.height() + fontmetrics.leading(),
@@ -156,7 +156,7 @@ void tst_QFontMetrics::metrics()
                     font = fdb.font(family, style, size);
 
                     QFontMetrics fontmetrics(font);
-                    QCOMPARE(fontmetrics.ascent() + fontmetrics.descent() + 1,
+                    QCOMPARE(fontmetrics.ascent() + fontmetrics.descent(),
                             fontmetrics.height());
                     QCOMPARE(fontmetrics.height() + fontmetrics.leading(),
                             fontmetrics.lineSpacing());

diff --git a/tests/auto/gui/text/qtextlayout/tst_qtextlayout.cpp b/tests/auto/gui/text/qtextlayout/tst_qtextlayout.cpp
index e743574..8920e63 100644 (file)
--- a/tests/auto/gui/text/qtextlayout/tst_qtextlayout.cpp
+++ b/tests/auto/gui/text/qtextlayout/tst_qtextlayout.cpp
@@ -352,7 +352,7 @@ void tst_QTextLayout::threeLineBoundingRect()
     QCOMPARE(qRound(line.naturalTextWidth()), thirdLineWidth);
     y += qRound(line.ascent() + line.descent());
 
-    QCOMPARE(layout.boundingRect(), QRectF(0, 0, longestLine, y + 1));
+    QCOMPARE(layout.boundingRect(), QRectF(0, 0, longestLine, y));
 }
 
 void tst_QTextLayout::boundingRectWithLongLineAndNoWrap()
@@ -386,7 +386,7 @@ void tst_QTextLayout::forcedBreaks()
     QCOMPARE(line.textStart(), pos);
     QCOMPARE(line.textLength(),2);
     QCOMPARE(qRound(line.naturalTextWidth()),testFont.pixelSize());
-    QCOMPARE((int) line.height(), testFont.pixelSize() + 1); // + 1 baseline
+    QCOMPARE((int) line.height(), testFont.pixelSize());
     QCOMPARE(line.xToCursor(0), line.textStart());
     pos += line.textLength();
 
@@ -395,7 +395,7 @@ void tst_QTextLayout::forcedBreaks()
     QCOMPARE(line.textStart(),pos);
     QCOMPARE(line.textLength(),1);
     QCOMPARE(qRound(line.naturalTextWidth()), 0);
-    QCOMPARE((int) line.height(), testFont.pixelSize() + 1); // + 1 baseline
+    QCOMPARE((int) line.height(), testFont.pixelSize());
     QCOMPARE(line.xToCursor(0), line.textStart());
     pos += line.textLength();
 
@@ -404,7 +404,7 @@ void tst_QTextLayout::forcedBreaks()
     QCOMPARE(line.textStart(),pos);
     QCOMPARE(line.textLength(),2);
     QCOMPARE(qRound(line.naturalTextWidth()),testFont.pixelSize());
-    QCOMPARE(qRound(line.height()), testFont.pixelSize() + 1); // + 1 baseline
+    QCOMPARE(qRound(line.height()), testFont.pixelSize());
     QCOMPARE(line.xToCursor(0), line.textStart());
     pos += line.textLength();
 
@@ -413,7 +413,7 @@ void tst_QTextLayout::forcedBreaks()
     QCOMPARE(line.textStart(),pos);
     QCOMPARE(line.textLength(),1);
     QCOMPARE(qRound(line.naturalTextWidth()), testFont.pixelSize());
-    QCOMPARE((int) line.height(), testFont.pixelSize() + 1); // + 1 baseline
+    QCOMPARE((int) line.height(), testFont.pixelSize());
     QCOMPARE(line.xToCursor(0), line.textStart());
 }