From 98c19547a5c3830e5cdf57da8bbfcd1b38af6027 Mon Sep 17 00:00:00 2001 From: midzelis Date: Sat, 25 Apr 2026 17:54:56 +0000 Subject: [PATCH] fix(web): Create Person face preview not working for video assets FaceEditor previously required an HTMLImageElement | HTMLVideoElement prop to compute layout metrics and generate the face crop preview. This was unavailable for video assets, so the preview thumbnail in the Create Person modal was always missing, and face positions could be NaN during image load (naturalWidth is 0 before the image decodes). Replace the DOM element prop with assetSize: Size and containerSize: Size, using asset metadata dimensions that are always available from the API response. computeContentMetrics() is extracted as a pure utility alongside mapContentRectToNatural() for converting face rect coordinates back to original image space. For videos, VideoNativeViewer now captures the current frame to canvas when face edit mode opens and sets assetViewerManager.imgRef, giving FaceEditor the same image-based preview path as photo assets. Change-Id: I0e9da549e3af40211abad4ab2c0270706a6a6964 --- .../asset-viewer/PhotoViewer.svelte | 8 +- .../asset-viewer/VideoNativeViewer.svelte | 43 +++++++- .../face-editor/FaceEditor.svelte | 84 ++++++--------- web/src/lib/utils/container-utils.spec.ts | 100 ++++++++++++------ web/src/lib/utils/container-utils.ts | 44 +++++--- 5 files changed, 176 insertions(+), 103 deletions(-) diff --git a/web/src/lib/components/asset-viewer/PhotoViewer.svelte b/web/src/lib/components/asset-viewer/PhotoViewer.svelte index 37844a5459..76cff13607 100644 --- a/web/src/lib/components/asset-viewer/PhotoViewer.svelte +++ b/web/src/lib/components/asset-viewer/PhotoViewer.svelte @@ -286,7 +286,11 @@ {/snippet} - {#if assetViewerManager.isFaceEditMode && assetViewerManager.imgRef} - + {#if assetViewerManager.isFaceEditMode && assetViewerManager.imgRef && asset.width && asset.height} + {/if} diff --git a/web/src/lib/components/asset-viewer/VideoNativeViewer.svelte b/web/src/lib/components/asset-viewer/VideoNativeViewer.svelte index 8f84466295..5fdf8ec86a 100644 --- a/web/src/lib/components/asset-viewer/VideoNativeViewer.svelte +++ b/web/src/lib/components/asset-viewer/VideoNativeViewer.svelte @@ -308,9 +308,40 @@ let containerHeight = $state(0); $effect(() => { - if (assetViewerManager.isFaceEditMode) { - videoPlayer?.pause(); + if (!assetViewerManager.isFaceEditMode || !videoPlayer) { + return; } + videoPlayer.pause(); + + const { videoWidth, videoHeight } = videoPlayer; + if (videoWidth === 0 || videoHeight === 0) { + return; + } + + const canvas = document.createElement('canvas'); + canvas.width = videoWidth; + canvas.height = videoHeight; + const context = canvas.getContext('2d'); + if (!context) { + return; + } + + context.drawImage(videoPlayer, 0, 0); + const dataUrl = canvas.toDataURL('image/png'); + canvas.width = 0; + + const img = new Image(); + const onLoad = () => { + assetViewerManager.imgRef = img; + }; + img.addEventListener('load', onLoad); + img.src = dataUrl; + + return () => { + img.removeEventListener('load', onLoad); + img.src = ''; + assetViewerManager.imgRef = undefined; + }; }); // The time is only refreshed on HLS fragment decode by default, @@ -454,8 +485,12 @@ {/if} - {#if assetViewerManager.isFaceEditMode && videoPlayer} - + {#if assetViewerManager.isFaceEditMode} + {/if} {/if} diff --git a/web/src/lib/components/asset-viewer/face-editor/FaceEditor.svelte b/web/src/lib/components/asset-viewer/face-editor/FaceEditor.svelte index 5ec423a9c4..22b9f32cdc 100644 --- a/web/src/lib/components/asset-viewer/face-editor/FaceEditor.svelte +++ b/web/src/lib/components/asset-viewer/face-editor/FaceEditor.svelte @@ -4,7 +4,7 @@ import { assetViewerManager } from '$lib/managers/asset-viewer-manager.svelte'; import FaceCreateTagModal from '$lib/modals/CreateFaceModal.svelte'; import { getPeopleThumbnailUrl } from '$lib/utils'; - import { getNaturalSize, scaleToFit } from '$lib/utils/container-utils'; + import { computeContentMetrics, mapContentRectToNatural, type Size } from '$lib/utils/container-utils'; import { handleError } from '$lib/utils/handle-error'; import { createFace, getAllPeople, type PersonResponseDto } from '@immich/sdk'; import { Button, Input, modalManager, toastManager } from '@immich/ui'; @@ -14,13 +14,12 @@ import { t } from 'svelte-i18n'; type Props = { - htmlElement: HTMLImageElement | HTMLVideoElement; - containerWidth: number; - containerHeight: number; + imageSize: Size; + containerSize: Size; assetId: string; }; - let { htmlElement, containerWidth, containerHeight, assetId }: Props = $props(); + let { imageSize, containerSize, assetId }: Props = $props(); let canvasEl: HTMLCanvasElement | undefined = $state(); let canvas: Canvas | undefined = $state(); @@ -54,7 +53,7 @@ }; const setupCanvas = () => { - if (!canvasEl || !htmlElement) { + if (!canvasEl) { return; } @@ -86,24 +85,14 @@ searchInputEl?.focus(); }); - const imageContentMetrics = $derived.by(() => { - const natural = getNaturalSize(htmlElement); - const container = { width: containerWidth, height: containerHeight }; - const { width: contentWidth, height: contentHeight } = scaleToFit(natural, container); - return { - contentWidth, - contentHeight, - offsetX: (containerWidth - contentWidth) / 2, - offsetY: (containerHeight - contentHeight) / 2, - }; - }); + const imageContentMetrics = $derived(computeContentMetrics(imageSize, containerSize)); const setDefaultFaceRectanglePosition = (faceRect: Rect) => { - const { offsetX, offsetY } = imageContentMetrics; + const { offsetX, offsetY, contentWidth, contentHeight } = imageContentMetrics; faceRect.set({ - top: offsetY + 200, - left: offsetX + 200, + top: offsetY + contentHeight / 2 - 56, + left: offsetX + contentWidth / 2 - 56, }); faceRect.setCoords(); @@ -116,8 +105,8 @@ } canvas.setDimensions({ - width: containerWidth, - height: containerHeight, + width: containerSize.width, + height: containerSize.height, }); if (!faceRect) { @@ -167,6 +156,9 @@ const gap = 15; const padding = faceRect.padding ?? 0; const rawBox = faceRect.getBoundingRect(); + if (Number.isNaN(rawBox.left) || Number.isNaN(rawBox.width)) { + return; + } const faceBox = { left: rawBox.left - padding, top: rawBox.top - padding, @@ -175,11 +167,11 @@ }; const selectorWidth = faceSelectorEl.offsetWidth; const chromeHeight = faceSelectorEl.offsetHeight - scrollableListEl.offsetHeight; - const listHeight = Math.min(MAX_LIST_HEIGHT, containerHeight - gap * 2 - chromeHeight); + const listHeight = Math.min(MAX_LIST_HEIGHT, containerSize.height - gap * 2 - chromeHeight); const selectorHeight = listHeight + chromeHeight; - const clampTop = (top: number) => clamp(top, gap, containerHeight - selectorHeight - gap); - const clampLeft = (left: number) => clamp(left, gap, containerWidth - selectorWidth - gap); + const clampTop = (top: number) => clamp(top, gap, containerSize.height - selectorHeight - gap); + const clampLeft = (left: number) => clamp(left, gap, containerSize.width - selectorWidth - gap); const overlapArea = (position: { top: number; left: number }) => { const selectorRight = position.left + selectorWidth; @@ -238,45 +230,37 @@ }); const getFaceCroppedCoordinates = () => { - if (!faceRect || !htmlElement) { + if (!faceRect || imageContentMetrics.contentWidth === 0) { return; } - const { left, top, width, height } = faceRect.getBoundingRect(); - const { offsetX, offsetY, contentWidth, contentHeight } = imageContentMetrics; - const natural = getNaturalSize(htmlElement); - - const scaleX = natural.width / contentWidth; - const scaleY = natural.height / contentHeight; - const imageX = (left - offsetX) * scaleX; - const imageY = (top - offsetY) * scaleY; + const imageRect = mapContentRectToNatural(faceRect.getBoundingRect(), imageContentMetrics, imageSize); return { - imageWidth: natural.width, - imageHeight: natural.height, - x: Math.floor(imageX), - y: Math.floor(imageY), - width: Math.floor(width * scaleX), - height: Math.floor(height * scaleY), + imageWidth: imageSize.width, + imageHeight: imageSize.height, + x: Math.floor(imageRect.left), + y: Math.floor(imageRect.top), + width: Math.floor(imageRect.width), + height: Math.floor(imageRect.height), }; }; type FaceCoordinates = NonNullable>; const getFacePreviewUrl = (data: FaceCoordinates) => { - if (!htmlElement) { + const imgRef = assetViewerManager.imgRef; + if (!imgRef || imageContentMetrics.contentWidth === 0) { return; } - const natural = getNaturalSize(htmlElement); - if (natural.width <= 0 || natural.height <= 0) { - return; - } + const scaleX = imgRef.naturalWidth / imageSize.width; + const scaleY = imgRef.naturalHeight / imageSize.height; - const x = clamp(data.x, 0, natural.width - 1); - const y = clamp(data.y, 0, natural.height - 1); - const width = clamp(data.width, 1, natural.width - x); - const height = clamp(data.height, 1, natural.height - y); + const x = clamp(Math.floor(data.x * scaleX), 0, imgRef.naturalWidth - 1); + const y = clamp(Math.floor(data.y * scaleY), 0, imgRef.naturalHeight - 1); + const width = clamp(Math.floor(data.width * scaleX), 1, imgRef.naturalWidth - x); + const height = clamp(Math.floor(data.height * scaleY), 1, imgRef.naturalHeight - y); if (width <= 0 || height <= 0) { return; @@ -292,7 +276,7 @@ } try { - context.drawImage(htmlElement, x, y, width, height, 0, 0, width, height); + context.drawImage(imgRef, x, y, width, height, 0, 0, width, height); return canvas.toDataURL('image/png'); } catch { return; diff --git a/web/src/lib/utils/container-utils.spec.ts b/web/src/lib/utils/container-utils.spec.ts index d6a1efbe6a..acdc778d10 100644 --- a/web/src/lib/utils/container-utils.spec.ts +++ b/web/src/lib/utils/container-utils.spec.ts @@ -1,18 +1,15 @@ import { - getContentMetrics, + computeContentMetrics, getNaturalSize, + mapContentRectToNatural, mapNormalizedRectToContent, mapNormalizedToContent, scaleToCover, scaleToFit, } from '$lib/utils/container-utils'; -const mockImage = (props: { - naturalWidth: number; - naturalHeight: number; - width: number; - height: number; -}): HTMLImageElement => props as unknown as HTMLImageElement; +const mockImage = (props: { naturalWidth: number; naturalHeight: number }): HTMLImageElement => + props as unknown as HTMLImageElement; const mockVideo = (props: { videoWidth: number; @@ -49,48 +46,85 @@ describe('scaleToFit', () => { }); }); -describe('getContentMetrics', () => { - it('should compute zero offsets when aspect ratios match', () => { - const img = mockImage({ naturalWidth: 1600, naturalHeight: 900, width: 800, height: 450 }); - expect(getContentMetrics(img)).toEqual({ +describe('computeContentMetrics', () => { + it('should return zero metrics for zero-width content', () => { + expect(computeContentMetrics({ width: 0, height: 1080 }, { width: 800, height: 600 })).toEqual({ + contentWidth: 0, + contentHeight: 0, + offsetX: 0, + offsetY: 0, + }); + }); + + it('should return zero metrics for zero-height content', () => { + expect(computeContentMetrics({ width: 1920, height: 0 }, { width: 800, height: 600 })).toEqual({ + contentWidth: 0, + contentHeight: 0, + offsetX: 0, + offsetY: 0, + }); + }); + + it('should center wide content vertically', () => { + expect(computeContentMetrics({ width: 2000, height: 1000 }, { width: 800, height: 600 })).toEqual({ + contentWidth: 800, + contentHeight: 400, + offsetX: 0, + offsetY: 100, + }); + }); + + it('should center tall content horizontally', () => { + expect(computeContentMetrics({ width: 1000, height: 2000 }, { width: 800, height: 600 })).toEqual({ + contentWidth: 300, + contentHeight: 600, + offsetX: 250, + offsetY: 0, + }); + }); + + it('should produce zero offsets when aspect ratios match', () => { + expect(computeContentMetrics({ width: 1600, height: 900 }, { width: 800, height: 450 })).toEqual({ contentWidth: 800, contentHeight: 450, offsetX: 0, offsetY: 0, }); }); +}); - it('should compute horizontal letterbox offsets for tall image', () => { - const img = mockImage({ naturalWidth: 1000, naturalHeight: 2000, width: 800, height: 600 }); - const metrics = getContentMetrics(img); - expect(metrics.contentWidth).toBe(300); - expect(metrics.contentHeight).toBe(600); - expect(metrics.offsetX).toBe(250); - expect(metrics.offsetY).toBe(0); +describe('mapContentRectToNatural', () => { + it('should map a full-content rect back to natural size', () => { + const metrics = { contentWidth: 800, contentHeight: 400, offsetX: 0, offsetY: 100 }; + const rect = mapContentRectToNatural({ left: 0, top: 100, width: 800, height: 400 }, metrics, { + width: 2000, + height: 1000, + }); + expect(rect).toEqual({ left: 0, top: 0, width: 2000, height: 1000 }); }); - it('should compute vertical letterbox offsets for wide image', () => { - const img = mockImage({ naturalWidth: 2000, naturalHeight: 1000, width: 800, height: 600 }); - const metrics = getContentMetrics(img); - expect(metrics.contentWidth).toBe(800); - expect(metrics.contentHeight).toBe(400); - expect(metrics.offsetX).toBe(0); - expect(metrics.offsetY).toBe(100); + it('should map a centered sub-rect to natural coordinates', () => { + const metrics = { contentWidth: 800, contentHeight: 400, offsetX: 0, offsetY: 100 }; + const rect = mapContentRectToNatural({ left: 200, top: 200, width: 400, height: 200 }, metrics, { + width: 2000, + height: 1000, + }); + expect(rect).toEqual({ left: 500, top: 250, width: 1000, height: 500 }); }); - it('should use clientWidth/clientHeight for video elements', () => { - const video = mockVideo({ videoWidth: 1920, videoHeight: 1080, clientWidth: 800, clientHeight: 600 }); - const metrics = getContentMetrics(video); - expect(metrics.contentWidth).toBe(800); - expect(metrics.contentHeight).toBe(450); - expect(metrics.offsetX).toBe(0); - expect(metrics.offsetY).toBe(75); + it('should handle letterboxed content with horizontal offset', () => { + const metrics = { contentWidth: 300, contentHeight: 600, offsetX: 250, offsetY: 0 }; + const rect = mapContentRectToNatural({ left: 250, top: 0, width: 300, height: 600 }, metrics, { + width: 1000, + height: 2000, + }); + expect(rect).toEqual({ left: 0, top: 0, width: 1000, height: 2000 }); }); }); describe('getNaturalSize', () => { it('should return naturalWidth/naturalHeight for images', () => { - const img = mockImage({ naturalWidth: 4000, naturalHeight: 3000, width: 800, height: 600 }); + const img = mockImage({ naturalWidth: 4000, naturalHeight: 3000 }); expect(getNaturalSize(img)).toEqual({ width: 4000, height: 3000 }); }); diff --git a/web/src/lib/utils/container-utils.ts b/web/src/lib/utils/container-utils.ts index 36e260fcc7..9f8be33b9d 100644 --- a/web/src/lib/utils/container-utils.ts +++ b/web/src/lib/utils/container-utils.ts @@ -49,13 +49,6 @@ export const scaleToFit = (dimensions: Size, container: Size): Size => { }; }; -const getElementSize = (element: HTMLImageElement | HTMLVideoElement): Size => { - if (element instanceof HTMLVideoElement) { - return { width: element.clientWidth, height: element.clientHeight }; - } - return { width: element.width, height: element.height }; -}; - export const getNaturalSize = (element: HTMLImageElement | HTMLVideoElement): Size => { if (element instanceof HTMLVideoElement) { return { width: element.videoWidth, height: element.videoHeight }; @@ -63,17 +56,18 @@ export const getNaturalSize = (element: HTMLImageElement | HTMLVideoElement): Si return { width: element.naturalWidth, height: element.naturalHeight }; }; -export const getContentMetrics = (element: HTMLImageElement | HTMLVideoElement): ContentMetrics => { - const natural = getNaturalSize(element); - const client = getElementSize(element); - const { width: contentWidth, height: contentHeight } = scaleToFit(natural, client); +export function computeContentMetrics(content: Size, container: Size): ContentMetrics { + if (content.width === 0 || content.height === 0) { + return { contentWidth: 0, contentHeight: 0, offsetX: 0, offsetY: 0 }; + } + const { width: contentWidth, height: contentHeight } = scaleToFit(content, container); return { contentWidth, contentHeight, - offsetX: (client.width - contentWidth) / 2, - offsetY: (client.height - contentHeight) / 2, + offsetX: (container.width - contentWidth) / 2, + offsetY: (container.height - contentHeight) / 2, }; -}; +} export function mapNormalizedToContent(point: Point, sizeOrMetrics: Size | ContentMetrics): Point { if ('contentWidth' in sizeOrMetrics) { @@ -109,3 +103,25 @@ export function mapNormalizedRectToContent( height: br.y - tl.y, }; } + +function mapContentToNatural(point: Point, metrics: ContentMetrics, naturalSize: Size): Point { + return { + x: ((point.x - metrics.offsetX) / metrics.contentWidth) * naturalSize.width, + y: ((point.y - metrics.offsetY) / metrics.contentHeight) * naturalSize.height, + }; +} + +export function mapContentRectToNatural(rect: Rect, metrics: ContentMetrics, naturalSize: Size): Rect { + const topLeft = mapContentToNatural({ x: rect.left, y: rect.top }, metrics, naturalSize); + const bottomRight = mapContentToNatural( + { x: rect.left + rect.width, y: rect.top + rect.height }, + metrics, + naturalSize, + ); + return { + top: topLeft.y, + left: topLeft.x, + width: bottomRight.x - topLeft.x, + height: bottomRight.y - topLeft.y, + }; +}