Add CLAHE to opencv image tool #8730
Conversation
WalkthroughA new feature has been added to the CVAT project that implements Contrast Limited Adaptive Histogram Equalization (CLAHE) for image processing. This includes modifications to the OpenCV control component for user input, the addition of a new CLAHE implementation file, and updates to the documentation. The changes enhance the existing image processing capabilities by allowing users to adjust parameters for CLAHE directly within the interface. Changes
Assessment against linked issues
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Outside diff range and nitpick comments (11)
changelog.d/20241121_062608_neildencklau_opencv_image_clahe.md (1)
1-4: Enhance the changelog entry with more details.While the entry correctly documents the addition of CLAHE, it could be more informative for users by including:
- The purpose (improved local contrast enhancement)
- Key configurable parameters (clip limit, tile grid size)
- Reference to the issue being addressed ([Feature request] Using CLAHE for Histogram equalization #4929)
Consider expanding the entry like this:
### Added -Added contrast limited adaptive histogram equalization tool -(<https://github.com/cvat-ai/cvat/pull/8730>) +- Added contrast limited adaptive histogram equalization (CLAHE) tool for improved local contrast enhancement + - Configurable parameters: clip limit, tile rows, and tile columns + - Better visibility for images with varying lighting conditions + - Addresses image contrast issues (#4929) + (<https://github.com/cvat-ai/cvat/pull/8730>)cvat-ui/src/utils/image-processing.tsx (1)
Line range hint
1-45: Add test coverage for the new CLAHE filter.While the enum addition is straightforward, we should ensure proper test coverage for the CLAHE functionality, especially around the
hasFilterfunction with the new filter alias.Would you like me to help generate test cases for:
- Verifying
hasFilterworks with the new CLAHE alias- Testing filter registration/configuration
- Integration tests with the OpenCV control component
cvat-ui/src/utils/opencv-wrapper/opencv-wrapper.ts (2)
304-304: Consider adding error handling for OpenCV initializationWhile the implementation is correct, consider adding error handling for cases where OpenCV initialization might fail. This would be consistent with the error handling pattern used in other methods of the class.
clahe: () => new CLAHEImplementation(this.cv), +clahe: () => { + try { + return new CLAHEImplementation(this.cv); + } catch (error) { + console.error('Failed to initialize CLAHE:', error); + throw new Error('Failed to initialize CLAHE implementation'); + } +},
Line range hint
1-324: Add unit tests for CLAHE functionalityThe implementation looks solid, but there are no tests covering the new CLAHE functionality. Consider adding unit tests to verify:
- Proper initialization of CLAHE
- Error handling scenarios
- Integration with the OpenCV wrapper
- Parameter validation
This aligns with the PR objectives which mention that tests haven't been added yet.
Would you like me to help generate unit test cases for the CLAHE implementation?
site/content/en/docs/manual/advanced/ai-tools.md (2)
Line range hint
286-312: Consider adding context about limitationsThe section effectively explains histogram equalization, but it would be helpful to briefly mention its limitations that led to implementing CLAHE. This context would help users understand when to use each method.
Add a note about limitations, for example:
It is useful in images with backgrounds and foregrounds that are bright or dark. + Note: While histogram equalization works well for many images, it applies the enhancement globally. + This may not be optimal for images with varying lighting conditions across different regions. + In such cases, consider using CLAHE (see below). To improve the contrast of the image, do the following:
314-314: Fix typo in CLAHE descriptionThere's a spelling error in the text.
-Contrast Limited Adaptive Histogram Equalization (CLAHE) increases contrast by applying clipped histogram equalization to multiple tiles across the input image. In images where there are both very bright and very dark regions, this improves contrast in the dark regions without loosing contrast in the bright ones. +Contrast Limited Adaptive Histogram Equalization (CLAHE) increases contrast by applying clipped histogram equalization to multiple tiles across the input image. In images where there are both very bright and very dark regions, this improves contrast in the dark regions without losing contrast in the bright ones.cvat-ui/src/components/annotation-page/standard-workspace/controls-side-bar/opencv-control.tsx (4)
88-90: Consider using constants for CLAHE default valuesThe default values for CLAHE parameters could be defined as class constants or exported constants, making them easier to maintain and reuse.
+ const DEFAULT_CLAHE_CLIP_LIMIT = 40; + const DEFAULT_CLAHE_TILE_SIZE = 8; class OpenCVControlComponent extends React.PureComponent<Props & DispatchToProps, State> { // ... this.state = { - claheClipLimit: 40, - claheTileColumns: 8, - claheTileRows: 8, + claheClipLimit: DEFAULT_CLAHE_CLIP_LIMIT, + claheTileColumns: DEFAULT_CLAHE_TILE_SIZE, + claheTileRows: DEFAULT_CLAHE_TILE_SIZE, };Also applies to: 163-165
620-621: Consider adding more descriptive tooltip textThe current tooltip text is technical. Consider adding a brief explanation of what CLAHE does for users who might not be familiar with the technique.
- title='Contrast Limited Adaptive Histogram Equalization' + title='Enhance local contrast (CLAHE) - Improves image details in both bright and dark regions'
655-675: Enhance input validation and user feedbackThe numeric inputs could benefit from:
- Tooltips explaining the impact of each parameter
- Immediate visual feedback when values are out of optimal ranges
- Debounced updates to prevent excessive re-rendering
Consider wrapping the InputNumber components in a custom component that handles these improvements:
interface CLAHEInputProps { label: string; value: number; min: number; max: number; onChange: (value: number) => void; tooltip: string; defaultValue: number; } const CLAHEInput: React.FC<CLAHEInputProps> = ({ label, value, min, max, onChange, tooltip, defaultValue }) => { const debouncedOnChange = useMemo( () => debounce(onChange, 300), [onChange] ); return ( <Tooltip title={tooltip}> <InputNumber value={value} min={min} max={max} onChange={(v) => debouncedOnChange(v || defaultValue)} status={value > max * 0.8 ? 'warning' : undefined} /> </Tooltip> ); };Also applies to: 681-701, 706-726
619-730: Add tests for CLAHE functionalityThe new CLAHE feature would benefit from unit tests covering:
- Parameter validation
- UI interaction
- Integration with OpenCV wrapper
- Error handling
Would you like me to help create a test suite for the CLAHE implementation?
cvat-ui/src/utils/opencv-wrapper/clahe.ts (1)
70-70: Consider rethrowing the original error to preserve stack traceThrowing a new error with
e.toString()loses the original stack trace, which can hinder debugging. Rethrow the original error instead.Apply this change:
- throw new Error(e.toString()); + throw e;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (2)
site/content/en/images/image221.jpgis excluded by!**/*.jpgsite/content/en/images/opencv-image-clahe_interaction.jpgis excluded by!**/*.jpg
📒 Files selected for processing (6)
changelog.d/20241121_062608_neildencklau_opencv_image_clahe.md(1 hunks)cvat-ui/src/components/annotation-page/standard-workspace/controls-side-bar/opencv-control.tsx(5 hunks)cvat-ui/src/utils/image-processing.tsx(1 hunks)cvat-ui/src/utils/opencv-wrapper/clahe.ts(1 hunks)cvat-ui/src/utils/opencv-wrapper/opencv-wrapper.ts(3 hunks)site/content/en/docs/manual/advanced/ai-tools.md(3 hunks)
🔇 Additional comments (3)
cvat-ui/src/utils/opencv-wrapper/opencv-wrapper.ts (2)
9-9: LGTM: Import statement follows existing patterns
The import statement for CLAHE follows the established pattern and correctly imports both the implementation and interface.
37-37: LGTM: Interface extension is clean and consistent
The addition of the clahe() method to the ImgProc interface follows the established pattern and correctly defines the return type.
site/content/en/docs/manual/advanced/ai-tools.md (1)
285-285: LGTM!
The section header is correctly formatted.
| @@ -32,6 +32,7 @@ export interface ImageFilter { | |||
|
|
|||
| export enum ImageFilterAlias { | |||
| HISTOGRAM_EQUALIZATION = 'opencv.histogramEqualizaton', | |||
| CLAHE = 'opencv.CLAHE', | |||
There was a problem hiding this comment.
💡 Codebase verification
Based on the search results, I can now provide a final response:
The CLAHE implementation is properly integrated but lacks test coverage
The verification shows:
- CLAHE is correctly implemented in
cvat-ui/src/utils/opencv-wrapper/clahe.ts - It's properly integrated in the UI controls (
opencv-control.tsx) - The enum is used consistently with the implementation
- However, no test files were found for this feature
Action items:
- Add unit tests for the CLAHE implementation in
cvat-ui/src/utils/opencv-wrapper/clahe.ts - Add integration tests for the OpenCV controls component
- Consider adding visual regression tests for the CLAHE filter effects
🔗 Analysis chain
LGTM! Verify consistent usage across the codebase.
The new CLAHE enum value follows the established naming and value format conventions.
Let's verify the consistent usage of this new filter alias:
Let me gather more information about the CLAHE implementation to verify its usage.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for consistent usage of the CLAHE filter alias
# Expected: Should find references in OpenCV control component and tests
# Search for CLAHE usage
rg -l "opencv\.CLAHE"
# Search for potential test files
fd -e test.tsx -e test.ts -e spec.tsx -e spec.ts
Length of output: 113
Script:
#!/bin/bash
# Search for CLAHE-related code in the codebase
rg -g '!*.test.*' -g '!*.spec.*' "CLAHE" -A 5 -B 5
# Look for image processing related components
fd -e tsx -e ts | rg -i "image|filter|opencv"
# Check for any test files containing image processing related terms
fd -e test.tsx -e test.ts -e spec.tsx -e spec.ts | rg -i "image|filter|opencv"
Length of output: 19892
| ### Contrast Limited Adaptive Histogram Equalization | ||
| Contrast Limited Adaptive Histogram Equalization (CLAHE) increases contrast by applying clipped histogram equalization to multiple tiles across the input image. In images where there are both very bright and very dark regions, this improves contrast in the dark regions without loosing contrast in the bright ones. | ||
|
|
||
| #### Parameters | ||
| * Clip Limit: Maximum value a pixel can be adjusted. Higher values allow for noise to be over-amplified. | ||
| * Tile Rows: How many rows of tiles to break the image into. | ||
| * Tile Columns: How many columns of tiles to break the image into. | ||
|
|
||
|
|
||
| User Interface | ||
|  |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance CLAHE documentation for better user guidance
While the section provides a good overview, it needs several improvements to match the quality and format of the Histogram Equalization section:
- Missing step-by-step instructions for using CLAHE
- Parameter descriptions could be more detailed with recommended ranges
- Missing before/after example images to demonstrate the effect
- UI screenshot could use a caption or description
Consider applying these improvements:
### Contrast Limited Adaptive Histogram Equalization
Contrast Limited Adaptive Histogram Equalization (CLAHE) increases contrast by applying clipped histogram equalization to multiple tiles across the input image. In images where there are both very bright and very dark regions, this improves contrast in the dark regions without loosing contrast in the bright ones.
+ To apply CLAHE to your image:
+
+ 1. In the **OpenCV** menu, go to the **Image** tab
+ 2. Click on the **CLAHE** button
+ 3. Adjust the parameters as needed (see below)
+
#### Parameters
- * Clip Limit: Maximum value a pixel can be adjusted. Higher values allow for noise to be over-amplified.
- * Tile Rows: How many rows of tiles to break the image into.
- * Tile Columns: How many columns of tiles to break the image into.
+ * **Clip Limit** (Range: 1-10): Maximum value for contrast enhancement.
+ - Lower values (1-3) prevent noise amplification but provide subtle enhancement
+ - Higher values (4-10) give stronger enhancement but may amplify noise
+ * **Tile Rows** (Range: 2-16): Number of rows to divide the image into.
+ - More rows allow for more localized enhancement
+ - Recommended: Start with 8 rows and adjust based on image size
+ * **Tile Columns** (Range: 2-16): Number of columns to divide the image into.
+ - More columns allow for more localized enhancement
+ - Recommended: Start with 8 columns and adjust based on image size
-User Interface
+#### User Interface
+The CLAHE interface provides sliders to adjust each parameter in real-time:
+#### Example Results
+Below is an example showing the effect of CLAHE:
+Committable suggestion skipped: line range outside the PR's diff.
| - [OpenCV](#opencv) | ||
| - [Histogram Equalization](#opencv-histogram-equalization) | ||
| - [Contrast Limited Adaptive Histogram Equalization](#opencv-clahe) |
There was a problem hiding this comment.
Fix anchor tags in TOC to match section IDs
The anchor tags in the TOC links don't match the actual section IDs. This will cause broken navigation.
Apply this diff to fix the anchor tags:
- - [OpenCV](#opencv)
- - [Histogram Equalization](#opencv-histogram-equalization)
- - [Contrast Limited Adaptive Histogram Equalization](#opencv-clahe)
+ - [OpenCV](#opencv-1)
+ - [Histogram Equalization](#histogram-equalization)
+ - [Contrast Limited Adaptive Histogram Equalization](#contrast-limited-adaptive-histogram-equalization)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - [OpenCV](#opencv) | |
| - [Histogram Equalization](#opencv-histogram-equalization) | |
| - [Contrast Limited Adaptive Histogram Equalization](#opencv-clahe) | |
| - [OpenCV](#opencv-1) | |
| - [Histogram Equalization](#histogram-equalization) | |
| - [Contrast Limited Adaptive Histogram Equalization](#contrast-limited-adaptive-histogram-equalization) |
| onChange={(value) => { | ||
| this.setState({ | ||
| claheClipLimit: value || 40, | ||
| }); | ||
| enableImageFilter({ | ||
| modifier: openCVWrapper.imgproc.clahe(), | ||
| alias: ImageFilterAlias.CLAHE, | ||
| }, { | ||
| clipLimit: value || 40, | ||
| tileGridSize: { | ||
| columns: claheTileColumns, | ||
| rows: claheTileRows, | ||
| }, | ||
| }); | ||
| }} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Reduce code duplication in parameter update handlers
The parameter update logic is repeated across multiple onChange handlers. Consider extracting this into a shared method.
+ private updateCLAHEParameters(
+ paramName: 'claheClipLimit' | 'claheTileColumns' | 'claheTileRows',
+ value: number
+ ): void {
+ const defaultValues = {
+ claheClipLimit: 40,
+ claheTileColumns: 8,
+ claheTileRows: 8
+ };
+
+ this.setState({
+ [paramName]: value || defaultValues[paramName]
+ });
+
+ this.enableImageFilter({
+ modifier: openCVWrapper.imgproc.clahe(),
+ alias: ImageFilterAlias.CLAHE,
+ }, {
+ clipLimit: paramName === 'claheClipLimit' ? value : this.state.claheClipLimit,
+ tileGridSize: {
+ columns: paramName === 'claheTileColumns' ? value : this.state.claheTileColumns,
+ rows: paramName === 'claheTileRows' ? value : this.state.claheTileRows,
+ },
+ });
+ }
- onChange={(value) => {
- this.setState({
- claheClipLimit: value || 40,
- });
- enableImageFilter({
- modifier: openCVWrapper.imgproc.clahe(),
- alias: ImageFilterAlias.CLAHE,
- }, {
- clipLimit: value || 40,
- tileGridSize: {
- columns: claheTileColumns,
- rows: claheTileRows,
- },
- });
- }}
+ onChange={(value) => this.updateCLAHEParameters('claheClipLimit', value)}Also applies to: 685-699, 710-724
cvat-ui/src/components/annotation-page/standard-workspace/controls-side-bar/opencv-control.tsx
Show resolved
Hide resolved
| const arr = new Uint8ClampedArray(RGBADist.data, RGBADist.cols, RGBADist.rows); | ||
| const imgData = new ImageData(arr, src.width, src.height); |
There was a problem hiding this comment.
Correct the construction of Uint8ClampedArray
The Uint8ClampedArray constructor is incorrectly called with three arguments. It should be constructed directly from RGBADist.data.
Apply this fix:
- const arr = new Uint8ClampedArray(RGBADist.data, RGBADist.cols, RGBADist.rows);
+ const arr = new Uint8ClampedArray(RGBADist.data);This ensures that the array correctly represents the pixel data for the ImageData object.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const arr = new Uint8ClampedArray(RGBADist.data, RGBADist.cols, RGBADist.rows); | |
| const imgData = new ImageData(arr, src.width, src.height); | |
| const arr = new Uint8ClampedArray(RGBADist.data); | |
| const imgData = new ImageData(arr, src.width, src.height); |
| tileGridSize?: { width: number, height: number } | ||
| }): void { | ||
| if (options.clipLimit !== undefined) { | ||
| this.clipLimit = options.clipLimit; | ||
| } | ||
| if (options.tileGridSize !== undefined) { | ||
| this.tileGridSize = options.tileGridSize; | ||
| } |
There was a problem hiding this comment.
Fix mismatch in tileGridSize property names in configure method
In the configure method, tileGridSize uses width and height, but the class property tileGridSize expects rows and columns. This inconsistency could lead to incorrect behavior when configuring the CLAHE parameters.
Apply this diff to align the property names:
public configure(options: {
clipLimit?: number,
- tileGridSize?: { width: number, height: number }
+ tileGridSize?: { rows: number, columns: number }
}): void {
if (options.clipLimit !== undefined) {
this.clipLimit = options.clipLimit;
}
if (options.tileGridSize !== undefined) {
- this.tileGridSize = options.tileGridSize;
+ this.tileGridSize = options.tileGridSize;
}
}Alternatively, if you prefer to keep width and height, map them accordingly:
public configure(options: {
clipLimit?: number,
tileGridSize?: { width: number, height: number }
}): void {
if (options.clipLimit !== undefined) {
this.clipLimit = options.clipLimit;
}
if (options.tileGridSize !== undefined) {
this.tileGridSize = {
+ rows: options.tileGridSize.height,
+ columns: options.tileGridSize.width,
};
}
}Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
Hi, thanks for your contribution!
I have a few suggestions to improve this PR:
-
Add Padding Between Icons
- Let's add some padding between the icons for better spacing:
- Let's add some padding between the icons for better spacing:
-
Feature Settings Improvements
- Add padding between the settings for better readability.
- Align the settings to the left.
- Include a category label above them (e.g.,
CLAHE:) to specify what the settings refer to. This will make it easier to navigate if more filter settings are added in the future.
-
Check comments from coderabbit, some of them are applicable
| claheClipLimit: number; | ||
| claheTileColumns: number; | ||
| claheTileRows: number; |
There was a problem hiding this comment.
Lets create separate state object filterSettings where we can store all filter settings here.
| claheClipLimit: 40, | ||
| claheTileColumns: 8, | ||
| claheTileRows: 8, |
There was a problem hiding this comment.
I noticed that the constants are duplicated here and in clahe.ts. Would it be possible to import the defaults directly from clahe.ts to avoid redundancy?
| } | ||
| } | ||
|
|
||
| public processImage(src:ImageData, frameNumber: number) : ImageData { |
There was a problem hiding this comment.
Much of the code here is duplicated from histogram-equalization.ts. Could we create a base class to centralize the shared logic, particularly for the image processing functionality?
| hasFilter(filters, ImageFilterAlias.CLAHE) ? | ||
| 'cvat-opencv-clahe-tool-button cvat-opencv-image-tool-active' : 'cvat-opencv-clahe-tool-button' | ||
| } | ||
| onClick={(e: React.MouseEvent<HTMLElement>) => { |
There was a problem hiding this comment.
The code for enabling the filter (and for the histogram as well) could be extracted into a separate function to reduce duplication
Implement Contrast limited adaptive histogram equalization. This is similar to the existing histogram equalization tool but allows for locally boosting contrast.
The implementation is based on the existing histogram equalization, but added input fields for clip limit, tile rows, and tile columns.
Motivation and context
How has this been tested?
Manual testing
Checklist
developbranchI have increased versions of npm packages if it is necessary(cvat-canvas,
cvat-core,
cvat-data and
cvat-ui)
License
Feel free to contact the maintainers if that's a concern.
Summary by CodeRabbit
Release Notes
New Features
Documentation