whytofly
/
qt6-bb10
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

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>
bb10
Edward Welbourne 2020-09-09 14:59:32 +02:00
parent c25687fa0b
commit 1c591fd924
3 changed files with 63 additions and 47 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

39
src/corelib/tools/qline.cpp
View File

@ -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

13
src/corelib/tools/qline.h
View File

@ -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

58
tests/auto/corelib/tools/qline/tst_qline.cpp
View File

@ -40,8 +40,6 @@ private slots:
void testLength();
void testLength_data();
void testSetLength();
void testCenter();
void testCenter_data();
@ -214,11 +212,13 @@ void tst_QLine::testLength_data()
QTest::addColumn<double>("vx");
QTest::addColumn<double>("vy");
QTest::newRow("[1,0]*2") << 0.0 << 0.0 << 1.0 << 0.0 << 1.0 << 2.0 << 2.0 << 0.0;
QTest::newRow("[0,1]*2") << 0.0 << 0.0 << 0.0 << 1.0 << 1.0 << 2.0 << 0.0 << 2.0;
QTest::newRow("[-1,0]*2") << 0.0 << 0.0 << -1.0 << 0.0 << 1.0 << 2.0 << -2.0 << 0.0;
QTest::newRow("[0,-1]*2") << 0.0 << 0.0 << 0.0 << -1.0 << 1.0 << 2.0 << 0.0 << -2.0;
QTest::newRow("[1,1]->|1|") << 0.0 << 0.0 << 1.0 << 1.0
// Test name: [dx,dy]->|lenToSet| (x1,x2)
// with the last part omitted if (0,0)
QTest::newRow("[1,0]->|2|") << 0.0 << 0.0 << 1.0 << 0.0 << 1.0 << 2.0 << 2.0 << 0.0;
QTest::newRow("[0,1]->|2|") << 0.0 << 0.0 << 0.0 << 1.0 << 1.0 << 2.0 << 0.0 << 2.0;
QTest::newRow("[-1,0]->|2|") << 0.0 << 0.0 << -1.0 << 0.0 << 1.0 << 2.0 << -2.0 << 0.0;
QTest::newRow("[0,-1]->|2|") << 0.0 << 0.0 << 0.0 << -1.0 << 1.0 << 2.0 << 0.0 << -2.0;
QTest::newRow("[1,1]->->|1|") << 0.0 << 0.0 << 1.0 << 1.0
<< M_SQRT2 << 1.0 << M_SQRT1_2 << M_SQRT1_2;
QTest::newRow("[-1,1]->|1|") << 0.0 << 0.0 << -1.0 << 1.0
<< M_SQRT2 << 1.0 << -M_SQRT1_2 << M_SQRT1_2;
@ -226,10 +226,10 @@ void tst_QLine::testLength_data()
<< M_SQRT2 << 1.0 << M_SQRT1_2 << -M_SQRT1_2;
QTest::newRow("[-1,-1]->|1|") << 0.0 << 0.0 << -1.0 << -1.0
<< M_SQRT2 << 1.0 << -M_SQRT1_2 << -M_SQRT1_2;
QTest::newRow("[1,0]*2 (2,2)") << 2.0 << 2.0 << 3.0 << 2.0 << 1.0 << 2.0 << 2.0 << 0.0;
QTest::newRow("[0,1]*2 (2,2)") << 2.0 << 2.0 << 2.0 << 3.0 << 1.0 << 2.0 << 0.0 << 2.0;
QTest::newRow("[-1,0]*2 (2,2)") << 2.0 << 2.0 << 1.0 << 2.0 << 1.0 << 2.0 << -2.0 << 0.0;
QTest::newRow("[0,-1]*2 (2,2)") << 2.0 << 2.0 << 2.0 << 1.0 << 1.0 << 2.0 << 0.0 << -2.0;
QTest::newRow("[1,0]->|2| (2,2)") << 2.0 << 2.0 << 3.0 << 2.0 << 1.0 << 2.0 << 2.0 << 0.0;
QTest::newRow("[0,1]->|2| (2,2)") << 2.0 << 2.0 << 2.0 << 3.0 << 1.0 << 2.0 << 0.0 << 2.0;
QTest::newRow("[-1,0]->|2| (2,2)") << 2.0 << 2.0 << 1.0 << 2.0 << 1.0 << 2.0 << -2.0 << 0.0;
QTest::newRow("[0,-1]->|2| (2,2)") << 2.0 << 2.0 << 2.0 << 1.0 << 1.0 << 2.0 << 0.0 << -2.0;
QTest::newRow("[1,1]->|1| (2,2)") << 2.0 << 2.0 << 3.0 << 3.0
<< M_SQRT2 << 1.0 << M_SQRT1_2 << M_SQRT1_2;
QTest::newRow("[-1,1]->|1| (2,2)") << 2.0 << 2.0 << 1.0 << 3.0
@ -238,6 +238,20 @@ void tst_QLine::testLength_data()
<< M_SQRT2 << 1.0 << M_SQRT1_2 << -M_SQRT1_2;
QTest::newRow("[-1,-1]->|1| (2,2)") << 2.0 << 2.0 << 1.0 << 1.0
<< M_SQRT2 << 1.0 << -M_SQRT1_2 << -M_SQRT1_2;
const double small = qSqrt(std::numeric_limits<qreal>::denorm_min()) / 8;
QTest::newRow("[small,small]->|2| (-small/2,-small/2)")
<< -(small * .5) << -(small * .5) << (small * .5) << (small * .5)
<< (small * M_SQRT2) << (2 * M_SQRT2) << 2.0 << 2.0;
const double tiny = std::numeric_limits<qreal>::min() / 2;
QTest::newRow("[tiny,tiny]->|2| (-tiny/2,-tiny/2)")
<< -(tiny * .5) << -(tiny * .5) << (tiny * .5) << (tiny * .5)
<< (tiny * M_SQRT2) << (2 * M_SQRT2) << 2.0 << 2.0;
QTest::newRow("[1+3e-13,1+4e-13]|1895| (1, 1)")
<< 1.0 << 1.0 << (1 + 3e-13) << (1 + 4e-13) // isNull(), so ignores setLength()
<< 5e-13 << 1895.0 << 3e-13 << 4e-13;
QTest::newRow("[4e-323,5e-324]|1892|") // Unavoidable underflow: denormals
<< 0.0 << 0.0 << 4e-323 << 5e-324
<< 4e-323 << 1892.0 << 4e-323 << 5e-324; // vx, vy values ignored
}
void tst_QLine::testLength()
@ -252,21 +266,17 @@ void tst_QLine::testLength()
QFETCH(double, vy);
QLineF l(x1, y1, x2, y2);
QCOMPARE(l.length(), qreal(length));
const bool wasNull = l.isNull();
if (!wasNull)
QCOMPARE(l.length(), qreal(length));
l.setLength(lengthToSet);
QCOMPARE(l.length(), qreal(lengthToSet));
QCOMPARE(l.dx(), qreal(vx));
QCOMPARE(l.dy(), qreal(vy));
}
void tst_QLine::testSetLength()
{
QLineF l(0, 0, 4e-323, 5e-324);
const qreal newLength = 1892;
const qreal oldLength = l.length();
l.setLength(newLength);
QCOMPARE(l.length(), oldLength ? newLength : 0);
QCOMPARE(l.length(), wasNull ? qreal(length) : qreal(lengthToSet));
// Scaling tiny values up to big can be imprecise: don't try to test vx, vy
if (wasNull || !qFuzzyIsNull(length)) {
QCOMPARE(l.dx(), qreal(vx));
QCOMPARE(l.dy(), qreal(vy));
}
}
void tst_QLine::testCenter()