ENH: Ingest ITKGrowCut into Modules/Segmentation/GrowCut#6274
Open
hjmjohnson wants to merge 44 commits into
Open
ENH: Ingest ITKGrowCut into Modules/Segmentation/GrowCut#6274hjmjohnson wants to merge 44 commits into
hjmjohnson wants to merge 44 commits into
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;
| ^~~~~~~~~~~~~~~~
github-actions
Bot
added
type:Infrastructure
Member
Author
|
@greptileai review this draft before I make it official |
1 similar comment
Member
Author
|
@greptileai review this draft before I make it official |
This comment was marked as resolved.
This comment was marked as resolved.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
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.
Member
Author
|
/azp run |
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.
dzenanz
reviewed
May 14, 2026
Member
dzenanz
left a comment
dzenanz
left a comment
There was a problem hiding this comment.
If memory serves me well, ExtractITKImageROI is used.
dzenanz
reviewed
May 14, 2026
50 tasks
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
Member
Author
|
/azp run |
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.
Member
Author
|
/azp run |
Comment on lines
+397
to
+403
| Superclass::GenerateInputRequestedRegion(); | ||
| if (this->GetInput()) | ||
| { | ||
| typename InputImageType::Pointer input = const_cast<TInputImage *>(this->GetInput()); | ||
| input->SetRequestedRegionToLargestPossibleRegion(); | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
ImageToImageFilter::GenerateInputRequestedRegion() only propagates the output region to input 0 (the intensity image); inputs 1 and 2 are left with whatever region the pipeline previously assigned. If a seed or mask image is connected to a streaming-capable source (reader, upstream filter), only the sub-region that was requested gets buffered, and InitializationAHP() will then access seedLabelVolumePtr[index] / maskLabelVolumePtr[index] beyond the buffered extent, producing wrong results or a crash.
Suggested change
| Superclass::GenerateInputRequestedRegion(); | |
| if (this->GetInput()) | |
| { | |
| typename InputImageType::Pointer input = const_cast<TInputImage *>(this->GetInput()); | |
| input->SetRequestedRegionToLargestPossibleRegion(); | |
| } | |
| } | |
| Superclass::GenerateInputRequestedRegion(); | |
| if (this->GetInput()) | |
| { | |
| typename InputImageType::Pointer input = const_cast<TInputImage *>(this->GetInput()); | |
| input->SetRequestedRegionToLargestPossibleRegion(); | |
| } | |
| if (this->GetSeedImage()) | |
| { | |
| auto seedImage = const_cast<LabelImageType *>(this->GetSeedImage()); | |
| seedImage->SetRequestedRegionToLargestPossibleRegion(); | |
| } | |
| if (this->GetMaskImage()) | |
| { | |
| auto maskImage = const_cast<MaskImageType *>(this->GetMaskImage()); | |
| maskImage->SetRequestedRegionToLargestPossibleRegion(); | |
| } | |
| } |
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/: 268b691c57ad4cd68e2dd4d5260b84bb1ed6cea001External-data fixtures
CID / .sha512 content-links present in this ingest:
If any fixtures resolve via ITKTestingData, that repo must contain the matching content-link before this PR can merge cleanly.
Follow-on commits (on top of the unrelated-histories merge)