feat(studio): timeline collision and placement model#2195
Open
ukimsanov wants to merge 35 commits into
Open
Conversation
Collaborator
Author
This was referenced Jul 11, 2026
858368d to
6ecac1c
Compare
055fc12 to
eb1e603
Compare
miga-heygen
reviewed
Jul 11, 2026
miga-heygen
left a comment
Contributor
There was a problem hiding this comment.
Reviewed as part of the 25-PR NLE overhaul stack. Full stack review on #2205 (the keystone). No blockers on this PR. — Miga
vanceingalls
left a comment
Collaborator
There was a problem hiding this comment.
Verdict: LGTM (green) — pure math, complete coverage
Elementary, but pleasing in its restraint. The placement resolver has been reduced to a set of unit-consistent primitives — resolvePlacement, clampTrackToZone, isInsertAllowedForZone, resolveZoneDropPlacement — each modest, each testable, each with its coverage certificate in the sibling file.
The pointed queries
- Unit consistency (seconds throughout).
packages/studio/src/player/components/timelineCollision.ts:127—timeRangesOverlap(aStart, aEnd, bStart, bEnd)operates in seconds;end = start + durationis the sole arithmetic; no pixel term contaminates the calculation.resolveInsertRowat :105 is the only pixel-domain input (rowFloatin track-height units) and returns a boundary integer — cleanly typed at the boundary. - Half-open at the edge.
timeRangesOverlapuses strict<on both sides (:127). Clip A ending at t=5 and clip B starting at t=5 →5 < 5is false, so they are adjacent, not overlapping. Test file at :20 pins this explicitly (touching at 2→ false). TheresolveZoneDropPlacement — shares a track for sequential (non-overlapping) clipscase at :245 verifies the invariant end-to-end. - Empty inputs.
resolvePlacementwith adesiredTrackoutsidetrackOrder(:188) short-circuits to{ track: desiredTrack, needsInsert: false }— defensive and correct.resolveInsertRowwithtrackCount === 0(:110) returns 0 (insert-at-top). Both edges named in the focus are handled. - Zone identity preserved.
clampTrackToZone(:10) andisInsertAllowedForZone(:30) form the boundary; a visual clip clamped is guaranteed to remain in the visual zone (walks up to the last visual row), an audio clip clamped is guaranteed to remain in the audio zone. Cross-tested at :256–:290. - Self-exclusion during drag.
isLaneFree(excludeKey)at :139–:147 correctly filters the dragged clip out of its own collision computation — the "placeholder-scenario" test at :121–:133 nails it.
Cross-batch coherence
Half-open convention is consistent with #2198 (overlapsInTime uses < too, with an epsilon guard); adjacent-not-overlapping is the operative contract in both.
R1 by Via
…asks_safe_for_source_loaders fix(core): make timing masks safe for source loaders
…rce_tests_by_execution_lane test(producer): gate source tests by execution lane
The deselect restore read a ref recomputed from RENDER state — on a direct A→B selection switch, state re-syncs to B before A's effect cleanup runs, so after a committed crop gesture A was restored with B's crop string (or lost its crop when B had none). The committed value is now written at gesture-commit time (tri-state: none committed / crop removal / the exact committed string), so cleanup never touches render state. Adds component tests for lift/restore ordering, including the direct A→B switch and the uneditable-clip stand-down.
- merge gsapResizeIntercept's duplicate module imports - move the core-constant imports to the file headers (picker, domEditingDom) - justify the cross-realm HTMLElement casts (iframe-realm nodes fail instanceof; access is duck-typed)
… tail verify sample
Review follow-ups (both reviewers, all findings): - resize captures scope to the resize group: convert-to-keyframes resolvedFromValues and the whole-offset backfill pass the group filter, so an opacity-touching intro tween can't ride into a converted scale tween (the rotation fix's contract, now uniform across intercepts) - commitStaticSet resolves every group's target set BEFORE committing and coalesces groups landing on the same legacy mixed set into one commit — the second commit can no longer chase a stale group-derived id - installAuthoredOpacityCapture also stamps an element the moment it GAINS data-color-grading at runtime (attributeFilter), not just at insertion - both writer twins now share the same emitted-set dedupe shape - applySoftReload's positional tail becomes a SoftReloadOptions object - readAllAnimatedProperties builds the group-filtered key set immutably instead of deleting from the set mid-iteration - applyAuthoredInlineOpacity documents the priority-lossy round-trip - the marquee hit-test reads activeCompositionPathRef like its neighbors New tests: resize intercept (scale route + group filter + non-uniform longhands), after-write-HTML / stamp / empty-stamp opacity restore, the no-op-commit-with-missed-instant-patch soft-reload contract, and the runtime-gained-grading stamp.
Selecting a rotated (cropped) element appeared to straighten it: the crop dim and dashed window were drawn on the axis-aligned bounding box, so the bright window was a straight rectangle and the element's rotated corners were masked to near-black — while the DOM transform was untouched. clip-path applies in the element's LOCAL frame, before its transform, so the crop visualization now renders inside a container rotated with the element: readElementCropFrame decomposes the computed 2D matrix into angle + per-axis scale (element scale finally factored into the px mapping too) and 3D/unparseable transforms keep the axis-aligned presentation. Pointer deltas rotate into the element frame before the inset resolvers, so edge/pan drags track the rotated handles correctly.
…iting fix(studio): graded elements survive manual editing (disappear/resize/rotate/crop/panel)
3916b0f to
e0ad57b
Compare
… tail verify sample (#2247) ## What Two fixes from adversarial testing of the DE parallel router (10 hostile comps, routed vs screenshot-baseline PSNR). The router itself held — both bugs are in general drawElement fast capture, and one slipped past self-verify. ### 1. Compile gate: ancestor background-image (`producer`) `drawElementService`'s per-frame ancestor fill replicates what lies behind the captured subtree by walking up the DOM for the nearest non-transparent **`backgroundColor`**. A background-**image** (`linear-gradient`, `url()`) on `body`/`html`/a wrapper reads as transparent in that scan, so a deeper ancestor's solid color paints instead wherever the subtree leaves pixels uncovered. Measured repro: body `linear-gradient` + html solid color + an element shrinking late in the comp → DE paints the html purple instead of the body gradient. 30.9 dB min frame vs baseline, visually unmistakable. Identical damage single-worker and parallel — general DE bug, in every wild DE render matching this (very common) authoring pattern. Fix: `detectAncestorBackgroundImage()` in the compiler (DOM-aware — inline styles on the root's ancestor chain + `<style>` rules resolved via `querySelectorAll`, so class-selected wrappers are covered; backgrounds *inside* the root are deliberately not matched). New compile gate `ancestor_background_image`, same shape as the 3D/mix-blend gates, bypass `HF_FAST_CAPTURE_ANCESTOR_BG=true`. ### 2. Self-verify tail sample (`engine`) The verify grid sampled at `(i+1)/(k+1)` → [20/40/60/80]% of the timeline. The damage above starts at ~79% and peaks after the last sample — verification **passed** on output that bottomed at 30.9 dB (threshold 32 dB would have caught it, it just never looked there). Fix: `computeDeVerifySampleFractions()` — first k−1 samples evenly spaced, last pinned at 95%. Default k=4 grid becomes [25/50/75/95]%. Kills the whole late-onset damage class, not just this repro. ## Validation - Repro comp (body gradient + shrink reveal): now gates → baseline route, 54.7 dB avg vs ground truth (was 41.6 avg / 30.9 min with the purple surround) - Control comp (nested stacked fades, routed): still routes, verify grid `[90, 180, 270, 342] of 360`, passes, 60.6 dB avg — unchanged - Full adversarial matrix context: 6/10 comps routed clean (49–68 dB min), blend/3D gated correctly, animated-canvas damage caught by verify at 16.5 dB with clean revert, video comps route legitimately (frames pre-extracted) - Tests: 7 new detection cases (`htmlCompiler.test.ts`), 5 new grid cases (`frameCapture-verifySampleFractions.test.ts`); `compileStage.test.ts` + `frameCapture.test.ts` suites green 🤖 Generated with [Claude Code](https://claude.com/claude-code)
…l flow to error-string matching Addresses non-blocking review feedback on #2256: the previous shape used err.message.includes(...) inside a catch to distinguish a real headroom violation from a statfsSync failure — a future message tweak would silently fail open (statfs-unsupported and headroom-violation would take the same code path). Now statfsSync failure returns early (skip the gate) and the headroom check/throw happens in plain control flow outside any try/catch.
…de_parallel_router_below_a_machine_ram_floor fix(producer): withhold de parallel router below a machine ram floor
…welement_self-verify_with_parallel_worker_count fix(engine): densify drawelement self-verify with parallel worker count
…ek_font_subset_loads_before_audit_screenshots fix(cli): settle post-seek font subset loads before audit screenshots
…wser-no-gpu_for_gpu-unstable_hosts feat(cli): preview --browser-no-gpu for gpu-unstable hosts
…room-gate-before-hdr-pre-extraction fix(producer): disk-headroom gate before hdr raw frame pre-extraction
…prefs What: extends three leaf modules to their final NLE-stack form, tests in the same change: timelineInspector (isAudioTimelineElement, resolveBeatSourceTrack), timelineZoom (zoom/pps math incl. computePinnedZoomPercent), and studioUiPreferences (persisted editor prefs). Why: leaf dependencies of the NLE timeline stack; landing them first keeps the later glue PRs to wiring. How: additive from the consumer side — every export main already uses is unchanged (typecheck against main's consumers passes untouched); every new export is exercised by a test in this PR. Test plan: bunx vitest run on the three test files; tsc --noEmit in packages/studio; fallow audit --base origin/main clean.
What: useTimelineSyncCallbacks gains resolveReloadSeekTime and revealIframe — the pure contract for where the playhead lands after a preview reload (pending seek > deep-link seek > store playhead, clamped) and for undoing refreshPlayer's iframe hide. Test suite included. Why: the NLE editor reloads the preview on every committed edit; this contract is the difference between "playhead restores" and "jumps to 0". How: additive exports on an existing hook file; main's consumers unchanged. Test plan: bunx vitest run useTimelineSyncCallbacks.test.ts; tsc --noEmit in packages/studio; fallow audit clean.
What: the studio-server files route at its final NLE-stack form, with its test suite (25 tests). Why: standalone package seam — the server-side dependency of the studio asset workflow, reviewable in isolation. How: additive route behavior; existing route consumers unchanged. Test plan: bunx vitest run src/routes/files.test.ts in packages/studio-server; tsc --noEmit in packages/studio-server; fallow audit clean.
What: new pure module timelineCollision — zone-aware drop placement (clampTrackToZone, resolveZoneDropPlacement, resolveInsertRow, resolvePlacement, lane/overlap predicates) with its full test suite. Why: the no-overlap core of the NLE clip-drag engine; plain functions, no DOM, no React, no store writes. How: new files only; type-only imports from the existing playerStore. First runtime consumer arrives with the drag-engine PRs. Test plan: bunx vitest run timelineCollision.test.ts; tsc --noEmit; fallow audit clean (all exports test-consumed).
5299032 to
7694155
Compare
e0ad57b to
28f4251
Compare
7694155 to
db3b224
Compare
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
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.

What
new pure module timelineCollision — zone-aware drop placement (clampTrackToZone, resolveZoneDropPlacement, resolveInsertRow, resolvePlacement, lane/overlap predicates) with its full test suite.
Why
the no-overlap core of the NLE clip-drag engine; plain functions, no DOM, no React, no store writes.
How
new files only; type-only imports from the existing playerStore. First runtime consumer arrives with the drag-engine PRs.
Test plan
bunx vitest run timelineCollision.test.ts; tsc --noEmit; fallow audit clean (all exports test-consumed).
Stack position 4/25 — studio NLE overhaul (CapCut-parity timeline + canvas). Graphite manages bases; merge from #2192 upward. Temporary
TEMP(studio-dnd)fallow entries (unwired-component windows) are all removed by #2213 (app-shell swap), whose tree is byte-identical to the fully-verified integration branch.