[CIS-2152] Add support for CDN v2#2339
Conversation
Generated by 🚫 Danger |
LateResize was not doing any actual resize, loading the full bytes of the image.
…bility to just download
2247a33 to
90b67e1
Compare
📏 Size AnalysisTotal install size 10.5 MB | This change: ⬆️ +33.3 kB (+0.317%)🗂 See size breakdown
🔎 See the full size analysis (2459d11) merging into develop (f065659)
|
martinmitrevski
left a comment
There was a problem hiding this comment.
Great work @nuno-vieira 👏
I've added some comments and I still need to do the manual QA. Also, please see why the checks are failing.
Additionally, let's revisit the breaking changes again.
Sources/StreamChatUI/ChatMessageList/Attachments/ChatMessageImageGallery+ImagePreview.swift
Show resolved
Hide resolved
c1f66d2 to
7183420
Compare
martinmitrevski
left a comment
There was a problem hiding this comment.
LGTM ✅ Also did some manual QA, everything works smoothly.
About the tuples thing, it's really up-to you, if you feel that's more intuitive, we can merge it like this.
polqf
left a comment
There was a problem hiding this comment.
Lovely to see this finally happening.
I just have one concern around braking changes and possible behavior changes.
Is there any way to keep the current APIs but with a deprecation warning instead of completely removing them? Also, is there any chance that the behavior changes for existing customers after these changes?
3d45409 to
1802b0b
Compare
martinmitrevski
left a comment
There was a problem hiding this comment.
great work 👏 ready to be merged IMO.
|
Kudos, SonarCloud Quality Gate passed! |
|
Did a round of QA, LGTM 🚢 |
| } | ||
|
|
||
| /// The formula to calculate the new resolution is based on the max pixels and | ||
| /// the original aspect ratio. To get the formula, a system of questions is needed. |
There was a problem hiding this comment.
System of equations 🙃 🤦♂️









🔗 Issue Links
CIS-2152
🎯 Goal
Use Stream's CDN v2 to improve the memory footprint of the application. Now, images are downloaded with a lower resolution instead of the original one.
📝 Summary
ImageLoadingAPIImageCDNwith some needed breaking changesComposerVC.addAttachmentToContent()has a newinfoproperty (minor breaking change)CGSize.avatarThumbnailSizeand moves it toComponents.default🛠 Implementation
ImageCDNrequired breaking changes so that the API is more understandable but also more scalable. The breaking changes are also minimal and will be documented in the changelog.🎨 Showcase
Since we are reducing the resolution of the loaded images, the memory footprint reduces a lot. Doing a test with a message list with about 16 images that have high resolution, the memory footprint is reduced by ~6x times.
After exporting the memory graph and analysing with
VVMap, we can see that especially the Image Raster data is much lower:The reason for these improvements is that for memory use the image resolution is the most important, not the file size. The memory is used by the decoding and decompression of the image which is dependent on the resolution.
For example, if we are talking about an image of 3300x4500, that will be 57MB! For only one image. Since each pixel takes up 4 bytes (if sRGB), we can calculate this with the following formula
3300 * 4500 * 4 / 1024 / 1024 = 56.6MB.🧪 Manual Testing Notes
☑️ Contributor Checklist
🎁 Meme