Skip to content

feat(studio): timeline z-stacking sync model#2198

Open
ukimsanov wants to merge 1 commit into
studio-dnd/pr06-multidrag-previewfrom
studio-dnd/pr07-stacking-sync
Open

feat(studio): timeline z-stacking sync model#2198
ukimsanov wants to merge 1 commit into
studio-dnd/pr06-multidrag-previewfrom
studio-dnd/pr07-stacking-sync

Conversation

@ukimsanov

@ukimsanov ukimsanov commented Jul 11, 2026

Copy link
Copy Markdown
Collaborator

What

new pure module timelineStackingSync — lane order ↔ z-index reconciliation (laneIsAbove, computeStackingPatches) with tests.

Why

the single source of truth for how timeline lane order maps to canvas stacking; the ordering rules and tie-breaks live here.

How

new files only; consumed later by timelineZones and the stacking-sync hook.

Test plan

bunx vitest run timelineStackingSync.test.ts; tsc --noEmit; fallow audit clean.


Stack position 7/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.

ukimsanov commented Jul 11, 2026

Copy link
Copy Markdown
Collaborator Author

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

This stack of pull requests is managed by Graphite. Learn more about stacking.

@ukimsanov ukimsanov force-pushed the studio-dnd/pr07-stacking-sync branch from a3c734a to e90bfcc Compare July 11, 2026 00:55
@ukimsanov ukimsanov force-pushed the studio-dnd/pr06-multidrag-preview branch from 4622bb9 to d0a6ab1 Compare July 11, 2026 00:55
@ukimsanov ukimsanov force-pushed the studio-dnd/pr07-stacking-sync branch from e90bfcc to af7dd22 Compare July 11, 2026 01:15
@ukimsanov ukimsanov force-pushed the studio-dnd/pr06-multidrag-preview branch from d0a6ab1 to 206097c Compare July 11, 2026 01:15

@miga-heygen miga-heygen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed as part of the 25-PR NLE overhaul stack. Full stack review on #2205 (the keystone). No blockers on this PR. — Miga

@vanceingalls vanceingalls left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: LGTM (green) — permutation-preserving, cascade minimal

The z-stacking resolver's cleverness is in its restraint: try the single-clip patch first (authored z sacred), fall through to cascade only when ties or the z=0 floor make a single value inexpressible. And the cascade lifts only the neighbours that would otherwise no longer paint strictly above. Handsomely done.

The pointed queries

  • Output is a valid permutation of inputs. packages/studio/src/player/components/timelineStackingSync.ts:230–:258 — patches are emitted through a changed: Map<key, zIndex> deduplicated by emitted: Set. No key appears twice; no key from elements is dropped from consideration (the resolver iterates every edited clip and every overlapping neighbour). By construction the patch set is a subset-permutation of the input keys, one z per key. No drops, no duplicates. Tested throughout — see the multi-clip test at :115–:120 ({ a: 7, b: 6 } is single-emit per key) and the cascade at :166–:180.
  • Round-trip through normalizeToZones (cross-PR invariant with #2199). This is verified directly in #2199's test file at timelineZones.test.ts:332–:405lane-move → z patch → re-discovery orders lanes by that same z → identical lanes and qa-clean: drag video BELOW ralu → z patch → re-pack keeps it below, no oscillation. Both close the loop: computeStackingPatches(#2198) output projected onto TimelineElement.zIndex, fed back through normalizeToZones(#2199), yields a fixed point. That is the invariant this batch turns on and it is empirically nailed.
  • Half-open overlap. overlapsInTime at :57–:59 uses < with an EPS = 1e-6 guard on the right side (a.start < b.start + b.duration - EPS). Different from #2195's exact strict-< — the epsilon defends against floating-point fuzz creating spurious overlaps in the stacking resolver (a visible artifact) where #2195 doesn't need it because a collision decision is boolean and idempotent. Justified divergence. Test at :109–:113: touching-but-not-overlapping intervals do NOT counta (0..5) and b (5..5+5), no patch. Correct.
  • Authored z sacred (fast path). trySingleZ at :162–:176 attempts a strictly-between integer first; only when minAbove - maxBelow < 2 or when the only above-neighbour is at z=0 does it fall to the cascade. Test at :50–:54 confirms already-correct edits yield no patch; :91–:101 confirms only-below / only-above bounds.
  • Cascade minimality. liftAbove at :183–:196 sorts above-neighbours lowest-first and bumps each only if its current z ≤ the floor. Test at :166–:180 pins only the blockers move — a non-overlapping neighbour x is not touched.
  • DOM-order tie-breaking. paintsAbove at :78–:82 uses domIndex when zs are equal — the sync sees the actual paint reality and doesn't under-patch on equal-z ties. Test at :145–:153 uses domIndex on equal-z clips and verifies the cascade fires.
  • Audio exclusion. :123 filters out isAudio neighbours before the overlap set is computed. music neighbour test at :85–:89 pins the invariant.

Nits (non-blocking)

  • overlapsInTime uses - EPS while timeRangesOverlap in #2195 uses exact <. The divergence is intentional and defensive here, but a one-line comment noting the choice would help a future reader who reads both files back-to-back and wonders whether the two are meant to align.

R1 by Via

@ukimsanov ukimsanov force-pushed the studio-dnd/pr07-stacking-sync branch from af7dd22 to bf91abd Compare July 11, 2026 02:56
@ukimsanov ukimsanov force-pushed the studio-dnd/pr06-multidrag-preview branch from 1ed5c27 to f2fbc25 Compare July 11, 2026 08:18
@ukimsanov ukimsanov force-pushed the studio-dnd/pr07-stacking-sync branch from bf91abd to 0691107 Compare July 11, 2026 08:18
@ukimsanov ukimsanov marked this pull request as ready for review July 11, 2026 09:06
@ukimsanov ukimsanov force-pushed the studio-dnd/pr07-stacking-sync branch from 0691107 to 9e58745 Compare July 11, 2026 10:49
@ukimsanov ukimsanov force-pushed the studio-dnd/pr06-multidrag-preview branch from f2fbc25 to 6730438 Compare July 11, 2026 10:49
What: new pure module timelineStackingSync — lane order ↔ z-index
reconciliation (laneIsAbove, computeStackingPatches) with tests.

Why: the single source of truth for how timeline lane order maps to canvas
stacking; the ordering rules and tie-breaks live here.

How: new files only; consumed later by timelineZones and the stacking-sync
hook.

Test plan: bunx vitest run timelineStackingSync.test.ts; tsc --noEmit;
fallow audit clean.
@ukimsanov ukimsanov force-pushed the studio-dnd/pr06-multidrag-preview branch from 6730438 to 7f34c4e Compare July 11, 2026 12:51
@ukimsanov ukimsanov force-pushed the studio-dnd/pr07-stacking-sync branch from 9e58745 to 468cb87 Compare July 11, 2026 12:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants