From f36fc2ab96268a31dfa32f712ef58f177a93db0a Mon Sep 17 00:00:00 2001 From: Kai Pastor Date: Fri, 24 Jan 2025 09:20:50 +0100 Subject: [PATCH 1/3] PathObjectTest: Test SplitPathCoord::at() --- test/path_object_t.cpp | 101 +++++++++++++++++++++++++++++++++++++++++ test/path_object_t.h | 3 ++ 2 files changed, 104 insertions(+) diff --git a/test/path_object_t.cpp b/test/path_object_t.cpp index a5b458ede..f8beec0ae 100644 --- a/test/path_object_t.cpp +++ b/test/path_object_t.cpp @@ -20,6 +20,9 @@ #include "path_object_t.h" +#include + +#include #include #include "core/map.h" @@ -1206,6 +1209,104 @@ void PathObjectTest::recalculatePartsTest() } } +void PathObjectTest::splitPathCoordAtTest() +{ + const PathCoord::length_type lengths_to_check[] = { + -0.2f, 0.2f, 1.0f, 1.2f, 1.4f, 1.5f, 1.8, 2.0f, 2.2f, 3.2f, 5.0f + }; + using std::begin; using std::end; + { + // Square, straight, closed + const MapCoordVector coords = { + { 0.0, 0.0 }, { 1.0, 0.0 }, { 1.0, 1.0 }, { 0.0, 1.0 }, { 0.0, 0.0, MapCoord::HolePoint } + }; + + PathCoordVector path_coords { coords }; + path_coords.update(0); + QCOMPARE(path_coords.size(), std::size_t(5)); + + const auto max_clen = path_coords.back().clen; + QCOMPARE(max_clen, 4.0f); + + const auto split = SplitPathCoord::at(1.5f, SplitPathCoord::begin(path_coords)); + { + const auto& prev = path_coords[split.path_coord_index]; + QCOMPARE(prev.clen, 1.0f); + const auto& next = path_coords[split.path_coord_index + 1]; + QCOMPARE(next.clen, 2.0f); + } + + auto prev_clen = SplitPathCoord::at(lengths_to_check[0], split).clen; + for (auto it = begin(lengths_to_check)+1, last = end(lengths_to_check); it != last; ++it) + { + const auto requested_clen = *it; + const auto clen = SplitPathCoord::at(requested_clen, split).clen; + if (qFuzzyCompare(requested_clen, 1.2f)) + { + const auto non_monotone = QString::fromLatin1("requested clen (%1) yields %2, previous clen is %3").arg(requested_clen).arg(clen).arg(prev_clen).toLocal8Bit(); + QEXPECT_FAIL("", non_monotone.constData(), Continue); + } + QVERIFY(clen >= prev_clen); // monotone + + if (requested_clen > max_clen) + { + const auto upper_bound = QString::fromLatin1("requested clen (%1) yields %2, max clen is %3").arg(requested_clen).arg(clen).arg(max_clen).toLocal8Bit(); + QEXPECT_FAIL("", upper_bound.constData(), Continue); + } + if (requested_clen >= max_clen) + QCOMPARE(clen, max_clen); // upper bound + else if (requested_clen >= split.clen) + QCOMPARE(clen, requested_clen); // expected case + prev_clen = clen; + } + } + + { + // Line, curved + const MapCoordVector coords = { + { 0.0, 0.0, MapCoord::CurveStart }, { 1.0, 0.0 }, { 1.0, 1.0 }, { 0.0, 1.0, MapCoord::HolePoint } + }; + + PathCoordVector path_coords { coords }; + path_coords.update(0); + QCOMPARE(path_coords.size(), std::size_t(10)); + + const auto max_clen = path_coords.back().clen; + QCOMPARE(max_clen, 2.00034f); + + const auto split = SplitPathCoord::at(1.5f, SplitPathCoord::begin(path_coords)); + { + const auto& prev = path_coords[split.path_coord_index]; + QCOMPARE(prev.clen, 1.29314f); + const auto& next = path_coords[split.path_coord_index + 1]; + QCOMPARE(next.clen, 1.52751f); + } + + auto prev_clen = SplitPathCoord::at(lengths_to_check[0], split).clen; + for (auto it = begin(lengths_to_check)+1, last = end(lengths_to_check); it != last; ++it) + { + const auto requested_clen = *it; +#ifndef NDEBUG + if (requested_clen > path_coords[split.path_coord_index].clen && requested_clen < split.clen) + continue; // triggers assert() +#endif + const auto clen = SplitPathCoord::at(requested_clen, split).clen; + QVERIFY(clen >= prev_clen); // monotone + + if (requested_clen > max_clen) + { + const auto upper_bound = QString::fromLatin1("requested clen (%1) yields %2, max clen is %3").arg(requested_clen).arg(clen).arg(max_clen).toLocal8Bit(); + QEXPECT_FAIL("", upper_bound.constData(), Continue); + } + if (requested_clen >= max_clen) + QCOMPARE(clen, max_clen); // upper bound + else if (requested_clen >= split.clen) + QCOMPARE(clen, requested_clen); // expected case + prev_clen = clen; + } + } +} + /* diff --git a/test/path_object_t.h b/test/path_object_t.h index 0a9c07382..4db42c45a 100644 --- a/test/path_object_t.h +++ b/test/path_object_t.h @@ -78,6 +78,9 @@ private slots: /** Tests recalculation of path parts from input coords. */ void recalculatePartsTest(); void recalculatePartsTest_data(); + + /** Test SplitPathCoord.at stability. */ + void splitPathCoordAtTest(); }; #endif From 4c2c588470523657e8c65b4972e0b5c552c55ccb Mon Sep 17 00:00:00 2001 From: Kai Pastor Date: Fri, 24 Jan 2025 06:09:27 +0100 Subject: [PATCH 2/3] SplitPathCoord: Add some const to 'at' --- src/core/path_coord.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/core/path_coord.cpp b/src/core/path_coord.cpp index 12ca41bd5..8b5348704 100644 --- a/src/core/path_coord.cpp +++ b/src/core/path_coord.cpp @@ -383,9 +383,9 @@ SplitPathCoord SplitPathCoord::at( length_type length, const SplitPathCoord& first ) { - auto& path_coords = *first.path_coords; - auto& coords = path_coords.coords(); - auto& flags = path_coords.flags(); + const auto& path_coords = *first.path_coords; + const auto& coords = path_coords.coords(); + const auto& flags = path_coords.flags(); SplitPathCoord split = first; split.path_coord_index = path_coords.upperBound(length, first.path_coord_index, first.path_coords->size()-1); From 7fe399c3a763e23ec08b72874e9c89f14c0325e2 Mon Sep 17 00:00:00 2001 From: Kai Pastor Date: Sun, 2 Feb 2025 10:20:44 +0100 Subject: [PATCH 3/3] SplitPathCoord: Ensure monotony and bounds of at() --- src/core/path_coord.cpp | 14 ++++++++++---- test/path_object_t.cpp | 21 --------------------- 2 files changed, 10 insertions(+), 25 deletions(-) diff --git a/src/core/path_coord.cpp b/src/core/path_coord.cpp index 8b5348704..6437e524f 100644 --- a/src/core/path_coord.cpp +++ b/src/core/path_coord.cpp @@ -388,7 +388,10 @@ SplitPathCoord SplitPathCoord::at( const auto& flags = path_coords.flags(); SplitPathCoord split = first; - split.path_coord_index = path_coords.upperBound(length, first.path_coord_index, first.path_coords->size()-1); + if (length > first.clen) + { + split.path_coord_index = path_coords.upperBound(length, first.path_coord_index, first.path_coords->size()-1); + } if (split.path_coord_index > first.path_coord_index) { // New path coordinate, really @@ -403,10 +406,12 @@ SplitPathCoord SplitPathCoord::at( auto factor = 1.0f; if (qFuzzyCompare(1.0f + length, 1.0f + current_coord.clen) || - qFuzzyCompare(1.0f + curve_length, 1.0f)) + qFuzzyCompare(1.0f + curve_length, 1.0f) || + length > current_coord.clen) { // Close match at current path coordinate, - // or near-zero curve length. + // or near-zero curve length, + // or length exceeding path length. split.pos = current_coord.pos; split.clen = current_coord.clen; split.param = current_coord.param; @@ -506,9 +511,10 @@ SplitPathCoord SplitPathCoord::at( { --split.path_coord_index; } + + split.index = path_coords[split.path_coord_index].index; } - split.index = path_coords[split.path_coord_index].index; return split; } diff --git a/test/path_object_t.cpp b/test/path_object_t.cpp index f8beec0ae..f3158c253 100644 --- a/test/path_object_t.cpp +++ b/test/path_object_t.cpp @@ -1241,18 +1241,7 @@ void PathObjectTest::splitPathCoordAtTest() { const auto requested_clen = *it; const auto clen = SplitPathCoord::at(requested_clen, split).clen; - if (qFuzzyCompare(requested_clen, 1.2f)) - { - const auto non_monotone = QString::fromLatin1("requested clen (%1) yields %2, previous clen is %3").arg(requested_clen).arg(clen).arg(prev_clen).toLocal8Bit(); - QEXPECT_FAIL("", non_monotone.constData(), Continue); - } QVERIFY(clen >= prev_clen); // monotone - - if (requested_clen > max_clen) - { - const auto upper_bound = QString::fromLatin1("requested clen (%1) yields %2, max clen is %3").arg(requested_clen).arg(clen).arg(max_clen).toLocal8Bit(); - QEXPECT_FAIL("", upper_bound.constData(), Continue); - } if (requested_clen >= max_clen) QCOMPARE(clen, max_clen); // upper bound else if (requested_clen >= split.clen) @@ -1286,18 +1275,8 @@ void PathObjectTest::splitPathCoordAtTest() for (auto it = begin(lengths_to_check)+1, last = end(lengths_to_check); it != last; ++it) { const auto requested_clen = *it; -#ifndef NDEBUG - if (requested_clen > path_coords[split.path_coord_index].clen && requested_clen < split.clen) - continue; // triggers assert() -#endif const auto clen = SplitPathCoord::at(requested_clen, split).clen; QVERIFY(clen >= prev_clen); // monotone - - if (requested_clen > max_clen) - { - const auto upper_bound = QString::fromLatin1("requested clen (%1) yields %2, max clen is %3").arg(requested_clen).arg(clen).arg(max_clen).toLocal8Bit(); - QEXPECT_FAIL("", upper_bound.constData(), Continue); - } if (requested_clen >= max_clen) QCOMPARE(clen, max_clen); // upper bound else if (requested_clen >= split.clen)