From 4b5b0d75a357c30c8118c65695c3b70d149f3384 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 18 Feb 2026 13:43:18 +0000 Subject: [PATCH 1/2] Fix four high/critical architectural issues identified in review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1. View→Model violation (high): modelViewTransformProperty removed from SimModel. It is now a local Property in SimScreenView, computed by the existing Multilink, and threaded to VideoPlayerNode and AutoTrackerNode as a TReadOnlyProperty constructor parameter. The model no longer holds view-derived state. 2. markData[] stale on delete/reset (high): added `trackId` field to the MarkData type in VideoPlayerNode. rebuildMarks() now subscribes to model.tracksProperty and prunes entries whose track no longer exists, so dots are correctly cleared when a track is deleted or reset. 3. Auto-tracker data not persisted (high): AutoTrackerNode now receives the model and modelViewTransformProperty. On each tracking frame it converts the video-pixel position to model coordinates via localToGlobalPoint + inversePosition2 and calls model.addPointToTrack, guarded by a per-frame deduplication check so playback does not produce duplicate points. 4. Missing DataTableNode (critical): created DataTableNode.ts. The panel sits below TrackListPanel, shows the digitized position (x, y in the calibration unit) for each track at the current frame, and updates reactively on tracksProperty, currentTimeProperty, and unitProperty changes. Hides until a video is loaded. https://claude.ai/code/session_01BomKebzDTMC2wNohChtjpG --- src/screen-name/model/SimModel.ts | 8 +- src/screen-name/view/AutoTrackerNode.ts | 29 ++++- src/screen-name/view/DataTableNode.ts | 163 ++++++++++++++++++++++++ src/screen-name/view/SimScreenView.ts | 18 ++- src/screen-name/view/VideoPlayerNode.ts | 37 ++++-- 5 files changed, 230 insertions(+), 25 deletions(-) create mode 100644 src/screen-name/view/DataTableNode.ts diff --git a/src/screen-name/model/SimModel.ts b/src/screen-name/model/SimModel.ts index 3dd6c2e..82af944 100644 --- a/src/screen-name/model/SimModel.ts +++ b/src/screen-name/model/SimModel.ts @@ -1,5 +1,4 @@ import { BooleanProperty, Property } from "scenerystack/axon"; -import { Matrix3, Transform3 } from "scenerystack/dot"; import type { Track, TrackPoint } from "./Track.js"; // ── Track colour palette (one per letter, repeats after 8) ───────────────── @@ -28,12 +27,6 @@ export class SimModel { public readonly magnifyVideoProperty = new BooleanProperty( false ); public readonly autoTrackingProperty = new BooleanProperty( false ); - // Maps between real-world model coordinates and view (pixel) coordinates. - // Updated by SimScreenView whenever the coordinate system or calibration tool changes. - public readonly modelViewTransformProperty = new Property( - new Transform3( Matrix3.IDENTITY ) - ); - // ── Manual particle tracks ──────────────────────────────────────────── public readonly tracksProperty = new Property( [] ); @@ -92,6 +85,7 @@ export class SimModel { this.magnifyVideoProperty.reset(); this.autoTrackingProperty.reset(); this.tracksProperty.value = []; + this.activeTrackIdProperty.value = null; this.nextSymbolCode = 65; } diff --git a/src/screen-name/view/AutoTrackerNode.ts b/src/screen-name/view/AutoTrackerNode.ts index c1c93b5..9224c77 100644 --- a/src/screen-name/view/AutoTrackerNode.ts +++ b/src/screen-name/view/AutoTrackerNode.ts @@ -4,13 +4,16 @@ import { Shape } from "scenerystack/kite"; import { Tandem } from "scenerystack/tandem"; import { Vector2 } from "scenerystack/dot"; import type { TReadOnlyProperty } from "scenerystack/axon"; +import type { Transform3 } from "scenerystack/dot"; import { OpenCVTracker } from "../../tracking/OpenCVTracker.js"; import TrackLabColors from "../../TrackLabColors.js"; +import type { SimModel } from "../model/SimModel.js"; const VIDEO_W = 640; const VIDEO_H = 360; const MAX_TRAIL = 150; const CROSSHAIR_SIZE = 16; +const FRAME_DURATION = 1 / 30; // assumes 30 fps /** * Transparent SceneryStack overlay that sits directly on top of the video element. @@ -22,6 +25,8 @@ const CROSSHAIR_SIZE = 16; * 4. On each video frame (timeupdate / seeked), the best-match position is * computed via OpenCV template matching and shown as a red crosshair. * 5. Past positions are shown as a green trail of dots. + * 6. If a track is active (model.activeTrackIdProperty), each new unique frame + * position is recorded to the model via addPointToTrack. * * Local coordinates of this node correspond directly to video-pixel coordinates * (0,0 = top-left of video) because it is added to the same layer as the video @@ -43,7 +48,9 @@ export class AutoTrackerNode extends Node { public constructor( videoElement: HTMLVideoElement, - autoTrackingShownProperty: TReadOnlyProperty + autoTrackingShownProperty: TReadOnlyProperty, + model: SimModel, + modelViewTransformProperty: TReadOnlyProperty ) { super( { visible: false } ); @@ -170,7 +177,25 @@ export class AutoTrackerNode extends Node { this.trail.push( pt ); if ( this.trail.length > MAX_TRAIL ) this.trail.shift(); -this.updateTrackerVisuals( pt ); + this.updateTrackerVisuals( pt ); + + // ── Record position to model if a track is active ───────────────── + const activeId = model.activeTrackIdProperty.value; + if ( activeId ) { + const time = videoElement.currentTime; + const frame = Math.round( time / FRAME_DURATION ); + + // Avoid duplicate points for the same frame. + const activeTrack = model.tracksProperty.value.find( t => t.id === activeId ); + const alreadyRecorded = activeTrack ? activeTrack.points.some( p => p.frame === frame ) : false; + + if ( !alreadyRecorded ) { + // Convert video-pixel coords to global coords, then to model coords. + const globalPt = this.localToGlobalPoint( new Vector2( pt.x, pt.y ) ); + const modelPt = modelViewTransformProperty.value.inversePosition2( globalPt ); + model.addPointToTrack( activeId, frame, time, modelPt.x, modelPt.y ); + } + } }; videoElement.addEventListener( 'timeupdate', onFrame ); videoElement.addEventListener( 'seeked', onFrame ); diff --git a/src/screen-name/view/DataTableNode.ts b/src/screen-name/view/DataTableNode.ts new file mode 100644 index 0000000..6a856d0 --- /dev/null +++ b/src/screen-name/view/DataTableNode.ts @@ -0,0 +1,163 @@ +/** + * DataTableNode.ts + * + * Panel displayed below the TrackListPanel showing the digitized position + * of each track at (or just before) the current video frame. + * + * Columns: colour badge with symbol | x position | y position + * Values are shown in the unit selected in the CalibrationToolNode. + */ + +import type { TReadOnlyProperty } from "scenerystack/axon"; +import { Circle, HBox, Node, Rectangle, Text, VBox } from "scenerystack/scenery"; +import { PhetFont } from "scenerystack/scenery-phet"; +import { Panel } from "scenerystack/sun"; +import { Color } from "scenerystack"; +import type { SimModel } from "../model/SimModel.js"; +import type { Track, TrackPoint } from "../model/Track.js"; +import TrackLabColors from "../../TrackLabColors.js"; + +const FRAME_DURATION = 1 / 30; // assumes 30 fps +const PANEL_WIDTH = 165; +const BADGE_R = 8; + +const HEADER_FONT = new PhetFont( { size: 13, weight: "bold" } ); +const LABEL_FONT = new PhetFont( 11 ); +const VALUE_FONT = new PhetFont( { size: 11, weight: "bold" } ); + +/** + * Returns the last TrackPoint at or before `frame`, or null if none exists. + */ +function getPointAtFrame( track: Track, frame: number ): TrackPoint | null { + const candidates = track.points.filter( p => p.frame <= frame ); + if ( candidates.length === 0 ) return null; + return candidates.reduce( ( best, p ) => p.frame > best.frame ? p : best ); +} + +/** Builds one compact row for a track. */ +function buildTrackRow( track: Track, frame: number, unit: string ): Node { + const trackColor = new Color( track.color ); + + const badge = new Circle( BADGE_R, { + fill: track.color, + } ); + const symbolLabel = new Text( track.symbol, { + font: new PhetFont( { size: 10, weight: "bold" } ), + fill: "white", + } ); + symbolLabel.center = badge.center; + + const badgeNode = new Node( { children: [ badge, symbolLabel ] } ); + + const point = getPointAtFrame( track, frame ); + const xStr = point ? point.x.toFixed( 3 ) : "—"; + const yStr = point ? point.y.toFixed( 3 ) : "—"; + + const xLabel = new Text( `x: ${ xStr } ${ unit }`, { + font: LABEL_FONT, + fill: TrackLabColors.textOnDarkProperty, + maxWidth: PANEL_WIDTH - BADGE_R * 2 - 20, + } ); + const yLabel = new Text( `y: ${ yStr } ${ unit }`, { + font: LABEL_FONT, + fill: TrackLabColors.textOnDarkProperty, + maxWidth: PANEL_WIDTH - BADGE_R * 2 - 20, + } ); + + const valuesBox = new VBox( { + children: [ xLabel, yLabel ], + spacing: 2, + align: "left", + } ); + + const bg = new Rectangle( 0, 0, PANEL_WIDTH, 0, 4, 4, { + fill: trackColor.withAlpha( 0.12 ), + stroke: trackColor.withAlpha( 0.5 ), + lineWidth: 1, + pickable: false, + } ); + + const row = new HBox( { + children: [ badgeNode, valuesBox ], + spacing: 8, + align: "center", + } ); + + // Wrap in a node to add the background + const container = new Node( { children: [ bg, row ] } ); + row.left = 6; + row.centerY = 0; + bg.rectHeight = row.height + 8; + bg.top = row.top - 4; + + return container; +} + +export class DataTableNode extends Panel { + public constructor( + model: SimModel, + videoLoadedProperty: TReadOnlyProperty, + unitProperty: TReadOnlyProperty + ) { + const widthSpacer = new Rectangle( 0, 0, PANEL_WIDTH, 1, { + fill: null, + stroke: null, + pickable: false, + } ); + + const headerLabel = new Text( "Position Data", { + font: HEADER_FONT, + fill: TrackLabColors.textOnDarkProperty, + } ); + + const noDataLabel = new Text( "No digitized points", { + font: LABEL_FONT, + fill: TrackLabColors.textOnDarkProperty, + } ); + + const rowsVBox = new VBox( { + children: [ noDataLabel ], + spacing: 6, + align: "left", + } ); + + const content = new VBox( { + children: [ widthSpacer, headerLabel, rowsVBox ], + spacing: 8, + align: "center", + } ); + + super( content, { + fill: TrackLabColors.panelFillProperty, + stroke: TrackLabColors.panelStrokeProperty, + cornerRadius: 8, + xMargin: 10, + yMargin: 10, + visible: false, + } ); + + const rebuildRows = () => { + const tracks = model.tracksProperty.value; + const currentFrame = Math.round( model.currentTimeProperty.value / FRAME_DURATION ); + const unit = unitProperty.value; + + // Find tracks that have at least one point at or before the current frame. + const tracksWithData = tracks.filter( t => t.points.some( p => p.frame <= currentFrame ) ); + + if ( tracksWithData.length === 0 ) { + rowsVBox.children = [ noDataLabel ]; + } + else { + rowsVBox.children = tracksWithData.map( t => buildTrackRow( t, currentFrame, unit ) ); + } + }; + + model.tracksProperty.link( rebuildRows ); + model.currentTimeProperty.link( rebuildRows ); + unitProperty.link( rebuildRows ); + + videoLoadedProperty.link( loaded => { + this.visible = loaded; + } ); + } +} diff --git a/src/screen-name/view/SimScreenView.ts b/src/screen-name/view/SimScreenView.ts index 584d4d9..025eb0a 100644 --- a/src/screen-name/view/SimScreenView.ts +++ b/src/screen-name/view/SimScreenView.ts @@ -1,4 +1,4 @@ -import { DerivedProperty, Multilink } from "scenerystack/axon"; +import { DerivedProperty, Multilink, Property } from "scenerystack/axon"; import { Matrix3, Transform3, Vector2 } from "scenerystack/dot"; import { ResetAllButton } from "scenerystack/scenery-phet"; import { ScreenView, type ScreenViewOptions } from "scenerystack/sim"; @@ -50,10 +50,6 @@ export class SimScreenView extends ScreenView { public constructor( model: SimModel, options?: ScreenViewOptions ) { super( options ); - this.videoPlayerNode = new VideoPlayerNode( model, this ); - this.videoPlayerNode.center = this.layoutBounds.center.plusXY( 0, -20 ); - this.addChild( this.videoPlayerNode ); - // True once a video with a finite duration has been loaded. const videoLoadedProperty = new DerivedProperty( [ model.durationProperty ], d => d > 0 ); @@ -87,7 +83,10 @@ export class SimScreenView extends ScreenView { ); this.addChild( this.calibrationToolNode ); - // ── ModelViewTransform: recomputed whenever either tool changes ──────── + // ── ModelViewTransform: lives here in the view, recomputed whenever either tool changes. + // The model no longer owns this; it is threaded to child nodes that need it. ────────── + const modelViewTransformProperty = new Property( new Transform3( Matrix3.IDENTITY ) ); + Multilink.multilink( [ this.coordinateSystemNode.viewPositionProperty, @@ -97,12 +96,17 @@ export class SimScreenView extends ScreenView { this.calibrationToolNode.distanceProperty, ], ( origin, angle, p1, p2, dist ) => { - model.modelViewTransformProperty.value = buildModelViewTransform( + modelViewTransformProperty.value = buildModelViewTransform( origin, angle, p1, p2, dist ); } ); + // ── Video player (depends on tools being created first for the transform) ── + this.videoPlayerNode = new VideoPlayerNode( model, this, modelViewTransformProperty ); + this.videoPlayerNode.center = this.layoutBounds.center.plusXY( 0, -20 ); + this.addChild( this.videoPlayerNode ); + // ── Control panel (left side) ───────────────────────────────────────── const controlPanel = new ControlPanel( model ); controlPanel.left = this.layoutBounds.left + 10; diff --git a/src/screen-name/view/VideoPlayerNode.ts b/src/screen-name/view/VideoPlayerNode.ts index 346d5a7..7e1de44 100644 --- a/src/screen-name/view/VideoPlayerNode.ts +++ b/src/screen-name/view/VideoPlayerNode.ts @@ -1,5 +1,5 @@ -import { BooleanProperty, DerivedProperty, EnumerationProperty, Property } from "scenerystack/axon"; -import { Dimension2, Range } from "scenerystack/dot"; +import { BooleanProperty, DerivedProperty, EnumerationProperty, Property, type TReadOnlyProperty } from "scenerystack/axon"; +import { Dimension2, Range, type Transform3 } from "scenerystack/dot"; import { Circle, DOM, FireListener, HBox, Line, Node, Path, Rectangle, Text, VBox } from "scenerystack/scenery"; import { Shape } from "scenerystack/kite"; import { PhetFont, TimeControlNode, TimeSpeed } from "scenerystack/scenery-phet"; @@ -31,7 +31,11 @@ export class VideoPlayerNode extends Node { private readonly model: SimModel; private isScrubbing = false; - public constructor( model: SimModel, listParent: Node ) { + public constructor( + model: SimModel, + listParent: Node, + modelViewTransformProperty: TReadOnlyProperty + ) { super(); this.model = model; @@ -76,7 +80,12 @@ export class VideoPlayerNode extends Node { [ videoLoadedProperty, model.autoTrackingProperty ], ( loaded, tracking ) => loaded && tracking ); - const autoTrackerNode = new AutoTrackerNode( this.videoElement, autoTrackingShownProperty ); + const autoTrackerNode = new AutoTrackerNode( + this.videoElement, + autoTrackingShownProperty, + model, + modelViewTransformProperty + ); // ── Manual digitizing overlay ───────────────────────────────────────── // Sits on top of the video; active when a track checkbox is checked. @@ -198,12 +207,20 @@ export class VideoPlayerNode extends Node { }, } ); - // Coloured dots at click positions, filtered by current frame - type MarkData = { frame: number; localX: number; localY: number; color: string }; + // ── Mark data: pixel-space dots for digitized positions ─────────────── + // trackId is included so that marks can be filtered when a track is deleted. + type MarkData = { trackId: string; frame: number; localX: number; localY: number; color: string }; const markData: MarkData[] = []; const marksLayer = new Node( { pickable: false } ); const rebuildMarks = () => { + const activeTrackIds = new Set( model.tracksProperty.value.map( t => t.id ) ); + // Remove any marks whose track has been deleted or all tracks on reset. + for ( let i = markData.length - 1; i >= 0; i-- ) { + if ( !activeTrackIds.has( markData[ i ].trackId ) ) { + markData.splice( i, 1 ); + } + } const currentFrame = Math.round( model.currentTimeProperty.value / FRAME_DURATION ); marksLayer.children = markData .filter( m => m.frame <= currentFrame ) @@ -211,6 +228,8 @@ export class VideoPlayerNode extends Node { }; model.currentTimeProperty.link( () => rebuildMarks() ); + // Rebuild (and prune) whenever tracks are added, removed, or cleared. + model.tracksProperty.link( () => rebuildMarks() ); model.activeTrackIdProperty.link( activeId => { digitizingOverlay.visible = activeId !== null; @@ -232,10 +251,10 @@ export class VideoPlayerNode extends Node { const time = model.currentTimeProperty.value; const frame = Math.round( time / FRAME_DURATION ); - const mvt = model.modelViewTransformProperty.value; + const mvt = modelViewTransformProperty.value; const modelPt = mvt.inversePosition2( globalPt ); - markData.push( { frame, localX: localPt.x, localY: localPt.y, color: track.color } ); + markData.push( { trackId: activeId, frame, localX: localPt.x, localY: localPt.y, color: track.color } ); rebuildMarks(); model.addPointToTrack( activeId, frame, time, modelPt.x, modelPt.y ); @@ -451,7 +470,7 @@ export class VideoPlayerNode extends Node { /** Pause playback and advance by exactly one frame (1/30 s). */ public stepForward(): void { this.model.isPlayingProperty.value = false; - const raw = this.videoElement.currentTime + FRAME_DURATION; + const raw = this.videoElement.currentTime + ( 1 / 30 ); const clamped = Math.max( 0, Math.min( raw, this.videoElement.duration ) ); this.videoElement.currentTime = clamped; this.model.currentTimeProperty.value = clamped; From d6a036f58350f0c8bf06fab7bcb88120eaa83c96 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 18 Feb 2026 13:55:51 +0000 Subject: [PATCH 2/2] Implement Option A: model owns all tool state and computes the transform MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The model-view transform is now a true DerivedProperty computed entirely inside SimModel. No view code writes to the model to produce it. SimModel changes: - Add coordOriginProperty (Property) and coordAngleProperty (NumberProperty) for the coordinate system tool. - Add calibPoint1Property, calibPoint2Property (Property), calibDistanceProperty (NumberProperty with range), and calibUnitProperty (Property) for the calibration tool. - Export CalibrationUnit, CALIBRATION_UNITS, and CALIBRATION_DISTANCE_RANGE so view files can type the combo-box items without duplication. - Compute modelViewTransformProperty as a DerivedProperty from the five tool-state properties above. buildModelViewTransform() moves here from SimScreenView. - reset() resets all new properties back to their initial values; the initial positions are computed from layout constants (1024×618 default ScreenView bounds, 640×360 video, matching SimScreenView's positioning). CoordinateSystemNode changes: - Constructor drops initialPosition param; accepts model instead. - Links directly to model.coordOriginProperty and model.coordAngleProperty. - RichDragListeners read/write model properties directly. - No longer owns or exposes viewPositionProperty / rotationAngleProperty. - reset() removed; model.reset() handles state. CalibrationToolNode changes: - Constructor drops initialCenter param; accepts model instead. - All four tool properties replaced by model.calibPoint1/2/DistanceProperty and model.calibUnitProperty. - Imports CalibrationUnit and CALIBRATION_UNITS from SimModel. - reset() removed; model.reset() handles state. SimScreenView changes: - Removes local Property, Multilink, Matrix3, Transform3, Vector2 imports, and buildModelViewTransform (moved to SimModel). - Tool nodes constructed with model; no initial positions passed. - DataTableNode receives model.calibUnitProperty directly. - VideoPlayerNode constructed with (model, this) — no MVT param. - reset() override removed; model.reset() suffices. VideoPlayerNode + AutoTrackerNode: - Drop modelViewTransformProperty constructor parameter. - Both now read model.modelViewTransformProperty directly. https://claude.ai/code/session_01BomKebzDTMC2wNohChtjpG --- src/screen-name/model/SimModel.ts | 101 +++++++++++++++-- src/screen-name/view/AutoTrackerNode.ts | 6 +- src/screen-name/view/CalibrationToolNode.ts | 77 +++++-------- src/screen-name/view/CoordinateSystemNode.ts | 29 ++--- src/screen-name/view/SimScreenView.ts | 108 ++++--------------- src/screen-name/view/VideoPlayerNode.ts | 19 +--- 6 files changed, 155 insertions(+), 185 deletions(-) diff --git a/src/screen-name/model/SimModel.ts b/src/screen-name/model/SimModel.ts index 82af944..27e9027 100644 --- a/src/screen-name/model/SimModel.ts +++ b/src/screen-name/model/SimModel.ts @@ -1,4 +1,5 @@ -import { BooleanProperty, Property } from "scenerystack/axon"; +import { BooleanProperty, DerivedProperty, NumberProperty, Property, type TReadOnlyProperty } from "scenerystack/axon"; +import { Matrix3, Range, Transform3, Vector2 } from "scenerystack/dot"; import type { Track, TrackPoint } from "./Track.js"; // ── Track colour palette (one per letter, repeats after 8) ───────────────── @@ -13,24 +14,99 @@ const TRACK_COLORS = [ '#76FF03', // H – light green ]; +// ── Calibration unit type ────────────────────────────────────────────────── +export const CALIBRATION_UNITS = [ 'mm', 'cm', 'm', 'km', 'in', 'ft' ] as const; +export type CalibrationUnit = typeof CALIBRATION_UNITS[ number ]; +export const CALIBRATION_DISTANCE_RANGE = new Range( 0.001, 100000 ); + +// ── Layout constants ─────────────────────────────────────────────────────── +// SceneryStack's ScreenView.DEFAULT_LAYOUT_BOUNDS = Bounds2(0, 0, 1024, 618). +// The VideoPlayerNode is centered at layoutBounds.center + (0, -20). +const LAYOUT_CENTER_X = 512; // 1024 / 2 +const LAYOUT_CENTER_Y = 309; // 618 / 2 +const VIDEO_CENTER_X = LAYOUT_CENTER_X; // 512 +const VIDEO_CENTER_Y = LAYOUT_CENTER_Y - 20; // 289 +const VIDEO_WIDTH = 640; +const VIDEO_HEIGHT = 360; +const CALIB_HALF_LEN = 100; // pixels from center to each calibration endpoint + +// Initial tool positions (view / pixel space) +const COORD_ORIGIN_INITIAL = new Vector2( VIDEO_CENTER_X - VIDEO_WIDTH / 4, VIDEO_CENTER_Y ); +const CALIB_CENTER_INITIAL = new Vector2( VIDEO_CENTER_X, VIDEO_CENTER_Y + VIDEO_HEIGHT / 4 ); +const CALIB_P1_INITIAL = CALIB_CENTER_INITIAL.plusXY( -CALIB_HALF_LEN, 0 ); +const CALIB_P2_INITIAL = CALIB_CENTER_INITIAL.plusXY( CALIB_HALF_LEN, 0 ); + +// ── Model-view transform builder ─────────────────────────────────────────── +/** + * Builds a Transform3 from the coordinate-system tool and calibration tool. + * + * Composed as: T(origin) · R(θ) · S(s, −s) + * + * S(s, −s) — scale model units to pixels, flip Y (model +y points up) + * R(θ) — rotate by the coord-system angle (clockwise on screen) + * T(origin) — translate so model origin lands on the coord-system view position + * + * where s = |p2 − p1| / calibrationDistance (pixels per model unit). + * Returns the identity transform when the calibration segment has zero length. + */ +function buildModelViewTransform( + origin: Vector2, + angle: number, + p1: Vector2, + p2: Vector2, + dist: number +): Transform3 { + const pixelDist = p1.distance( p2 ); + if ( pixelDist < 1e-6 || dist < 1e-9 ) { + return new Transform3( Matrix3.IDENTITY ); + } + const s = pixelDist / dist; // pixels per model unit + + const matrix = Matrix3.translationFromVector( origin ) + .timesMatrix( Matrix3.rotation2( angle ) ) + .timesMatrix( Matrix3.scaling( s, -s ) ); + + return new Transform3( matrix ); +} + export class SimModel { - public readonly isPlayingProperty = new BooleanProperty( false ); - public readonly currentTimeProperty = new Property( 0 ); - public readonly durationProperty = new Property( 0 ); - public readonly videoUrlProperty = new Property( null ); + public readonly isPlayingProperty = new BooleanProperty( false ); + public readonly currentTimeProperty = new Property( 0 ); + public readonly durationProperty = new Property( 0 ); + public readonly videoUrlProperty = new Property( null ); // ── Overlay visibility ──────────────────────────────────────────────── - public readonly axesVisibleProperty = new BooleanProperty( true ); + public readonly axesVisibleProperty = new BooleanProperty( true ); public readonly calibrationVisibleProperty = new BooleanProperty( true ); // ── Future features (not yet implemented) ──────────────────────────── public readonly magnifyVideoProperty = new BooleanProperty( false ); public readonly autoTrackingProperty = new BooleanProperty( false ); - // ── Manual particle tracks ──────────────────────────────────────────── - public readonly tracksProperty = new Property( [] ); + // ── Coordinate system tool state (view / pixel space) ───────────────── + public readonly coordOriginProperty = new Property( COORD_ORIGIN_INITIAL.copy() ); + public readonly coordAngleProperty = new NumberProperty( 0 ); + + // ── Calibration tool state ──────────────────────────────────────────── + public readonly calibPoint1Property = new Property( CALIB_P1_INITIAL.copy() ); + public readonly calibPoint2Property = new Property( CALIB_P2_INITIAL.copy() ); + public readonly calibDistanceProperty = new NumberProperty( 1, { range: CALIBRATION_DISTANCE_RANGE } ); + public readonly calibUnitProperty = new Property( 'm' ); + + // ── Model-view transform (derived; the view never writes to this) ───── + public readonly modelViewTransformProperty: TReadOnlyProperty = new DerivedProperty( + [ + this.coordOriginProperty, + this.coordAngleProperty, + this.calibPoint1Property, + this.calibPoint2Property, + this.calibDistanceProperty, + ], + ( origin, angle, p1, p2, dist ) => buildModelViewTransform( origin, angle, p1, p2, dist ) + ); - // The track currently selected for manual digitizing (null = none). + // ── Manual particle tracks ──────────────────────────────────────────── + public readonly tracksProperty = new Property( [] ); public readonly activeTrackIdProperty = new Property( null ); private nextSymbolCode = 65; // ASCII code for 'A' @@ -84,8 +160,13 @@ export class SimModel { this.calibrationVisibleProperty.reset(); this.magnifyVideoProperty.reset(); this.autoTrackingProperty.reset(); + this.coordOriginProperty.reset(); + this.coordAngleProperty.reset(); + this.calibPoint1Property.reset(); + this.calibPoint2Property.reset(); + this.calibDistanceProperty.reset(); + this.calibUnitProperty.reset(); this.tracksProperty.value = []; - this.activeTrackIdProperty.value = null; this.nextSymbolCode = 65; } diff --git a/src/screen-name/view/AutoTrackerNode.ts b/src/screen-name/view/AutoTrackerNode.ts index 9224c77..e2d2fac 100644 --- a/src/screen-name/view/AutoTrackerNode.ts +++ b/src/screen-name/view/AutoTrackerNode.ts @@ -4,7 +4,6 @@ import { Shape } from "scenerystack/kite"; import { Tandem } from "scenerystack/tandem"; import { Vector2 } from "scenerystack/dot"; import type { TReadOnlyProperty } from "scenerystack/axon"; -import type { Transform3 } from "scenerystack/dot"; import { OpenCVTracker } from "../../tracking/OpenCVTracker.js"; import TrackLabColors from "../../TrackLabColors.js"; import type { SimModel } from "../model/SimModel.js"; @@ -49,8 +48,7 @@ export class AutoTrackerNode extends Node { public constructor( videoElement: HTMLVideoElement, autoTrackingShownProperty: TReadOnlyProperty, - model: SimModel, - modelViewTransformProperty: TReadOnlyProperty + model: SimModel ) { super( { visible: false } ); @@ -192,7 +190,7 @@ export class AutoTrackerNode extends Node { if ( !alreadyRecorded ) { // Convert video-pixel coords to global coords, then to model coords. const globalPt = this.localToGlobalPoint( new Vector2( pt.x, pt.y ) ); - const modelPt = modelViewTransformProperty.value.inversePosition2( globalPt ); + const modelPt = model.modelViewTransformProperty.value.inversePosition2( globalPt ); model.addPointToTrack( activeId, frame, time, modelPt.x, modelPt.y ); } } diff --git a/src/screen-name/view/CalibrationToolNode.ts b/src/screen-name/view/CalibrationToolNode.ts index 73dd5e0..f931230 100644 --- a/src/screen-name/view/CalibrationToolNode.ts +++ b/src/screen-name/view/CalibrationToolNode.ts @@ -1,39 +1,25 @@ import { Circle, HBox, Line, Node, RichDragListener, Text } from "scenerystack/scenery"; import { Keypad, PhetFont } from "scenerystack/scenery-phet"; import { KeypadDialog } from "scenerystack/sim"; -import { Range, type Vector2 } from "scenerystack/dot"; -import { DerivedProperty, NumberProperty, Property, type TReadOnlyProperty } from "scenerystack/axon"; +import { DerivedProperty } from "scenerystack/axon"; +import type { TReadOnlyProperty } from "scenerystack/axon"; import { ComboBox, type ComboBoxItem, Panel, TextPushButton } from "scenerystack/sun"; import { Tandem } from "scenerystack/tandem"; +import type { SimModel } from "../model/SimModel.js"; +import { CALIBRATION_UNITS } from "../model/SimModel.js"; import TrackLabColors from "../../TrackLabColors.js"; const FONT = new PhetFont( 14 ); const ENDPOINT_RADIUS = 8; -const INITIAL_HALF_LENGTH = 100; // pixels from center to each endpoint - -const UNITS = [ 'mm', 'cm', 'm', 'km', 'in', 'ft' ] as const; -type Unit = typeof UNITS[ number ]; - -const DISTANCE_RANGE = new Range( 0.001, 100000 ); export class CalibrationToolNode extends Node { - public readonly distanceProperty: NumberProperty; - public readonly unitProperty: Property; - public readonly point1Property: Property; - public readonly point2Property: Property; - public constructor( videoLoadedProperty: TReadOnlyProperty, listParent: Node, - initialCenter: Vector2 + model: SimModel ) { super(); - this.point1Property = new Property( initialCenter.plusXY( -INITIAL_HALF_LENGTH, 0 ) ); - this.point2Property = new Property( initialCenter.plusXY( INITIAL_HALF_LENGTH, 0 ) ); - this.distanceProperty = new NumberProperty( 1, { range: DISTANCE_RANGE } ); - this.unitProperty = new Property( 'm' ); - // ── Connecting line ──────────────────────────────────────────────────── const calibrationLine = new Line( 0, 0, 0, 0, { stroke: TrackLabColors.calibrationStrokeProperty, @@ -69,15 +55,15 @@ export class CalibrationToolNode extends Node { } ); // Pattern shown inside the dialog as "Range: {{min}} – {{max}} " const rangePatternProperty = new DerivedProperty( - [ this.unitProperty ], - ( unit: Unit ) => `{{min}} – {{max}} ${unit}` + [ model.calibUnitProperty ], + unit => `{{min}} – {{max}} ${ unit }` ); // ── Midpoint panel ──────────────────────────────────────────────────── // Button showing current value + unit; clicking it opens the keypad. const buttonLabelProperty = new DerivedProperty( - [ this.distanceProperty, this.unitProperty ], - ( dist: number, unit: Unit ) => `${dist.toFixed( 2 )} ${unit}` + [ model.calibDistanceProperty, model.calibUnitProperty ], + ( dist, unit ) => `${ dist.toFixed( 2 ) } ${ unit }` ); const distanceButton = new TextPushButton( buttonLabelProperty, { @@ -86,8 +72,8 @@ export class CalibrationToolNode extends Node { textFill: TrackLabColors.textOnDarkProperty, listener: () => { keypadDialog.beginEdit( - ( value: number ) => { this.distanceProperty.value = value; }, - DISTANCE_RANGE, + ( value: number ) => { model.calibDistanceProperty.value = value; }, + model.calibDistanceProperty.range, rangePatternProperty, () => {} ); @@ -96,12 +82,12 @@ export class CalibrationToolNode extends Node { } ); // Unit selector - const unitItems: ComboBoxItem[] = UNITS.map( unit => ( { + const unitItems: ComboBoxItem[] = CALIBRATION_UNITS.map( unit => ( { value: unit, createNode: () => new Text( unit, { font: FONT } ), tandemName: `${ unit }Item`, } ) ); - const unitComboBox = new ComboBox( this.unitProperty, unitItems, listParent, { + const unitComboBox = new ComboBox( model.calibUnitProperty, unitItems, listParent, { tandem: Tandem.OPT_OUT, } ); @@ -124,8 +110,8 @@ export class CalibrationToolNode extends Node { // ── Update geometry when endpoints move ─────────────────────────────── const updateGeometry = () => { - const p1 = this.point1Property.value; - const p2 = this.point2Property.value; + const p1 = model.calibPoint1Property.value; + const p2 = model.calibPoint2Property.value; calibrationLine.setLine( p1.x, p1.y, p2.x, p2.y ); endpoint1.translation = p1; endpoint2.translation = p2; @@ -133,31 +119,22 @@ export class CalibrationToolNode extends Node { midpointPanel.centerX = mid.x; midpointPanel.bottom = mid.y - 12; }; - this.point1Property.link( updateGeometry ); - this.point2Property.link( updateGeometry ); + model.calibPoint1Property.link( updateGeometry ); + model.calibPoint2Property.link( updateGeometry ); // ── Drag listeners for endpoints ────────────────────────────────────── - const makeDragListener = ( pointProperty: Property ) => { - return new RichDragListener( { - positionProperty: pointProperty, - keyboardDragListenerOptions: { - dragSpeed: 200, - shiftDragSpeed: 40, - }, - tandem: Tandem.OPT_OUT, - } ); - }; - endpoint1.addInputListener( makeDragListener( this.point1Property ) ); - endpoint2.addInputListener( makeDragListener( this.point2Property ) ); + endpoint1.addInputListener( new RichDragListener( { + positionProperty: model.calibPoint1Property, + keyboardDragListenerOptions: { dragSpeed: 200, shiftDragSpeed: 40 }, + tandem: Tandem.OPT_OUT, + } ) ); + endpoint2.addInputListener( new RichDragListener( { + positionProperty: model.calibPoint2Property, + keyboardDragListenerOptions: { dragSpeed: 200, shiftDragSpeed: 40 }, + tandem: Tandem.OPT_OUT, + } ) ); // ── Visibility ───────────────────────────────────────────────────────── videoLoadedProperty.link( loaded => { this.visible = loaded; } ); } - - public reset(): void { - this.point1Property.reset(); - this.point2Property.reset(); - this.distanceProperty.reset(); - this.unitProperty.reset(); - } } diff --git a/src/screen-name/view/CoordinateSystemNode.ts b/src/screen-name/view/CoordinateSystemNode.ts index 09a8363..828565c 100644 --- a/src/screen-name/view/CoordinateSystemNode.ts +++ b/src/screen-name/view/CoordinateSystemNode.ts @@ -1,8 +1,8 @@ import { Circle, Node, RichDragListener, Text } from "scenerystack/scenery"; import { ArrowNode, PhetFont } from "scenerystack/scenery-phet"; -import type { Vector2 } from "scenerystack/dot"; -import { NumberProperty, Property, type TReadOnlyProperty } from "scenerystack/axon"; import { Tandem } from "scenerystack/tandem"; +import type { TReadOnlyProperty } from "scenerystack/axon"; +import type { SimModel } from "../model/SimModel.js"; import TrackLabColors from "../../TrackLabColors.js"; const ARROW_LENGTH = 120; @@ -10,15 +10,9 @@ const HANDLE_FRACTION = 1 / 3; const FONT = new PhetFont( { size: 14, weight: 'bold' } ); export class CoordinateSystemNode extends Node { - public readonly viewPositionProperty: Property; - public readonly rotationAngleProperty: NumberProperty; - - public constructor( videoLoadedProperty: TReadOnlyProperty, initialPosition: Vector2 ) { + public constructor( videoLoadedProperty: TReadOnlyProperty, model: SimModel ) { super(); - this.viewPositionProperty = new Property( initialPosition.copy() ); - this.rotationAngleProperty = new NumberProperty( 0 ); - // ── Rotating node: axes + rotation handle ───────────────────────────── const rotatingNode = new Node(); @@ -75,7 +69,7 @@ export class CoordinateSystemNode extends Node { lineWidth: 1, } ); - // ── Position wrapper: translates with viewPositionProperty ─────────── + // ── Position wrapper: translates with model.coordOriginProperty ─────── const positionNode = new Node( { children: [ rotatingNode, originMarker ], cursor: 'move', @@ -86,12 +80,12 @@ export class CoordinateSystemNode extends Node { this.addChild( positionNode ); // ── Property → scene-graph linkage ──────────────────────────────────── - this.viewPositionProperty.link( pos => { positionNode.translation = pos; } ); - this.rotationAngleProperty.link( angle => { rotatingNode.rotation = angle; } ); + model.coordOriginProperty.link( pos => { positionNode.translation = pos; } ); + model.coordAngleProperty.link( angle => { rotatingNode.rotation = angle; } ); // ── Drag: translate the entire coordinate system ────────────────────── positionNode.addInputListener( new RichDragListener( { - positionProperty: this.viewPositionProperty, + positionProperty: model.coordOriginProperty, keyboardDragListenerOptions: { dragSpeed: 300, shiftDragSpeed: 50, @@ -106,7 +100,7 @@ export class CoordinateSystemNode extends Node { dragListenerOptions: { drag: ( event ) => { const p = positionNode.globalToLocalPoint( event.pointer.point ); - this.rotationAngleProperty.value = Math.atan2( p.y, p.x ); + model.coordAngleProperty.value = Math.atan2( p.y, p.x ); }, }, keyboardDragListenerOptions: { @@ -114,7 +108,7 @@ export class CoordinateSystemNode extends Node { dragSpeed: 100, shiftDragSpeed: 20, drag: ( _event, listener ) => { - this.rotationAngleProperty.value += listener.modelDelta.x * ( Math.PI / 180 ); + model.coordAngleProperty.value += listener.modelDelta.x * ( Math.PI / 180 ); }, }, tandem: Tandem.OPT_OUT, @@ -123,9 +117,4 @@ export class CoordinateSystemNode extends Node { // ── Visibility: only shown once a video with a finite duration is loaded videoLoadedProperty.link( loaded => { this.visible = loaded; } ); } - - public reset(): void { - this.viewPositionProperty.reset(); - this.rotationAngleProperty.reset(); - } } diff --git a/src/screen-name/view/SimScreenView.ts b/src/screen-name/view/SimScreenView.ts index 025eb0a..a0d8cfb 100644 --- a/src/screen-name/view/SimScreenView.ts +++ b/src/screen-name/view/SimScreenView.ts @@ -1,5 +1,4 @@ -import { DerivedProperty, Multilink, Property } from "scenerystack/axon"; -import { Matrix3, Transform3, Vector2 } from "scenerystack/dot"; +import { DerivedProperty } from "scenerystack/axon"; import { ResetAllButton } from "scenerystack/scenery-phet"; import { ScreenView, type ScreenViewOptions } from "scenerystack/sim"; import type { SimModel } from "../model/SimModel.js"; @@ -10,42 +9,8 @@ import { DataTableNode } from "./DataTableNode.js"; import { TrackListPanel } from "./TrackListPanel.js"; import { VideoPlayerNode } from "./VideoPlayerNode.js"; -/** - * Builds a Transform3 from the coordinate-system tool and calibration tool. - * - * Composed as: T(origin) · R(θ) · S(s, −s) - * - * S(s, −s) — scale model units to pixels, flip Y (model +y points up) - * R(θ) — rotate by the coord-system angle (clockwise on screen) - * T(origin) — translate so model origin lands on the coord-system view position - * - * where s = |p2 − p1| / calibrationDistance (pixels per model unit). - * Returns the identity transform when the calibration segment has zero length. - */ -function buildModelViewTransform( - origin: Vector2, - angle: number, - p1: Vector2, - p2: Vector2, - dist: number -): Transform3 { - const pixelDist = p1.distance( p2 ); - if ( pixelDist < 1e-6 || dist < 1e-9 ) { - return new Transform3( Matrix3.IDENTITY ); - } - const s = pixelDist / dist; // pixels per model unit - - const matrix = Matrix3.translationFromVector( origin ) - .timesMatrix( Matrix3.rotation2( angle ) ) - .timesMatrix( Matrix3.scaling( s, -s ) ); - - return new Transform3( matrix ); -} - export class SimScreenView extends ScreenView { private readonly videoPlayerNode: VideoPlayerNode; - private readonly coordinateSystemNode: CoordinateSystemNode; - private readonly calibrationToolNode: CalibrationToolNode; public constructor( model: SimModel, options?: ScreenViewOptions ) { super( options ); @@ -53,11 +18,6 @@ export class SimScreenView extends ScreenView { // True once a video with a finite duration has been loaded. const videoLoadedProperty = new DerivedProperty( [ model.durationProperty ], d => d > 0 ); - // The video element is 640×360. videoCenter approximates the video element center. - const VIDEO_WIDTH = 640; - const VIDEO_HEIGHT = 360; - const videoCenter = this.layoutBounds.center.plusXY( 0, -20 ); - // Combined visibility: video loaded AND user-toggled model flag. const axesShownProperty = new DerivedProperty( [ videoLoadedProperty, model.axesVisibleProperty ], @@ -68,82 +28,56 @@ export class SimScreenView extends ScreenView { ( loaded, visible ) => loaded && visible ); - // Coord system origin: center of the left half of the video (¼ from left edge, mid-height). - this.coordinateSystemNode = new CoordinateSystemNode( - axesShownProperty, - videoCenter.plusXY( -VIDEO_WIDTH / 4, 0 ) - ); - this.addChild( this.coordinateSystemNode ); - - // Calibration line: horizontally centered, ¼ above the video bottom (¾ from top). - this.calibrationToolNode = new CalibrationToolNode( - calibrationShownProperty, - this, - videoCenter.plusXY( 0, VIDEO_HEIGHT / 4 ) - ); - this.addChild( this.calibrationToolNode ); - - // ── ModelViewTransform: lives here in the view, recomputed whenever either tool changes. - // The model no longer owns this; it is threaded to child nodes that need it. ────────── - const modelViewTransformProperty = new Property( new Transform3( Matrix3.IDENTITY ) ); - - Multilink.multilink( - [ - this.coordinateSystemNode.viewPositionProperty, - this.coordinateSystemNode.rotationAngleProperty, - this.calibrationToolNode.point1Property, - this.calibrationToolNode.point2Property, - this.calibrationToolNode.distanceProperty, - ], - ( origin, angle, p1, p2, dist ) => { - modelViewTransformProperty.value = buildModelViewTransform( - origin, angle, p1, p2, dist - ); - } - ); - - // ── Video player (depends on tools being created first for the transform) ── - this.videoPlayerNode = new VideoPlayerNode( model, this, modelViewTransformProperty ); + // ── Coordinate system overlay ───────────────────────────────────────── + // Reads/writes model.coordOriginProperty and model.coordAngleProperty. + const coordinateSystemNode = new CoordinateSystemNode( axesShownProperty, model ); + this.addChild( coordinateSystemNode ); + + // ── Calibration tool overlay ────────────────────────────────────────── + // Reads/writes model.calibPoint1/2Property, model.calibDistanceProperty, + // and model.calibUnitProperty. + const calibrationToolNode = new CalibrationToolNode( calibrationShownProperty, this, model ); + this.addChild( calibrationToolNode ); + + // ── Video player ────────────────────────────────────────────────────── + // Uses model.modelViewTransformProperty (a DerivedProperty computed inside + // SimModel from the tool state properties above). + this.videoPlayerNode = new VideoPlayerNode( model, this ); this.videoPlayerNode.center = this.layoutBounds.center.plusXY( 0, -20 ); this.addChild( this.videoPlayerNode ); // ── Control panel (left side) ───────────────────────────────────────── const controlPanel = new ControlPanel( model ); - controlPanel.left = this.layoutBounds.left + 10; + controlPanel.left = this.layoutBounds.left + 10; controlPanel.centerY = this.layoutBounds.centerY; this.addChild( controlPanel ); // ── Track list panel (right of the video) ──────────────────────────── const trackListPanel = new TrackListPanel( model, videoLoadedProperty ); this.addChild( trackListPanel ); - // Position: just right of the video player, top-aligned with it. trackListPanel.left = this.videoPlayerNode.right + 12; trackListPanel.top = this.videoPlayerNode.top; // ── Data table (beneath the track list panel, same column) ─────────── - const dataTableNode = new DataTableNode( model, videoLoadedProperty, this.calibrationToolNode.unitProperty ); + const dataTableNode = new DataTableNode( model, videoLoadedProperty, model.calibUnitProperty ); this.addChild( dataTableNode ); dataTableNode.left = trackListPanel.left; trackListPanel.boundsProperty.link( () => { dataTableNode.top = trackListPanel.bottom + 8; } ); + // ── Reset all ───────────────────────────────────────────────────────── const resetAllButton = new ResetAllButton( { listener: () => { - model.reset(); + model.reset(); // resets all model state including tool positions this.reset(); }, - right: this.layoutBounds.maxX - 10, + right: this.layoutBounds.maxX - 10, bottom: this.layoutBounds.maxY - 10, } ); this.addChild( resetAllButton ); } - public reset(): void { - this.coordinateSystemNode.reset(); - this.calibrationToolNode.reset(); - } - public override step( dt: number ): void { super.step( dt ); this.videoPlayerNode.step(); diff --git a/src/screen-name/view/VideoPlayerNode.ts b/src/screen-name/view/VideoPlayerNode.ts index 7e1de44..5651dc5 100644 --- a/src/screen-name/view/VideoPlayerNode.ts +++ b/src/screen-name/view/VideoPlayerNode.ts @@ -1,5 +1,5 @@ -import { BooleanProperty, DerivedProperty, EnumerationProperty, Property, type TReadOnlyProperty } from "scenerystack/axon"; -import { Dimension2, Range, type Transform3 } from "scenerystack/dot"; +import { BooleanProperty, DerivedProperty, EnumerationProperty, Property } from "scenerystack/axon"; +import { Dimension2, Range } from "scenerystack/dot"; import { Circle, DOM, FireListener, HBox, Line, Node, Path, Rectangle, Text, VBox } from "scenerystack/scenery"; import { Shape } from "scenerystack/kite"; import { PhetFont, TimeControlNode, TimeSpeed } from "scenerystack/scenery-phet"; @@ -31,11 +31,7 @@ export class VideoPlayerNode extends Node { private readonly model: SimModel; private isScrubbing = false; - public constructor( - model: SimModel, - listParent: Node, - modelViewTransformProperty: TReadOnlyProperty - ) { + public constructor( model: SimModel, listParent: Node ) { super(); this.model = model; @@ -80,12 +76,7 @@ export class VideoPlayerNode extends Node { [ videoLoadedProperty, model.autoTrackingProperty ], ( loaded, tracking ) => loaded && tracking ); - const autoTrackerNode = new AutoTrackerNode( - this.videoElement, - autoTrackingShownProperty, - model, - modelViewTransformProperty - ); + const autoTrackerNode = new AutoTrackerNode( this.videoElement, autoTrackingShownProperty, model ); // ── Manual digitizing overlay ───────────────────────────────────────── // Sits on top of the video; active when a track checkbox is checked. @@ -251,7 +242,7 @@ export class VideoPlayerNode extends Node { const time = model.currentTimeProperty.value; const frame = Math.round( time / FRAME_DURATION ); - const mvt = modelViewTransformProperty.value; + const mvt = model.modelViewTransformProperty.value; const modelPt = mvt.inversePosition2( globalPt ); markData.push( { trackId: activeId, frame, localX: localPt.x, localY: localPt.y, color: track.color } );