Skip to content

Commit 28485fe

Browse files
vanceingallsclaude
andcommitted
fix(studio): stop the last two automation writes reloading the preview
The quiet commit added here was only used by the FX group. Two writers still went through the refreshing one, so they reloaded the preview and restarted every playing track — the exact chop the live write during a drag exists to avoid: - releasing a dragged breakpoint, so the audio hitched at the end of every point you moved; - clicking the volume toggle, while the same click on an effect parameter was already silent. Both are quiet now: still persisted, still resyncing the selection so a following edit computes from the value just written. Also fixes the seeded volume. `Number(dataAttributes.volume ?? "1")` is 0 for an attribute that is present but empty, so automating such a track started its lane at silence while the engine read the same empty value as unity. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 0043ae7 commit 28485fe

4 files changed

Lines changed: 50 additions & 17 deletions

File tree

.fallowrc.jsonc

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,13 @@
176176
"withLane",
177177
],
178178
},
179+
// propertyPanelAutomation is the shared reader for both panel sections; the
180+
// FX group that consumes these two lands one PR upstack, so a per-PR audit
181+
// against the merge base sees them as unused.
182+
{
183+
"file": "packages/studio/src/components/editor/propertyPanelAutomation.ts",
184+
"exports": ["automatedTargetsOf", "resolveAutomationRange"],
185+
},
179186
// drawElementService is the bottom of the fast-capture Graphite stack
180187
// (#1917): its consumers (frameCapture in #1919) land two PRs upstack, so
181188
// a per-PR audit diffing against the merge base sees these exports as

packages/studio/src/components/editor/useVolumeAutomation.test.tsx

Lines changed: 28 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,12 @@ import type { DomEditSelection } from "./domEditingTypes";
88
(globalThis as unknown as { IS_REACT_ACT_ENVIRONMENT: boolean }).IS_REACT_ACT_ENVIRONMENT = true;
99

1010
function bind(dataAttributes: Record<string, string>) {
11-
const onSetAttribute = vi.fn();
11+
const onSetAttributeQuiet = vi.fn();
1212
const captured: { current: VolumeAutomationBinding | null } = { current: null };
1313
function Probe() {
1414
captured.current = useVolumeAutomation(
1515
{ dataAttributes } as unknown as DomEditSelection,
16-
onSetAttribute,
16+
onSetAttributeQuiet,
1717
);
1818
return null;
1919
}
@@ -23,7 +23,7 @@ function bind(dataAttributes: Record<string, string>) {
2323
createRoot(host).render(<Probe />);
2424
});
2525
if (!captured.current) throw new Error("hook never ran");
26-
return { binding: captured.current, onSetAttribute };
26+
return { binding: captured.current, onSetAttributeQuiet };
2727
}
2828

