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

fix(mobile): Multipart image loading for iOS double swipe #7064

Merged
merged 6 commits into from
Feb 12, 2024

Conversation

martyfuhry
Copy link
Contributor

No description provided.

@martyfuhry martyfuhry changed the title Fix/ios double swipe fix(mobile): Multipart image loading for iOS double swipe Feb 12, 2024
// TODO: Use local preview
// Load a small thumbnail
final thumbBytes = await asset.local?.thumbnailDataWithSize(
const ThumbnailSize.square(2000),
Copy link
Contributor

Choose a reason for hiding this comment

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

If we reuse the identical settings (default) add for thumbs on the grid, this will share the cached image because it's identical and super fast (already loaded for the grid view...)

Otherwise if this is too small: at least make the dimensions multiples of 16 (image codecs usually work in batches of 8x8 or 16x16) and set the quality to 80 (100 wastes a lot of space for almost identical visual quality as photo manager stores these thumbnails on the device)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How does 256 with 80 quality sound?

Copy link
Contributor

Choose a reason for hiding this comment

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

Perfect to me. However... The default is 250? for dumb reasons. So, we should optimally stick to whatever is currently specified in ImmichImage class

quality: 100,
);
if (thumbBytes == null) {
throw StateError("Loading thumb for ${asset.fileName} failed");
Copy link
Contributor

Choose a reason for hiding this comment

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

No need to error. We can simply try the original still..?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point.

Copy link

cloudflare-workers-and-pages bot commented Feb 12, 2024

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 6438d51
Status: ✅  Deploy successful!
Preview URL: https://160d5149.immich.pages.dev
Branch Preview URL: https://fix-ios-double-swipe.immich.pages.dev

View logs

@alextran1502 alextran1502 merged commit ea293df into refactor/immich-image-provider Feb 12, 2024
16 checks passed
@alextran1502 alextran1502 deleted the fix/ios-double-swipe branch February 12, 2024 22:09
alextran1502 added a commit that referenced this pull request Feb 13, 2024
* Adds image provider

* uses image provider

* wip load preview

* wip everything but activity asset thumbnail needs some help with a remote id

* Immich provider used in gallery

* First draft of the immich image provider, working nicely!

* Removed OriginalImageProvider

* Fixes for thumbnails

* feat(mobile): thumbhash support (#7028)

* feat(mobile): thumbhash support

* perf(mobile): store bmp thumbhash bytes in Isar

---------

Co-authored-by: shenlong-tanwen <[email protected]>

* Uses octoimage for fade in and placeholders

* fixes thumbnails, removes unused values, adds better thumbnail size

* removes thumbhash support for now

* Forgot one thumbhash removal

* Use big thumbnail for local image on ios

* fix(mobile): Multipart image loading for iOS double swipe (#7064)

* uses local thumb first

* Multipart thumbnail

* Clean up file delete

* await file delete

* Fynn's comments, made thumbnail smaller and doesn't crash on erroring out on thumbnail

* lint

---------

Co-authored-by: Marty Fuhry <[email protected]>
Co-authored-by: Alex <[email protected]>

* Moves http client to global private place for reuse

* Got rid of usePreview for local image providers since we always show a thumbnail anyway first

* linter

---------

Co-authored-by: shenlong <[email protected]>
Co-authored-by: shenlong-tanwen <[email protected]>
Co-authored-by: Alex <[email protected]>
Co-authored-by: Marty Fuhry <[email protected]>
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.

3 participants