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): thumbhash support #7028

Merged
merged 2 commits into from
Feb 12, 2024

Conversation

shenlong-tanwen
Copy link
Member

Changes made:

  • Finally (Hopefully) a non-flickering thumb hash implementation in mobile

Copy link

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

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 147b748
Status: ✅  Deploy successful!
Preview URL: https://70b233f7.immich.pages.dev
Branch Preview URL: https://feat-mobile-thumbhash.immich.pages.dev

View logs

@@ -116,6 +118,8 @@ class Asset {
/// because Isar cannot sort lists of byte arrays
String checksum;

String? thumbhash;
Copy link
Contributor

Choose a reason for hiding this comment

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

Please make this a List<byte> to store the raw bytes instead of the longer base64. This also avoids the wasteful decoding of base64 to bytes on every render.

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated. Thank you 😄

if (hash == null) {
return null;
}
return base64.decode(base64.normalize(hash)).toList();
Copy link
Contributor

Choose a reason for hiding this comment

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

base64.decode already returns an Uint8List. No need to call toList() which makes it less efficient.

also, I don't think normalize is needed?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'll merge this in to my branch and address this and the other comment there.

void initState() {
super.initState();
if (widget.asset?.thumbhash != null) {
final bytes = Uint8List.fromList(widget.asset!.thumbhash!);
Copy link
Contributor

Choose a reason for hiding this comment

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

it should be safe to cast the thumbhash to an Uint8List (after my other comment is addressed). Isar also returns its List<byte> as Uint8List. This removes 1 unnecessary array copy operation.

@martyfuhry martyfuhry merged commit e270ae0 into refactor/immich-image-provider Feb 12, 2024
16 checks passed
@martyfuhry martyfuhry deleted the feat/mobile-thumbhash branch February 12, 2024 14:07
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 <139912620+shalong-tanwen@users.noreply.github.com>

* 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 <marty@fuhry.farm>
Co-authored-by: Alex <alex.tran1502@gmail.com>

* 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 <139912620+shenlong-tanwen@users.noreply.github.com>
Co-authored-by: shenlong-tanwen <139912620+shalong-tanwen@users.noreply.github.com>
Co-authored-by: Alex <alex.tran1502@gmail.com>
Co-authored-by: Marty Fuhry <marty@fuhry.farm>
shenlong-tanwen added a commit that referenced this pull request Feb 16, 2024
* feat(mobile): thumbhash support

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

---------

Co-authored-by: shenlong-tanwen <139912620+shalong-tanwen@users.noreply.github.com>
shenlong-tanwen added a commit that referenced this pull request Feb 21, 2024
* feat(mobile): thumbhash support

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

---------

Co-authored-by: shenlong-tanwen <139912620+shalong-tanwen@users.noreply.github.com>
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