Add ComputeIndex() member function to ImageConstIterator#5787
Merged
hjmjohnson merged 2 commits intoInsightSoftwareConsortium:mainfrom Feb 13, 2026
Merged
Conversation
dzenanz
reviewed
Feb 10, 2026
Member
dzenanz
left a comment
dzenanz
left a comment
There was a problem hiding this comment.
I have reservations about the main change.
github-actions
bot
added
area:Filtering
N-Dekker
commented
Feb 11, 2026
Modules/Filtering/DisplacementField/include/itkInvertDisplacementFieldImageFilter.hxx
Show resolved
Hide resolved
N-Dekker
commented
Feb 11, 2026
Modules/Filtering/DisplacementField/include/itkIterativeInverseDisplacementFieldImageFilter.hxx
Show resolved
Hide resolved
N-Dekker
commented
Feb 11, 2026
Modules/Filtering/ImageFeature/include/itkHoughTransform2DLinesImageFilter.hxx
Show resolved
Hide resolved
N-Dekker
commented
Feb 11, 2026
dzenanz
reviewed
Feb 11, 2026
Member
dzenanz
left a comment
dzenanz
left a comment
There was a problem hiding this comment.
You should implement your own performance-enhancing suggestions 😄, preferably as a separate PR (on which this one should then be rebased).
Member
|
Possibly also take the first 2 non-contentious commits to the separate PR, which should then be merged quickly. |
Provides an alternative to `ImageConstIterator::GetIndex()`, making it more clear that potentially expensive computation is involved.
Replaced function calls of the form `iterator.GetIndex()` with `iterator.ComputeIndex()`, for iterators derived from `ImageConstIterator`. For those iterators, it may take a relatively expensive computation to get the index. Using `ComputeIndex()` instead of the equivalent `GetIndex()` aims to raise awareness of the potential performance cost of those function calls.
N-Dekker
force-pushed
the
ImageConstIterator-ComputeIndex
branch
from
a4c0b2e to
cc5b301
Compare
dzenanz
approved these changes
Feb 12, 2026
Member
dzenanz
left a comment
dzenanz
left a comment
There was a problem hiding this comment.
Good even as is, but future legacy remove of GetIndex could totally be a third commit here.
Contributor
Author
Thanks @dzenanz. I prefer to do the future legacy remove in a separate PR, which would then also replace the "computative" |
There was a problem hiding this comment.
Pull request overview
This PR adds a clearer alternative to ImageConstIterator::GetIndex() by introducing ComputeIndex(), emphasizing that computing the index from the iterator offset may be relatively expensive. It also updates a broad set of call sites across ITK modules to use the new name where appropriate.
Changes:
- Add
ImageConstIterator::ComputeIndex()and makeGetIndex()delegate to it. - Update many filters/iterators to call
ComputeIndex()instead ofGetIndex(). - Adjust iterator implementations that depended on index computation to use the new API.
Reviewed changes
Copilot reviewed 32 out of 32 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| Modules/Core/Common/include/itkImageConstIterator.h | Introduces ComputeIndex() and updates GetIndex() to call it, with updated docs. |
| Modules/Core/Common/include/itkImageScanlineConstIterator.h | Uses ComputeIndex() in a cast constructor to make index computation cost explicit. |
| Modules/Core/Common/include/itkImageReverseConstIterator.h | Uses ComputeIndex() when constructing from an ImageConstIterator. |
| Modules/Segmentation/Watersheds/include/itkWatershedSegmenter.hxx | Switches index retrieval to ComputeIndex() in watershed routines. |
| Modules/Segmentation/Watersheds/include/itkTobogganImageFilter.hxx | Switches to ComputeIndex() for current position tracking. |
| Modules/Segmentation/SuperPixel/include/itkSLICImageFilter.hxx | Switches multiple iterator index reads to ComputeIndex(). |
| Modules/Segmentation/LevelSets/include/itkLevelSetNeighborhoodExtractor.hxx | Uses ComputeIndex() before distance calculations. |
| Modules/Segmentation/ConnectedComponents/include/itkHardConnectedComponentImageFilter.hxx | Uses ComputeIndex() for neighborhood index manipulation. |
| Modules/Segmentation/ConnectedComponents/include/itkConnectedComponentImageFilter.hxx | Uses ComputeIndex() at run starts in scanline iteration. |
| Modules/Numerics/FEM/include/itkFEMSolver.hxx | Uses ComputeIndex() for transforming index to physical points. |
| Modules/Numerics/FEM/include/itkFEMScatteredDataPointSetToImageFilter.hxx | Uses ComputeIndex() for index-to-physical transforms. |
| Modules/Numerics/FEM/include/itkFEMRobustSolver.hxx | Uses ComputeIndex() for index-to-physical transforms in initialization. |
| Modules/Filtering/SpatialFunction/include/itkSpatialFunctionImageEvaluatorFilter.hxx | Uses ComputeIndex() before evaluating the spatial function. |
| Modules/Filtering/Smoothing/include/itkBinomialBlurImageFilter.hxx | Uses ComputeIndex() when determining output pixel indices. |
| Modules/Filtering/Path/include/itkContourExtractor2DImageFilter.hxx | Uses ComputeIndex() while building label bounding boxes. |
| Modules/Filtering/MathematicalMorphology/include/itkValuedRegionalExtremaImageFilter.hxx | Uses ComputeIndex() when syncing neighborhood iterators to the output iterator. |
| Modules/Filtering/LabelMap/include/itkBinaryImageToLabelMapFilter.hxx | Uses ComputeIndex() at run starts in scanline iteration. |
| Modules/Filtering/ImageStatistics/include/itkAccumulateImageFilter.hxx | Uses ComputeIndex() while accumulating along a dimension. |
| Modules/Filtering/ImageSources/include/itkGaussianImageSource.hxx | Uses ComputeIndex() to evaluate the source function at each pixel. |
| Modules/Filtering/ImageLabel/include/itkLabelContourImageFilter.hxx | Uses ComputeIndex() for line ID computation and scanline integration. |
| Modules/Filtering/ImageLabel/include/itkBinaryContourImageFilter.hxx | Uses ComputeIndex() for line IDs and run start indices. |
| Modules/Filtering/ImageGrid/include/itkResampleImageFilter.hxx | Uses ComputeIndex() while deriving scanline geometry for resampling. |
| Modules/Filtering/ImageGrid/include/itkMirrorPadImageFilter.hxx | Uses ComputeIndex() when converting output indices to input indices. |
| Modules/Filtering/ImageGrid/include/itkFlipImageFilter.hxx | Uses ComputeIndex() to determine output scanline start indices. |
| Modules/Filtering/ImageGrid/include/itkExpandImageFilter.hxx | Uses ComputeIndex() to determine output indices per scanline. |
| Modules/Filtering/ImageGrid/include/itkBinShrinkImageFilter.hxx | Uses ComputeIndex() for mapping output scanlines back to input. |
| Modules/Filtering/ImageGradient/include/itkDifferenceOfGaussiansGradientImageFilter.hxx | Uses ComputeIndex() for output pixel index determination. |
| Modules/Filtering/ImageFeature/include/itkHoughTransform2DLinesImageFilter.hxx | Uses ComputeIndex() while interpreting accumulator space indices. |
| Modules/Filtering/ImageFeature/include/itkCannyEdgeDetectionImageFilter.hxx | Uses ComputeIndex() to seed and follow edges. |
| Modules/Filtering/DisplacementField/include/itkTransformToDisplacementFieldFilter.hxx | Uses ComputeIndex() while generating displacement fields (including scanline logic). |
| Modules/Filtering/DisplacementField/include/itkIterativeInverseDisplacementFieldImageFilter.hxx | Uses ComputeIndex() for output index-to-point transforms. |
| Modules/Filtering/DisplacementField/include/itkInvertDisplacementFieldImageFilter.hxx | Uses ComputeIndex() for boundary condition enforcement indexing. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Modules/Filtering/ImageFeature/include/itkHoughTransform2DLinesImageFilter.hxx
Show resolved
Hide resolved
Modules/Filtering/DisplacementField/include/itkTransformToDisplacementFieldFilter.hxx
Show resolved
Hide resolved
Modules/Filtering/ImageFeature/include/itkCannyEdgeDetectionImageFilter.hxx
Show resolved
Hide resolved
Modules/Filtering/ImageFeature/include/itkHoughTransform2DLinesImageFilter.hxx
Show resolved
Hide resolved
Contributor
Author
|
Sorry, I'm not putting all possible code improvements in one big PR. Adjustment of ComputeIndex() calls in existing filters must be done very carefully, to avoid breaking code. In some cases, the Iterator variable should be replaced with the corresponding IteratorWithIndex. In other cases, the computed index should be cached into a local variable. ComputeIndex() should also be used in the tests. And |
hjmjohnson
approved these changes
Feb 13, 2026
N-Dekker
added a commit
to N-Dekker/ITK
that referenced
this pull request
Feb 13, 2026
Replaced function calls of the form `iterator.GetIndex()` with `iterator.ComputeIndex()`, for iterators derived from `ImageConstIterator`, in tests. Follow-up to pull request InsightSoftwareConsortium#5787 commit cc5b301 "STYLE: Replace GetIndex() calls on iterators with ComputeIndex()"
N-Dekker
added a commit
to N-Dekker/ITK
that referenced
this pull request
Feb 13, 2026
…ilter Replaced `outputIt.GetIndex()` with `outputIt.ComputeIndex()`. Follow-up to pull request InsightSoftwareConsortium#5787 commit cc5b301 "STYLE: Replace GetIndex() calls on iterators with ComputeIndex()"
N-Dekker
added a commit
to N-Dekker/ITK
that referenced
this pull request
Feb 13, 2026
Replaced function calls of the form `iterator.GetIndex()` with `iterator.ComputeIndex()`, for iterators derived from `ImageConstIterator`, in tests. Follow-up to pull request InsightSoftwareConsortium#5787 commit cc5b301 "STYLE: Replace GetIndex() calls on iterators with ComputeIndex()"
N-Dekker
added a commit
to N-Dekker/ITK
that referenced
this pull request
Feb 13, 2026
Replaced function calls of the form `iterator.GetIndex()` with `iterator.ComputeIndex()`, for iterators derived from `ImageConstIterator`, in Modules/Registration/Common/test/RegistrationITKv3 Follow-up to pull request InsightSoftwareConsortium#5787 commit cc5b301 "STYLE: Replace GetIndex() calls on iterators with ComputeIndex()"
N-Dekker
added a commit
to N-Dekker/ITK
that referenced
this pull request
Feb 13, 2026
Replaced function calls of the form `iterator.GetIndex()` with `iterator.ComputeIndex()`, for iterators derived from `ImageConstIterator`, in Examples. Follow-up to pull request InsightSoftwareConsortium#5787 commit cc5b301 "STYLE: Replace GetIndex() calls on iterators with ComputeIndex()"
N-Dekker
added a commit
to N-Dekker/ITK
that referenced
this pull request
Feb 13, 2026
Indicated that `ImageConstIterator::GetIndex()` will be removed in the future. The name "ComputeIndex" appears much clearer, as it indicates that it does a potentially expensive computation, in order to retrieve the index. Follow-up to pull request InsightSoftwareConsortium#5787 commit 3e5a75a "ENH: Add `ComputeIndex()` member function to ImageConstIterator"
N-Dekker
added a commit
to N-Dekker/ITK
that referenced
this pull request
Feb 13, 2026
Replaced `outputIt.GetIndex()` with `outputIt.ComputeIndex()`, in `PolylineMaskImageFilter::GenerateData()`. Follow-up to pull request InsightSoftwareConsortium#5787 commit cc5b301 "STYLE: Replace GetIndex() calls on iterators with ComputeIndex()"
N-Dekker
added a commit
to N-Dekker/ITK
that referenced
this pull request
Feb 13, 2026
Replaced function calls of the form `iterator.GetIndex()` with `iterator.ComputeIndex()`, for iterators derived from `ImageConstIterator`, in tests. Follow-up to pull request InsightSoftwareConsortium#5787 commit cc5b301 "STYLE: Replace GetIndex() calls on iterators with ComputeIndex()"
N-Dekker
added a commit
to N-Dekker/ITK
that referenced
this pull request
Feb 13, 2026
Replaced function calls of the form `iterator.GetIndex()` with `iterator.ComputeIndex()`, for iterators derived from `ImageConstIterator`, in Modules/Registration/Common/test/RegistrationITKv3 Follow-up to pull request InsightSoftwareConsortium#5787 commit cc5b301 "STYLE: Replace GetIndex() calls on iterators with ComputeIndex()"
N-Dekker
added a commit
to N-Dekker/ITK
that referenced
this pull request
Feb 13, 2026
Replaced function calls of the form `iterator.GetIndex()` with `iterator.ComputeIndex()`, for iterators derived from `ImageConstIterator`, in Examples. Follow-up to pull request InsightSoftwareConsortium#5787 commit cc5b301 "STYLE: Replace GetIndex() calls on iterators with ComputeIndex()"
N-Dekker
added a commit
to N-Dekker/ITK
that referenced
this pull request
Feb 13, 2026
Indicated that `ImageConstIterator::GetIndex()` will be removed in the future. The name "ComputeIndex" appears much clearer, as it indicates that it does a potentially expensive computation, in order to retrieve the index. Follow-up to pull request InsightSoftwareConsortium#5787 commit 3e5a75a "ENH: Add `ComputeIndex()` member function to ImageConstIterator"
N-Dekker
added a commit
to N-Dekker/ITK
that referenced
this pull request
Feb 13, 2026
Indicated that `ImageConstIterator::GetIndex()` will be removed in the future. The name "ComputeIndex" appears much clearer, as it indicates that it does a potentially expensive computation, in order to retrieve the index. Follow-up to pull request InsightSoftwareConsortium#5787 commit 3e5a75a "ENH: Add `ComputeIndex()` member function to ImageConstIterator"
N-Dekker
added a commit
to N-Dekker/ITK
that referenced
this pull request
Feb 14, 2026
Replaced `outputIt.GetIndex()` with `outputIt.ComputeIndex()`, in `PolylineMaskImageFilter::GenerateData()`. Follow-up to pull request InsightSoftwareConsortium#5787 commit cc5b301 "STYLE: Replace GetIndex() calls on iterators with ComputeIndex()"
N-Dekker
added a commit
to N-Dekker/ITK
that referenced
this pull request
Feb 14, 2026
Replaced function calls of the form `iterator.GetIndex()` with `iterator.ComputeIndex()`, for iterators derived from `ImageConstIterator`, in tests. Follow-up to pull request InsightSoftwareConsortium#5787 commit cc5b301 "STYLE: Replace GetIndex() calls on iterators with ComputeIndex()"
N-Dekker
added a commit
to N-Dekker/ITK
that referenced
this pull request
Feb 14, 2026
Replaced function calls of the form `iterator.GetIndex()` with `iterator.ComputeIndex()`, for iterators derived from `ImageConstIterator`, in Modules/Registration/Common/test/RegistrationITKv3 Follow-up to pull request InsightSoftwareConsortium#5787 commit cc5b301 "STYLE: Replace GetIndex() calls on iterators with ComputeIndex()"
N-Dekker
added a commit
to N-Dekker/ITK
that referenced
this pull request
Feb 14, 2026
Replaced function calls of the form `iterator.GetIndex()` with `iterator.ComputeIndex()`, for iterators derived from `ImageConstIterator`, in Examples. Follow-up to pull request InsightSoftwareConsortium#5787 commit cc5b301 "STYLE: Replace GetIndex() calls on iterators with ComputeIndex()"
N-Dekker
added a commit
to N-Dekker/ITK
that referenced
this pull request
Feb 14, 2026
Indicated that `ImageConstIterator::GetIndex()` will be removed in the future. The name "ComputeIndex" appears much clearer, as it indicates that it does a potentially expensive computation, in order to retrieve the index. Follow-up to pull request InsightSoftwareConsortium#5787 commit 3e5a75a "ENH: Add `ComputeIndex()` member function to ImageConstIterator"
N-Dekker
added a commit
to N-Dekker/ITK
that referenced
this pull request
Feb 15, 2026
Replaced `outputIt.GetIndex()` with `outputIt.ComputeIndex()`, in `PolylineMaskImageFilter::GenerateData()`. Follow-up to pull request InsightSoftwareConsortium#5787 commit cc5b301 "STYLE: Replace GetIndex() calls on iterators with ComputeIndex()"
N-Dekker
added a commit
to N-Dekker/ITK
that referenced
this pull request
Feb 15, 2026
Replaced function calls of the form `iterator.GetIndex()` with `iterator.ComputeIndex()`, for iterators derived from `ImageConstIterator`, in tests. Follow-up to pull request InsightSoftwareConsortium#5787 commit cc5b301 "STYLE: Replace GetIndex() calls on iterators with ComputeIndex()"
N-Dekker
added a commit
to N-Dekker/ITK
that referenced
this pull request
Feb 15, 2026
Replaced function calls of the form `iterator.GetIndex()` with `iterator.ComputeIndex()`, for iterators derived from `ImageConstIterator`, in Modules/Registration/Common/test/RegistrationITKv3 Follow-up to pull request InsightSoftwareConsortium#5787 commit cc5b301 "STYLE: Replace GetIndex() calls on iterators with ComputeIndex()"
N-Dekker
added a commit
to N-Dekker/ITK
that referenced
this pull request
Feb 15, 2026
Replaced function calls of the form `iterator.GetIndex()` with `iterator.ComputeIndex()`, for iterators derived from `ImageConstIterator`, in Examples. Follow-up to pull request InsightSoftwareConsortium#5787 commit cc5b301 "STYLE: Replace GetIndex() calls on iterators with ComputeIndex()"
N-Dekker
added a commit
to N-Dekker/ITK
that referenced
this pull request
Feb 15, 2026
Indicated that `ImageConstIterator::GetIndex()` will be removed in the future. Adding a note to the migration guide. The name "ComputeIndex" appears much clearer, as it indicates that it does a potentially expensive computation, in order to retrieve the index. Follow-up to pull request InsightSoftwareConsortium#5787 commit 3e5a75a "ENH: Add `ComputeIndex()` member function to ImageConstIterator"
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
4 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.
Provides an alternative to
ImageConstIterator::GetIndex(), making it more clear that potentially expensive computation is involved.