-
Notifications
You must be signed in to change notification settings - Fork 372
refactor(ui): Optimize image thumbnail loading and caching #2444
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This commit refactors image thumbnail handling for better performance and efficiency. It introduces `ThumbnailSizeCalculator`, a utility that calculates the optimal dimensions for thumbnails based on the widget's constraints and the device's pixel ratio. This ensures images are decoded and cached at an appropriate size, preventing excessive memory usage from large images. Key changes: - **`StreamImageAttachmentThumbnail`** now uses `LayoutBuilder` to determine the available space and calculates the best thumbnail size. It also passes the calculated `memCacheWidth` and `memCacheHeight` to `CachedNetworkImage` for optimized caching. - **`UserAvatar`** and **`StreamChannelAvatar`** are updated to leverage this new mechanism, resizing avatar images to fit their display constraints, which improves performance, especially in lists. - **`GiphyAttachmentThumbnail`** and **`VideoAttachmentThumbnail`** are simplified to delegate to `StreamImageAttachmentThumbnail`, centralizing the image loading logic. - The direct usage of `CachedNetworkImage` in `GalleryFooter` is replaced with `StreamImageAttachmentThumbnail` for consistency.
WalkthroughIntroduces dynamic thumbnail sizing, cache-dimension propagation, and thumbnail-rendering refactors across image, video, giphy, avatar, and gallery components; adds ThumbnailSizeCalculator and tests; updates changelog and makes imageThumbnailSize nullable in StreamImageAttachment. Changes
Sequence Diagram(s)sequenceDiagram
participant Widget as Attachment/Avatar Widget
participant Layout as LayoutBuilder
participant Calc as ThumbnailSizeCalculator
participant Loader as StreamImageAttachmentThumbnail
participant Cache as Image Cache / Engine
Widget->>Layout: build → provide constraints
Layout->>Calc: calculate(originalSize?, targetSize, pixelRatio)
Calc-->>Layout: return effective thumbnail Size? (or null)
Layout->>Loader: render with url/file + cacheWidth/cacheHeight
Loader->>Cache: request image with memCacheWidth/memCacheHeight
Cache-->>Loader: deliver optimized image bytes
Loader-->>Widget: paint thumbnail
rect rgb(220,240,220)
Note over Calc,Cache: New: dynamic sizing and cache dims\nOld: fixed 400x400 → higher memory use
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (6)
packages/stream_chat_flutter/lib/src/channel/stream_channel_avatar.dart (1)
119-151: Guard cache dimensions against unbounded/zero constraintsThe new LayoutBuilder correctly ties
memCacheWidth/memCacheHeightto the rendered avatar size, but it assumesconstraints.biggestis finite and non‑zero. In more flexible layouts (e.g. missingconstraints/theme constraints, or unusual parents), this can yield0or infinite dimensions, which may trip assertions or cause unexpected behavior inCachedNetworkImage.You can make this more robust by clamping to positive, finite values and skipping
memCacheWidth/memCacheHeightwhen they’re not valid, e.g.:final devicePixel = MediaQuery.devicePixelRatioOf(context); final biggest = constraints.biggest; final scaled = biggest * devicePixel; int? cacheWidth; int? cacheHeight; if (scaled.width.isFinite && scaled.height.isFinite && scaled.width > 0 && scaled.height > 0) { cacheWidth = scaled.width.round(); cacheHeight = scaled.height.round(); } return CachedNetworkImage( imageUrl: channelImage, memCacheWidth: cacheWidth, memCacheHeight: cacheHeight, errorWidget: (_, __, ___) => fallbackWidget, fit: BoxFit.cover, );Also, if
channel.image/imageStreamcan ever benullat runtime, consider guardingchannelImage.isEmptyaccordingly.packages/stream_chat_flutter/lib/src/attachment/thumbnail/video_attachment_thumbnail.dart (1)
3-4: Tighten thumbnail availability check and confirmStreamImageAttachmentThumbnailsemanticsThe new
thumbUrlpath is a nice simplification, and reusingStreamImageAttachmentThumbnailshould help with consistent sizing/caching.Two small follow‑ups:
- Consider treating only non‑empty URLs as “has thumbnail” to avoid passing an empty string through:
final containsThumbnail = (video.thumbUrl ?? '').isNotEmpty; if (containsThumbnail) { return StreamImageAttachmentThumbnail( image: video, width: width, height: height, fit: fit, errorBuilder: errorBuilder, ); }
- Please double‑check that
StreamImageAttachmentThumbnailcorrectly handlesAttachmentType.videoby usingthumbUrlrather thanimageUrl, so we don’t regress any cases where videos relied on their thumbnail.Also applies to: 57-67
packages/stream_chat_flutter/lib/src/attachment/thumbnail/giphy_attachment_thumbnail.dart (1)
58-68: Giphy now correctly reuses the shared image thumbnail pipelineUsing
giphy.giphyInfo(type)and delegating toStreamImageAttachmentThumbnailkeeps sizing, caching, and error handling consistent with other attachments. This looks good and should simplify future maintenance. The only subtle dependency is thatAttachment.copyWith(imageUrl: info?.url)relies oncopyWithpreserving the existingimageUrlwheninfo?.urlis null; if that ever changes, switching toimage: giphy.copyWith(imageUrl: info?.url ?? giphy.imageUrl)would make the fallback explicit.packages/stream_chat_flutter/test/src/attachment/thumbnail/thumbnail_size_calculator_test.dart (1)
1-272: ThumbnailSizeCalculator test coverage is strongThe tests exercise the critical paths (null/invalid inputs, finite vs infinite constraints, a variety of aspect ratios, and multiple pixel ratios) and closely mirror the intended algorithm. This should give good confidence that sizing and scaling behave as expected. If you want even more robustness, you could add a couple of edge tests for degenerate inputs (e.g.,
pixelRatio <= 0ortargetSizewith a zero dimension) to lock in behavior there as well.packages/stream_chat_flutter/lib/src/attachment/thumbnail/thumbnail_size_calculator.dart (1)
1-78: Aspect‑ratio–preserving size calculation looks correct; consider tightening a couple of edge casesThe core algorithm (handling infinities, preserving aspect ratio, and then applying
pixelRatio) is sound and aligns with the tests. A couple of small refinements could make it more robust:
- Treat obviously invalid
pixelRatiovalues (e.g.,<= 0) as a signal to skip optimization and returnnull.- After computing
thumbnailWidth/thumbnailHeight, bail out if either ends up<= 0instead of returning a zero‑sizedSize, which would later yieldcacheWidth/cacheHeightof0.For example:
static Size? calculate({ Size? originalSize, required Size targetSize, required double pixelRatio, }) { + if (pixelRatio <= 0) return null; @@ - // Calculate size that maintains aspect ratio within constraints - final targetAspectRatio = thumbnailWidth / thumbnailHeight; + // Calculate size that maintains aspect ratio within constraints + final targetAspectRatio = thumbnailWidth / thumbnailHeight; @@ - // Apply pixel ratio to get physical pixel dimensions - return Size(thumbnailWidth * pixelRatio, thumbnailHeight * pixelRatio); + if (thumbnailWidth <= 0 || thumbnailHeight <= 0) { + return null; + } + + // Apply pixel ratio to get physical pixel dimensions + return Size(thumbnailWidth * pixelRatio, thumbnailHeight * pixelRatio); }This keeps behavior unchanged for normal cases but avoids propagating obviously unusable sizes.
packages/stream_chat_flutter/lib/src/attachment/thumbnail/image_attachment_thumbnail.dart (1)
75-135: Layout‑based sizing + ThumbnailSizeCalculator integration achieves the intended memory optimizationThe
LayoutBuilder+ThumbnailSizeCalculatorflow and the unified remote/local handling look well thought through:
effectiveThumbnailSizeis computed once per build, withthumbnailSizeoverriding the calculated size, and falls back cleanly whenoriginalSizeor constraints don’t allow optimization.- Using
image.thumbUrl ?? image.imageUrlprioritizes server‑side thumbnails while still handling plain image URLs.- Passing
cacheWidth/cacheHeightderived fromeffectiveThumbnailSizeinto both remote and local paths ensures decode sizes are aligned with layout, which should significantly reduce memory pressure in galleries.Two minor, defensive tweaks you might consider:
- If
MediaQuery.devicePixelRatioOf(context)isn’t available (e.g., in certain test setups or if you ever support older Flutter versions), defaulting to1.0would make this more resilient.- If
effectiveThumbnailSizeever ends up with non‑positive dimensions (e.g., due to odd constraints), it may be safer to treat that the same asnulland skip settingcacheWidth/cacheHeightand URL resizing rather than passing zeros through.Overall, this is a solid step toward predictable thumbnail sizing and caching.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (10)
packages/stream_chat_flutter/CHANGELOG.md(1 hunks)packages/stream_chat_flutter/lib/src/attachment/image_attachment.dart(2 hunks)packages/stream_chat_flutter/lib/src/attachment/thumbnail/giphy_attachment_thumbnail.dart(1 hunks)packages/stream_chat_flutter/lib/src/attachment/thumbnail/image_attachment_thumbnail.dart(7 hunks)packages/stream_chat_flutter/lib/src/attachment/thumbnail/thumbnail_size_calculator.dart(1 hunks)packages/stream_chat_flutter/lib/src/attachment/thumbnail/video_attachment_thumbnail.dart(2 hunks)packages/stream_chat_flutter/lib/src/avatars/user_avatar.dart(3 hunks)packages/stream_chat_flutter/lib/src/channel/stream_channel_avatar.dart(1 hunks)packages/stream_chat_flutter/lib/src/gallery/gallery_footer.dart(1 hunks)packages/stream_chat_flutter/test/src/attachment/thumbnail/thumbnail_size_calculator_test.dart(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-25T08:19:01.469Z
Learnt from: xsahil03x
Repo: GetStream/stream-chat-flutter PR: 2394
File: packages/stream_chat_flutter/lib/src/message_action/message_actions_builder.dart:82-92
Timestamp: 2025-09-25T08:19:01.469Z
Learning: In the Stream Chat Flutter library, when deleting a message with MessageSendingStatus.failed or MessageSendingStatus.failed_update status, the _deleteMessage method in channel.dart automatically handles deletion locally via _deleteLocalMessage without making API calls, preventing 404 errors and deletingFailed states.
Applied to files:
packages/stream_chat_flutter/lib/src/attachment/thumbnail/image_attachment_thumbnail.dart
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: analyze_legacy_versions
- GitHub Check: build (android)
- GitHub Check: build (ios)
- GitHub Check: test
- GitHub Check: stream_chat_localizations
- GitHub Check: stream_chat_flutter_core
- GitHub Check: stream_chat_persistence
- GitHub Check: stream_chat_flutter
🔇 Additional comments (5)
packages/stream_chat_flutter/lib/src/gallery/gallery_footer.dart (1)
240-249: Centralizing grid thumbnails onStreamImageAttachmentThumbnaillooks goodSwitching the grid item from a raw
CachedNetworkImagetoStreamImageAttachmentThumbnailkeeps the existing layout/gesture behavior while reusing the new thumbnail sizing/caching logic. This is a good alignment with the rest of the refactor.packages/stream_chat_flutter/CHANGELOG.md (1)
16-18: Changelog entry clearly documents the memory fixThe new bullet explicitly ties the high‑memory fix to image attachments and links back to #2228, which nicely documents the behavior change for consumers.
packages/stream_chat_flutter/lib/src/attachment/thumbnail/image_attachment_thumbnail.dart (2)
138-191: Local image attachments now respect cache dimensions consistentlyThreading
cacheWidth/cacheHeightinto_LocalImageAttachmentand forwarding them to bothImage.memoryandImage.filealigns local image decoding with the same sizing logic used for remote images. That should avoid decoding full‑resolution local photos when they’re shown as thumbnails and materially reduce per‑image memory usage without changing layout behavior.
194-245: Remote image attachments correctly use memCacheWidth/memCacheHeightPropagating
cacheWidth/cacheHeightintoCachedNetworkImage’smemCacheWidth/memCacheHeight, and optionally resizing the URL viagetResizedImageUrlwhen an effective thumbnail size is available, neatly aligns server‑side resizing with client‑side decode constraints. This should directly address the high‑memory gallery scenarios while keeping placeholders and error handling unchanged.packages/stream_chat_flutter/lib/src/attachment/image_attachment.dart (1)
15-35: MakingimageThumbnailSizenullable aligns this API with the new sizing strategyRemoving the hard‑coded
const Size(400, 400)default and makingimageThumbnailSizenullable so that it simply passes through toStreamImageAttachmentThumbnail.thumbnailSizeis consistent with the new layout‑driven sizing. Callers that never customizedimageThumbnailSizewill now get automatically computed sizes instead of a fixed 400×400 cache target, which is desirable for memory. It’s worth double‑checking any internal usages that relied on that specific default, but from an API perspective this change looks coherent.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #2444 +/- ##
==========================================
+ Coverage 64.53% 64.60% +0.07%
==========================================
Files 419 420 +1
Lines 26196 26210 +14
==========================================
+ Hits 16905 16934 +29
+ Misses 9291 9276 -15 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This commit adds a check to ensure `thumbnailSize` is finite and not empty before calculating `cacheWidth` and `cacheHeight` for `UserAvatar` and `StreamChannelAvatar`. Previously, if the avatar widget was rendered with unconstrained dimensions, `thumbnailSize` could be infinite, leading to a crash when `round()` was called. This change prevents the crash by only setting `cacheWidth` and `cacheHeight` for `CachedNetworkImage` if the constraints are finite.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/stream_chat_flutter/lib/src/avatars/user_avatar.dart (1)
80-141: Avatar thumbnail still double‑decodes and doesn’t use cache dimensions forCachedNetworkImageLines 108–116 compute
cacheWidth/cacheHeightbut they’re only used insideResizeImageinimageBuilder, not passed toCachedNetworkImageitself. Combined withCachedNetworkImage’s own decoding, this likely causes:
- A full‑resolution decode for the cached image.
- A second decode at avatar size via
ResizeImage.That works functionally but undercuts the PR’s memory‑reduction goal and can keep both large and resized bitmaps in memory.
You can simplify the pipeline and align it with the thumbnail strategy used elsewhere in the PR by:
- Guarding the computed size per dimension (finite and > 0).
- Passing those dimensions directly into
CachedNetworkImageviamemCacheWidth/memCacheHeight.- Removing
ResizeImageso the cached, correctly‑sized image is used directly.- Ensuring
effectiveBorderRadiusis always non‑null where needed.Example patch:
- // Calculate optimal thumbnail size for the avatar - final devicePixelRatio = MediaQuery.devicePixelRatioOf(context); - final thumbnailSize = constraints.biggest * devicePixelRatio; - - int? cacheWidth, cacheHeight; - if (thumbnailSize.isFinite && !thumbnailSize.isEmpty) { - cacheWidth = thumbnailSize.width.round(); - cacheHeight = thumbnailSize.height.round(); - } + // Calculate optimal thumbnail size for the avatar. + final devicePixelRatio = MediaQuery.devicePixelRatioOf(context); + final thumbnailSize = constraints.biggest * devicePixelRatio; + + int? cacheWidth, cacheHeight; + if (thumbnailSize.width.isFinite && + thumbnailSize.height.isFinite && + thumbnailSize.width > 0 && + thumbnailSize.height > 0) { + cacheWidth = thumbnailSize.width.round(); + cacheHeight = thumbnailSize.height.round(); + } @@ - return CachedNetworkImage( - fit: BoxFit.cover, - filterQuality: FilterQuality.high, - imageUrl: imageUrl, - errorWidget: (_, __, ___) => backupGradientAvatar, - placeholder: switch (effectivePlaceholder) { - final holder? => (context, __) => holder(context, user), - _ => null, - }, - imageBuilder: (context, imageProvider) => DecoratedBox( - decoration: BoxDecoration( - borderRadius: effectiveBorderRadius, - image: DecorationImage( - fit: BoxFit.cover, - image: ResizeImage( - imageProvider, - width: cacheWidth, - height: cacheHeight, - ), - ), - ), - ), - ); + return CachedNetworkImage( + fit: BoxFit.cover, + filterQuality: FilterQuality.high, + imageUrl: imageUrl, + memCacheWidth: cacheWidth, + memCacheHeight: cacheHeight, + errorWidget: (_, __, ___) => backupGradientAvatar, + placeholder: switch (effectivePlaceholder) { + final holder? => (context, __) => holder(context, user), + _ => null, + }, + imageBuilder: (context, imageProvider) => DecoratedBox( + decoration: BoxDecoration( + borderRadius: effectiveBorderRadius ?? BorderRadius.zero, + image: DecorationImage( + fit: BoxFit.cover, + image: imageProvider, + ), + ), + ), + );If the new
ThumbnailSizeCalculatorutility in this PR is intended to be generic, consider reusing it here instead of open‑coding the size math so attachments and avatars stay in sync.
🧹 Nitpick comments (1)
packages/stream_chat_flutter/lib/src/avatars/user_avatar.dart (1)
100-152: Avoid repeating the constraints expression for base and selected avatarsLines 100 and 151 both resolve
constraints ?? avatarTheme?.constraints. Not a problem, but you could simplify by computing aneffectiveConstraintsonce and reusing it:- Widget avatar = FittedBox( + final effectiveConstraints = constraints ?? avatarTheme?.constraints; + + Widget avatar = FittedBox( fit: BoxFit.cover, child: Container( - constraints: constraints ?? avatarTheme?.constraints, + constraints: effectiveConstraints, @@ if (selected) { avatar = ClipRRect( @@ child: Container( - constraints: constraints ?? avatarTheme?.constraints, + constraints: effectiveConstraints,This keeps the layout logic in one place and makes it easier to adjust avatar sizing later.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
packages/stream_chat_flutter/lib/src/avatars/user_avatar.dart(3 hunks)packages/stream_chat_flutter/lib/src/channel/stream_channel_avatar.dart(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/stream_chat_flutter/lib/src/channel/stream_channel_avatar.dart
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: analyze_legacy_versions
- GitHub Check: stream_chat_flutter
- GitHub Check: stream_chat_flutter_core
- GitHub Check: stream_chat_localizations
- GitHub Check: stream_chat_persistence
- GitHub Check: build (android)
- GitHub Check: stream_chat
- GitHub Check: build (ios)
- GitHub Check: test
Brazol
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Submit a pull request
Fixes: #2228
Description of the pull request
This PR refactors image thumbnail handling for better performance and efficiency.
It introduces
ThumbnailSizeCalculator, a utility that calculates the optimal dimensions for thumbnails based on the widget's constraints and the device's pixel ratio. This ensures images are decoded and cached at an appropriate size, preventing excessive memory usage from large images.Key changes:
StreamImageAttachmentThumbnailnow usesLayoutBuilderto determine the available space and calculates the best thumbnail size. It also passes the calculatedmemCacheWidthandmemCacheHeighttoCachedNetworkImagefor optimized caching.UserAvatarandStreamChannelAvatarare updated to leverage this new mechanism, resizing avatar images to fit their display constraints, which improves performance, especially in lists.GiphyAttachmentThumbnailandVideoAttachmentThumbnailare simplified to delegate toStreamImageAttachmentThumbnail, centralizing the image loading logic.CachedNetworkImageinGalleryFooteris replaced withStreamImageAttachmentThumbnailfor consistency.Summary by CodeRabbit
Bug Fixes
Improvements
Tests
Documentation