diff --git a/src/TrackLabConstants.ts b/src/TrackLabConstants.ts index 2e954fa..55c6a65 100644 --- a/src/TrackLabConstants.ts +++ b/src/TrackLabConstants.ts @@ -10,6 +10,11 @@ // Shared corner radius used by the main side panels. export const PANEL_CORNER_RADIUS = 8; +// ── Screen layout bounds ─────────────────────────────────────────────────────── +// SceneryStack's ScreenView.DEFAULT_LAYOUT_BOUNDS = Bounds2(0, 0, 1024, 618). +const LAYOUT_WIDTH = 1024; +const LAYOUT_HEIGHT = 618; + // ── Video display dimensions ─────────────────────────────────────────────────── // The video element is always rendered at this fixed pixel size. // Both the OpenCV tracker and all overlay nodes depend on these values. @@ -17,18 +22,16 @@ export const VIDEO_WIDTH = 640; export const VIDEO_HEIGHT = 360; // ── Video position in screen (layout) coordinates ──────────────────────────── -// SceneryStack's ScreenView.DEFAULT_LAYOUT_BOUNDS = Bounds2(0, 0, 1024, 618). // The video element is centered at layoutBounds.center + (0, VIDEO_PLAYER_Y_OFFSET). -export const VIDEO_CENTER_X = 512; // 1024 / 2 -export const VIDEO_CENTER_Y = 289; // 618 / 2 + VIDEO_PLAYER_Y_OFFSET (309 - 20) +export const VIDEO_PLAYER_Y_OFFSET = -20; // video center offset below layout center +export const VIDEO_CENTER_X = LAYOUT_WIDTH / 2; // 512 +export const VIDEO_CENTER_Y = LAYOUT_HEIGHT / 2 + VIDEO_PLAYER_Y_OFFSET; // 289 // ── Initial calibration tool geometry ───────────────────────────────────────── // Half-length of the default calibration segment (pixels from centre to each endpoint). export const CALIB_HALF_LENGTH = 100; // ── Screen layout offsets ───────────────────────────────────────────────────── -// SceneryStack's ScreenView.DEFAULT_LAYOUT_BOUNDS = Bounds2(0, 0, 1024, 618). -export const VIDEO_PLAYER_Y_OFFSET = -20; // video center offset below layout center export const CONTROL_PANEL_LEFT_MARGIN = 10; // control panel inset from layout left edge export const TRACK_LIST_LEFT_SPACING = -100; // offset from video right edge (negative moves panels left) export const DATA_TABLE_TOP_SPACING = 8; // gap between track list bottom and data table diff --git a/src/i18n/StringManager.ts b/src/i18n/StringManager.ts index 8e356a6..43b6451 100644 --- a/src/i18n/StringManager.ts +++ b/src/i18n/StringManager.ts @@ -9,6 +9,16 @@ import { LocalizedString, type ReadOnlyProperty } from "scenerystack"; import strings_en from "./strings_en.json"; import strings_fr from "./strings_fr.json"; +// ── Compile-time key-parity check ───────────────────────────────────────────── +// These assignments are never executed at runtime; they exist solely so +// TypeScript verifies that both language files share identical key structures. +// If a key is added to one file but not the other, a type error will appear +// here before the app is ever run. +// biome-ignore lint/correctness/noUnusedVariables: compile-time type guard only +const _enMatchesFr: typeof strings_fr = strings_en; +// biome-ignore lint/correctness/noUnusedVariables: compile-time type guard only +const _frMatchesEn: typeof strings_en = strings_fr; + /** * Manages all localized strings for the simulation */ diff --git a/src/screen-name/graph/GraphInteractionHandler.ts b/src/screen-name/graph/GraphInteractionHandler.ts index b5925a2..7122e42 100644 --- a/src/screen-name/graph/GraphInteractionHandler.ts +++ b/src/screen-name/graph/GraphInteractionHandler.ts @@ -539,7 +539,11 @@ export default class GraphInteractionHandler { if (mouseDragStartY !== null && mouseDragInitialYRange) { const deltaY = event.pointer.point.y - mouseDragStartY; - // Convert delta to model coordinates + // Convert delta to model coordinates. + // Screen Y increases downward while model Y increases upward, so a + // positive screen delta (drag down) corresponds to a positive model + // delta (shift range up). The result is that content follows the + // drag — dragging down pans the view down — matching the X-axis UX. const modelDeltaY = deltaY * (mouseDragInitialYRange.getLength() / this.graphHeight); @@ -766,7 +770,12 @@ export default class GraphInteractionHandler { if (mouseDragStartX !== null && mouseDragInitialXRange) { const deltaX = event.pointer.point.x - mouseDragStartX; - // Convert delta to model coordinates + // Convert delta to model coordinates. + // Screen X and model X share the same direction, so without the + // negation a rightward drag would shift the range right and the + // content would move LEFT. The negation makes the content follow + // the drag — dragging right pans the view right — matching the + // Y-axis UX above. const modelDeltaX = -deltaX * (mouseDragInitialXRange.getLength() / this.graphWidth); diff --git a/src/screen-name/model/SimModel.ts b/src/screen-name/model/SimModel.ts index 9de0c16..0e192cd 100644 --- a/src/screen-name/model/SimModel.ts +++ b/src/screen-name/model/SimModel.ts @@ -56,6 +56,14 @@ const CALIB_CENTER_INITIAL = new Vector2( const CALIB_P1_INITIAL = CALIB_CENTER_INITIAL.plusXY(-CALIB_HALF_LENGTH, 0); const CALIB_P2_INITIAL = CALIB_CENTER_INITIAL.plusXY(CALIB_HALF_LENGTH, 0); +// ── Bounds for clamping the coordinate-system origin ───────────────────────── +// The origin must stay within the video area so the axes are always visible. +// These are layout / pixel-space bounds, matching the view-layer video rectangle. +const COORD_ORIGIN_BOUNDS_MIN_X = VIDEO_CENTER_X - VIDEO_WIDTH / 2; +const COORD_ORIGIN_BOUNDS_MAX_X = VIDEO_CENTER_X + VIDEO_WIDTH / 2; +const COORD_ORIGIN_BOUNDS_MIN_Y = VIDEO_CENTER_Y - VIDEO_HEIGHT / 2; +const COORD_ORIGIN_BOUNDS_MAX_Y = VIDEO_CENTER_Y + VIDEO_HEIGHT / 2; + // ── Model-view transform builder ─────────────────────────────────────────── /** * Builds a Transform3 from the coordinate-system tool and calibration tool. @@ -90,6 +98,36 @@ function buildModelViewTransform( } // ── Kinematics computation ─────────────────────────────────────────────── + +/** + * Scalar finite difference at index i within an array of n values. + * + * Uses forward difference at the first point, backward difference at the last, + * and central differences for all interior points. Returns null if the array + * has fewer than 2 elements, if the time interval is non-positive, or if + * either endpoint value is null. + * + * @param getValue Returns the scalar quantity at index j (null = unknown). + * @param getTime Returns the time stamp at index j. + * @param i Index to differentiate at. + * @param n Total number of elements. + */ +function finiteDifference( + getValue: (j: number) => number | null, + getTime: (j: number) => number, + i: number, + n: number, +): number | null { + if (n < 2) return null; + const [prevIdx, nextIdx] = + i === 0 ? [0, 1] : i === n - 1 ? [n - 2, n - 1] : [i - 1, i + 1]; + const prev = getValue(prevIdx); + const next = getValue(nextIdx); + const dt = getTime(nextIdx) - getTime(prevIdx); + if (prev === null || next === null || dt <= 0) return null; + return (next - prev) / dt; +} + /** * Computes velocity and acceleration for each point in a track. * Uses central differences where possible for better accuracy. @@ -110,153 +148,27 @@ function computeTrackKinematics(track: Track): TrackKinematics { return { ...track, points: [] }; } - // Helper to safely get a point (returns undefined if out of bounds) - const getPoint = (idx: number): TrackPoint | undefined => points[idx]; - - // Compute velocity at index i using finite differences - const computeVelocity = ( - i: number, - ): { vx: number | null; vy: number | null } => { - if (n < 2) { - return { vx: null, vy: null }; - } - - const curr = getPoint(i); - if (!curr) return { vx: null, vy: null }; - - if (i === 0) { - // Forward difference for first point - const next = getPoint(1); - if (!next) return { vx: null, vy: null }; - const dt = next.time - curr.time; - if (dt <= 0) return { vx: null, vy: null }; - return { - vx: (next.x - curr.x) / dt, - vy: (next.y - curr.y) / dt, - }; - } - - if (i === n - 1) { - // Backward difference for last point - const prev = getPoint(n - 2); - if (!prev) return { vx: null, vy: null }; - const dt = curr.time - prev.time; - if (dt <= 0) return { vx: null, vy: null }; - return { - vx: (curr.x - prev.x) / dt, - vy: (curr.y - prev.y) / dt, - }; - } - - // Central difference for interior points - const prev = getPoint(i - 1); - const next = getPoint(i + 1); - if (!prev || !next) return { vx: null, vy: null }; - const dt = next.time - prev.time; - if (dt <= 0) return { vx: null, vy: null }; - return { - vx: (next.x - prev.x) / dt, - vy: (next.y - prev.y) / dt, - }; - }; - - // First pass: compute velocities - const velocities = points.map((_, i) => computeVelocity(i)); - - // Helper to safely get velocity - const getVelocity = ( - idx: number, - ): { vx: number | null; vy: number | null } | undefined => velocities[idx]; - - // Compute acceleration at index i using finite differences on velocities - const computeAcceleration = ( - i: number, - ): { ax: number | null; ay: number | null } => { - if (n < 2) { - return { ax: null, ay: null }; - } - - const curr = getPoint(i); - if (!curr) return { ax: null, ay: null }; - - if (i === 0) { - // Forward difference - const v0 = getVelocity(0); - const v1 = getVelocity(1); - const next = getPoint(1); - if (!v0 || !v1 || !next) return { ax: null, ay: null }; - if ( - v0.vx === null || - v1.vx === null || - v0.vy === null || - v1.vy === null - ) { - return { ax: null, ay: null }; - } - const dt = next.time - curr.time; - if (dt <= 0) return { ax: null, ay: null }; - return { - ax: (v1.vx - v0.vx) / dt, - ay: (v1.vy - v0.vy) / dt, - }; - } + const getTime = (j: number) => points[j]?.time ?? 0; + const getX = (j: number) => points[j]?.x ?? null; + const getY = (j: number) => points[j]?.y ?? null; - if (i === n - 1) { - // Backward difference - const vPrev = getVelocity(n - 2); - const vCurr = getVelocity(n - 1); - const prev = getPoint(n - 2); - if (!vPrev || !vCurr || !prev) return { ax: null, ay: null }; - if ( - vPrev.vx === null || - vCurr.vx === null || - vPrev.vy === null || - vCurr.vy === null - ) { - return { ax: null, ay: null }; - } - const dt = curr.time - prev.time; - if (dt <= 0) return { ax: null, ay: null }; - return { - ax: (vCurr.vx - vPrev.vx) / dt, - ay: (vCurr.vy - vPrev.vy) / dt, - }; - } - - // Central difference - const vPrev = getVelocity(i - 1); - const vNext = getVelocity(i + 1); - const prev = getPoint(i - 1); - const next = getPoint(i + 1); - if (!vPrev || !vNext || !prev || !next) return { ax: null, ay: null }; - if ( - vPrev.vx === null || - vNext.vx === null || - vPrev.vy === null || - vNext.vy === null - ) { - return { ax: null, ay: null }; - } - const dt = next.time - prev.time; - if (dt <= 0) return { ax: null, ay: null }; - return { - ax: (vNext.vx - vPrev.vx) / dt, - ay: (vNext.vy - vPrev.vy) / dt, - }; - }; + // First pass: velocities via finite difference of position + const vxArr = points.map((_, i) => finiteDifference(getX, getTime, i, n)); + const vyArr = points.map((_, i) => finiteDifference(getY, getTime, i, n)); - // Second pass: compute accelerations - const accelerations = points.map((_, i) => computeAcceleration(i)); + // Second pass: accelerations via finite difference of velocity + const axArr = points.map((_, i) => + finiteDifference((j) => vxArr[j] ?? null, getTime, i, n), + ); + const ayArr = points.map((_, i) => + finiteDifference((j) => vyArr[j] ?? null, getTime, i, n), + ); - // Combine all data into KinematicPoints const kinematicPoints: KinematicPoint[] = points.map((pt, i) => { - const vel = velocities[i]; - const acc = accelerations[i]; - const vx = vel?.vx ?? null; - const vy = vel?.vy ?? null; - const ax = acc?.ax ?? null; - const ay = acc?.ay ?? null; - + const vx = vxArr[i] ?? null; + const vy = vyArr[i] ?? null; + const ax = axArr[i] ?? null; + const ay = ayArr[i] ?? null; return { frame: pt.frame, time: pt.time, @@ -376,11 +288,27 @@ export class SimModel { public readonly canAddTrackProperty = new BooleanProperty(true); // ── Derived kinematics for all tracks ─────────────────────────────────── - // Automatically computes velocity and acceleration from position data + // Cache keyed by track ID; only recomputes kinematics for tracks whose + // point array reference has changed since the last derivation. Because + // addPointToTrack() always creates a new points array, reference equality + // is sufficient to detect modifications. + private readonly kinematicsCache = new Map< + string, + { points: Track["points"]; kinematics: TrackKinematics } + >(); + public readonly trackKinematicsProperty: TReadOnlyProperty< readonly TrackKinematics[] > = new DerivedProperty([this.tracksProperty], (tracks) => - tracks.map((track) => computeTrackKinematics(track)), + tracks.map((track) => { + const cached = this.kinematicsCache.get(track.id); + if (cached && cached.points === track.points) { + return cached.kinematics; + } + const kinematics = computeTrackKinematics(track); + this.kinematicsCache.set(track.id, { points: track.points, kinematics }); + return kinematics; + }), ); // Symbols are assigned sequentially (A → Z) and intentionally not reused // after a track is removed. Stable, unique symbols matter for data export @@ -389,6 +317,27 @@ export class SimModel { private nextSymbolCode = TRACK_SYMBOL_FIRST_CODE; public constructor() { + // ── Clamp coord origin to the video area ──────────────────────────────── + // Validation lives here rather than in the view so that any writer of + // coordOriginProperty (drag listener, programmatic reset, etc.) benefits + // from the constraint without needing per-call clamping logic. + // The guard `if (clampedX !== pos.x || clampedY !== pos.y)` prevents + // infinite recursion: after the clamped value is written, the listener + // fires again but finds the condition false and exits. + this.coordOriginProperty.lazyLink((pos) => { + const clampedX = Math.max( + COORD_ORIGIN_BOUNDS_MIN_X, + Math.min(COORD_ORIGIN_BOUNDS_MAX_X, pos.x), + ); + const clampedY = Math.max( + COORD_ORIGIN_BOUNDS_MIN_Y, + Math.min(COORD_ORIGIN_BOUNDS_MAX_Y, pos.y), + ); + if (clampedX !== pos.x || clampedY !== pos.y) { + this.coordOriginProperty.value = pos.copy().setXY(clampedX, clampedY); + } + }); + this.modelViewTransformProperty.lazyLink((newMVT) => { if (this.prevModelViewTransform === null) { this.prevModelViewTransform = newMVT; @@ -455,10 +404,16 @@ export class SimModel { const tracks = this.tracksProperty.value.map((track) => { if (track.id !== id) return track; - // Reject duplicate frames. Without this guard a second click on the - // same frame (manual digitizing) or a repeated timeupdate event would - // add a second point at the same frame index, corrupting kinematics. - if (track.points.some((p) => p.frame === frame)) return track; + // If the user re-digitizes a point at the same frame (e.g. to correct a + // misclick), replace the existing coordinates rather than silently + // discarding the new position. Adding a second point at the same frame + // would corrupt kinematics, so replacement is the only safe update path. + const existingIndex = track.points.findIndex((p) => p.frame === frame); + if (existingIndex !== -1) { + const updatedPoints = [...track.points]; + updatedPoints[existingIndex] = { frame, time, x, y }; + return { ...track, points: updatedPoints }; + } const point: TrackPoint = { frame, time, x, y }; const updated: Track = { ...track, points: [...track.points, point] }; @@ -469,6 +424,7 @@ export class SimModel { public reset(): void { this.prevModelViewTransform = null; + this.kinematicsCache.clear(); this.isPlayingProperty.reset(); this.currentTimeProperty.reset(); this.durationProperty.reset(); diff --git a/src/screen-name/view/AutoTrackerNode.ts b/src/screen-name/view/AutoTrackerNode.ts index 5e609a2..2360026 100644 --- a/src/screen-name/view/AutoTrackerNode.ts +++ b/src/screen-name/view/AutoTrackerNode.ts @@ -46,7 +46,14 @@ const TRAIL_DOT_RADIUS = 3; // radius of each past-position dot in the trail */ export class AutoTrackerNode extends Node { private readonly model: SimModel; - private readonly trail: Array<{ x: number; y: number }> = []; + + // ── Trail: O(1) ring buffer ──────────────────────────────────────────── + // Using a fixed-size circular buffer instead of a plain array so that the + // oldest-point eviction at 30 Hz is O(1) rather than O(n) (Array.shift). + private readonly trailBuf: Array<{ x: number; y: number }> = + new Array(MAX_TRAIL); + private trailHead = 0; // index of the slot where the NEXT write will land + private trailSize = 0; // number of valid entries (0 … MAX_TRAIL) /** Frames already recorded to the active track; cleared on track change or reset. */ private readonly recordedFrames = new Set(); @@ -143,7 +150,8 @@ export class AutoTrackerNode extends Node { // ── Drag listener: region selection ────────────────────────────────── const dragListener = new DragListener({ start: (event) => { - this.trail.length = 0; + this.trailHead = 0; + this.trailSize = 0; // Bump version so any in-flight initFromVideo call is discarded when it resolves. this.initVersion++; this.model.tracker.dispose(); @@ -209,6 +217,19 @@ export class AutoTrackerNode extends Node { if (this.initVersion !== capturedVersion) { // A newer drag has already started; discard this result. this.model.tracker.dispose(); + return; + } + // Guard against the race condition where the user removes the + // active track while WASM was loading. If the track no longer + // exists, abort tracking so the crosshair doesn't appear with + // nowhere to record points. + const activeId = this.model.activeTrackIdProperty.value; + const trackStillExists = + activeId !== null && + this.model.tracksProperty.value.some((t) => t.id === activeId); + if (!trackStillExists) { + this.model.tracker.dispose(); + this.hintText.visible = true; } }) .catch((err) => { @@ -234,8 +255,10 @@ export class AutoTrackerNode extends Node { const pt = this.model.tracker.track(videoElement); if (!pt) return; - this.trail.push(pt); - if (this.trail.length > MAX_TRAIL) this.trail.shift(); + // O(1) ring-buffer write: overwrite the oldest slot when full. + this.trailBuf[this.trailHead] = pt; + this.trailHead = (this.trailHead + 1) % MAX_TRAIL; + if (this.trailSize < MAX_TRAIL) this.trailSize++; this.updateTrackerVisuals(pt); // ── Record position to model if a track is active ───────────────── @@ -289,8 +312,12 @@ export class AutoTrackerNode extends Node { private updateTrackerVisuals(pt: { x: number; y: number }): void { const shape = new Shape(); - for (const p of this.trail) { - shape.circle(p.x, p.y, TRAIL_DOT_RADIUS); + // Iterate the ring buffer from oldest to newest. + // tail = (head - size + MAX_TRAIL) % MAX_TRAIL is the index of the oldest entry. + for (let i = 0; i < this.trailSize; i++) { + const idx = (this.trailHead - this.trailSize + i + MAX_TRAIL) % MAX_TRAIL; + const p = this.trailBuf[idx]; + if (p) shape.circle(p.x, p.y, TRAIL_DOT_RADIUS); } this.trailPath.shape = shape; this.trailPath.visible = true; @@ -306,7 +333,8 @@ export class AutoTrackerNode extends Node { /** Clear tracking state (template, trail, visuals). */ public reset(): void { this.model.tracker.dispose(); - this.trail.length = 0; + this.trailHead = 0; + this.trailSize = 0; this.recordedFrames.clear(); this.selecting = false; this.selectionRect.visible = false; diff --git a/src/screen-name/view/CalibrationToolNode.ts b/src/screen-name/view/CalibrationToolNode.ts index 213a686..5333206 100644 --- a/src/screen-name/view/CalibrationToolNode.ts +++ b/src/screen-name/view/CalibrationToolNode.ts @@ -1,6 +1,6 @@ import { Color } from "scenerystack"; import type { TReadOnlyProperty } from "scenerystack/axon"; -import { DerivedProperty } from "scenerystack/axon"; +import { DerivedProperty, Multilink } from "scenerystack/axon"; import { Shape } from "scenerystack/kite"; import { Circle, @@ -47,6 +47,8 @@ const OVERLAP_WARNING_DISTANCE = 10; const ENDPOINT_WARNING_COLOR = new Color(255, 60, 60); export class CalibrationToolNode extends Node { + private readonly disposeCalibrationToolNode: () => void; + public constructor( videoLoadedProperty: TReadOnlyProperty, listParent: Node, @@ -238,8 +240,13 @@ export class CalibrationToolNode extends Node { overlapWarning.top = mid.y + MIDPOINT_Y_OFFSET; } }; - model.calibPoint1Property.link(updateGeometry); - model.calibPoint2Property.link(updateGeometry); + // A single Multilink replaces two separate link() calls so that geometry + // is rebuilt once per change event regardless of which endpoint moved, + // and disposal is managed in one place. + const calibMultilink = Multilink.multilink( + [model.calibPoint1Property, model.calibPoint2Property], + updateGeometry, + ); // ── Drag listeners for endpoints ────────────────────────────────────── endpoint1.addInputListener( @@ -264,8 +271,21 @@ export class CalibrationToolNode extends Node { ); // ── Visibility ───────────────────────────────────────────────────────── - videoLoadedProperty.link((loaded) => { + const onVideoLoaded = (loaded: boolean) => { this.visible = loaded; - }); + }; + videoLoadedProperty.link(onVideoLoaded); + + this.disposeCalibrationToolNode = () => { + calibMultilink.dispose(); + videoLoadedProperty.unlink(onVideoLoaded); + rangePatternProperty.dispose(); + buttonLabelProperty.dispose(); + }; + } + + public override dispose(): void { + this.disposeCalibrationToolNode(); + super.dispose(); } } diff --git a/src/screen-name/view/CoordinateSystemNode.ts b/src/screen-name/view/CoordinateSystemNode.ts index 3660b25..874623c 100644 --- a/src/screen-name/view/CoordinateSystemNode.ts +++ b/src/screen-name/view/CoordinateSystemNode.ts @@ -1,28 +1,12 @@ import type { TReadOnlyProperty } from "scenerystack/axon"; -import { Bounds2 } from "scenerystack/dot"; +import { Vector2 } from "scenerystack/dot"; import { Shape } from "scenerystack/kite"; import { Circle, Node, RichDragListener, Text } from "scenerystack/scenery"; import { ArrowNode, PhetFont } from "scenerystack/scenery-phet"; import { Tandem } from "scenerystack/tandem"; import { StringManager } from "../../i18n/StringManager.js"; import TrackLabColors from "../../TrackLabColors.js"; -import { - VIDEO_CENTER_X, - VIDEO_CENTER_Y, - VIDEO_HEIGHT, - VIDEO_WIDTH, -} from "../../TrackLabConstants.js"; import type { SimModel } from "../model/SimModel.js"; - -const ARROW_LENGTH = 120; - -// Bounds of the video area in view (pixel) coordinates — used to clamp the coord origin drag. -const VIDEO_BOUNDS = new Bounds2( - VIDEO_CENTER_X - VIDEO_WIDTH / 2, - VIDEO_CENTER_Y - VIDEO_HEIGHT / 2, - VIDEO_CENTER_X + VIDEO_WIDTH / 2, - VIDEO_CENTER_Y + VIDEO_HEIGHT / 2, -); const HANDLE_FRACTION = 1 / 3; const FONT = new PhetFont({ size: 14, weight: "bold" }); @@ -44,6 +28,8 @@ const ROTATE_SHIFT_DRAG_SPEED = 20; const DEG_TO_RAD = Math.PI / 180; export class CoordinateSystemNode extends Node { + private readonly disposeCoordinateSystemNode: () => void; + public constructor( videoLoadedProperty: TReadOnlyProperty, model: SimModel, @@ -143,34 +129,15 @@ export class CoordinateSystemNode extends Node { this.addChild(positionNode); // ── Property → scene-graph linkage ──────────────────────────────────── - model.coordOriginProperty.link((pos) => { + const onOriginChange = (pos: Vector2) => { positionNode.translation = pos; - }); - model.coordAngleProperty.link((angle) => { - rotatingNode.rotation = angle; - }); + }; + model.coordOriginProperty.link(onOriginChange); - // ── Clamp coord origin to video bounds on every change ──────────────── - // Prevents the user from dragging the coordinate system completely off-screen. - let isClamping = false; - model.coordOriginProperty.lazyLink((pos) => { - if (isClamping) return; - const clampedX = Math.max( - VIDEO_BOUNDS.minX, - Math.min(VIDEO_BOUNDS.maxX, pos.x), - ); - const clampedY = Math.max( - VIDEO_BOUNDS.minY, - Math.min(VIDEO_BOUNDS.maxY, pos.y), - ); - if (clampedX !== pos.x || clampedY !== pos.y) { - isClamping = true; - model.coordOriginProperty.value = model.coordOriginProperty.value - .copy() - .setXY(clampedX, clampedY); - isClamping = false; - } - }); + const onAngleChange = (angle: number) => { + rotatingNode.rotation = angle; + }; + model.coordAngleProperty.link(onAngleChange); // ── Drag: translate the entire coordinate system ────────────────────── positionNode.addInputListener( @@ -209,8 +176,20 @@ export class CoordinateSystemNode extends Node { ); // ── Visibility: only shown once a video with a finite duration is loaded - videoLoadedProperty.link((loaded) => { + const onVideoLoaded = (loaded: boolean) => { this.visible = loaded; - }); + }; + videoLoadedProperty.link(onVideoLoaded); + + this.disposeCoordinateSystemNode = () => { + model.coordOriginProperty.unlink(onOriginChange); + model.coordAngleProperty.unlink(onAngleChange); + videoLoadedProperty.unlink(onVideoLoaded); + }; + } + + public override dispose(): void { + this.disposeCoordinateSystemNode(); + super.dispose(); } } diff --git a/src/screen-name/view/PlaybackControlsNode.ts b/src/screen-name/view/PlaybackControlsNode.ts index 98e7faf..a5ec05c 100644 --- a/src/screen-name/view/PlaybackControlsNode.ts +++ b/src/screen-name/view/PlaybackControlsNode.ts @@ -27,6 +27,7 @@ const INFO_DISPLAY_SPACING = 4; // gap between time label and frame counter */ export class PlaybackControlsNode extends HBox { private isScrubbing = false; + private readonly disposePlaybackControlsNode: () => void; public constructor( model: SimModel, @@ -51,14 +52,16 @@ export class PlaybackControlsNode extends HBox { const timeSpeedProperty = new EnumerationProperty(TimeSpeed.NORMAL); // view → model - timeSpeedProperty.link((speed) => { + const onSpeedChange = (speed: TimeSpeed) => { model.playbackRateProperty.value = speedMap.get(speed) ?? SPEED_NORMAL; - }); + }; + timeSpeedProperty.link(onSpeedChange); // model → view (handles reset and any future programmatic rate changes) - model.playbackRateProperty.lazyLink((rate: number) => { + const onRateChange = (rate: number) => { timeSpeedProperty.value = rateToSpeed.get(rate) ?? TimeSpeed.NORMAL; - }); + }; + model.playbackRateProperty.lazyLink(onRateChange); // ── TimeControlNode: play/pause + step back + step forward + speed ───── const timeControlNode = new TimeControlNode(model.isPlayingProperty, { @@ -100,11 +103,12 @@ export class PlaybackControlsNode extends HBox { enabledProperty: model.videoLoadedProperty, }); - model.currentTimeProperty.lazyLink((time) => { + const onTimeChange = (time: number) => { if (this.isScrubbing) { videoElement.currentTime = time; } - }); + }; + model.currentTimeProperty.lazyLink(onTimeChange); // ── Time and frame info display ──────────────────────────────────────── const formatDuration = (seconds: number): string => { @@ -123,12 +127,15 @@ export class PlaybackControlsNode extends HBox { [ model.currentTimeProperty, model.durationProperty, - model.frameDurationProperty, + model.frameRateProperty, ], - (time: number, duration: number, frameDuration: number) => { + (time: number, duration: number, frameRate: number) => { if (duration <= 0) return "0/0"; - const current = Math.round(time / frameDuration); - const total = Math.round(duration / frameDuration); + // Multiply by frame rate directly rather than dividing by frameDuration + // (1/fps) to avoid cascading floating-point error at non-integer fps + // values like 29.97, matching the approach used in AutoTrackerNode. + const current = Math.round(time * frameRate); + const total = Math.round(duration * frameRate); return `${current}/${total}`; }, ); @@ -149,9 +156,24 @@ export class PlaybackControlsNode extends HBox { }); this.children = [infoDisplay, timeControlNode, scrubber]; + + this.disposePlaybackControlsNode = () => { + timeSpeedProperty.unlink(onSpeedChange); + model.playbackRateProperty.unlink(onRateChange); + model.currentTimeProperty.unlink(onTimeChange); + timeSpeedProperty.dispose(); + rangeProperty.dispose(); + totalTimeTextProperty.dispose(); + frameCountTextProperty.dispose(); // no longer observes frameDurationProperty + }; } public get scrubbing(): boolean { return this.isScrubbing; } + + public override dispose(): void { + this.disposePlaybackControlsNode(); + super.dispose(); + } } diff --git a/src/screen-name/view/WebcamPanel.ts b/src/screen-name/view/WebcamPanel.ts index 1e3cb22..2117ceb 100644 --- a/src/screen-name/view/WebcamPanel.ts +++ b/src/screen-name/view/WebcamPanel.ts @@ -499,4 +499,9 @@ export class WebcamPanel extends Node { this.timerInterval = null; } } + + public override dispose(): void { + this.cleanup(); + super.dispose(); + } } diff --git a/src/tracking/OpenCVTracker.ts b/src/tracking/OpenCVTracker.ts index 11204e2..bcf5d81 100644 --- a/src/tracking/OpenCVTracker.ts +++ b/src/tracking/OpenCVTracker.ts @@ -184,12 +184,12 @@ export class OpenCVTracker { // read outside the source image and crash. const clampedX = Math.round(Math.max(0, region.x)); const clampedY = Math.round(Math.max(0, region.y)); - const roi = new cv.Rect( - clampedX, - clampedY, - Math.round(Math.min(region.w, this.offscreen.width - clampedX)), - Math.round(Math.min(region.h, this.offscreen.height - clampedY)), - ); + const roiW = Math.round(Math.min(region.w, this.offscreen.width - clampedX)); + const roiH = Math.round(Math.min(region.h, this.offscreen.height - clampedY)); + if (roiW <= 0 || roiH <= 0) { + throw new Error(`Invalid ROI dimensions: ${roiW}x${roiH}`); + } + const roi = new cv.Rect(clampedX, clampedY, roiW, roiH); this.templateMat = gray.roi(roi).clone(); } finally { frame.delete(); @@ -211,8 +211,10 @@ export class OpenCVTracker { let imageData: ImageData; try { imageData = this.captureFrame(video); - } catch { - // Cross-origin video — silently skip this frame rather than crashing. + } catch (e) { + // Cross-origin video without CORS headers — skip this frame and warn + // so developers can diagnose the source of the failure. + console.warn("[OpenCVTracker] Frame capture failed — video may be cross-origin:", e); return null; } const frame = cv.matFromImageData(imageData);