BUG: Fix boundary propagation, topology labeling and zero speed in FastMarchingImageFilterBase

[physwkim]

Three defects in FastMarchingImageFilterBase: boundary seeds never propagate in the +1 direction, alive nodes are left unlabeled under the NoHandles topology check, and a zero-speed node yields a finite arrival time. Addresses B36, B37 and B38 of #6575.

B36 — boundary seeds do not propagate along both axis directions

UpdateNeighbors() bounds-checked the center index v against start/last instead of the candidate neighbor v + s, and on failure left neighIndex[j] at the center value instead of skipping that direction. A seed placed at the last valid index along an axis therefore re-visited itself rather than its -1 neighbor, and never visited its +1 neighbor at all — that half of the axis stayed at the large-value sentinel.

const typename NodeType::IndexValueType temp = v + s;
if ((temp < start) || (temp > last))
{
  continue;
}
neighIndex[j] = temp;

B37 — alive nodes carry no connected-component label

CheckTopology() assigned a connected-component label only to nodes that triggered a handle merge. Every other alive node kept label 0, so a later merge comparing two unlabeled components saw 0 == 0, concluded they were already the same component, and rejected the merge as a false handle violation.

Every alive node now takes the label of an alive neighbour, or a fresh label from m_NextConnectedComponentLabel (seeded in InitializeOutput() from relabeler->GetNumberOfObjects() + 1).

The merge-relabel scan also walks the connected-component iterator to the end of the image; the label lookup that follows then read past the buffer. Re-centering ItC on iNode closes that.

B38 — zero speed yields a finite arrival time

Solve() nudged a near-zero speed away from zero with itk::Math::eps before inverting, so an exactly-zero speed produced a large-but-finite value that downstream comparisons treat as reachable. It now returns the large-value sentinel directly.

Local validation

New GoogleTest itkFastMarchingImageFilterBaseGTest.cxx in a new ITKFastMarchingGTests driver, one test per item, each verified to fail on origin/main and pass with its fix:

• CornerSeedPropagatesAlongBothBoundaryAxes — fail-before 3.40282347e+38 vs expected 100; pass-after.
• AliveNodesAreLabeledUnderNoHandlesTopologyCheck — fail-before; pass-after.
• ZeroSpeedNodeIsUnreachable — fail-before; pass-after.

All 12 existing FastMarching ExternalData baseline tests (including wm_multipleSeeds_NoHandlesTopo) pass at ImageError = 0 at each of the three commits. The out-of-bounds read in B37 was found by exactly that suite — it reproduced as a segfault on the 256x300x256 baseline while the small unit test passed.

AI assistance

• Tool: Claude Code
• Role: implementation and regression-test authoring from the B36/B37/B38 analysis in BUG: Findings collected while porting ~60 filters — reported together in one issue #6575
• All code was reviewed, built, and tested locally before committing

[greptile-apps]

Greptile Summary

This PR updates FastMarchingImageFilterBase behavior and adds focused tests.

• Boundary neighbor propagation now checks candidate indices.
• NoHandles topology now labels accepted alive nodes.
• Exact zero-speed pixels now stay unreachable.
• A new GoogleTest driver covers the three regressions.

Confidence Score: 4/5

The near-zero speed-image path needs a fix before merging.

Tiny nonzero speed values now reach the reciprocal path, which can produce overflow, an exception, or a huge finite arrival time. The CMake and test-driver changes follow the existing ITK pattern.

Modules/Filtering/FastMarching/include/itkFastMarchingImageFilterBase.hxx

T-Rex Logs

What T-Rex did

• Ran a focused C++17 harness that mirrors the near-zero speed reciprocal path to reproduce the overflow behavior on a denormal nonzero speed.
• Observed that the nonzero current speed skips the exact-zero barrier and causes the reciprocal calculation to produce inf and -inf, instead of returning the intended large barrier.
• Inspected environment and test-setup logs, revealing missing build tools, a failed initial configure, and that GoogleTest is registered with three test cases whose prompt names do not match the local test names.

_{Ran code and verified through T-Rex}

Important Files Changed

Filename	Overview
Modules/Filtering/FastMarching/include/itkFastMarchingImageFilterBase.h	Adds the next connected-component label counter used by the topology-labeling update.
Modules/Filtering/FastMarching/include/itkFastMarchingImageFilterBase.hxx	Fixes neighbor bounds, topology component labeling, iterator recentering, and zero-speed handling; near-zero speed handling still needs adjustment.
Modules/Filtering/FastMarching/itk-module.cmake	Adds ITKGoogleTest as a test dependency.
Modules/Filtering/FastMarching/test/CMakeLists.txt	Registers the new FastMarching GoogleTest driver.
Modules/Filtering/FastMarching/test/itkFastMarchingImageFilterBaseGTest.cxx	Adds regression tests for boundary propagation, NoHandles labeling, and zero-speed pixels.

_{Reviews (1): Last reviewed commit: "BUG: Return large value for zero-speed n..." | Re-trigger Greptile}

[hjmjohnson]

Force-push ff7a73d → c32872d (maintainer edit): gersemi formatting fix for test/CMakeLists.txt squashed into the commit that introduced it — no functional change. Fixes the red pre-commit check.

[hjmjohnson]

Pushed ce821ed (maintainer edit, plain fast-forward): addresses the greptile P1 plus findings from a deeper multi-agent review of the NoHandles labeling scheme.

What changed and why

• Missed label merge on perpendicular joins: when two fronts first meet at a node whose Alive neighbors lie on different axes (no strict-topology violation), the components joined physically but kept distinct labels, so a later reconnection closed an undetected handle. All distinct neighbor labels are now merged on every accepted join.
• All-dimensions handle check: the loop previously stopped at the first both-sides-Alive axis, skipping a possible same-component reconnection in a later dimension.
• Hot-path cost: the two radius-1 NeighborhoodIterators built per accepted node are replaced with GetPixel face-neighbor reads and an ImageRegionIterator relabel scan.
• Near-zero speeds (greptile P1): non-finite reciprocals now return m_LargeValue.
• MSVC narrowing: GetNumberOfObjects() + 1 is cast to the label type.
• New GTest: fronts wrapping a zero-speed obstacle may not close the ring into a handle.

Local validation: all 53 FastMarching ctests pass, including the three zero-tolerance *_NoHandlesTopo baseline comparisons, and 4/4 GTests; pre-commit run --all-files exits 0.

[hjmjohnson]

First force-push = plain rebase on main; second = review fixes only, folded into the zero-speed commit: cc == 0.0 → itk::Math::ExactlyEquals (the module's warning-clean idiom), and Solve() now returns min(solution, m_LargeValue) so a tiny-but-nonzero speed (e.g. 1e-40) can no longer produce a solution that overflows the cast to a float output pixel. Local ctest -R FastMarching: 49/49 pass.

Review notes for the author (design-level, not acted on)

• Zero-speed nodes now stay Far permanently rather than eventually going Alive at a huge finite time. That changes behavior for target/threshold stopping criteria whose target sits inside a zero-speed region (previously stopped; now floods the image and returns). If intended, worth a line in the PR body.
• The NoHandles merge path rewrites the whole connected-component image on every front merge — O(voxels × merges). A label-equivalence map (union-find consulted on lookup) would be O(1) per merge.
• The 3-line rejection idiom in CheckTopology now appears three times; a small private helper would keep the bookkeeping consistent.