Refactor: Extract overlay tools state into OverlayToolsModel - #59
Merged
Conversation
…tion, simplify track auto-creation Issues addressed from architecture review (#1.1, #1.2, #1.3): **1.2 — Tracker encapsulation (SimModel)** - Make `tracker` private; no view can access OpenCVTracker directly - Add facade methods: `isTrackerReady`, `resetTracker()`, `resizeTracker()`, `initTracker()`, `trackFrame()` — the full tracker API now lives behind the model boundary **1.1 — Atomic video source activation (SimModel + VideoSourceControlNode)** - Add `activateRecording()`, `activateUpload()`, `activateBundledVideo()` that set all related properties (isWebcamVideo, frameRate, totalFrameCount, currentWebcamBlob) in a single call, eliminating visible intermediate states - Consolidate all activation logic into the `selectedVideoProperty.lazyLink` handler — the single canonical dispatch point - Remove scattered direct property writes from the webp upload handler, `storeAndLoad`, and the webcam panel `onVideoReady` callback - Fix pre-existing bug: `onWebcamReady` was being called twice in the webcam recording and file upload flows (once from the lazyLink, once directly) - `VideoSelectedCallback` type simplified to `(url: string) => void` since fps is now owned by `activateBundledVideo` **1.3 — Track auto-creation (SimModel + AutoTrackerNode)** - Add `addTrackAndActivate()` to SimModel; replaces the 8-line view-side block that read `tracksProperty.value` directly after `addTrack()` to find and activate the newest track https://claude.ai/code/session_01LAwGKZUVqF9MNrUgaFtFyS
Issues addressed: - **2 + 3**: Extract OverlayToolsModel from SimModel — all geometric overlay state (axes, calibration, tape, angle, coord system, MVT, unit strings) now lives in a dedicated OverlayToolsModel owned by SimModel. Remove the identity distanceUnitProperty; callers use calibUnitProperty directly. - **1.4**: Remove label formatting from model layer — WebcamRecording/ UploadedVideo no longer carry a pre-formatted 'label' string. Types gain 'num: number' and 'name: string' fields; formatRecordingLabel/ formatUploadLabel helpers in VideoSourceControlNode format on the fly. - **1.5**: Extract TrackExporter.ts — DataRow type, buildDataRows(), and generateCsv() move from DataTableNode (view) to the model layer. - **4**: Add VideoPlayerNode.addVideoOverlay() and make videoContentLayer private; SimScreenView no longer reaches into the layer directly. - **5**: Move getAnimatedWebPInfo() from VideoSourceControlNode to webcam.ts; imported back via the established webcam module boundary. - **6**: Replace Track.color (CSS string) with Track.colorIndex (number); all consumers look up TRACK_COLORS[colorIndex] at render time. - **7**: Add optional targetTicks param to GraphDataManager.calculateTickSpacing; PlaybackControlsNode uses it (targetTicks=15) instead of a local duplicate. - **8**: Extract keyboard shortcut handler from VideoPlayerNode constructor into a private createKeyboardHandler() factory method. https://claude.ai/code/session_01LAwGKZUVqF9MNrUgaFtFyS
- Replace TRACK_COLORS[n]! assertions with getTrackColor() helper in TrackLabColors.ts - Remove unused re-export of calibration constants from SimModel.ts (noBarrelFile) - Fix unused import block (CALIBRATION_DISTANCE_RANGE etc.) in SimModel.ts - Use tracks[length-1] instead of .at(-1) for TypeScript target compatibility - Add await in trackFrame() to satisfy biome useAwait rule - Fix GraphDataManager named-vs-default import in PlaybackControlsNode.ts - Apply formatter corrections (4 files touched by biome format) Result: tsc --noEmit passes, biome lint 0 errors 0 warnings, biome format no changes. https://claude.ai/code/session_01LAwGKZUVqF9MNrUgaFtFyS
Biome merged the two separate TrackLabColors imports and reordered the TrackExporter imports to match the project's canonical style. https://claude.ai/code/session_01LAwGKZUVqF9MNrUgaFtFyS
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This refactor extracts all measurement and coordinate-system overlay tool state from
SimModelinto a new dedicatedOverlayToolsModelclass. This separation of concerns keeps video playback and track management logic distinct from geometric tool state, improving code organization and maintainability.Key Changes
New Model Layer
Created
OverlayToolsModel: A new class that owns all reactive state for overlay tools:clampCoordOrigin()for constraining origin to video boundsRe-exported calibration constants from
OverlayToolsModelinSimModelto maintain backward compatibility for existing importersSimModel Refactoring
OverlayToolsModelpublic readonly overlayTools = new OverlayToolsModel()as the single source of truthmodelViewTransformPropertyreferences to useoverlayTools.modelViewTransformPropertytrackerprivate (was public) and addedresizeTracker()andresetTracker()public methods for controlled accessactivateRecording()andactivateUpload()methods to atomically set video propertiesaddTrackAndActivate()convenience method for auto-tracking workflowprevModelViewTransformcache to constructor scope for cleaner organizationView Layer Updates
Updated all view nodes to access overlay properties via
model.overlayTools.*:SimScreenView,VideoPlayerNode,AutoTrackerNodeCalibrationToolNode,CoordinateSystemNodeMeasuringTapeNode,AngleToolNodeControlPanel,MeasurementToolsPanelUpdated graph/kinematics properties to use
model.overlayTools.calibUnitPropertyData Model Improvements
Track.colorfrom CSS string tocolorIndex(integer), with colors resolved at display time fromTRACK_COLORSarraylabelstring tonum(number) with formatting helpers inVideoSourceControlNodeUploadedVideo.namefield to store original filename separately from display labelNew Utility Module
TrackExporter: Pure functions for CSV export logic extracted fromDataTableNode:buildDataRows(): Collects unique frames across all tracksgenerateCsv(): Generates CSV text with proper formattingUtility Function Relocation
getAnimatedWebPInfo()fromVideoSourceControlNodetowebcam.tsfor reusabilityNotable Implementation Details
OverlayToolsModelconstructor initializes all tool positions in video-local coordinates, ensuring consistent coordinate space across all overlaysSimModelbut now usesoverlayTools.modelViewTransformPropertyfor MVT changesoverlayToolsobject, creating a clear dependency graphhttps://claude.ai/code/session_01LAwGKZUVqF9MNrUgaFtFyS