diff options
author | Edward Welbourne <edward.welbourne@qt.io> | 2020-09-09 14:59:32 +0200 |
---|---|---|
committer | Edward Welbourne <edward.welbourne@qt.io> | 2020-10-27 16:21:37 +0200 |
commit | 36cc28470f6573998dccd8d3d873a4a97fd94323 (patch) | |
tree | fe2807c3a2e3273965d09a2fba1de024d4654e0d /src | |
parent | a6699dd0df0be39fc98a54642ac320e1d7a611d2 (diff) |
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.)
Reworked QLine::setLength() to allow for the possibility that the unit
vector it computes as transient may not have length exactly one.
Add tests against {ov,und}erflow and divide-by-zero problems in QLine.
Picked version for 5.12 needed to adapt the test to simulate 5.15 and
later's QCOMPARE(act, exp) effectively testing qFuzzyIsNull(exp) ?
qFuzzyIsNull(act) : qFuzzyCompare(act, exp) where 5.12 just uses plain
qFuzzyCompare().
Change-Id: I7b71d66b872ccc08a64e941acd36b45b0ea15fab
Reviewed-by: Sze Howe Koh <szehowe.koh@gmail.com>
(cherry picked from commit 1c591fd9246ca776304a3c370dd2578bd886feac)
Reviewed-by: Paul Wicking <paul.wicking@qt.io>
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
Diffstat (limited to 'src')
-rw-r--r-- | src/corelib/tools/qline.cpp | 40 | ||||
-rw-r--r-- | src/corelib/tools/qline.h | 6 |
2 files changed, 28 insertions, 18 deletions
diff --git a/src/corelib/tools/qline.cpp b/src/corelib/tools/qline.cpp index 949f63ea15..24801a7404 100644 --- a/src/corelib/tools/qline.cpp +++ b/src/corelib/tools/qline.cpp @@ -1,6 +1,6 @@ /**************************************************************************** ** -** Copyright (C) 2016 The Qt Company Ltd. +** Copyright (C) 2020 The Qt Company Ltd. ** Contact: https://www.qt.io/licensing/ ** ** This file is part of the QtCore module of the Qt Toolkit. @@ -38,6 +38,7 @@ ****************************************************************************/ #include "qline.h" + #include "qdebug.h" #include "qdatastream.h" #include "qmath.h" @@ -99,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. */ @@ -428,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() */ /*! @@ -512,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() */ /*! @@ -560,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()); } /*! @@ -637,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 6361c1af9f..f15ee40856 100644 --- a/src/corelib/tools/qline.h +++ b/src/corelib/tools/qline.h @@ -1,6 +1,6 @@ /**************************************************************************** ** -** Copyright (C) 2016 The Qt Company Ltd. +** Copyright (C) 2020 The Qt Company Ltd. ** Contact: https://www.qt.io/licensing/ ** ** This file is part of the QtCore module of the Qt Toolkit. @@ -375,7 +375,9 @@ inline void QLineF::setLength(qreal len) { if (isNull()) return; - QLineF v = unitVector(); + Q_ASSERT(length() > 0); + const QLineF v = unitVector(); + len /= v.length(); // In case it's not quite exactly 1. pt2 = QPointF(pt1.x() + v.dx() * len, pt1.y() + v.dy() * len); } |