ENH: Ingest ITKGrowCut into Modules/Segmentation/GrowCut#6274
Merged
hjmjohnson merged 47 commits intoMay 19, 2026
Conversation
Code taken from: https://github.com/SCIInstitute/Seg3D/tree/growcut/src/Application/Tools/Algorithm at 879f887cddaa1905afa732da98aa9850b7d4d050.
src\HeapNode.cc(45): warning C4244: '=': conversion from 'double' to 'float', possible loss of data
Using the same type for labels as for image intensities does not make much sense.
* use CMake v3.21.1 in CI * add export DLL specifications to classes in FibHeap * update README.rst to reflect Slicer-inspired refactoring * add a missing Reset() method
The ability to include either .h or .hxx files as header files required recursively reading the .h files twice. The added complexity is unnecessary, costly, and can confuse static analysis tools that monitor header guardes (due to reaching the maximum depth of recursion limits for nested #ifdefs in checking).
warning.
The resultLabelVolumePtr variable is declared with the same value
before and within the if statement. The second declaration was removed.
This resolves the following Compiler warning:
/localscratch/Users/kjweimer/ITK/Modules/Remote/GrowCut/include/itkFastGrowCut.hxx:295:24: warning: declaration of ‘resultLabelVolumePtr’ shadows a previous local [-Wshadow]
295 | LabelPixelType * resultLabelVolumePtr = resultLabelVolume->GetBufferPointer();
| ^~~~~~~~~~~~~~~~~~~~
/localscratch/Users/kjweimer/ITK/Modules/Remote/GrowCut/include/itkFastGrowCut.hxx:288:30: note: shadowed declaration is here
288 | LabelPixelType * resultLabelVolumePtr = resultLabelVolume->GetBufferPointer();
| ^~~~~~~~~~~~~~~~~~~~
Class member variables m_DimX, m_DimY, and m_DimZ were redeclared as instance variables
in a class function. This was changed to instead use the existing member variables.
This resolvs the following compiler shadowing warning:
/localscratch/Users/kjweimer/ITK/Modules/Remote/GrowCut/include/itkFastGrowCut.hxx:99:17: warning: declaration of ‘m_DimX’ shadows a member of ‘itk::FastGrowCut<itk::Image<short int, 3>, itk::Image<unsigned char, 3> >’ [-Wshadow]
99 | NodeIndexType m_DimX = region.GetSize(0);
| ^~~~~~
In file included from /localscratch/Users/kjweimer/ITK/Modules/Remote/GrowCut/test/itkFastGrowCutTest.cxx:19:
/localscratch/Users/kjweimer/ITK/Modules/Remote/GrowCut/include/itkFastGrowCut.h:181:17: note: shadowed declaration is here
181 | NodeIndexType m_DimX;
| ^~~~~~
(This warning is repeated for m_DimY and m_DimZ)
These `seedImage` and `compareTolerance` variables are only used within a code
block that is currently commented out. Alternatively, these variables could be
deleted if it is decided that the commented out code is no longer wanted.
This resolves the following compiler warnings:
/localscratch/Users/kjweimer/ITK/Modules/Remote/GrowCut/include/itkFastGrowCut.hxx:376:14: warning: unused variable ‘seedImage’ [-Wunused-variable]
376 | auto seedImage = this->GetSeedImage();
| ^~~~~~~~~
/localscratch/Users/kjweimer/ITK/Modules/Remote/GrowCut/include/itkFastGrowCut.hxx:381:16: warning: unused variable ‘compareTolerance’ [-Wunused-variable]
381 | const double compareTolerance = (spacing[0] + spacing[1] + spacing[2]) / 3.0 * 0.01;
| ^~~~~~~~~~~~~~~~
Member
|
Is there any chance we can address InsightSoftwareConsortium/ITKGrowCut#18 during this refactoring? It is the only outstanding issue in this remote. |
Member
Author
|
/azp run |
Member
Author
|
@greptileai review |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
hjmjohnson
added a commit
to hjmjohnson/ITK
that referenced
this pull request
May 17, 2026
Replace the placeholder "Module ingested from upstream." with text describing what the module does (Dijkstra/Fibonacci-heap seeded region growing with optional masking and adaptive re-segmentation), per reviewer feedback on PR InsightSoftwareConsortium#6274.
hjmjohnson
added a commit
to hjmjohnson/ITK
that referenced
this pull request
May 17, 2026
The IPFS gateway (dweb.link / w3s.link) returned HTTP 504 for these 7 CIDs during the Populate ExternalData CI step, blocking PR InsightSoftwareConsortium#6274. The blobs themselves are present on data.kitware.com (verified via hashsum/sha512 lookup), so switching the content link from CID to SHA512 restores access via the Kitware mirror. The SHA512 digests were recovered from commit 5a1cbc5 ("Convert from md5 to .cid tags.") which had the prior .sha512 stubs in tree before the cid migration. Affected files: test/Input/Seeds.seg.nrrd test/Baseline/DzZ_L1-label.nrrd test/Baseline/DzZ_T1.seg.nrrd test/Baseline/MedianResult.seg.nrrd test/Baseline/NoisyResult.seg.nrrd test/Baseline/itkFastGrowCutTestMedian-label.nrrd test/Baseline/itkFastGrowCutTestNoisy-label.nrrd
hjmjohnson
force-pushed
the
ingest-GrowCut
branch
from
ee37dc4 to
e73c075
Compare
This was referenced May 19, 2026
Member
Author
Addressed and closes: |
Brings GrowCut from a configure-time remote fetch into the ITK source tree at Modules/Segmentation/GrowCut/ using the v4 ingestion pipeline (whitelist filter-repo + per-commit clang-format + black + commit-prefix sanitization). Upstream repo: https://github.com/InsightSoftwareConsortium/ITKGrowCut.git Upstream tip: 56e4eae7d92496ec7d0bd023d033b8ddf22cbc0e Ingest date: 2026-05-14 Whitelist: default.list Per-commit transforms applied across all 34 commits: - filter-repo --paths-from-file (whitelist) - filter-repo --to-subdirectory-filter Modules/Segmentation/GrowCut - clang-format -style=file (ITK main's .clang-format) for *.cxx/.h/.hxx/... - black for *.py - heuristic ITK prefix added to commit subjects without one Merge topology preserved: 6 -> 1 merge(s). Primary author: Dženan Zukić <dzenan.zukic@kitware.com> Co-authored-by: Hans J. Johnson <hans-johnson@uiowa.edu> Co-authored-by: Hans Johnson <hans-johnson@uiowa.edu> Co-authored-by: Jean-Christophe Fillion-Robin <jchris.fillionr@kitware.com> Co-authored-by: Jon Haitz Legarreta Gorroño <jon.haitz.legarreta@gmail.com> Co-authored-by: Kian Weimer <kianweimer@gmail.com> Co-authored-by: Tom Birdsong <tom.birdsong@kitware.com>
Module ingested into Modules/Segmentation/GrowCut; activate the in-tree build via configure-ci option and retire the now-redundant remote-module fetch stub. Gersemi reformat of src/CMakeLists.txt absorbed in the same commit.
Drop unreachable null check after operator new (throws std::bad_alloc), the unused ExtractITKImageROI helper, the unused itkImageMaskSpatialObject.h include, and the corresponding ITKSpatialObjects module dependency.
The defaulted destructor leaked the raw owning pointers if the filter was destroyed after InitializationAHP() allocated them, including on exception unwind.
Replace bare OK/NOTOK macros with constexpr ints to avoid global-namespace collisions with platform headers, guard the parent-index check against a null theParent so the documented default Print() does not dereference null, and drop the interactive cin>>ch wait that hung non-interactive callers.
The if(NOT ITK_SOURCE_DIR) branch (find_package(ITK) + include(ITKModuleExternal)) only executes when building the module standalone outside ITK. In-tree builds always take the itk_module_impl() else branch. Drop the dead branch following the cleanup pattern from InsightSoftwareConsortium#6272/InsightSoftwareConsortium#6279.
Replace the placeholder "Module ingested from upstream." with text describing what the module does (Dijkstra/Fibonacci-heap seeded region growing with optional masking and adaptive re-segmentation), per reviewer feedback on PR InsightSoftwareConsortium#6274.
The IPFS gateway (dweb.link / w3s.link) returned HTTP 504 for these 7 CIDs during the Populate ExternalData CI step, blocking PR InsightSoftwareConsortium#6274. The blobs themselves are present on data.kitware.com (verified via hashsum/sha512 lookup), so switching the content link from CID to SHA512 restores access via the Kitware mirror. The SHA512 digests were recovered from commit 5a1cbc5 ("Convert from md5 to .cid tags.") which had the prior .sha512 stubs in tree before the cid migration. Affected files: test/Input/Seeds.seg.nrrd test/Baseline/DzZ_L1-label.nrrd test/Baseline/DzZ_T1.seg.nrrd test/Baseline/MedianResult.seg.nrrd test/Baseline/NoisyResult.seg.nrrd test/Baseline/itkFastGrowCutTestMedian-label.nrrd test/Baseline/itkFastGrowCutTestNoisy-label.nrrd
KWStyle's per-module Header check scans src/*.cxx (not just itk*.cxx), so FibHeap.cxx must carry the ITK Apache 2.0 banner. Add the same banner to FibHeap.h for symmetry. The original John Boyer (1996) attribution comment is preserved immediately below the new banner. Fixes GrowCutKWStyleTest in the Pixi-Cxx CI matrix.
GenerateInputRequestedRegion() forced only input 0 (intensity) to the largest possible region. InitializationAHP() reads the seed (input 1) and mask (input 2) over their full extent, so a streaming-capable source feeding either would be under-buffered, causing out-of-bounds access or wrong results. Mirror the input-0 handling for both.
FastGrowCut caches the distance volume and, once m_bSegInitialized is set by the first Update(), every subsequent Update() takes the incremental path that reuses it. The itkSetMacro for DistancePenalty called Modified() but never invalidated that cache, so changing DistancePenalty after an initial Update() had no effect on the result. Replace the macro with a setter that, on an actual value change, calls Reset() (clearing m_bSegInitialized and the cached distance volume) before Modified(), forcing a full recomputation. Closes InsightSoftwareConsortium/ITKGrowCut#18
The test swapped in a median-filtered intensity image and called Update() again without Reset(). With m_bSegInitialized still true the filter takes the incremental path and reads the freshly-allocated, uninitialized output buffer, yielding non-deterministic labels. Call Reset() before the second Update(), as the public API documents.
hjmjohnson
force-pushed
the
ingest-GrowCut
branch
from
3edb822 to
c06b521
Compare
dzenanz
approved these changes
May 19, 2026
Member
|
/azp run ITK.macOS.Python |
Member
|
This algorithm is needlessly complicated by hand-rolled 3D iterators and manual handling of regions. It can therefore significantly benefit from refactoring to use standard ITK iterators and filtering pipeline mechanism. This can be left as an improvement to be done after ingestion. |
hjmjohnson
merged commit b8c281f
into
InsightSoftwareConsortium:main
19 of 20 checks passed
hjmjohnson
added a commit
to InsightSoftwareConsortium/ITKGrowCut
that referenced
this pull request
May 27, 2026
The GrowCut module has been ingested into ITK main under Modules/Segmentation/GrowCut via InsightSoftwareConsortium/ITK#6274. Delete the whitelisted module sources, preserve the original README, and promote the migration notice to README.md so the GitHub landing page reflects archived status.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ingest the ITKGrowCut remote module into
Modules/Segmentation/GrowCut(group: Segmentation). Source upstream:InsightSoftwareConsortium/ITKGrowCut. Tracking issue: #6160.Ingest stats
Utilities/Maintenance/RemoteModuleIngest/INGESTION_STRATEGY.md.Modules/Segmentation/GrowCut/: 26ee37dc4fa2External-data fixtures
7 fixtures whose upstream
.cidwere unreachable have been converted to.sha512content-links; the matching blobs are staged in ITKTestingData PR #56 (still draft). The remaining 8.cidlinks resolve from the existing ITKData store.This PR depends on ITKTestingData#56 merging before the converted fixtures resolve cleanly in CI; a
.sha512that resolves only after #56 is a known dependency, not a defect in this PR.Follow-on commits (on top of the unrelated-histories merge)
ee37dc4fa2BUG: Force seed/mask inputs to LargestPossibleRegion in FastGrowCut8785e3a4a9COMP: Prepend ITK Apache 2.0 header to FibHeap source files0f0e6bcf63ENH: Convert 7 unreachable GrowCut .cid files to .sha5122966e5b3ccDOC: Improve GrowCut module DESCRIPTION1076f4b7aeCOMP: Remove stale standalone-build branch from CMakeLists76d0993b59BUG: Make FibHeap::Print non-blocking and null-safee9eb36bd9cBUG: Free m_Heap and m_HeapNodes in FastGrowCut destructor8e7b52a6ecSTYLE: Remove dead code and unused dependencies in FastGrowCut43f5629d33BUG: Restore FastGrowCut::Reset() body so it actually resets stateCloses #6306