QQuickText: fix fractional pointSize rounding error bug

Problem: The layout is initialized with a copy of the current font
before the laying out begins.
In cases when setupTextLayout() iterates through the main loop multiple
times, it will update the layout font, if it differs. (See code below)

if (!once) {
    if (pixelSize)
        scaledFont.setPixelSize(scaledFontSize);
    else
        scaledFont.setPointSizeF(scaledFontSize);
    if (layout.font() != scaledFont)
        layout.setFont(scaledFont);
}

The whole reason why we might update the font in the first place, is
because the QQuickText has a fontSizeMode property which can be set to
e.g. HorizontalFit, which will cause the layouting to downscale the font
in order to fit (assuming the text doesn't fit the space available),
instead of eliding.

The problem here is that QFont internally uses a float to store the point
size, and we would convert that value to an int when temporarily storing
it on the stack. This would modify the pointSize value in cases where
the pointSize is fractional, and cause a mismatch between the QQuickText
and QTextLayout.

The lineWidth is set only during the first loop iteration. During
successive iterations of the main loop, the layout's font would be updated
to one that has either a floored or ceiled pointSize.
If the pointSize is a fractional value, and is ceiled when being updated
during the second loop iteration, the layout would use a larger font
value, which would cause the QTextLayout::naturalTextWidth() to return a
larger value than that of the lineWidth, triggering eliding.

Solution: Keep the font precision, by storing the values as floats
instead of ints.

Fixes: QTBUG-92006
Change-Id: Ibf64ac2dfbf262c6aae05b8eb8251d2f5a869b69
Reviewed-by: Jan Arve Sæther <jan-arve.saether@qt.io>
(cherry picked from commit ae62122a8f57b1de654e891125f19d2e18d6f5df)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>

4 files changed, 88 insertions, 26 deletions

diff --git a/src/quick/items/qquicktext.cpp b/src/quick/items/qquicktext.cpp
index f5dca6e0f5..66baa7ed7c 100644
--- a/src/quick/items/qquicktext.cpp
+++ b/src/quick/items/qquicktext.cpp
@@ -772,11 +772,15 @@ QRectF QQuickTextPrivate::setupTextLayout(qreal *const baseline)
     const bool pixelSize = font.pixelSize() != -1;
     QString layoutText = layout.text();
 
-    int largeFont = pixelSize ? font.pixelSize() : font.pointSize();
-    int smallFont = fontSizeMode() != QQuickText::FixedSize
-            ? qMin(pixelSize ? minimumPixelSize() : minimumPointSize(), largeFont)
-            : largeFont;
-    int scaledFontSize = largeFont;
+    const qreal minimumSize = pixelSize
+                            ? static_cast<qreal>(minimumPixelSize())
+                            : minimumPointSize();
+    qreal largeFont = pixelSize ? font.pixelSize() : font.pointSizeF();
+    qreal smallFont = fontSizeMode() != QQuickText::FixedSize
+                    ? qMin<qreal>(minimumSize, largeFont)
+                    : largeFont;
+    qreal scaledFontSize = largeFont;
+    const qreal sizeFittingThreshold(0.01);
 
     bool widthChanged = false;
     widthExceeded = availableWidth() <= 0 && (singlelineElide || canWrap || horizontalFit);
@@ -805,7 +809,7 @@ QRectF QQuickTextPrivate::setupTextLayout(qreal *const baseline)
             if (pixelSize)
                 scaledFont.setPixelSize(scaledFontSize);
             else
-                scaledFont.setPointSize(scaledFontSize);
+                scaledFont.setPointSizeF(scaledFontSize);
             if (layout.font() != scaledFont)
                 layout.setFont(scaledFont);
         }
@@ -1069,40 +1073,45 @@ QRectF QQuickTextPrivate::setupTextLayout(qreal *const baseline)
         if (!horizontalFit && !verticalFit)
             break;
 
+        // Can't find a better fit
+        if (qFuzzyCompare(smallFont, largeFont))
+            break;
+
         // Try and find a font size that better fits the dimensions of the element.
         if (horizontalFit) {
             if (unelidedRect.width() > lineWidth || (!verticalFit && wrapped)) {
                 widthExceeded = true;
-                largeFont = scaledFontSize - 1;
-                if (smallFont > largeFont)
-                    break;
+                largeFont = scaledFontSize;
+
                 scaledFontSize = (smallFont + largeFont) / 2;
-                if (pixelSize)
-                    scaledFont.setPixelSize(scaledFontSize);
-                else
-                    scaledFont.setPointSize(scaledFontSize);
+
                 continue;
             } else if (!verticalFit) {
                 smallFont = scaledFontSize;
-                if (smallFont == largeFont)
+
+                // Check to see if the current scaledFontSize is acceptable
+                if ((largeFont - smallFont) < sizeFittingThreshold)
                     break;
-                scaledFontSize = (smallFont + largeFont + 1) / 2;
+
+                scaledFontSize = (smallFont + largeFont) / 2;
             }
         }
 
         if (verticalFit) {
             if (truncateHeight || unelidedRect.height() > maxHeight) {
                 heightExceeded = true;
-                largeFont = scaledFontSize - 1;
-                if (smallFont > largeFont)
-                    break;
+                largeFont = scaledFontSize;
+
                 scaledFontSize = (smallFont + largeFont) / 2;
 
             } else {
                 smallFont = scaledFontSize;
-                if (smallFont == largeFont)
+
+                // Check to see if the current scaledFontSize is acceptable
+                if ((largeFont - smallFont) < sizeFittingThreshold)
                     break;
-                scaledFontSize = (smallFont + largeFont + 1) / 2;
+
+                scaledFontSize = (smallFont + largeFont) / 2;
             }
         }
     }
