Skip to content

BUG: Backport Voronoi infinite loop fix to release-5.4#6052

Merged
hjmjohnson merged 2 commits intoInsightSoftwareConsortium:release-5.4from
hjmjohnson:backport-voronoi-fix
Apr 14, 2026
Merged

BUG: Backport Voronoi infinite loop fix to release-5.4#6052
hjmjohnson merged 2 commits intoInsightSoftwareConsortium:release-5.4from
hjmjohnson:backport-voronoi-fix

Conversation

@hjmjohnson
Copy link
Copy Markdown
Member

Backport 3b3f3c24d4 from main to release-5.4. Fixes an infinite loop in VoronoiDiagram2DGenerator::ConstructDiagram when Fortune's algorithm produces degenerate near-zero-length edges.

Part of #6051 (backport candidates for 5.4.6).

Cherry-pick details

Cherry-picked from commit 3b3f3c24d4 (merged to main via PR #6017). Required manual conflict resolution:

  • itkVoronoiDiagram2DGenerator.hxx: moved frontbnd/backbnd declarations from function scope into block scope (release-5.4 had them at outer scope; main moved them to inner loop + second usage block)
  • CMakeLists.txt: added new test file to the test list while preserving release-5.4 formatting style

Local build and test verified: 8/8 Voronoi tests pass including the new itkVoronoiDiagram2DInfiniteLoopTest.

@github-actions github-actions bot added type:Bug Inconsistencies or issues which will cause an incorrect result under some or all circumstances type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots type:Testing Ensure that the purpose of a class is met/the results on a wide set of test cases are correct area:Segmentation Issues affecting the Segmentation module labels Apr 13, 2026
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Apr 13, 2026

Greptile Summary

This PR backports commit 3b3f3c24d4 from main to release-5.4, fixing an infinite loop in VoronoiDiagram2DGenerator::ConstructDiagram triggered by near-collinear seeds that cause Fortune's algorithm to emit near-zero-length edges with distinct vertex IDs but geometrically identical endpoints. The fix introduces a stall counter to detect when chain assembly makes no forward progress and adds explicit degenerate-edge detection using the existing differentPoint() / DIFF_TOLERENCE check, accompanied by a new regression test.

Confidence Score: 5/5

Safe to merge — the fix is logically sound, preserves all existing retry semantics, and converts a guaranteed hang into a clear exception for truly stuck configurations.

All remaining findings are P2 style suggestions. The stall counter correctly terminates after one complete no-progress pass (matching the original infinite-loop condition), the degenerate-edge discard reuses the pre-existing DIFF_TOLERENCE check, and the exception guard is strictly better than the previous silent hang. The conflict resolution (variable scope) is correct, and the regression test directly reproduces the reported bug.

No files require special attention.

Important Files Changed

Filename Overview
Modules/Segmentation/Voronoi/include/itkVoronoiDiagram2DGenerator.hxx Core fix: stall counter + degenerate-edge discard replaces the previously unbounded retry loop; variable scoping conflict with release-5.4 resolved correctly via block-scope redeclarations of frontbnd/backbnd.
Modules/Segmentation/Voronoi/test/itkVoronoiDiagram2DInfiniteLoopTest.cxx New regression test with hardcoded near-collinear seeds that previously caused an infinite loop; uses ITK_TRY_EXPECT_NO_EXCEPTION and verifies all 6 cell boundaries are non-empty.
Modules/Segmentation/Voronoi/test/CMakeLists.txt Adds new test source and itk_add_test() registration; minor formatting inconsistency in the NAME keyword layout compared to the rest of the file.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[ConstructDiagram: pop first edge → seed chain] --> B[remainingBeforeStall = deque.size]
    B --> C{deque empty\nor stall=0?}
    C -- No --> D[pop edge from deque\ndecrement stall counter]
    D --> E{edgeStart ≈ edgeEnd\ndifferentPoint = false?}
    E -- Yes: degenerate --> F[discard edge\nreset stall counter]
    F --> C
    E -- No: valid edge --> G{Can attach to\nchain front/back?}
    G -- Yes --> H[attach edge\nupdate front/back\nreset stall counter]
    H --> C
    G -- No: boundary bridge? --> I{Boundary ID\nmatch?}
    I -- Yes --> J[insert bridge + edge\nreset stall counter]
    J --> C
    I -- No --> K[push edge back\nno stall reset]
    K --> C
    C -- Yes: done --> L{deque empty?}
    L -- Yes --> M[close chain if needed\nbuild cell]
    L -- No: stuck edges remain --> N[itkExceptionMacro\nunexpected configuration]
Loading

Reviews (1): Last reviewed commit: "BUG: Fix infinite loop in VoronoiDiagram..." | Re-trigger Greptile

Comment thread Modules/Segmentation/Voronoi/test/CMakeLists.txt
@hjmjohnson hjmjohnson force-pushed the backport-voronoi-fix branch 2 times, most recently from 09a39e0 to 0db6b82 Compare April 14, 2026 01:49
@hjmjohnson hjmjohnson requested a review from thewtex April 14, 2026 10:24
@hjmjohnson
Copy link
Copy Markdown
Member Author

CI failures are known flakes — not related to this PR's code changes.

  • ITK.macOS.Python: exit code 255 from ci_completed_successfully in the dashboard script (treats warnings as fatal). CDash shows 0 compile errors and 0 test failures.
  • CDash: status check is stale/pending — actual CDash results are clean.

All 8 Voronoi tests pass locally (verified on macOS ARM64). The spell check that previously failed (unattachable not in dictionary) was fixed in the current HEAD.

@hjmjohnson
Copy link
Copy Markdown
Member Author

/azp run ITK.macOS.Python

@hjmjohnson hjmjohnson force-pushed the backport-voronoi-fix branch 3 times, most recently from 5a3c880 to 784cbd6 Compare April 14, 2026 12:10
Backport the itk_dict.txt word list from upstream/main to release-5.4,
then add 20 words found in release-5.4 source that are not in main's
dictionary (code identifiers in ThirdParty headers, legitimate terms
like Hounsfield and Dask, and variable names in comments that cannot
be changed on the release branch).

Uses main's sort order to minimize future merge diffs.
Backport from main (3b3f3c2). Adds stall detection and degenerate
edge filtering to prevent infinite loops in ConstructDiagram when
Fortune's algorithm produces near-zero-length edges.

Adds itkVoronoiDiagram2DInfiniteLoopTest regression test.

(cherry picked from commit 3b3f3c2)
@hjmjohnson hjmjohnson force-pushed the backport-voronoi-fix branch from 784cbd6 to 6b157cf Compare April 14, 2026 12:22
@thewtex thewtex added this to the ITK 5.4.6 milestone Apr 14, 2026
@hjmjohnson hjmjohnson merged commit 5934cc8 into InsightSoftwareConsortium:release-5.4 Apr 14, 2026
14 of 18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:Segmentation Issues affecting the Segmentation module type:Bug Inconsistencies or issues which will cause an incorrect result under some or all circumstances type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots type:Testing Ensure that the purpose of a class is met/the results on a wide set of test cases are correct

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants