immich
immich copied to clipboard
immich-app
feat(mobile): add thumbhash support to gallery
Adds thumbhash support to gallery.
Doesn't look perfect but is a big improvement for poor network conditions.
https://github.com/immich-app/immich/assets/58121030/6ead3115-5cec-49cf-8744-b85d27d91d0a
The latest updates on your projects. Learn more about Vercel for Git ↗︎
| Name | Status | Preview | Comments | Updated (UTC) |
|---|---|---|---|---|
| immich | ✅ Ready (Inspect) | Visit Preview | 💬 Add feedback | Aug 5, 2023 0:11am |
![vercel[bot] avatar](https://avatars.githubusercontent.com/in/8329?v=4 "Published by a pub.dev verified publisher"){kind=small}
Nicely done!
The Isar DLL and lock file should not be included in the repo.
Also we'd need to add a migration in migration.dart : simply clear and increase version. Otherwise assets that already had thumb hash on the server won't show them on the app.
Should be good now.
Should be good now.
There is one outstanding comment that needs to be dealt with. Once this is done, PR looks good from my side.
Should be good now.
There is one outstanding comment that needs to be dealt with. Once this is done, PR looks good from my side.
I staged my changes improperly :/ Sorry bout that.
Is that what you meant by the DB migration?
@alextran1502 can you check this works fine on iOS? I could only test Android as you know :-)
Hello @lukehmcc, thank you for the PR. Sorry, I am a bit late in reviewing and checking this PR out.
One issue I see is that the thumbnail is flashing in transition, which makes the browsing experience unpleasant. Look like it tries to transition between multiple states.
Can we figure out a way to fix the issue to have the transition smoother?
Hello @lukehmcc, thank you for the PR. Sorry, I am a bit late in reviewing and checking this PR out.
One issue I see is that the thumbnail is flashing in transition, which makes the browsing experience unpleasant. Look like it tries to transition between multiple states.
Can we figure out a way to fix the issue to have the transition smoother?
Yeah fs.
I think we should use a placeholder instead of the progressIndicatorBuilder (that is rebuilt often)
Found the culprit: the draggable scrollbar. When released, it triggers a rebuild of the asset grid (in turn rebuilding the placeholders / resetting the transition effect). This is not caused by the PR, it simply never surfaced before. The scrollbar already uses a RepaintBoundary, so not sure how to fix this.
edit: made #3010 to fix this issue. It also greatly improves performance!
Found the culprit: the draggable scrollbar. When released, it triggers a rebuild of the asset grid (in turn rebuilding the placeholders / resetting the transition effect). This is not caused by the PR, it simply never surfaced before. The scrollbar already uses a RepaintBoundary, so not sure how to fix this.
edit: made #3010 to fix this issue. It also greatly improves performance!
Hmm even after applying this change to my branch I'm still getting flickering issues. I was previously testing via an emulator but I'm now using a physical device which makes these artfacts far more apparent. Will look into this more.
It looks like using the FadeInImage class fixes the flicker but has its own issues so I'll keep looking. (It transitions from the placeholder, two black, to the main image which looks weird.)
It looks like using the
FadeInImageclass fixes the flicker but has its own issues so I'll keep looking. (It transitions from the placeholder, two black, to the main image which looks weird.)
Ah good looks like blurhashes also have this issue. Will try to use the proposed solutions from there.
Update: It looks like flicker is a recurring issue with flutter cached network image. I don't think it has to do with any immich code at this point.
@lukehmcc Thank you for the reminder, I've just tested it again, and it looks a lot better. However, I still see some weird flashing issue when viewing different pages, like the album page or the page that show the assets of a person. Do you see the same issue?
@lukehmcc Thank you for the reminder, I've just tested it again, and it looks a lot better. However, I still see some weird flashing issue when viewing different pages, like the album page or the page that show the assets of a person. Do you see the same issue?
Hmm yes I do see that now. There must be a stray rebuild somewhere that I missed as this isn't the same flicker as before. Will dig a little.
Can confirm that any given page (currently looking at the faces page) is built twice. Here is a debug log after clicking on a face:
I/ViewRootImpl@1918418[MainActivity]( 3289): ViewPostIme pointer 0
I/ViewRootImpl@1918418[MainActivity]( 3289): ViewPostIme pointer 1
...
I/flutter ( 3289): rebuildXhgKFIQmaop6Z2dPaKkYDmTDQA==
...
I/flutter ( 3289): rebuildXhgKFIQmaop6Z2dPaKkYDmTDQA==
Still not quite sure what exactly is causing this rebuild but at least I have down that the rebuild is happening only once.
Still not quite sure what exactly is causing this rebuild but at least I have down that the rebuild is happening only once.
Made #3129 to reduce rebuilds of the asset grid. maybe this solves the issue?
Still not quite sure what exactly is causing this rebuild but at least I have down that the rebuild is happening only once.
Made #3129 to reduce rebuilds of the asset grid. maybe this solves the issue?
Exactly we what needed! It now looks wonderful. Once that is merged this should be good :crossed_fingers:
Just merged with main on my local branch and the flickering is now significantly worse. The thumbnail seems to rebuild half a dozen times before the image loads. Shoulda gotten this merged earlier when it wasn't an issue :/
EDIT: I didn't change any code, I just hot restarted a couple times and this flickering is now gone... Pls lmk if the flickering is still an issue.
@lukehmcc the flicker issue was like that before any changes was made that was why we haven't merged the pr in yet.
I don't believe we changed anything that would make it worse
@lukehmcc the flicker issue was like that before any changes was made that was why we haven't merged the pr in yet.
I don't believe we changed anything that would make it worse
Yeah really not sure what was wrong there. Everything seems to be good on my end once more.
@alextran1502 @lukehmcc has this PR been forgotten, or is there something blocking it?
@alextran1502 @lukehmcc has this PR been forgotten, or is there something blocking it?
Alex found flickering in some views that I couldn't reproduce. Last time I tested it was fine so I'm not sure.
@lukehmcc / @alextran1502 I took a stab at the flickering issue in #4605. Can you check if you face the issue when scrolling the timeline? I was not sure whether it was okay to directly push my changes to your PR, so opened a new one for testing. If all is good, we can cherry pick the change and close my PR 😅
