From 1e0459dc2d096cf54e9403994482790e89dadcc0 Mon Sep 17 00:00:00 2001 From: midzelis Date: Tue, 10 Mar 2026 16:04:00 +0000 Subject: [PATCH] fix(web): skip thumbhash fade for offscreen-loaded thumbnails MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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` --- .../assets/thumbnail/thumbnail.svelte | 15 ++- .../gallery-viewer/gallery-viewer.svelte | 19 +++- .../components/timeline/AssetLayout.svelte | 4 +- web/src/lib/components/timeline/Month.svelte | 16 +++- .../lib/components/timeline/Timeline.svelte | 3 +- .../intersection-support.svelte.spec.ts | 95 +++++++++++++++++++ .../internal/intersection-support.svelte.ts | 90 ++++++++++-------- .../timeline-manager/viewer-asset.svelte.ts | 14 ++- 8 files changed, 204 insertions(+), 52 deletions(-) create mode 100644 web/src/lib/managers/timeline-manager/internal/intersection-support.svelte.spec.ts diff --git a/web/src/lib/components/assets/thumbnail/thumbnail.svelte b/web/src/lib/components/assets/thumbnail/thumbnail.svelte index 64b5a835ed..d3e2a1ec18 100644 --- a/web/src/lib/components/assets/thumbnail/thumbnail.svelte +++ b/web/src/lib/components/assets/thumbnail/thumbnail.svelte @@ -34,6 +34,7 @@ thumbnailSize?: number; thumbnailWidth?: number; thumbnailHeight?: number; + actuallyIntersecting?: boolean; selected?: boolean; selectionCandidate?: boolean; disabled?: boolean; @@ -56,6 +57,7 @@ thumbnailSize = undefined, thumbnailWidth = undefined, thumbnailHeight = undefined, + actuallyIntersecting = true, selected = false, selectionCandidate = false, disabled = false, @@ -82,6 +84,17 @@ let loaded = $state(false); let thumbError = $state(false); + let loadedEffectRan = $state(false); + let skipFade = $state(false); + $effect(() => { + if (loaded && !loadedEffectRan) { + loadedEffectRan = true; + if (!actuallyIntersecting) { + skipFade = true; + } + } + }); + let width = $derived(thumbnailSize || thumbnailWidth || 235); let height = $derived(thumbnailSize || thumbnailHeight || 235); @@ -300,7 +313,7 @@ window.top; }; + const isActuallyIntersecting = (i: number) => { + const geo = geometry; + const top = geo.getTop(i) + pageHeaderOffset; + const bottom = top + geo.getHeight(i); + const viewportTop = (scrollTop || 0) - slidingWindowOffset; + const viewportBottom = viewportTop + viewport.height + slidingWindowOffset; + return top < viewportBottom && bottom > viewportTop; + }; + let shiftKeyIsDown = $state(false); let lastAssetMouseEvent: TimelineAsset | null = $state(null); let scrollTop = $state(0); + const { + TIMELINE: { INTERSECTION_EXPAND_TOP, INTERSECTION_EXPAND_BOTTOM }, + } = TUNABLES; + let slidingWindow = $derived.by(() => { - const top = (scrollTop || 0) - slidingWindowOffset; - const bottom = top + viewport.height + slidingWindowOffset; + const top = (scrollTop || 0) - slidingWindowOffset - INTERSECTION_EXPAND_TOP; + const bottom = top + viewport.height + slidingWindowOffset + INTERSECTION_EXPAND_BOTTOM; return { top, bottom, @@ -391,6 +405,7 @@ asset={currentAsset} selected={assetInteraction.hasSelectedAsset(currentAsset.id)} selectionCandidate={assetInteraction.hasSelectionCandidate(currentAsset.id)} + actuallyIntersecting={isActuallyIntersecting(i)} thumbnailWidth={geometry.getWidth(i)} thumbnailHeight={geometry.getHeight(i)} /> diff --git a/web/src/lib/components/timeline/AssetLayout.svelte b/web/src/lib/components/timeline/AssetLayout.svelte index 8b06d9b72b..cf4090bc1f 100644 --- a/web/src/lib/components/timeline/AssetLayout.svelte +++ b/web/src/lib/components/timeline/AssetLayout.svelte @@ -20,6 +20,7 @@ { asset: TimelineAsset; position: CommonPosition; + actuallyIntersecting: boolean; }, ] >; @@ -41,6 +42,7 @@ {#each filterIntersecting(viewerAssets) as viewerAsset (viewerAsset.id)} {@const position = viewerAsset.position!} {@const asset = viewerAsset.asset!} + {@const actuallyIntersecting = viewerAsset.actuallyIntersecting!}
- {@render thumbnail({ asset, position })} + {@render thumbnail({ asset, position, actuallyIntersecting })} {@render customThumbnailLayout?.(asset)}
{/each} diff --git a/web/src/lib/components/timeline/Month.svelte b/web/src/lib/components/timeline/Month.svelte index 91073a0a5f..b6c4e50a1e 100644 --- a/web/src/lib/components/timeline/Month.svelte +++ b/web/src/lib/components/timeline/Month.svelte @@ -14,7 +14,17 @@ import type { Snippet } from 'svelte'; type Props = { - thumbnail: Snippet<[{ asset: TimelineAsset; position: CommonPosition; dayGroup: DayGroup; groupIndex: number }]>; + thumbnail: Snippet< + [ + { + asset: TimelineAsset; + position: CommonPosition; + dayGroup: DayGroup; + groupIndex: number; + actuallyIntersecting: boolean; + }, + ] + >; customThumbnailLayout?: Snippet<[TimelineAsset]>; singleSelect: boolean; assetInteraction: AssetInteraction; @@ -99,8 +109,8 @@ width={dayGroup.width} {customThumbnailLayout} > - {#snippet thumbnail({ asset, position })} - {@render thumbnailWithGroup({ asset, position, dayGroup, groupIndex })} + {#snippet thumbnail({ asset, position, actuallyIntersecting })} + {@render thumbnailWithGroup({ asset, position, dayGroup, groupIndex, actuallyIntersecting })} {/snippet} diff --git a/web/src/lib/components/timeline/Timeline.svelte b/web/src/lib/components/timeline/Timeline.svelte index d6ce722c96..cafd867295 100644 --- a/web/src/lib/components/timeline/Timeline.svelte +++ b/web/src/lib/components/timeline/Timeline.svelte @@ -673,7 +673,7 @@ manager={timelineManager} onDayGroupSelect={handleGroupSelect} > - {#snippet thumbnail({ asset, position, dayGroup, groupIndex })} + {#snippet thumbnail({ asset, position, dayGroup, groupIndex, actuallyIntersecting })} {@const isAssetSelectionCandidate = assetInteraction.hasSelectionCandidate(asset.id)} {@const isAssetSelected = assetInteraction.hasSelectedAsset(asset.id) || timelineManager.albumAssets.has(asset.id)} @@ -684,6 +684,7 @@ {asset} {albumUsers} {groupIndex} + {actuallyIntersecting} onClick={(asset) => { if (typeof onThumbnailClick === 'function') { onThumbnailClick(asset, timelineManager, dayGroup, _onClick); diff --git a/web/src/lib/managers/timeline-manager/internal/intersection-support.svelte.spec.ts b/web/src/lib/managers/timeline-manager/internal/intersection-support.svelte.spec.ts new file mode 100644 index 0000000000..4b2a7aa460 --- /dev/null +++ b/web/src/lib/managers/timeline-manager/internal/intersection-support.svelte.spec.ts @@ -0,0 +1,95 @@ +import { describe, expect, it } from 'vitest'; +import type { TimelineManager } from '../timeline-manager.svelte'; +import { Intersection, calculateViewerAssetIntersecting, isIntersecting } from './intersection-support.svelte'; + +function createMockTimelineManager(windowTop: number, windowBottom: number, headerHeight: number = 0): TimelineManager { + return { + visibleWindow: { top: windowTop, bottom: windowBottom }, + headerHeight, + } as TimelineManager; +} + +describe('isIntersecting', () => { + it('should return true when region top is inside window', () => { + expect(isIntersecting(50, 150, 0, 100)).toBe(true); + }); + + it('should return true when region bottom is inside window', () => { + expect(isIntersecting(-50, 50, 0, 100)).toBe(true); + }); + + it('should return true when region spans entire window', () => { + expect(isIntersecting(-50, 150, 0, 100)).toBe(true); + }); + + it('should return true when region is fully inside window', () => { + expect(isIntersecting(25, 75, 0, 100)).toBe(true); + }); + + it('should return false when region is entirely above window', () => { + expect(isIntersecting(-100, -10, 0, 100)).toBe(false); + }); + + it('should return false when region is entirely below window', () => { + expect(isIntersecting(110, 200, 0, 100)).toBe(false); + }); + + it('should return true when region bottom touches window top (inclusive)', () => { + expect(isIntersecting(-50, 0, 0, 100)).toBe(true); + }); + + it('should return true when region top equals window top (inclusive)', () => { + expect(isIntersecting(0, 50, 0, 100)).toBe(true); + }); + + it('should return false when region top equals window bottom (exclusive)', () => { + expect(isIntersecting(100, 150, 0, 100)).toBe(false); + }); +}); + +describe('calculateViewerAssetIntersecting', () => { + // viewport 0-1000, no header, default expand margins 500/500 + const manager = createMockTimelineManager(0, 1000); + + it('should return ACTUAL when asset is within viewport', () => { + expect(calculateViewerAssetIntersecting(manager, 100, 50)).toBe(Intersection.ACTUAL); + }); + + it('should return ACTUAL when asset is at viewport top edge', () => { + expect(calculateViewerAssetIntersecting(manager, 0, 50)).toBe(Intersection.ACTUAL); + }); + + it('should return ACTUAL when asset partially overlaps viewport bottom', () => { + expect(calculateViewerAssetIntersecting(manager, 980, 50)).toBe(Intersection.ACTUAL); + }); + + it('should return PRE when asset is just above viewport within expand margin', () => { + expect(calculateViewerAssetIntersecting(manager, -200, 50)).toBe(Intersection.PRE); + }); + + it('should return PRE when asset is just below viewport within expand margin', () => { + expect(calculateViewerAssetIntersecting(manager, 1200, 50)).toBe(Intersection.PRE); + }); + + it('should return NONE when asset is far above viewport', () => { + expect(calculateViewerAssetIntersecting(manager, -1000, 50)).toBe(Intersection.NONE); + }); + + it('should return NONE when asset is far below viewport', () => { + expect(calculateViewerAssetIntersecting(manager, 2000, 50)).toBe(Intersection.NONE); + }); + + it('should account for header height in viewport bounds', () => { + const managerWithHeader = createMockTimelineManager(100, 500, 50); + // viewport effectively becomes (100-50)=50 to (500+50)=550 + // asset at 40-90 overlaps the effective viewport + expect(calculateViewerAssetIntersecting(managerWithHeader, 40, 50)).toBe(Intersection.ACTUAL); + }); + + it('should return PRE not ACTUAL for asset outside viewport but within header-adjusted expand', () => { + const managerWithHeader = createMockTimelineManager(100, 500, 50); + // effective viewport: 50-550, expand: 500 each way -> -450 to 1050 + // asset at -400 to -350 is outside viewport but within expand + expect(calculateViewerAssetIntersecting(managerWithHeader, -400, 50)).toBe(Intersection.PRE); + }); +}); diff --git a/web/src/lib/managers/timeline-manager/internal/intersection-support.svelte.ts b/web/src/lib/managers/timeline-manager/internal/intersection-support.svelte.ts index 3c6f2d8256..3810942d81 100644 --- a/web/src/lib/managers/timeline-manager/internal/intersection-support.svelte.ts +++ b/web/src/lib/managers/timeline-manager/internal/intersection-support.svelte.ts @@ -6,31 +6,8 @@ const { TIMELINE: { INTERSECTION_EXPAND_TOP, INTERSECTION_EXPAND_BOTTOM }, } = TUNABLES; -export function updateIntersectionMonthGroup(timelineManager: TimelineManager, month: MonthGroup) { - const actuallyIntersecting = calculateMonthGroupIntersecting(timelineManager, month, 0, 0); - let preIntersecting = false; - if (!actuallyIntersecting) { - preIntersecting = calculateMonthGroupIntersecting( - timelineManager, - month, - INTERSECTION_EXPAND_TOP, - INTERSECTION_EXPAND_BOTTOM, - ); - } - month.intersecting = actuallyIntersecting || preIntersecting; - month.actuallyIntersecting = actuallyIntersecting; - if (preIntersecting || actuallyIntersecting) { - timelineManager.clearDeferredLayout(month); - } -} - /** * General function to check if a rectangular region intersects with a window. - * @param regionTop - Top position of the region to check - * @param regionBottom - Bottom position of the region to check - * @param windowTop - Top position of the window - * @param windowBottom - Bottom position of the window - * @returns true if the region intersects with the window */ export function isIntersecting(regionTop: number, regionBottom: number, windowTop: number, windowBottom: number) { return ( @@ -40,34 +17,65 @@ export function isIntersecting(regionTop: number, regionBottom: number, windowTo ); } -export function calculateMonthGroupIntersecting( - timelineManager: TimelineManager, - monthGroup: MonthGroup, - expandTop: number, - expandBottom: number, -) { - const monthGroupTop = monthGroup.top; - const monthGroupBottom = monthGroupTop + monthGroup.height; - const topWindow = timelineManager.visibleWindow.top - expandTop; - const bottomWindow = timelineManager.visibleWindow.bottom + expandBottom; +export function updateIntersectionMonthGroup(timelineManager: TimelineManager, month: MonthGroup) { + const monthGroupTop = month.top; + const monthGroupBottom = monthGroupTop + month.height; + const windowTop = timelineManager.visibleWindow.top; + const windowBottom = timelineManager.visibleWindow.bottom; - return isIntersecting(monthGroupTop, monthGroupBottom, topWindow, bottomWindow); + const actuallyIntersecting = isIntersecting(monthGroupTop, monthGroupBottom, windowTop, windowBottom); + + let intersecting = actuallyIntersecting; + if (!actuallyIntersecting) { + intersecting = isIntersecting( + monthGroupTop, + monthGroupBottom, + windowTop - INTERSECTION_EXPAND_TOP, + windowBottom + INTERSECTION_EXPAND_BOTTOM, + ); + } + + month.intersecting = intersecting; + month.actuallyIntersecting = actuallyIntersecting; + if (intersecting) { + timelineManager.clearDeferredLayout(month); + } } +// Bit flags for intersection state +export const Intersection = { + NONE: 0, + PRE: 1, + ACTUAL: 3, // includes PRE (both bits set) +} as const; + /** - * Calculate intersection for viewer assets with additional parameters like header height + * Returns a numeric flag: NONE (0), PRE (1, within expanded margin only), or ACTUAL (3, truly visible). */ export function calculateViewerAssetIntersecting( timelineManager: TimelineManager, positionTop: number, positionHeight: number, - expandTop: number = INTERSECTION_EXPAND_TOP, - expandBottom: number = INTERSECTION_EXPAND_BOTTOM, ) { - const topWindow = timelineManager.visibleWindow.top - timelineManager.headerHeight - expandTop; - const bottomWindow = timelineManager.visibleWindow.bottom + timelineManager.headerHeight + expandBottom; - const positionBottom = positionTop + positionHeight; + const headerHeight = timelineManager.headerHeight; + const windowTop = timelineManager.visibleWindow.top - headerHeight; + const windowBottom = timelineManager.visibleWindow.bottom + headerHeight; - return isIntersecting(positionTop, positionBottom, topWindow, bottomWindow); + if (isIntersecting(positionTop, positionBottom, windowTop, windowBottom)) { + return Intersection.ACTUAL; + } + + if ( + isIntersecting( + positionTop, + positionBottom, + windowTop - INTERSECTION_EXPAND_TOP, + windowBottom + INTERSECTION_EXPAND_BOTTOM, + ) + ) { + return Intersection.PRE; + } + + return Intersection.NONE; } diff --git a/web/src/lib/managers/timeline-manager/viewer-asset.svelte.ts b/web/src/lib/managers/timeline-manager/viewer-asset.svelte.ts index 161cc049f1..10299c83eb 100644 --- a/web/src/lib/managers/timeline-manager/viewer-asset.svelte.ts +++ b/web/src/lib/managers/timeline-manager/viewer-asset.svelte.ts @@ -1,15 +1,15 @@ import type { CommonPosition } from '$lib/utils/layout-utils'; import type { DayGroup } from './day-group.svelte'; -import { calculateViewerAssetIntersecting } from './internal/intersection-support.svelte'; +import { Intersection, calculateViewerAssetIntersecting } from './internal/intersection-support.svelte'; import type { TimelineAsset } from './types'; export class ViewerAsset { readonly #group: DayGroup; - intersecting = $derived.by(() => { + #intersection = $derived.by(() => { if (!this.position) { - return false; + return Intersection.NONE; } const store = this.#group.monthGroup.timelineManager; @@ -18,6 +18,14 @@ export class ViewerAsset { return calculateViewerAssetIntersecting(store, positionTop, this.position.height); }); + get intersecting() { + return this.#intersection !== Intersection.NONE; + } + + get actuallyIntersecting() { + return this.#intersection === Intersection.ACTUAL; + } + position: CommonPosition | undefined = $state.raw(); asset: TimelineAsset = $state(); id: string = $derived(this.asset.id);