Conversation
There was a problem hiding this comment.
Pull request overview
This pull request adds a border cropping feature for the manga reader that allows users to automatically trim white and black borders from manga pages. The feature is implemented as a toggleable setting that users can enable or disable.
Changes:
- Adds new
image_cropper.dartutility with cropping logic and custom image widget - Implements UI toggle in reader settings for crop borders feature
- Updates reader view to conditionally use cropped images based on user preference
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 11 comments.
| File | Description |
|---|---|
| lib/utils/image_cropper.dart | New utility implementing image fetching, border detection/cropping algorithm, and custom CroppedNetworkImage widget with in-memory caching |
| lib/screens/manga/widgets/reader/settings_view.dart | Adds "Crop Borders" toggle switch in reader settings panel |
| lib/screens/manga/widgets/reader/reader_view.dart | Conditionally renders CroppedNetworkImage or ExtendedImage based on cropImages setting |
| lib/screens/manga/controller/reader_controller.dart | Adds cropImages state variable and persistence logic for the setting |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
|
|
||
| class _CroppedNetworkImageState extends State<CroppedNetworkImage> { | ||
| static final Map<String, Uint8List> _cache = {}; |
There was a problem hiding this comment.
The static cache map in _CroppedNetworkImageState can grow indefinitely as images are loaded, potentially causing memory issues. Each cropped image is stored in memory without any size limit or eviction policy. Consider implementing a cache size limit with an LRU (Least Recently Used) eviction strategy, or using a more sophisticated caching solution like flutter_cache_manager.
| img.Image _applyCrop(img.Image image, {required bool isWhite}) { | ||
| int width = image.width; | ||
| int height = image.height; | ||
| int left = 0; | ||
| int top = 0; | ||
| int right = width - 1; | ||
| int bottom = height - 1; | ||
| int threshold = 10; | ||
|
|
||
| bool isPixelContent(int x, int y) { | ||
| var pixel = image.getPixel(x, y); | ||
| num r = pixel.r; | ||
| num g = pixel.g; | ||
| num b = pixel.b; | ||
| num brightness = r + g + b; | ||
|
|
||
| if (isWhite) { | ||
| return brightness < (765 - (threshold * 3)); | ||
| } else { | ||
| return brightness > (threshold * 3); | ||
| } | ||
| } | ||
|
|
||
| for (int x = 0; x < width; x++) { | ||
| bool stop = false; | ||
| for (int y = 0; y < height; y++) { | ||
| if (isPixelContent(x, y)) { | ||
| left = x; | ||
| stop = true; | ||
| break; | ||
| } | ||
| } | ||
| if (stop) break; | ||
| } | ||
|
|
||
| for (int x = width - 1; x >= left; x--) { | ||
| bool stop = false; | ||
| for (int y = 0; y < height; y++) { | ||
| if (isPixelContent(x, y)) { | ||
| right = x; | ||
| stop = true; | ||
| break; | ||
| } | ||
| } | ||
| if (stop) break; | ||
| } | ||
|
|
||
| for (int y = 0; y < height; y++) { | ||
| bool stop = false; | ||
| for (int x = left; x <= right; x++) { | ||
| if (isPixelContent(x, y)) { | ||
| top = y; | ||
| stop = true; | ||
| break; | ||
| } | ||
| } | ||
| if (stop) break; | ||
| } | ||
|
|
||
| for (int y = height - 1; y >= top; y--) { | ||
| bool stop = false; | ||
| for (int x = left; x <= right; x++) { | ||
| if (isPixelContent(x, y)) { | ||
| bottom = y; | ||
| stop = true; | ||
| break; | ||
| } | ||
| } | ||
| if (stop) break; | ||
| } | ||
|
|
||
| if (left > 0 || top > 0 || right < width - 1 || bottom < height - 1) { | ||
| int w = right - left + 1; | ||
| int h = bottom - top + 1; | ||
| if (w > 0 && h > 0) { | ||
| return img.copyCrop(image, x: left, y: top, width: w, height: h); | ||
| } | ||
| } | ||
|
|
||
| return image; |
There was a problem hiding this comment.
The image cropping operation is performed synchronously for every image load without timeouts. For large manga pages, the nested loops in _applyCrop can be computationally expensive (O(width * height) in worst case for each border detection), potentially blocking the UI and causing performance issues. Consider adding a timeout mechanism or optimizing the algorithm to sample pixels instead of checking every pixel.
|
|
||
| image = _applyCrop(image, isWhite: false); | ||
|
|
||
| return img.encodePng(image); |
There was a problem hiding this comment.
The image is re-encoded to PNG format regardless of the original format (line 126). This can significantly increase file size and memory usage for images that were originally JPEG or WebP, which are common formats for manga pages. Consider preserving the original format or choosing an appropriate format based on the image content.
| return img.encodePng(image); | |
| // Choose an appropriate output format based on image content: | |
| // - If the image has an alpha channel, keep PNG to preserve transparency. | |
| // - Otherwise, use JPEG to avoid unnecessary file size inflation. | |
| if (image.hasAlpha) { | |
| return img.encodePng(image); | |
| } else { | |
| return img.encodeJpg(image, quality: 90); | |
| } |
| return FutureBuilder<Uint8List>( | ||
| future: _futureBytes, | ||
| builder: (context, snapshot) { | ||
| if (snapshot.connectionState != ConnectionState.done) { | ||
| return widget.placeholder ?? const SizedBox.shrink(); | ||
| } | ||
| if (snapshot.hasData && | ||
| snapshot.data != null && | ||
| snapshot.data!.isNotEmpty) { | ||
| return Image.memory( | ||
| snapshot.data!, | ||
| width: widget.width, | ||
| height: widget.height, | ||
| fit: widget.fit, | ||
| alignment: widget.alignment, | ||
| ); | ||
| } | ||
| // Fallback: show nothing or a placeholder | ||
| return widget.placeholder ?? const SizedBox.shrink(); | ||
| }, |
There was a problem hiding this comment.
The FutureBuilder does not handle error states explicitly. If _loadBytes() throws an exception, the FutureBuilder will show hasData as false but won't distinguish between a loading state and an error state. Consider checking snapshot.hasError to provide appropriate error feedback to users.
| } catch (_) { | ||
| // Fall through to return empty/failed bytes |
There was a problem hiding this comment.
The catch block silently swallows all exceptions without logging or providing specific error information. This makes debugging network failures, parsing errors, or other issues difficult. Consider logging the error or providing more specific error handling to distinguish between different failure types (network errors, malformed URLs, image decoding failures, etc.).
| } catch (_) { | |
| // Fall through to return empty/failed bytes | |
| } catch (e, stackTrace) { | |
| // Log the error for debugging while still falling back to empty bytes. | |
| debugPrint('fetchAndCropImageBytes failed for URL "$url": $e'); | |
| debugPrint('$stackTrace'); |
| ? CroppedNetworkImage( | ||
| url: page.url, | ||
| headers: (page.headers?.isEmpty ?? true) | ||
| ? { | ||
| 'Referer': | ||
| sourceController.activeMangaSource.value?.baseUrl ?? | ||
| '' | ||
| } | ||
| : page.headers, | ||
| fit: BoxFit.contain, | ||
| alignment: Alignment.center, | ||
| ) |
There was a problem hiding this comment.
CroppedNetworkImage lacks loading and error state handling, unlike ExtendedImage.network which has comprehensive loadStateChanged callbacks. When cropping is enabled, users won't see progress indicators during loading or error messages with retry options when images fail to load. This creates an inconsistent user experience. Consider adding similar loading and error state widgets.
| ? CroppedNetworkImage( | ||
| url: page.url, | ||
| headers: (page.headers?.isEmpty ?? true) | ||
| ? { | ||
| 'Referer': sourceController | ||
| .activeMangaSource.value?.baseUrl ?? | ||
| '' | ||
| } | ||
| : page.headers, | ||
| fit: BoxFit.contain, | ||
| alignment: Alignment.center, | ||
| ) |
There was a problem hiding this comment.
CroppedNetworkImage lacks loading and error state handling, unlike ExtendedImage.network which has comprehensive loadStateChanged callbacks. When cropping is enabled, users won't see progress indicators during loading or error messages with retry options when images fail to load. This creates an inconsistent user experience. Consider adding similar loading and error state widgets.
@Copilot "The redundant .toString() call on line 69 has no effect since the expression is already a String. The map operation already produces a String via join(), so calling toString() on it is unnecessary. This line can be simplified to just use the result of join(';') directly or use an empty string as the null-coalescing fallback."
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@Copilot "The HTTP request in fetchAndCropImageBytes lacks timeout configuration. Network requests without timeouts can hang indefinitely, especially on slow or unreliable connections. Consider adding a timeout parameter to the http.get call to prevent indefinite waiting and provide a better user experience." Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Pull Request
Title:
Manga Border Cropper
Description:
Add a new setting to toggle on/off border cropping for the manga reader. (Actual image cropping functionality mad by NITHINSPACETIME)
Summary of Changes:
Toggle for image cropping
Type of Changes:
Screen.Recording.2026-01-21.114243.mp4
Testing Notes:
Android Emulator
Linked Issue(s):
https://discord.com/channels/1303000390505336893/1436297600356843581
Additional Context:
CREDITS TO ORIGINAL COMMIT BY https://github.com/NITHINSPACETIME
aayush2622/Dartotsu#136
https://github.com/aayush2622/Dartotsu/blob/99596986956d17b1985df9c5be737f48c00a4167/lib/Functions/ImageCropper.dart
Submission Checklist: