fix(web): skip thumbhash fade for offscreen-loaded thumbnails#26846
fix(web): skip thumbhash fade for offscreen-loaded thumbnails#26846
Conversation
|
Preview environment has been removed. |
| $effect(() => { | ||
| if (loaded && !loadedEffectRan) { | ||
| loadedEffectRan = true; | ||
| if (!actuallyIntersecting) { |
There was a problem hiding this comment.
Do we really need this check? Elements are only added to the DOM if they're visible or near visible already?
There was a problem hiding this comment.
This check skips the thumbhash fade-out. Elements are only added to the DOM when they are within +/- 500px of the viewport (intersecting) or in the viewport itself. actuallyIntersecting means the element is actually IN the viewport. So when !actuallyIntersecting is true at load time, the thumbnail loaded within the offscreen margins. In that case, we skip the thumbhash fade-out — the placeholder is instantly hidden.
Before this change, the 100ms fade-out would play offscreen, and would almost always complete before the element scrolled into view. This change eliminates the edge case where fast scrolling (>500px/100ms) could cause you to see a partial thumbhash fade-out on an element that loaded in the margins.
5000px/s is fast but achievable. In practice, any partial fade at these scroll speeds would be too brief to notice — but this change eliminates it entirely.
|
I don't think this is quite aligned with the original intention of the changes. If you scroll up and down in search, you will always see a thumbhash and fade when the assets come into view at either the top or bottom depending on direction. There's no real reason for this other than the async nature of the web APIs. The images should be cached locally and should load instantly, but they don't. https://pr-26846.preview.internal.immich.build/search?query=%7B%22query%22%3A%22dog%22%7D |
|
IDK if you can see this, but just look at the top of the video behind the search bar on the preview environment for this PR. The image always fades in when scrolling. 2026-03-11_14-06-02.mp4 |
This change was meant for timeline - I didn't realize you were referring to search results view. Search uses a very different system for displaying the images/thumbnails - it would have to be adapted for that. |
midzelis
force-pushed
the
push-kulzwwtvyqkk
branch
2 times, most recently
from
7c4f27e to
1ec63b4
Compare
| export const Intersection = { | ||
| NONE: 0, | ||
| PRE: 1, | ||
| ACTUAL: 3, // includes PRE (both bits set) |
There was a problem hiding this comment.
Pre intersecting. 500 pc before or after the viewport. Used to preload up-and-coming thumbs. Means thumb is about to be onscreen. Also known as "intersecting".
"Actual" represents "actuallyIntersecting" which means the thumb is actually on screen.
There was a problem hiding this comment.
I think the whole “intersecting vs actually intersecting” wording is confusing personally. Intersecting should mean “actually intersecting”, the other one should have a clearer term like “near viewport” or something.
There was a problem hiding this comment.
In my last push, using flags (NONE,NEARBY,VISIBLE) and a derived flag RENDERABLE which is NEARBY | VISIBLE.
Struggled with naming, but I think RENDERABLE makes sense as a composition of NEARBY and VISIBLE. Its not JUST visible - since it includes NEARBY. NEARBY_OR_VISIBLE was a little too long. And, I can't use the past tense, since its not YET rendered (thats would be when it actually goes into the DOM). Also considered IN_DOM (but again, that wouldn't be accurate) or DOM_ELIGIBLE - but RENDERABLE was my preferred.
This was a fairly big change, as it trickled down to a lot of places that use intersecting/isIntersecting/actuallyIntersecting.
I also improved/consolidated the intersection calculation code in intersection-support - and its a little more performant by flipping the conditions for NONE first.
These changes were in a separate commit, so they can be broken out into another PR if you prefer.
There was a problem hiding this comment.
naming is getting harder and harder ngl 😅
midzelis
force-pushed
the
push-kulzwwtvyqkk
branch
from
1ec63b4 to
8440a82
Compare
midzelis
force-pushed
the
push-kulzwwtvyqkk
branch
2 times, most recently
from
3563d2c to
1df73f2
Compare
Thumbnails that finish loading while offscreen (in the 500px pre-load zone) were still playing the thumbhash fade-out transition, causing visual flicker when scrolling. This adds an `actuallyIntersecting` property (zero-margin viewport check) alongside the existing `intersecting` (500px expanded margins) to distinguish between pre-loaded and truly visible assets. - Refactor `calculateViewerAssetIntersecting` to return a numeric flag (NONE/PRE/ACTUAL) avoiding object allocation in the hot derived path - Inline `calculateMonthGroupIntersecting` into `updateIntersectionMonthGroup` to avoid intermediate object allocation - Thread `actuallyIntersecting` through AssetLayout → Month → Timeline → Thumbnail snippet chain - Use `actuallyIntersecting` in Thumbnail to skip the fade when loaded offscreen - Add unit tests for `isIntersecting` and `calculateViewerAssetIntersecting`
…ersecting Change-Id: Id6f63247441fe290e7732e489f73e8c16a6a6964
midzelis
force-pushed
the
push-kulzwwtvyqkk
branch
from
1df73f2 to
379678d
Compare
4 participants
See #20773 as the original source/inspiration