feat(contour tracing): Suzuki-Abe-style contour tracing - #219
Conversation
📝 WalkthroughWalkthroughReplaces in-place small-region merging with contour tracing after K-Means. kmeans now writes both pixel output and per-pixel Int32 labels; a new graph-based kmeans_clustering_graph produces region merging and contours. JS worker/hook updated to pass/receive Int32 labels; docs and tests adjusted; mergeSmallRegionsInPlace removed. Changes
Sequence Diagram(s)sequenceDiagram
participant React as React Component
participant Hook as useWasmWorker Hook
participant Worker as Web Worker
participant WASM as WASM Module
React->>Hook: kmeans({ pixels, width, height, num_colors })
Hook->>Worker: postMessage { call: "kmeans", buffers: [...], args: [...] }
Worker->>WASM: ccall('kmeans', ptr_in, ptr_out_pixels, ptr_out_labels, width, height, k, max_iter)
WASM-->>Worker: out_pixels + out_labels
Worker-->>Hook: postMessage { pixels: Uint8ClampedArray, labels: Int32Array }
Hook-->>React: resolve({ pixels, labels })
React->>Hook: findContours({ pixels, labels, width, height, min_area })
Hook->>Worker: postMessage { call: "kmeans_clustering_graph", args: [...] }
Worker->>WASM: ccall('kmeans_clustering_graph', ptr_pixels, ptr_labels, width, height, min_area, draw_borders)
WASM-->>Worker: modified pixels (and contour metadata internal)
Worker-->>Hook: postMessage { pixels: Uint8ClampedArray }
Hook-->>React: resolve(contours)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~55 minutes Possibly related issues
Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In @src/hooks/useWasmWorker.js:
- Around line 60-63: The function expression findContours is missing a trailing
semicolon which causes Prettier/formatting failures; add a semicolon after the
closing brace of the findContours async function expression (the one returning
call('visualize_contours'...). Ensure the pattern matches other function
expressions in the file by placing the semicolon immediately after the function
expression's closing brace.
In @src/wasm/modules/image/include/find_contours.h:
- Around line 21-32: The getter inner_pixel() can return uninitialized data
because inner_pixel_ isn't initialized; initialize inner_pixel_ at declaration
(e.g., default-construct ImageLib::RGBAPixel<uint8_t> inner_pixel_ = {}), or
alternatively add a has_inner_pixel() method that checks is_inner_pixel_set_ and
make inner_pixel() either return a default-initialized pixel when unset or
assert/throw; update references to inner_pixel_, inner_pixel(),
set_inner_pixel(), and is_inner_pixel_set_ accordingly so callers cannot observe
uninitialized pixels.
In @src/wasm/modules/image/src/find_contours.cpp:
- Around line 164-172: The contour's starting pixel (start_x, start_y) is never
pushed into the contour points, so add the start coordinate to the contour
before entering the neighbor-walking loop: after creating Contour contour and
setting contour.set_inner_pixel(image(start_x, start_y)), append the starting
coordinate to contour.points (or call the contour's point-add method) so the
vector of points includes {start_x, start_y} as the first element; keep the
existing current = {start_x, start_y} and prev_dir = 7 logic intact so
subsequent neighbor discovery continues from that initial point.
- Around line 86-155: The static function find_next_contour_pixel is dead code
(never called) and contains four debug log() calls; either delete the entire
function to remove maintenance burden, or if you want to keep it for future use,
strip out the log(...) statements inside find_next_contour_pixel (the calls
emitting "1. skipping neighbor...", "2. skipping neighbor...", "3. border
pixel...", "4. incorrect at...") and leave the rest unchanged; locate the
function by its name and remove or clean the logging accordingly.
🧹 Nitpick comments (5)
src/wasm/modules/image/src/kmeans.cpp (1)
144-150: Consider applying consistent brace-init style tokmeans_clustering_spatial.The
kmeans_clusteringfunction was updated to use brace-initialization, butkmeans_clustering_spatialstill uses the older style. For consistency within the file, consider applying the same modernization here.src/components/WasmImageProcessor.jsx (1)
118-129: Consider extracting threshold logic into a helper function.The nested ternary for
minimumAllowedMinAreais functional but could be more readable as a small helper function or lookup.Optional refactor
const getMinimumAllowedMinArea = (area) => { if (area > 100_000_000) return 25; if (area > 10_000_000) return 20; if (area > 1_000_000) return 15; return 10; }; // Usage: const minimumAllowedMinArea = getMinimumAllowedMinArea(area);src/wasm/modules/image/src/find_contours.cpp (3)
6-8: Duplicate#include <iostream>.Line 8 duplicates the include on line 6.
Proposed fix
#include "PixelConverters.h" #include <iostream> #include <array> -#include <iostream> #include <string>
173-194: Loop termination condition is misleading.The
while (current != std::make_pair(start_x, start_y))condition will never evaluate to true because:
- The starting pixel is already labeled before entering the loop (line 219 in
find_contours)- Line 182 skips already-labeled pixels (
labels[ny][nx] != -1)The loop always terminates via
found_dir == -1(line 193). Consider simplifying towhile (true)or documenting that the termination relies on exhausting unlabeled neighbors.
73-84: Remove unusedupdate_labelfunction.This function is not called anywhere in the codebase and appears to be a remnant from a previous implementation approach.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/components/WasmImageProcessor.jsxsrc/hooks/useWasmWorker.jssrc/wasm/modules/image/include/Pixel.hsrc/wasm/modules/image/include/find_contours.hsrc/wasm/modules/image/include/visualize_contours.hsrc/wasm/modules/image/src/find_contours.cppsrc/wasm/modules/image/src/kmeans.cppsrc/wasm/modules/image/src/visualize_contours.cpp
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-12-31T17:46:54.476Z
Learnt from: fransafu
Repo: Ryan-Millard/Img2Num PR: 176
File: src/wasm/modules/image/src/bilateral_filter.cpp:98-101
Timestamp: 2025-12-31T17:46:54.476Z
Learning: In bilateral_filter.cpp (src/wasm/modules/image/src/bilateral_filter.cpp), rely on the fact that the center pixel contributes a weight of exactly 1.0 to both spatial and range components (exp(0) = 1) so the normalization sum cannot be zero. This implies you do not need explicit guards against division by zero for normalization in this implementation.
Applied to files:
src/wasm/modules/image/src/visualize_contours.cppsrc/wasm/modules/image/src/kmeans.cppsrc/wasm/modules/image/src/find_contours.cpp
📚 Learning: 2026-01-06T04:56:57.269Z
Learnt from: Ryan-Millard
Repo: Ryan-Millard/Img2Num PR: 191
File: src/wasm/modules/image/src/kmeans.cpp:128-134
Timestamp: 2026-01-06T04:56:57.269Z
Learning: In C++ sources compiled for WASM with Emscripten/Clang, designated initializers (e.g., RGBXY{.r = ..., .g = ...}) are allowed as a C++17 extension. When reviewing code that relies on designated initializers, verify that the target toolchain enables CXX_STANDARD 17 or higher and that the build system (CMake/emsdk) uses Emscripten with a compatible clang. If not, avoid such initializers or provide portable alternatives.
Applied to files:
src/wasm/modules/image/src/visualize_contours.cppsrc/wasm/modules/image/src/kmeans.cppsrc/wasm/modules/image/src/find_contours.cpp
📚 Learning: 2026-01-06T21:06:24.476Z
Learnt from: Ryan-Millard
Repo: Ryan-Millard/Img2Num PR: 191
File: src/wasm/modules/image/src/cielab.cpp:43-57
Timestamp: 2026-01-06T21:06:24.476Z
Learning: In the Img2Num project, prefer recommending and using the provided docker/script wrappers (e.g., ./img2num format-wasm, ./img2num clean-wasm) instead of invoking local tools directly (e.g., clang-format -i). This reduces dependency requirements for users and ensures consistent tooling across environments. Apply this guidance to C++ source files under the project when reviewing changes.
Applied to files:
src/wasm/modules/image/src/visualize_contours.cppsrc/wasm/modules/image/src/kmeans.cppsrc/wasm/modules/image/src/find_contours.cpp
📚 Learning: 2026-01-06T05:10:03.461Z
Learnt from: Ryan-Millard
Repo: Ryan-Millard/Img2Num PR: 191
File: src/wasm/modules/image/src/cielab.cpp:31-39
Timestamp: 2026-01-06T05:10:03.461Z
Learning: When reviewing color space transformation code in C++ (specifically in src/wasm/modules/image/src/*.cpp), flag and require checks for precision inconsistencies between forward and inverse matrix coefficients. Ensure that round-trip conversions do not accumulate unacceptable rounding errors by comparing forward and inverse results within a defined numerical tolerance, documenting tolerances, and adding tests that verify symmetry (forward then inverse).
Applied to files:
src/wasm/modules/image/src/visualize_contours.cppsrc/wasm/modules/image/src/kmeans.cppsrc/wasm/modules/image/src/find_contours.cpp
🧬 Code graph analysis (5)
src/components/WasmImageProcessor.jsx (3)
src/hooks/useWasmWorker.js (3)
useWasmWorker(4-66)findContours(60-63)mergeSmallRegionsInPlace(56-59)src/wasm/modules/image/src/mergeSmallRegionsInPlace.cpp (4)
minArea(44-46)minArea(44-44)mergeSmallRegionsInPlace(51-117)mergeSmallRegionsInPlace(51-52)src/wasm/modules/image/include/find_contours.h (1)
contours(10-36)
src/wasm/modules/image/src/visualize_contours.cpp (2)
src/wasm/modules/image/include/find_contours.h (2)
contours(10-36)RGBAPixel(21-23)src/wasm/modules/image/src/find_contours.cpp (2)
find_contours(199-224)find_contours(199-199)
src/wasm/modules/image/include/find_contours.h (1)
src/wasm/modules/image/src/find_contours.cpp (2)
find_contours(199-224)find_contours(199-199)
src/wasm/modules/image/include/visualize_contours.h (1)
src/wasm/modules/image/src/visualize_contours.cpp (2)
visualize_contours(36-57)visualize_contours(36-36)
src/wasm/modules/image/src/find_contours.cpp (1)
src/wasm/modules/image/include/find_contours.h (1)
contours(10-36)
🪛 Clang (14.0.6)
src/wasm/modules/image/src/visualize_contours.cpp
[error] 1-1: 'visualize_contours.h' file not found
(clang-diagnostic-error)
src/wasm/modules/image/src/find_contours.cpp
[error] 3-3: 'find_contours.h' file not found
(clang-diagnostic-error)
🪛 GitHub Actions: CI
src/hooks/useWasmWorker.js
[warning] 1-1: Code style issues found in 1 file. Run 'prettier --write' to fix.
src/components/WasmImageProcessor.jsx
[warning] 1-1: Code style issues found in 1 file. Run 'prettier --write' to fix.
🔇 Additional comments (10)
src/wasm/modules/image/include/Pixel.h (1)
8-9: LGTM!The static_assert message now correctly references
Pixel<NumberT>instead of the outdatedRGBPixel<NumberT>, improving diagnostic clarity.src/wasm/modules/image/include/visualize_contours.h (1)
1-10: LGTM!Clean header with proper include guard. The exported function signature is well-defined and includes are minimal and appropriate.
src/wasm/modules/image/src/kmeans.cpp (1)
28-58: LGTM! Style modernization looks good.The brace-initialization style updates are consistent and follow modern C++ practices.
src/components/WasmImageProcessor.jsx (2)
194-209: LGTM!Memoizing static JSX with
useMemois a reasonable optimization to prevent unnecessary re-creation on each render.
171-191: The concern aboutImageDatacompatibility is invalid. The WASM worker already returnsUint8ClampedArray(seewasmWorker.jsline 49), which is the correct type required by theImageDataconstructor. No conversion is needed in the canvas drawing code.Likely an incorrect or invalid review comment.
src/wasm/modules/image/src/visualize_contours.cpp (1)
36-57: LGTM!The
visualize_contoursimplementation correctly loads the image, finds contours, assigns unique colors, and writes the modified data back to the buffer. The static analysis warning about missing header is a false positive—the build system handles include paths.src/wasm/modules/image/include/find_contours.h (1)
1-38: LGTM overall!The
Contourclass structure is clean with appropriate encapsulation for the inner pixel. Thefind_contoursdeclaration matches the implementation. The hierarchy index fields (parent_index,first_child_index, etc.) provide good support for future hole/nesting handling per the PR objectives.src/wasm/modules/image/src/find_contours.cpp (3)
14-28: LGTM!The Moore 8-neighbor offsets are correctly defined in clockwise order with clear documentation.
30-54: LGTM!The border mask computation correctly identifies pixels adjacent to differently-colored neighbors. The early break optimization is good.
199-224: LGTM with minor suggestion.The main contour-finding logic correctly identifies and traces contours from unvisited border pixels. For large images, consider reserving capacity for
contoursif you can estimate the typical number of contours to reduce reallocations.
935cc50 to
3c22c71
Compare
…and provide more data - input is const - out_ params are return values - return labels & modified image
|
how can i visualize |
I removed the code to visualize it because it isn't something I want on the live site. Ask ChatGPT to update WasmImageProcessor.jsx to display the image from findContours instead of navigating to the editor page. By the way, were you familiar with React or JavaScript at all before you started contributing to this repo? It doesn't really change much, I just want to know where your skills lie. |
|
I can figure it out... have limited experience with react. more with javascript... |
|
The problem is on the main branch I think. |
|
yeah strange. |
|
@Krasner, I have absolutely no clue. I'm going to look into it. The bilateral filter was definitely the last change made to the C++ on the main branch: [21:58] ~/projects/Img2Num $ git log --oneline -20 -- src/wasm/modules/image/
c399755 feat(bilateral filter): implement bilateral filter for denoising before K-Means (#191)
43e6160 feat(cross platform refactor & docker): Add Docker setup, CMake and cross-platform build scripts (#139)
8c40079 create(Docusaurus Skeleton): Basic template with starter code (#75)
1b85d2d feat(Merge Small Regions): Detect & merge regions in processed images that are difficult to click
25d4289 fix(.editorconfig & prettier): Fixes bad commit in 875da1e8b6d1988dd67167eb9e8a5fca9bbdac4b
34fad6d update(Call chain): Better call chain in JS for image preprocessing before K-Means
6978a71 create(Gaussian Blur FFT): Preprocessor for K-Means
1798830 create(2D FFT): Fast Fourier in 2 dimensions
632068e create(Iterative FFT): Fast Fourier Transform - spatial -> frequency domain
d482097 create(exported.h): Macro simplifies exported functions. Implemented in other files (#36)
6e7281c chore(Refactor K-Means into dedicated module): Segregated image_utils.cpp logically (#30)
eef0a57 fixed(Makefile): fixed Makefile flags the kmeans_clustering() function doesn't cause OOM errors.
7b28b87 squash(OOP Image Abstractions): Add several classes to assist with buffer manipulation (#18)
c2f96cf fix(image_utils.cpp): Fix alpha bug in kmeans_clustering function (#17 & #19)
a387ce5 refactor(Dev builds): Setup change detection & hot reloads/rebuilds (#6)
[21:58] ~/projects/Img2Num $ git branch
docs/readme/maintainers-contributors
feat/bilater_filter_gpu
feat/contour-tracing
feat/contour-tracing-cleanup
feat/export-processed-image/issue-85
* main
modified
[22:00] ~/projects/Img2Num $ |
|
my feat/contour-tracing is after the bilateralFilter and so far i don't see this issue. |
That doesn't make sense either. How do you have no problem? It should be the same code.😭 |
|
Can you run the command below from your own contours branch for me please: git diff origin/main -- src/components/WasmImageProcessor.jsx src/wasm/modules/image/src/{image_utils.cpp,bilateral_filter.cpp,kmeans.cpp} |
see attached |
|
another solution is to run kmeans again after contours to force a fixed number of colors again. that might have changed as regions were merged |
|
We'll add a feature to fix this soon. I'm not sure who will, but I'll try to get it going. |
|
OK! In that case we should have a better kmeans color initialization approach. Some how initialize to important colors based on a histogram, for example. |
There's no need - the problem is with a single parameter. I changed it to 8 in the bilateral filter PR so users wouldn't have to wait so long to get a result. We need to get some buttons added to the home screen to configure the parameters. |
|
Otherwise, do you think this PR can be merged? |
|
Yes I think it's good to go |
|
Thanks! |







✨ Feature Pull Request
📌 Description
🔗 Issue
Fixes #189
📦 Type of Change
🧪 How Has This Been Tested?
✔️ Checklist
📸 Screenshots / Demo
See #220
Summary by CodeRabbit
New Features
Improvements
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.