Take management should be per clip instance — deleting a take from one
half of a comped split shouldn't pull it out from under the other half.
That's cleanest if the takes themselves live on the instance rather than
the clip, so they do now.
AudioClipType::TakeFolder is gone entirely. A clip is plain Sampled/Midi
content again, and an instance with a non-empty `takes` list simply
OVERRIDES it with whichever take is active. Splitting already clones the
instance, so each half gets its own take list for free — no index
remapping across instances, no copy-on-write, no shared-state surprise —
and comping still works, because the halves can still each select a
different take. It collapsed machinery too: resolve() moved from the clip
to the instance, and owns_audio_pool_index went back to a one-liner.
Management (DeleteTakeAction, DeleteUnusedTakesAction, RenameTakeAction):
- Right-click a clip with more than one take: `Delete "<active take>"`
and "Delete Unused Takes". Deletion is named after the take that's
PLAYING rather than being a generic entry, so you pick the victim by
selecting it — one clear act instead of hunting a small trash icon in a
list (which is where this started, and it was fiddly).
- Double-click a take in the dropdown to rename it in place.
- What happens to the selection on delete is the subtle part, and there's
a test per case: deleting a take BELOW the active one shifts the
selection down so you keep hearing the same take; deleting the ACTIVE
take lands on whatever slid into its place (not silently back to take
1); deleting the LAST take steps back one. The only take can't be
deleted at all — the menu item isn't offered.
- Deleted takes' audio stays in the pool: undo has to put it back, and
the other half of a split may still be playing it.
Fixes:
- A recording that stopped before the loop came round wasn't joining an
existing take list — it landed as a separate overlapping clip. Trigger-
on-wrap is right for the FIRST recording, but once takes exist there,
a further run is plainly another take however short. The engine can't
know that (it's document state), so the editor passes `force_takes`
with the start-recording command and the run is cut and padded to the
region even with zero wraps. This forced cycle_loop_len and `wrapped`
apart on the MIDI side: the region length has to be known from the
start, but the clip should only pin to full-region length AFTER a pass
completes, or the bar jumps to full width the moment you hit record.
- Recording a second take left BOTH sounding. append_cycle_takes tore
down the recording's backend clip by looking it up in
clip_instance_to_backend_map — but on the audio path the recording
instance isn't in that map yet; it's only added during promotion, which
the append path skips. The event already carries the engine's clip id,
so it's handed over explicitly now.
- The take badge is hidden when there's only one take — no choice to make.
Three bugs in a row came from the same root: a time value crossing an API
boundary as a bare f64, with the caller and the callee disagreeing about
whether it meant seconds or beats. Recording landed at the wrong time,
MIDI clips grew too fast, and a 1-second split played back as half a
second. Each was "obviously" one domain at the call site and read as the
other on the far side. This makes the mismatch a compile error.
Backend API — every time-carrying f64 is gone:
- Commands: Seek/SetOffset/SetTrimStart/SetTrimEnd -> Seconds; MoveClip/
ExtendClip/CreateMidiClip/AddMidiNote/AddLoadedMidiClip/
UpdateMidiClipNotes/AddMidiClipSync and all four automation commands ->
Beats; TrimClip -> TrimRange.
- Events/queries: PlaybackPosition, WaveformChunksReady's time range,
AudioFileReady::duration, PoolFileInfo, get_playhead_seconds -> Seconds.
- Serialized: MidiClipData::duration and AutomationKeyframeData::time ->
Beats. Both newtypes are #[serde(transparent)], so the .beam on-disk
format is unchanged.
- Several controller methods ALREADY took Beats and unwrapped it to shove
into the command — the newtype was being discarded at the very boundary
it existed to protect.
TrimRange, for the domain-polymorphic case: a clip's content time is
SECONDS for sampled audio but BEATS for MIDI, so a single newtype can't
express it (there was even a comment in engine.rs saying so, and that
rationalization is what let the bug through). A domain-tagged enum can.
The engine rejects a range whose domain doesn't match the track, and the
range is built from the clip (clip.trim_range()) so callers can't pick
the wrong variant.
ContentTime, for the trim fields: ClipInstance::trim_start/trim_end are
content times, and were the last untyped f64 — the actual root of the
split bug. ContentTime is deliberately a DEAD END: no .to_seconds(), no
.to_beats(), no arithmetic with Seconds or Beats. Content times combine
freely with each other (same clip, same domain — safe), so the ~100
passthrough sites cost nothing; the only exit is resolving against the
clip that knows the domain (AudioClip::resolve_content_time /
Document::resolve_content_time / ClipDuration::same_domain). Mixing
domains no longer compiles.
Two more live bugs the types surfaced:
- ClipInstance::effective_duration_beats took a SECONDS clip duration and
subtracted trim_start from it. For a TRIMMED MIDI clip that subtracted
a beats offset from a seconds duration, so the clip's timeline length
was wrong at any tempo but 60 BPM. Untrimmed clips happened to work,
which is why it hid. It now takes a ClipDuration and resolves in the
clip's own domain: beats content carries over directly (tempo-
invariant), wall-clock content converts at the clip's position.
Regression test asserts a clip trimmed to beats 2..6 is 4 beats long at
60/90/120 BPM.
- Trim validation clamped a content-domain trim against a wall-clock gap.
gap_to_content/content_to_secs now convert at the clip's position.
Also folds two more copies of the backend add-logic into
BackendContext::add_clip_instance (split and remove_clip_instances both
re-add clips), so the trim/duration conversions live in exactly one place
instead of four.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Recording into a cycle region now produces one take per pass. Pick a take
from a badge on the clip; split the clip and pick different takes on the
halves, and you've comped.
Data model (phase 2):
- AudioClipType::TakeFolder { takes, recorded_loop_beats } holds the take
list on the CLIP; ClipInstance::active_take holds the selection on the
INSTANCE. That split is what makes comping fall out of the existing
split action for free — split clones the instance, so the two halves
share one take list but choose independently. recorded_loop_beats lets
a future time-stretch/conform pass reconcile audio takes if the tempo
moves under them.
- AudioClip::resolve(active_take) -> ResolvedContent{Audio|Midi|Recording}
collapses a take folder to what an instance actually plays. A folder is
not a distinct *case* at call sites — it's an audio or MIDI clip whose
identity depends on which take is live — so every backend-sync site now
resolves through this instead of matching clip_type raw. Reverse lookups
go through owns_audio_pool_index/owns_midi_clip_id, since a folder owns
one pool file per take, not just the active one.
- BackendContext::add_clip_instance/remove_clip_instance: switching takes
is a remove + re-add (there's no in-place pool-swap command), and that's
the same work AddClipInstanceAction does. One implementation, on the
context that already owns the controller and both ID maps, so the
seconds-vs-beats conversions can't drift between copies.
Capture (phase 3):
- Takes are cut GEOMETRICALLY at stop, in exact loop-length multiples. The
playhead advances before the capture block in process(), so the wrap
instant isn't sample-exact against the buffer just captured — but the
geometry is. wrap_count only decides *whether* the recording is
multi-take, never where the cuts land.
- Partial passes are padded with silence: punch in mid-region and take 1
gets silence prepended back to the region start; stop mid-pass and the
last take gets silence appended. Every take is the same length, which is
the invariant comping depends on. A final take under 50ms of real audio
is dropped as a stop artifact (but a take that FILLED the region never
is, however short the region).
- MIDI merges, and it falls out for free: anchoring the recording at
loop_start rather than the punch-in point means the transport always
wraps back INTO the region, so every note's offset already lands inside
[0, loop_len) and passes overdub with no folding logic at all.
- The whole session commits as ONE undoable action via push_applied.
Fixes found on the way:
- Split was seconds/beats confused on MIDI. trim_start/trim_end are
domain-polymorphic exactly like AudioClip::duration was — SECONDS for
audio/video/vector, BEATS for MIDI — and split mapped the split point
into clip content in seconds unconditionally. Now it works in the clip's
own domain via Document::clip_trim_duration(). Regression test included.
- TrimClip took raw f64s whose meaning flipped by track type, and the
engine set an AUDIO clip's external_duration = Beats(end - start) where
those bounds were SECONDS — so a 1-second split played back as half a
second at 120 BPM. Replaced with a domain-tagged TrimRange, built from
the clip (clip.trim_range()) so the wrong unit isn't expressible, and
the span is now converted at the clip's position on the timeline.
- The live preview grew past the loop end while the playhead wrapped. It
now grows through the first pass then pins at the region length, and the
waveform inside restarts at the region start on each pass. The pass
offset is derived from the captured buffer, not the playhead — those
advance on different clocks, and differencing them made the waveform
jitter horizontally.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Recording (audio and MIDI) mutated the document directly in the AudioEvent
handlers, outside the action system — so it was never undoable, and dirty-
tracking leaned on an ad-hoc `media_modified` flag that the MIDI stop handler
forgot to set (hence: recording a MIDI clip didn't trigger the save-on-close
prompt).
Recording is temporal (the clip streams into the document live over the take),
so it can't be applied by one synchronous execute(). Instead, commit the
finished take as an *already-applied* action:
- ActionExecutor::push_applied(action) — registers an action whose effect is
already present (clears redo, bumps the epoch so the doc reads as modified,
pushes to the undo stack) WITHOUT re-running execute()/execute_backend().
Undo then removes the content via rollback/rollback_backend; redo re-adds it.
- AddClipInstanceAction::already_applied(...) — constructs the action pre-seeded
into its post-execute state (executed + the existing backend clip id) so the
first undo can remove the live-recorded clip from both doc and backend, and
redo re-adds it through the normal path.
- Both recording stop handlers now finalize, then push_applied this action.
Keeping the clip in the document (not a transient) matters for streaming-to-
disk and keeps the doc the single source of truth.
Recordings now bump the epoch like every other edit, so the media_modified
flag is dropped for recordings (kept only as a defensive fallback if the action
can't be built). Whole workspace compiles; 299 core tests pass.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
First half of the autosave/recovery feature — the write side.
Every ~45s while the document is dirty, write the full current state into a
per-session recovery .beam in the app data dir (directories::ProjectDirs data
dir + /recovery/session-<uuid>.beam). Fully background: reuses the existing file
worker, so the pool serialization / encode / DB write all happen off the UI
thread. The only UI-thread cost is one document clone — and build_save_command
now stamps the UI layout onto the *snapshot clone* rather than the live document
(the old prepare_document_for_save mutated live state via Arc::make_mut, which
could deep-clone the whole document mid-frame). Removed that dead helper.
Dirtiness is tracked centrally via ActionExecutor::epoch() (now also bumped on
undo/redo, not just execute) plus a pending_event flag set by non-action changes
(imports, finished recordings). The baseline is rebased on new/load/manual-save
so a freshly-loaded or just-saved project stays quiet. Idle-after-edit still gets
one snapshot via a single request_repaint_after wakeup; completion is polled
lazily (no forced repaints during the write).
on_exit deletes the session's recovery file, so a leftover file on next launch
means an unclean shutdown — the hook the recovery prompt (next commit) keys on.
`RasterStrokeAction`/`RasterFillAction` stored the whole before+after RGBA frame
(~16 MB/action at 1080p → up to ~1.6 GB at the 100-action cap). They now store a
`RasterDiff` — only the changed bounding box's pixels before and after — computed
once in `new()` from the full buffers, which are then dropped. A brush dab shrinks
from ~16 MB to tens of KB; a full-canvas fill is unchanged (its bbox is the frame).
Paging interaction: a diff overwrites just the bbox, so the keyframe's pixels must
be resident when undo/redo applies. A clean evicted frame's container bytes equal
its current logical state, so the editor faults the target frame in (synchronously)
before undo/redo via a new `Action::raster_resident_hint` + `peek_undo/redo_raster_hint`.
Dirty frames are never evicted, so they're already resident. If a base is somehow
not resident the apply is skipped (logged), never resized-and-corrupted.
Unit tests cover exact before/after round-trip, blank-first-stroke, no-op, and the
non-resident-base skip.
Skyler Lehmkuhl