refactor(mobile): cleanup iOS image loading pipeline#27672
Merged
alextran1502 merged 17 commits intomainfrom Apr 10, 2026
Merged
refactor(mobile): cleanup iOS image loading pipeline#27672alextran1502 merged 17 commits intomainfrom
alextran1502 merged 17 commits intomainfrom
Conversation
…ue for image processing
… and finish helpers
…moval in image processing
This was referenced Apr 9, 2026
LeLunZ
commented
Apr 9, 2026
|
|
||
| private static func handleCompletion(requestId: Int64, encoded: Bool, data: Data?, response: URLResponse?, error: Error?) { | ||
| guard let request = registry.remove(requestId: requestId) else { | ||
| guard let request = registry.get(requestId: requestId) else { |
Collaborator
Author
There was a problem hiding this comment.
here we had two issues:
- if request had been cancelled before the http part is finished, the request wouldn't be in the registry anymore and we would return here without invoking the callback
- if we didn't cancel we would here call remove and if now dart wants to cancel in the cancelRequest method, we can't get the request anymore as it has been removed from the registry
LeLunZ
commented
Apr 9, 2026
Comment on lines
+45
to
+48
| let request = RemoteImageRequest(id: requestId, task: task) { result in | ||
| Self.registry.remove(requestId: requestId) | ||
| completion(result) | ||
| } |
Collaborator
Author
There was a problem hiding this comment.
For remote images we can just say, we always call remove together with the completion
LeLunZ
commented
Apr 9, 2026
Comment on lines
+129
to
+126
| Self.registry.remove(requestId: requestId)?.cancel() | ||
| // For now don't call .remove here since we call completion in the handleCompletion method | ||
| Self.registry.get(requestId: requestId)?.cancel() |
Collaborator
Author
There was a problem hiding this comment.
This removal here caused issue 1. above
LeLunZ
commented
Apr 9, 2026
Comment on lines
+10
to
+12
| // only include if we can guarantee that for a queued operation that's | ||
| // cancelled we also call the callback with the cancelled result | ||
| // operation?.cancel() |
Collaborator
Author
There was a problem hiding this comment.
If a operation was queued but not yet run, this would stop it from starting, and invoking our cancellation callback.
Don't calling cancel runs the block, and fixes our issue
mertalev
reviewed
Apr 9, 2026
mertalev
approved these changes
Apr 10, 2026
eleboucher
pushed a commit
to eleboucher/homelab
that referenced
this pull request
Apr 10, 2026
…4) (#139) This PR contains the following updates: | Package | Update | Change | |---|---|---| | [immich-app/immich](https://github.com/immich-app/immich) | patch | `v2.7.3` → `v2.7.4` | --- ### Release Notes <details> <summary>immich-app/immich (immich-app/immich)</summary> ### [`v2.7.4`](https://github.com/immich-app/immich/releases/tag/v2.7.4) [Compare Source](immich-app/immich@v2.7.3...v2.7.4) ### v2.7.4 This release addresses some issues with image rendering on the mobile app #### What's Changed ##### 🐛 Bug fixes - refactor(mobile): cleanup iOS image loading pipeline by [@​LeLunZ](https://github.com/LeLunZ) in [#​27672](immich-app/immich#27672) - fix(server): hide original filename when not showing metadata by [@​meesfrensel](https://github.com/meesfrensel) in [#​27581](immich-app/immich#27581) - fix(mobile): fix Flutter cache eviction on thumbnails by [@​LeLunZ](https://github.com/LeLunZ) in [#​27663](immich-app/immich#27663) - chore: pump cronet version by [@​shenlong-tanwen](https://github.com/shenlong-tanwen) in [#​27685](immich-app/immich#27685) **Full Changelog**: <immich-app/immich@v2.7.3...v2.7.4> </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0My4xMDEuMSIsInVwZGF0ZWRJblZlciI6IjQzLjEwMS4xIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJyZW5vdmF0ZS9naXRodWItcmVsZWFzZSIsInR5cGUvcGF0Y2giXX0=--> Reviewed-on: https://git.erwanleboucher.dev/eleboucher/homelab/pulls/139 Co-authored-by: bot-owl <bot@erwanleboucher.dev> Co-committed-by: bot-owl <bot@erwanleboucher.dev>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Description
First this adds a ImageRequest base class, which holds the completion callback and the isCancelled property (isCancelled is thread safe)
Second this moves the encoded data block out of the BlockOperation of the remote image request. At this point it's only copying some data, so nothing resource intensive and worth starting an operation for.
This PR should fix all these issues:
Drawbacks:
This PR removes cancelling
BlockOperationsfor now. This is needed because a still queued but cancelled BlockOperation wouldn't launch a new thread, and thus wouldn't call our cancel completion callback.This is a performance regression, as now we the system has to dispatch the block operation just to afterwards check the
isCancelledflag.I plan to fix the regression, @mertalev and me are still in discussion on what's the best way.
This for now fixes all issues I could find.
How Has This Been Tested?
Checklist:
Please describe to which degree, if any, an LLM was used in creating this pull request.
used LLM for questions about the Swift Concurrency (@sendable etc.)