From 180f78d12680f1954f9ea3f4b00200ecfa1a853e Mon Sep 17 00:00:00 2001
From: Volker Hilsheimer <volker.hilsheimer@qt.io>
Date: Wed, 17 Mar 2021 15:45:38 +0100
Subject: [PATCH] QDial: use qRound to round

The calculation rounds early and often, which is intentional. Add
unit test to make sure we don't regress.

Fixes static analzyer report about incorrect rounding in
c903a34347776fe3b89785faa35c446d.

Address some outdated comments and documentation. The property
is read only and calculated, so don't imply that it can be changed
from its default value.

Change-Id: If2dbd9890e533dfccda3eae4cbc96db4f1246f4d
Reviewed-by: Eirik Aavitsland <eirik.aavitsland@qt.io>
---
 src/widgets/widgets/qdial.cpp                 | 22 +++++--------
 .../auto/widgets/widgets/qdial/tst_qdial.cpp  | 31 +++++++++++++++++++
 2 files changed, 39 insertions(+), 14 deletions(-)

diff --git a/src/widgets/widgets/qdial.cpp b/src/widgets/widgets/qdial.cpp
index 934be4fb6e..594fe0a779 100644
--- a/src/widgets/widgets/qdial.cpp
+++ b/src/widgets/widgets/qdial.cpp
@@ -370,12 +370,10 @@ bool QDial::wrapping() const
     \property QDial::notchSize
     \brief the current notch size
 
-    The notch size is in range control units, not pixels, and if
-    possible it is a multiple of singleStep() that results in an
+    The notch size is in range control units, not pixels, and is
+    calculated to be a multiple of singleStep() that results in an
     on-screen notch size near notchTarget().
 
-    By default, this property has a value of 1.
-
     \sa notchTarget(), singleStep()
 */
 
@@ -383,21 +381,17 @@ int QDial::notchSize() const
 {
     Q_D(const QDial);
     // radius of the arc
-    int r = qMin(width(), height())/2;
+    qreal r = qMin(width(), height())/2.0;
     // length of the whole arc
-    int l = (int)(r * (d->wrapping ? 6 : 5) * Q_PI / 6);
+    int l = qRound(r * (d->wrapping ? 6.0 : 5.0) * Q_PI / 6.0);
     // length of the arc from minValue() to minValue()+pageStep()
     if (d->maximum > d->minimum + d->pageStep)
-        l = (int)(0.5 + l * d->pageStep / (d->maximum - d->minimum));
+        l = qRound(l * d->pageStep / double(d->maximum - d->minimum));
     // length of a singleStep arc
-    l = l * d->singleStep / (d->pageStep ? d->pageStep : 1);
-    if (l < 1)
-        l = 1;
+    l = qMax(l * d->singleStep / (d->pageStep ? d->pageStep : 1), 1);
     // how many times singleStep can be draw in d->target pixels
-    l = (int)(0.5 + d->target / l);
-    // we want notchSize() to be a non-zero multiple of lineStep()
-    if (!l)
-        l = 1;
+    l = qMax(qRound(d->target / l), 1);
+    // we want notchSize() to be a non-zero multiple of singleStep()
     return d->singleStep * l;
 }
 
diff --git a/tests/auto/widgets/widgets/qdial/tst_qdial.cpp b/tests/auto/widgets/widgets/qdial/tst_qdial.cpp
index 29aee1c397..86928414c8 100644
--- a/tests/auto/widgets/widgets/qdial/tst_qdial.cpp
+++ b/tests/auto/widgets/widgets/qdial/tst_qdial.cpp
@@ -42,6 +42,9 @@ private slots:
     void valueChanged();
     void sliderMoved();
     void wrappingCheck();
+
+    void notchSize_data();
+    void notchSize();
 };
 
 // Testing get/set functions
@@ -194,5 +197,33 @@ void tst_QDial::wrappingCheck()
     }
 }
 
+/*
+    Verify that the notchSizes calculated don't change compared
+    to Qt 5.15 results for dial sizes at the edge values of the
+    algorithm.
+*/
+void tst_QDial::notchSize_data()
+{
+    QTest::addColumn<int>("diameter");
+    QTest::addColumn<int>("notchSize");
+
+    QTest::newRow("data0") << 50 << 4;
+    QTest::newRow("data1") << 80 << 4;
+    QTest::newRow("data2") << 95 << 4;
+    QTest::newRow("data3") << 110 << 4;
+    QTest::newRow("data4") << 152 << 2;
+    QTest::newRow("data5") << 210 << 2;
+    QTest::newRow("data6") << 228 << 1;
+}
+
+void tst_QDial::notchSize()
+{
+    QFETCH(int, diameter);
+    QFETCH(int, notchSize);
+    QDial dial;
+    dial.setFixedSize(QSize(diameter, diameter));
+    QCOMPARE(dial.notchSize(), notchSize);
+}
+
 QTEST_MAIN(tst_QDial)
 #include "tst_qdial.moc"