Skip to content

fix(mobile): revert cache fixes #17786

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

Merged
merged 2 commits into from
Apr 22, 2025
Merged

fix(mobile): revert cache fixes #17786

merged 2 commits into from
Apr 22, 2025

Conversation

alextran1502
Copy link
Contributor

@alextran1502
Copy link
Contributor Author

@EinToni @lukaszwawrzyk

During my test round for release, I identified issues when the two PRs are combined.

  1. When navigating to the Backup Page > Album Selection, clicking on the button to view the assets in the album displays the same asset thumbnail for all assets.
  2. After scrolling the timeline up and down, then entering the detail view to view local assets, swiping left and right a couple of times. I then sign out and sign back in the same instance with a different account. The cached thumbnail is now displayed incorrectly, as in showing the thumbnail of a different asset rather than the one it should represent.

I think this issue will take some time to investigate and come up with the right solution, so I revert these two PRs to prepping for the release

@alextran1502 alextran1502 changed the title fix(mobile): revert cache fix fix(mobile): revert cache fixes Apr 22, 2025
@alextran1502 alextran1502 merged commit 0986a71 into main Apr 22, 2025
48 of 49 checks passed
@alextran1502 alextran1502 deleted the fix-cache-issue branch April 22, 2025 17:15
@lukaszwawrzyk
Copy link
Contributor

lukaszwawrzyk commented Apr 22, 2025

@alextran1502 That's interesting. Is it confirmed that 2 PRs conflict cause problems together or it might just be that only one of them is problematic?

@alextran1502
Copy link
Contributor Author

@lukaszwawrzyk they caused the two issue I describe. Your PR caused the issue with the thumbnails render the same asset in the album selection view, and the other with a custom cache implementation caused issue of incorrect thumbnail displayed

@EinToni
Copy link
Contributor

EinToni commented Apr 22, 2025

@alextran1502 Can it be can the local id of an asset can be equal on two accounts? So Image A could have the local ID 001 on account X and Image B has also the local ID 001 but on account B.
That would explain the behavior, as the key for the cached thumbnails is "localID_{width} x{height}".

@alextran1502
Copy link
Contributor Author

alextran1502 commented Apr 22, 2025

@EinToni This is on the same device, so yes, the localId should be similar on both accounts. And it should refer to the same asset

I reimplemented the mechanism but without a custom cache and it seems to work well as well #17787

@EinToni
Copy link
Contributor

EinToni commented Apr 22, 2025

@alextran1502 It would be great to have to actual functionality with the caching. If you can wait until tomorrow I would he happy to open another PR with my changes and the caching key adapted to use the user id as well. Then this behavor will be fixed.

@alextran1502
Copy link
Contributor Author

@EinToni I am planning for a release tomorrow afternoon CTS timezone. If you can put it in before that, I can help with testing

@EinToni
Copy link
Contributor

EinToni commented Apr 22, 2025

@alextran1502 Okay thank you I opened another PR with the respective changes. I'm in UTC+2 so I'll go to bed now, but will do all I can to get this merged tomorrow :D

@lukaszwawrzyk
Copy link
Contributor

@alextran1502 #17794 I fixed my PR too, I tested my scenario and scenario that you reported as broken and it seems to work fine.

shenlong-tanwen pushed a commit that referenced this pull request Jun 8, 2025
* Revert "fix(mobile): use immutable cache keys for local images (#17736)"

This reverts commit 010b144.

* Revert "perf(mobile): remove small thumbnail and cache generated thumbnails (#17682)"

This reverts commit b71039e.
savely-krasovsky pushed a commit to savely-krasovsky/immich that referenced this pull request Jun 8, 2025
* Revert "fix(mobile): use immutable cache keys for local images (immich-app#17736)"

This reverts commit 010b144.

* Revert "perf(mobile): remove small thumbnail and cache generated thumbnails (immich-app#17682)"

This reverts commit b71039e.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants