Skip to content

SplitPathCoord: Ensure monotony and bounds for at #2331

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

SplitPathCoord: Ensure monotony and bounds for at #2331

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f36fc2a
PathObjectTest: Test SplitPathCoord::at()
dg0yt Jan 24, 2025
4c2c588
SplitPathCoord: Add some const to 'at'
dg0yt Jan 24, 2025
7fe399c
SplitPathCoord: Ensure monotony and bounds of at()
dg0yt Feb 2, 2025
6414409
Merge branch 'OpenOrienteering:master' into split-path-coord
dg0yt Mar 16, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 10 additions & 4 deletions src/core/path_coord.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Comment on lines -391 to +394
Copy link
Member Author

@dg0yt dg0yt Feb 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix for monotony.

if (split.path_coord_index > first.path_coord_index)
{
// New path coordinate, really
Expand All @@ -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.
Comment on lines 408 to +414
Copy link
Member Author

@dg0yt dg0yt Feb 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix for upper bound.

split.pos = current_coord.pos;
split.clen = current_coord.clen;
split.param = current_coord.param;
Expand Down Expand Up @@ -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;
Comment on lines +514 to -511
Copy link
Member Author

@dg0yt dg0yt Feb 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Drive-by change. When not entering the if branch, the right split.index is implied by initialization of split.

return split;
}

Expand Down
21 changes: 0 additions & 21 deletions test/path_object_t.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down