WIP: added color adjust feature #62
Draft
Conversation
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
Reviewer's GuideAdds a per-palette color intensity adjustment feature by introducing a new provider for managing color weights, wiring it through the widget tree, extending the processing pipeline (processImages, Epd and quantizers) to accept and apply those weights, enhancing the RemapQuantizer’s distance calculation, and providing a UI modal with sliders to tweak and apply weights in the ImageEditor. Sequence diagram for user adjusting color intensitiessequenceDiagram
actor User
participant ImageEditor
participant BottomActionMenu
participant ColorAdjustmentSliders
participant ColorAdjustmentProvider
participant processImages
participant RemapQuantizer
User->>BottomActionMenu: Tap 'Adjust Colors'
BottomActionMenu->>ColorAdjustmentSliders: Show modal with sliders
User->>ColorAdjustmentSliders: Adjust sliders, tap 'Apply'
ColorAdjustmentSliders->>ColorAdjustmentProvider: updateWeights(newWeights)
ColorAdjustmentProvider-->>ImageEditor: Notifies listeners
ImageEditor->>processImages: processImages(..., weights)
processImages->>RemapQuantizer: RemapQuantizer(..., weights)
RemapQuantizer-->>processImages: Processed image
processImages-->>ImageEditor: Updated images
Class diagram for ColorAdjustmentProvider and integrationclassDiagram
class ColorAdjustmentProvider {
- Map<Color, double> _weights
+ Map<Color, double> get weights
+ void resetWeights(List<Color> colors)
+ void updateWeights(Map<Color, double> newWeights)
}
class ImageEditor {
- Map<Color, double> _currentWeights
+ void _updateProcessedImages(img.Image? sourceImage, Map<Color, double> weights)
}
class BottomActionMenu {
+ void _showColorAdjustmentSheet(BuildContext context)
}
class ColorAdjustmentSliders {
- List<Color> colors
}
ColorAdjustmentProvider <|.. ImageEditor : used by
ImageEditor o-- BottomActionMenu : has
BottomActionMenu o-- ColorAdjustmentSliders : shows
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @Vishveshwara - I've reviewed your changes - here's some feedback:
- Avoid comparing
Map<Color, double>by callingtoString()—instead implement a proper deep‐equality check or maintain a hash/version so you can tell when weights actually change. - In
RemapQuantizer, building a newColorper pixel to look up weights is expensive—use integer channel values (e.g. the 32-bit color int) or a palette‐indexed weight list for O(1) access without object allocation. - You’re calling
resetWeights(_colors)in multiple callbacks (initState, import, edit, etc.); centralize initialization (e.g. default provider state or a single listener) to avoid duplication and reduce boilerplate.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Avoid comparing `Map<Color, double>` by calling `toString()`—instead implement a proper deep‐equality check or maintain a hash/version so you can tell when weights actually change.
- In `RemapQuantizer`, building a new `Color` per pixel to look up weights is expensive—use integer channel values (e.g. the 32-bit color int) or a palette‐indexed weight list for O(1) access without object allocation.
- You’re calling `resetWeights(_colors)` in multiple callbacks (initState, import, edit, etc.); centralize initialization (e.g. default provider state or a single listener) to avoid duplication and reduce boilerplate.
## Individual Comments
### Comment 1
<location> `lib/view/image_editor.dart:110` </location>
<code_context>
- _updateProcessedImages(imgLoader.image);
+ var colorAdjuster = context.watch<ColorAdjustmentProvider>();
+
+ if (imgLoader.image != null && colorAdjuster.weights.isEmpty) {
+ colorAdjuster.resetWeights(_colors);
+ }
+
</code_context>
<issue_to_address>
Possible race condition when resetting color weights.
Since the provider's state may not update synchronously, this logic could trigger multiple resets and cause UI issues. Move the reset to a more suitable lifecycle method or ensure the provider updates before the next build.
</issue_to_address>
### Comment 2
<location> `lib/provider/color_adjustment_provider.dart:8` </location>
<code_context>
+
+ Map<Color, double> get weights => _weights;
+
+ void resetWeights(List<Color> colors) {
+ _weights = {for (var color in colors) color: 1.0};
+ }
+
</code_context>
<issue_to_address>
resetWeights does not notify listeners.
Since notifyListeners() is not called after updating _weights, UI components depending on this data may not refresh. Please add notifyListeners() after resetting the weights.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+110
to
+111
| if (imgLoader.image != null && colorAdjuster.weights.isEmpty) { | ||
| colorAdjuster.resetWeights(_colors); |
There was a problem hiding this comment.
issue (bug_risk): Possible race condition when resetting color weights.
Since the provider's state may not update synchronously, this logic could trigger multiple resets and cause UI issues. Move the reset to a more suitable lifecycle method or ensure the provider updates before the next build.
Comment on lines
+8
to
+9
| void resetWeights(List<Color> colors) { | ||
| _weights = {for (var color in colors) color: 1.0}; |
There was a problem hiding this comment.
issue (bug_risk): resetWeights does not notify listeners.
Since notifyListeners() is not called after updating _weights, UI components depending on this data may not refresh. Please add notifyListeners() after resetting the weights.
|
@kienvo can you please review the logic for adjusting the colors via the remap quantizer? |
|
@kienvo @AsCress @Jhalakupadhyay Can you please review this ? is this approach good , since it is working through the quantizer . |
|
Modifying the original image would be a better and simpler approach. https://github.com/brendan-duncan/image/blob/main/doc/filters.md And something like real-time adjustment would be good, but I'm not sure if Flutter can be able to do this. |
I have already added a feature ,which adjusts the original image in this pr modyfying the original image is good but it doesn't have fine tune controls for having more (red,black,white) in the display, which I think will give a good control if we use displays supporting more colors, for example (red,black,white,yellow) and want yellow to be more dominant this feature would be good |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes #60
Screen Recording:
color.adjust.feature.mp4
Issues:
I would like someone to review this first , and suggest if it is a viable solution , and then I can update this PR
Summary by Sourcery
Implement a color adjustment feature by introducing a provider-driven weight model, a slider-based UI for per-color intensity control, and integrating those weights into the image processing pipeline’s quantizer while also offering manual editing via ProImageEditor.
New Features:
Enhancements: