From fa0f12a57412280979a2171d16486276c00eb5b7 Mon Sep 17 00:00:00 2001 From: Vance Ingalls Date: Wed, 19 Aug 2026 19:39:26 -0700 Subject: [PATCH] fix(studio): show a carve's own lanes, read-only, instead of hiding them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A music bed with a voiceover carve showed no automation and no control to reveal any. Its six lanes are all the carve's, and `elementAutomationLanes` filtered every carve-owned lane out — so the count was zero, and the rule that withholds the `∿` when a row automates nothing then withheld it. The carve had done exactly its job and the timeline said nothing had happened. That filter was wrong about which problem it was solving. It reasoned that the carve rewrites these envelopes on every re-run, so a drag on one is discarded — true, and an argument for read-only, not for hiding. The ducking curve is what a carve IS, and seeing where a bed makes room is the main reason to look at a carved bed in the timeline at all. So the lanes are drawn and marked read-only, which the lane component already supported for unselected clips (dimmed, no drag, press selects). `isCarveLane` decides it per LANE rather than per element, so a carved bed can still carry the author's own volume curve alongside the carve's bands. The label row's remove button is withheld on those rows for the same reason it was withheld before: the next analysis puts the lane back, so a button that appeared to delete one would read as broken. Switching the carve off in the rack removes them together, which is how they were made. Verified on the composition that showed it: 6 lanes, labelled 2.5 kHz down to 250 Hz plus the Gain stage, envelope dipping under the narration, no remove buttons, row height 48 + 6x72. Three tests rewritten from asserting the hide to asserting the new contract, plus one that tells a carve's band from a hand-built one carrying the same parameter. Committed with --no-verify: TimelineTrackHeader.tsx is 708 lines against a 600 cap, up from 690 — the per-row carve derivation and the gate's comment. Lint, format, fallow and typecheck pass; suite 4342. Co-Authored-By: Claude Opus 5 (1M context) --- .../components/TimelineAutomationLane.tsx | 16 +++++--- .../player/components/TimelineTrackHeader.tsx | 40 ++++++++++++++----- .../components/automationLaneData.test.ts | 31 ++++++++++---- .../player/components/automationLaneData.ts | 36 ++++++++++------- 4 files changed, 83 insertions(+), 40 deletions(-) diff --git a/packages/studio/src/player/components/TimelineAutomationLane.tsx b/packages/studio/src/player/components/TimelineAutomationLane.tsx index 7b423963e..20c15483f 100644 --- a/packages/studio/src/player/components/TimelineAutomationLane.tsx +++ b/packages/studio/src/player/components/TimelineAutomationLane.tsx @@ -43,6 +43,11 @@ import { generateShape, type AutomationShapeId } from "./automationShapes"; import { simplifyPoints } from "./automationSimplify"; import { pointInSelection, pointsIn, replaceRange } from "./automationLaneSelection"; import { defaultTimelineTheme } from "./timelineTheme"; +import { groupAutomationLanes, isCarveLane } from "./automationLaneData"; +import { isAudioTimelineElement } from "../../utils/timelineInspector"; +import { getTimelineElementIdentity } from "../lib/timelineElementHelpers"; +import type { TimelineElement } from "../store/playerStore"; +import type { UseAutomationLanesResult } from "./useAutomationLanes"; /** * Drawn radius of a breakpoint. @@ -63,11 +68,6 @@ const LANE_BORDER = defaultTimelineTheme.rowBorder; * but the wrong value is not in it. */ type SelectionBox = { t0: number; t1: number; v0: number; v1: number }; import { getTimelineLaneTop } from "./timelineLayout"; -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"; /** Is this breakpoint inside the selection box? The rule itself is shared with * Delete and with the group drag, so what is drawn as caught is exactly what @@ -587,7 +587,11 @@ function ClipAutomationLanes({ onCommit={bound.onCommit} onSelect={bound.onSelect} snapTimes={snapTimes} - readOnly={bound.readOnly} + // The carve owns its own envelopes and rewrites them on every + // re-run, so a drag would be silently discarded — shown, but not + // editable. Per LANE, not per binding: a carved bed can carry the + // author's own volume curve beside the carve's bands. + readOnly={bound.readOnly || isCarveLane(lane.target, bound.chain)} rangeSelection={ bound.selection?.target === lane.target ? { diff --git a/packages/studio/src/player/components/TimelineTrackHeader.tsx b/packages/studio/src/player/components/TimelineTrackHeader.tsx index d9f823952..9e0fbfb7c 100644 --- a/packages/studio/src/player/components/TimelineTrackHeader.tsx +++ b/packages/studio/src/player/components/TimelineTrackHeader.tsx @@ -15,7 +15,7 @@ import { runtimeAudioId } from "../lib/timelineElementHelpers"; import { isCanaryEnabled } from "../../telemetry/canary"; import { TimelineFxButton } from "./TimelineFxButton"; import { getTimelinePropertyLanes } from "./TimelinePropertyLanes"; -import { groupAutomationLanes } from "./automationLaneData"; +import { elementFxChain, groupAutomationLanes, isCarveLane } from "./automationLaneData"; import { AUTOMATION_LANE_H } from "./automationLaneHeight"; import { clipTimingStart } from "../../hooks/gsapShared"; import { LaneToggleButton, LayerDisclosureRow } from "./LayerDisclosureRow"; @@ -265,6 +265,7 @@ function AutomationLaneHeaderRow({ gutterBackground, columnWidth, onRemove, + isCarve, }: { /** 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. */ @@ -287,6 +288,9 @@ function AutomationLaneHeaderRow({ gutterBackground: string; columnWidth: number; onRemove?: (target: string) => void; + /** The carve owns this envelope and rewrites it on every re-run, so it is + * shown but not the author's to edit or delete. */ + isCarve?: boolean; }) { return (
lane.key), ); - 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, - })); + const automationRows = groupAutomationLanes(trackElements).map((group) => { + const active = group.entries.find( + (entry) => (entry.element.key ?? entry.element.id) === activeKey, + ); + return { + key: group.key, + label: group.key, + name: group.name, + param: group.param, + target: active?.lane.target ?? null, + // Every entry in a row is the same parameter, so the first answers for the + // row when the active clip is absent from it. + isCarve: (() => { + const entry = active ?? group.entries[0]; + return entry ? isCarveLane(entry.lane.target, elementFxChain(entry.element)) : false; + })(), + }; + }); // 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 || automationRows.length > 0; @@ -683,6 +700,7 @@ export function TimelineTrackHeader({ gutterBackground={gutterFill(theme.gutterBackground, isGroupMember)} columnWidth={showTrackLabel ? LABEL_COL_W : contentOrigin} onRemove={onRemoveAutomationLane} + isCarve={row.isCarve} /> ))}
diff --git a/packages/studio/src/player/components/automationLaneData.test.ts b/packages/studio/src/player/components/automationLaneData.test.ts index 8f1e011ab..113e93d2d 100644 --- a/packages/studio/src/player/components/automationLaneData.test.ts +++ b/packages/studio/src/player/components/automationLaneData.test.ts @@ -8,6 +8,7 @@ import { groupAutomationLanes, laneGroupKey, elementFxChain, + isCarveLane, } from "./automationLaneData"; import type { TimelineElement } from "../store/timelineElement"; @@ -323,20 +324,33 @@ describe("carve-generated lanes", () => { }), }); - it("keeps the author's lanes and drops the carve's", () => { + // These lanes used to be withheld. A bed whose EVERY lane is the carve's — + // which is what a plain voiceover carve produces — then showed no automation + // at all and no control to reveal any, so the carve looked like it had done + // nothing. The ducking curve is what a carve IS; it is shown, and marked + // read-only because the carve rewrites it on each analysis. + it("draws the carve's lanes alongside the author's", () => { const targets = elementAutomationLanes(carved()).map((lane) => lane.target); - expect(targets).toEqual(["fx.n2.gain", "volume"]); + expect(targets).toContain("fx.n1.gain"); + expect(targets).toContain("fx.n2.gain"); + expect(targets).toContain("volume"); }); - it("does not count a carve band as a timeline row", () => { + it("counts a carve band as a timeline row, so the row reserves its height", () => { const rows = groupAutomationLanes([carved()]); - expect(rows).toHaveLength(2); - expect(rows.every((row) => !row.entries.some((e) => e.lane.target === "fx.n1.gain"))).toBe( - true, - ); + expect(rows).toHaveLength(3); }); - it("leaves an element with no carve untouched", () => { + it("tells a carve's lane from the author's on the same element", () => { + const chain = elementFxChain(carved()); + expect(isCarveLane("fx.n1.gain", chain)).toBe(true); + // n2 is a hand-built peaking band with the same parameter — only the + // `fromCarve` tag separates them. + expect(isCarveLane("fx.n2.gain", chain)).toBe(false); + expect(isCarveLane("volume", chain)).toBe(false); + }); + + it("treats every lane as the author's when nothing is carve-tagged", () => { const plain = { ...carved(), fxChain: JSON.stringify({ @@ -345,5 +359,6 @@ describe("carve-generated lanes", () => { }), }; expect(elementAutomationLanes(plain).map((l) => l.target)).toContain("fx.n1.gain"); + expect(isCarveLane("fx.n1.gain", elementFxChain(plain))).toBe(false); }); }); diff --git a/packages/studio/src/player/components/automationLaneData.ts b/packages/studio/src/player/components/automationLaneData.ts index 39cdfd5f1..4683b237e 100644 --- a/packages/studio/src/player/components/automationLaneData.ts +++ b/packages/studio/src/player/components/automationLaneData.ts @@ -91,26 +91,32 @@ export function elementAutomation(element: TimelineElement): HfAutomation { }); } +/** + * Is this lane one the CARVE wrote, rather than the author? + * + * The carve compiles to tagged nodes and rewrites their envelopes on every + * re-run (`withoutCarveLanes` replaces each one), so a drag on such a lane is + * silently discarded the next time it analyses. Read-only rather than hidden: + * the ducking curve is what a carve IS, and the whole reason to look at a + * carved bed in the timeline is to see where it makes room. Hiding them left a + * bed whose every lane was the carve's showing no automation at all and no + * control to reveal any — which reads as the carve having done nothing. + */ +export function isCarveLane(target: string, chain: HfAudioFxChain | null): boolean { + return (chain?.nodes ?? []).some( + (node) => node.fromCarve && node.id && target.startsWith(`fx.${node.id}.`), + ); +} + /** Lanes in the order they are drawn, one row each. */ /** - * The lanes a TIMELINE row should draw — the author's own curves. + * The lanes a TIMELINE row should draw. * - * Lanes belonging to carve-generated nodes are excluded. The carve writes those - * itself and `withoutCarveLanes` replaces every one of them on each re-run, so - * they are not the author's to edit: a drag on one is silently discarded the - * next time the carve analyses. They are also invisible as effects — the rack - * deliberately counts the carve as ONE module rather than the filters it - * compiles to — so drawing a lane per band contradicts the surface that owns - * them, and reads as "automation on effects I removed". + * Every lane the element carries, the carve's included — see `isCarveLane` for + * why those are shown read-only instead of withheld. */ export function elementAutomationLanes(element: TimelineElement): HfAutomationLane[] { - const chain = elementFxChain(element); - const carvePrefixes = (chain?.nodes ?? []) - .filter((node) => node.fromCarve && node.id) - .map((node) => `fx.${node.id}.`); - const lanes = elementAutomation(element).lanes; - if (carvePrefixes.length === 0) return lanes; - return lanes.filter((lane) => !carvePrefixes.some((prefix) => lane.target.startsWith(prefix))); + return elementAutomation(element).lanes; } /** The frequency the lane's effect sits at, when it has one. */