Skip to content

Commit 8d2e62d

Browse files
committed
fix(studio): stop the audition reverting itself, and cancel it sideways
Two bugs in the hover-audition that shipped in the commit before this, both invisible to its tests because a static mount never re-renders and never moves the pointer between two entries. **It reverted itself about thirty times a second.** The teardown that puts the chain back was keyed on `onChainPreview`, which the group passes as an inline arrow — and the group re-renders on every playhead tick to move the automation readouts. So React tore the effect down and re-ran it on every tick, and each teardown saw an audition in progress and undid it. The preset was heard for one frame and then silently reverted with the pointer still on the button, during playback, which is the only time there is anything to audition at all. The handler moves into a ref and the effect gets empty deps, so it runs on teardown and at no other time. **Moving to the effect beside it left a measurement in flight.** The levelling audition was only called off by leaving the whole shelf, so sliding from Even Out Levels to Reverb kept the decode running — and when it finished it wrote a levelled version of the chain as it had been, on top of the reverb being auditioned, through the channel the document never sees. Exactly the failure the run counter was added to prevent, one gesture to its left. Every entry in the shelf now calls its neighbours' auditions off: the effects cancel levelling, levelling cancels the chain audition, and Tone cancels both despite having none of its own. The keyboard path was already right — `focusout` bubbles, so a move within the menu fires the shelf's own handler. Only the mouse leaked. Falsified: restoring the dep array fails the re-render test, and dropping the effect buttons' cancel fails the sideways-move one. studio 3685 passing, 18 todo.
1 parent 93457f9 commit 8d2e62d

3 files changed

Lines changed: 124 additions & 22 deletions

File tree

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

Lines changed: 54 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -366,23 +366,21 @@ describe("AudioFxGroup dynamic carve", () => {
366366
);
367367
}
368368

