diff --git a/CLAUDE.md b/CLAUDE.md index 1728232..d40ea01 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -61,14 +61,21 @@ Fleet-standard Vitest layout: | `tests/setup.ts` | Canvas / AudioContext mocks + `init({ name: "…" })` before SceneryStack imports | | `tests/**/*.test.ts` | Model/physics unit tests — mirror `src/` under `tests/` | | `tests/memory-leak.test.ts` | WeakRef + `forceGC` dispose regression (fleet pattern) | +| `tsconfig.tests.json` | Typecheck config for `tests/` (adds `node` types), run by `npm run check` | Actual specs: - `tests/track-lab/model/KinematicsComputer.test.ts` +- `tests/track-lab/model/TrackingModel.test.ts` - `tests/memory-leak.test.ts` Run `npm test`. CI runs the suite when a `test` script is present. +`npm run check` typechecks three programs — `src` (root `tsconfig.json`), `scripts` +(`tsconfig.scripts.json`), and `tests` (`tsconfig.tests.json`). Vitest transpiles without +typechecking, so a passing `npm test` does not imply the specs typecheck; `npm run check` is what +catches that. + ## Commands ```bash diff --git a/package.json b/package.json index 849aa95..0e9618b 100644 --- a/package.json +++ b/package.json @@ -29,7 +29,7 @@ "lint": "biome check .", "format": "biome format --write .", "fix": "biome check --write .", - "check": "tsc --noEmit && tsc -p tsconfig.scripts.json --noEmit", + "check": "tsc --noEmit && tsc -p tsconfig.scripts.json --noEmit && tsc -p tsconfig.tests.json --noEmit", "test": "vitest run", "test:watch": "vitest", "release": "npm run check && npm run lint && npm run build && npm version patch && git push && git push --tags", diff --git a/src/TrackLabColors.ts b/src/TrackLabColors.ts index 0434907..18b45cd 100644 --- a/src/TrackLabColors.ts +++ b/src/TrackLabColors.ts @@ -253,6 +253,8 @@ const TrackLabColors = { tableEmptyTextProperty: profileColor("tableEmptyText", new Color(136, 136, 136), new Color(120, 120, 120)), tableSymbolShadowProperty: profileColor("tableSymbolShadow", new Color(0, 0, 0, 0.5), new Color(0, 0, 0, 0.3)), tableBackgroundProperty: profileColor("tableBackground", WHITE, new Color(250, 250, 250)), + // Outline marking the row for the frame the video is parked on. + tableCurrentRowProperty: profileColor("tableCurrentRow", new Color(232, 119, 34), new Color(214, 100, 20)), exportButtonProperty: profileColor("exportButton", new Color(76, 175, 80), new Color(60, 150, 65)), // Graph (ConfigurableGraph, GraphDataManager, GraphControlsPanel) diff --git a/src/TrackLabIcons.ts b/src/TrackLabIcons.ts index c7acfad..e8358f3 100644 --- a/src/TrackLabIcons.ts +++ b/src/TrackLabIcons.ts @@ -143,6 +143,76 @@ export function makeTrashIcon(): Node { return new Node({ children: [handle, lid, body, l1, l2, l3] }); } +/** + * Eraser icon: a tilted eraser block with a divider between the rubber and the + * sleeve. Deliberately unlike makeTrashIcon() — the erase-point and + * remove-track actions must never be mistaken for one another. + */ +export function makeEraserIcon(): Node { + const w = 12; // eraser block length + const h = 6.5; // eraser block height + const lw = 1.2; + const sleeveX = 4.5; // divider between rubber tip and sleeve + const tilt = -Math.PI / 6; // block leans like a held eraser + + const body = new Rectangle(0, 0, w, h, 1, 1, { + stroke: TrackLabColors.playbackButtonIconColorProperty, + lineWidth: lw, + fill: null, + }); + const divider = new Line(sleeveX, 0, sleeveX, h, { + stroke: TrackLabColors.playbackButtonIconColorProperty, + lineWidth: lw, + }); + + const block = new Node({ children: [body, divider] }); + block.rotation = tilt; + // Re-wrap so the caller receives an unrotated node whose origin is its centre. + return new Node({ children: [block] }); +} + +/** + * Restore icon: a counter-clockwise arrow looping back on itself ("undo the + * last delete"). + */ +export function makeRestoreIcon(): Node { + const r = 5; // arc radius + const lw = 1.6; + const headLength = 5; // tip-to-base distance of the arrowhead + const headHalfWidth = 3; // half the arrowhead's base + // The glyph is only ~12px on screen, so the head has to be large relative to + // the arc — a subtle one just reads as a letter "C". + const headAngle = Math.PI * 0.4; + const tailAngle = Math.PI * 1.75; + + const arc = new Path(new Shape().arc(0, 0, r, headAngle, tailAngle, false), { + stroke: TrackLabColors.playbackButtonIconColorProperty, + lineWidth: lw, + fill: null, + }); + + // Arrowhead at the arc's start, pointing back against the sweep so the loop + // reads as "go backwards". Tangent at headAngle is (-sin, cos); negate it. + const hx = r * Math.cos(headAngle); + const hy = r * Math.sin(headAngle); + const tipX = Math.sin(headAngle); + const tipY = -Math.cos(headAngle); + // Perpendicular to the tip direction, used to lay out the base corners. + const perpX = -tipY; + const perpY = tipX; + + const head = new Path( + new Shape() + .moveTo(hx + tipX * headLength, hy + tipY * headLength) + .lineTo(hx + perpX * headHalfWidth, hy + perpY * headHalfWidth) + .lineTo(hx - perpX * headHalfWidth, hy - perpY * headHalfWidth) + .close(), + { fill: TrackLabColors.playbackButtonIconColorProperty }, + ); + + return new Node({ children: [arc, head] }); +} + /** * Two small XY arrows for the coordinate-system control / info-dialog row. */ diff --git a/src/i18n/StringManager.ts b/src/i18n/StringManager.ts index 23fbd79..9b3a7af 100644 --- a/src/i18n/StringManager.ts +++ b/src/i18n/StringManager.ts @@ -88,11 +88,13 @@ export class StringManager { titleStringProperty: ReadOnlyProperty; simulationControlsStringProperty: ReadOnlyProperty; graphInteractionsStringProperty: ReadOnlyProperty; + digitizingStringProperty: ReadOnlyProperty; playPauseSimulationStringProperty: ReadOnlyProperty; resetSimulationStringProperty: ReadOnlyProperty; stepBackwardStringProperty: ReadOnlyProperty; stepForwardStringProperty: ReadOnlyProperty; rewindToStartStringProperty: ReadOnlyProperty; + erasePointStringProperty: ReadOnlyProperty; resetZoomStringProperty: ReadOnlyProperty; zoomInOutStringProperty: ReadOnlyProperty; panViewStringProperty: ReadOnlyProperty; @@ -101,11 +103,13 @@ export class StringManager { titleStringProperty: stringProperties.keyboardShortcuts.titleStringProperty, simulationControlsStringProperty: stringProperties.keyboardShortcuts.simulationControlsStringProperty, graphInteractionsStringProperty: stringProperties.keyboardShortcuts.graphInteractionsStringProperty, + digitizingStringProperty: stringProperties.keyboardShortcuts.digitizingStringProperty, playPauseSimulationStringProperty: stringProperties.keyboardShortcuts.playPauseSimulationStringProperty, resetSimulationStringProperty: stringProperties.keyboardShortcuts.resetSimulationStringProperty, stepBackwardStringProperty: stringProperties.keyboardShortcuts.stepBackwardStringProperty, stepForwardStringProperty: stringProperties.keyboardShortcuts.stepForwardStringProperty, rewindToStartStringProperty: stringProperties.keyboardShortcuts.rewindToStartStringProperty, + erasePointStringProperty: stringProperties.keyboardShortcuts.erasePointStringProperty, resetZoomStringProperty: stringProperties.keyboardShortcuts.resetZoomStringProperty, zoomInOutStringProperty: stringProperties.keyboardShortcuts.zoomInOutStringProperty, panViewStringProperty: stringProperties.keyboardShortcuts.panViewStringProperty, @@ -441,6 +445,8 @@ export class StringManager { digitizingAreaStringProperty: ReadOnlyProperty; digitizeTrackStringProperty: ReadOnlyProperty; removeTrackStringProperty: ReadOnlyProperty; + erasePointStringProperty: ReadOnlyProperty; + restorePointStringProperty: ReadOnlyProperty; dataTableStringProperty: ReadOnlyProperty; exportCSVStringProperty: ReadOnlyProperty; measuringTapeBaseStringProperty: ReadOnlyProperty; @@ -485,6 +491,8 @@ export class StringManager { digitizingAreaStringProperty: stringProperties.a11y.digitizingAreaStringProperty, digitizeTrackStringProperty: stringProperties.a11y.digitizeTrackStringProperty, removeTrackStringProperty: stringProperties.a11y.removeTrackStringProperty, + erasePointStringProperty: stringProperties.a11y.erasePointStringProperty, + restorePointStringProperty: stringProperties.a11y.restorePointStringProperty, dataTableStringProperty: stringProperties.a11y.dataTableStringProperty, exportCSVStringProperty: stringProperties.a11y.exportCSVStringProperty, measuringTapeBaseStringProperty: stringProperties.a11y.measuringTapeBaseStringProperty, diff --git a/src/i18n/strings_en.json b/src/i18n/strings_en.json index cbdf2b2..be3bcdc 100644 --- a/src/i18n/strings_en.json +++ b/src/i18n/strings_en.json @@ -42,11 +42,13 @@ "title": "Keyboard Shortcuts", "simulationControls": "Simulation Controls", "graphInteractions": "Graph Interactions", + "digitizing": "Digitizing", "playPauseSimulation": "Play/Pause Simulation", "resetSimulation": "Reset Simulation", "stepBackward": "Step Backward", "stepForward": "Step Forward", "rewindToStart": "Rewind to Start", + "erasePoint": "Erase Point at Current Frame", "resetZoom": "Reset Zoom", "zoomInOut": "Zoom In/Out", "panView": "Pan View" @@ -158,7 +160,7 @@ "addTrackTitle": "4. Add a Track", "addTrackBody": "In the Tracks panel, click “+ Add Track” to create a new track (A, B, C…). Up to 4 tracks can be active at a time.", "digitizeTitle": "5. Digitize Frame by Frame", - "digitizeBody": "Select a track and click the object’s position in the video. Enable the Magnifier for precision. The video advances one frame after each click.", + "digitizeBody": "Select a track and click the object’s position in the video. Enable the Magnifier for precision. The video advances one frame after each click. Click the same frame again to move a point, or press Delete to erase it.", "autoTrackTitle": "Auto-Tracking", "autoTrackBody": "Enable Auto-Tracking in the control panel, then drag a selection box around the object. TrackLab will track it automatically across the remaining frames.", "measuringTapeTitle": "Measuring Tape", @@ -176,9 +178,11 @@ "videoPlayer": "Video player", "videoScrubber": "Video timeline — drag to seek", "rewindToStart": "Rewind to start", - "digitizingArea": "Video digitizing area — click to record the position of the active track", + "digitizingArea": "Video digitizing area — click to record the position of the active track, or press Delete to erase the point at this frame", "digitizeTrack": "Digitize track {{symbol}}", "removeTrack": "Remove track {{symbol}}", + "erasePoint": "Erase the digitized point at the current frame", + "restorePoint": "Restore the last erased point", "dataTable": "Track data", "exportCSV": "Export track data as CSV file", "measuringTapeBase": "Measuring tape base endpoint", diff --git a/src/i18n/strings_es.json b/src/i18n/strings_es.json index 14f6eff..7f0402e 100644 --- a/src/i18n/strings_es.json +++ b/src/i18n/strings_es.json @@ -42,11 +42,13 @@ "title": "Atajos de teclado", "simulationControls": "Controles de simulación", "graphInteractions": "Interacciones con la gráfica", + "digitizing": "Digitalización", "playPauseSimulation": "Reproducir/Pausar la simulación", "resetSimulation": "Reiniciar la simulación", "stepBackward": "Retroceder un paso", "stepForward": "Avanzar un paso", "rewindToStart": "Rebobinar al inicio", + "erasePoint": "Borrar punto en el fotograma actual", "resetZoom": "Restablecer el zoom", "zoomInOut": "Acercar/Alejar", "panView": "Desplazar la vista" @@ -158,7 +160,7 @@ "addTrackTitle": "4. Añadir una pista", "addTrackBody": "En el panel de pistas, haz clic en «+ Añadir pista» para crear una nueva pista (A, B, C…). Pueden estar activas hasta 4 pistas a la vez.", "digitizeTitle": "5. Digitalizar fotograma a fotograma", - "digitizeBody": "Selecciona una pista y haz clic en la posición del objeto en el video. Activa la lupa para mayor precisión. El video avanza un fotograma después de cada clic.", + "digitizeBody": "Selecciona una pista y haz clic en la posición del objeto en el video. Activa la lupa para mayor precisión. El video avanza un fotograma después de cada clic. Vuelve a hacer clic en el mismo fotograma para mover un punto, o pulsa Supr para borrarlo.", "autoTrackTitle": "Seguimiento automático", "autoTrackBody": "Activa el seguimiento automático en el panel de control, luego arrastra un cuadro de selección alrededor del objeto. TrackLab lo seguirá automáticamente a lo largo de los fotogramas restantes.", "measuringTapeTitle": "Cinta métrica", @@ -176,9 +178,11 @@ "videoPlayer": "Reproductor de video", "videoScrubber": "Línea de tiempo del video — arrastra para buscar", "rewindToStart": "Rebobinar al inicio", - "digitizingArea": "Área de digitalización del video — haz clic para registrar la posición de la pista activa", + "digitizingArea": "Área de digitalización del video — haz clic para registrar la posición de la pista activa, o pulsa Supr para borrar el punto de este fotograma", "digitizeTrack": "Digitalizar la pista {{symbol}}", "removeTrack": "Eliminar la pista {{symbol}}", + "erasePoint": "Borrar el punto digitalizado en el fotograma actual", + "restorePoint": "Restaurar el último punto borrado", "dataTable": "Datos de la pista", "exportCSV": "Exportar los datos de la pista como archivo CSV", "measuringTapeBase": "Extremo base de la cinta métrica", diff --git a/src/i18n/strings_fr.json b/src/i18n/strings_fr.json index 4979272..c9be99e 100644 --- a/src/i18n/strings_fr.json +++ b/src/i18n/strings_fr.json @@ -42,11 +42,13 @@ "title": "Raccourcis clavier", "simulationControls": "Contrôles de simulation", "graphInteractions": "Interactions avec le graphique", + "digitizing": "Numérisation", "playPauseSimulation": "Lecture/Pause de la simulation", "resetSimulation": "Réinitialiser la simulation", "stepBackward": "Reculer d'un pas", "stepForward": "Avancer d'un pas", "rewindToStart": "Revenir au début", + "erasePoint": "Effacer le point à l'image actuelle", "resetZoom": "Réinitialiser le zoom", "zoomInOut": "Zoom avant/arrière", "panView": "Déplacer la vue" @@ -158,7 +160,7 @@ "addTrackTitle": "4. Ajouter une piste", "addTrackBody": "Dans le panneau Pistes, cliquez sur « + Ajouter une piste » pour créer une nouvelle piste étiquetée A, B, C, …", "digitizeTitle": "5. Numériser image par image", - "digitizeBody": "Sélectionnez une piste et cliquez sur la position de l’objet dans la vidéo. Activez la Loupe pour plus de précision. La vidéo avance automatiquement d’une image après chaque clic.", + "digitizeBody": "Sélectionnez une piste et cliquez sur la position de l’objet dans la vidéo. Activez la Loupe pour plus de précision. La vidéo avance automatiquement d’une image après chaque clic. Cliquez à nouveau sur la même image pour déplacer un point, ou appuyez sur Suppr pour l’effacer.", "autoTrackTitle": "Suivi automatique", "autoTrackBody": "Activez le Suivi automatique dans le panneau de contrôle, puis faites glisser un cadre de sélection autour de l’objet. TrackLab le suivra automatiquement sur les images restantes.", "measuringTapeTitle": "Ruban de mesure", @@ -176,9 +178,11 @@ "videoPlayer": "Lecteur vidéo", "videoScrubber": "Ligne de temps vidéo — glisser pour naviguer", "rewindToStart": "Revenir au début", - "digitizingArea": "Zone de numérisation vidéo — cliquer pour enregistrer la position de la piste active", + "digitizingArea": "Zone de numérisation vidéo — cliquer pour enregistrer la position de la piste active, ou appuyer sur Suppr pour effacer le point de cette image", "digitizeTrack": "Numériser la piste {{symbol}}", "removeTrack": "Supprimer la piste {{symbol}}", + "erasePoint": "Effacer le point numérisé à l'image actuelle", + "restorePoint": "Restaurer le dernier point effacé", "dataTable": "Données de piste", "exportCSV": "Exporter les données de piste en fichier CSV", "measuringTapeBase": "Extrémité de base du ruban de mesure", diff --git a/src/track-lab/model/TrackLabModel.ts b/src/track-lab/model/TrackLabModel.ts index 60a69b9..8b12383 100644 --- a/src/track-lab/model/TrackLabModel.ts +++ b/src/track-lab/model/TrackLabModel.ts @@ -61,12 +61,36 @@ export class TrackLabModel { * The view passes raw inputs (track id + pixel position); this method * coordinates all sub-model interactions so the view does not need to reach * into playback or overlayTools directly. + * + * Clicking a frame that already holds a point *replaces* it: re-clicking is + * how a user corrects a misclick, and silently ignoring the second click + * reads as the tool being broken. Auto-tracking keeps the first-wins policy + * so it never clobbers a hand-placed point. */ public recordTrackPoint(trackId: string, pixelPoint: Vector2): void { const time = this.playback.currentTimeProperty.value; - const frame = Math.round(time * this.playback.frameRateProperty.value); + // Same derivation as deleteTrackPointAtCurrentFrame(), so the frame a click + // writes is exactly the frame the erase button targets. + const frame = this.playback.currentFrameProperty.value; const modelPt = this.overlayTools.modelViewTransformProperty.value.inversePosition2(pixelPoint); - this.tracking.addPointToTrack(trackId, frame, time, modelPt.x, modelPt.y); + this.tracking.addOrReplacePointOnTrack(trackId, frame, time, modelPt.x, modelPt.y); + } + + /** + * Delete the active track's point at the current playback frame, if any. + * Returns true when a point was actually removed. + */ + public deleteTrackPointAtCurrentFrame(): boolean { + const trackId = this.tracking.activeTrackIdProperty.value; + if (trackId === null) { + return false; + } + const frame = this.playback.currentFrameProperty.value; + if (!this.tracking.hasPointAtFrame(trackId, frame)) { + return false; + } + this.tracking.removePointFromTrack(trackId, frame); + return true; } // ── Video source activation ───────────────────────────────────────────── diff --git a/src/track-lab/model/TrackingModel.ts b/src/track-lab/model/TrackingModel.ts index 63f8b06..3c25b09 100644 --- a/src/track-lab/model/TrackingModel.ts +++ b/src/track-lab/model/TrackingModel.ts @@ -26,6 +26,24 @@ import { OpenCVTracker, type TrackerRegion } from "../../tracking/OpenCVTracker. import { computeTrackKinematics } from "./KinematicsComputer.js"; import type { Track, TrackKinematics, TrackPoint } from "./Track.js"; +/** + * Insert `point` into `points`, keeping the array sorted by ascending frame. + * Always returns a new array so the kinematics cache (keyed on array identity) + * invalidates. + * + * Sorted insertion matters because KinematicsComputer differentiates by array + * index and assumes time increases along it. Appending is only safe while + * digitizing runs forwards; restoring a deleted interior point, or digitizing + * out of order after scrubbing backwards, both break that assumption. + */ +function insertPointSorted(points: readonly TrackPoint[], point: TrackPoint): TrackPoint[] { + const index = points.findIndex((p) => p.frame > point.frame); + if (index === -1) { + return [...points, point]; + } + return [...points.slice(0, index), point, ...points.slice(index)]; +} + /** * Owns all reactive state for particle tracks, kinematics caching, and the * OpenCV template-matching tracker facade. @@ -44,17 +62,30 @@ export class TrackingModel { (tracks) => tracks.length < MAX_TRACKS, ); + // ── Single-level undo for point deletion ────────────────────────────── + // Only the most recently deleted point is remembered. A deletion is a + // deliberate, low-stakes action (the video frame is still there to + // re-digitize), so one level of undo covers the "wrong button" mistake + // without introducing a general command history. + private readonly lastDeletedPointProperty = new Property<{ trackId: string; point: TrackPoint } | null>(null); + + public readonly canRestorePointProperty: TReadOnlyProperty = new DerivedProperty( + [this.lastDeletedPointProperty], + (deleted) => deleted !== null, + ); + // ── Derived kinematics for all tracks ─────────────────────────────────── // Cache keyed by track ID; only recomputes kinematics for tracks whose // point array reference has changed since the last derivation. // // CACHE INVARIANT: validity is determined by object identity // (`cached.points === track.points`). This is correct because every - // mutation path (addPointToTrack, retransformTrackPoints) replaces the - // entire Track object and its points array, so a stale entry always has a - // different reference. removeTrack() explicitly evicts the entry for the - // removed track to prevent an unbounded memory leak when tracks are added - // and removed repeatedly. + // mutation path (addPointToTrack, addOrReplacePointOnTrack, + // removePointFromTrack, retransformTrackPoints) replaces the entire Track + // object and its points array, so a stale entry always has a different + // reference. removeTrack() explicitly evicts the entry for the removed + // track to prevent an unbounded memory leak when tracks are added and + // removed repeatedly. private readonly kinematicsCache = new Map(); public readonly trackKinematicsProperty: TReadOnlyProperty = new DerivedProperty( @@ -119,6 +150,12 @@ export class TrackingModel { } this.tracksProperty.value = this.tracksProperty.value.filter((t) => t.id !== id); this.kinematicsCache.delete(id); + + // The undo stash points at a track that no longer exists; restoring it + // would be a silent no-op, so drop it and let the button disable. + if (this.lastDeletedPointProperty.value?.trackId === id) { + this.lastDeletedPointProperty.value = null; + } } /** @@ -138,12 +175,84 @@ export class TrackingModel { } const point: TrackPoint = { frame, time, x, y }; - const updated: Track = { ...track, points: [...track.points, point] }; + const updated: Track = { ...track, points: insertPointSorted(track.points, point) }; return updated; }); this.tracksProperty.value = tracks; } + /** + * Record a digitized position for `frame` on the track identified by `id`, + * overwriting any position already recorded for that frame. + * + * This is the manual-digitizing entry point. Re-clicking a frame is the + * natural way a user corrects a misclick, so the *last* recorded position + * wins here — the opposite of addPointToTrack()'s first-wins policy, which + * auto-tracking relies on to avoid clobbering hand-placed points. + */ + public addOrReplacePointOnTrack(id: string, frame: number, time: number, x: number, y: number): void { + const point: TrackPoint = { frame, time, x, y }; + + this.tracksProperty.value = this.tracksProperty.value.map((track) => { + if (track.id !== id) { + return track; + } + + const existing = track.points.some((p) => p.frame === frame); + const points = existing + ? track.points.map((p) => (p.frame === frame ? point : p)) + : insertPointSorted(track.points, point); + + return { ...track, points }; + }); + } + + /** + * Delete the point recorded for `frame` on the track identified by `id`. + * Does nothing if the track or the point does not exist. + * + * The removed point is stashed so that restoreLastDeletedPoint() can undo + * this call. Deleting an interior point leaves a gap in the frame series; + * that is safe — KinematicsComputer differentiates against each point's + * recorded timestamp, not against an assumed constant frame interval. + */ + public removePointFromTrack(id: string, frame: number): void { + const track = this.tracksProperty.value.find((t) => t.id === id); + const removed = track?.points.find((p) => p.frame === frame); + if (!(track && removed)) { + return; + } + + this.tracksProperty.value = this.tracksProperty.value.map((t) => + // filter() allocates a new array, which invalidates the kinematics cache. + t.id === id ? { ...t, points: t.points.filter((p) => p.frame !== frame) } : t, + ); + this.lastDeletedPointProperty.value = { trackId: id, point: removed }; + } + + /** + * Re-insert the most recently deleted point. Does nothing if no point has + * been deleted, or if its track has since been removed. + */ + public restoreLastDeletedPoint(): void { + const deleted = this.lastDeletedPointProperty.value; + if (!deleted) { + return; + } + const { trackId, point } = deleted; + this.lastDeletedPointProperty.value = null; + this.addOrReplacePointOnTrack(trackId, point.frame, point.time, point.x, point.y); + } + + /** True when the given track has a point recorded at `frame`. */ + public hasPointAtFrame(id: string | null, frame: number): boolean { + if (id === null) { + return false; + } + const track = this.tracksProperty.value.find((t) => t.id === id); + return track?.points.some((p) => p.frame === frame) ?? false; + } + /** * Create a new track and immediately make it the active track. * Does nothing if the track limit or symbol limit has been reached. @@ -239,6 +348,7 @@ export class TrackingModel { this.kinematicsCache.clear(); this.tracksProperty.value = []; this.activeTrackIdProperty.value = null; + this.lastDeletedPointProperty.value = null; this.nextSymbolCode = TRACK_SYMBOL_FIRST_CODE; this.initVersion++; this.tracker.dispose(); diff --git a/src/track-lab/view/DataTableNode.ts b/src/track-lab/view/DataTableNode.ts index 008dcb3..7daf102 100644 --- a/src/track-lab/view/DataTableNode.ts +++ b/src/track-lab/view/DataTableNode.ts @@ -40,6 +40,7 @@ import { import TrackLabNamespace from "../../TrackLabNamespace.js"; import { generateCsv } from "../model/TrackExporter.js"; import type { TrackingModel } from "../model/TrackingModel.js"; +import type { VideoPlaybackModel } from "../model/VideoPlaybackModel.js"; import { type A11yLabels, type TableColors, type TableLabels, TableRenderer } from "./TableRenderer.js"; // ── Fonts ──────────────────────────────────────────────────────────────────── @@ -69,6 +70,7 @@ function getTableColors(): TableColors { emptyText: TrackLabColors.tableEmptyTextProperty.value.toCSS(), symbolShadow: TrackLabColors.tableSymbolShadowProperty.value.toCSS(), background: TrackLabColors.tableBackgroundProperty.value.toCSS(), + currentRow: TrackLabColors.tableCurrentRowProperty.value.toCSS(), }; } @@ -86,6 +88,7 @@ export class DataTableNode extends Node { public constructor( tracking: TrackingModel, + playback: VideoPlaybackModel, videoLoadedProperty: TReadOnlyProperty, unitProperty: TReadOnlyProperty, ) { @@ -103,7 +106,12 @@ export class DataTableNode extends Node { }); // ── TableRenderer owns all DOM and incremental-update state ─────────────── - const tableRenderer = new TableRenderer(getTableColors(), getLabels(), getA11yLabels()); + // Clicking a data row seeks the video to that frame, which makes the table + // a navigation surface: find a bad point here, then erase it from the + // playback controls without hunting for a 2px dot on the video. + const tableRenderer = new TableRenderer(getTableColors(), getLabels(), getA11yLabels(), (frame: number) => { + playback.currentTimeProperty.value = frame / playback.frameRateProperty.value; + }); const tableDomNode = new DOM(tableRenderer.wrapper, { allowInput: true }); @@ -204,6 +212,9 @@ export class DataTableNode extends Node { const unitListener = () => runUpdate(); unitProperty.link(unitListener); + 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(); @@ -408,6 +419,7 @@ export class DataTableNode extends Node { resizeObserver.disconnect(); tracking.tracksProperty.unlink(tracksListener); unitProperty.unlink(unitListener); + playback.currentFrameProperty.unlink(currentFrameListener); TrackLabColors.tableHeaderBackgroundProperty.unlink(tableHeaderBgListener); dataTableStrings.frameStringProperty.unlink(frameStringListener); videoLoadedProperty.unlink(videoLoadedListener); diff --git a/src/track-lab/view/DigitizingOverlayNode.ts b/src/track-lab/view/DigitizingOverlayNode.ts index 9310f3d..9313acb 100644 --- a/src/track-lab/view/DigitizingOverlayNode.ts +++ b/src/track-lab/view/DigitizingOverlayNode.ts @@ -8,7 +8,7 @@ import type { TReadOnlyProperty } from "scenerystack/axon"; import { type Dimension2, type Transform3, Vector2 } from "scenerystack/dot"; import { Shape } from "scenerystack/kite"; -import { DOM, FireListener, Node, Path, Rectangle } from "scenerystack/scenery"; +import { DOM, FireListener, KeyboardListener, Node, Path, Rectangle } from "scenerystack/scenery"; import { Tandem } from "scenerystack/tandem"; import { StringManager } from "../../i18n/StringManager.js"; import TrackLabColors, { getTrackColor } from "../../TrackLabColors.js"; @@ -25,6 +25,8 @@ type DigitizingOverlayNodeOptions = { modelViewTransformProperty: TReadOnlyProperty; /** Record a digitized point on the given track at the given pixel position. */ recordPoint: (trackId: string, pixelPoint: Vector2) => void; + /** Delete the active track's point at the current frame, if there is one. */ + erasePoint: () => void; }; const OUTER_R = 20; @@ -46,6 +48,12 @@ const MAG_CROSSHAIR_LINE_WIDTH = 1; const MARK_DOT_RADIUS = 2; // radius of each digitized-point dot drawn on the video +// Ring drawn around the active track's point at the current frame — the point +// the erase button and the Delete key act on. Showing the target before the +// user commits is what makes frame-based deletion legible. +const CURRENT_MARK_RING_RADIUS = 6; +const CURRENT_MARK_RING_LINE_WIDTH = 1.5; + // Magnifier canvas box-shadow: "offsetX offsetY blur color" const MAG_SHADOW_OFFSET_X = 0; // px, horizontal shadow offset const MAG_SHADOW_OFFSET_Y = 2; // px, vertical shadow offset @@ -246,6 +254,7 @@ export class DigitizingOverlayNode extends Node { cursor: "none", visible: false, tagName: "div", + focusable: true, accessibleName: a11yStrings.digitizingAreaStringProperty, }); digitizingOverlay.addChild(cursorNode); @@ -308,6 +317,15 @@ export class DigitizingOverlayNode extends Node { const marksLayer = new Node({ pickable: false }); const trackPaths = new Map(); // track id → Path + // A single reused Path holding at most one ring — never one node per point, + // which is what the per-track batching above exists to avoid. + const currentMarkRing = new Path(null, { + stroke: TrackLabColors.digitizingCursorStrokeProperty, + lineWidth: CURRENT_MARK_RING_LINE_WIDTH, + fill: null, + pickable: false, + }); + const rebuildMarks = () => { const currentFrame = playback.currentFrameProperty.value; const mvt = modelViewTransformProperty.value; @@ -340,6 +358,17 @@ export class DigitizingOverlayNode extends Node { trackPaths.delete(id); } } + + // Ring the erase target: the active track's point at the current frame. + const activeId = tracking.activeTrackIdProperty.value; + const activeTrack = activeId === null ? undefined : tracks.find((t) => t.id === activeId); + const currentPoint = activeTrack?.points.find((p) => p.frame === currentFrame); + if (currentPoint) { + const localPt = mvt.transformPosition2(new Vector2(currentPoint.x, currentPoint.y)); + currentMarkRing.shape = new Shape().circle(localPt.x, localPt.y, CURRENT_MARK_RING_RADIUS); + } else { + currentMarkRing.shape = null; + } }; const currentTimeListener = () => { @@ -362,6 +391,9 @@ export class DigitizingOverlayNode extends Node { if (!activeId) { cursorNode.visible = false; } + // The erase-target ring follows the active track, so it must be redrawn + // when the active track changes, not only when points change. + rebuildMarks(); }; tracking.activeTrackIdProperty.link(activeTrackListener); @@ -398,6 +430,17 @@ export class DigitizingOverlayNode extends Node { }), ); + // Delete/Backspace erases the point under the ring. The overlay is + // focusable so keyboard users reach this without a pointer; the erase + // button in the playback controls does the same thing. + digitizingOverlay.addInputListener( + new KeyboardListener({ + keys: ["delete", "backspace"], + fire: () => options.erasePoint(), + }), + ); + + marksLayer.addChild(currentMarkRing); this.addChild(digitizingOverlay); this.addChild(marksLayer); @@ -418,6 +461,7 @@ export class DigitizingOverlayNode extends Node { path.dispose(); } trackPaths.clear(); + currentMarkRing.dispose(); }; } diff --git a/src/track-lab/view/PlaybackControlsNode.ts b/src/track-lab/view/PlaybackControlsNode.ts index 34ac422..3010097 100644 --- a/src/track-lab/view/PlaybackControlsNode.ts +++ b/src/track-lab/view/PlaybackControlsNode.ts @@ -18,8 +18,10 @@ import TrackLabColors from "../../TrackLabColors.js"; const a11yStrings = StringManager.getInstance().getA11y(); +import { makeEraserIcon, makeRestoreIcon } from "../../TrackLabIcons.js"; import TrackLabNamespace from "../../TrackLabNamespace.js"; import GraphDataManager from "../graph/GraphDataManager.js"; +import type { TrackingModel } from "../model/TrackingModel.js"; import type { VideoPlaybackModel } from "../model/VideoPlaybackModel.js"; const LABEL_FONT = new PhetFont(14); @@ -46,9 +48,11 @@ export class PlaybackControlsNode extends HBox { public constructor( playback: VideoPlaybackModel, + tracking: TrackingModel, videoElement: HTMLVideoElement, onStepBackward: () => void, onStepForward: () => void, + onErasePoint: () => void, ) { super({ spacing: CONTROLS_SPACING, align: "center" }); @@ -288,9 +292,45 @@ export class PlaybackControlsNode extends HBox { }, ); - this.children = [timeControlNode, this.scrubber, rewindButton, infoDisplay]; + // ── Erase / restore point buttons ────────────────────────────────────── + // These act on the *current frame* of the *active track*, which together + // identify exactly one digitized point — no hit-testing and no selection + // state. They live beside the step buttons because that is where + // "act on this frame" already lives, and deliberately not in + // TrackListPanel, where they would sit one target away from the trash + // button that deletes an entire track. + const canErasePointProperty = new DerivedProperty( + [tracking.tracksProperty, tracking.activeTrackIdProperty, playback.currentFrameProperty], + (_tracks, activeId, frame) => tracking.hasPointAtFrame(activeId, frame), + ); + + // Dim background and icon together, matching the rewind button. + const dimWhenDisabled = (enabled: boolean, button: import("scenerystack/scenery").Node) => { + button.opacity = enabled ? 1 : 0.45; + }; + + const erasePointButton = createTrackLabButton(makeEraserIcon(), { + baseColor: TrackLabColors.playbackButtonBaseProperty, + enabledAppearanceStrategy: dimWhenDisabled, + enabledProperty: canErasePointProperty, + accessibleName: a11yStrings.erasePointStringProperty, + listener: onErasePoint, + }); + + const restorePointButton = createTrackLabButton(makeRestoreIcon(), { + baseColor: TrackLabColors.playbackButtonBaseProperty, + enabledAppearanceStrategy: dimWhenDisabled, + enabledProperty: tracking.canRestorePointProperty, + accessibleName: a11yStrings.restorePointStringProperty, + listener: () => tracking.restoreLastDeletedPoint(), + }); + + this.children = [timeControlNode, this.scrubber, rewindButton, erasePointButton, restorePointButton, infoDisplay]; this.disposePlaybackControlsNode = () => { + erasePointButton.dispose(); + restorePointButton.dispose(); + canErasePointProperty.dispose(); timeSpeedProperty.unlink(onSpeedChange); playback.playbackRateProperty.unlink(onRateChange); playback.currentTimeProperty.unlink(onTimeChange); diff --git a/src/track-lab/view/TableRenderer.ts b/src/track-lab/view/TableRenderer.ts index 55ff769..203dbd8 100644 --- a/src/track-lab/view/TableRenderer.ts +++ b/src/track-lab/view/TableRenderer.ts @@ -57,8 +57,12 @@ export type TableColors = { emptyText: string; symbolShadow: string; background: string; + currentRow: string; }; +/** Seek the video to `frame`. Invoked when the user clicks a data row. */ +export type FrameSelectedCallback = (frame: number) => void; + export type TableLabels = { frame: string; timeSeconds: string; @@ -83,6 +87,7 @@ function buildHtmlTable( a11y: A11yLabels, maxWidth: number, maxHeight: number, + onFrameSelected: FrameSelectedCallback, ): HTMLDivElement { const dataRows = buildDataRows(tracks); @@ -196,7 +201,7 @@ function buildHtmlTable( if (!row) { continue; } - tbody.appendChild(buildDataRow(row, tracks, makeCellStyle(colors), i % 2 !== 0, colors)); + tbody.appendChild(buildDataRow(row, tracks, makeCellStyle(colors), i % 2 !== 0, colors, onFrameSelected)); } } table.appendChild(tbody); @@ -210,10 +215,22 @@ function buildDataRow( cellStyle: string, isEven: boolean, colors: TableColors, + onFrameSelected: FrameSelectedCallback, ): HTMLTableRowElement { const tr = document.createElement("tr"); tr.style.background = isEven ? colors.rowEven : colors.rowOdd; + // Clicking a row seeks the video to that frame. This is what lets the table + // act as a navigation surface: spot a bad point here, click it, and the + // erase button in the playback controls lights up on the right frame. + tr.style.cursor = "pointer"; + tr.addEventListener("click", (event) => { + // The whole panel is wrapped in a pan DragListener; without this the click + // can be swallowed as the tail of a drag. + event.stopPropagation(); + onFrameSelected(row.frame); + }); + const addCell = (text: string) => { const td = document.createElement("td"); td.textContent = text; @@ -253,9 +270,23 @@ export class TableRenderer { private currentMaxWidth: number; private currentMaxHeight: number; - public constructor(initialColors: TableColors, initialLabels: TableLabels, initialA11y: A11yLabels) { + // ── Current-frame highlight state ────────────────────────────────────────── + // The frame the video is parked on. Re-applied after every rebuild, since a + // rebuild throws away the highlighted . + private currentFrame: number | null = null; + private currentRowColor: string; + private readonly onFrameSelected: FrameSelectedCallback; + + public constructor( + initialColors: TableColors, + initialLabels: TableLabels, + initialA11y: A11yLabels, + onFrameSelected: FrameSelectedCallback, + ) { this.currentMaxWidth = MAX_TABLE_WIDTH; this.currentMaxHeight = MAX_TABLE_HEIGHT; + this.currentRowColor = initialColors.currentRow; + this.onFrameSelected = onFrameSelected; this.wrapper = buildHtmlTable( [], "m", @@ -264,9 +295,40 @@ export class TableRenderer { initialA11y, this.currentMaxWidth, this.currentMaxHeight, + onFrameSelected, ); } + /** + * Mark the row for `frame` as the current one and scroll it into view. + * Cheap — moves styling between two existing rows, never rebuilds. + */ + public setCurrentFrame(frame: number | null): void { + this.currentFrame = frame; + this.applyCurrentFrameHighlight(true); + } + + /** + * Paint the current-frame highlight onto the live DOM. + * + * @param scrollIntoView - true for user-driven frame changes; false after a + * rebuild, where the frame did not actually move and yanking the scroll + * position would fight the user's own scrolling. + */ + private applyCurrentFrameHighlight(scrollIntoView: boolean): void { + for (const [frame, tr] of this.frameRowMap) { + const isCurrent = frame === this.currentFrame; + tr.style.outline = isCurrent ? `2px solid ${this.currentRowColor}` : ""; + // Draw the outline inside the cell box so it is not clipped by the + // scroll container at the first and last rows. + tr.style.outlineOffset = isCurrent ? "-2px" : ""; + tr.setAttribute("aria-current", isCurrent ? "true" : "false"); + if (isCurrent && scrollIntoView) { + tr.scrollIntoView({ block: "nearest" }); + } + } + } + /** Apply user-set max-width / max-height to the wrapper CSS. */ public applyDimensions(maxWidth: number, maxHeight: number): void { this.currentMaxWidth = maxWidth; @@ -297,7 +359,17 @@ export class TableRenderer { labels: TableLabels, a11y: A11yLabels, ): void { - const newWrapper = buildHtmlTable(tracks, unit, colors, labels, a11y, this.currentMaxWidth, this.currentMaxHeight); + this.currentRowColor = colors.currentRow; + const newWrapper = buildHtmlTable( + tracks, + unit, + colors, + labels, + a11y, + this.currentMaxWidth, + this.currentMaxHeight, + this.onFrameSelected, + ); this.wrapper.innerHTML = ""; if (newWrapper.firstChild) { @@ -330,6 +402,10 @@ export class TableRenderer { this.lastTrackIds = tracks.map((t) => t.id); this.lastUnit = unit; + + // The rebuild discarded the highlighted ; restore it on the new DOM. + // No scroll — the frame has not moved, only the rows around it. + this.applyCurrentFrameHighlight(false); } /** @@ -363,6 +439,24 @@ export class TableRenderer { // ── Incremental path: same track structure, only new points ────────────── const dataRows = buildDataRows(tracks); + // The incremental path can append rows but never remove them, so a deleted + // point would leave its behind showing stale coordinates. If any + // rendered frame is gone from the data, fall back to a full rebuild — that + // also re-stripes the positional row backgrounds and resets frameRowMap / + // maxRenderedFrame. Same bail-out idiom as the hasOutOfOrder guard below. + const currentFrames = new Set(dataRows.map((row) => row.frame)); + let hasRemovedRow = false; + for (const frame of this.frameRowMap.keys()) { + if (!currentFrames.has(frame)) { + hasRemovedRow = true; + break; + } + } + if (hasRemovedRow) { + this.rebuild(tracks, unit, colors, labels, a11y); + return; + } + // If any new row precedes the last rendered frame the sort order breaks; // fall back to full rebuild (rare: out-of-order manual digitizing). const hasOutOfOrder = dataRows.some((row) => !this.frameRowMap.has(row.frame) && row.frame < this.maxRenderedFrame); @@ -378,6 +472,7 @@ export class TableRenderer { this.tableBodyRef.innerHTML = ""; } + let appendedRow = false; for (const row of dataRows) { const tr = this.frameRowMap.get(row.frame); if (tr !== undefined) { @@ -399,13 +494,21 @@ export class TableRenderer { } else { // Append a brand-new row. const rowIndex = this.frameRowMap.size; - const newRow = buildDataRow(row, tracks, cellStyle, rowIndex % 2 !== 0, colors); + const newRow = buildDataRow(row, tracks, cellStyle, rowIndex % 2 !== 0, colors, this.onFrameSelected); this.tableBodyRef.appendChild(newRow); this.frameRowMap.set(row.frame, newRow); + appendedRow = true; if (row.frame > this.maxRenderedFrame) { this.maxRenderedFrame = row.frame; } } } + + // A row appended for the current frame starts out unhighlighted — this is + // the common case while digitizing, where each click adds the row the + // video is parked on. + if (appendedRow) { + this.applyCurrentFrameHighlight(true); + } } } diff --git a/src/track-lab/view/TrackLabKeyboardHelpContent.ts b/src/track-lab/view/TrackLabKeyboardHelpContent.ts index f2c0af5..2448888 100644 --- a/src/track-lab/view/TrackLabKeyboardHelpContent.ts +++ b/src/track-lab/view/TrackLabKeyboardHelpContent.ts @@ -15,20 +15,29 @@ * (coordinate system, calibration, tape, angle) * • BasicActionsKeyboardHelpSection — Tab, press buttons, toggle checkboxes, * Reset All, Escape + * • Digitizing — Delete/Backspace on the digitizing overlay + * (hand-written: the binding is a plain + * KeyboardListener, not HotkeyData-backed) */ import { ArrowKeyIconDisplay, BasicActionsKeyboardHelpSection, + KeyboardHelpSection, + KeyboardHelpSectionRow, MoveDraggableItemsKeyboardHelpSection, SliderControlsKeyboardHelpSection, + TextKeyNode, TimeControlsKeyboardHelpSection, TwoColumnKeyboardHelpContent, } from "scenerystack/scenery-phet"; +import { StringManager } from "../../i18n/StringManager.js"; import TrackLabNamespace from "../../TrackLabNamespace.js"; export class TrackLabKeyboardHelpContent extends TwoColumnKeyboardHelpContent { public constructor() { + const shortcutStrings = StringManager.getInstance().getKeyboardShortcutsStrings(); + super( [ new TimeControlsKeyboardHelpSection(), @@ -36,6 +45,14 @@ export class TrackLabKeyboardHelpContent extends TwoColumnKeyboardHelpContent { new SliderControlsKeyboardHelpSection({ arrowKeyIconDisplay: ArrowKeyIconDisplay.LEFT_RIGHT, }), + // Erases the active track's point at the current frame, matching the + // eraser button in the playback controls. + new KeyboardHelpSection(shortcutStrings.digitizingStringProperty, [ + KeyboardHelpSectionRow.labelWithIconList(shortcutStrings.erasePointStringProperty, [ + TextKeyNode.delete(), + TextKeyNode.backspace(), + ]), + ]), ], [ // Coordinate system, calibration tool, measuring tape, angle tool are all diff --git a/src/track-lab/view/TrackLabScreenView.ts b/src/track-lab/view/TrackLabScreenView.ts index 7369c06..08d8fb4 100644 --- a/src/track-lab/view/TrackLabScreenView.ts +++ b/src/track-lab/view/TrackLabScreenView.ts @@ -169,6 +169,7 @@ export class TrackLabScreenView extends ScreenView { // ── Data table (top right, shifts left when window is wider than layoutBounds) ─ const dataTableNode = new DataTableNode( model.tracking, + model.playback, model.playback.videoLoadedProperty, model.overlayTools.calibUnitProperty, ); diff --git a/src/track-lab/view/VideoPlayerNode.ts b/src/track-lab/view/VideoPlayerNode.ts index c6cedac..8c2fc29 100644 --- a/src/track-lab/view/VideoPlayerNode.ts +++ b/src/track-lab/view/VideoPlayerNode.ts @@ -173,6 +173,7 @@ export class VideoPlayerNode extends Node { magnifyVideoProperty: model.overlayTools.magnifyVideoProperty, modelViewTransformProperty: model.overlayTools.modelViewTransformProperty, recordPoint: (trackId, pixelPoint) => model.recordTrackPoint(trackId, pixelPoint), + erasePoint: () => model.deleteTrackPointAtCurrentFrame(), }, () => this.stepForward(), ); @@ -223,9 +224,11 @@ export class VideoPlayerNode extends Node { // ── Playback controls (positioned by TrackLabScreenView at screen bottom) ── this.playbackControlsNode = new PlaybackControlsNode( model.playback, + model.tracking, this.videoElement, () => this.seekByFrames(-1), () => this.seekByFrames(1), + () => model.deleteTrackPointAtCurrentFrame(), ); // Pin to the video width so internal text changes never shift the row. this.playbackControlsNode.preferredWidth = VIDEO_WIDTH; diff --git a/tests/fuzz/fuzz.spec.ts b/tests/fuzz/fuzz.spec.ts index 153f0bb..fefbd1d 100644 --- a/tests/fuzz/fuzz.spec.ts +++ b/tests/fuzz/fuzz.spec.ts @@ -10,9 +10,9 @@ import { expect, test } from "@playwright/test"; const FUZZ_DURATION = parseInt(process.env["FUZZ_DURATION"] || "15", 10) * 1000; -const FUZZ_SEED = process.env["FUZZ_SEED"] || Math.floor(Math.random() * 1_000_000).toString(); -const FUZZ_RATE = process.env["FUZZ_RATE"] || "100"; -const FUZZ_POINTERS = process.env["FUZZ_POINTERS"] || "1"; +const FUZZ_SEED: string = process.env["FUZZ_SEED"] || Math.floor(Math.random() * 1_000_000).toString(); +const FUZZ_RATE: string = process.env["FUZZ_RATE"] || "100"; +const FUZZ_POINTERS: string = process.env["FUZZ_POINTERS"] || "1"; interface ConsoleMessage { type: string; diff --git a/tests/memory-leak.test.ts b/tests/memory-leak.test.ts index f7f3f3f..3cb5037 100644 --- a/tests/memory-leak.test.ts +++ b/tests/memory-leak.test.ts @@ -30,14 +30,27 @@ async function forceGC(earlyExitRef?: WeakRef): Promise { } } -const SAMPLE_PLOTTABLES: PlottableProperty[] = [ - { name: "t", accessor: (point) => point.t ?? 0 }, - { name: "x", accessor: (point) => point.x ?? 0 }, -]; +// Named separately from the array so callers can reference them without an +// index lookup, which under noUncheckedIndexedAccess would widen to +// `PlottableProperty | undefined` and force a non-null assertion. +// Bracket access because `point` is a Record and the project +// sets noPropertyAccessFromIndexSignature. +const TIME_PLOTTABLE: PlottableProperty = { + name: "t", + accessor: (point: Record) => point["t"] ?? 0, +}; +const POSITION_PLOTTABLE: PlottableProperty = { + name: "x", + accessor: (point: Record) => point["x"] ?? 0, +}; + +const SAMPLE_PLOTTABLES: PlottableProperty[] = [TIME_PLOTTABLE, POSITION_PLOTTABLE]; function createAndDisposeGraphControlsPanel(): WeakRef { - const xPropertyProperty = new Property(SAMPLE_PLOTTABLES[0]!); - const yPropertyProperty = new Property(SAMPLE_PLOTTABLES[1]!); + // Explicit type argument: TypeScript narrows these consts to RecordPlottable + // from their initializers, and Property is invariant in T. + const xPropertyProperty = new Property(TIME_PLOTTABLE); + const yPropertyProperty = new Property(POSITION_PLOTTABLE); const panel = new GraphControlsPanel(SAMPLE_PLOTTABLES, xPropertyProperty, yPropertyProperty, 200); const ref = new WeakRef(panel); panel.dispose(); @@ -64,8 +77,8 @@ describe("Memory leak regression", () => { }); it("double dispose() does not throw", () => { - const xPropertyProperty = new Property(SAMPLE_PLOTTABLES[0]!); - const yPropertyProperty = new Property(SAMPLE_PLOTTABLES[1]!); + const xPropertyProperty = new Property(TIME_PLOTTABLE); + const yPropertyProperty = new Property(POSITION_PLOTTABLE); const panel = new GraphControlsPanel(SAMPLE_PLOTTABLES, xPropertyProperty, yPropertyProperty, 200); panel.dispose(); expect(() => panel.dispose()).not.toThrow(); diff --git a/tests/setup.ts b/tests/setup.ts index e6bee0b..59992a2 100644 --- a/tests/setup.ts +++ b/tests/setup.ts @@ -141,6 +141,22 @@ class MockAudioContext { (globalThis as Record)["AudioContext"] = MockAudioContext; (globalThis as Record)["webkitAudioContext"] = MockAudioContext; +// ── Web Worker mock ────────────────────────────────────────────────────────── +// TrackingModel constructs an OpenCVTracker (and therefore a Worker) as a field +// initializer, so model tests need Worker to exist. happy-dom does not provide +// it. Messages are swallowed: no test drives the OpenCV worker protocol, and a +// real worker cannot run under happy-dom anyway. +class MockWorker { + onmessage: ((event: MessageEvent) => void) | null = null; + onerror: ((event: ErrorEvent) => void) | null = null; + postMessage: () => void = noop; + terminate: () => void = noop; + addEventListener: () => void = noop; + removeEventListener: () => void = noop; + dispatchEvent: () => boolean = () => false; +} +(globalThis as Record)["Worker"] = MockWorker; + // ── patch getContext("2d") before any scenerystack import ──────────────────── const origGetContext: typeof HTMLCanvasElement.prototype.getContext = HTMLCanvasElement.prototype.getContext; HTMLCanvasElement.prototype.getContext = function (this: HTMLCanvasElement, contextId: string, ...args: unknown[]) { diff --git a/tests/track-lab/model/TrackingModel.test.ts b/tests/track-lab/model/TrackingModel.test.ts new file mode 100644 index 0000000..124cba5 --- /dev/null +++ b/tests/track-lab/model/TrackingModel.test.ts @@ -0,0 +1,205 @@ +import { beforeEach, describe, expect, it } from "vitest"; +import { computeTrackKinematics } from "../../../src/track-lab/model/KinematicsComputer.js"; +import type { TrackPoint } from "../../../src/track-lab/model/Track.js"; +import { TrackingModel } from "../../../src/track-lab/model/TrackingModel.js"; + +/** Frames of the first (and usually only) track, in stored order. */ +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); +} + +function pointAt(model: TrackingModel, frame: number, trackId = "track-A"): TrackPoint | undefined { + const track = model.tracksProperty.value.find((t) => t.id === trackId); + return track?.points.find((p) => p.frame === frame); +} + +/** Digitize frames 0..count-1, one model unit apart along x, at 1 s intervals. */ +function digitize(model: TrackingModel, count: number, trackId = "track-A"): void { + for (let frame = 0; frame < count; frame++) { + model.addOrReplacePointOnTrack(trackId, frame, frame, frame, 0); + } +} + +describe("TrackingModel point deletion", () => { + let model: TrackingModel; + + beforeEach(() => { + model = new TrackingModel(); + model.addTrack(); + }); + + it("removes the point at the given frame and leaves the rest intact", () => { + digitize(model, 4); + + model.removePointFromTrack("track-A", 2); + + expect(frames(model)).toEqual([0, 1, 3]); + }); + + it("removes the first and the last point", () => { + digitize(model, 3); + + model.removePointFromTrack("track-A", 0); + expect(frames(model)).toEqual([1, 2]); + + model.removePointFromTrack("track-A", 2); + expect(frames(model)).toEqual([1]); + }); + + it("removes the only point, leaving an empty track", () => { + digitize(model, 1); + + model.removePointFromTrack("track-A", 0); + + expect(frames(model)).toEqual([]); + expect(model.tracksProperty.value).toHaveLength(1); + }); + + it("is a no-op for a frame with no point, and for an unknown track", () => { + digitize(model, 2); + const before = model.tracksProperty.value; + + model.removePointFromTrack("track-A", 99); + model.removePointFromTrack("track-Z", 0); + + expect(model.tracksProperty.value).toBe(before); + expect(model.canRestorePointProperty.value).toBe(false); + }); + + it("replaces the points array so the kinematics cache invalidates", () => { + digitize(model, 3); + const before = model.trackKinematicsProperty.value[0]; + + model.removePointFromTrack("track-A", 1); + const after = model.trackKinematicsProperty.value[0]; + + expect(after).not.toBe(before); + expect(after?.points.map((p) => p.frame)).toEqual([0, 2]); + }); + + it("recomputes velocity across the gap left by a deleted point", () => { + digitize(model, 3); + + model.removePointFromTrack("track-A", 1); + + // Two points 2 s apart, 2 units apart → 1 unit/s, computed from the + // recorded timestamps rather than an assumed constant frame interval. + const track = model.tracksProperty.value[0]; + expect(track).toBeDefined(); + if (!track) { + return; + } + const kinematics = computeTrackKinematics(track); + expect(kinematics.points[0]?.vx).toBeCloseTo(1, 6); + expect(kinematics.points[1]?.vx).toBeCloseTo(1, 6); + }); +}); + +describe("TrackingModel restore last deleted point", () => { + let model: TrackingModel; + + beforeEach(() => { + model = new TrackingModel(); + model.addTrack(); + }); + + it("round-trips a deleted interior point back into frame order", () => { + digitize(model, 4); + + model.removePointFromTrack("track-A", 1); + expect(model.canRestorePointProperty.value).toBe(true); + + model.restoreLastDeletedPoint(); + + // Restored in sorted position, not appended — KinematicsComputer + // differentiates by array index and assumes time increases along it. + expect(frames(model)).toEqual([0, 1, 2, 3]); + expect(model.canRestorePointProperty.value).toBe(false); + }); + + it("restores the point's original coordinates", () => { + model.addOrReplacePointOnTrack("track-A", 5, 0.5, 12.5, -3.25); + + model.removePointFromTrack("track-A", 5); + model.restoreLastDeletedPoint(); + + expect(pointAt(model, 5)).toEqual({ frame: 5, time: 0.5, x: 12.5, y: -3.25 }); + }); + + it("only remembers the most recent deletion", () => { + digitize(model, 3); + + model.removePointFromTrack("track-A", 0); + model.removePointFromTrack("track-A", 1); + model.restoreLastDeletedPoint(); + + expect(frames(model)).toEqual([1, 2]); + expect(model.canRestorePointProperty.value).toBe(false); + }); + + it("drops the stash when the point's track is removed", () => { + digitize(model, 2); + + model.removePointFromTrack("track-A", 0); + expect(model.canRestorePointProperty.value).toBe(true); + + model.removeTrack("track-A"); + + expect(model.canRestorePointProperty.value).toBe(false); + model.restoreLastDeletedPoint(); + expect(model.tracksProperty.value).toHaveLength(0); + }); + + it("clears the stash on reset", () => { + digitize(model, 1); + model.removePointFromTrack("track-A", 0); + + model.reset(); + + expect(model.canRestorePointProperty.value).toBe(false); + }); +}); + +describe("TrackingModel point insertion policies", () => { + let model: TrackingModel; + + beforeEach(() => { + model = new TrackingModel(); + model.addTrack(); + }); + + it("keeps the first position when auto-tracking re-reports a frame", () => { + model.addPointToTrack("track-A", 0, 0, 1, 1); + model.addPointToTrack("track-A", 0, 0, 9, 9); + + expect(pointAt(model, 0)).toMatchObject({ x: 1, y: 1 }); + }); + + it("overwrites the position when the user re-digitizes a frame", () => { + model.addOrReplacePointOnTrack("track-A", 0, 0, 1, 1); + model.addOrReplacePointOnTrack("track-A", 0, 0, 9, 9); + + expect(pointAt(model, 0)).toMatchObject({ x: 9, y: 9 }); + expect(frames(model)).toEqual([0]); + }); + + it("stores points in frame order when digitized out of order", () => { + model.addOrReplacePointOnTrack("track-A", 4, 4, 0, 0); + model.addOrReplacePointOnTrack("track-A", 1, 1, 0, 0); + model.addPointToTrack("track-A", 2, 2, 0, 0); + + expect(frames(model)).toEqual([1, 2, 4]); + }); + + it("leaves other tracks untouched", () => { + model.addTrack(); + digitize(model, 2, "track-A"); + digitize(model, 2, "track-B"); + + model.removePointFromTrack("track-A", 0); + + expect(frames(model, "track-A")).toEqual([1]); + expect(frames(model, "track-B")).toEqual([0, 1]); + }); +}); diff --git a/tsconfig.tests.json b/tsconfig.tests.json new file mode 100644 index 0000000..331b228 --- /dev/null +++ b/tsconfig.tests.json @@ -0,0 +1,8 @@ +{ + "extends": "./tsconfig.json", + "compilerOptions": { + "noEmit": true, + "types": ["node"] + }, + "include": ["tests"] +}