BUG: Sort the full pixel buffer in AttributeMorphologyBase#6581
Merged
hjmjohnson merged 1 commit intoJul 10, 2026
Merged
Conversation
github-actions
Bot
added
type:Bug
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}
AttributeMorphologyBaseImageFilter sorted the pixel buffer with an exclusive end iterator that was off by one: std::stable_sort(&(m_SortPixels[0]), &(m_SortPixels[buffsize - 1]), ...) This covers only the first buffsize-1 entries, so the last slot keeps its identity value and the image's last raster-order pixel is exempt from the value sort and is always swept last, regardless of its gray value. When that last pixel is an isolated extremum forming its own undersized component, its neighbors are processed first; a neighbor's union-find lookup sees the unvisited node's sentinel attribute (-1), which satisfies the criterion and merges wrongly, and the last pixel's own MakeSet then overwrites the premature parent link and survives as a permanent root. The undersized component at the last raster pixel thus survives an attribute criterion (Lambda) that should have removed it. Use .get() pointer arithmetic for the range so the sort covers all buffsize entries: std::stable_sort(m_SortPixels.get(), m_SortPixels.get() + buffsize, ...) Add a regression test that runs an area opening on a single-row image with a single-pixel peak at the last raster position; the fix removes the peak while the unfixed code left it in place. A mid-row control case guards the semantics. Addresses item B19 of InsightSoftwareConsortium#6575.
hjmjohnson
force-pushed
the
bug-attribute-morphology-sort-range
branch
from
4c7556e to
71b4ad9
Compare
hjmjohnson
approved these changes
Jul 10, 2026
7 tasks
Member
|
/azp run ITK.Linux.Python |
physwkim
added a commit
to physwkim/sitk-rs
that referenced
this pull request
Jul 12, 2026
itkAttributeMorphologyBaseImageFilter.hxx:123 sorted with an exclusive end iterator one slot short of the buffer, so the last raster-order pixel was exempt from the value sort and always swept last regardless of its gray value. Combined with FindRoot not distinguishing "never visited" (-1) from "genuine root" (-2), an undersized component at that flat index could survive a Lambda it should have failed. Matches fix PR InsightSoftwareConsortium/ITK#6581 (item B19 of #6575) exactly: sort_pixels now sorts the whole array instead of [..total - 1]. Once every pixel is sorted, the sweep's own invariant guarantees a neighbor is only looked up via find_root after it has already been swept, so find_root's INACTIVE/ACTIVE conflation becomes dead code rather than a live defect -- no change to find_root itself was needed, matching the upstream fix's scope. Renamed and hand-recomputed the pinning test: an isolated area-1 peak at the last raster pixel ([0, 0, 5], Lambda = 2.0) now correctly collapses to [0, 0, 0], same as the identical peak swept mid-array in the sibling test, traced through the corrected union-find sweep by hand and cross-checked against the upstream fix PR's own regression test (5-pixel row, peak at index 4).
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
2 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.
Addresses item B19 of #6575.
AttributeMorphologyBaseImageFilter(the engine behindAreaOpeningImageFilter/AreaClosingImageFilter) sorts the pixel indices by gray value before its union-find sweep:The exclusive end iterator is off by one, so the sort covers only the first
buffsize - 1elements and the slot for the image's last raster-order pixel keeps its identity value — that pixel is exempt from sorting and is always swept last, regardless of its gray value. When the last raster pixel is a local extremum forming its own small component, its neighbors are processed first; a neighbor's union step sees the still-unvisited node's sentinel attribute (−1), which satisfies the attribute criterion and merges prematurely, and once the last pixel's ownMakeSetruns it overwrites the premature parent link and survives as a permanent root. Net effect: an undersized component at the last raster position survives aLambdathat should remove it.The fix sorts the full range:
std::stable_sort(m_SortPixels.get(), m_SortPixels.get() + buffsize, m_CompareOffset);.The new regression test puts an isolated single-pixel peak at the last raster position of a 5×1 row (
[0, 0, 0, 0, 5]) and runsAreaOpeningImageFilterwithLambda = 2: the area-1 peak must be removed. A control case with the identical peak mid-row ([0, 0, 5, 0, 0]) is removed by both the fixed and unfixed code, pinning the semantics. The union-find trace of the unfixed failure is in the test's header comment.Verified locally (Linux, GCC 13, Release): the new test fails on unmodified
main(output[0, 0, 0, 0, 5]— the last-pixel peak survives) and passes with the fix; the full ITKReview suite plus the ITKImageStatistics, ITKLabelVoting, ITKDisplacementField, and ITKRegionGrowing suites pass with the fix applied.PR Checklist
AI assistance