Skip to content

Add missing commits from release-7.0 branch #199

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

Merged
gagelarsen merged 4 commits into from
Aug 17, 2026
Merged

Add missing commits from release-7.0 branch #199

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6a58b01
Add a method to GmPtSearch that lets you find points within a box.
Jun 27, 2025
a07f38f
Add a point in box 3d test.
Jul 17, 2025
e54c868
Fix issue 0015785: HydroAS arcs snap weirdly
Jul 22, 2025
fb0ca5d
Update a test baseline.
Jul 22, 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
86 changes: 86 additions & 0 deletions test_files/bug-0015785.xmugrid
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
ASCII XmUGrid Version 2
LOCATIONS 48
POINT 0 -38528.08 254321.0 769.744019
POINT 1 -38528.0 254321.25 769.744019
POINT 2 -38528.0 254321.0 769.744019
POINT 3 -36899.68 262325.09 769.744019
POINT 4 -36892.32 262327.386 769.744019
POINT 5 -36897.87 262322.32 769.744019
POINT 6 -40659.55 247159.57 675.431478
POINT 7 -40665.6009 247157.7 675.431478
POINT 8 -40663.43 247164.9 675.431478
POINT 9 -38526.34 254328.197 723.4776
POINT 10 -38526.568 254330.333 723.418762
POINT 11 -38525.608 254328.434 723.4776
POINT 12 -38526.757 254333.772 724.232239
POINT 13 -38525.3923 254328.592 724.625953
POINT 14 -38525.534 254330.438 723.418762
POINT 15 -38525.709 254333.817 724.399628
POINT 16 -38524.6954 254328.775 724.307859
POINT 17 -38524.7269 254330.534 724.372841
POINT 18 -38524.7873 254333.899 724.49722
POINT 19 -38523.9986 254328.958 723.989764
POINT 20 -38523.9341 254330.627 723.935229
POINT 21 -38524.2333 254333.949 724.219912
POINT 22 -38523.3775 254329.029 723.866525
POINT 23 -38523.1413 254330.721 723.497616
POINT 24 -38523.6793 254333.999 723.942604
POINT 25 -38522.7565 254329.101 723.743286
POINT 26 -38522.3485 254330.815 723.060004
POINT 27 -38523.1339 254334.047 723.512122
POINT 28 -38522.1354 254329.172 723.620047
POINT 29 -38521.5557 254330.909 722.622391
POINT 30 -38522.6326 254334.092 723.081258
POINT 31 -38520.7629 254331.003 722.184779
POINT 32 -38520.4547 254329.366 721.993129
POINT 33 -38521.3661 254334.206 722.559801
POINT 34 -38519.8416 254331.112 721.720529
POINT 35 -38519.1657 254329.513 721.800792
POINT 36 -38520.2036 254334.309 722.415745
POINT 37 -38518.552 254331.353 721.256279
POINT 38 -38517.8767 254329.659 721.608454
POINT 39 -38519.0418 254334.413 722.27193
POINT 40 -38517.0778 254331.439 720.32778
POINT 41 -38516.83 254329.95 720.0238
POINT 42 -38517.5943 254334.543 720.961488
POINT 43 -38515.934 254331.574 720.144667
POINT 44 -38515.064 254330.029 719.84848
POINT 45 -38516.3064 254334.658 720.592489
POINT 46 -38514.7903 254331.71 719.961554
POINT 47 -38515.0185 254334.773 720.223491
CELL_STREAM 191
CELL 0 TRIANGLE 3 1 0 2
CELL 1 TRIANGLE 3 4 3 5
CELL 2 TRIANGLE 3 7 6 8
CELL 3 QUAD 4 9 11 14 10
CELL 4 QUAD 4 12 10 14 15
CELL 5 TRIANGLE 3 11 13 14
CELL 6 QUAD 4 16 17 14 13
CELL 7 QUAD 4 17 18 15 14
CELL 8 QUAD 4 19 20 17 16
CELL 9 TRIANGLE 3 20 18 17
CELL 10 TRIANGLE 3 20 21 18
CELL 11 QUAD 4 20 19 22 23
CELL 12 QUAD 4 24 20 23 27
CELL 13 TRIANGLE 3 20 24 21
CELL 14 TRIANGLE 3 25 23 22
CELL 15 QUAD 4 26 23 25 28
CELL 16 QUAD 4 27 23 26 30
CELL 17 TRIANGLE 3 29 26 28
CELL 18 TRIANGLE 3 30 26 29
CELL 19 TRIANGLE 3 29 28 32
CELL 20 TRIANGLE 3 31 29 32
CELL 21 TRIANGLE 3 29 31 33
CELL 22 TRIANGLE 3 29 33 30
CELL 23 QUAD 4 32 35 34 31
CELL 24 QUAD 4 36 33 31 34
CELL 25 QUAD 4 35 38 37 34
CELL 26 QUAD 4 36 34 37 39
CELL 27 TRIANGLE 3 38 40 37
CELL 28 QUAD 4 39 37 40 42
CELL 29 TRIANGLE 3 40 38 41
CELL 30 TRIANGLE 3 43 40 41
CELL 31 QUAD 4 45 42 40 43
CELL 32 TRIANGLE 3 43 41 44
CELL 33 TRIANGLE 3 44 46 43
CELL 34 QUAD 4 45 43 46 47
52 changes: 51 additions & 1 deletion xmsgrid/geometry/GmMultiPolyIntersectionSorterTerse.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,9 @@ void GmMultiPolyIntersectionSorterTerse::Sort(

RemoveCornerTouches();

@gagelarsen gagelarsen Aug 17, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

[Medium] New pass runs after RemoveCornerTouches, which can already orphan half a legitimate pair.

RemoveIntersectionsWithoutMatch() is inserted after RemoveCornerTouches(), which can itself already have deleted one half of a legitimate entry/exit pair. FindPreviousNextNeither only inspects the immediately adjacent t-groups, so a cell entered in t-group i (a group with >1 member, e.g. a shared node) and exited two or more groups later has its entry classified inNeither and erased by the (i > 0 && i + 2 < tChange.size()) branch.

Previously the orphaned exit survived and the cell still appeared in the traversal output; now the new pass deletes the orphan too, so the cell vanishes entirely and the neighbouring cell's span silently absorbs its interval.

Running the new pass before RemoveCornerTouches (or teaching it about m_polys1/m_polys2) would avoid compounding the two removals.

RemoveDuplicateEdges();
RemoveIntersectionsWithoutMatch();
SwapAdjacents();

@gagelarsen gagelarsen Aug 17, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

[Medium] The second RemoveCornerTouches() was deleted, not just supplemented.

That call ran after SwapAdjacents() and was the only corner-touch pass to see the post-swap ordering. RemoveIntersectionsWithoutMatch is not a superset of it: a corner touch that yields two intersections for the same cell at nearly equal (but not bitwise equal) t values is treated by the new pass as a valid entry/exit pair and kept, whereas the removed pass would have grouped them via EQ_TOL(..., m_tol) and dropped them.

If the deletion is intentional it deserves a comment explaining why the new pass subsumes it; otherwise the call should stay.

RemoveCornerTouches();

IntersectionsToPolyIdsAndTValues(polyids, tvalues, a_pts);
FixArrays(polyids, tvalues, a_pts);
} // GmMultiPolyIntersectionSorterTerse::Sort
Expand Down Expand Up @@ -264,6 +265,55 @@ void GmMultiPolyIntersectionSorterTerse::SwapAdjacents() {
}
}
} // GmMultiPolyIntersectionSorterTerse::SwapAdjacents
//------------------------------------------------------------------------------
/// \brief Remove an entry/exit without a matching exit/entry.
/// See test GmMultiPolyIntersector2IntermediateTests::testBug15785.
//------------------------------------------------------------------------------
void GmMultiPolyIntersectionSorterTerse::RemoveIntersectionsWithoutMatch()
{
VecBool hasMatch(m_d->m_ixs.size(), false);
if (hasMatch.size() < 2)

@gagelarsen gagelarsen Aug 17, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

[Low] hasMatch.size() < 2 guard makes the pass's contract size-dependent.

A single unmatched intersection is preserved here (and FixArrays then synthesizes an exit, so the cell is still reported), while two or more all-unmatched intersections wipe m_ixs and yield empty polyIds/tValues/points.

Either the lone orphan should also be dropped, or the guard should be documented; as written it reads like an accidental special case rather than a deliberate one.

{
return;
}

for (int i = 0; i < m_d->m_ixs.size(); i++)

@gagelarsen gagelarsen Aug 17, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

[Medium] Greedy first-match pairing mispairs cells with an odd number of surviving intersections.

Concrete case (possible with a concave UGrid cell, or a cell the line grazes at a vertex mid-crossing): cell A has entry@0.1, vertex-graze@0.5, exit@0.9. i=0 pairs 0.1 with the graze at 0.5 and breaks; 0.9 is then unmatched and erased.

The output reports A as ending at t=0.5 instead of 0.9 — wrong exit point, plus a gap in the traversal. Correct behaviour would pair outermost-first, or drop the middle singleton.

@gagelarsen gagelarsen Aug 17, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

[Low] Signed/unsigned comparison and an avoidable full-vector copy.

int i / int j are compared against m_d->m_ixs.size() / oldIx.size() at lines 280, 287 and 308, which produces signed/unsigned warnings; the rest of this file consistently uses size_t.

Also, std::vector<xms::ix> oldIx = m_d->m_ixs; (line 308) copies the whole vector just to filter it — a std::remove_if over hasMatch in place would avoid the copy. (Minor: m_ixs is small.)

{
if (hasMatch[i])
{
continue;
}

double t = m_d->m_ixs[i].m_t;
int cell = m_d->m_ixs[i].m_i;

for (int j = i + 1; j < m_d->m_ixs.size(); j++)
{
// If it's already paired, or not part of a future block.
if (hasMatch[j] || m_d->m_ixs[j].m_t <= t)

@gagelarsen gagelarsen Aug 17, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

[Low] Exact floating-point <= on t values conflicts with the tolerance-based t-grouping used everywhere else in this file.

FindWhereTValuesChange uses EQ_TOL(..., m_tol) and SwapAdjacents uses EQ_EPS(..., FLT_EPSILON). The comment says "not part of a future block", but the code's notion of "block" disagrees with the rest of the pipeline for any pair separated by less than m_tol (which TraverseLineSegment sets to min(minCellFraction * 1e-5, 1e-5)).

Suggest !EQ_TOL(m_d->m_ixs[j].m_t, t, m_tol) && m_d->m_ixs[j].m_t > t for consistency.

{
continue;
}

if (m_d->m_ixs[j].m_i == cell)
{
hasMatch[i] = hasMatch[j] = true;
break;
}
}
}

std::vector<xms::ix> oldIx = m_d->m_ixs;
m_d->m_ixs.clear();
for (int i = 0; i < oldIx.size(); i++)
{
if (hasMatch[i])
{
m_d->m_ixs.push_back(oldIx[i]);
}
}
} // GmMultiPolyIntersectionSorterTerse::RemoveIntersectionsWithoutMatch

//------------------------------------------------------------------------------
/// \brief Add endpoint polygon IDs that may be removed as duplicates.
/// \param a_tChange: indexes of t-value changes.
Expand Down
1 change: 1 addition & 0 deletions xmsgrid/geometry/GmMultiPolyIntersectionSorterTerse.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ class GmMultiPolyIntersectionSorterTerse
void RemoveCornerTouches();
void RemoveDuplicateEdges();
void SwapAdjacents();
void RemoveIntersectionsWithoutMatch();
void AddMissingEndpointIds(const std::vector<int>& a_tChange);
void IntersectionsToPolyIdsAndTValuesFor2(std::vector<int> &polyids,
std::vector<double> &tvalues,
Expand Down
Loading
Loading