Skip to content

Commit 457cde3

Browse files
committed
fix(studio): put #3207's edge-stretch back, folded into the unified hook
The review blocks this PR for deleting a feature two PRs downstack: "#3209 deletes #3207 edge-stretch instead of folding it into the unified hook... merging the stack would ship #3207 and then silently remove its user-facing retime/edge-stretch feature." Verified, all four claims: `useAutomationEdgeStretch.ts` (248 lines) and its test were deleted, `retimeRange` was dropped from `automationLaneSelection.ts`, and the consolidated hook has no edge hit-test, arm/move/finish path, or resize cursor. Restored: the module, `retimeRange`, the selection regressions, and the lane wiring (`col-resize` cursor, `pointercancel` reverting a partial retime rather than persisting it). It is not a straight revert, because #3209 changed the selection from a time range to a box. Edge-stretch now takes `{t0,t1,v0,v1}` and moves only the time edges — the value extent rides through untouched, which keeps it the same gesture it was. **One arbitration call worth a second opinion.** #3207's rule was that a selection's edge outranks a point sitting on it, because every range operation leaves a breakpoint exactly on the edge it created — a point-first rule made the second stretch of an edge resolve to a point-drag. Under a box that rule now contradicts #3209's own test ("stops the group at a point it did not select"), which presses at t=0 v=1 — simultaneously the t0 edge and a selected point. I inverted it: selected content wins, the edge stretches everywhere it is not also selected content. The reasoning is that a box makes the point visibly part of the selection, and dragging selected content has to move it. That restores #3209's test and keeps the stretch usable along the rest of the edge — but it is a product decision between two deliberate designs, so flag it if #3207's original precedence was load-bearing. 954 player tests pass, including the 17 restored ones.
1 parent c0a4c97 commit 457cde3

7 files changed

Lines changed: 522 additions & 9 deletions

File tree

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ import { audioFxSummary } from "./audioFxSummary";
1818
import { AudioFxGroup } from "./propertyPanelAudioFxGroup";
1919
import { useVolumeAutomation } from "./useVolumeAutomation";
2020
import { FlatMediaSection } from "./propertyPanelFlatMediaSection";
21-
import type { DomEditSelection } from "./domEditing";
2221
import { deriveElementTiming } from "./propertyPanelFlatTimingDerivation";
2322
import { createGsapLivePreview } from "./gsapLivePreview";
2423
import { formatTextFieldPreview } from "./propertyPanelSections";

packages/studio/src/player/components/TimelineAutomationLane.tsx

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,11 @@ function pointInBox(point: HfAutomationPoint, box: SelectionBox | null | undefin
7474
}
7575