369-
afterEach(() => vi.unstubAllGlobals());
370-
371369
/**
372-
* Hover-auditioning the leveller has to measure before there is anything to
373-
* hear, and measuring a long voiceover takes seconds — by which time the
374-
* pointer has usually moved on. Applying then would put levelling on a track
375-
* nobody asked to level, through a channel that does not persist: audible,
376-
* absent from the document, and gone on the next reload.
370+
* The same voice, but the decode does not finish until it is let go.
371+
*
372+
* Hover-auditioning the leveller is the one path where the result can arrive
373+
* after the author has moved on, so the tests that cover that need to hold the
374+
* decode open across a second gesture.
377375
*/
378-
it("drops a levelling measurement that lands after the pointer has gone", async () => {
376+
function stubGatedDecode(): { release: () => void; decoded: Promise<void> } {
379377
const sampleRate = 48000;
380378
const data = new Float32Array(sampleRate * 4);
381379
for (let i = 0; i < data.length; i++) {
382380
const t = i / sampleRate;
383381
data[i] = t > 1 && t < 3 ? 0.7 * Math.sin(2 * Math.PI * 1000 * t) : 0;
384382
}
385-
let release: (() => void) | null = null;
383+
let release = (): void => {};
386384
const decoded = new Promise<void>((r) => {
387385
release = r;
388386
});
@@ -399,7 +397,29 @@ describe("AudioFxGroup dynamic carve", () => {
399397
}
400398
},
401399
);
400+
return { release: () => release(), decoded };
401+
}
402402

403+
/** Let the held decode finish, and the measurement it feeds after it. */
404+
async function settleDecode(release: () => void, decoded: Promise<void>): Promise<void> {
405+
await act(async () => {
406+
release();
407+
await decoded;
408+
await Promise.resolve();
409+
});
410+
}
411+
412+
afterEach(() => vi.unstubAllGlobals());
413+
414+
/**
415+
* Hover-auditioning the leveller has to measure before there is anything to
416+
* hear, and measuring a long voiceover takes seconds — by which time the
417+
* pointer has usually moved on. Applying then would put levelling on a track
418+
* nobody asked to level, through a channel that does not persist: audible,
419+
* absent from the document, and gone on the next reload.
420+
*/
421+
it("drops a levelling measurement that lands after the pointer has gone", async () => {
422+
const { release, decoded } = stubGatedDecode();
403423
const { host, onSetAttributeLive } = mount({ "fx-chain": CHAIN });
404424
document.getElementById("bed")?.setAttribute("src", "bed.wav");
405425
act(() => byTextButton(host, "Add effect")?.click());
@@ -412,12 +432,7 @@ describe("AudioFxGroup dynamic carve", () => {
412432
.querySelector(".hf-fx-add-menu")
413433
?.dispatchEvent(new FocusEvent("focusout", { bubbles: true }));
414434
});
415-
await act(async () => {
416-
release?.();
417-
await decoded;
418-
// Two turns: the decode resolves, then the measurement it feeds.
419-
await Promise.resolve();
420-
});
435+
await settleDecode(release, decoded);
421436

422437
// The revert on the way out is allowed to write; a levelling stage is not.
423438
const levelled = onSetAttributeLive.mock.calls.filter((c) =>
@@ -426,6 +441,30 @@ describe("AudioFxGroup dynamic carve", () => {
426441
expect(levelled).toEqual([]);
427442
});
428443

444+
/**
445+
* Sliding from the leveller to the effect beside it is not leaving the menu,
446+
* so the shelf's own leave never fires — and the measurement already in flight
447+
* used to land on top of whatever was being auditioned next, writing a
448+
* levelled version of the chain as it was through a channel the document never
449+
* sees. Every entry in the shelf calls its neighbours' auditions off.
450+
*/
451+
it("calls the levelling measurement off when the pointer moves to the effect beside it", async () => {
452+
const { release, decoded } = stubGatedDecode();
453+
const { host, onSetAttributeLive } = mount({ "fx-chain": CHAIN });
454+
document.getElementById("bed")?.setAttribute("src", "bed.wav");
455+
act(() => byTextButton(host, "Add effect")?.click());
456+
act(() => byTextButton(host, "Even Out Levels")?.focus());
457+
// Straight to a neighbour, without ever leaving the shelf.
458+
act(() =>
459+
byTextButton(host, "Reverb")?.dispatchEvent(new MouseEvent("mouseover", { bubbles: true })),
460+
);
461+
await settleDecode(release, decoded);
462+
463+
expect(
464+
onSetAttributeLive.mock.calls.filter((c) => String(c[1] ?? "").includes("fromLeveller")),
465+
).toEqual([]);
466+
});
467+
429468
it("automates the carve filters' gain from the voice, in the bed's own time", async () => {
430469
stubDecode();
431470
// Voice starts 10s into the composition, bed at 0: the envelope is measured

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

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -360,6 +360,34 @@ describe("FxSection chain", () => {
360360
expect(back.nodes.map((n) => n.type)).toEqual(["peaking"]);
361361
});
362362

363+
it("survives the panel re-rendering under it, which playback does constantly", () => {
364+
// The group re-renders on every playhead tick to move the automation
365+
// readouts, handing down a fresh preview callback each time. A teardown
366+
// keyed on that callback ran on every tick, so an audition reverted itself
367+
// about thirty times a second — during playback, which is the only time
368+
// there is anything to audition.
369+
const { host, root, onChainPreview } = mount({ chain: chainOf("peaking") });
370+
click(byText(host, "button", "Presets"));
371+
enter(presetButton(host, "telephone"));
372+
const auditions = onChainPreview.mock.calls.length;
373+
374+
// Same behaviour, new identity — exactly what a tick hands down.
375+
act(() =>
376+
root.render(
377+
<FxSection
378+
chain={chainOf("peaking")}
379+
onChainChange={vi.fn()}
380+
onChainPreview={(next) => onChainPreview(next)}
381+
carve={null}
382+
onCarveChange={vi.fn()}
383+
sourceOptions={[{ id: "vo", label: "Voiceover" }]}
384+
/>,
385+
),
386+
);
387+
388+
expect(onChainPreview.mock.calls.length).toBe(auditions);
389+
});
390+
363391
it("auditions an effect the add menu is offering", () => {
364392
const { host, onChainPreview, onChainChange } = mount({ chain: chainOf("peaking") });
365393
click(byText(host, "button", "Add effect"));

packages/studio/src/components/editor/propertyPanelFxSection.tsx

Lines changed: 42 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -184,14 +184,28 @@ export function FxSection({
184184
[chain, onChainPreview],
185185
);
186186

187+
/**
188+
* The preview handler as of the last render, held rather than closed over.
189+
*
190+
* The teardown below must run on teardown and at no other time, so its deps
191+
* have to be empty — and `onChainPreview` is an inline arrow in the group,
192+
* which re-renders on every playhead tick to move the automation readouts. A
193+
* dep on it made React tear down and re-run the effect on every one of those
194+
* ticks, so an audition reverted itself about 30 times a second while the
195+
* pointer was still on the button: the preset was heard for a frame during
196+
* playback, which is the exact case the whole affordance exists for.
197+
*/
198+
const previewRef = useRef(onChainPreview);
199+
previewRef.current = onChainPreview;
200+
187201
// Leaving by any route other than the pointer — the element deselected, the
188202
// panel closed — would otherwise leave the audition playing over a chain the
189203
// document does not have.
190204
useEffect(
191205
() => () => {
192-
if (auditionBase.current) onChainPreview?.(auditionBase.current);
206+
if (auditionBase.current) previewRef.current?.(auditionBase.current);
193207
},
194-
[onChainPreview],
208+
[],
195209
);
196210

197211
const applyPreset = useCallback(
@@ -426,7 +440,14 @@ export function FxSection({
426440
// hear. So it says it is working rather than doing nothing
427441
// visible, and whoever handles this must drop a result that
428442
// arrives after the pointer has gone.
429-
onMouseEnter={levelled ? undefined : () => onAuditionLevel?.(true)}
443+
onMouseEnter={
444+
levelled
445+
? undefined
446+
: () => {
447+
audition(null);
448+
onAuditionLevel?.(true);
449+
}
450+
}
430451
onFocus={levelled ? undefined : () => onAuditionLevel?.(true)}
431452
>
432453
{levelled ? "Remove levelling" : "Even Out Levels"}
@@ -440,9 +461,14 @@ export function FxSection({
440461
// must not include it.
441462
className="hf-fx-add-composite rounded-[3px] bg-panel-surface px-1.5 py-0.5 text-[10px] text-panel-text-1 hover:text-panel-text-0"
442463
title="Bass, middle and treble on one set of faders."
443-
// No audition: a Tone module arrives with every band at 0 dB, so
444-
// there is nothing to hear until a fader moves. A hover that
445-
// changes nothing teaches that hovering does nothing.
464+
// No audition of its own: a Tone module arrives with every band at
465+
// 0 dB, so there is nothing to hear until a fader moves, and a
466+
// hover that changes nothing teaches that hovering does nothing.
467+
// It still has to call the neighbours' auditions off.
468+
onMouseEnter={() => {
469+
audition(null);
470+
onAuditionLevel?.(false);
471+
}}
446472
onClick={addEq}
447473
>
448474
Tone (EQ)
@@ -460,7 +486,16 @@ export function FxSection({
460486
className="hf-fx-add-item rounded-[3px] bg-panel-surface px-1.5 py-0.5 text-[10px] text-panel-text-1 hover:text-panel-text-0"
461487
title={d.description}
462488
onClick={() => addEffect(d.id)}
463-
onMouseEnter={() => audition((base) => withEffect(base, d.id))}
489+
// Cancels the levelling audition as well as starting its own.
490+
// The shelf's leave handler only fires on the way OUT of the
491+
// menu, so sliding from Even Out Levels straight to here left a
492+
// measurement in flight — and it landed on top of this one, a
493+
// levelled version of the chain as it was, written through a
494+
// channel the document never sees.
495+
onMouseEnter={() => {
496+
onAuditionLevel?.(false);
497+
audition((base) => withEffect(base, d.id));
498+
}}
464499
onFocus={() => audition((base) => withEffect(base, d.id))}
465500
>
466501
{d.label}

0 commit comments

Comments
 (0)