diff --git a/CLAUDE.md b/CLAUDE.md index d40ea01..538fccd 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -34,6 +34,8 @@ Physics for educators: `doc/model.md`. Architecture: `doc/implementation-notes.m - `ModelViewTransformFactory.buildModelViewTransform()` maps real-world units to video pixels from coordinate-system position/rotation and calibration endpoints. Moving axes or calibration **re-expresses** digitized track points so they stay pinned on the video. - Velocity and acceleration are **finite-difference estimates** from position series — noise amplifies on differentiation. +- **Time sync is one-way and centralized.** `VideoPlayerNode` links `currentTimeProperty` → `videoElement.currentTime` (guarded by the `scrubbing` flag and a half-frame deadband). Writers of `currentTimeProperty` should *not* seek the element themselves; the video follows the model automatically. +- **`TrackPoint.time` is authoritative, `frame` is derived.** Changing the frame rate renumbers `currentFrameProperty`, so `TrackingModel.retimeTrackPoints()` re-derives every stored point's index from its timestamp (wired in `TrackLabModel`). Lowering the rate can collide two points onto one index; first wins. - Up to 4 concurrent tracks (labels A–Z available; symbols are not reused) with independent colors for graphs and table rows. ## Accessibility @@ -67,7 +69,12 @@ Actual specs: - `tests/track-lab/model/KinematicsComputer.test.ts` - `tests/track-lab/model/TrackingModel.test.ts` -- `tests/memory-leak.test.ts` +- `tests/track-lab/model/TrackRetiming.test.ts` — frame indices re-derived from timestamps on frame-rate change +- `tests/track-lab/model/ModelViewTransformFactory.test.ts` — includes the retransform round-trip (points stay pinned to the same video pixel) +- `tests/track-lab/model/TrackExporter.test.ts` +- `tests/track-lab/model/VideoPlaybackModel.test.ts` +- `tests/track-lab/graph/GraphDataManager.test.ts` — tick spacing stays on the 1-2-5 sequence +- `tests/memory-leak.test.ts` — WeakRef dispose regression, plus listener-count regressions for `PlaybackControlsNode` (an HSlider that is rebuilt but never inserted must still be disposed, or it retains listeners on `currentTimeProperty`) Run `npm test`. CI runs the suite when a `test` script is present. @@ -89,5 +96,5 @@ npm run generate-svg-icon # bouncing-ball icon SVG Extra `src/` root files beyond the standard set, each justified by cross-screen use: `TrackLabIcons.ts`, `TrackLabButton.ts`, `webcam.ts`, and the `src/tracking/` folder (OpenCV Web Worker). -- **OpenCV WASM** requires COOP/COEP headers (configured in Vite dev + production). Sample videos in `public/videos/`. +- **OpenCV WASM** requires COOP/COEP headers (configured in Vite dev + production). Sample videos live in top-level `videos/`, served at `/videos/` by the `serveVideos()` Vite plugin (Range-request support for seeking) and copied to `dist/videos/` on build — they are not under `public/`. - **Wall-clock timers (allowed exception)** — webcam/video code uses raw `setTimeout`/`setInterval` rather than `stepTimer`: camera-init timeout in `webcam.ts`, FPS-sampling interval in `WebcamPanel.ts`, source-switch debounce in `VideoSourceControlNode.ts`. These track real hardware time, independent of the sim clock. diff --git a/README.md b/README.md index 61f8fe2..8983fa3 100644 --- a/README.md +++ b/README.md @@ -10,7 +10,7 @@ export. - Draggable coordinate system and calibration tool for real-world units - OpenCV auto-tracking and manual frame-by-frame digitizing with magnifier - Kinematics graph, data table, measuring tape, and angle tool -- English and French UI, projector color profile, and installable PWA +- English, French, and Spanish UI, projector color profile, and installable PWA - OpenCV.js WASM tracking in a Web Worker (requires COOP/COEP headers) ## Quick Start diff --git a/public/opencv-worker.js b/public/opencv-worker.js index d852178..073223d 100644 --- a/public/opencv-worker.js +++ b/public/opencv-worker.js @@ -112,7 +112,15 @@ self.onmessage = async (event) => { if (templateMat) templateMat.delete(); const roi = new cv.Rect(clampedX, clampedY, roiW, roiH); - templateMat = blurred.roi(roi).clone(); + // roi() returns a new Mat header sharing `blurred`'s buffer; it must be + // deleted explicitly. Only the clone owns memory that outlives this + // message, and WASM heap allocations are never reclaimed by the JS GC. + const roiMat = blurred.roi(roi); + try { + templateMat = roiMat.clone(); + } finally { + roiMat.delete(); + } self.postMessage({ id, @@ -129,7 +137,10 @@ self.onmessage = async (event) => { } } else if (type === 'track') { if (!cv || !templateMat) { - self.postMessage({ id, type: 'track-result', x: null, y: null, confidence: 0 }); + // confidence 0 is below any threshold, so the main thread discards this + // result. x/y are still numbers so the payload matches the declared + // WorkerResponse shape in OpenCVTracker.ts. + self.postMessage({ id, type: 'track-result', x: 0, y: 0, confidence: 0 }); return; } diff --git a/src/i18n/StringManager.ts b/src/i18n/StringManager.ts index 9b3a7af..9815242 100644 --- a/src/i18n/StringManager.ts +++ b/src/i18n/StringManager.ts @@ -36,11 +36,18 @@ function applyStringTest(obj: T): T { } // ── Compile-time key-parity check ───────────────────────────────────────────── -// satisfies errors immediately if either locale file is missing keys from the other. +// satisfies errors immediately if any locale file is missing keys from another. +// Every shipped locale is checked against English in both directions; omitting +// one lets it drift silently, which matters because the language picker +// (supportsDynamicLocale) exposes all of them at runtime. // biome-ignore lint/complexity/noVoid: intentional compile-time type assertion void (stringsEn satisfies typeof stringsFr); // biome-ignore lint/complexity/noVoid: intentional compile-time type assertion void (stringsFr satisfies typeof stringsEn); +// biome-ignore lint/complexity/noVoid: intentional compile-time type assertion +void (stringsEn satisfies typeof stringsEs); +// biome-ignore lint/complexity/noVoid: intentional compile-time type assertion +void (stringsEs satisfies typeof stringsEn); // ── Build the reactive string property tree ─────────────────────────────────── const stringProperties = LocalizedString.getNestedStringProperties({ diff --git a/src/track-lab/graph/GraphDataManager.ts b/src/track-lab/graph/GraphDataManager.ts index 6215525..eb34327 100644 --- a/src/track-lab/graph/GraphDataManager.ts +++ b/src/track-lab/graph/GraphDataManager.ts @@ -20,6 +20,9 @@ export interface GridVisualizationConfig { } export default class GraphDataManager { + /** Upper bound on ticks per axis; spacing escalates until the count fits. */ + private static readonly MAX_TICKS = 20; + // ── Circular buffer ────────────────────────────────────────────────────── // Fixed-size ring buffer so that oldest-point eviction is O(1) rather than // O(n) (Array.shift reindexes every remaining element after removal). @@ -316,7 +319,17 @@ export default class GraphDataManager { spacing = 10 * magnitude; } - return Math.max(spacing, rangeLength / 20); + // Cap the tick count by stepping up through the 1-2-5 sequence rather than + // clamping to rangeLength / MAX_TICKS, which would hand back an arbitrary + // non-round spacing (visible at targetTicks = 15, where the nice value can + // fall just under the cap). + while (rangeLength / spacing > GraphDataManager.MAX_TICKS) { + const decade = 10 ** Math.floor(Math.log10(spacing)); + const step = Math.round(spacing / decade); + spacing = step < 2 ? 2 * decade : step < 5 ? 5 * decade : 10 * decade; + } + + return spacing; } /** diff --git a/src/track-lab/model/TrackLabModel.ts b/src/track-lab/model/TrackLabModel.ts index 8b12383..6ad75a0 100644 --- a/src/track-lab/model/TrackLabModel.ts +++ b/src/track-lab/model/TrackLabModel.ts @@ -45,6 +45,14 @@ export class TrackLabModel { this.overlayTools.modelViewTransformProperty.lazyLink((newMvt, oldMvt) => { this.tracking.retransformTrackPoints(oldMvt, newMvt); }); + + // currentFrameProperty is derived as round(time × fps), so changing the + // frame rate renumbers the *current* frame. Already-digitized points store + // a frame index too, so they must be renumbered from their timestamps in + // the same step or they stop lining up with the frame the UI is on. + this.playback.frameRateProperty.lazyLink((frameRate) => { + this.tracking.retimeTrackPoints(frameRate); + }); } /** diff --git a/src/track-lab/model/TrackingModel.ts b/src/track-lab/model/TrackingModel.ts index 3c25b09..bdf6817 100644 --- a/src/track-lab/model/TrackingModel.ts +++ b/src/track-lab/model/TrackingModel.ts @@ -287,6 +287,59 @@ export class TrackingModel { })); } + /** + * Re-derive every stored point's frame index from its recorded timestamp + * using `frameRate`. Called by TrackLabModel when the frame rate changes. + * + * `time` is the authoritative quantity — it is captured from the video + * element — while `frame` is a derived index that must stay consistent with + * `VideoPlaybackModel.currentFrameProperty`, which recomputes as + * `round(time × fps)`. Without this, changing the frame rate after + * digitizing leaves stored points labelled with indices from the old rate, + * and everything keyed on `frame` (the erase target, the current-frame ring, + * the table row highlight) silently addresses the wrong point. + * + * Lowering the frame rate can map two points onto the same index. Frames are + * unique keys within a track, so a collision keeps the earlier point, + * matching addPointToTrack()'s first-wins policy. + */ + public retimeTrackPoints(frameRate: number): void { + if (!(frameRate > 0)) { + return; + } + const tracks = this.tracksProperty.value; + if (tracks.length === 0) { + return; + } + + this.tracksProperty.value = tracks.map((track) => { + const seen = new Set(); + const points: TrackPoint[] = []; + // Points are already sorted by ascending frame, and frame is monotonic in + // time, so this walks the track in time order — the first point to claim + // an index is the earliest one. + for (const pt of track.points) { + const frame = Math.round(pt.time * frameRate); + if (seen.has(frame)) { + continue; + } + seen.add(frame); + points.push({ ...pt, frame }); + } + return { ...track, points }; + }); + + // The stash holds a point carrying an index from the previous frame rate. + // Restoring it would place it at the wrong frame, so re-derive it too. + const deleted = this.lastDeletedPointProperty.value; + if (deleted) { + this.lastDeletedPointProperty.value = { + trackId: deleted.trackId, + point: { ...deleted.point, frame: Math.round(deleted.point.time * frameRate) }, + }; + } + } + // ── Tracker facade ────────────────────────────────────────────────────── // Views interact with the tracker exclusively through these methods so that // the tracker implementation stays encapsulated inside the model layer. diff --git a/src/track-lab/view/DataTableNode.ts b/src/track-lab/view/DataTableNode.ts index 7daf102..072c98b 100644 --- a/src/track-lab/view/DataTableNode.ts +++ b/src/track-lab/view/DataTableNode.ts @@ -146,8 +146,17 @@ export class DataTableNode extends Node { const link = document.createElement("a"); link.href = url; link.download = `export${this.exportCounter}.csv`; + // The anchor must be in the document for the synthetic click to start + // a download in Firefox, and the object URL must outlive the click: + // revoking it synchronously cancels the download in some browsers. + // Wall-clock setTimeout is deliberate here — this is browser download + // plumbing, not simulation time, so it must not follow the sim clock. + document.body.appendChild(link); link.click(); - URL.revokeObjectURL(url); + setTimeout(() => { + link.remove(); + URL.revokeObjectURL(url); + }, 0); this.exportCounter++; }, }, @@ -196,16 +205,6 @@ export class DataTableNode extends Node { ); }; - const runRebuild = () => { - tableRenderer.rebuild( - tracking.tracksProperty.value, - unitProperty.value, - getTableColors(), - getLabels(), - getA11yLabels(), - ); - }; - const tracksListener = () => runUpdate(); tracking.tracksProperty.link(tracksListener); @@ -215,13 +214,37 @@ export class DataTableNode extends Node { const currentFrameListener = (frame: number) => tableRenderer.setCurrentFrame(frame); playback.currentFrameProperty.link(currentFrameListener); - // Color-theme and locale changes require a full rebuild because cell colours - // and label strings are baked into the DOM. - const tableHeaderBgListener = () => runRebuild(); - TrackLabColors.tableHeaderBackgroundProperty.lazyLink(tableHeaderBgListener); - - const frameStringListener = () => runRebuild(); - dataTableStrings.frameStringProperty.lazyLink(frameStringListener); + // Colour-theme and locale changes are baked into the DOM as CSS strings and + // text nodes, so they need a rebuild. Every contributing property is + // observed rather than one representative of each group: switching colour + // profile or locale updates these properties one at a time, so a single + // representative can fire while its siblings still hold their old values. + // TableRenderer.update() compares the full colour/label snapshot and + // rebuilds when it differs, so the last notification wins with final values. + const themeProperties = [ + TrackLabColors.tableHeaderBackgroundProperty, + TrackLabColors.tableHeaderTextProperty, + TrackLabColors.tableRowOddProperty, + TrackLabColors.tableRowEvenProperty, + TrackLabColors.tableGridStrokeProperty, + TrackLabColors.tableEmptyTextProperty, + TrackLabColors.tableSymbolShadowProperty, + TrackLabColors.tableBackgroundProperty, + TrackLabColors.tableCurrentRowProperty, + ]; + const localeProperties = [ + dataTableStrings.frameStringProperty, + dataTableStrings.timeSecondsStringProperty, + dataTableStrings.noDataStringProperty, + a11yStrings.dataTableStringProperty, + ]; + const themeOrLocaleListener = () => runUpdate(); + for (const property of themeProperties) { + property.lazyLink(themeOrLocaleListener); + } + for (const property of localeProperties) { + property.lazyLink(themeOrLocaleListener); + } const videoLoadedListener = (loaded: boolean) => { this.visible = loaded; @@ -420,8 +443,12 @@ export class DataTableNode extends Node { tracking.tracksProperty.unlink(tracksListener); unitProperty.unlink(unitListener); playback.currentFrameProperty.unlink(currentFrameListener); - TrackLabColors.tableHeaderBackgroundProperty.unlink(tableHeaderBgListener); - dataTableStrings.frameStringProperty.unlink(frameStringListener); + for (const property of themeProperties) { + property.unlink(themeOrLocaleListener); + } + for (const property of localeProperties) { + property.unlink(themeOrLocaleListener); + } videoLoadedProperty.unlink(videoLoadedListener); exportButton.dispose(); panel.localBoundsProperty.unlink(localBoundsListener); diff --git a/src/track-lab/view/KinematicsGraphNode.ts b/src/track-lab/view/KinematicsGraphNode.ts index 5ad83db..28b624f 100644 --- a/src/track-lab/view/KinematicsGraphNode.ts +++ b/src/track-lab/view/KinematicsGraphNode.ts @@ -6,7 +6,7 @@ * The velocity and acceleration groups can be hidden via user preferences. */ -import { Property, type TReadOnlyProperty } from "scenerystack/axon"; +import { DerivedProperty, Property, type TReadOnlyProperty } from "scenerystack/axon"; import { Node, Text, VBox } from "scenerystack/scenery"; import { PhetFont } from "scenerystack/scenery-phet"; import { Checkbox } from "scenerystack/sun"; @@ -35,7 +35,12 @@ export class KinematicsGraphNode extends Node { private readonly tracking: TrackingModel; private readonly selectedTracksProperty: Property>; private readonly trackCheckboxPanel: Node; - private readonly trackCheckboxes: Map }> = new Map(); + private readonly trackCheckboxes: Map< + string, + { checkbox: Checkbox; property: Property; nameProperty: TReadOnlyProperty } + > = new Map(); + /** VBox wrapping the current checkboxes; disposed and replaced on each rebuild. */ + private checkboxContainer: VBox | null = null; private readonly disposeKinematicsGraph: () => void; public constructor( @@ -108,9 +113,10 @@ export class KinematicsGraphNode extends Node { // Update checkboxes when tracks change (link fires immediately, building initial checkboxes) const tracksListener = (tracks: readonly Track[]) => { // Dispose old checkboxes - for (const [, { checkbox, property }] of this.trackCheckboxes) { + for (const [, { checkbox, property, nameProperty }] of this.trackCheckboxes) { checkbox.dispose(); property.dispose(); + nameProperty.dispose(); } this.trackCheckboxes.clear(); @@ -191,11 +197,14 @@ export class KinematicsGraphNode extends Node { preferencesModel.showAccelerationInGraphProperty.unlink(accelerationPrefListener); videoLoadedProperty.unlink(videoLoadedListener); this.graph.localBoundsProperty.unlink(graphBoundsListener); - for (const [, { checkbox, property }] of this.trackCheckboxes) { + for (const [, { checkbox, property, nameProperty }] of this.trackCheckboxes) { checkbox.dispose(); property.dispose(); + nameProperty.dispose(); } this.trackCheckboxes.clear(); + this.checkboxContainer?.dispose(); + this.checkboxContainer = null; this.selectedTracksProperty.dispose(); this.graph.dispose(); }; @@ -213,8 +222,13 @@ export class KinematicsGraphNode extends Node { private rebuildTrackCheckboxes(): void { const tracks = this.tracking.tracksProperty.value; + // The previous container is detached here and disposed either way; leaving + // it to be garbage collected would leak a VBox on every track change. + this.trackCheckboxPanel.children = []; + this.checkboxContainer?.dispose(); + this.checkboxContainer = null; + if (tracks.length === 0) { - this.trackCheckboxPanel.children = []; return; } @@ -241,18 +255,23 @@ export class KinematicsGraphNode extends Node { fill: getTrackColor(track.colorIndex), }); - // Create checkbox with accessibility label + // Create checkbox with accessibility label. Derived rather than a + // `.value` snapshot so the name re-translates when the user switches + // language (the sim enables supportsDynamicLocale). const kinematicsGraphStrings = StringManager.getInstance().getKinematicsGraph(); + const nameProperty = new DerivedProperty([kinematicsGraphStrings.trackItemStringProperty], (pattern) => + pattern.split("{{symbol}}").join(track.symbol), + ); const checkbox = new Checkbox(checkboxProperty, label, { checkboxColor: TrackLabColors.checkboxColorProperty, checkboxColorBackground: TrackLabColors.checkboxColorBackgroundProperty, spacing: 4, - accessibleName: kinematicsGraphStrings.trackItemStringProperty.value.split("{{symbol}}").join(track.symbol), + accessibleName: nameProperty, }); checkbox.addInputListener({ down: () => checkbox.focus() }); // Store for later disposal - this.trackCheckboxes.set(track.id, { checkbox, property: checkboxProperty }); + this.trackCheckboxes.set(track.id, { checkbox, property: checkboxProperty, nameProperty }); checkboxNodes.push(checkbox); } @@ -264,6 +283,7 @@ export class KinematicsGraphNode extends Node { align: "left", }); + this.checkboxContainer = checkboxContainer; this.trackCheckboxPanel.children = [checkboxContainer]; this.updateCheckboxPositions(); } diff --git a/src/track-lab/view/PlaybackControlsNode.ts b/src/track-lab/view/PlaybackControlsNode.ts index 3010097..04015af 100644 --- a/src/track-lab/view/PlaybackControlsNode.ts +++ b/src/track-lab/view/PlaybackControlsNode.ts @@ -144,12 +144,22 @@ export class PlaybackControlsNode extends HBox { minorTickLength: 4, minorTickStroke: TrackLabColors.textOnDarkProperty, minorTickLineWidth: 1, + // startDrag/endDrag fire for pointer interaction only. Keyboard input + // goes through AccessibleValueHandler's startInput/endInput, so both + // pairs are wired — otherwise arrow-key scrubbing would leave + // isScrubbing false and the video would never follow the thumb. startDrag: () => { this.isScrubbing = true; }, endDrag: () => { this.isScrubbing = false; }, + startInput: () => { + this.isScrubbing = true; + }, + endInput: () => { + this.isScrubbing = false; + }, enabledProperty: playback.videoLoadedProperty, accessibleName: a11yStrings.videoScrubberStringProperty, }); @@ -185,14 +195,20 @@ export class PlaybackControlsNode extends HBox { this.scrubber = createScrubber(); - // Update range and ticks when duration changes + // Update range and ticks when duration changes. + // lazyLink, not link: the scrubber built just above already reflects the + // current duration, and a synchronous first firing would run + // replaceScrubber() before this.children is assigned at the end of the + // constructor — indexOf() would return -1 and the freshly built slider + // would be dropped undisposed, permanently retaining its listeners on + // currentTimeProperty. const durationListener = (duration: number) => { // Update scrubber range — guard against Infinity (reported by WebM files) this.scrubberRange.max = Number.isFinite(duration) && duration > 0 ? duration : 1; // Recreate scrubber with new tick marks this.replaceScrubber(createScrubber()); }; - playback.durationProperty.link(durationListener); + playback.durationProperty.lazyLink(durationListener); // Recreate scrubber when frame rate changes const frameRateListener = () => { @@ -352,19 +368,26 @@ export class PlaybackControlsNode extends HBox { const oldScrubber = this.scrubber; const scrubberIndex = this.children.indexOf(oldScrubber); - if (scrubberIndex !== -1) { - // Remove old scrubber from scene graph first - this.removeChild(oldScrubber); + if (scrubberIndex === -1) { + // The current scrubber is not in the scene graph, so there is nothing to + // swap it with. Dispose the replacement rather than dropping it on the + // floor: an undisposed HSlider keeps listeners on currentTimeProperty + // alive forever. + newScrubber.dispose(); + return; + } - // Update reference - this.scrubber = newScrubber; + // Remove old scrubber from scene graph first + this.removeChild(oldScrubber); - // Insert new scrubber at the same position - this.insertChild(scrubberIndex, newScrubber); + // Update reference + this.scrubber = newScrubber; - // Dispose old scrubber after it's removed from scene graph - oldScrubber.dispose(); - } + // Insert new scrubber at the same position + this.insertChild(scrubberIndex, newScrubber); + + // Dispose old scrubber after it's removed from scene graph + oldScrubber.dispose(); } public get scrubbing(): boolean { diff --git a/src/track-lab/view/TableRenderer.ts b/src/track-lab/view/TableRenderer.ts index 203dbd8..f6468d1 100644 --- a/src/track-lab/view/TableRenderer.ts +++ b/src/track-lab/view/TableRenderer.ts @@ -75,6 +75,26 @@ export type A11yLabels = { // ── Private helpers ─────────────────────────────────────────────────────────── +/** Stable serialisation of every colour baked into the table DOM. */ +function colorsKey(colors: TableColors): string { + return [ + colors.headerBg, + colors.headerText, + colors.rowOdd, + colors.rowEven, + colors.gridStroke, + colors.emptyText, + colors.symbolShadow, + colors.background, + colors.currentRow, + ].join("|"); +} + +/** Stable serialisation of every string baked into the table DOM. */ +function labelsKey(labels: TableLabels, a11y: A11yLabels): string { + return [labels.frame, labels.timeSeconds, labels.noData, a11y.tableCaption].join("|"); +} + function makeCellStyle(colors: TableColors): string { return `padding: ${TABLE_CELL_PADDING_Y}px ${TABLE_CELL_PADDING_X}px; border: 1px solid ${colors.gridStroke}; text-align: center;`; } @@ -262,6 +282,13 @@ export class TableRenderer { // ── Incremental-update state ─────────────────────────────────────────────── private lastTrackIds: string[] = []; private lastUnit: string = ""; + // Colours and labels are baked into the DOM as CSS strings and text nodes, so + // a change to any of them needs a structural rebuild. Comparing serialised + // snapshots means correctness does not depend on *which* colour or string + // property happened to fire the update: whichever notification arrives last + // with the final values is the one that rebuilds. + private lastColorsKey: string = ""; + private lastLabelsKey: string = ""; private tableBodyRef: HTMLTableSectionElement | null = null; private readonly frameRowMap: Map = new Map(); private maxRenderedFrame: number = -Infinity; @@ -402,6 +429,8 @@ export class TableRenderer { this.lastTrackIds = tracks.map((t) => t.id); this.lastUnit = unit; + this.lastColorsKey = colorsKey(colors); + this.lastLabelsKey = labelsKey(labels, a11y); // The rebuild discarded the highlighted ; restore it on the new DOM. // No scroll — the frame has not moved, only the rows around it. @@ -429,7 +458,9 @@ export class TableRenderer { const isStructural = unit !== this.lastUnit || trackIds.length !== this.lastTrackIds.length || - trackIds.some((id, i) => id !== this.lastTrackIds[i]); + trackIds.some((id, i) => id !== this.lastTrackIds[i]) || + colorsKey(colors) !== this.lastColorsKey || + labelsKey(labels, a11y) !== this.lastLabelsKey; if (isStructural || !this.tableBodyRef) { this.rebuild(tracks, unit, colors, labels, a11y); diff --git a/src/track-lab/view/TrackListPanel.ts b/src/track-lab/view/TrackListPanel.ts index 51e7f79..71fc419 100644 --- a/src/track-lab/view/TrackListPanel.ts +++ b/src/track-lab/view/TrackListPanel.ts @@ -104,10 +104,19 @@ class TrackRowNode extends Node { }; isDigitizingProperty.lazyLink(digitizingListener); + // Derived rather than a `.value` snapshot so the name re-translates when the + // user switches language (the sim enables supportsDynamicLocale). + const digitizeNameProperty = new DerivedProperty([a11yStrings.digitizeTrackStringProperty], (pattern) => + pattern.split("{{symbol}}").join(track.symbol), + ); + const removeNameProperty = new DerivedProperty([a11yStrings.removeTrackStringProperty], (pattern) => + pattern.split("{{symbol}}").join(track.symbol), + ); + const checkbox = new Checkbox(isDigitizingProperty, new Rectangle(0, 0, 0, 0), { boxWidth: CHECKBOX_BOX_WIDTH, tandem: Tandem.OPT_OUT, - accessibleName: a11yStrings.digitizeTrackStringProperty.value.split("{{symbol}}").join(track.symbol), + accessibleName: digitizeNameProperty, }); checkbox.addInputListener({ down: () => checkbox.focus() }); checkbox.left = CHECKBOX_X; @@ -117,7 +126,7 @@ class TrackRowNode extends Node { const trashButton = createTrackLabButton(makeTrashIcon(), { baseColor: TrackLabColors.trashButtonBaseProperty, listener: () => tracking.removeTrack(track.id), - accessibleName: a11yStrings.removeTrackStringProperty.value.split("{{symbol}}").join(track.symbol), + accessibleName: removeNameProperty, }); trashButton.centerY = ROW_CY; trashButton.right = PANEL_WIDTH - TRASH_BUTTON_RIGHT_OFFSET; @@ -135,6 +144,8 @@ class TrackRowNode extends Node { checkbox.dispose(); trashButton.dispose(); isDigitizingProperty.dispose(); + digitizeNameProperty.dispose(); + removeNameProperty.dispose(); }; } diff --git a/src/track-lab/view/VideoPlayerNode.ts b/src/track-lab/view/VideoPlayerNode.ts index 8c2fc29..5b88780 100644 --- a/src/track-lab/view/VideoPlayerNode.ts +++ b/src/track-lab/view/VideoPlayerNode.ts @@ -85,7 +85,12 @@ export class VideoPlayerNode extends Node { this.videoElement.preload = "metadata"; this.videoElement.crossOrigin = "anonymous"; this.videoElement.style.display = "block"; - this.videoElement.setAttribute("aria-label", a11yStrings.videoPlayerStringProperty.value); + // Linked rather than snapshotted so the label re-translates when the user + // switches language (the sim enables supportsDynamicLocale). + const videoAriaLabelListener = (label: string) => { + this.videoElement.setAttribute("aria-label", label); + }; + a11yStrings.videoPlayerStringProperty.link(videoAriaLabelListener); const videoBackgroundListener = (c: import("scenerystack").Color) => { this.videoElement.style.background = c.toCSS(); }; @@ -373,6 +378,32 @@ export class VideoPlayerNode extends Node { }; this.videoElement.addEventListener("timeupdate", onTimeUpdate, { signal }); + // ── Sync video element from model time (model → video) ───────────────── + // Every writer of currentTimeProperty used to be responsible for also + // assigning videoElement.currentTime itself. Two writers did not — the + // data-table row click and the scrubber's *keyboard* interaction (Scenery's + // Slider routes keyboard input through startInput/endInput, not the + // startDrag/endDrag that set `scrubbing`) — so the model clock moved while + // the displayed frame stayed put. This single listener makes the sync + // structural instead of a convention each call site has to remember. + // + // Two guards keep it from fighting the other direction of the loop: + // - `scrubbing`: pointer drags are already synced in PlaybackControlsNode. + // - half-frame deadband: during playback onTimeUpdate writes the video's + // own time into the model, which lands back here; seeking the element + // to the time it already holds would stutter playback. + const syncVideoFromModelTime = (time: number) => { + if (this.playbackControlsNode.scrubbing || !Number.isFinite(time)) { + return; + } + const halfFrame = model.playback.frameDurationProperty.value / 2; + if (Math.abs(this.videoElement.currentTime - time) < halfFrame) { + return; + } + this.videoElement.currentTime = time; + }; + model.playback.currentTimeProperty.lazyLink(syncVideoFromModelTime); + // ── Apply video transform (translate + uniform scale) ──────────────── const videoTransformListener = (matrix: import("scenerystack/dot").Matrix3) => { this.videoContentLayer.matrix = matrix; @@ -394,6 +425,8 @@ export class VideoPlayerNode extends Node { model.playback.playbackRateProperty.unlink(playbackRateListener); model.playback.videoTransformProperty.unlink(videoTransformListener); model.playback.panelSizeScaleProperty.unlink(panelSizeScaleListener); + model.playback.currentTimeProperty.unlink(syncVideoFromModelTime); + a11yStrings.videoPlayerStringProperty.unlink(videoAriaLabelListener); if (this.currentBlobUrl) { URL.revokeObjectURL(this.currentBlobUrl); this.currentBlobUrl = null; diff --git a/src/tracking/OpenCVTracker.ts b/src/tracking/OpenCVTracker.ts index e0fc483..08c2f43 100644 --- a/src/tracking/OpenCVTracker.ts +++ b/src/tracking/OpenCVTracker.ts @@ -162,8 +162,21 @@ export class OpenCVTracker { } } - /** Send a message to the worker and return a Promise that resolves with the response. */ + /** + * Send a message to the worker and return a Promise that resolves with the response. + * + * Only one request may be in flight at a time. Callers currently serialise + * their own calls, but overwriting the pending handlers without settling them + * would strand the earlier Promise forever (an `await` that never returns), + * so a superseded request is rejected explicitly. + */ private send(msg: object): Promise { + if (this.pendingReject) { + this.pendingReject(new Error("Tracker request superseded by a newer request")); + this.pendingResolve = null; + this.pendingReject = null; + this.pendingId = -1; + } const id = this.nextMsgId++; return new Promise((resolve, reject) => { this.pendingId = id; diff --git a/tests/memory-leak.test.ts b/tests/memory-leak.test.ts index 3cb5037..56ffb83 100644 --- a/tests/memory-leak.test.ts +++ b/tests/memory-leak.test.ts @@ -7,10 +7,13 @@ * a block scope) so local strong references die when the helper returns. */ -import { Property } from "scenerystack/axon"; +import { Property, type TReadOnlyProperty } from "scenerystack/axon"; import { describe, expect, it } from "vitest"; import GraphControlsPanel from "../src/track-lab/graph/GraphControlsPanel.js"; import type { PlottableProperty } from "../src/track-lab/graph/PlottableProperty.js"; +import { TrackingModel } from "../src/track-lab/model/TrackingModel.js"; +import { VideoPlaybackModel } from "../src/track-lab/model/VideoPlaybackModel.js"; +import { PlaybackControlsNode } from "../src/track-lab/view/PlaybackControlsNode.js"; /** * Force garbage collection with multiple passes. When `earlyExitRef` is supplied @@ -46,6 +49,24 @@ const POSITION_PLOTTABLE: PlottableProperty = { const SAMPLE_PLOTTABLES: PlottableProperty[] = [TIME_PLOTTABLE, POSITION_PLOTTABLE]; +/** + * Number of listeners currently attached to `property`. + * + * Axon's TinyEmitter exposes getListenerCount() publicly but ReadOnlyProperty + * re-declares it private, so the cast is deliberate. The public alternative, + * hasListeners(), is too coarse: these properties always carry listeners from + * the model's own DerivedProperties, so only the exact count distinguishes a + * clean dispose from a retained one. + */ +function listenerCount(property: TReadOnlyProperty): number { + return (property as unknown as { getListenerCount(): number }).getListenerCount(); +} + +/** Shared no-op callback for view constructors under test. */ +const noop = (): void => { + /* no-op */ +}; + function createAndDisposeGraphControlsPanel(): WeakRef { // Explicit type argument: TypeScript narrows these consts to RecordPlottable // from their initializers, and Property is invariant in T. @@ -86,6 +107,45 @@ describe("Memory leak regression", () => { yPropertyProperty.dispose(); }); + // PlaybackControlsNode builds an HSlider bound to currentTimeProperty and + // rebuilds it whenever duration, frame rate, or frame count changes. A + // replacement that is never inserted into the scene graph must still be + // disposed: an orphaned HSlider keeps its listeners on the model property + // alive for the lifetime of the model, which no WeakRef on the node catches. + it("PlaybackControlsNode leaves no listeners on currentTimeProperty after dispose", () => { + const playback = new VideoPlaybackModel(); + const tracking = new TrackingModel(); + const videoElement = document.createElement("video"); + + const baseline = listenerCount(playback.currentTimeProperty); + + for (let i = 0; i < 3; i++) { + const node = new PlaybackControlsNode(playback, tracking, videoElement, noop, noop, noop); + node.dispose(); + } + + expect(listenerCount(playback.currentTimeProperty)).toBe(baseline); + }); + + it("PlaybackControlsNode scrubber rebuilds do not accumulate listeners", () => { + const playback = new VideoPlaybackModel(); + const tracking = new TrackingModel(); + const videoElement = document.createElement("video"); + + const baseline = listenerCount(playback.currentTimeProperty); + const node = new PlaybackControlsNode(playback, tracking, videoElement, noop, noop, noop); + + const afterConstruct = listenerCount(playback.currentTimeProperty); + // Each of these triggers replaceScrubber(). + playback.durationProperty.value = 10; + playback.durationProperty.value = 20; + playback.frameRateProperty.value = 60; + expect(listenerCount(playback.currentTimeProperty)).toBe(afterConstruct); + + node.dispose(); + expect(listenerCount(playback.currentTimeProperty)).toBe(baseline); + }); + it("repeated create/dispose cycles leave no survivors", async () => { const refs: WeakRef[] = []; for (let i = 0; i < 10; i++) { diff --git a/tests/track-lab/graph/GraphDataManager.test.ts b/tests/track-lab/graph/GraphDataManager.test.ts new file mode 100644 index 0000000..e4c6419 --- /dev/null +++ b/tests/track-lab/graph/GraphDataManager.test.ts @@ -0,0 +1,48 @@ +import { describe, expect, it } from "vitest"; +import GraphDataManager from "../../../src/track-lab/graph/GraphDataManager.js"; + +/** True for values of the form (1|2|5) x 10^k, the "nice number" sequence. */ +function isNiceNumber(value: number): boolean { + const decade = 10 ** Math.floor(Math.log10(value)); + const mantissa = value / decade; + return [1, 2, 5, 10].some((nice) => Math.abs(mantissa - nice) < 1e-9); +} + +describe("GraphDataManager.calculateTickSpacing", () => { + it("falls back to 1 for degenerate ranges", () => { + expect(GraphDataManager.calculateTickSpacing(0)).toBe(1); + expect(GraphDataManager.calculateTickSpacing(-5)).toBe(1); + expect(GraphDataManager.calculateTickSpacing(Number.NaN)).toBe(1); + expect(GraphDataManager.calculateTickSpacing(Number.POSITIVE_INFINITY)).toBe(1); + }); + + it("returns round numbers at the default tick target", () => { + for (const range of [1, 3, 7, 10, 47, 100, 250, 1000, 0.05]) { + expect(isNiceNumber(GraphDataManager.calculateTickSpacing(range))).toBe(true); + } + }); + + it("returns round numbers at the scrubber's 15-tick target", () => { + // Regression: the old rangeLength / 20 floor could override the nice value + // here and hand back arbitrary spacing like 15.35. + for (const totalFrames of [21, 33, 44, 52, 62, 176, 191, 249, 307, 328, 1000]) { + const spacing = GraphDataManager.calculateTickSpacing(totalFrames, 15); + expect(isNiceNumber(spacing)).toBe(true); + } + }); + + it("never produces more than 20 ticks", () => { + for (const targetTicks of [5, 10, 15, 20]) { + for (const range of [1, 17, 33, 99, 250, 1234]) { + const spacing = GraphDataManager.calculateTickSpacing(range, targetTicks); + expect(range / spacing).toBeLessThanOrEqual(20 + 1e-9); + } + } + }); + + it("stays near the requested tick count", () => { + const spacing = GraphDataManager.calculateTickSpacing(100, 5); + expect(100 / spacing).toBeGreaterThanOrEqual(2); + expect(100 / spacing).toBeLessThanOrEqual(10); + }); +}); diff --git a/tests/track-lab/model/ModelViewTransformFactory.test.ts b/tests/track-lab/model/ModelViewTransformFactory.test.ts new file mode 100644 index 0000000..f07b31c --- /dev/null +++ b/tests/track-lab/model/ModelViewTransformFactory.test.ts @@ -0,0 +1,102 @@ +import { type Transform3, Vector2 } from "scenerystack/dot"; +import { describe, expect, it } from "vitest"; +import { buildModelViewTransform } from "../../../src/track-lab/model/ModelViewTransformFactory.js"; +import { TrackingModel } from "../../../src/track-lab/model/TrackingModel.js"; + +/** 100 px maps to 1 model unit, origin at (100, 100), no rotation. */ +function standardTransform(): Transform3 { + return buildModelViewTransform(new Vector2(100, 100), 0, new Vector2(0, 0), new Vector2(100, 0), 1); +} + +describe("buildModelViewTransform", () => { + it("places the model origin at the coordinate-system position", () => { + const mvt = standardTransform(); + const pixel = mvt.transformPosition2(new Vector2(0, 0)); + + expect(pixel.x).toBeCloseTo(100); + expect(pixel.y).toBeCloseTo(100); + }); + + it("scales model units to pixels using the calibration length", () => { + const mvt = standardTransform(); + const pixel = mvt.transformPosition2(new Vector2(2, 0)); + + expect(pixel.x).toBeCloseTo(300); // 100 px origin + 2 units x 100 px/unit + }); + + it("flips y so that model +y points up the screen", () => { + const mvt = standardTransform(); + const pixel = mvt.transformPosition2(new Vector2(0, 1)); + + // Screen y grows downward, so a positive model y lands above the origin. + expect(pixel.y).toBeCloseTo(0); + }); + + it("round-trips a pixel through inverse and forward transforms", () => { + const mvt = buildModelViewTransform(new Vector2(37, 91), 0.7, new Vector2(10, 20), new Vector2(150, 60), 2.5); + const pixel = new Vector2(400, 250); + + const roundTripped = mvt.transformPosition2(mvt.inversePosition2(pixel)); + + expect(roundTripped.x).toBeCloseTo(pixel.x); + expect(roundTripped.y).toBeCloseTo(pixel.y); + }); + + it("returns identity when the calibration segment has zero length", () => { + const mvt = buildModelViewTransform(new Vector2(50, 50), 0, new Vector2(10, 10), new Vector2(10, 10), 1); + const pixel = mvt.transformPosition2(new Vector2(7, 8)); + + expect(pixel.x).toBeCloseTo(7); + expect(pixel.y).toBeCloseTo(8); + }); + + it("returns identity when the calibration distance is degenerate", () => { + const mvt = buildModelViewTransform(new Vector2(50, 50), 0, new Vector2(0, 0), new Vector2(100, 0), 0); + const pixel = mvt.transformPosition2(new Vector2(7, 8)); + + expect(pixel.x).toBeCloseTo(7); + expect(pixel.y).toBeCloseTo(8); + }); +}); + +describe("retransformTrackPoints", () => { + it("keeps every digitized point pinned to the same video pixel", () => { + const model = new TrackingModel(); + model.addTrack(); + + const oldMvt = standardTransform(); + // Digitize three points expressed in the OLD transform's model coordinates. + const originalPixels = [new Vector2(150, 120), new Vector2(300, 400), new Vector2(20, 60)]; + originalPixels.forEach((pixel, i) => { + const modelPt = oldMvt.inversePosition2(pixel); + model.addOrReplacePointOnTrack("track-A", i, i, modelPt.x, modelPt.y); + }); + + // Move the origin, rotate the axes, and rescale the calibration. + const newMvt = buildModelViewTransform(new Vector2(250, 60), 0.4, new Vector2(0, 0), new Vector2(37, 0), 3); + model.retransformTrackPoints(oldMvt, newMvt); + + const points = model.tracksProperty.value[0]?.points ?? []; + expect(points).toHaveLength(3); + points.forEach((pt, i) => { + const pixel = newMvt.transformPosition2(new Vector2(pt.x, pt.y)); + expect(pixel.x).toBeCloseTo(originalPixels[i]?.x ?? Number.NaN); + expect(pixel.y).toBeCloseTo(originalPixels[i]?.y ?? Number.NaN); + }); + }); + + it("leaves frame and time untouched", () => { + const model = new TrackingModel(); + model.addTrack(); + model.addOrReplacePointOnTrack("track-A", 7, 0.25, 1, 1); + + model.retransformTrackPoints( + standardTransform(), + buildModelViewTransform(new Vector2(0, 0), 1, new Vector2(0, 0), new Vector2(50, 0), 1), + ); + + const point = model.tracksProperty.value[0]?.points[0]; + expect(point?.frame).toBe(7); + expect(point?.time).toBe(0.25); + }); +}); diff --git a/tests/track-lab/model/TrackExporter.test.ts b/tests/track-lab/model/TrackExporter.test.ts new file mode 100644 index 0000000..f8b5538 --- /dev/null +++ b/tests/track-lab/model/TrackExporter.test.ts @@ -0,0 +1,89 @@ +import { describe, expect, it } from "vitest"; +import type { Track } from "../../../src/track-lab/model/Track.js"; +import { buildDataRows, generateCsv } from "../../../src/track-lab/model/TrackExporter.js"; + +const LABELS = { frame: "Frame", timeSeconds: "t (s)" }; + +function track(id: string, symbol: string, points: Array<[number, number, number, number]>): Track { + return { + id, + symbol, + colorIndex: 0, + points: points.map(([frame, time, x, y]) => ({ frame, time, x, y })), + }; +} + +describe("buildDataRows", () => { + it("returns no rows when there are no tracks", () => { + expect(buildDataRows([])).toEqual([]); + }); + + it("returns no rows when tracks carry no points", () => { + expect(buildDataRows([track("track-A", "A", [])])).toEqual([]); + }); + + it("sorts rows ascending by frame regardless of insertion order", () => { + const a = track("track-A", "A", [ + [5, 0.5, 1, 1], + [0, 0, 2, 2], + [3, 0.3, 3, 3], + ]); + + expect(buildDataRows([a]).map((r) => r.frame)).toEqual([0, 3, 5]); + }); + + it("merges two tracks that share a frame into one row", () => { + const a = track("track-A", "A", [[2, 0.2, 1, 1]]); + const b = track("track-B", "B", [[2, 0.2, 9, 9]]); + + const rows = buildDataRows([a, b]); + + expect(rows).toHaveLength(1); + expect(rows[0]?.values.get("track-A")).toEqual({ x: 1, y: 1 }); + expect(rows[0]?.values.get("track-B")).toEqual({ x: 9, y: 9 }); + }); + + it("keeps frames that only one track recorded", () => { + const a = track("track-A", "A", [ + [0, 0, 1, 1], + [1, 0.1, 2, 2], + ]); + const b = track("track-B", "B", [[1, 0.1, 8, 8]]); + + const rows = buildDataRows([a, b]); + + expect(rows.map((r) => r.frame)).toEqual([0, 1]); + expect(rows[0]?.values.has("track-B")).toBe(false); + }); +}); + +describe("generateCsv", () => { + it("emits a header per track and one line per frame", () => { + const a = track("track-A", "A", [ + [0, 0, 1, 2], + [1, 0.5, 3, 4], + ]); + + const lines = generateCsv([a], "m", LABELS).split("\n"); + + expect(lines[0]).toBe("Frame,t (s),x_A (m),y_A (m)"); + expect(lines).toHaveLength(3); + expect(lines[1]).toBe("0,0.0000,1.0000,2.0000"); + }); + + it("leaves cells empty for a track with no point at that frame", () => { + const a = track("track-A", "A", [[0, 0, 1, 2]]); + const b = track("track-B", "B", [[1, 0.5, 3, 4]]); + + const lines = generateCsv([a, b], "cm", LABELS).split("\n"); + + // Frame 0: track A has data, track B does not. + expect(lines[1]).toBe("0,0.0000,1.0000,2.0000,,"); + // Frame 1: the reverse. + expect(lines[2]).toBe("1,0.5000,,,3.0000,4.0000"); + }); + + it("emits only the header row when there is no data", () => { + expect(generateCsv([], "m", LABELS)).toBe("Frame,t (s)"); + }); +}); diff --git a/tests/track-lab/model/TrackRetiming.test.ts b/tests/track-lab/model/TrackRetiming.test.ts new file mode 100644 index 0000000..f54aa0c --- /dev/null +++ b/tests/track-lab/model/TrackRetiming.test.ts @@ -0,0 +1,110 @@ +/** + * Regression tests for frame-index retiming. + * + * TrackPoint.time is authoritative (captured from the video element) while + * TrackPoint.frame is a derived index that has to stay consistent with + * VideoPlaybackModel.currentFrameProperty. Changing the frame rate renumbers + * the current frame, so stored points must be renumbered in the same step. + */ + +import { beforeEach, describe, expect, it } from "vitest"; +import { TrackingModel } from "../../../src/track-lab/model/TrackingModel.js"; +import { TrackLabModel } from "../../../src/track-lab/model/TrackLabModel.js"; + +function frames(model: TrackingModel, trackId = "track-A"): number[] { + const track = model.tracksProperty.value.find((t) => t.id === trackId); + return (track?.points ?? []).map((p) => p.frame); +} + +describe("TrackingModel.retimeTrackPoints", () => { + let model: TrackingModel; + + beforeEach(() => { + model = new TrackingModel(); + model.addTrack(); + }); + + it("re-derives frame indices from timestamps", () => { + // Digitized at 30 fps: frames 0, 30, 60 at t = 0 s, 1 s, 2 s. + model.addOrReplacePointOnTrack("track-A", 0, 0, 1, 1); + model.addOrReplacePointOnTrack("track-A", 30, 1, 2, 2); + model.addOrReplacePointOnTrack("track-A", 60, 2, 3, 3); + + model.retimeTrackPoints(60); + + expect(frames(model)).toEqual([0, 60, 120]); + }); + + it("leaves timestamps and coordinates untouched", () => { + model.addOrReplacePointOnTrack("track-A", 30, 1, 4, 5); + + model.retimeTrackPoints(15); + + const point = model.tracksProperty.value[0]?.points[0]; + expect(point?.frame).toBe(15); + expect(point?.time).toBe(1); + expect(point?.x).toBe(4); + expect(point?.y).toBe(5); + }); + + it("keeps frames unique when a lower rate collides two points", () => { + // At 30 fps these are distinct frames; at 2 fps both round to frame 0. + model.addOrReplacePointOnTrack("track-A", 0, 0, 1, 1); + model.addOrReplacePointOnTrack("track-A", 3, 0.1, 2, 2); + + model.retimeTrackPoints(2); + + // First-wins, matching addPointToTrack()'s dedup policy. + expect(frames(model)).toEqual([0]); + expect(model.tracksProperty.value[0]?.points[0]?.time).toBe(0); + }); + + it("keeps points sorted ascending by frame", () => { + for (let i = 0; i < 5; i++) { + model.addOrReplacePointOnTrack("track-A", i * 30, i, i, i); + } + + model.retimeTrackPoints(24); + + const result = frames(model); + expect(result).toEqual([...result].sort((a, b) => a - b)); + }); + + it("retimes the undo stash so a restored point lands on the right frame", () => { + model.addOrReplacePointOnTrack("track-A", 30, 1, 7, 7); + model.removePointFromTrack("track-A", 30); + + model.retimeTrackPoints(60); + model.restoreLastDeletedPoint(); + + expect(frames(model)).toEqual([60]); + }); + + it("ignores a non-positive frame rate", () => { + model.addOrReplacePointOnTrack("track-A", 30, 1, 1, 1); + + model.retimeTrackPoints(0); + + expect(frames(model)).toEqual([30]); + }); +}); + +describe("TrackLabModel frame-rate wiring", () => { + it("retimes stored points so they stay aligned with the current frame", () => { + const model = new TrackLabModel(); + model.tracking.addTrack(); + + // Park the video at t = 1 s and digitize there at the default 30 fps. + model.playback.currentTimeProperty.value = 1; + const frameAtDigitize = model.playback.currentFrameProperty.value; + model.tracking.addOrReplacePointOnTrack("track-A", frameAtDigitize, 1, 5, 5); + expect(model.tracking.hasPointAtFrame("track-A", model.playback.currentFrameProperty.value)).toBe(true); + + // Switching frame rate renumbers currentFrameProperty; the stored point has + // to follow, or the erase button silently targets nothing. + model.playback.frameRateProperty.value = 60; + + expect(model.playback.currentFrameProperty.value).toBe(60); + expect(model.tracking.hasPointAtFrame("track-A", model.playback.currentFrameProperty.value)).toBe(true); + }); +}); diff --git a/tests/track-lab/model/VideoPlaybackModel.test.ts b/tests/track-lab/model/VideoPlaybackModel.test.ts new file mode 100644 index 0000000..0bcb6fa --- /dev/null +++ b/tests/track-lab/model/VideoPlaybackModel.test.ts @@ -0,0 +1,84 @@ +import { beforeEach, describe, expect, it } from "vitest"; +import { DEFAULT_FRAME_RATE, VideoPlaybackModel } from "../../../src/track-lab/model/VideoPlaybackModel.js"; + +describe("VideoPlaybackModel frame derivation", () => { + let model: VideoPlaybackModel; + + beforeEach(() => { + model = new VideoPlaybackModel(); + }); + + it("derives the current frame by rounding time x frame rate", () => { + model.currentTimeProperty.value = 1; + expect(model.currentFrameProperty.value).toBe(DEFAULT_FRAME_RATE); + }); + + it("keeps adjacent timestamps on distinct frames at 29.97 fps", () => { + model.frameRateProperty.value = 29.97; + const frameDuration = 1 / 29.97; + + // Ten consecutive frame boundaries must map to ten distinct indices; this is + // what multiplying by fps (rather than dividing by 1/fps) protects. + const derived = new Set(); + for (let i = 0; i < 10; i++) { + model.currentTimeProperty.value = i * frameDuration; + derived.add(model.currentFrameProperty.value); + } + + expect(derived.size).toBe(10); + }); + + it("agrees with timeToFrame()", () => { + model.frameRateProperty.value = 25; + model.currentTimeProperty.value = 3.44; + + expect(model.timeToFrame(3.44)).toBe(model.currentFrameProperty.value); + }); +}); + +describe("VideoPlaybackModel seeking", () => { + let model: VideoPlaybackModel; + + beforeEach(() => { + model = new VideoPlaybackModel(); + model.durationProperty.value = 10; + }); + + it("advances by exactly one frame duration", () => { + model.seekByFrames(1); + expect(model.currentTimeProperty.value).toBeCloseTo(1 / DEFAULT_FRAME_RATE); + }); + + it("clamps at the start of the video", () => { + model.seekByFrames(-1); + expect(model.currentTimeProperty.value).toBe(0); + }); + + it("clamps at the end of the video", () => { + model.currentTimeProperty.value = 10; + model.seekByFrames(1); + expect(model.currentTimeProperty.value).toBe(10); + }); + + it("does nothing when no video is loaded", () => { + const empty = new VideoPlaybackModel(); + empty.seekByFrames(1); + expect(empty.currentTimeProperty.value).toBe(0); + }); + + it("pauses playback when stepping", () => { + model.isPlayingProperty.value = true; + model.seekByFrames(1); + expect(model.isPlayingProperty.value).toBe(false); + }); + + it("seekToStart pauses and rewinds", () => { + model.currentTimeProperty.value = 5; + model.isPlayingProperty.value = true; + + model.seekToStart(); + + expect(model.currentTimeProperty.value).toBe(0); + expect(model.isPlayingProperty.value).toBe(false); + }); +}); diff --git a/vite.config.ts b/vite.config.ts index 8712c00..21c27a5 100644 --- a/vite.config.ts +++ b/vite.config.ts @@ -118,7 +118,7 @@ function serveOpenCV(): Plugin { /** * Security headers required for: - * - COOP/COEP: SharedArrayBuffer (FFmpeg WASM) + * - COOP/COEP: SharedArrayBuffer (OpenCV WASM in the tracking worker) * - CSP: restrict resource loading to same-origin + known blob/data exceptions * - X-Content-Type-Options: prevent MIME sniffing * - X-Frame-Options: prevent clickjacking (belt-and-suspenders alongside frame-ancestors) @@ -128,10 +128,10 @@ const securityHeaders: Record = { "Cross-Origin-Embedder-Policy": "require-corp", "Content-Security-Policy": [ "default-src 'self'", - // 'wasm-unsafe-eval' is required for FFmpeg/OpenCV WASM modules + // 'wasm-unsafe-eval' is required for the OpenCV WASM module // 'unsafe-eval' is required for SceneryStack query parameter parsing "script-src 'self' 'wasm-unsafe-eval' 'unsafe-eval'", - // FFmpeg and OpenCV spin up blob: workers + // OpenCV spins up blob: workers "worker-src blob: 'self'", // Inline styles are set via element.style / cssText throughout the UI layer "style-src 'self' 'unsafe-inline'",