WIP: feat(bilateral filter): implement bilateral filter for denoising before K-Means in RGB & CIELAB#192
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughThe pull request extends the bilateral filter to support CIELAB color space processing alongside RGB, adds a color_space parameter to the public API, and introduces lab_to_rgb conversion utilities. Documentation is updated with expanded explanations and sidebar positioning adjustments. Build flags are updated to enable fast-math optimizations. Changes
Sequence DiagramsequenceDiagram
participant Caller
participant bilateral_filter
participant SpatialWeights
participant RGBPath as RGB Path<br/>(LUT-based)
participant CIELABPath as CIELAB Path<br/>(On-the-fly)
participant rgb_to_lab
participant lab_to_rgb
Caller->>bilateral_filter: Call with image, width, height,<br/>sigma_spatial, sigma_range, color_space
bilateral_filter->>SpatialWeights: Precompute spatial weights
SpatialWeights-->>bilateral_filter: spatial_weights[]
alt color_space == RGB
bilateral_filter->>RGBPath: Use range_lut for weights
loop For each pixel
RGBPath->>RGBPath: Apply spatial weight
RGBPath->>RGBPath: Lookup range weight from LUT
RGBPath->>RGBPath: Update per-channel accumulators (R,G,B)
end
RGBPath->>RGBPath: Normalize & write RGB output
RGBPath-->>bilateral_filter: Modified RGB image
else color_space == CIELAB
bilateral_filter->>rgb_to_lab: Convert image to LAB
rgb_to_lab-->>bilateral_filter: cie_image(L,A,B)
bilateral_filter->>CIELABPath: Use on-the-fly range weights
loop For each pixel
CIELABPath->>CIELABPath: Apply spatial weight
CIELABPath->>CIELABPath: Compute LAB distance,<br/>calculate range weight (Gaussian)
CIELABPath->>CIELABPath: Update per-channel accumulators (L,A,B)
end
CIELABPath->>lab_to_rgb: Convert averaged LAB back to RGB
lab_to_rgb-->>CIELABPath: RGB uint8_t values
CIELABPath-->>bilateral_filter: Modified RGB image
end
bilateral_filter-->>Caller: Return processed image
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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 |
|
deleted a comment about raw pointers vs std::vector for |
Ryan-Millard
left a comment
Ryan-Millard
left a comment
There was a problem hiding this comment.
This is much faster. Thank you!
I noticed quite a large difference between the RGB and CIELAB blurs:
Left: CIELAB, Right: RGB
Please will you have a look at why it's so much blurrier for the CIELAB implementation (it could be SIGMA_RADIUS_FACTOR - I think you used 1.5 instead of 3.0 in your original implementation).
There was a problem hiding this comment.
Please write some documentation for this change because it poses possible future risks and may make future code break. As a result, I'd like some clear documentation on how this file is set up in case we run into problems in the future.
A good place for the documentation file would be in docs/docs/reference/wasm/modules/image - please create a new folder for this documentation.
| #include <algorithm> | ||
| #include <cstring> | ||
| #include <climits> | ||
| #include <iostream> |
There was a problem hiding this comment.
Please remove this since it is not needed in production code - I'm thinking of adding a debug module in the future that will allow use to use things like std::cout in debug mode.
| #include <iostream> |
There was a problem hiding this comment.
yeah std::cout does work and I can see the print outs in the browser console
There was a problem hiding this comment.
can you attach the original image you used for testing?
There was a problem hiding this comment.
can you attach the original image you used for testing?
There was a problem hiding this comment.
yeah std::cout does work and I can see the print outs in the browser console
I did set up Emscripten's flags to allow std::cout, but we don't want that happening in production since it is an unnecessary slow-down (especially when it is coming from the C++ WebAssembly because it needs to switch to a JavaScript context to print it).
In the future, I'd like to create a module that uses the preprocessor to determine whether to include debug lines like std::cout to reduce the size of production binaries and speed it up slightly.
There was a problem hiding this comment.
try CIELAB with sigma_range = 30.0
There was a problem hiding this comment.
See the comment I left below.
|
This is really stunning! Original
After processing
|
The Supposed "Bug" I Mentioned EarlierI found the problem: Before My ChangesSnippet from
|
|
Currently should we put in a scaling factor in the bilateralFilter function to automatically scale |
I just opened #195 to discuss that. I'm not sure whether we should internally handle it or document that the users of the function must manage it themselves. |
|
before we conclude on this I want to try another approach: currently let me implement... |
|
https://github.com/Krasner/Img2Num/tree/feat/kmean-preprocessing/bilateral-filter-debug2 check this branch. hard to say if there is a visual change but mathematically probably more correct. then combined and converted back to rgb if looks good then I'll merge |
|
@Ryan-Millard please review https://github.com/Krasner/Img2Num/tree/feat/kmean-preprocessing/bilateral-filter-debug2 |
Hi. I'm having a hard time identifying what you changed - please will you merge it into into this branch so I can use the GitHub UI to see the difference. |
|
@Krasner, have you merged it yet? I don't see anything on my side. |
|
Yes it's merged |
Oh, okay. I thought you were referring to the changes on your |
|
@Krasner, please will you review the documentation I just pushed here. |
|
looks good. thanks! |
Your code looks great! I'm cleaning it up a bit and I'll push it soon. |
…ization
- Replace acc0/acc1/acc2 with descriptive weight_acc_channel_0/1/2
- Use brace-initialization for ints and doubles
- Minor spacing and formatting cleanup
Ryan-Millard
left a comment
Ryan-Millard
left a comment
There was a problem hiding this comment.
This is really nice. Thank you @Krasner.
Please see my comments on the individual files as well as write some documentation for cielab.h & cielab.cpp - it doesn't need to be a lot because it's quite simple.
Ryan-Millard
left a comment
Ryan-Millard
left a comment
There was a problem hiding this comment.
Thank you!
Please will you write some docs for the CIELAB conversions.
|
@CodeRabbit perform a full review and write some Docusaurus documentation to help with the CIELAB docs |
🧩 Analysis chain@Ryan-Millard, I'll perform a full review and create Docusaurus documentation for the CIELAB bilateral filtering. Let me first examine the documentation structure. 🏁 Script executed: Length of output: 1198 🏁 Script executed: Length of output: 20204 🏁 Script executed: Length of output: 37804 @Ryan-Millard, I'll create comprehensive Docusaurus documentation for the CIELAB color space considerations and then perform a full review. Here's the new documentation file: New Documentation: Color Space Selection GuideCreate a new file at ---
id: color-spaces
title: Color Space Selection — RGB vs CIELAB
sidebar_label: Color Spaces
sidebar_position: 3
---
# Color Space Selection — RGB vs CIELAB
The bilateral filter in Img2Num supports two color spaces for computing range (color) distances: **RGB** and **CIELAB**. This guide explains the differences, trade-offs, and when to use each.
## Quick Comparison
| Aspect | RGB | CIELAB |
|:---|:---|:---|
| **Perceptual accuracy** | Lower — equal Euclidean distances don't correspond to equal perceived color differences | Higher — designed to be perceptually uniform |
| **Performance** | Faster — uses precomputed LUT | Slower — requires conversion and on-the-fly computation |
| **Edge preservation** | Good for most images | Better for images with subtle color transitions |
| **Best for** | General purpose, real-time applications | High-quality processing, perceptual accuracy |
## When to Use Each Color Space
### Use RGB when:
- **Performance is critical** — RGB processing is significantly faster due to LUT optimization
- **Working with high-contrast images** — where edge preservation is less sensitive to color space choice
- **Real-time processing** — where milliseconds matter
- **Sigma_range values are well-tuned** — and visual results are satisfactory
### Use CIELAB when:
- **Perceptual uniformity matters** — you want visually equal smoothing across different hues
- **Working with skin tones or subtle gradients** — where human perception is sensitive
- **Quality over speed** — when processing time is less critical than output quality
- **Processing medical or scientific imagery** — where perceptual accuracy is important
## Mathematical Differences
### Distance Metrics
Both color spaces compute the Euclidean distance between color vectors, but the ranges differ significantly.
#### RGB Color Space
RGB channels are bounded `[0, 255]` per channel:
$$
\text{distance}_{\text{RGB}} = \sqrt{\Delta R^2 + \Delta G^2 + \Delta B^2}
$$
Maximum possible distance:
$$
\text{max}_{\text{RGB}} = \sqrt{255^2 + 255^2 + 255^2} \approx 441.67
$$
#### CIELAB Color Space
CIELAB channels have different ranges:
- **L\***: `[0, 100]` (lightness)
- **a\***: approximately `[-128, 127]` (green-red)
- **b\***: approximately `[-128, 127]` (blue-yellow)
$$
\text{distance}_{\text{LAB}} = \sqrt{\Delta L^2 + \Delta a^2 + \Delta b^2}
$$
Maximum theoretical distance:
$$
\text{max}_{\text{LAB}} = \sqrt{100^2 + 255^2 + 255^2} \approx 373.56
$$
:::important Key Insight
In practice, most real-world pixel differences are **much smaller** than the maximum possible distance. CIELAB distances for neighboring pixels are typically smaller than RGB distances due to:
1. **Numerical compression** from the RGB→LAB conversion
2. **Perceptual scaling** — LAB is designed to reflect human vision, which perceives smaller differences
:::
## Sigma_range Behavior Differences
The `sigma_range` parameter controls edge preservation by weighting color similarity. However, the same `sigma_range` value produces **different visual results** in RGB vs CIELAB.
### The Range Weight Formula
The bilateral filter computes range weights using a Gaussian:
$$
w_{\text{range}} = \exp\left(-\frac{\text{distance}^2}{2\sigma_{\text{range}}^2}\right)
$$
- When distance is **small**, weight is **high** (≈1) → strong contribution
- When distance is **large**, weight is **low** (≈0) → weak contribution
### Why the Same Sigma Produces Different Results
<div style={{ display: 'flex', justifyContent: 'center', alignItems: 'center', marginBottom: '20px' }}>
<svg
style={{ width: '100%', maxWidth: '600px' }}
viewBox="0 0 500 200"
xmlns="http://www.w3.org/2000/svg"
>
{/* Background */}
<rect x="0" y="0" width="100%" height="100%" fill="#f8f9fa" />
{/* RGB curve (sigma_range = 50) */}
<path
d={(() => {
const sigma = 50;
const points = Array.from({ length: 151 }, (_, i) => {
const dist = i * 3; // 0 to 450
const weight = Math.exp(-(dist*dist)/(2*sigma*sigma));
const x = 50 + i * 2.5;
const y = 170 - weight * 120;
return `${x},${y}`;
});
return `M${points.join(" L")}`;
})()}
fill="none"
stroke="#ff6b6b"
strokeWidth="3"
/>
{/* CIELAB curve (sigma_range = 50) */}
<path
d={(() => {
const sigma = 50;
const points = Array.from({ length: 126 }, (_, i) => {
const dist = i * 3; // 0 to 375
const weight = Math.exp(-(dist*dist)/(2*sigma*sigma));
const x = 50 + i * 2.5;
const y = 170 - weight * 120;
return `${x},${y}`;
});
return `M${points.join(" L")}`;
})()}
fill="none"
stroke="#4dabf7"
strokeWidth="3"
strokeDasharray="5,5"
/>
{/* Axes */}
<line x1="50" y1="170" x2="450" y2="170" stroke="#000" strokeWidth="1" />
<line x1="50" y1="30" x2="50" y2="170" stroke="#000" strokeWidth="1" />
{/* Labels */}
<text x="250" y="195" fontSize="14" textAnchor="middle" fill="#000">Color Distance</text>
<text x="20" y="100" fontSize="14" textAnchor="middle" fill="#000" transform="rotate(-90 20 100)">Weight</text>
{/* Legend */}
<line x1="320" y1="20" x2="360" y2="20" stroke="#ff6b6b" strokeWidth="3" />
<text x="370" y="25" fontSize="12" fill="#000">RGB (σᵣ = 50)</text>
<line x1="320" y1="40" x2="360" y2="40" stroke="#4dabf7" strokeWidth="3" strokeDasharray="5,5" />
<text x="370" y="45" fontSize="12" fill="#000">CIELAB (σᵣ = 50)</text>
{/* Annotation */}
<text x="150" y="90" fontSize="11" fill="#666" textAnchor="middle">CIELAB: Steeper falloff</text>
<text x="150" y="105" fontSize="11" fill="#666" textAnchor="middle">→ More blur</text>
</svg>
</div>
**With `sigma_range = 50`**:
- **RGB**: Typical neighboring pixel distances are small relative to 50, so many neighbors contribute significantly → **moderate blur**
- **CIELAB**: Typical neighboring pixel distances are even smaller, so almost all neighbors contribute strongly → **stronger blur**
### Sigma_range Scaling for Visual Consistency
To achieve **visually similar** blur between RGB and CIELAB, you can scale `sigma_range`:
```javascript
// Example: Scaling RGB sigma_range to match CIELAB visual output
const sigma_range_base = 50.0; // Target CIELAB sigma_range
let sigma_range_actual;
if (color_space === COLOR_SPACE_RGB) {
// Scale RGB sigma_range to match CIELAB perceptually
sigma_range_actual = sigma_range_base * 4.18;
} else {
sigma_range_actual = sigma_range_base;
}:::important Scaling Factor
Visual ExampleUsing the same
Performance ConsiderationsRGB Performance
CIELAB Performance
Performance Impact: CIELAB is typically 2-4× slower than RGB, depending on image size and kernel radius. :::tip Optimization Note
Implementation DetailsRGB Range Weights (LUT)// Precompute all possible RGB distances
std::vector<double> range_lut(MAX_RGB_DIST_SQ + 1);
for (int i = 0; i <= MAX_RGB_DIST_SQ; ++i) {
range_lut[i] = gaussian(std::sqrt(i), sigma_range);
}
// Later, during filtering:
const int dr = r_neighbor - r_center;
const int dg = g_neighbor - g_center;
const int db = b_neighbor - b_center;
const int dist_sq = dr*dr + dg*dg + db*db;
double w_range = range_lut[dist_sq]; // O(1) lookupCIELAB Range Weights (On-the-fly)// Precompute full-image RGB → LAB conversion
std::vector<double> cie_image(width * height * 4);
for (each pixel) {
rgb_to_lab(r, g, b, L, A, B);
cie_image[idx] = L; cie_image[idx+1] = A; cie_image[idx+2] = B;
}
// Later, during filtering:
double dL = L_neighbor - L_center;
double dA = A_neighbor - A_center;
double dB = B_neighbor - B_center;
double dist = std::sqrt(dL*dL + dA*dA + dB*dB);
double w_range = gaussian(dist, sigma_range); // Computed on-the-flyRecommendationsDefault ChoiceFor most applications, RGB is the recommended default:
When to Switch to CIELABConsider CIELAB when you observe:
Parameter TuningStarting values:
Adjustment guidelines:
See Also
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Fix all issues with AI Agents 🤖
In @docs/docs/reference/wasm/modules/image/bilateral_filter/overview.md:
- Around line 20-22: Update the paragraph that begins "Similarly to Gaussian
blurs..." to clarify that Gaussian blurs already use spatial distance in their
kernel and that the bilateral filter’s key distinction is that it additionally
weights by intensity/color similarity; replace the current sentences with a
concise rephrasing like: "Unlike Gaussian blurs (which only consider spatial
distance), bilateral filters also weight by intensity/color similarity, allowing
them to preserve sharp edges while still smoothing flat regions." Ensure the new
text replaces both sentences in that block to avoid implying Gaussian blurs
ignore spatial position.
In @src/wasm/modules/image/src/bilateral_filter.cpp:
- Around line 138-140: The code reads L/A/B from cie_image unconditionally
causing out-of-bounds when color_space != COLOR_SPACE_OPTION_CIELAB; move the
accesses to cie_image[neighbor_idx], cie_image[neighbor_idx + 1],
cie_image[neighbor_idx + 2] (the L, A, B variables) inside the branch that
checks color_space == COLOR_SPACE_OPTION_CIELAB so they are only read when
cie_image is populated, and ensure any logic that uses L/A/B only runs in that
CIELAB branch or uses safe RGB alternatives when color_space == RGB.
In @src/wasm/modules/image/src/cielab.cpp:
- Around line 101-105: Remove the invalid constexpr and stray trailing semicolon
from gamma_encode: change the function declaration from constexpr to inline (or
plain) so it doesn't require constexpr-evaluated std::pow, and delete the extra
semicolon after the function body; note that gamma_encode is only used at
runtime (see lab_to_rgb) so switching to inline fixes the compile error without
changing behavior.
♻️ Duplicate comments (1)
src/wasm/modules/image/CMakeLists.txt (1)
52-52: Verify-ffast-mathimplications for color space conversions.Adding
-ffast-mathenables aggressive floating-point optimizations that can improve performance for the LAB conversion math. However, it relaxes IEEE 754 compliance—disabling strict NaN/Inf handling, enabling reassociation, and potentially affecting precision.For the LAB↔RGB conversions (which use
std::pow,std::cbrt,std::exp), this is generally acceptable, but verify that edge cases (e.g., out-of-gamut colors, near-zero values) don't produce unexpected results due to denormal flushing or NaN mishandling.Regarding documentation: a past review comment already requested documentation for this CMakeLists.txt configuration. Please ensure that is addressed.
Also applies to: 58-58
🧹 Nitpick comments (2)
src/wasm/modules/image/include/cielab.h (1)
8-10: LGTM!The
lab_to_rgbdeclaration correctly complementsrgb_to_labfor bidirectional conversion. The signature matches the implementation.Minor note: Consider using the
out_prefix for output parameters (out_r_u8,out_g_u8,out_b_u8) to match the naming convention inrgb_to_labfor consistency.src/wasm/modules/image/src/bilateral_filter.cpp (1)
77-77: Consider reducingcie_imagememory footprint.Currently allocating 4 doubles per pixel (
width * height * 4) but only using 3 (L, A, B). You could usewidth * height * 3and compute indices asy * width * 3 + x * 3to save ~25% memory on large images.This is a minor optimization—the current approach simplifies index consistency with the RGBA layout.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
docs/docs/reference/wasm/modules/image/bilateral_filter/api.mddocs/docs/reference/wasm/modules/image/bilateral_filter/explained.mddocs/docs/reference/wasm/modules/image/bilateral_filter/implementation.mddocs/docs/reference/wasm/modules/image/bilateral_filter/keywords.mddocs/docs/reference/wasm/modules/image/bilateral_filter/overview.mdsrc/wasm/modules/image/CMakeLists.txtsrc/wasm/modules/image/include/cielab.hsrc/wasm/modules/image/src/bilateral_filter.cppsrc/wasm/modules/image/src/cielab.cpp
🧰 Additional context used
🧠 Learnings (5)
📚 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 implementations, the center pixel always contributes a weight of exactly 1.0 to the weight accumulator (spatial weight = exp(0) = 1.0 and range weight = exp(0) = 1.0), which inherently prevents division by zero during normalization without requiring explicit guards.
Applied to files:
docs/docs/reference/wasm/modules/image/bilateral_filter/overview.mddocs/docs/reference/wasm/modules/image/bilateral_filter/keywords.mddocs/docs/reference/wasm/modules/image/bilateral_filter/implementation.mddocs/docs/reference/wasm/modules/image/bilateral_filter/explained.mddocs/docs/reference/wasm/modules/image/bilateral_filter/api.md
📚 Learning: 2026-01-04T15:34:04.654Z
Learnt from: Ryan-Millard
Repo: Ryan-Millard/Img2Num PR: 146
File: docs/docs/reference/react/components/ThemeSwitch/tests.md:123-124
Timestamp: 2026-01-04T15:34:04.654Z
Learning: In Docusaurus documentation, relative directory links (e.g., ../ or ../../../hooks/useTheme) resolve to index.md within the target directory. Do not require or force linking to index.md explicitly; such relative links remain valid and should not be changed when reviewing docs under the docs directory.
Applied to files:
docs/docs/reference/wasm/modules/image/bilateral_filter/overview.mddocs/docs/reference/wasm/modules/image/bilateral_filter/keywords.mddocs/docs/reference/wasm/modules/image/bilateral_filter/implementation.mddocs/docs/reference/wasm/modules/image/bilateral_filter/explained.mddocs/docs/reference/wasm/modules/image/bilateral_filter/api.md
📚 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/bilateral_filter.cppsrc/wasm/modules/image/src/cielab.cpp
📚 Learning: 2025-12-20T20:11:28.422Z
Learnt from: Ryan-Millard
Repo: Ryan-Millard/Img2Num PR: 0
File: :0-0
Timestamp: 2025-12-20T20:11:28.422Z
Learning: In the Img2Num repository, all documentation should be properly organized in the docs/docs/ folder structure following the Docusaurus conventions, either as a dedicated category or integrated into existing categories like project-scripts.
Applied to files:
src/wasm/modules/image/CMakeLists.txt
📚 Learning: 2025-12-17T22:39:25.711Z
Learnt from: Ryan-Millard
Repo: Ryan-Millard/Img2Num PR: 0
File: :0-0
Timestamp: 2025-12-17T22:39:25.711Z
Learning: In the Img2Num repository, all documentation must be placed in the docs/docs/ folder following the Docusaurus structure, not at the repository root.
Applied to files:
src/wasm/modules/image/CMakeLists.txt
🧬 Code graph analysis (2)
src/wasm/modules/image/include/cielab.h (1)
src/wasm/modules/image/src/cielab.cpp (2)
lab_to_rgb(107-133)lab_to_rgb(107-108)
src/wasm/modules/image/src/bilateral_filter.cpp (1)
src/wasm/modules/image/src/cielab.cpp (4)
rgb_to_lab(65-94)rgb_to_lab(65-66)lab_to_rgb(107-133)lab_to_rgb(107-108)
🪛 markdownlint-cli2 (0.18.1)
docs/docs/reference/wasm/modules/image/bilateral_filter/implementation.md
53-53: Code block style
Expected: fenced; Actual: indented
(MD046, code-block-style)
176-176: Code block style
Expected: fenced; Actual: indented
(MD046, code-block-style)
🔇 Additional comments (11)
docs/docs/reference/wasm/modules/image/bilateral_filter/keywords.md (1)
1-28: LGTM!Well-structured glossary that accurately defines the key concepts used throughout the bilateral filter documentation. The distinction between LUT-based RGB weights and on-the-fly CIELAB computation (line 23) correctly reflects the implementation.
src/wasm/modules/image/src/cielab.cpp (1)
107-133: LGTM!The
lab_to_rgbimplementation correctly follows the standard Lab → XYZ → linear RGB → sRGB pipeline with proper gamma encoding and clamping. The conversion matrices and D65 white point constants are accurate.src/wasm/modules/image/src/bilateral_filter.cpp (1)
176-196: LGTM!The output paths correctly handle both color spaces:
- RGB mode: direct weighted average with proper clamping
- CIELAB mode: LAB-to-RGB conversion before output
Alpha channel preservation is correctly implemented for both paths. Based on learnings, division by
weight_accis safe since the center pixel always contributes weight 1.0.docs/docs/reference/wasm/modules/image/bilateral_filter/api.md (1)
12-18: LGTM!The API documentation accurately reflects the updated function signature with the new
color_spaceparameter. The alpha channel preservation note and parameter descriptions are clear and helpful.Also applies to: 29-29
docs/docs/reference/wasm/modules/image/bilateral_filter/explained.md (2)
26-46: Comprehensive notation and formula breakdown—excellent clarity.The expanded definitions of
$x$ ,$\Omega$ ,$I(x_i)$ , and$C(x_i)$ with explicit RGB vs CIELAB representations make the mathematical foundation clear and accessible. The color distance formulas (lines 45–46) correctly capture Euclidean distance in both spaces.
63-96: Dual-path implementation strategy is well-articulated.The LUT section (lines 63–74) and on-the-fly section (lines 76–96) clearly explain why RGB uses precomputed tables (bounded integer space) while CIELAB computes dynamically (unbounded floating-point space). The code examples reinforce the concepts effectively.
docs/docs/reference/wasm/modules/image/bilateral_filter/implementation.md (5)
27-81: SVG kernel visualization provides valuable pedagogical aid.The 19×19 kernel diagram with center pixel and radius labels effectively communicates the spatial layout. The explicit "Kernel Dimensions" note (line 80) prevents misunderstanding. Note that Docusaurus uses MDX, which requires empty lines above and below language blocks for the MDX parser to recognize Markdown syntax and not JSX—verify the SVG block has appropriate blank lines before/after to ensure proper parsing.
83-158: In-depth LUT vs. on-the-fly comparison is technically sound and well-motivated.The explanation of why RGB uses precomputed tables (bounded 8-bit channels, lines 120–133) versus CIELAB's on-the-fly computation (continuous floating-point values, lines 137–158) demonstrates clear understanding of the implementation trade-offs. The Gaussian helper function (lines 96–100) is concise and correct.
160-241: Collapsible visual comparison enhances comprehension.The dual SVG diagrams showing RGB LUT versus CIELAB on-the-fly weight curves provide intuitive contrast. The annotations clarify why precomputation is feasible in one space but not the other. However, MDX requires empty lines around syntax blocks for the parser to differentiate Markdown from JSX; ensure blank lines surround this entire collapsible section to avoid parsing issues.
243-255: Sliding window loop description correctly reflects dual-path implementation.The step-by-step breakdown (lines 1–5) now properly separates RGB (LUT) and CIELAB (on-the-fly) range weight retrieval, aligning with the implementation changes described in the PR. The terminology and logic are accurate.
1-259: Static analysis hints about code-block-style are likely false positives.The markdownlint reports at lines 53 and 176 (MD046) flag "indented code block" style, but these lines fall within JSX/SVG content (not traditional markdown code blocks). The actual C++ code blocks at lines 96–100, 124–126, and 140–142 use correct triple-backtick fencing. The false positives are likely due to the linter misinterpreting JSX syntax as markdown code. No action required unless rendering issues occur in the published documentation.
| Similarly to Gaussian blurs, it acts as a *low-pass filter* that reduces noise. | ||
| Conversely, it is *less aggressive than Gaussian blurs, since it takes spatial position (x & y coordinates) into account* - | ||
| allowing it to preserve sharp edges. |
There was a problem hiding this comment.
Clarify the distinction from Gaussian blurs.
The current wording suggests Gaussian blurs don't consider spatial position, but they do—that's the basis of their kernel. The key distinction is that bilateral filters additionally consider intensity/color similarity, which is what preserves edges.
Consider rewording to:
Unlike Gaussian blurs (which only consider spatial distance), bilateral filters also weight by intensity/color similarity, allowing them to preserve sharp edges while still smoothing flat regions.
🤖 Prompt for AI Agents
In @docs/docs/reference/wasm/modules/image/bilateral_filter/overview.md around
lines 20-22, Update the paragraph that begins "Similarly to Gaussian blurs..."
to clarify that Gaussian blurs already use spatial distance in their kernel and
that the bilateral filter’s key distinction is that it additionally weights by
intensity/color similarity; replace the current sentences with a concise
rephrasing like: "Unlike Gaussian blurs (which only consider spatial distance),
bilateral filters also weight by intensity/color similarity, allowing them to
preserve sharp edges while still smoothing flat regions." Ensure the new text
replaces both sentences in that block to avoid implying Gaussian blurs ignore
spatial position.
| double L{cie_image[neighbor_idx]}; | ||
| double A{cie_image[neighbor_idx + 1]}; | ||
| double B{cie_image[neighbor_idx + 2]}; |
There was a problem hiding this comment.
Critical: Out-of-bounds access when color_space == RGB.
Lines 138-140 unconditionally read from cie_image, but cie_image is only populated when color_space == COLOR_SPACE_OPTION_CIELAB. In RGB mode, cie_image is empty (size 0), causing undefined behavior.
Move these reads inside the CIELAB case block:
🔎 Proposed fix
uint8_t r{image[neighbor_idx]};
uint8_t g{image[neighbor_idx + 1]};
uint8_t b{image[neighbor_idx + 2]};
- double L{cie_image[neighbor_idx]};
- double A{cie_image[neighbor_idx + 1]};
- double B{cie_image[neighbor_idx + 2]};
-
w_space = spatial_weights[(ky + radius) * kernel_diameter + (kx + radius)];
switch (color_space) {
case COLOR_SPACE_OPTION_RGB: {
const int dr{static_cast<int>(r) - r0};
const int dg{static_cast<int>(g) - g0};
const int db{static_cast<int>(b) - b0};
const int dist_sq{dr*dr + dg*dg + db*db};
w_range = range_lut[dist_sq];
weight_acc_channel_0 += r * w_space * w_range;
weight_acc_channel_1 += g * w_space * w_range;
weight_acc_channel_2 += b * w_space * w_range;
break;
}
case COLOR_SPACE_OPTION_CIELAB: {
+ const double L{cie_image[neighbor_idx]};
+ const double A{cie_image[neighbor_idx + 1]};
+ const double B{cie_image[neighbor_idx + 2]};
+
dL = L - L0;
dA = A - A0;
dB = B - B0;Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In @src/wasm/modules/image/src/bilateral_filter.cpp around lines 138-140, The
code reads L/A/B from cie_image unconditionally causing out-of-bounds when
color_space != COLOR_SPACE_OPTION_CIELAB; move the accesses to
cie_image[neighbor_idx], cie_image[neighbor_idx + 1], cie_image[neighbor_idx +
2] (the L, A, B variables) inside the branch that checks color_space ==
COLOR_SPACE_OPTION_CIELAB so they are only read when cie_image is populated, and
ensure any logic that uses L/A/B only runs in that CIELAB branch or uses safe
RGB alternatives when color_space == RGB.
|
@CodeRabbit, you were meant to help with documenting |
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain@Ryan-Millard, you're absolutely right—I apologize for the misunderstanding. Let me read the CIELAB implementation files and create proper documentation for them. 🏁 Script executed: Length of output: 5564 @Ryan-Millard, thank you for the clarification! Here's comprehensive documentation for the CIELAB conversion functions: 1. Enhanced Header Documentation (
|
| Parameter | Type | Range | Description |
|---|---|---|---|
r_u8 |
uint8_t |
[0, 255] | Red channel (input) |
g_u8 |
uint8_t |
[0, 255] | Green channel (input) |
b_u8 |
uint8_t |
[0, 255] | Blue channel (input) |
out_l |
double& |
[0, 100] | L* lightness (output, clamped) |
out_a |
double& |
~[-128, 127] | a* green-red axis (output) |
out_b |
double& |
~[-128, 127] | b* blue-yellow axis (output) |
Transformation Pipeline
-
sRGB → Linear RGB: Inverse gamma correction (gamma expansion)
- Applies IEC 61966-2-1:1999 sRGB transfer function
- Converts [0, 255] → [0, 1] → linear [0, 1]
-
Linear RGB → XYZ: Matrix multiplication
- Uses D65 illuminant (standard daylight, 6500K)
- Applies ITU-R BT.709 color primaries
-
XYZ → CIELAB: Normalization and nonlinear transform
- Normalizes by D65 reference white point
- Applies CIE-defined piecewise function (cube root or linear near zero)
Example
#include "cielab.h"
// Convert bright red to LAB
uint8_t r = 255, g = 0, b = 0;
double L, a, b_lab;
rgb_to_lab(r, g, b, L, a, b_lab);
// Result: L ≈ 53.2, a ≈ 80.1, b ≈ 67.2lab_to_rgb
Convert CIELAB to 8-bit sRGB color space.
void lab_to_rgb(
const double L,
const double A,
const double B,
uint8_t& r_u8,
uint8_t& g_u8,
uint8_t& b_u8
);Parameters
| Parameter | Type | Range | Description |
|---|---|---|---|
L |
double |
[0, 100] | L* lightness (input) |
A |
double |
~[-128, 127] | a* green-red axis (input) |
B |
double |
~[-128, 127] | b* blue-yellow axis (input) |
r_u8 |
uint8_t& |
[0, 255] | Red channel (output, clamped) |
g_u8 |
uint8_t& |
[0, 255] | Green channel (output, clamped) |
b_u8 |
uint8_t& |
[0, 255] | Blue channel (output, clamped) |
Transformation Pipeline
-
CIELAB → XYZ: Inverse nonlinear transform and denormalization
- Applies inverse piecewise function (cube or linear)
- Denormalizes by D65 white point
-
XYZ → Linear RGB: Inverse matrix multiplication
- May produce out-of-gamut values (negative or >1.0)
-
Linear RGB → sRGB: Gamma correction
- Applies gamma compression using sRGB transfer function
- Clamps to [0, 1], rounds, and converts to [0, 255]
Out-of-Gamut Handling
:::warning Out-of-Gamut Colors
Not all LAB colors are representable in sRGB. Colors outside the sRGB gamut are clamped to the nearest valid RGB value, which may result in color shifts or loss of hue.
:::
Example
#include "cielab.h"
// Convert LAB back to RGB
double L = 53.2, a = 80.1, b = 67.2;
uint8_t r, g, b_rgb;
lab_to_rgb(L, a, b, r, g, b_rgb);
// Result: r ≈ 255, g ≈ 0, b ≈ 0 (bright red)Technical Specifications
Color Space Standards
| Property | Value |
|---|---|
| Color space | sRGB (IEC 61966-2-1:1999) |
| Illuminant | D65 (6500K daylight) |
| Observer | CIE 1931 2° Standard Observer |
| Gamma | 2.4 (sRGB standard) |
| RGB primaries | ITU-R BT.709 |
CIELAB Coordinate System
-
L* (Lightness): Perceptual lightness
- 0 = black
- 100 = white
- 50 ≈ mid-gray
-
a* (Green-Red): Color opponent dimension
- Negative values = green
- Positive values = red
- 0 = neutral (gray axis)
-
b* (Blue-Yellow): Color opponent dimension
- Negative values = blue
- Positive values = yellow
- 0 = neutral (gray axis)
Distance Metric
Euclidean distance in CIELAB space approximates perceptual color difference:
Interpretation:
- ΔE < 1: Imperceptible difference
- ΔE < 2: Perceptible with close observation
- ΔE < 10: Noticeable at a glance
- ΔE > 10: Significant color difference
Usage in Bilateral Filter
The CIELAB color space is used in the bilateral filter to compute perceptually uniform range weights:
// In bilateral_filter.cpp (CIELAB mode)
double dL = L_neighbor - L_center;
double dA = A_neighbor - A_center;
double dB = B_neighbor - B_center;
double dist = std::sqrt(dL*dL + dA*dA + dB*dB);
double w_range = gaussian(dist, sigma_range);This produces more perceptually consistent smoothing compared to RGB Euclidean distance.
Performance Considerations
Computational Cost
| Operation | Complexity |
|---|---|
rgb_to_lab |
~20-30 floating-point operations |
lab_to_rgb |
~25-35 floating-point operations |
Key operations:
- Gamma correction: piecewise with
pow()for nonlinear segment - Matrix multiplication: 3×3 matrix
- XYZ transform:
cbrt()or linear approximation
Optimization Notes
- Batch conversion: When processing entire images, consider vectorization (SIMD)
- LUT for gamma: Can be precomputed for all 256 uint8_t values
- Fast approximations: Polynomial approximations for
pow()andcbrt()can improve speed at slight accuracy cost
See Also
- Bilateral Filter Color Spaces — How CIELAB is used in filtering
- Bilateral Filter Implementation — Implementation details
- CIE 1976 L*a*b* Color Space (Wikipedia)
- sRGB Specification (IEC 61966-2-1:1999)
---
These documentation updates provide:
1. **Inline comments** explaining constants, algorithms, and implementation details
2. **Function-level documentation** with parameter descriptions and transformation pipelines
3. **Docusaurus API reference** with examples, technical specifications, and usage guidance
Would you like me to create any additional documentation, such as a visual guide or comparison charts?
Ryan-Millard
changed the title
|
is this auto generated documentation good for you? or do you want something else? |
It looks good.😁 |
|
i guess coderabbitai didn't place these suggestions in the actual repo.... you can place them :) |
|
Thanks @Krasner! |
Ryan-Millard
merged commit 7baacc8
into
Ryan-Millard:feat/kmean-preprocessing/bilateral-filter
|
I just realized that I rebased the commits into #192, so you'll probably have to just through hoops to fix your history with something like I'm sorry. |
2 participants
Please choose one of the following:
If none of these fit, you may use this default to describe your change manually.
If this is the right template, go ahead and complete it below 👇
📌 Description
Please describe the changes made in this PR and why they are necessary.
Fixes #issue-number (if applicable)
✅ Type of Change
Place an "x" in the brackets below:
🧪 How Has This Been Tested?
Please describe how you tested your changes (e.g., unit tests, manual testing, screenshots, etc.)
🧩 Checklist
Place an "x" in the brackets below:
📸 Screenshots / Demo (if applicable)
Paste images, GIFs, or demo links here.
💬 Additional Context
Anything else relevant to the PR.
Summary by CodeRabbit
New Features
Documentation
Performance
✏️ Tip: You can customize this high-level summary in your review settings.