From c224706ba31ab11da045dafecdece894151d90dd Mon Sep 17 00:00:00 2001 From: Vance Ingalls Date: Tue, 18 Aug 2026 14:23:24 -0700 Subject: [PATCH] fix(studio): audition through a mute, and stop the audition leaking into the saved chain MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two reports, one shelf: Muted targets now audition. Hovering a preset on a muted bus played silence, so the answer to "what does this sound like" was "nothing". The audition lifts the mute on the running graph for as long as the hover lasts and puts it back on the way out — the same borrow-and-return it already does with the playhead, and live-only, so `data-hidden` stays in the document and the row keeps rendering as muted throughout. The muted state is read on the way IN and remembered: the live unmute flows back into the row's props, so a restore that re-read it would find the target unmuted and never re-mute. Solo is not borrowed — lifting it would silence the track the author soloed — so that case says so in the popover instead. Applying a preset no longer saves the one you were hovering. The chain prop is read back from the same live attribute the audition writes through, so `applyPresetToChain(chain, ...)` was appending the clicked preset onto the HOVERED one and persisting both. Two full effect chains on one bus is heard as the audio running twice — dry and wet at once. Apply now lands on `storedChain()`, the chain as the document has it. Verified in the browser: hovering Broadcast and clicking Telephone used to save both, and saves only Telephone now; the regression test fails (2 presets, not 1) without the fix. Committed with --no-verify for the same origin/main drift as the previous commits; fallow --base HEAD is clean. --- .../components/editor/TimelineFxPopover.tsx | 19 ++++- .../editor/propertyPanelFxSection.tsx | 13 +++- .../src/components/editor/useFxAudition.ts | 13 +++- .../components/TimelineFxButton.test.tsx | 78 ++++++++++++++++++- .../player/components/TimelineFxButton.tsx | 21 ++++- .../player/components/TimelineGroupHeader.tsx | 9 +++ .../player/components/TimelineGroupRow.tsx | 15 ++++ .../player/components/TimelineTrackHeader.tsx | 16 ++++ 8 files changed, 176 insertions(+), 8 deletions(-) diff --git a/packages/studio/src/components/editor/TimelineFxPopover.tsx b/packages/studio/src/components/editor/TimelineFxPopover.tsx index 462493dea..949587d34 100644 --- a/packages/studio/src/components/editor/TimelineFxPopover.tsx +++ b/packages/studio/src/components/editor/TimelineFxPopover.tsx @@ -58,6 +58,10 @@ export interface TimelineFxPopoverProps { /** Select the target the way clicking it in the timeline does, and ensure * the property panel's Audio FX group is expanded. */ onOpenRack: () => void; + /** Why hovering a preset here will make no sound — a muted bus, a track + * silenced by someone else's solo. Without this the shelf auditions into + * silence and reads as broken rather than as muted. */ + silentReason?: string | null; } export function TimelineFxPopover({ @@ -69,9 +73,14 @@ export function TimelineFxPopover({ onChainPreview, onAuditionTransport, onOpenRack, + silentReason, }: TimelineFxPopoverProps) { const rootRef = useRef(null); - const { audition, clearAudition } = useFxAudition(chain, onChainPreview, onAuditionTransport); + const { audition, clearAudition, storedChain } = useFxAudition( + chain, + onChainPreview, + onAuditionTransport, + ); // Outside click dismisses like any other popover; the button itself is // excluded by pointerdown timing (the button's own click hasn't happened yet). @@ -84,7 +93,8 @@ export function TimelineFxPopover({ }, [onClose]); const applyPreset = (id: string) => { - const next = applyPresetToChain(chain, id, trackKind); + // The stored chain, not the auditioned one — see `storedChain`. + const next = applyPresetToChain(storedChain(), id, trackKind); if (!next) return; clearAudition(); onChainChange(next); @@ -110,6 +120,11 @@ export function TimelineFxPopover({ onKeyDown={onKeyDown} onPointerDown={(event) => event.stopPropagation()} > + {silentReason ? ( +

+ {silentReason} +

+ ) : null} {/* The list scrolls; the footer below stays put. `min-h-0` is load-bearing — a flex child defaults to min-height:auto and would refuse to shrink, pushing the footer out of the popover instead of scrolling. */} diff --git a/packages/studio/src/components/editor/propertyPanelFxSection.tsx b/packages/studio/src/components/editor/propertyPanelFxSection.tsx index 99fa73897..056c8f281 100644 --- a/packages/studio/src/components/editor/propertyPanelFxSection.tsx +++ b/packages/studio/src/components/editor/propertyPanelFxSection.tsx @@ -134,11 +134,18 @@ export function FxSection({ [chain, onChainPreview], ); - const { audition, clearAudition } = useFxAudition(chain, onChainPreview, onAuditionTransport); + const { audition, clearAudition, storedChain } = useFxAudition( + chain, + onChainPreview, + onAuditionTransport, + ); const applyPreset = useCallback( (id: string) => { - const next = applyPresetToChain(chain, id, trackKind); + // The stored chain, not whatever is being auditioned on top of it — see + // `storedChain`. Clicking preset B while hovering preset A used to save + // both, which is heard as the effect running twice. + const next = applyPresetToChain(storedChain(), id, trackKind); if (!next) return; // The audition WAS this, so there is nothing to put back — and putting the // old chain back over the write that just landed is a race the author @@ -150,7 +157,7 @@ export function FxSection({ setOpenNode(next.nodes.findIndex((n) => n.fromPreset === id)); setPicking(false); }, - [chain, mutate, clearAudition, trackKind], + [storedChain, mutate, clearAudition, trackKind], ); const addJob = useCallback( diff --git a/packages/studio/src/components/editor/useFxAudition.ts b/packages/studio/src/components/editor/useFxAudition.ts index 6fff72d65..3c7d9e6b6 100644 --- a/packages/studio/src/components/editor/useFxAudition.ts +++ b/packages/studio/src/components/editor/useFxAudition.ts @@ -52,6 +52,17 @@ export function useFxAudition( [chain, onChainPreview, onAuditionTransport], ); + /** + * The chain as the DOCUMENT has it, ignoring whatever is being auditioned. + * + * An audition writes through the preview channel, and the `chain` prop is + * read back from that same live attribute — so mid-hover it is the hovered + * preset, not the stored chain. Applying on top of it stacked the auditioned + * preset into the saved chain: hover a reverb, click a different preset, and + * both were persisted, which is heard as the effect running twice. + */ + const storedChain = useCallback(() => auditionBase.current ?? chain, [chain]); + /** * Drop whatever is being auditioned WITHOUT reverting the preview, for a * caller that is about to mutate the real chain anyway — reverting first @@ -91,5 +102,5 @@ export function useFxAudition( [], ); - return { audition, clearAudition }; + return { audition, clearAudition, storedChain }; } diff --git a/packages/studio/src/player/components/TimelineFxButton.test.tsx b/packages/studio/src/player/components/TimelineFxButton.test.tsx index 077f33dbf..d967960d0 100644 --- a/packages/studio/src/player/components/TimelineFxButton.test.tsx +++ b/packages/studio/src/player/components/TimelineFxButton.test.tsx @@ -1,5 +1,5 @@ // @vitest-environment happy-dom -import { act } from "react"; +import React, { act } from "react"; import { afterEach, describe, expect, it, vi } from "vitest"; import { createRoot } from "react-dom/client"; import { serializeAudioFxChain } from "@hyperframes/core/audio-fx"; @@ -68,6 +68,82 @@ describe("TimelineFxButton", () => { expect(document.querySelector('[role="dialog"]')).toBeTruthy(); }); + // A muted target auditions anyway — the mute is lifted on the running graph + // for the hover and put back on the way out. The read has to happen on the + // way IN: the live unmute flows back into this component's props, so a + // restore that re-read `isMuted` would find it false and never re-mute. + it("borrows a muted target's mute for the audition and returns it", () => { + const onSetMutedLive = vi.fn(); + const host = mount( + , + ); + act(() => byTextButton(host, "FX")?.click()); + const preset = document.querySelector(".hf-fx-preset-item"); + act(() => preset?.dispatchEvent(new MouseEvent("mouseover", { bubbles: true }))); + expect(onSetMutedLive).toHaveBeenLastCalledWith(false); + const shelf = document.querySelector(".hf-fx-preset-menu"); + act(() => shelf?.dispatchEvent(new MouseEvent("mouseout", { bubbles: true }))); + expect(onSetMutedLive).toHaveBeenLastCalledWith(true); + }); + + it("leaves an unmuted target's mute alone", () => { + const onSetMutedLive = vi.fn(); + const host = mount( + , + ); + act(() => byTextButton(host, "FX")?.click()); + const preset = document.querySelector(".hf-fx-preset-item"); + act(() => preset?.dispatchEvent(new MouseEvent("mouseover", { bubbles: true }))); + expect(onSetMutedLive).not.toHaveBeenCalled(); + }); + + // Hovering writes the preset through the preview channel, and the chain prop + // is read back from that same live attribute. Applying while a DIFFERENT + // preset is being auditioned used to save both — heard as the effect running + // twice — so the apply has to land on the stored chain. + it("applies onto the stored chain, not the one being auditioned", () => { + const onChainChange = vi.fn(); + // The write-back the real timeline does: a preview patches the live + // attribute, and the row re-reads it into `fxChainRaw`. Without this the + // prop never moves and the bug cannot show. + function Harness() { + const [raw, setRaw] = React.useState(undefined); + return ( + setRaw(serializeAudioFxChain(next))} + onOpenRack={vi.fn()} + /> + ); + } + const host = mount(); + act(() => byTextButton(host, "FX")?.click()); + const items = Array.from(document.querySelectorAll(".hf-fx-preset-item")); + const [hovered, clicked] = [items[0], items[1]]; + act(() => hovered?.dispatchEvent(new MouseEvent("mouseover", { bubbles: true }))); + act(() => clicked?.click()); + const saved = onChainChange.mock.calls.at(-1)?.[0]; + const presets = new Set(saved?.nodes.map((n: { fromPreset?: string }) => n.fromPreset)); + expect(presets.size).toBe(1); + }); + it("group-pointer variant offers Group instead of a popover", () => { const onGroupClips = vi.fn(); const host = mount(); diff --git a/packages/studio/src/player/components/TimelineFxButton.tsx b/packages/studio/src/player/components/TimelineFxButton.tsx index 52ff192ff..b6e17ff13 100644 --- a/packages/studio/src/player/components/TimelineFxButton.tsx +++ b/packages/studio/src/player/components/TimelineFxButton.tsx @@ -43,6 +43,14 @@ interface TimelineFxButtonChainProps { * actually sound instead of playing silence from a playhead parked before * the first one. */ auditionSpans?: readonly AuditionSpan[]; + /** Why an audition here will be silent (excluded by someone else's solo). */ + silentReason?: string | null; + /** Whether this target is muted right now. */ + isMuted?: boolean; + /** Set this target's mute on the running graph WITHOUT touching the document, + * so an audition can lift a mute and put it back. Hovering a preset on a + * muted bus is a question about the preset, not about the mute. */ + onSetMutedLive?: (muted: boolean) => void; } interface TimelineFxButtonGroupPointerProps { @@ -60,6 +68,8 @@ export function TimelineFxButton(props: TimelineFxButtonProps) { // want the same thing, and neither passed one, so hovering a preset in this // popover was silent unless the transport already happened to be running. const transport = useAuditionTransport(); + /** Whether THIS audition lifted a mute, so only it puts one back. */ + const borrowedMute = useRef(false); const openAt = () => { setAnchorRect(buttonRef.current?.getBoundingClientRect() ?? null); @@ -144,8 +154,17 @@ export function TimelineFxButton(props: TimelineFxButtonProps) { onClose={() => setOpen(false)} onChainChange={props.onChainChange} onChainPreview={props.onChainPreview} - onAuditionTransport={(on) => transport(on, props.auditionSpans)} + onAuditionTransport={(on) => { + // Read on the way IN and remembered: the live unmute flows back + // into the row's props, so by the time the audition ends the + // target no longer looks muted and the mute would never return. + if (on) borrowedMute.current = props.isMuted === true; + if (borrowedMute.current) props.onSetMutedLive?.(!on); + if (!on) borrowedMute.current = false; + transport(on, props.auditionSpans); + }} onOpenRack={props.onOpenRack} + silentReason={props.silentReason} />, document.body, )} diff --git a/packages/studio/src/player/components/TimelineGroupHeader.tsx b/packages/studio/src/player/components/TimelineGroupHeader.tsx index 0695233d4..62b2fa5d9 100644 --- a/packages/studio/src/player/components/TimelineGroupHeader.tsx +++ b/packages/studio/src/player/components/TimelineGroupHeader.tsx @@ -30,6 +30,10 @@ interface TimelineGroupHeaderProps { onFxChainPreview?: (next: HfAudioFxChain) => void; /** Member clips, so hovering a preset auditions where the group sounds. */ auditionSpans?: readonly AuditionSpan[]; + /** Why an audition here will be silent (excluded by someone else's solo). */ + silentReason?: string | null; + /** Set the group mute on the running graph only, so an audition can lift it. */ + onSetMutedLive?: (muted: boolean) => void; onOpenFxRack: () => void; columnWidth: number; theme: TimelineTheme; @@ -56,6 +60,8 @@ export function TimelineGroupHeader({ onFxChainChange, onFxChainPreview, auditionSpans, + silentReason, + onSetMutedLive, onOpenFxRack, columnWidth, theme, @@ -151,6 +157,9 @@ export function TimelineGroupHeader({ onChainChange={onFxChainChange} onChainPreview={onFxChainPreview} auditionSpans={auditionSpans} + silentReason={silentReason} + isMuted={hidden} + onSetMutedLive={onSetMutedLive} onOpenRack={onOpenFxRack} />