From 26fbf6459a90ff87eccd979b421022cc72a72f68 Mon Sep 17 00:00:00 2001 From: "Claude (thor)" Date: Mon, 10 Aug 2026 17:59:00 -0400 Subject: [PATCH] fix(web): don't stretch images Lay out the viewer image from its natural, orientation-baked size instead of asset.width/height, which can be wrong for some cameras (e.g. Sony orientation-8 JPEGs) and stretched portrait images into a landscape box. Co-Authored-By: Claude Opus 4.8 --- web/src/lib/components/AdaptiveImage.spec.ts | 46 ++++++++++++++++++++ web/src/lib/components/AdaptiveImage.svelte | 24 +++++----- web/src/lib/utils/container-utils.spec.ts | 34 +++++++++++++++ web/src/lib/utils/container-utils.ts | 12 +++++ 4 files changed, 106 insertions(+), 10 deletions(-) create mode 100644 web/src/lib/components/AdaptiveImage.spec.ts diff --git a/web/src/lib/components/AdaptiveImage.spec.ts b/web/src/lib/components/AdaptiveImage.spec.ts new file mode 100644 index 0000000000..5cba93ae7b --- /dev/null +++ b/web/src/lib/components/AdaptiveImage.spec.ts @@ -0,0 +1,46 @@ +import { AssetTypeEnum } from '@immich/sdk'; +import { fireEvent, render } from '@testing-library/svelte'; +import { tick } from 'svelte'; +import AdaptiveImage from '$lib/components/AdaptiveImage.svelte'; +import { assetFactory } from '@test-data/factories/asset-factory'; + +vi.mock('$lib/utils/sw-messaging', () => ({ + cancelImageUrl: vi.fn(), +})); + +const setNaturalSize = (img: HTMLImageElement, width: number, height: number) => { + Object.defineProperties(img, { + naturalWidth: { value: width, configurable: true }, + naturalHeight: { value: height, configurable: true }, + }); +}; + +const getDisplayBox = (baseElement: Element) => + baseElement.querySelector('[style*="inset-inline-start"]'); + +const pixels = (value: string) => Number(value.replace('px', '')); + +describe('AdaptiveImage', () => { + it('lays the image out with the aspect ratio of the loaded pixels, not the metadata dimensions', async () => { + const asset = assetFactory.build({ type: AssetTypeEnum.Image, width: 3872, height: 2592 }); + + const { baseElement } = render(AdaptiveImage, { + asset, + container: { width: 1000, height: 1000 }, + }); + + const thumbnail = baseElement.querySelector('img[data-testid="thumbnail"]'); + expect(thumbnail).not.toBeNull(); + + setNaturalSize(thumbnail!, 2592, 3872); + await fireEvent.load(thumbnail!); + await tick(); + + const box = getDisplayBox(baseElement); + expect(box).not.toBeNull(); + + const width = pixels(box!.style.width); + const height = pixels(box!.style.height); + expect(width).toBeLessThan(height); + }); +}); diff --git a/web/src/lib/components/AdaptiveImage.svelte b/web/src/lib/components/AdaptiveImage.svelte index 89b10b2d77..7e3ce6286b 100644 --- a/web/src/lib/components/AdaptiveImage.svelte +++ b/web/src/lib/components/AdaptiveImage.svelte @@ -58,7 +58,7 @@ import { assetViewerManager } from '$lib/managers/asset-viewer-manager.svelte'; import { getAssetUrls } from '$lib/utils'; import { AdaptiveImageLoader, type QualityList } from '$lib/utils/adaptive-image-loader.svelte'; - import { scaleToCover, scaleToFit, type Size } from '$lib/utils/container-utils'; + import { resolveImageDimensions, scaleToCover, scaleToFit, type Size } from '$lib/utils/container-utils'; import { getAltText } from '$lib/utils/thumbnail-util'; import { toTimelineAsset } from '$lib/utils/timeline-util'; import type { AssetResponseDto, SharedLinkResponseDto } from '@immich/sdk'; @@ -81,7 +81,6 @@ let { ref = $bindable(), - // eslint-disable-next-line no-useless-assignment imgRef = $bindable(), asset, sharedLink, @@ -136,19 +135,20 @@ ); }); + let naturalSize = $state(); + $effect.pre(() => { const loader = adaptiveImageLoader; - untrack(() => assetViewerManager.resetZoomState()); + untrack(() => { + assetViewerManager.resetZoomState(); + naturalSize = undefined; + }); return () => loader.destroy(); }); - const imageDimensions = $derived.by(() => { - const { width, height } = asset; - if (width && width > 0 && height && height > 0) { - return { width, height }; - } - return { width: 1, height: 1 }; - }); + const imageDimensions = $derived( + resolveImageDimensions(naturalSize, { width: asset.width ?? 0, height: asset.height ?? 0 }), + ); const { insetInlineStart, top, displayWidth, displayHeight, rasterWidth, rasterHeight, rasterScale } = $derived.by( () => { @@ -216,6 +216,10 @@ (quality.original === 'success' ? originalElement : undefined) ?? (quality.preview === 'success' ? previewElement : undefined) ?? (quality.thumbnail === 'success' ? thumbnailElement : undefined); + + if (imgRef && imgRef.naturalWidth > 0 && imgRef.naturalHeight > 0) { + naturalSize = { width: imgRef.naturalWidth, height: imgRef.naturalHeight }; + } }); diff --git a/web/src/lib/utils/container-utils.spec.ts b/web/src/lib/utils/container-utils.spec.ts index d6a1efbe6a..6a1efdb57c 100644 --- a/web/src/lib/utils/container-utils.spec.ts +++ b/web/src/lib/utils/container-utils.spec.ts @@ -3,6 +3,7 @@ import { getNaturalSize, mapNormalizedRectToContent, mapNormalizedToContent, + resolveImageDimensions, scaleToCover, scaleToFit, } from '$lib/utils/container-utils'; @@ -177,3 +178,36 @@ describe('mapNormalizedRectToContent', () => { expect(rect).toEqual({ left: 200, top: 100, width: 400, height: 200 }); }); }); + +describe('resolveImageDimensions', () => { + it('should prefer the natural size when it is available', () => { + expect(resolveImageDimensions({ width: 2592, height: 3872 }, { width: 4000, height: 3000 })).toEqual({ + width: 2592, + height: 3872, + }); + }); + + it('should keep a portrait image portrait even when metadata claims landscape', () => { + const natural = { width: 2592, height: 3872 }; + const metadata = { width: 3872, height: 2592 }; + const resolved = resolveImageDimensions(natural, metadata); + expect(resolved.width).toBeLessThan(resolved.height); + expect(resolved).toEqual(natural); + }); + + it('should fall back to metadata dimensions before the image has loaded', () => { + expect(resolveImageDimensions(undefined, { width: 4000, height: 3000 })).toEqual({ width: 4000, height: 3000 }); + }); + + it('should ignore a degenerate natural size and fall back to metadata', () => { + expect(resolveImageDimensions({ width: 0, height: 0 }, { width: 4000, height: 3000 })).toEqual({ + width: 4000, + height: 3000, + }); + }); + + it('should return a 1x1 square when neither size is valid', () => { + expect(resolveImageDimensions(undefined, { width: 0, height: 0 })).toEqual({ width: 1, height: 1 }); + expect(resolveImageDimensions(undefined, undefined)).toEqual({ width: 1, height: 1 }); + }); +}); diff --git a/web/src/lib/utils/container-utils.ts b/web/src/lib/utils/container-utils.ts index 36e260fcc7..74ca6244ea 100644 --- a/web/src/lib/utils/container-utils.ts +++ b/web/src/lib/utils/container-utils.ts @@ -63,6 +63,18 @@ export const getNaturalSize = (element: HTMLImageElement | HTMLVideoElement): Si return { width: element.naturalWidth, height: element.naturalHeight }; }; +const isValidSize = (size: Size | undefined): size is Size => size !== undefined && size.width > 0 && size.height > 0; + +export const resolveImageDimensions = (natural: Size | undefined, fallback: Size | undefined): Size => { + if (isValidSize(natural)) { + return natural; + } + if (isValidSize(fallback)) { + return fallback; + } + return { width: 1, height: 1 }; +}; + export const getContentMetrics = (element: HTMLImageElement | HTMLVideoElement): ContentMetrics => { const natural = getNaturalSize(element); const client = getElementSize(element);