feat(imgproc): add find_contours with hierarchy modes#797
Open
mathurojus wants to merge 2 commits intokornia:mainfrom
Open
feat(imgproc): add find_contours with hierarchy modes#797mathurojus wants to merge 2 commits intokornia:mainfrom
mathurojus wants to merge 2 commits intokornia:mainfrom
Conversation
Contributor
ⓘ You are approaching your monthly quota for Qodo. Upgrade your plan Review Summary by QodoAdd find_contours with OpenCV-compatible hierarchy modes
WalkthroughsDescription• Implement find_contours function with OpenCV-compatible retrieval modes - External, List, CComp, Tree modes for different hierarchy extraction strategies - Contour approximation modes (None, Simple) for point compression • Add comprehensive contour tracing and component labeling algorithms - 8-connected component labeling with flood-fill - Boundary tracing using directional search - Hierarchy construction supporting nested contours • Include backward-compatible alias find_countours for misspelled API • Add four test cases covering rectangles, holes, nesting, and approximation modes Diagramflowchart LR
BinaryImage["Binary Image<br/>0=background, non-zero=foreground"]
LabelFG["Label Foreground<br/>Components"]
TraceBoundary["Trace Component<br/>Boundaries"]
Approximate["Approximate Contour<br/>Points"]
LabelBG["Label Background<br/>Components"]
BuildHierarchy["Build Hierarchy<br/>Based on Mode"]
Result["FindContoursResult<br/>contours + hierarchy"]
BinaryImage --> LabelFG
BinaryImage --> LabelBG
LabelFG --> TraceBoundary
LabelBG --> TraceBoundary
TraceBoundary --> Approximate
Approximate --> BuildHierarchy
BuildHierarchy --> Result
File Changes1. crates/kornia-imgproc/src/contours.rs
|
Contributor
Code Review by Qodo
1. README missing contours module
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+15
to
+16
| /// contour extraction module. | ||
| pub mod contours; |
Contributor
There was a problem hiding this comment.
1. Readme missing contours module 📘 Rule violation ✧ Quality
A new public contours module is exported, but the crate README’s module list is not updated to mention it. This makes the new API harder to discover and violates the requirement to update user-facing docs for public API changes.
Agent Prompt
## Issue description
A new public module `kornia_imgproc::contours` was added, but the crate README does not list or describe it, reducing discoverability.
## Issue Context
The public API surface was extended by exporting `pub mod contours;`. The README has a `Modules` section enumerating available modules, but it currently omits `contours`.
## Fix Focus Areas
- crates/kornia-imgproc/src/lib.rs[15-16]
- crates/kornia-imgproc/README.md[83-104]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| let mut prev = (start_x - 1, start_y); | ||
| let start_prev = prev; | ||
| let mut safety = 0usize; | ||
| let safety_limit = width * height * 16; |
Contributor
There was a problem hiding this comment.
2. Magic number 16 in tracing 📘 Rule violation ⛯ Reliability
The boundary tracing safety limit uses a hard-coded multiplier 16, which is an algorithm parameter and is not self-documenting. This makes the algorithm harder to tune and reason about.
Agent Prompt
## Issue description
The contour tracing loop uses a hard-coded safety multiplier `16` (`width * height * 16`) to prevent infinite loops. This is an algorithm parameter and should be a named constant to clarify intent and allow future tuning.
## Issue Context
The safety limit affects algorithm termination behavior and should be self-documenting.
## Fix Focus Areas
- crates/kornia-imgproc/src/contours.rs[354-365]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+114
to
+120
| let fg_mask: Vec<bool> = src.as_slice().iter().map(|&v| v != 0).collect(); | ||
| let fg_labels = label_components(&fg_mask, width, height); | ||
| let fg_count = fg_labels | ||
| .iter() | ||
| .copied() | ||
| .max() | ||
| .map_or(0usize, |m| (m as usize) + 1); |
Contributor
There was a problem hiding this comment.
3. Panic on empty labels 🐞 Bug ⛯ Reliability
fg_count/bg_count are computed as max_label + 1 without handling the all--1 case, which overflows in debug/test builds and panics for images with no foreground (or no background). This breaks expected behavior where such inputs should return zero contours.
Agent Prompt
## Issue description
`find_contours` computes `fg_count`/`bg_count` as `(max_label as usize) + 1`. When the image has no components, labels remain `-1` and `max()` returns `-1`; casting to `usize` and adding `1` overflows in debug builds, causing a panic.
## Issue Context
`label_components` initializes all labels to `-1` and only assigns non-negative labels when it finds a `true` pixel.
## Fix Focus Areas
- crates/kornia-imgproc/src/contours.rs[114-120]
- crates/kornia-imgproc/src/contours.rs[151-156]
- crates/kornia-imgproc/src/contours.rs[272-304]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+212
to
+233
| RetrievalMode::Tree => { | ||
| let mut parent = vec![None; entries.len()]; | ||
| for (idx, e) in entries.iter().enumerate() { | ||
| match e.kind { | ||
| ContourKind::Hole => { | ||
| parent[idx] = e.parent_hint; | ||
| } | ||
| ContourKind::Foreground => { | ||
| if let Some(bg_comp_id) = find_adjacent_background_component( | ||
| &fg_labels, | ||
| &bg_labels, | ||
| width, | ||
| height, | ||
| e.component_id, | ||
| ) { | ||
| let bg_id = bg_comp_id as usize; | ||
| if bg_id < bg_touches_border.len() | ||
| && !bg_touches_border[bg_id] | ||
| && bg_id < bg_comp_to_entry.len() | ||
| { | ||
| parent[idx] = bg_comp_to_entry[bg_id]; | ||
| } |
Contributor
There was a problem hiding this comment.
4. Tree parent selection wrong 🐞 Bug ✓ Correctness
In RetrievalMode::Tree, a foreground contour’s parent is chosen as the first adjacent non-border background component found by a raster scan, which is order-dependent and can pick an internal hole instead of the outside background. For border-touching foreground components with holes (border fully covered by foreground), this can even create a parent cycle (foreground ↔ hole).
Agent Prompt
## Issue description
`RetrievalMode::Tree` assigns a foreground contour’s parent to the first adjacent non-border background component returned by a raster scan. This is order-dependent and can select an internal hole background component, producing incorrect hierarchy and (for border-touching foregrounds with holes) can create a foreground↔hole parent cycle.
## Issue Context
- Holes set `parent_hint` to an adjacent foreground component.
- Foregrounds set `parent` to an adjacent non-border background component, but the adjacency search returns the first found label, not necessarily the outside component.
## Fix Focus Areas
- crates/kornia-imgproc/src/contours.rs[212-238]
- crates/kornia-imgproc/src/contours.rs[173-188]
- crates/kornia-imgproc/src/contours.rs[483-509]
- crates/kornia-imgproc/src/contours.rs[528-571]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Author
|
Thanks for the review! I've addressed all the issues:
Just pushed - please take a look when you get a chance! |
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
1 participant
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.
Implemented find_contours for kornia-imgproc with OpenCV-like behavior for binary images.
What’s included
Tests added
Reference issue event:
#169 (comment)
Closes #169