Deal with {und,ov}erflow issues in QLine's length handling

Use std::hypot() instead of sqrt() of a sum of squares.
This ensures length() can't be zero when isNull() is false.
Use length() in QLine::setLength() rather than duplicating that.

Clarify and expand some documentation; isNull() never said what
constituted validity, nor did unitVector() mention that is should not
be used on a line for which isNull() is true. Make clear that lines of
denormal length cannot be rescaled accurately.

Given that we use fuzzy comparison to determine equality of
end-points, isNull() can be false for a line with displacements less
than sqrt(numeric_limits<qreal>::denorm_min()) between the coordinates
of its end-points (as long as these are not much bigger); squaring
these would give zero, hence a zero length, where using hypot() avoids
the underflow and gives a non-zero length. Having a zero length for a
line with isNull() false would lead to problems in setLength(), which
uses an isNull() pre-test, protecting a call to unitVector().

(It was already possible for a null line to have non-zero length; this
now arises in more cases.)

Restored QLine::setLength() to the form it had before a recent change
to avoid division by zero (which resulted from underflow in computing
the length of a non-null line) but allow for the possibility that the
unit vector it computes as transient may not have length exactly one.

Add tests against {ov,und}erflow problems in QLine. Reworked the test
added during the divide-by-zero fix to make it part of the existing
test.

Pick-to: 5.15 5.12
Change-Id: I7b71d66b872ccc08a64e941acd36b45b0ea15fab
Reviewed-by: Sze Howe Koh <szehowe.koh@gmail.com>

2 files changed, 29 insertions, 23 deletions

diff --git a/src/corelib/tools/qline.cpp b/src/corelib/tools/qline.cpp
index d284883dcc..ebcbba0195 100644
--- a/src/corelib/tools/qline.cpp
+++ b/src/corelib/tools/qline.cpp
@@ -38,8 +38,10 @@
 ****************************************************************************/
 
 #include "qline.h"
+
 #include "qdebug.h"
 #include "qdatastream.h"
+#include "qmath.h"
 #include <private/qnumeric_p.h>
 
 QT_BEGIN_NAMESPACE
@@ -98,7 +100,7 @@ QT_BEGIN_NAMESPACE
 /*!
     \fn bool QLine::isNull() const
 
-    Returns \c true if the line is not set up with valid start and end point;
+    Returns \c true if the line does not have distinct start and end points;
     otherwise returns \c false.
 */
 
@@ -427,8 +429,14 @@ QDataStream &operator>>(QDataStream &stream, QLine &line)
 /*!
     \fn bool QLineF::isNull() const
 
-    Returns \c true if the line is not set up with valid start and end point;
-    otherwise returns \c false.
+    Returns \c true if the line does not have distinct start and end points;
+    otherwise returns \c false. The start and end points are considered distinct
+    if qFuzzyCompare() can distinguish them in at least one coordinate.
+
+    \note Due to the use of fuzzy comparison, isNull() may return \c true for
+    lines whose length() is not zero.
+
+    \sa qFuzzyCompare(), length()
 */
 
 /*!
@@ -511,10 +519,10 @@ QDataStream &operator>>(QDataStream &stream, QLine &line)
     Sets the length of the line to the given \a length. QLineF will
     move the end point - p2() - of the line to give the line its new length.
 
-    If the line is a null line, the length will remain zero regardless
-    of the length specified.
+    A null line will not be rescaled. For non-null lines with very short lengths
+    (represented by denormal floating-point values), results may be imprecise.
 
-    \sa length(), isNull()
+    \sa length(), isNull(), unitVector()
 */
 
 /*!
@@ -559,13 +567,12 @@ QDataStream &operator>>(QDataStream &stream, QLine &line)
 /*!
     Returns the length of the line.
 
-    \sa setLength()
+    \sa setLength(), isNull()
 */
 qreal QLineF::length() const
 {
-    qreal x = pt2.x() - pt1.x();
-    qreal y = pt2.y() - pt1.y();
-    return qSqrt(x*x + y*y);
+    using std::hypot;
+    return hypot(dx(), dy());
 }
 
 /*!
@@ -636,16 +643,18 @@ QLineF QLineF::fromPolar(qreal length, qreal angle)
 
 /*!
     Returns the unit vector for this line, i.e a line starting at the
-    same point as \e this line with a length of 1.0.
+    same point as \e this line with a length of 1.0, provided the line
+    is non-null.
 
-    \sa normalVector()
+    \sa normalVector(), setLength()
 */
 QLineF QLineF::unitVector() const
 {
-    qreal x = pt2.x() - pt1.x();
-    qreal y = pt2.y() - pt1.y();
+    qreal x = dx();
+    qreal y = dy();
+    using std::hypot;
+    qreal len = hypot(x, y);
 
-    qreal len = qSqrt(x*x + y*y);
     QLineF f(p1(), QPointF(pt1.x() + x/len, pt1.y() + y/len));
 
 #ifndef QT_NO_DEBUG
diff --git a/src/corelib/tools/qline.h b/src/corelib/tools/qline.h
index adef545d86..8d57e09c4a 100644
--- a/src/corelib/tools/qline.h
+++ b/src/corelib/tools/qline.h
@@ -40,8 +40,6 @@
 #ifndef QLINE_H
 #define QLINE_H
 
-#include <QtCore/qmath.h>
-
 #include <QtCore/qpoint.h>
 
 QT_BEGIN_NAMESPACE
@@ -372,15 +370,14 @@ constexpr inline QPointF QLineF::center() const
     return QPointF(0.5 * pt1.x() + 0.5 * pt2.x(), 0.5 * pt1.y() + 0.5 * pt2.y());
 }
 
-QT_WARNING_DISABLE_FLOAT_COMPARE
-
 inline void QLineF::setLength(qreal len)
 {
-    const qreal oldlength = qSqrt(dx() * dx() + dy() * dy());
-    if (!oldlength)
+    if (isNull())
         return;
-    const qreal factor = len / oldlength;
-    pt2 = QPointF(pt1.x() + dx() * factor, pt1.y() + dy() * factor);
+    Q_ASSERT(length() > 0);
+    const QLineF v = unitVector();
+    len /= v.length(); // In case it's not quite exactly 1.
+    pt2 = QPointF(pt1.x() + len * v.dx(), pt1.y() + len * v.dy());
 }
 
 constexpr inline QPointF QLineF::pointAt(qreal t) const