Skip to content
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

feat(mobile): Remote thumbnails and images use an on-disk image cache #7929

Merged
merged 9 commits into from
Mar 14, 2024

Conversation

martyfuhry
Copy link
Contributor

@martyfuhry martyfuhry commented Mar 13, 2024

  • Fixed a few issues in the equals operator for thumbnails
  • Replaced all references of ImmichRemoteImageProvider using thumbnail: true with ImmichRemoteThumbnailProvider
  • Replaces the HTTP lookup with a ImageCacheManager.getImageFile which uses an on device cache
  • Custom cache uses 5000 thumbnails and 500 "full size" remote images with a stale period of 30 days

format

format

Fix typo in equals

remove unused import

renames image loader
Copy link

cloudflare-pages bot commented Mar 13, 2024

Deploying immich with  Cloudflare Pages  Cloudflare Pages

Latest commit: 96b7984
Status: ✅  Deploy successful!
Preview URL: https://83f0a77e.immich.pages.dev
Branch Preview URL: https://feat-on-disk-image-cache.immich.pages.dev

View logs

Copy link
Contributor

@fyfrey fyfrey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! This should improve the app experience substantially

}
}

throw Exception('No file found in cache');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I hope we never reach this exception. If we do, it's a bit misleading. This exception can also occur if the download failed I believe.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah this is one of those "this code will never execute" scenarios

😂

I largely copied the code from the CachedNetworkImage folks because their implementation worked so dang well, but they use a Stream and I only need a Future, so I needed to return something... What do you think we should do here? I could try to rework this into something more legible...

Copy link
Contributor

@fyfrey fyfrey Mar 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe just "could not load image ID"

@alextran1502
Copy link
Contributor

Amazing work! Thank you

@alextran1502 alextran1502 merged commit 582cdca into main Mar 14, 2024
24 checks passed
@alextran1502 alextran1502 deleted the feat/on-disk-image-cache branch March 14, 2024 20:29
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.

None yet

3 participants