From 9b18fa7a26bd5e7b4f8b4b4e10c984585767620b Mon Sep 17 00:00:00 2001 From: Vance Ingalls Date: Thu, 13 Aug 2026 02:47:15 -0700 Subject: [PATCH] feat(studio): give the clips on a track one automation lane row (#3214) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(ci): allowlist the build-script consolidation in the no-main-deletions guard build-audio-fx-runtime.ts and build-position-edits-render.ts were merged into build-inline-artifact.ts to kill a fallow duplication finding; the deletion guard flagged that as an accidental loss since main still has both originals. * fix(core): dedupe the wet/dry mix math between delayFeedback and chorusLfo Both effect builders set wet.gain to the mix and dry.gain to its complement in identical two-line blocks; fallow kept re-flagging it as a 10-line clone on every unrelated change. Extracted setWetDryMix. * fix(core): remove the build-audio-fx-runtime.ts stray resurrected by a main merge An earlier merge with main brought this deleted file back (git's merge/delete handling on an unchanged-on-one-side file); package.json already points at build-inline-artifact.ts, so it sat unreachable and duplicating that file's config, both of which fallow flagged. * fix(studio): pull TimelineLanes under the 600-line cap TimelineLanes.tsx hit 620 lines. Extracted the three per-clip pointer gestures (resize-start, pointer-down move-arm, click/razor-split) into createClipGestureHandlers — one factory call per rendered clip instead of ~120 lines of inline handler bodies in the render loop. 529 lines now. * fix(studio): split the extracted pointerdown handler under the CRAP threshold Moving the ~120-line gesture logic into timelineClipGestureHandlers.ts concentrated it into two functions fallow flagged (onPointerDown at CRAP 63.6, onResizeStart at 31.6). Split the decision logic (which gesture a pointerdown implies) into a pure resolvePointerDownAction, then split its own intent-blocking check into isIntentBlocked. onResizeStart's guard moved into canStartResize. Every function now scores under 30. * fix(studio): drop the unused DomEditSelection import in PropertyPanelFlat CI caught it on PR #3026 (wa-12-panel-params); a later refactor in the stack removed the last use of the type here without removing the import. * fix(studio): close the typecheck and fallow gaps wa-18b-reschedule opened useAutomationLanes.ts's write() assumed gesture-scoped coalescing and a preview-only commit that useDomEditAttributeCommits.ts never grew — backported that option support from its own later commit so the two sides of the API agree. The paste path and its tests were missing the box selection's v0/v1 bounds a sibling commit added to AutomationSelection. The FX panel's carve controls still edited the six mechanism numbers (maxCutDb, bands, intelligibilityBias) after carveProfile() collapsed authoring to one Strength knob, so those fields no longer existed on HfCarveSettings; UI now edits strength, and analyseCarveBands is called with carveProfile(strength). Also closes fallow's complexity, dead-code and duplication findings on this PR's diff: extracted automationLaneDragMath.ts (pure group/point-move math) and useAutomationRangeDrag.ts (the marquee-select gesture) out of useAutomationLaneGestures.ts, pulled a couple of render-loop ternaries and a resolver into named functions, dropped an export nothing outside its file used, and shared a step-simplifier between audioCarve's two envelope builders. The edge-stretch vs. box-select priority test in TimelineAutomationLane.test was still pinning the pre-box-select rule (edge wins over a point sitting on it) that a sibling commit deliberately reversed — a point inside the box is now selected content, so grabbing it drags the group instead. Updated the test to the shipped rule instead of the old one. Co-Authored-By: Claude Opus 5 (1M context) * fix(core): cap the via conic's weight so an edge-clamped via point can't NaN A via point pulled out past the segment (viaX: 5, viaY: -3) clamps to (0.999, 0.001) — exactly on the steady region's edge, where edge - viaX is 0. viaConic divided by that zero to get an infinite weight, and shapeVia turned Infinity into NaN a few steps later (Infinity - Infinity in the quadratic coefficient). NaN reaching setValueCurveAtTime silences the automated parameter for the rest of the render. Capped the weight at 1e6 instead of leaving it unbounded — past that point the arc already reads as touching the via point, so nothing visible is lost. Also hardened shapeVia's existing denominator guard (`<= 0`) to `!(> 0)`, since NaN fails the original comparison and fell through it. Review by Miga (PR #3208). * fix(studio-server): fingerprint the proactive waveform cache key too The route already keys the waveform cache on the asset's size and mtime as well as its path, so a rebuilt-in-place file gets fresh peaks instead of stale ones. generateWaveformCache — the proactive path that runs on upload — still called buildWaveformCacheKey with the path alone, so it wrote to a different key than the route reads from (making the pre-generated cache never found) and kept the exact collision bug this fingerprint exists to fix on its own path. Review by Miga (PR #3211). * style(docs): run oxfmt on the /hyperframes-audio skill docs Table column widths had drifted out of alignment with oxfmt's own rules, failing format:check and blocking the Preflight gate every downstream branch inherits. Whitespace only, no content change. * fix(core): stop \b from missing underscore-separated names, guard clipsOverlap's negative duration \b treats `_` as a word character, so \bbed\b never matched bed_01, music_bed_loop, or theme_song, and \bvo\b/\bvox\b/\btts\b had the same gap — an underscore-separated bed classified as "unknown" and could end up offered as its own carve source. Replaced the short hints with a boundary that actually excludes letters and digits on both sides. clipsOverlap computed end = start + duration without guarding sign, so a negative duration put end before start — an interval that does not describe anything, and one specific case showed it silently dropping a real overlap (a shorter, earlier broken end rejected a clip that genuinely contained the point). Duration clamps to zero instead: a clip cannot un-play time, and a zero-length clip at its start is the sane reading of "duration nobody wrote down as positive." Review by Miga (PR #3212). * fix(studio): widen PropertyPanel's resetModules render timeout again The 20s margin (already once widened for the same reason) is timing out in CI's full-monorepo Test run — the resetModules()+fresh-import render this test needs is uncached and competes with every other package's test suite for the same worker pool, and the same test passes in well under 2s standalone. Went to 45s rather than re-tuning to whatever number happens to clear the current CI load, since that number moves every time CI gains a package. * fix(studio): stop the single-candidate auto-apply carve firing twice Two auto-apply effects both fire when sourceOptions.length === 1: the multi-candidate effect only guards length === 0, so a single candidate passes it too, and the single-candidate effect passes its own guard right after — both compute the same sources list and both call setCarve, so the common case (one narrator, one bed) triggered two decodes, two FFT runs, and two concurrent attribute writes for one decision. The multi-candidate effect now defers to its sibling for exactly one candidate, which already has its own detailed handling for that case. Review by Miga (PR #3213). * feat(core): carve against every voice over a bed, always (#3212) * feat(core): carve against every voice over a bed, always dynamically A bed usually runs under a whole sequence — a narrator, an interview answer, a second presenter — and carving against one of them left the others fighting it. `source` becomes `sources`, and `mixCarveSources` sums every voice onto the BED's clock before anything is measured. That is what keeps one analysis sufficient: the chain is fixed, so there is no per-voice filter to switch between, and bands drawn from all the speech there is with envelopes that rise wherever any of it happens answer the actual question — where and when is speech masking this bed. Summed rather than averaged: two people talking at once mask more than either alone. Audio before the bed starts is dropped rather than folded in at zero, since it plays over nothing and shifting it would put a cut where there is no voice. `dynamic` is gone. A fixed depth thins the bed through every pause, and once both have been heard there is no reason to want it, so every carve follows the speech. Two helpers the panel and the headless script now share instead of each carrying a copy — two definitions of "what does this name suggest" drift, and then the two disagree about which track is the voice: - `classifyAudioName` reads a track's kind from its id and filename together. `unknown` is deliberately common: treating an unrecognised name as "not a voice" would hide the one track somebody needs to pick. - `clipsOverlap` keeps out a voice that never plays while the bed does. An unwritten duration counts as unbounded, not zero — refusing a clip whose length the composition leaves to the media would drop the commonest case there is. Files written before this still load: a single `source` reads as a one-voice list, a stored `dynamic` is ignored, and an absent attribute means the defaults whole. Co-Authored-By: Claude Opus 5 (1M context) * fix(core): stop \b from missing underscore-separated names, guard clipsOverlap's negative duration \b treats `_` as a word character, so \bbed\b never matched bed_01, music_bed_loop, or theme_song, and \bvo\b/\bvox\b/\btts\b had the same gap — an underscore-separated bed classified as "unknown" and could end up offered as its own carve source. Replaced the short hints with a boundary that actually excludes letters and digits on both sides. clipsOverlap computed end = start + duration without guarding sign, so a negative duration put end before start — an interval that does not describe anything, and one specific case showed it silently dropping a real overlap (a shorter, earlier broken end rejected a clip that genuinely contained the point). Duration clamps to zero instead: a clip cannot un-play time, and a zero-length clip at its start is the sane reading of "duration nobody wrote down as positive." Review by Miga (PR #3212). --------- Co-authored-by: Claude Opus 5 (1M context) * fix(studio): port the carve UI off the removed source/dynamic fields #3212 (accidentally squash-merged into this branch instead of main) changed HfCarveSettings from a single `source` + `dynamic` toggle to a `sources` list with dynamic mode removed outright — the multi-voice UI consumer that goes with that shape lands in the very next PR, so this branch was left with a type that no longer matched its own code. Minimal port, not the multi-voice redesign that PR does properly: the "Listen to" picker and analyse() treat sources[0] as the one voice this UI still understands, and every dynamic-mode branch (the automated envelope lanes, the toggle, the checkbox) is gone along with the field — a carve is now always the static value the analysis computes, matching what the type change made permanent. Test suite trimmed the same way: the automation-lane and toggle tests covered behavior that no longer exists. --------- Co-authored-by: Claude Opus 5 (1M context) --- .../player/components/LayerDisclosureRow.tsx | 9 +- .../src/player/components/Timeline.test.ts | 37 ++++++ .../components/TimelineAutomationLane.tsx | 122 ++++++++++++++---- .../TimelineAutomationLaneSlot.test.tsx | 108 +++++++++++++++- .../src/player/components/TimelineLanes.tsx | 50 +++++-- .../components/TimelineTrackHeader.test.tsx | 101 ++++++++++++++- .../player/components/TimelineTrackHeader.tsx | 86 ++++++------ .../components/automationLaneData.test.ts | 109 ++++++++++++++++ .../player/components/automationLaneData.ts | 82 ++++++++++++ .../components/useTimelineTrackLayout.test.ts | 56 ++++++++ .../components/useTimelineTrackLayout.ts | 42 +++++- plans/automation-lanes-shared-rows.md | 58 +++++++++ 12 files changed, 769 insertions(+), 91 deletions(-) create mode 100644 plans/automation-lanes-shared-rows.md diff --git a/packages/studio/src/player/components/LayerDisclosureRow.tsx b/packages/studio/src/player/components/LayerDisclosureRow.tsx index ba17174a1..4dd05436a 100644 --- a/packages/studio/src/player/components/LayerDisclosureRow.tsx +++ b/packages/studio/src/player/components/LayerDisclosureRow.tsx @@ -1,12 +1,11 @@ import { CaretRight } from "@phosphor-icons/react"; -import type { TimelineElement } from "../store/playerStore"; import { TRACK_H } from "./timelineLayout"; import { TrackClipCount } from "./TrackClipCount"; // Layer row (Figma order: disclosure ▸/▾, diamond, name) — the disclosure lives // here, not on the clip bar, and re-expands a collapsed layer. export function LayerDisclosureRow({ - keyframeClip, + name, clipCount, isExpanded, gutterBackground, @@ -15,7 +14,10 @@ export function LayerDisclosureRow({ onToggleClipExpanded, children, }: { - keyframeClip: TimelineElement; + /** What this row is called. The active clip's own name when it is alone on the + * track; the track itself once it holds several, since naming a shared row + * after one of its clips reads as if the rows under it were that clip's. */ + name: string; clipCount: number; isExpanded: boolean; gutterBackground: string; @@ -31,7 +33,6 @@ export function LayerDisclosureRow({ /** Trailing controls that act on the LAYER (the visibility eye), not on a lane. */ children?: React.ReactNode; }) { - const name = keyframeClip.label ?? keyframeClip.domId ?? keyframeClip.id; return (
{ act(() => root.unmount()); }); + // The caret belongs to the row, not to whichever clip on it is selected: the + // automation lanes below it are the track's, shared per property. Toggling one + // clip left the row's state depending on the selection, and a collapse that + // only dropped the active clip left the row stuck open. + it("expands and collapses every clip on a shared track together", () => { + const host = createSizedTimelineHost(720); + const automation = JSON.stringify({ + version: 1, + lanes: [{ target: "volume", points: [{ t: 0, v: 1 }] }], + }); + usePlayerStore.setState({ + duration: 8, + timelineReady: true, + elements: [ + { id: "narration-1", tag: "audio", start: 0, duration: 4, track: 0, automation }, + { id: "narration-2", tag: "audio", start: 4, duration: 4, track: 0, automation }, + ], + }); + const root = createRoot(host); + act(() => root.render(React.createElement(Timeline))); + + const row = host.querySelector('[data-el-id="narration-1"]')?.parentElement + ?.parentElement; + // A row of several clips is named for the track, so the caret is too. + const caret = () => host.querySelector('button[aria-label$=" keyframes"]'); + expect(caret()?.getAttribute("aria-label")).toBe("Expand Track 1 keyframes"); + + act(() => caret()?.click()); + // One shared volume row, and BOTH clips hold it open. + expectTrackExpansion(row, ["narration-1", "narration-2"], TRACK_H + AUTOMATION_LANE_H); + + act(() => caret()?.click()); + expectTrackExpansion(row, [], TRACK_H); + act(() => root.unmount()); + }); + it("marks every clip in selectedElementIds as selected", () => { const host = createSizedTimelineHost(720); diff --git a/packages/studio/src/player/components/TimelineAutomationLane.tsx b/packages/studio/src/player/components/TimelineAutomationLane.tsx index 8aa2de83e..0340756dc 100644 --- a/packages/studio/src/player/components/TimelineAutomationLane.tsx +++ b/packages/studio/src/player/components/TimelineAutomationLane.tsx @@ -44,6 +44,9 @@ import { simplifyPoints } from "./automationSimplify"; import { pointInSelection, pointsIn, replaceRange } from "./automationLaneSelection"; import { getTimelineLaneTop } from "./timelineLayout"; import { defaultTimelineTheme } from "./timelineTheme"; +import { groupAutomationLanes } from "./automationLaneData"; +import { isAudioTimelineElement } from "../../utils/timelineInspector"; +import { getTimelineElementIdentity } from "../lib/timelineElementHelpers"; import type { TimelineElement } from "../store/playerStore"; import type { UseAutomationLanesResult } from "./useAutomationLanes"; @@ -497,35 +500,42 @@ export function TimelineAutomationLane({ ); } -export interface TimelineAutomationLaneSlotProps { - element: TimelineElement; - isSelected: boolean; - lanes: UseAutomationLanesResult; - pps: number; - /** Keyframe lanes already stacked above, which automation sits under. */ - laneCount: number; - accentColor: string; - /** Composition-time playhead; the slot converts it to clip-local. */ - currentTime: number; - /** Composition-time beat grid; the slot converts it to clip-local too. */ - beatTimes?: readonly number[]; +/** Which shared rows one clip draws into, and with which of its lanes. */ +interface ClipLaneRow { + lane: HfAutomationLane; + rowIndex: number; } /** - * Every automated parameter on this clip, one lane per row — the way a DAW - * stacks them, so two envelopes can be read and edited without swapping a - * control to see either. + * One clip's envelopes, each in the shared row its property owns. + * + * Its own component because every clip on the row needs its own binding, its own + * gestures and its own selection box — a shared row is a shared lane track, not a + * shared envelope, and two clips' curves must never drag as one thing. Hooks + * cannot run in a loop, so the loop is over components. */ -export function TimelineAutomationLaneSlot({ +function ClipAutomationLanes({ element, + rows, isSelected, lanes, pps, - laneCount, + top, accentColor, currentTime, beatTimes, -}: TimelineAutomationLaneSlotProps) { +}: { + element: TimelineElement; + rows: readonly ClipLaneRow[]; + isSelected: boolean; + lanes: UseAutomationLanesResult; + pps: number; + /** y of the first automation row on this track. */ + top: number; + accentColor: string; + currentTime: number; + beatTimes?: readonly number[]; +}) { // Beats inside this clip, in the clip's own frame — the lane's times are // clip-local, and a beat outside the clip can never be snapped to anyway. const snapTimes = useMemo( @@ -539,19 +549,19 @@ export function TimelineAutomationLaneSlot({ // Stale-selection guard: the selected lane's target can vanish out from under // it (e.g. its effect got deleted from the chain, dropping the lane), leaving // a rectangle selecting nothing. Clear it rather than let it point at a - // target that no longer draws. + // target that no longer draws. Above the empty-rows return, because a clip + // that draws nothing is exactly when a selection goes stale. useEffect(() => { const target = bound.selection?.target; if (target !== undefined && !bound.lanes.some((lane) => lane.target === target)) { bound.onRangeClear(); } }, [bound]); - if (bound.lanes.length === 0) return null; + if (rows.length === 0) return null; const inClip = currentTime >= element.start && currentTime <= element.start + element.duration; - const top = getTimelineLaneTop(laneCount); return ( <> - {bound.lanes.map((lane, index) => { + {rows.map(({ lane, rowIndex }) => { const range = resolveAutomationRange(lane.target, bound.chain ?? undefined); // A lane whose target no longer resolves was already dropped upstream; // this is belt and braces so a row can never draw on the wrong axis. @@ -562,7 +572,7 @@ export function TimelineAutomationLaneSlot({ duration={element.duration} widthPx={Math.max(element.duration * pps, 4)} leftPx={element.start * pps} - topPx={top + index * AUTOMATION_LANE_H} + topPx={top + rowIndex * AUTOMATION_LANE_H} automation={bound.automation} target={lane.target} range={range} @@ -591,3 +601,69 @@ export function TimelineAutomationLaneSlot({ ); } + +export interface TimelineAutomationLaneSlotProps { + /** Every clip on the track, in row order — not just the selected one. */ + elements: readonly TimelineElement[]; + isSelected: (element: TimelineElement) => boolean; + lanes: UseAutomationLanesResult; + pps: number; + /** Keyframe lanes already stacked above, which automation sits under. */ + laneCount: number; + accentColor: string; + /** Composition-time playhead; the slot converts it to clip-local. */ + currentTime: number; + /** Composition-time beat grid; the slot converts it to clip-local too. */ + beatTimes?: readonly number[]; +} + +/** + * Every automated parameter on this TRACK, one lane per row — the way a DAW + * stacks them, so two envelopes can be read and edited without swapping a + * control to see either. + * + * Rows belong to the track, not to a clip: clips sharing a row share a row per + * property (see `groupAutomationLanes`), each drawing over its own span, and a + * clip that does not automate that property leaves its stretch empty. Binding one + * clip at a time is what made the visible envelopes change with the selection. + */ +export function TimelineAutomationLaneSlot({ + elements, + isSelected, + lanes, + pps, + laneCount, + accentColor, + currentTime, + beatTimes, +}: TimelineAutomationLaneSlotProps) { + const clips = elements.filter(isAudioTimelineElement); + const rowsByClip = new Map(); + groupAutomationLanes(clips).forEach((group, rowIndex) => { + for (const entry of group.entries) { + const key = getTimelineElementIdentity(entry.element); + const rows = rowsByClip.get(key); + if (rows) rows.push({ lane: entry.lane, rowIndex }); + else rowsByClip.set(key, [{ lane: entry.lane, rowIndex }]); + } + }); + const top = getTimelineLaneTop(laneCount); + return ( + <> + {clips.map((element) => ( + + ))} + + ); +} diff --git a/packages/studio/src/player/components/TimelineAutomationLaneSlot.test.tsx b/packages/studio/src/player/components/TimelineAutomationLaneSlot.test.tsx index d30c2390a..f30eb7a9e 100644 --- a/packages/studio/src/player/components/TimelineAutomationLaneSlot.test.tsx +++ b/packages/studio/src/player/components/TimelineAutomationLaneSlot.test.tsx @@ -3,6 +3,10 @@ import { act } from "react"; import { describe, expect, it, vi } from "vitest"; import { createRoot } from "react-dom/client"; import { TimelineAutomationLaneSlot } from "./TimelineAutomationLane"; +import { AUTOMATION_LANE_H } from "./automationLaneHeight"; +import { PAD_X } from "./automationLaneGeometry"; +import { getTimelineLaneTop } from "./timelineLayout"; +import { elementAutomation, elementAutomationLanes, elementFxChain } from "./automationLaneData"; import type { AutomationLaneBinding, UseAutomationLanesResult } from "./useAutomationLanes"; import type { TimelineElement } from "../store/timelineElement"; @@ -40,8 +44,8 @@ function mountSlot(binding: Partial) { act(() => { createRoot(host).render( false} lanes={lanes} pps={100} laneCount={0} @@ -53,6 +57,106 @@ function mountSlot(binding: Partial) { return { onRangeClear }; } +/** Two narration slices sharing a row, each with its own chain. */ +const chainOf = (nodes: unknown[]) => JSON.stringify({ version: 1, nodes }); +const lanesOf = (...targets: string[]) => + JSON.stringify({ + version: 1, + lanes: targets.map((target) => ({ target, points: [{ t: 0, v: 1 }] })), + }); + +const narration1: TimelineElement = { + ...element, + id: "narration-1", + key: "narration-1", + start: 0, + duration: 4, + fxChain: chainOf([ + { type: "lowpass", id: "n1", params: { frequency: 8000, q: 0.7, poles: "2" } }, + { type: "peaking", id: "n2", params: { frequency: 1000, gain: -3, q: 1.4 } }, + ]), + automation: lanesOf("fx.n2.q"), +}; +const narration2: TimelineElement = { + ...element, + id: "narration-2", + key: "narration-2", + start: 4, + duration: 4, + fxChain: chainOf([{ type: "peaking", id: "n1", params: { frequency: 1000, gain: -6, q: 1.4 } }]), + automation: lanesOf("fx.n1.q", "volume"), +}; + +/** Reads what a clip really carries, the way the live binding does. */ +const readingBind = (element: TimelineElement, isSelected: boolean): AutomationLaneBinding => ({ + automation: elementAutomation(element), + lanes: elementAutomationLanes(element), + chain: elementFxChain(element), + onPreview: vi.fn(), + onCommit: vi.fn(), + onSelect: vi.fn(), + readOnly: !isSelected, + commitTargetKey: null, + selection: null, + onRangeSelect: vi.fn(), + onRangeClear: vi.fn(), +}); + +/** Every drawn envelope as `row @ left`, which is the whole claim under test. */ +function mountRow(elements: readonly TimelineElement[], selectedKey?: string) { + const host = document.createElement("div"); + document.body.append(host); + act(() => { + createRoot(host).render( + el.key === selectedKey} + lanes={{ bind: readingBind }} + pps={100} + laneCount={0} + accentColor="#0af" + currentTime={0} + />, + ); + }); + return [...host.querySelectorAll(".hf-automation-lane")] + .map((lane) => `${lane.style.top} @ ${lane.querySelector("svg")?.style.left}`) + .sort(); +} + +const ROW_0 = `${getTimelineLaneTop(0)}px`; +const ROW_1 = `${getTimelineLaneTop(0) + AUTOMATION_LANE_H}px`; + +describe("TimelineAutomationLaneSlot shared rows", () => { + it("draws two clips' envelopes for one property in the same row", () => { + // One lane track, two envelopes — same row, each over its own span. The + // 1 kHz peaking Q is `fx.n2.q` on one clip and `fx.n1.q` on the other. + expect(mountRow([narration1, narration2])).toEqual( + [ + `${ROW_0} @ ${0 - PAD_X}px`, + `${ROW_0} @ ${400 - PAD_X}px`, + `${ROW_1} @ ${400 - PAD_X}px`, + ].sort(), + ); + }); + + it("leaves a clip's stretch empty in a row it does not automate", () => { + // Only narration-2 has a volume envelope, so row 1 carries one curve and + // narration-1's half of it stays blank rather than drawing a flat line. + expect(mountRow([narration1, narration2]).filter((row) => row.startsWith(ROW_1))).toEqual([ + `${ROW_1} @ ${400 - PAD_X}px`, + ]); + }); + + it("keeps the same rows whichever clip is selected", () => { + // The bug this replaces: the row listed only the selected clip's lanes, so + // clicking a sibling swapped which envelopes existed. + expect(mountRow([narration1, narration2], "narration-1")).toEqual( + mountRow([narration1, narration2], "narration-2"), + ); + }); +}); + describe("TimelineAutomationLaneSlot stale-selection guard", () => { it("clears the selection when its lane's target no longer exists", () => { const { onRangeClear } = mountSlot({ diff --git a/packages/studio/src/player/components/TimelineLanes.tsx b/packages/studio/src/player/components/TimelineLanes.tsx index e7902160c..b037a9f71 100644 --- a/packages/studio/src/player/components/TimelineLanes.tsx +++ b/packages/studio/src/player/components/TimelineLanes.tsx @@ -7,7 +7,11 @@ import { TimelineAutomationLaneSlot } from "./TimelineAutomationLane"; import { useAutomationLanes } from "./useAutomationLanes"; import { useAutomationSelectionKeyboard } from "../../hooks/useAutomationSelectionKeyboard"; import { TimelineTrackHeader } from "./TimelineTrackHeader"; -import { resolveTrackKeyframeClip, trackShowsBeatStrip } from "./useTimelineTrackLayout"; +import { + isTrackRowExpanded, + resolveTrackKeyframeClip, + trackShowsBeatStrip, +} from "./useTimelineTrackLayout"; import { trackDisplayNumber, trackDisplaySuffix } from "./timelineTrackDisplay"; import { clipTimingStart } from "../../hooks/gsapShared"; import { getTimelineEditCapabilities } from "./timelineEditing"; @@ -100,6 +104,8 @@ export function TimelineLanes({ const expandedClipIds = usePlayerStore((s) => s.expandedClipIds); const automationLanes = useAutomationLanes(); useAutomationSelectionKeyboard({ lanes: automationLanes }); + const expandClips = usePlayerStore((s) => s.expandClips); + const setClipExpanded = usePlayerStore((s) => s.setClipExpanded); const toggleClipExpanded = usePlayerStore((s) => s.toggleClipExpanded); const logicalRowsByTrack = useMemo(() => { const byTrack = new Map(); @@ -110,6 +116,16 @@ export function TimelineLanes({ } return byTrack; }, [logicalRows]); + // The caret belongs to the ROW, so it opens and closes every clip on it at + // once. Toggling only the active clip left the row's state depending on which + // sibling happened to be selected: expand one, click another, and the row + // collapsed under a caret that still pointed down. + const toggleRowExpandedTracked = (keys: readonly string[]) => { + const willExpand = !keys.some((key) => expandedClipIds.has(key)); + trackStudioKeyframeLaneExpand({ expanded: willExpand }); + if (willExpand) expandClips(keys); + else for (const key of keys) setClipExpanded(key, false); + }; const toggleClipExpandedTracked = (key: string) => { const willExpand = !expandedClipIds.has(key); trackStudioKeyframeLaneExpand({ expanded: willExpand }); @@ -192,9 +208,14 @@ export function TimelineLanes({ selectedElementIds, ); const keyframeClipKey = keyframeClip?.key ?? keyframeClip?.id; - const keyframeClipExpanded = - keyframeClipKey != null && expandedClipIds.has(keyframeClipKey); - // Link the sticky caret to the canvas lanes with a stable display-row id. + const rowExpanded = isTrackRowExpanded(els, expandedClipIds); + // The clips whose envelopes this row draws, at their dragged positions. + // Once per row, not once per clip in the map below. + const automationElements = els.map(getPreviewElement); + // Minted here because this is the only place that sees BOTH ends of + // the disclosure: the caret in the sticky header and the diamond lanes + // on the canvas. Keyed by display row, not by `trackNum`, which is a + // fractional sort key and would mint ids like `...-0.16666666666666666`. const lanesId = `${lanesIdPrefix}-track-${row}`; // The header's remove buttons write through the same binding the lanes // themselves edit through, so a deletion persists exactly like dragging @@ -245,17 +266,17 @@ export function TimelineLanes({ lanesId={lanesId} contentOrigin={contentOrigin} keyframeClip={keyframeClip} + trackElements={els} clipCount={els.length} - isExpanded={keyframeClipExpanded} + isExpanded={rowExpanded} animations={keyframeClipKey ? (gsapAnimations.get(keyframeClipKey) ?? []) : []} currentTime={currentTime} isTrackHidden={isTrackHidden} isAudioTrack={isAudioTrack} theme={theme} onToggleClipExpanded={() => { - if (keyframeClipKey) { - toggleClipExpandedTracked(keyframeClipKey); - } + const keys = els.map(getTimelineElementIdentity); + if (keys.length > 0) toggleRowExpandedTracked(keys); }} onToggleTrackHidden={onToggleTrackHidden} onTogglePropertyGroupKeyframe={onTogglePropertyGroupKeyframe} @@ -332,7 +353,7 @@ export function TimelineLanes({ // other clips (incl. siblings on a shared track) show compact // diamonds on their own bar instead. const isTrackKeyframeClip = elementKey === keyframeClipKey; - const showsLanes = isTrackKeyframeClip && keyframeClipExpanded; + const showsLanes = isTrackKeyframeClip && rowExpanded; const capabilities = getTimelineEditCapabilities(el); const isSelected = selectedElementId === elementKey || selectedElementIds.has(elementKey); @@ -483,10 +504,15 @@ export function TimelineLanes({ } suppressClickRef={suppressClickRef} footer={ - showsLanes && isAudioTimelineElement(el) ? ( + showsLanes ? ( + // Every clip on the row, not this one: the lanes are + // the TRACK's, one row per automated property. { + const key = getTimelineElementIdentity(element); + return selectedElementId === key || selectedElementIds.has(key); + }} lanes={automationLanes} pps={pps} laneCount={laneCounts.get(elementKey) ?? 0} diff --git a/packages/studio/src/player/components/TimelineTrackHeader.test.tsx b/packages/studio/src/player/components/TimelineTrackHeader.test.tsx index af763b725..55ec0a7b0 100644 --- a/packages/studio/src/player/components/TimelineTrackHeader.test.tsx +++ b/packages/studio/src/player/components/TimelineTrackHeader.test.tsx @@ -62,6 +62,8 @@ const OPACITY = animation("opacity-tween", "visual", [ interface RenderHeaderOptions { keyframeClip?: TimelineElement; + /** Every clip on the track; defaults to just the keyframe clip. */ + trackElements?: readonly TimelineElement[]; animations?: GsapAnimation[]; clipCount?: number; currentTime?: number; @@ -92,6 +94,7 @@ function renderHeader(options: RenderHeaderOptions = {}): { lanesId="timeline-lanes-track-0" contentOrigin={LABEL_COL_W} keyframeClip={next.keyframeClip ?? ELEMENT} + trackElements={next.trackElements ?? [next.keyframeClip ?? ELEMENT]} clipCount={next.clipCount ?? 1} isExpanded={next.expanded !== false} animations={next.animations ?? [POSITION, OPACITY]} @@ -459,9 +462,11 @@ describe("TimelineTrackHeader", () => { it("names every envelope in the label column", () => { const { host, root } = renderHeader({ keyframeClip: BED, animations: [] }); const rows = Array.from(host.querySelectorAll("[data-automation-lane-label]")); + // The attribute is the ROW's identity, which is the label: a row can hold + // several clips' envelopes, whose lane targets differ from each other. expect(rows.map((r) => r.getAttribute("data-automation-lane-label"))).toEqual([ - "fx.n1.gain", - "volume", + "Peaking EQ 1.6 kHz · Gain", + "Volume", ]); // A band is named by its frequency: with several of them, "Peaking EQ" says // nothing about which is which. Bands sit above the level lanes. @@ -528,4 +533,96 @@ describe("TimelineTrackHeader", () => { act(() => root.unmount()); }); }); + + /** + * Several clips on one row share a lane row per property, so the label column + * has to name the row for the property and the header for the track — not for + * whichever clip happens to be selected. + */ + describe("a track several clips share", () => { + const clip = (id: string, over: Partial): TimelineElement => + ({ + id, + key: id, + label: id, + tag: "audio", + start: 0, + duration: 4, + track: 0, + ...over, + }) as TimelineElement; + const PEAKING = (gain: number) => + JSON.stringify({ + version: 1, + nodes: [{ type: "peaking", id: "n1", params: { frequency: 1000, gain, q: 1.4 } }], + }); + const NARRATION_1 = clip("narration-1", { + fxChain: PEAKING(-3), + automation: JSON.stringify({ + version: 1, + lanes: [{ target: "fx.n1.gain", points: [{ t: 0, v: -3 }] }], + }), + }); + const NARRATION_2 = clip("narration-2", { + start: 4, + fxChain: PEAKING(-6), + automation: JSON.stringify({ + version: 1, + lanes: [ + { target: "fx.n1.gain", points: [{ t: 0, v: -6 }] }, + { target: "volume", points: [{ t: 0, v: 1 }] }, + ], + }), + }); + const ROW = { trackElements: [NARRATION_1, NARRATION_2], clipCount: 2, animations: [] }; + + it("lists every clip's envelopes, whichever clip is selected", () => { + const labels = (host: HTMLElement) => + Array.from(host.querySelectorAll("[data-automation-lane-label]")).map((r) => + r.getAttribute("data-automation-lane-label"), + ); + const first = renderHeader({ ...ROW, keyframeClip: NARRATION_1 }); + const second = renderHeader({ ...ROW, keyframeClip: NARRATION_2 }); + // narration-1 has no volume envelope, but the row is still there — it is + // the track's, and it was its sibling's before the selection moved. + expect(labels(first.host)).toEqual(["Peaking EQ 1 kHz · Gain", "Volume"]); + expect(labels(second.host)).toEqual(labels(first.host)); + act(() => first.root.unmount()); + act(() => second.root.unmount()); + }); + + it("removes only from the clip it is showing, and offers nothing where it has no lane", () => { + // A write can only reach the selected clip, so a button on a row that clip + // is absent from could only remove nothing, or somebody else's envelope. + const onRemoveAutomationLane = vi.fn(); + const { host, root } = renderHeader({ + ...ROW, + keyframeClip: NARRATION_1, + onRemoveAutomationLane, + }); + expect( + Array.from(host.querySelectorAll('button[aria-label$="automation"]')).map((b) => + b.getAttribute("aria-label"), + ), + ).toEqual(["Remove Peaking EQ 1 kHz · Gain automation"]); + act(() => host.querySelector('button[aria-label$="automation"]')?.click()); + expect(onRemoveAutomationLane).toHaveBeenCalledWith("fx.n1.gain"); + act(() => root.unmount()); + }); + + it("names the header for the track, not for one of the clips on it", () => { + const view = renderHeader({ ...ROW, keyframeClip: NARRATION_2 }); + expect(view.host.textContent).not.toContain("narration-2"); + expect(view.host.querySelector('[title="Track 1"]')?.textContent).toBe("Track 1"); + // Alone on the track it is still named for itself. + view.rerender({ + ...ROW, + keyframeClip: NARRATION_2, + trackElements: [NARRATION_2], + clipCount: 1, + }); + expect(view.host.querySelector('[title="narration-2"]')?.textContent).toBe("narration-2"); + act(() => view.root.unmount()); + }); + }); }); diff --git a/packages/studio/src/player/components/TimelineTrackHeader.tsx b/packages/studio/src/player/components/TimelineTrackHeader.tsx index 8241e8e1f..7244d717a 100644 --- a/packages/studio/src/player/components/TimelineTrackHeader.tsx +++ b/packages/studio/src/player/components/TimelineTrackHeader.tsx @@ -4,13 +4,7 @@ import { Music } from "../../icons/SystemIcons"; import type { TimelineElement } from "../store/playerStore"; import type { TimelineEditCallbacks } from "./timelineCallbacks"; import { getTimelinePropertyLanes } from "./TimelinePropertyLanes"; -import { automationLaneCountOf } from "./useTimelineTrackLayout"; -import { - automationLaneLabel, - automationLaneLabelParts, - elementAutomationLanes, - elementFxChain, -} from "./automationLaneData"; +import { groupAutomationLanes } from "./automationLaneData"; import { AUTOMATION_LANE_H } from "./automationLaneHeight"; import { clipTimingStart } from "../../hooks/gsapShared"; import { LayerDisclosureRow } from "./LayerDisclosureRow"; @@ -26,32 +20,6 @@ import { valueReadout } from "./trackHeaderLaneValues"; import { trackDisplaySuffix } from "./timelineTrackDisplay"; import { timelineLogicalRowCellId, timelinePropertyRowId } from "./timelineNavigationIdentity"; -/** One envelope's label-column row content, resolved against the FX chain. */ -interface AutomationRowInfo { - target: string; - label: string; - name: string; - param: string; -} - -/** - * Each envelope's name, resolved against the chain the same way the lane - * resolves its axis — a band is named by its frequency, not by its effect. The - * lane list is already in drawing order, which is the order these rows have to - * follow: a name beside the wrong envelope is worse than an awkward order. - */ -function resolveAutomationRows(keyframeClip: TimelineElement | null): AutomationRowInfo[] { - if (!keyframeClip) return []; - const chain = elementFxChain(keyframeClip); - return elementAutomationLanes(keyframeClip).flatMap((lane) => { - const parts = automationLaneLabelParts(lane.target, chain); - const label = automationLaneLabel(lane.target, chain); - return parts && label - ? [{ target: lane.target, label, name: parts.name, param: parts.param }] - : []; - }); -} - interface TimelineTrackHeaderProps { /** The track's real key: a FRACTIONAL z-order sort value. Routes callbacks; * never shown or announced. */ @@ -69,6 +37,9 @@ interface TimelineTrackHeaderProps { /** The track's active keyframe clip (selected, else primary) — the one whose * disclosure + property rows this header shows, whether expanded or not. */ keyframeClip: TimelineElement | null; + /** Every clip on this track. Automation rows are the track's, unioned over + * these, so they stop changing with the selection. */ + trackElements: readonly TimelineElement[]; /** Clips on this track, so the header can say how many the row holds. */ clipCount: number; isExpanded: boolean; @@ -347,8 +318,11 @@ function AutomationLaneHeaderRow({ columnWidth, onRemove, }: { - target: string; - /** The whole thing on one line, for the tooltip and the remove button's name. */ + /** The lane the ACTIVE clip draws in this row, or null when it draws none — + * the row belongs to the property, and a clip may be absent from it. */ + target: string | null; + /** The whole thing on one line: the row's identity, its tooltip, and the + * remove button's name. */ label: string; /** What the effect is — "Peaking EQ 1.6 kHz". */ name: string; @@ -362,7 +336,7 @@ function AutomationLaneHeaderRow({ }) { return (
= LABEL_COL_W; + // One row per automated property across the whole track, in the order the + // canvas draws them — a name beside the wrong envelope is worse than an awkward + // order. `target` is the ACTIVE clip's lane in that row, which is the only one + // the remove button can write to; null when the row belongs to its siblings. + const activeKey = keyframeClip ? (keyframeClip.key ?? keyframeClip.id) : null; + const automationRows = groupAutomationLanes(trackElements).map((group) => ({ + key: group.key, + label: group.key, + name: group.name, + param: group.param, + target: + group.entries.find((entry) => (entry.element.key ?? entry.element.id) === activeKey)?.lane + .target ?? null, + })); // Automation counts as something to disclose: gating the caret on tweens alone // left an audio clip's envelopes unreachable, since the track could not expand. - const disclosable = - lanes.length > 0 || (keyframeClip ? automationLaneCountOf(keyframeClip) : 0) > 0; - const automationRows = resolveAutomationRows(keyframeClip); + const disclosable = lanes.length > 0 || automationRows.length > 0; const isKeyframeLayer = !!keyframeClip && disclosable; return ( @@ -494,7 +486,15 @@ export function TimelineTrackHeader({ ) : ( <> 1 + ? `Track${trackDisplaySuffix(trackDisplayNumber)}` + : (keyframeClip.label ?? keyframeClip.domId ?? keyframeClip.id) + } clipCount={clipCount} isExpanded={isExpanded} gutterBackground={theme.gutterBackground} @@ -547,7 +547,7 @@ export function TimelineTrackHeader({ {isExpanded && automationRows.map((row, index) => ( { }); }); }); + +describe("laneGroupKey", () => { + // Two clips, each with their OWN chain, both automating a 1 kHz peaking Q. Node + // ids are minted per chain, so the ids collide across clips while meaning + // different things — and match across clips while meaning the same thing. + const clipA = parseChain({ + version: 1, + nodes: [ + { type: "lowpass", id: "n1", params: { frequency: 8000, q: 0.7, poles: "2" } }, + { type: "peaking", id: "n2", params: { frequency: 1000, gain: -3, q: 1.4 } }, + ], + }); + const clipB = parseChain({ + version: 1, + nodes: [{ type: "peaking", id: "n1", params: { frequency: 1000, gain: -6, q: 1.4 } }], + }); + + it("groups the same property of the same effect across clips", () => { + // The whole point: one row for "Peaking EQ 1 kHz · Q", whichever clip it is on. + expect(laneGroupKey("fx.n2.q", clipA)).toBe(laneGroupKey("fx.n1.q", clipB)); + }); + + it("does not group by lane target, which collides across chains", () => { + // `fx.n1.q` is a low-pass on one clip and a peaking EQ on the other. Grouping by + // target would put those two envelopes in one row. + expect(laneGroupKey("fx.n1.q", clipA)).not.toBe(laneGroupKey("fx.n1.q", clipB)); + }); + + it("separates parameters, and the same parameter on different bands", () => { + expect(laneGroupKey("fx.n2.q", clipA)).not.toBe(laneGroupKey("fx.n2.gain", clipA)); + const twoBands = parseChain({ + version: 1, + nodes: [ + { type: "peaking", id: "n1", params: { frequency: 400, gain: -6, q: 1.4 } }, + { type: "peaking", id: "n2", params: { frequency: 1600, gain: -6, q: 1.4 } }, + ], + }); + expect(laneGroupKey("fx.n1.gain", twoBands)).not.toBe(laneGroupKey("fx.n2.gain", twoBands)); + }); + + it("groups volume with volume, and nothing with an unresolvable target", () => { + expect(laneGroupKey("volume", clipA)).toBe(laneGroupKey("volume", clipB)); + expect(laneGroupKey("fx.gone.gain", clipA)).toBeNull(); + }); +}); + +describe("groupAutomationLanes", () => { + const chainOf = (nodes: unknown[]) => JSON.stringify({ version: 1, nodes }); + const lanesOf = (...targets: string[]) => + JSON.stringify({ + version: 1, + lanes: targets.map((target) => ({ target, points: [{ t: 0, v: 1 }] })), + }); + + // Two narration slices sharing a row. Each mints its own chain, so the node ids + // collide across them while meaning different things. + const narration1 = el({ + id: "narration-1", + key: "narration-1", + fxChain: chainOf([ + { type: "lowpass", id: "n1", params: { frequency: 8000, q: 0.7, poles: "2" } }, + { type: "peaking", id: "n2", params: { frequency: 1000, gain: -3, q: 1.4 } }, + ]), + automation: lanesOf("fx.n2.q"), + }); + const narration2 = el({ + id: "narration-2", + key: "narration-2", + fxChain: chainOf([ + { type: "peaking", id: "n1", params: { frequency: 1000, gain: -6, q: 1.4 } }, + ]), + automation: lanesOf("fx.n1.q", "volume"), + }); + + it("puts the same property of the same effect in one row, one entry per clip", () => { + const groups = groupAutomationLanes([narration1, narration2]); + expect(groups.map((g) => g.key)).toEqual(["Peaking EQ 1 kHz · Q", "Volume"]); + expect(groups[0]?.entries.map((e) => e.element.id)).toEqual(["narration-1", "narration-2"]); + expect(groups[0]).toMatchObject({ name: "Peaking EQ 1 kHz", param: "Q" }); + }); + + it("leaves a clip out of a row it does not automate", () => { + // The empty stretch is the point: narration-1 has no volume envelope, so it + // contributes nothing to that row rather than a flat line claiming one exists. + const groups = groupAutomationLanes([narration1, narration2]); + expect(groups[1]?.entries.map((e) => e.element.id)).toEqual(["narration-2"]); + }); + + it("does not merge lanes whose targets collide across chains", () => { + // `fx.n1.*` is a low-pass on one clip and a peaking EQ on the other. + const lowpass = el({ ...narration1, automation: lanesOf("fx.n1.q") }); + const peaking = el({ ...narration2, automation: lanesOf("fx.n1.q") }); + const groups = groupAutomationLanes([lowpass, peaking]); + expect(groups.map((g) => g.key)).toEqual(["Low-pass 8 kHz · Q", "Peaking EQ 1 kHz · Q"]); + expect(groups.map((g) => g.entries.length)).toEqual([1, 1]); + }); + + it("ignores clips that are not audio, the way the reserved height does", () => { + const video = el({ id: "titles", key: "titles", tag: "div", automation: lanesOf("volume") }); + expect(groupAutomationLanes([video])).toEqual([]); + }); + + it("skips a target that does not resolve against its clip's chain", () => { + const stale = el({ ...narration2, automation: lanesOf("fx.gone.q", "volume") }); + expect(groupAutomationLanes([stale]).map((g) => g.key)).toEqual(["Volume"]); + }); +}); diff --git a/packages/studio/src/player/components/automationLaneData.ts b/packages/studio/src/player/components/automationLaneData.ts index c1c8c850c..01a35df66 100644 --- a/packages/studio/src/player/components/automationLaneData.ts +++ b/packages/studio/src/player/components/automationLaneData.ts @@ -22,6 +22,7 @@ import { type HfAutomationLane, } from "@hyperframes/core/audio-automation"; import { parseAudioFxChain, type HfAudioFxChain } from "@hyperframes/core/audio-fx"; +import { isAudioTimelineElement } from "../../utils/timelineInspector"; import type { TimelineElement } from "../store/playerStore"; const EMPTY: HfAutomation = { version: 1, lanes: [] }; @@ -168,6 +169,87 @@ export function automationLaneLabelParts( return { name: name || param, param: name ? param : "" }; } +/** + * What makes two lanes, on two different clips, the same lane row. + * + * A lane is a property over time, not a clip's private strip: four narration slices + * on one track that each automate a 1 kHz peaking Q belong in ONE row, each drawing + * its envelope over its own span. + * + * The key cannot be the lane target. Targets are `fx..` and node ids + * are minted per chain, so `fx.n1.q` on one clip and `fx.n1.q` on another may be + * different effects entirely — grouping by target would put unrelated envelopes in + * one row and split matching ones apart. So the key is what identifies the parameter + * to a reader: the effect, whatever distinguishes it from its siblings (a filter's + * frequency), and the parameter. Which is exactly what the label already says, so + * the row's identity and its name cannot drift apart. + * + * Null when the target does not resolve, the same condition that stops it drawing. + */ +export function laneGroupKey(target: string, chain: HfAudioFxChain | null): string | null { + return automationLaneLabel(target, chain); +} + +/** One clip's envelope inside a shared row: the clip, and the lane it draws. */ +export interface AutomationLaneGroupEntry { + element: TimelineElement; + lane: HfAutomationLane; +} + +/** A lane row on a track: one property, and every clip that automates it. */ +export interface AutomationLaneGroup { + /** {@link laneGroupKey} — the row's identity, and also its whole label. */ + key: string; + /** The label's two lines, as {@link automationLaneLabelParts} splits them. */ + name: string; + param: string; + /** In track order. A clip that does not automate this property is simply + * absent, leaving its stretch of the row empty. */ + entries: AutomationLaneGroupEntry[]; +} + +/** + * The lane rows a TRACK shows, unioned over the clips sharing it. + * + * Several clips on one row (four narration slices, say) each carry their own + * chain and their own envelopes. Drawing only the selected clip's made a per-clip + * lane read as governing the whole row, and swapped which envelopes were visible + * whenever the selection moved. So the row is keyed by the property — a clip + * draws into the row for `Peaking EQ 1 kHz · Q` over its own span, and clips that + * automate nothing there leave it empty. + * + * Row order is first-seen: each clip's lanes are already in draw order (the + * spectrum, top down), so the first clip to carry a property fixes its row and + * later clips only append properties nobody has shown yet. + * + * Non-audio elements contribute nothing, matching `automationLaneCountOf` — the + * row's reserved height and its drawn lanes have to count the same clips. + */ +export function groupAutomationLanes(elements: readonly TimelineElement[]): AutomationLaneGroup[] { + const groups = new Map(); + for (const element of elements) { + if (!isAudioTimelineElement(element)) continue; + const chain = elementFxChain(element); + for (const lane of elementAutomationLanes(element)) { + const key = laneGroupKey(lane.target, chain); + const parts = automationLaneLabelParts(lane.target, chain); + // Null on both together: an unresolvable target draws no lane either. + if (!key || !parts) continue; + const group = groups.get(key); + if (group) group.entries.push({ element, lane }); + else { + groups.set(key, { + key, + name: parts.name, + param: parts.param, + entries: [{ element, lane }], + }); + } + } + } + return [...groups.values()]; +} + /** The whole label on one line, for a tooltip or an accessible name. */ export function automationLaneLabel(target: string, chain: HfAudioFxChain | null): string | null { const parts = automationLaneLabelParts(target, chain); diff --git a/packages/studio/src/player/components/useTimelineTrackLayout.test.ts b/packages/studio/src/player/components/useTimelineTrackLayout.test.ts index b622779d6..8aaf60c2f 100644 --- a/packages/studio/src/player/components/useTimelineTrackLayout.test.ts +++ b/packages/studio/src/player/components/useTimelineTrackLayout.test.ts @@ -6,6 +6,7 @@ import type { GsapAnimation } from "@hyperframes/core/gsap-parser"; import { afterEach, describe, expect, it } from "vitest"; import { usePlayerStore, type TimelineElement } from "../store/playerStore"; import { LANE_H, TRACK_H } from "./timelineLayout"; +import { AUTOMATION_LANE_H } from "./automationLaneHeight"; import { getTimelinePropertyLanes } from "./TimelinePropertyLanes"; import { resolveTrackKeyframeClip, useTimelineTrackLayout } from "./useTimelineTrackLayout"; @@ -102,6 +103,61 @@ const audioClip = (id: string, over: Partial = {}): TimelineEle ...over, }); +/** + * Clips sharing a row share a lane row per property, so the height they reserve + * is the track's grouped count — and the row is open when ANY of them is + * expanded, or clicking a sibling collapsed it. + */ +describe("a track several clips share", () => { + const peaking = (gain: number) => + JSON.stringify({ + version: 1, + nodes: [{ type: "peaking", id: "n1", params: { frequency: 1000, gain, q: 1.4 } }], + }); + const lanes = (...targets: string[]) => + JSON.stringify({ + version: 1, + lanes: targets.map((target) => ({ target, points: [{ t: 0, v: 1 }] })), + }); + const narration1 = audioClip("narration-1", { + fxChain: peaking(-3), + automation: lanes("fx.n1.gain"), + }); + const narration2 = audioClip("narration-2", { + start: 10, + fxChain: peaking(-6), + automation: lanes("fx.n1.gain", "volume"), + }); + + /** Reserved height for the row, with only narration-1 ever expanded. */ + function rowHeight(selectedElementId: string | null): number { + usePlayerStore.setState({ expandedClipIds: new Set(["narration-1"]) }); + let height = 0; + function Probe() { + height = + useTimelineTrackLayout([narration1, narration2], new Map(), selectedElementId, new Set()) + .rowHeights[0] ?? 0; + return null; + } + const root = createRoot(document.createElement("div")); + act(() => root.render(React.createElement(Probe))); + act(() => root.unmount()); + return height; + } + + it("reserves one row per property, not per clip's lane", () => { + // Two properties across the two clips — a shared 1 kHz peaking gain and a + // volume envelope on one of them — so two rows, not three. + expect(rowHeight("narration-1")).toBe(TRACK_H + 2 * AUTOMATION_LANE_H); + }); + + it("stays open at the same height when the selection moves to a sibling", () => { + // Expansion is stored per clip but reads as the row's: asking only about the + // active clip collapsed the row the moment another was clicked. + expect(rowHeight("narration-2")).toBe(rowHeight("narration-1")); + }); +}); + describe("resolveTrackKeyframeClip", () => { const none = new Map(); diff --git a/packages/studio/src/player/components/useTimelineTrackLayout.ts b/packages/studio/src/player/components/useTimelineTrackLayout.ts index 76c63ddff..7e91aa02d 100644 --- a/packages/studio/src/player/components/useTimelineTrackLayout.ts +++ b/packages/studio/src/player/components/useTimelineTrackLayout.ts @@ -2,7 +2,7 @@ import { useMemo, useRef } from "react"; import type { GsapAnimation } from "@hyperframes/core/gsap-parser"; import { animationLaneGroups } from "./TimelinePropertyLanes"; import { isAudioTimelineElement } from "../../utils/timelineInspector"; -import { elementAutomationLanes } from "./automationLaneData"; +import { elementAutomationLanes, groupAutomationLanes } from "./automationLaneData"; import { usePlayerStore, type TimelineElement } from "../store/playerStore"; import type { DraggedClipState } from "./timelineClipDragTypes"; import { useTimelineTrackDerivations } from "./useTimelineTrackDerivations"; @@ -42,10 +42,34 @@ export function trackShowsBeatStrip( * counts toward whether a track has anything to disclose. A function rather than * a map so every caller reads the same cached parse and none can drift. */ -export function automationLaneCountOf(element: TimelineElement): number { +function automationLaneCountOf(element: TimelineElement): number { return isAudioTimelineElement(element) ? elementAutomationLanes(element).length : 0; } +/** + * Automation rows a TRACK reserves: the union over the clips sharing it, since + * clips on one row share a lane row per property. Counting only the active clip's + * lanes reserved a height that changed with the selection. + */ +function trackAutomationLaneCount(elements: readonly TimelineElement[]): number { + return groupAutomationLanes(elements).length; +} + +/** + * Is this row disclosed? Expansion is stored per clip, but it reads as a property + * of the ROW: the active clip changes with the selection, so asking only about it + * collapsed the row the moment you clicked a sibling. Any expanded clip on the + * track holds the row open — and the caret expands and collapses all of them + * together (see TimelineLanes), so the two can only disagree on state predating + * this rule or written by the keyframe auto-expand. + */ +export function isTrackRowExpanded( + elements: readonly TimelineElement[], + expandedClipIds: ReadonlySet, +): boolean { + return elements.some((element) => expandedClipIds.has(element.key ?? element.id)); +} + /** * The single keyframed element whose property lanes a track shows when expanded. * A track can hold several elements (same z-index is common), but keyframes are @@ -112,8 +136,9 @@ function useTimelineRowHeights( const expandedClipIds = usePlayerStore((s) => s.expandedClipIds); const { laneCounts, rowGeometry } = useMemo(() => { const laneCounts = computeLaneCounts(tracks, gsapAnimations); - // Row height follows only the active keyframe clip, so a track with several + // Keyframe lanes follow only the active clip, so a track with several // keyframed elements never reserves empty lanes for the ones not shown. + // Automation lanes follow the whole row: they are shared per property. const heightTracks: TimelineTrackHeightClip[][] = tracks.map(([, elements]) => { const active = resolveTrackKeyframeClip( elements, @@ -123,11 +148,18 @@ function useTimelineRowHeights( ); if (!active) return []; const clipId = active.key ?? active.id; + // `trackHeights` gates the reserved lanes on this id being expanded, and the + // row is expanded when ANY of its clips is — so hand it whichever clip holds + // the row open, while the lane counts stay the active clip's (keyframes) and + // the track's (automation, shared across the row). + const holdingOpen = elements.find((element) => + expandedClipIds.has(element.key ?? element.id), + ); return [ { - clipId, + clipId: holdingOpen ? (holdingOpen.key ?? holdingOpen.id) : clipId, laneCount: laneCounts.get(clipId) ?? 0, - automationLaneCount: automationLaneCountOf(active), + automationLaneCount: trackAutomationLaneCount(elements), }, ]; }); diff --git a/plans/automation-lanes-shared-rows.md b/plans/automation-lanes-shared-rows.md new file mode 100644 index 000000000..fae2bc710 --- /dev/null +++ b/plans/automation-lanes-shared-rows.md @@ -0,0 +1,58 @@ +# Shared automation lane rows on a track + +Decision taken 2026-08-07. Not yet implemented. + +## The bug + +Four narration slices sit on one track row. The row's header is named after +whichever clip is selected ("Narration 2"), carries a `4` badge, and lists that +clip's automation lanes ("Peaking EQ 1 kHz / Q"). So a lane belonging to one clip +reads as governing the whole row, and changing the selection silently swaps which +envelopes are visible. + +The lane SVG itself is already confined to its clip — `TimelineAutomationLaneSlot` +passes `leftPx={element.start * pps}` and `widthPx={element.duration * pps}`. What +misleads is the label column and the full-width row, not the geometry. + +## What was decided, and what was rejected + +**Effects stay on the clip.** Track-level FX was considered and rejected: there is +no track to own a chain. `data-track-index` is parsed in exactly one place +(`core/src/runtime/timeline.ts:63`) and only to choose a row; there is no track +element, no manifest, and both runtimes build audio per element — each clip gets its +own source → chain → gain → master. Track FX would mean inventing a storage location +and a bus node in two runtimes. It is also wrong for the domain: two takes on one row +often want different treatment, and a carve names specific clips. + +**Clips on one row share a lane row when it is the same property of the same +effect.** One row for `Peaking EQ 1 kHz / Q`, with each clip's envelope drawn over +its own span. A different parameter, or the same parameter on a different effect, is +its own row. + +## Implementation notes + +- **The grouping key cannot be the lane target.** Targets are `fx..` + and node ids are minted per chain, so `fx.n1.q` in one clip and `fx.n1.q` in + another may be different effects entirely. Key on what identifies the parameter to + a reader: effect label + distinguishing setting + param — which is exactly what + `automationLaneLabelParts` in `automationLaneData.ts` already computes (it resolves + the frequency for peaking-style filters). `volume` groups by itself. +- **Rows come from the track, not the selection.** `TimelineAutomationLaneSlot` + currently binds one element. It needs the clips on the row, their chains, and the + union of their grouped lanes — so lanes stop appearing and disappearing as the + selection moves. +- **Gestures stay per clip.** Each clip keeps its own SVG, its own + `useAutomationLaneGestures`, and its own selection box; a shared row is a shared + *lane track*, not a shared envelope. Two clips' envelopes in one row must not be + draggable as one thing. +- **Row height** is `AUTOMATION_LANE_H` per grouped lane, not per clip-lane, so + `getTimelineLaneTop` and the header's row positions follow the grouped count. +- **Header labelling**: name the header for the track when it holds several clips + rather than for one of them, and let the lane label stand for the property (it is + the group's identity now, so it needs no clip qualifier). + +## Worth deciding while implementing + +A clip on the row that does *not* automate a grouped property has empty space in +that row. Leave it empty (the envelope is simply absent there) rather than drawing a +flat line at the stored value — a flat line would claim an envelope exists.