From 8e2e1581653edf7b82743d1c2c01ba738dcb5fbb Mon Sep 17 00:00:00 2001 From: webadderall <131426131+webadderall@users.noreply.github.com> Date: Sat, 12 Sep 2026 15:37:05 +1000 Subject: [PATCH] Address preview accuracy review feedback --- src/components/video-editor/VideoPlayback.tsx | 36 +++++++++++------- .../annotationVisibility.test.ts | 38 +++++++++++++++++++ .../videoPlayback/annotationVisibility.ts | 30 +++++++++++++++ ...rnVideoExporter.nativeStaticLayout.test.ts | 38 ++++++++++++++++++- src/lib/exporter/modernVideoExporter.ts | 2 + 5 files changed, 130 insertions(+), 14 deletions(-) create mode 100644 src/components/video-editor/videoPlayback/annotationVisibility.test.ts create mode 100644 src/components/video-editor/videoPlayback/annotationVisibility.ts diff --git a/src/components/video-editor/VideoPlayback.tsx b/src/components/video-editor/VideoPlayback.tsx index 4a007149..07727ec8 100644 --- a/src/components/video-editor/VideoPlayback.tsx +++ b/src/components/video-editor/VideoPlayback.tsx @@ -86,6 +86,10 @@ import { type ZoomRegion, type ZoomTransitionEasing, } from "./types"; +import { + isAnnotationActiveAtTime, + shouldClearSelectedAnnotation, +} from "./videoPlayback/annotationVisibility"; import { DEFAULT_FOCUS } from "./videoPlayback/constants"; import { type CursorFollowCameraState, @@ -1254,6 +1258,22 @@ const VideoPlayback = forwardRef( selectedZoomIdRef.current = selectedZoomId; }, [selectedZoomId]); + useEffect(() => { + if (!selectedAnnotationId || !onSelectAnnotation) { + return; + } + + if ( + shouldClearSelectedAnnotation( + annotationRegions ?? [], + selectedAnnotationId, + Math.round(currentTime * 1000), + ) + ) { + onSelectAnnotation(null); + } + }, [annotationRegions, currentTime, onSelectAnnotation, selectedAnnotationId]); + useEffect(() => { isPlayingRef.current = isPlaying; const bgVideo = bgVideoRef.current; @@ -2808,20 +2828,10 @@ const VideoPlayback = forwardRef( }} > {(() => { + const timeMs = Math.round(currentTime * 1000); const filtered = (annotationRegions || []).filter( - (annotation) => { - if ( - typeof annotation.startMs !== "number" || - typeof annotation.endMs !== "number" - ) - return false; - - const timeMs = Math.round(currentTime * 1000); - return ( - timeMs >= annotation.startMs && - timeMs <= annotation.endMs - ); - }, + (annotation) => + isAnnotationActiveAtTime(annotation, timeMs), ); const sorted = [...filtered].sort( diff --git a/src/components/video-editor/videoPlayback/annotationVisibility.test.ts b/src/components/video-editor/videoPlayback/annotationVisibility.test.ts new file mode 100644 index 00000000..5913b402 --- /dev/null +++ b/src/components/video-editor/videoPlayback/annotationVisibility.test.ts @@ -0,0 +1,38 @@ +import { describe, expect, it } from "vitest"; +import { isAnnotationActiveAtTime, shouldClearSelectedAnnotation } from "./annotationVisibility"; + +describe("isAnnotationActiveAtTime", () => { + it("includes both annotation range boundaries", () => { + const annotation = { startMs: 1_000, endMs: 2_000 }; + + expect(isAnnotationActiveAtTime(annotation, 1_000)).toBe(true); + expect(isAnnotationActiveAtTime(annotation, 2_000)).toBe(true); + }); + + it("excludes timestamps outside the annotation range", () => { + const annotation = { startMs: 1_000, endMs: 2_000 }; + + expect(isAnnotationActiveAtTime(annotation, 999)).toBe(false); + expect(isAnnotationActiveAtTime(annotation, 2_001)).toBe(false); + }); + + it("rejects invalid annotation timing", () => { + expect(isAnnotationActiveAtTime({ startMs: Number.NaN, endMs: 2_000 }, 1_500)).toBe(false); + }); +}); + +describe("shouldClearSelectedAnnotation", () => { + const annotation = { + id: "annotation-1", + startMs: 1_000, + endMs: 2_000, + } as never; + + it("clears selection after the playhead leaves its active range", () => { + expect(shouldClearSelectedAnnotation([annotation], annotation.id, 2_001)).toBe(true); + }); + + it("keeps selection while its annotation is active", () => { + expect(shouldClearSelectedAnnotation([annotation], annotation.id, 1_500)).toBe(false); + }); +}); diff --git a/src/components/video-editor/videoPlayback/annotationVisibility.ts b/src/components/video-editor/videoPlayback/annotationVisibility.ts new file mode 100644 index 00000000..5823c4c3 --- /dev/null +++ b/src/components/video-editor/videoPlayback/annotationVisibility.ts @@ -0,0 +1,30 @@ +import type { AnnotationRegion } from "../types"; + +/** Return whether an annotation is composited at the supplied media timestamp. */ +export function isAnnotationActiveAtTime( + annotation: Pick, + timeMs: number, +): boolean { + return ( + Number.isFinite(annotation.startMs) && + Number.isFinite(annotation.endMs) && + timeMs >= annotation.startMs && + timeMs <= annotation.endMs + ); +} + +/** Return whether the current playhead has left the selected annotation's range. */ +export function shouldClearSelectedAnnotation( + annotations: AnnotationRegion[], + selectedAnnotationId: string | null | undefined, + timeMs: number, +): boolean { + if (!selectedAnnotationId) { + return false; + } + + const selectedAnnotation = annotations.find( + (annotation) => annotation.id === selectedAnnotationId, + ); + return Boolean(selectedAnnotation && !isAnnotationActiveAtTime(selectedAnnotation, timeMs)); +} diff --git a/src/lib/exporter/modernVideoExporter.nativeStaticLayout.test.ts b/src/lib/exporter/modernVideoExporter.nativeStaticLayout.test.ts index 50f0e3ba..b06d0f80 100644 --- a/src/lib/exporter/modernVideoExporter.nativeStaticLayout.test.ts +++ b/src/lib/exporter/modernVideoExporter.nativeStaticLayout.test.ts @@ -1,5 +1,5 @@ import { afterEach, describe, expect, it, vi } from "vitest"; -import type { AudioRegion, SpeedRegion } from "@/components/video-editor/types"; +import type { AudioRegion, SpeedRegion, ZoomRegion } from "@/components/video-editor/types"; import { ModernVideoExporter } from "./modernVideoExporter"; import type { DecodedVideoInfo } from "./streamingDecoder"; @@ -70,6 +70,16 @@ function createExporter(overrides: Record = {}) { wallpaper: string, ) => CanvasGradient | null; getNativeStaticLayoutCursorSize: (contentWidth: number) => number; + getNativeStaticLayoutZoomTelemetry: ( + layout: { + centerOffsetX: number; + centerOffsetY: number; + croppedDisplayWidth: number; + croppedDisplayHeight: number; + }, + totalFrames: number, + cursorTelemetry: undefined, + ) => Array<{ timeMs: number; scale: number; x: number; y: number }> | undefined; }; } @@ -78,6 +88,32 @@ afterEach(() => { }); describe("ModernVideoExporter native static-layout eligibility", () => { + it("uses configured zoom transition durations for native telemetry", () => { + const zoomRegions: ZoomRegion[] = [ + { + id: "zoom-1", + startMs: 0, + endMs: 4_000, + depth: 2, + focus: { cx: 0.5, cy: 0.5 }, + mode: "manual", + }, + ]; + const layout = { + centerOffsetX: 0, + centerOffsetY: 0, + croppedDisplayWidth: 1920, + croppedDisplayHeight: 1080, + }; + const fastExporter = createExporter({ zoomRegions, zoomInDurationMs: 100 }); + const slowExporter = createExporter({ zoomRegions, zoomInDurationMs: 2_000 }); + + const fastSamples = fastExporter.getNativeStaticLayoutZoomTelemetry(layout, 60, undefined); + const slowSamples = slowExporter.getNativeStaticLayoutZoomTelemetry(layout, 60, undefined); + + expect(fastSamples?.[30].scale).toBeGreaterThan(slowSamples?.[30].scale ?? 0); + }); + it("allows native static-layout eligibility for VP9/WebM sources so main can proxy them", () => { const exporter = createExporter(); diff --git a/src/lib/exporter/modernVideoExporter.ts b/src/lib/exporter/modernVideoExporter.ts index f713227e..be3889d8 100644 --- a/src/lib/exporter/modernVideoExporter.ts +++ b/src/lib/exporter/modernVideoExporter.ts @@ -2141,6 +2141,8 @@ export class ModernVideoExporter { zoomRegions, timeMs, connectZooms: this.config.connectZooms, + zoomInDurationMs: this.config.zoomInDurationMs, + zoomOutDurationMs: this.config.zoomOutDurationMs, zoomClassicMode: this.config.zoomClassicMode, cursorTelemetry: cursorTelemetry ?? [], cursorFollowCamera,