You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
CropForegroundd works very well for mammography images that contain only the breast and background. However, some mammograms also contain foreground text or markers (e.g., laterality labels, acquisition text, scanner annotations). Since these annotations are non-zero pixels, they are detected as foreground by select_fn, causing the computed bounding box to include them and resulting in a larger crop than desired.
Thanks for the report. This is expected behavior - CropForeground computes the bounding box over all foreground pixels selected by select_fn, so disconnected text/markers (laterality labels, scanner annotations) get pulled into the box alongside the breast.
A clean, backward-compatible fix would be an opt-in flag to keep only the largest connected component of the foreground mask before computing the bbox - e.g. CropForeground(..., select_largest_component=False) (default False preserves current behavior). MONAI already has get_largest_connected_component_mask in monai/transforms/utils.py, so compute_bounding_box could apply it to the selected mask when the flag is set, before calling generate_spatial_bounding_box.
This would handle the mammography case (keep the breast, drop separate text regions) without breaking existing users. Does this direction sound right to the maintainers? If so, I'd be happy to open a PR (with tests) - could this be labeled/confirmed as an enhancement?
I looked into why this happens: CropForegroundd/CropForeground compute the bounding box via generate_spatial_bounding_box, which reduces select_fn(data) to a single boolean mask with .any(0) and then takes the per-axis min/max of the True positions — i.e. it treats every connected blob of foreground as one, and takes the outer bounding box across all of them. A laterality label or scanner annotation is non-zero, so it's part of that same mask, and if it sits away from the breast tissue, the projection-based bbox stretches out to cover it too.
Proposed fix: add an opt-in keep_largest_component: bool = False parameter to generate_spatial_bounding_box (threaded through CropForeground / CropForegroundd). When set, it applies MONAI's own get_largest_connected_component_mask (already used by KeepLargestConnectedComponent, so it reuses the existing skimage.measure.label-based, optional-dependency-guarded connected-component logic rather than adding a new one) to the select_fn(data).any(0) mask before the per-axis projection, keeping only the largest connected blob. For a mammogram, that's the breast tissue; the isolated text/marker pixels form separate, smaller components and get dropped before the bbox is computed.
Why opt-in rather than the default: select_fn legitimately produces multiple disjoint foreground regions for other use cases (e.g. several structures/organs in one volume), so keeping only the largest component can't be the default behavior without silently changing existing pipelines. allow_smaller's own default changed in v1.5.0 for a similar backward-compat reason, so I'd rather not repeat that with this one.
Sketch of the change (happy to adjust the flag name/placement if you'd prefer it live elsewhere, e.g. as a select_fn wrapper instead of a new bbox-generation parameter):
This does mean an extra skimage dependency at call time, but only when the flag is explicitly enabled — same optional-import pattern get_largest_connected_component_mask already uses, so CropForeground stays usable without skimage installed by default.
Would this direction work for you? If so, I'll open a PR with this plus regression tests (including a synthetic case shaped like this one: one large blob + one small disconnected blob outside it, asserting the box excludes the small one only when the flag is set).
Ah, I missed your comment from July before writing mine — sorry for the near-duplicate. Good to see we independently landed on the same approach (get_largest_connected_component_mask + an opt-in flag before generate_spatial_bounding_box), that's a reasonable signal it's the right direction.
Since this stalled for a couple of months without a maintainer confirmation: @MDSALMANSHAMS, have you already started on a PR? If so I'm happy to step back, or review/help. If not, I can open one with the flag + tests (mammography-shaped synthetic case: one large blob + a smaller disconnected blob, asserting the box excludes the small one only when the flag is set) — just don't want us to duplicate work a second time.
Thanks @agourakis82 — good to see we independently landed on the same approach. I'm picking this up: opening a PR today with the opt-in flag (reusing get_largest_connected_component_mask, same shape as your sketch above) plus regression tests, including the mammography-shaped case (one large blob + a smaller disconnected blob, box excludes the small one only when the flag is set). Will link the PR here shortly and tag you for review.
PR is up: #9126 — adds the opt-in keep_largest_component flag to generate_spatial_bounding_box, CropForeground, and CropForegroundd, plus regression tests (a large-blob + disconnected-pixel case, asserting the box excludes the isolated pixel only when the flag is set). @agourakis82 review/feedback welcome, especially on the flag name/placement if you'd prefer it live elsewhere.
Activity
MDSALMANSHAMS commentedon Jul 10, 2026
Thanks for the report. This is expected behavior -
CropForegroundcomputes the bounding box over all foreground pixels selected byselect_fn, so disconnected text/markers (laterality labels, scanner annotations) get pulled into the box alongside the breast.A clean, backward-compatible fix would be an opt-in flag to keep only the largest connected component of the foreground mask before computing the bbox - e.g.
CropForeground(..., select_largest_component=False)(defaultFalsepreserves current behavior). MONAI already hasget_largest_connected_component_maskinmonai/transforms/utils.py, socompute_bounding_boxcould apply it to the selected mask when the flag is set, before callinggenerate_spatial_bounding_box.This would handle the mammography case (keep the breast, drop separate text regions) without breaking existing users. Does this direction sound right to the maintainers? If so, I'd be happy to open a PR (with tests) - could this be labeled/confirmed as an enhancement?
agourakis82 commentedon Sep 22, 2026
I looked into why this happens:
CropForegroundd/CropForegroundcompute the bounding box viagenerate_spatial_bounding_box, which reducesselect_fn(data)to a single boolean mask with.any(0)and then takes the per-axis min/max of theTruepositions — i.e. it treats every connected blob of foreground as one, and takes the outer bounding box across all of them. A laterality label or scanner annotation is non-zero, so it's part of that same mask, and if it sits away from the breast tissue, the projection-based bbox stretches out to cover it too.Proposed fix: add an opt-in
keep_largest_component: bool = Falseparameter togenerate_spatial_bounding_box(threaded throughCropForeground/CropForegroundd). When set, it applies MONAI's ownget_largest_connected_component_mask(already used byKeepLargestConnectedComponent, so it reuses the existingskimage.measure.label-based, optional-dependency-guarded connected-component logic rather than adding a new one) to theselect_fn(data).any(0)mask before the per-axis projection, keeping only the largest connected blob. For a mammogram, that's the breast tissue; the isolated text/marker pixels form separate, smaller components and get dropped before the bbox is computed.Why opt-in rather than the default:
select_fnlegitimately produces multiple disjoint foreground regions for other use cases (e.g. several structures/organs in one volume), so keeping only the largest component can't be the default behavior without silently changing existing pipelines.allow_smaller's own default changed in v1.5.0 for a similar backward-compat reason, so I'd rather not repeat that with this one.Sketch of the change (happy to adjust the flag name/placement if you'd prefer it live elsewhere, e.g. as a
select_fnwrapper instead of a new bbox-generation parameter):This does mean an extra
skimagedependency at call time, but only when the flag is explicitly enabled — same optional-import patternget_largest_connected_component_maskalready uses, soCropForegroundstays usable withoutskimageinstalled by default.Would this direction work for you? If so, I'll open a PR with this plus regression tests (including a synthetic case shaped like this one: one large blob + one small disconnected blob outside it, asserting the box excludes the small one only when the flag is set).
agourakis82 commentedon Sep 22, 2026
Ah, I missed your comment from July before writing mine — sorry for the near-duplicate. Good to see we independently landed on the same approach (
get_largest_connected_component_mask+ an opt-in flag beforegenerate_spatial_bounding_box), that's a reasonable signal it's the right direction.Since this stalled for a couple of months without a maintainer confirmation: @MDSALMANSHAMS, have you already started on a PR? If so I'm happy to step back, or review/help. If not, I can open one with the flag + tests (mammography-shaped synthetic case: one large blob + a smaller disconnected blob, asserting the box excludes the small one only when the flag is set) — just don't want us to duplicate work a second time.
MDSALMANSHAMS commentedon Sep 22, 2026
Thanks @agourakis82 — good to see we independently landed on the same approach. I'm picking this up: opening a PR today with the opt-in flag (reusing
get_largest_connected_component_mask, same shape as your sketch above) plus regression tests, including the mammography-shaped case (one large blob + a smaller disconnected blob, box excludes the small one only when the flag is set). Will link the PR here shortly and tag you for review.MDSALMANSHAMS commentedon Sep 22, 2026
PR is up: #9126 — adds the opt-in
keep_largest_componentflag togenerate_spatial_bounding_box,CropForeground, andCropForegroundd, plus regression tests (a large-blob + disconnected-pixel case, asserting the box excludes the isolated pixel only when the flag is set). @agourakis82 review/feedback welcome, especially on the flag name/placement if you'd prefer it live elsewhere.