From 2be6164b73e1afac4d205d24b722c82585d270bd Mon Sep 17 00:00:00 2001 From: ChrisBeWithYou Date: Mon, 13 Jul 2026 22:17:05 -0500 Subject: [PATCH] fix(editor): snap dragged barlines to CHART-time onsets (issue #254) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit _tempoMapOnDragMove snapped a dragged barline to the nearest onset using _ensureOnsets() (BUFFER time) against rawT (CHART time). When the recording is shifted (S.audioShift != 0) the two diverge, so the barline snapped to the un-shifted attack. Use _ensureOnsetsShifted() — the chart-time onsets Suggest-fit already uses. Pre-existing since #235; low impact (only with a shifted audio), undoable, but a wrong snap target. Also drops the now-unused _ensureOnsets import. tempo_onset_snap.test.mjs's source-text assertion updated to _ensureOnsetsShifted. 146 JS green, lint 0-err. Fixes #254. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01EBQCHCNA81E9tHmSDHSe2Q --- src/tempo.js | 8 ++++++-- tests/tempo_onset_snap.test.mjs | 3 ++- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/tempo.js b/src/tempo.js index ccb265df..db7e8170 100644 --- a/src/tempo.js +++ b/src/tempo.js @@ -20,7 +20,7 @@ // Browser surface: `ctx` (the shared 2D context) plus the sync-inspector and // time-signature controls it builds into the toolbar. // ════════════════════════════════════════════════════════════════════ -import { _ensureOnsets, _ensureOnsetsShifted, _nearestOnsetTimePure } from './audio.js'; +import { _ensureOnsetsShifted, _nearestOnsetTimePure } from './audio.js'; import { _localTempoSeriesPure, _segmentRoughMapPure, _segmentTempoPure } from './tempo-segment.js'; import { beatOf, timeOf } from './beats.js'; import { DPR, canvas, ctx } from './canvas.js'; @@ -2514,7 +2514,11 @@ export function _tempoMapOnDragMove(x) { dg.snappedT = null; if (S.snapMode === 'onset' && !(orig[d] && orig[d].locked)) { const tol = _tempoOnsetSnapTolPure(xToTime(1) - xToTime(0), ONSET_SNAP_PX, ONSET_SNAP_MAX_S); - const res = _tempoOnsetSnapPure(rawT, _ensureOnsets(), tol, loBound, hiBound); + // rawT and the barlines are CHART time, so the snap must compare against + // CHART-time onsets — _ensureOnsetsShifted() (matching Suggest-fit), not the + // buffer-time _ensureOnsets(): with the audio shifted the two diverge and the + // barline would snap to the un-shifted attack (issue #254). + const res = _tempoOnsetSnapPure(rawT, _ensureOnsetsShifted(), tol, loBound, hiBound); newT = res.t; if (res.snapped) dg.snappedT = newT; } diff --git a/tests/tempo_onset_snap.test.mjs b/tests/tempo_onset_snap.test.mjs index 4619096a..227552a9 100644 --- a/tests/tempo_onset_snap.test.mjs +++ b/tests/tempo_onset_snap.test.mjs @@ -72,7 +72,8 @@ t('the drag move gates onset-snap on Snap = Onset and a non-locked pole', () => const b = body('export function _tempoMapOnDragMove', 'export function _tempoMapOnDragEnd'); assert.match(b, /S\.snapMode === 'onset'/, 'only snaps in Onset mode'); assert.match(b, /!\(orig\[d\] && orig\[d\]\.locked\)/, 'locked poles never snap'); - assert.match(b, /_tempoOnsetSnapPure\(rawT, _ensureOnsets\(\)/, 'uses the live onset cache'); + assert.match(b, /_tempoOnsetSnapPure\(rawT, _ensureOnsetsShifted\(\)/, + 'snaps against CHART-time onsets (shift-corrected), matching Suggest-fit — issue #254'); }); t('the drag end reports a snap', () => { const b = body('export function _tempoMapOnDragEnd', 'export function _makeTimeRemap');