fix: correct OpenCV ROI clamp and eliminate per-frame DOM rebuild - #9
Merged
Conversation
OpenCVTracker: the width/height bounds of the template ROI were computed against the unclamped region.x/y values. When a drag begins outside the video edge, region.x or region.y can be negative, making `offscreen.width - region.x` exceed the canvas width and causing an OpenCV native crash. Clamp x/y first, then derive w/h from the clamped origin. DataTableNode: tracksProperty fired on every addPointToTrack call (~30 Hz during auto-tracking), each time destroying and recreating the full HTML table. Replace with an incremental strategy: - Structural changes (track added/removed, unit/colour/locale change) still trigger a full rebuild. - Data-only changes (new points on existing tracks) append only the new <tr> elements and update cells in existing rows, leaving the rest of the DOM untouched. A frameRowMap (frame → <tr>) and maxRenderedFrame counter make both the duplication check and out-of-order detection O(1). https://claude.ai/code/session_015sLL1uRLTjrDsyMiPS13VJ
webcam.ts — fixWebmDuration: store the setTimeout ID and call clearTimeout() in every resolution branch (onloadedmetadata, onseeked, onerror). Previously the timeout fired after the Promise was already settled, double-revoking the blob URL and spuriously calling reject() on a resolved Promise. VideoPlayerNode.ts — track `currentBlobUrl` as a class field; revoke the previous blob URL before assigning a new one from a webcam recording, and revoke on dispose. Previously each recorded video silently leaked a blob URL for the lifetime of the browser tab. VideoPlayerNode.ts — stepForward() was an exact copy of seekByFrames(1). Collapse it to a one-liner delegate call to eliminate the duplication. DigitizingOverlayNode.ts — rebuildMarks() was called on every video frame (~30 Hz during playback) and created a fresh SceneryStack Circle node for every digitized point on every call. Replace with one reusable Path per track: rebuild only the Shape (O(n) in points) while the SceneryStack node itself is created once and updated in place. Remove the now-unused Circle import. AutoTrackerNode.ts — the per-frame duplicate-frame check scanned all existing track points with Array.some() (O(n)). Replace with a `recordedFrames: Set<number>` field for O(1) lookup. The set is cleared in reset() and also when the active track changes (via a new lazyLink on activeTrackIdProperty, unlinked in dispose()). SimModel.ts — addPointToTrack() now rejects duplicate frame numbers at the model level. Previously only AutoTrackerNode guarded against this, so manual digitizing (clicking the same frame twice) could corrupt the kinematics by introducing two points at the same frame index. https://claude.ai/code/session_015sLL1uRLTjrDsyMiPS13VJ
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.
OpenCVTracker: the width/height bounds of the template ROI were
computed against the unclamped region.x/y values. When a drag begins
outside the video edge, region.x or region.y can be negative, making
offscreen.width - region.xexceed the canvas width and causing anOpenCV native crash. Clamp x/y first, then derive w/h from the clamped
origin.
DataTableNode: tracksProperty fired on every addPointToTrack call
(~30 Hz during auto-tracking), each time destroying and recreating the
full HTML table. Replace with an incremental strategy:
still trigger a full rebuild.
A frameRowMap (frame → ) and maxRenderedFrame counter make
both the duplication check and out-of-order detection O(1).
https://claude.ai/code/session_015sLL1uRLTjrDsyMiPS13VJ