diff --git a/tests/auto/quick/qquicktext/tst_qquicktext.cpp b/tests/auto/quick/qquicktext/tst_qquicktext.cpp
index bf90a8efd4..5f3679ca00 100644
--- a/tests/auto/quick/qquicktext/tst_qquicktext.cpp
+++ b/tests/auto/quick/qquicktext/tst_qquicktext.cpp
@@ -4049,7 +4049,11 @@ static qreal expectedBaselineScaled(QQuickText *item)
 {
     QFont font = item->font();
     QTextLayout layout(item->text().replace(QLatin1Char('\n'), QChar::LineSeparator));
-    do {
+
+    qreal low = 0;
+    qreal high = 10000;
+
+    while (low < high) {
         layout.setFont(font);
         qreal width = 0;
         layout.beginLayout();
@@ -4059,12 +4063,23 @@ static qreal expectedBaselineScaled(QQuickText *item)
         }
         layout.endLayout();
 
-        if (width < item->width()) {
-            QFontMetricsF fm(layout.font());
-            return fm.ascent() + item->topPadding();
+        if (width > item->width()) {
+            high = font.pointSizeF();
+            font.setPointSizeF((high + low) / 2);
+        } else {
+            low = font.pointSizeF();
+
+            // When fontSizeMode != FixedSize, the font size will be scaled to a value
+            // The goal is to find a pointSize that uses as much space as possible while
+            // still fitting inside the available space. 0.01 is chosen as the threshold.
+            if ((high - low) < qreal(0.01)) {
+                QFontMetricsF fm(layout.font());
+                return fm.ascent() + item->topPadding();
+            }
+
+            font.setPointSizeF((high + low) / 2);
         }
-        font.setPointSize(font.pointSize() - 1);
-    } while (font.pointSize() > 0);
+    }
     return item->topPadding();
 }
 
diff --git a/tests/auto/quickcontrols2/qquickcontrol/data/fractionalFontSize.qml b/tests/auto/quickcontrols2/qquickcontrol/data/fractionalFontSize.qml
new file mode 100644
index 0000000000..a4574e460e
--- /dev/null
+++ b/tests/auto/quickcontrols2/qquickcontrol/data/fractionalFontSize.qml
@@ -0,0 +1,18 @@
+import QtQuick
+import QtQuick.Controls
+
+ApplicationWindow {
+    width: 400
+    height: 400
+
+    property alias control: ctrl
+
+    Control {
+        id: ctrl
+        contentItem: Text {
+            font.pointSize: 10.5
+            elide: Text.ElideRight
+            text: "This is some sample text"
+        }
+    }
+}
diff --git a/tests/auto/quickcontrols2/qquickcontrol/tst_qquickcontrol.cpp b/tests/auto/quickcontrols2/qquickcontrol/tst_qquickcontrol.cpp
index 94dccf6115..3ee9cc2328 100644
--- a/tests/auto/quickcontrols2/qquickcontrol/tst_qquickcontrol.cpp
+++ b/tests/auto/quickcontrols2/qquickcontrol/tst_qquickcontrol.cpp
@@ -33,6 +33,7 @@
 #include <QtQuickTestUtils/private/visualtestutils_p.h>
 #include <QtQuickTemplates2/private/qquickbutton_p.h>
 #include <QtQuickControlsTestUtils/private/qtest_quickcontrols_p.h>
+#include <QtQuick/private/qquicktext_p_p.h>
 
 using namespace QQuickVisualTestUtils;
 
@@ -46,6 +47,7 @@ public:
 private slots:
     void initTestCase() override;
     void flickable();
+    void fractionalFontSize();
 
 private:
     QScopedPointer<QPointingDevice> touchDevice;
@@ -93,6 +95,24 @@ void tst_QQuickControl::flickable()
     QTRY_COMPARE(buttonClickedSpy.count(), 1);
 }
 
+void tst_QQuickControl::fractionalFontSize()
+{
+    QQuickApplicationHelper helper(this, QStringLiteral("fractionalFontSize.qml"));
+    QQuickWindow *window = helper.window;
+    window->show();
+    QVERIFY(QTest::qWaitForWindowExposed(window));
+    const QQuickControl *control = window->property("control").value<QQuickControl *>();
+    QVERIFY(control);
+    QQuickText *contentItem = qobject_cast<QQuickText *>(control->contentItem());
+    QVERIFY(contentItem);
+
+    QVERIFY(!contentItem->truncated());
+
+    QVERIFY2(qFuzzyCompare(contentItem->contentWidth(),
+            QQuickTextPrivate::get(contentItem)->layout.boundingRect().width()),
+            "The QQuickText::contentWidth() doesn't match the layout's preferred text width");
+}
+
 QTEST_QUICKCONTROLS_MAIN(tst_QQuickControl)
 
 #include "tst_qquickcontrol.moc"