diff options
author | Edward Welbourne <edward.welbourne@qt.io> | 2020-09-09 14:59:32 +0200 |
---|---|---|
committer | Edward Welbourne <edward.welbourne@qt.io> | 2020-09-30 16:26:49 +0200 |
commit | 1c591fd9246ca776304a3c370dd2578bd886feac (patch) | |
tree | 02a46d7f631832955487de2fd5c5b5856dfbcfad /tests/auto/corelib/tools/qline | |
parent | c25687fa0b6e4be043e1f8c92c093d8b06ca06c4 (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.)
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>
Diffstat (limited to 'tests/auto/corelib/tools/qline')
-rw-r--r-- | tests/auto/corelib/tools/qline/tst_qline.cpp | 58 |
1 files changed, 34 insertions, 24 deletions
diff --git a/tests/auto/corelib/tools/qline/tst_qline.cpp b/tests/auto/corelib/tools/qline/tst_qline.cpp index 7e07e9d1e2..f4ac51fc24 100644 --- a/tests/auto/corelib/tools/qline/tst_qline.cpp +++ b/tests/auto/corelib/tools/qline/tst_qline.cpp @@ -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() |