2929
const volumeLane = (v: number) =>
@@ -50,29 +50,29 @@ describe("useVolumeAutomation", () => {
5050

5151
it("seeds a new lane at the level the slider already shows", () => {
5252
// Automating a track must not change how loud it is.
53-
const { binding, onSetAttribute } = bind({ volume: "0.55" });
53+
const { binding, onSetAttributeQuiet } = bind({ volume: "0.55" });
5454
act(() => binding.onAutomateVolume());
55-
expect(onSetAttribute).toHaveBeenCalledWith(
55+
expect(onSetAttributeQuiet).toHaveBeenCalledWith(
5656
"data-automation",
5757
JSON.stringify({ version: 1, lanes: [{ target: "volume", points: [{ t: 0, v: 0.55 }] }] }),
5858
);
5959
});
6060

6161
it("treats a missing data-volume as unity", () => {
62-
const { binding, onSetAttribute } = bind({});
62+
const { binding, onSetAttributeQuiet } = bind({});
6363
act(() => binding.onAutomateVolume());
64-
expect(JSON.parse(String(onSetAttribute.mock.calls[0][1])).lanes[0].points[0].v).toBe(1);
64+
expect(JSON.parse(String(onSetAttributeQuiet.mock.calls[0][1])).lanes[0].points[0].v).toBe(1);
6565
});
6666

6767
it("keeps FX lanes when adding the volume one", () => {
6868
const automation = JSON.stringify({
6969
version: 1,
7070
lanes: [{ target: "fx.n1.frequency", points: [{ t: 0, v: 400 }] }],
7171
});
72-
const { binding, onSetAttribute } = bind({ volume: "0.4", automation });
72+
const { binding, onSetAttributeQuiet } = bind({ volume: "0.4", automation });
7373
act(() => binding.onAutomateVolume());
7474
expect(
75-
JSON.parse(String(onSetAttribute.mock.calls[0][1])).lanes.map(
75+
JSON.parse(String(onSetAttributeQuiet.mock.calls[0][1])).lanes.map(
7676
(l: { target: string }) => l.target,
7777
),
7878
).toEqual(["fx.n1.frequency", "volume"]);
@@ -86,22 +86,37 @@ describe("useVolumeAutomation", () => {
8686
{ target: "fx.n1.frequency", points: [{ t: 0, v: 400 }] },
8787
],
8888
});
89-
const { binding, onSetAttribute } = bind({ volume: "0.4", automation });
89+
const { binding, onSetAttributeQuiet } = bind({ volume: "0.4", automation });
9090
act(() => binding.onRemoveVolumeAutomation());
9191
expect(
92-
JSON.parse(String(onSetAttribute.mock.calls[0][1])).lanes.map(
92+
JSON.parse(String(onSetAttributeQuiet.mock.calls[0][1])).lanes.map(
9393
(l: { target: string }) => l.target,
9494
),
9595
).toEqual(["fx.n1.frequency"]);
9696
});
9797

9898
it("clears the attribute when the volume lane was the only one", () => {
99-
const { binding, onSetAttribute } = bind({ volume: "0.4", automation: volumeLane(0.2) });
99+
const { binding, onSetAttributeQuiet } = bind({ volume: "0.4", automation: volumeLane(0.2) });
100100
act(() => binding.onRemoveVolumeAutomation());
101-
expect(onSetAttribute).toHaveBeenCalledWith("data-automation", "");
101+
// Null, not "": the quiet path removes an attribute it is given null for.
102+
expect(onSetAttributeQuiet).toHaveBeenCalledWith("data-automation", null);
102103
});
103104

104105
it("reads an unreadable attribute as no automation", () => {
105106
expect(bind({ volume: "0.55", automation: "{not json" }).binding.volumeAutomated).toBe(false);
106107
});
108+
109+
it("seeds at unity when data-volume is present but empty", () => {
110+
// Number("") is 0, so `?? "1"` alone seeded the lane at silence while the
111+
// engine read the same empty attribute as unity.
112+
const { binding, onSetAttributeQuiet } = bind({ volume: "" });
113+
act(() => binding.onAutomateVolume());
114+
expect(JSON.parse(String(onSetAttributeQuiet.mock.calls[0][1])).lanes[0].points[0].v).toBe(1);
115+
});
116+
117+
it("seeds at unity when data-volume is not a number", () => {
118+
const { binding, onSetAttributeQuiet } = bind({ volume: "loud" });
119+
act(() => binding.onAutomateVolume());
120+
expect(JSON.parse(String(onSetAttributeQuiet.mock.calls[0][1])).lanes[0].points[0].v).toBe(1);
121+
});
107122
});

packages/studio/src/components/editor/useVolumeAutomation.ts

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,15 +24,22 @@ export interface VolumeAutomationBinding {
2424

2525
export function useVolumeAutomation(
2626
element: DomEditSelection,
27-
onSetAttribute: (attr: string, value: string) => void | Promise<void>,
27+
onSetAttributeQuiet: (attr: string, value: string | null) => void | Promise<void>,
2828
): VolumeAutomationBinding {
2929
// The chain is not needed to resolve a volume lane — volume is always a valid
3030
// target — so this deliberately does not parse it.
3131
const automation = readPanelAutomation(element.dataAttributes?.["automation"], undefined);
3232
const write = (next: Parameters<typeof automationAttrValue>[0]): void => {
33-
void onSetAttribute(HF_AUDIO_AUTOMATION_ATTR, automationAttrValue(next));
33+
// Quiet: clicking the toggle used to reload the preview and restart every
34+
// playing track, while the same click on an effect parameter did not.
35+
void onSetAttributeQuiet(HF_AUDIO_AUTOMATION_ATTR, automationAttrValue(next) || null);
3436
};
35-
const current = Number(element.dataAttributes?.["volume"] ?? "1");
37+
// `??` alone would let an empty `data-volume` through as Number("") === 0, so
38+
// automating the track would seed its lane at silence. The engine reads the same
39+
// empty value as unity.
40+
const raw = element.dataAttributes?.["volume"];
41+
const parsed = raw ? Number(raw) : 1;
42+
const current = Number.isFinite(parsed) ? parsed : 1;
3643
return {
3744
volumeAutomated: automation.lanes.some((lane) => lane.target === VOLUME_TARGET),
3845
// Seeded at the level the slider already shows, so automating the track does

packages/studio/src/player/components/useAutomationLanes.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,11 @@ export function useAutomationLanes(): UseAutomationLanesResult {
5858
const write = (next: HfAutomation, persist: boolean): void => {
5959
if (!domEdit || !isSelected) return;
6060
const value = next.lanes.length > 0 ? serializeAutomation(next) : "";
61-
if (persist) void domEdit.handleDomAttributeCommit(HF_AUDIO_AUTOMATION_ATTR, value);
61+
// Quiet, not the refreshing commit: releasing a dragged point used to
62+
// reload the preview, which restarts every playing track — the same chop
63+
// the live write during the drag exists to avoid. Quiet still persists
64+
// and still resyncs the selection, so the next edit sees this one.
65+
if (persist) void domEdit.handleDomAttributeQuietCommit(HF_AUDIO_AUTOMATION_ATTR, value);
6266
// Dragging a point writes live: no preview refresh, so the composition
6367
// does not reload and restart playback on every pixel.
6468
else void domEdit.handleDomAttributeLiveCommit(HF_AUDIO_AUTOMATION_ATTR, value || null);

0 commit comments

Comments
 (0)