Stop snapping cursor into loop range on play/record start#11
Merged
Conversation
TransportControl::performPlay and performRecord forcibly relocated the play cursor to loopRange.start whenever the cursor sat outside the loop on play/record start. That diverges from how every modern DAW (Live, Bitwig, Logic, Cubase, Reaper, Studio One) handles loop mode and was the user-visible bug downstream in MAGDA issue #1204. performPlay now keeps the cursor in place and uses PlayHead's existing setRollInToLoop machinery to play through into the loop when the cursor is before loopStart. Cursor-past-loop plays forward without looping this session — looping re-engages on next play once the cursor is back in or before the region. performRecord just drops the snap; pre-roll and loop+punch composition stay in scope for a follow-up.
performPlay already treats cursor-past-loop as a non-looping session (matches the user's expectation that playback continues forward without jumping back to a loop region the cursor has already passed). performRecord was missing the equivalent: it called setLoopTimes(true, lr) and setRollInToLoop(prerollStart) unconditionally on looping=true, and PlayHead::setRollInToLoop clamps positions to loopEnd, so a record start with the cursor past the loop ended up rolling at the loop boundary instead of the cursor. Compute effectivelyLooping the same way performPlay does, and use it to gate the looped playhead branch.
…t loopEnd
The previous patch reused TE's snap-trigger predicate (cursor > loopEnd - 0.1s)
to decide whether to disable looping for the play session. Wrong action context:
that 100ms band includes positions still inside the loop, so cursor at e.g. 7.95
in a [4,8] loop ended up running forward to MaxEnd with looping silently off.
Split the logic into two clear cases at play-start:
- Cursor in the last 100ms of the loop: keep TE's original snap-to-loopStart
(one-time choice at play, the user just confirmed this is desirable — avoids
landing at a position that would wrap on the very first sample).
- Cursor strictly past loopEnd: play forward, no looping this session.
performRecord drops the 0.1s tolerance from the past-loop predicate too — record
just splits past/not-past, no near-end snap (narrow scope).
Previous iteration had two latent issues: 1. The cursor-vs-loop predicate was in seconds. Since the cursor and the loop-end both derive from beats via independent paths through the tempo sequence, FP drift could put a click that landed "exactly at bar 5" on either side of the seconds boundary, flipping play-vs-snap-vs-forward behavior arbitrarily. Switched the predicate to operate in beats via tempoSequence.toBeats so the boundary is exact in the canonical domain. 2. The periodic setLoopTimes call in performRunning re-asserted setLoopTimes(true, loopRange) every ~10 ticks whenever transport.looping was on, regardless of what performPlay had decided. This silently overrode "no looping this session" decisions: cursor-past-loop play would start forward, then a few ticks later the playhead would have its loop range set back to the loop region, position past loopEnd would wrap, and the user would see playback jump back to loopStart. Gated the re-assert on transportState->endTime so the play-session decision survives. The near-end seconds snap is gone — it was only ever there to defend against the FP boundary the beats comparison now eliminates.
The 8-core Windows runner isn't available to this fork, so the job queues forever. ubuntu-latest and macos-latest still run.
The 8-core Windows runner isn't available on this fork. Re-enable the Windows job on the standard windows-latest runner.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
TransportControl::performPlay and performRecord forcibly relocated the play cursor to loopRange.start whenever the cursor sat outside the loop on play/record start. That diverges from how every modern DAW (Live, Bitwig, Logic, Cubase, Reaper, Studio One) handles loop mode and was the user-visible bug downstream in MAGDA issue #1204.
performPlay now keeps the cursor in place and uses PlayHead's existing setRollInToLoop machinery to play through into the loop when the cursor is before loopStart. Cursor-past-loop plays forward without looping this session — looping re-engages on next play once the cursor is back in or before the region. performRecord just drops the snap; pre-roll and loop+punch composition stay in scope for a follow-up.