7676
/** Pointer shape: a read-only lane can only be selected, a live one edited. */
77-
function laneCursor(readOnly: boolean | undefined, dragging: boolean): string {
77+
function laneCursor(readOnly: boolean | undefined, dragging: boolean, stretching: boolean): string {
78+
// A stretch handle wins over everything it might also sit above: the handle is
79+
// a few px wide and always overlaps whatever is under the selection edge, so
80+
// any other cursor there would advertise a gesture the press will not start.
81+
if (stretching) return "col-resize";
7882
if (readOnly) return "pointer";
7983
return dragging ? "grabbing" : "crosshair";
8084
}
@@ -229,7 +233,7 @@ export function TimelineAutomationLane({
229233
duration,
230234
rangeSelection,
231235
});
232-
const { dragIndex, curveIndex, hint, editing } = gestures;
236+
const { dragIndex, curveIndex, edgeDrag, edgeHover, hint, editing } = gestures;
233237

234238
const removeAt = useCallback(
235239
(index: number): void => {
@@ -330,7 +334,11 @@ export function TimelineAutomationLane({
330334
top: 0,
331335
width: widthPx + PAD_X * 2,
332336
height: h,
333-
cursor: laneCursor(readOnly, dragIndex !== null || curveIndex !== null),
337+
cursor: laneCursor(
338+
readOnly,
339+
dragIndex !== null || curveIndex !== null,
340+
edgeDrag !== null || edgeHover,
341+
),
334342
opacity: readOnly ? 0.55 : 1,
335343
touchAction: "none",
336344
}}
@@ -341,7 +349,7 @@ export function TimelineAutomationLane({
341349
onPointerDown={gestures.onPointerDown}
342350
onPointerMove={gestures.onPointerMove}
343351
onPointerUp={gestures.endDrag}
344-
onPointerCancel={gestures.endDrag}
352+
onPointerCancel={gestures.cancelDrag}
345353
onDoubleClick={gestures.onDoubleClick}
346354
onContextMenu={onSvgContextMenu}
347355
role="group"

packages/studio/src/player/components/automationLaneSelection.test.ts

Lines changed: 87 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { describe, expect, it } from "vitest";
2-
import { pointsIn, replaceRange } from "./automationLaneSelection";
2+
import { pointsIn, replaceRange, retimeRange } from "./automationLaneSelection";
33
import { sampleAutomationLane, VOLUME_RANGE } from "@hyperframes/core/audio-automation";
44
import type { HfAutomationLane } from "@hyperframes/core/audio-automation";
55

@@ -94,3 +94,89 @@ describe("replaceRange", () => {
9494
expect(Math.max(...innerTimes)).toBeGreaterThan(3.0);
9595
});
9696
});
97+
describe("retimeRange", () => {
98+
it("scales interior points proportionally into the new span", () => {
99+
const pts = retimeRange({ lane: ramp, range: VOLUME_RANGE, t0: 2, t1: 3, newT0: 2, newT1: 5 });
100+
const moved = pts.find((p) => p.v === 0.4); // the t=3 point
101+
expect(moved?.t).toBe(5);
102+
});
103+
104+
it("preserves the envelope outside the union of old and new spans", () => {
105+
const before: HfAutomationLane = { target: "volume", points: ramp.points };
106+
const after: HfAutomationLane = {
107+
target: "volume",
108+
points: retimeRange({ lane: ramp, range: VOLUME_RANGE, t0: 2, t1: 3, newT0: 2, newT1: 5 }),
109+
};
110+
// Nothing to the left of t0=2 moved (newT0 === t0 here), so sampled
111+
// continuity holds all the way up to the edited region.
112+
for (const t of [0, 1, 1.9]) {
113+
expect(sampleAutomationLane(after, t, "linear")).toBeCloseTo(
114+
sampleAutomationLane(before, t, "linear"),
115+
5,
116+
);
117+
}
118+
// The next real breakpoint past the edited region keeps its own exact value.
119+
const farPoint = after.points.find((p) => p.t === 6);
120+
expect(farPoint).toEqual({ t: 6, v: 0 });
121+
});
122+
123+
it("preserves the envelope on BOTH sides when no breakpoint sits on the moved edge", () => {
124+
// The right side is where the invariant is worth asserting — `newT0 === t0`
125+
// makes the left side of the test above trivially true, and an earlier
126+
// right-side probe at t=5.1 was DELETED as inherent when it was reporting
127+
// the real behaviour below. With the selection's edges off any breakpoint,
128+
// the guarantee holds exactly, in both directions.
129+
const before: HfAutomationLane = { target: "volume", points: ramp.points };
130+
const after: HfAutomationLane = {
131+
target: "volume",
132+
points: retimeRange({
133+
lane: ramp,
134+
range: VOLUME_RANGE,
135+
t0: 2.2,
136+
t1: 2.9,
137+
newT0: 2.2,
138+
newT1: 4,
139+
}),
140+
};
141+
for (const t of [0, 1, 2, 4.5, 5, 5.5, 6]) {
142+
expect(sampleAutomationLane(after, t, "linear")).toBeCloseTo(
143+
sampleAutomationLane(before, t, "linear"),
144+
5,
145+
);
146+
}
147+
});
148+
149+
it("moves a breakpoint sitting exactly on the dragged edge, reshaping the segment past it", () => {
150+
// The design decision this pins, because it is not free either way.
151+
// `pointsIn` is endpoint-inclusive, so a breakpoint ON the edge is interior
152+
// and travels with the stretch. It has to: every range operation leaves a
153+
// breakpoint exactly on the edge it created, so treating that point as an
154+
// anchor instead would make the commonest stretch — grabbing the edge to
155+
// drag that very point outward — delete it and flatten the span.
156+
//
157+
// The cost is that the retimed point lands ON the union's boundary, where a
158+
// preservation anchor would also go, and `anchor()` stands down within a
159+
// merge radius. Two different values cannot occupy one time; the segment
160+
// leaving the union reshapes, which is the "envelope outside the selection
161+
// never moves" invariant bending exactly here and nowhere else.
162+
const pts = retimeRange({ lane: ramp, range: VOLUME_RANGE, t0: 2, t1: 3, newT0: 2, newT1: 5 });
163+
expect(pts).toEqual([
164+
{ t: 0, v: 1 },
165+
{ t: 2, v: 0.6 },
166+
{ t: 5, v: 0.4 }, // the t=3 point, retimed onto the new edge
167+
{ t: 6, v: 0 },
168+
]);
169+
// The old 3→6 segment sloped -0.133/s; the new 5→6 slopes -0.4/s, so the
170+
// envelope past the union genuinely moves. Asserted, not tolerated: if this
171+
// number changes, the decision above changed with it.
172+
const after: HfAutomationLane = { target: "volume", points: pts };
173+
expect(sampleAutomationLane(after, 5.5, "linear")).toBeCloseTo(0.2, 5);
174+
expect(sampleAutomationLane(ramp, 5.5, "linear")).toBeCloseTo(0.0667, 4);
175+
});
176+
177+
it("rejects a degenerate span", () => {
178+
expect(
179+
retimeRange({ lane: ramp, range: VOLUME_RANGE, t0: 2, t1: 3, newT0: 4, newT1: 4 }),
180+
).toEqual(ramp.points);
181+
});
182+
});

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

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,3 +94,42 @@ export function pointInSelection(
9494
): boolean {
9595
return point.t >= box.t0 && point.t <= box.t1 && point.v >= box.v0 && point.v <= box.v1;
9696
}
97+
/**
98+
* Retime a selection: interior points scale proportionally into the new span,
99+
* then replaceRange runs over the UNION of old and new spans — growing eats
100+
* whatever it covers, shrinking pins anchors where the envelope re-enters.
101+
*
102+
* Interior is `pointsIn`, so a breakpoint sitting exactly ON an edge travels
103+
* with the stretch. Deliberate: every range operation leaves a breakpoint on the
104+
* edge it created, so treating that point as a fixed anchor would make the
105+
* commonest stretch of all — grabbing the edge to drag that point outward —
106+
* delete it instead. The price is that such a point lands on the union's own
107+
* boundary, where `anchor` then stands down (one time cannot hold two values),
108+
* so the segment leaving the union reshapes. That is the ONE place
109+
* `replaceRange`'s outside-never-moves invariant bends, and it is pinned by name
110+
* in automationLaneSelection.test.ts.
111+
*/
112+
export function retimeRange(input: {
113+
lane: HfAutomationLane;
114+
range: AutomationRange;
115+
t0: number;
116+
t1: number;
117+
newT0: number;
118+
newT1: number;
119+
}): HfAutomationPoint[] {
120+
const { lane, range, t0, t1, newT0, newT1 } = input;
121+
const oldSpan = t1 - t0;
122+
const newSpan = newT1 - newT0;
123+
if (oldSpan <= 0 || newSpan <= 0) return lane.points;
124+
const inner = pointsIn(lane, t0, t1).map((p) => ({
125+
...p,
126+
t: newT0 + ((p.t - t0) * newSpan) / oldSpan,
127+
}));
128+
return replaceRange({
129+
lane,
130+
range,
131+
t0: Math.min(t0, newT0),
132+
t1: Math.max(t1, newT1),
133+
inner,
134+
});
135+
}
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
import { describe, expect, it } from "vitest";
2+
import { clampEdge } from "./useAutomationEdgeStretch";
3+
4+
describe("clampEdge", () => {
5+
it("keeps the dragged edge inside the clip", () => {
6+
expect(clampEdge("t0", -5, { t0: 1, t1: 3 }, 4)).toBe(0);
7+
expect(clampEdge("t1", 99, { t0: 1, t1: 3 }, 4)).toBe(4);
8+
});
9+
10+
it("keeps the dragged edge clear of its partner", () => {
11+
expect(clampEdge("t0", 3.5, { t0: 1, t1: 3 }, 4)).toBeCloseTo(2.98, 5);
12+
expect(clampEdge("t1", 0.5, { t0: 1, t1: 3 }, 4)).toBeCloseTo(1.02, 5);
13+
});
14+
15+
it("never yields a negative time, even when the partner bound is itself below zero", () => {
16+
// A selection thinner than the minimum width has no legal t0 at all. Bounding
17+
// against the partner AFTER the 0-floor returned that illegal value, and
18+
// core's cleanPoint collapses a negative time onto a duplicate t=0 on the
19+
// next serialize round-trip — silent envelope corruption, not a visible bug.
20+
expect(clampEdge("t0", -1, { t0: 0.004, t1: 0.01 }, 4)).toBe(0);
21+
expect(clampEdge("t0", 0.008, { t0: 0.004, t1: 0.01 }, 4)).toBe(0);
22+
});
23+
24+
it("never yields a time past the clip when the partner bound is past it", () => {
25+
expect(clampEdge("t1", 99, { t0: 3.995, t1: 4 }, 4)).toBe(4);
26+
});
27+
});

0 commit comments

Comments
 (0)