From 0331f8b02ff8b3b0a6fe135fa7032dc2a20c022c Mon Sep 17 00:00:00 2001 From: Cyberflow Date: Thu, 9 Jul 2026 07:42:17 +0300 Subject: [PATCH] feat: move playback marker from notation measures --- CHANGELOG.md | 1 + JammLab/DesignSystem/AppTheme.swift | 1 + JammLab/Services/NotationMeasureLayout.swift | 45 +++++++ .../AudioPlayerViewModel+Notation.swift | 6 + .../AudioPlayerViewModel+Playback.swift | 5 + JammLab/Views/MainWorkspacePanels.swift | 1 + JammLab/Views/NotationTrackView.swift | 59 ++++++++++ JammLab/Views/NotationWindowView.swift | 1 + JammLab/Views/WaveformTimelineView.swift | 2 + .../NotationMeasureGeometryTests.swift | 110 ++++++++++++++++++ .../ViewModelNotationPasteTests.swift | 1 + .../ViewModelNotationSelectionTests.swift | 72 ++++++++++-- .../ViewModelPlaybackStateTests.swift | 62 ++++++++++ 13 files changed, 359 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7d65f4a..25a7b24 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ development artifact builds use `vMAJOR.MINOR.PATCH-dev.N`. ## Unreleased +- Added Notation clicks for moving the playback position marker from measure selections and barlines in both the main track and Notation window. - Added Leland glyph duration controls to the Notation window and main Notation track, with 4/5/6/7 shortcuts for eighth, quarter, half, and whole notes. - Added selectable Notation rest durations with persisted measure layouts and split-rest MusicXML export. - Added MusicXML export metadata, project tempo marking, and boxed bold Region labels. diff --git a/JammLab/DesignSystem/AppTheme.swift b/JammLab/DesignSystem/AppTheme.swift index e4303f5..251f1e9 100644 --- a/JammLab/DesignSystem/AppTheme.swift +++ b/JammLab/DesignSystem/AppTheme.swift @@ -317,6 +317,7 @@ enum AppTheme { static let notationRegionLabelFontSize: CGFloat = 10 static let notationRegionLabelCornerRadius: CGFloat = 1.5 static let notationRegionLabelGap: CGFloat = AppTheme.Spacing.xs + static let notationBarlineHitWidth: CGFloat = 8 static let stemTrackHeight: CGFloat = 48 static let defaultVisibleStemRows = 4 static let trackSpacing: CGFloat = 6 diff --git a/JammLab/Services/NotationMeasureLayout.swift b/JammLab/Services/NotationMeasureLayout.swift index 563e464..9232db0 100644 --- a/JammLab/Services/NotationMeasureLayout.swift +++ b/JammLab/Services/NotationMeasureLayout.swift @@ -67,6 +67,22 @@ struct NotationBarlineGeometry: Equatable { let isOuterBoundary: Bool } +struct NotationBarlineHitTarget: Equatable, Identifiable { + enum Boundary: Equatable { + case leading + case trailing + } + + let measureIndex: Int + let boundary: Boundary + let x: CGFloat + let targetTime: TimeInterval + + var id: String { + "\(measureIndex)-\(boundary)" + } +} + struct NotationSelectionOverlayRun: Equatable, Identifiable { let startMeasureIndex: Int let endMeasureIndex: Int @@ -297,6 +313,35 @@ struct NotationMeasureLayout { return barlines } + static func barlineHitTargets( + for geometries: [NotationMeasureCanvasGeometry], + measures: [ScoreMeasure] + ) -> [NotationBarlineHitTarget] { + let measureCount = min(geometries.count, measures.count) + guard measureCount > 0 else { return [] } + + var targets: [NotationBarlineHitTarget] = [] + for index in 0.. Void var selectMeasure: (ScoreMeasure?, Bool) -> Void var selectItem: (NotationItemSelection?) -> Void + var locatePlaybackMarkerExactly: (TimeInterval) -> Void var saveHarmony: (HarmonySymbol) -> Void var deleteHarmony: (HarmonySymbol.ID) -> Void var adjacentHarmonyPlacement: (TimeInterval, HarmonyNavigationDirection) -> HarmonyPlacement? @@ -61,6 +62,11 @@ struct NotationTrackView: View { height: proxy.size.height, attributeDisplays: attributeDisplays ) + barlineHitLayer( + width: contentWidth, + height: proxy.size.height, + attributeDisplays: attributeDisplays + ) notationItemSelectionHitLayer( width: contentWidth, height: proxy.size.height, @@ -484,6 +490,46 @@ struct NotationTrackView: View { } } + private func barlineHitLayer( + width: CGFloat, + height: CGFloat, + attributeDisplays: [NotationAttributeDisplay] + ) -> some View { + let geometries = measureCanvasGeometries( + measureCount: renderedMeasureCount, + width: width, + attributeDisplays: attributeDisplays + ) + let targets = NotationMeasureLayout.barlineHitTargets( + for: geometries, + measures: state.visibleMeasures + ) + let staffTop = staffTop(in: height) + let hitY = max(0, staffTop - AppTheme.Spacing.xs) + let hitHeight = AppTheme.Timeline.notationStaffLineSpacing * 4 + AppTheme.Spacing.sm + let hitWidth = AppTheme.Timeline.notationBarlineHitWidth + + return ZStack(alignment: .topLeading) { + ForEach(targets) { target in + Rectangle() + .fill(Color.clear) + .contentShape(Rectangle()) + .frame(width: hitWidth, height: hitHeight) + .offset( + x: target.x - hitWidth / 2, + y: hitY + ) + .onTapGesture { + isTrackFocused = true + editingDraft = nil + actions.locatePlaybackMarkerExactly(target.targetTime) + } + .accessibilityLabel(barlineAccessibilityLabel(for: target)) + .help(barlineAccessibilityLabel(for: target)) + } + } + } + private func regionLabelsLayer( width: CGFloat, height: CGFloat, @@ -1053,6 +1099,18 @@ struct NotationTrackView: View { return "Measures \(first.number) through \(last.number), \(state.keySignature.displayName), \(state.timeSignature.displayText)\(selectedMeasureText)" } + private func barlineAccessibilityLabel(for target: NotationBarlineHitTarget) -> String { + switch target.boundary { + case .leading: + guard state.visibleMeasures.indices.contains(target.measureIndex) else { + return "Move position marker to measure start" + } + return "Move position marker to measure \(state.visibleMeasures[target.measureIndex].number)" + case .trailing: + return "Move position marker to end of measure" + } + } + private var isShiftClickActive: Bool { NSApp.currentEvent?.modifierFlags.contains(.shift) == true } @@ -1128,6 +1186,7 @@ private extension NotationTrackActions { selectHarmony: { _ in }, selectMeasure: { _, _ in }, selectItem: { _ in }, + locatePlaybackMarkerExactly: { _ in }, saveHarmony: { _ in }, deleteHarmony: { _ in }, adjacentHarmonyPlacement: { _, _ in nil } diff --git a/JammLab/Views/NotationWindowView.swift b/JammLab/Views/NotationWindowView.swift index 20d97d2..f2203eb 100644 --- a/JammLab/Views/NotationWindowView.swift +++ b/JammLab/Views/NotationWindowView.swift @@ -156,6 +156,7 @@ struct NotationWindowView: View { selectHarmony: { viewModel.selectHarmonySymbol(id: $0) }, selectMeasure: { viewModel.selectNotationMeasure($0, extendingSelection: $1) }, selectItem: { viewModel.selectNotationItem($0) }, + locatePlaybackMarkerExactly: { viewModel.locatePlaybackMarkerExactly(to: $0) }, saveHarmony: { viewModel.saveHarmonySymbol($0) }, deleteHarmony: { viewModel.deleteHarmonySymbol(id: $0) }, adjacentHarmonyPlacement: { viewModel.adjacentHarmonyPlacement(from: $0, direction: $1) } diff --git a/JammLab/Views/WaveformTimelineView.swift b/JammLab/Views/WaveformTimelineView.swift index 6657ae2..4be7714 100644 --- a/JammLab/Views/WaveformTimelineView.swift +++ b/JammLab/Views/WaveformTimelineView.swift @@ -45,6 +45,7 @@ struct TimelineViewState: Equatable { struct TimelineViewActions { var locatePlaybackMarker: (TimeInterval) -> Void + var locatePlaybackMarkerExactly: (TimeInterval) -> Void var addNote: (TimeInterval) -> Void var selectHarmony: (HarmonySymbol.ID?) -> Void var selectNotationMeasure: (ScoreMeasure?, Bool) -> Void @@ -271,6 +272,7 @@ struct WaveformTimelineView: View { selectHarmony: actions.selectHarmony, selectMeasure: actions.selectNotationMeasure, selectItem: actions.selectNotationItem, + locatePlaybackMarkerExactly: actions.locatePlaybackMarkerExactly, saveHarmony: actions.saveHarmony, deleteHarmony: actions.deleteHarmony, adjacentHarmonyPlacement: actions.adjacentHarmonyPlacement diff --git a/JammLabTests/NotationMeasureGeometryTests.swift b/JammLabTests/NotationMeasureGeometryTests.swift index 9bd78fa..ce8a5e5 100644 --- a/JammLabTests/NotationMeasureGeometryTests.swift +++ b/JammLabTests/NotationMeasureGeometryTests.swift @@ -137,4 +137,114 @@ final class NotationMeasureGeometryTests: XCTestCase { XCTAssertFalse(barlines.contains { abs($0.x - geometries[2].contentStartX) < 0.0001 }) } + func testBarlineHitTargetsMapVisibleBoundariesToMeasureTimes() { + let geometries = NotationMeasureLayout.fallbackCanvasGeometries( + measureCount: 3, + totalWidth: 300 + ) + let measures = [ + scoreMeasure(number: 1, start: 0, end: 2), + scoreMeasure(number: 2, start: 2, end: 4), + scoreMeasure(number: 3, start: 4, end: 6) + ] + + let targets = NotationMeasureLayout.barlineHitTargets( + for: geometries, + measures: measures + ) + + XCTAssertEqual(targets.map(\.measureIndex), [0, 1, 2, 2]) + XCTAssertEqual(targets.map(\.boundary), [.leading, .leading, .leading, .trailing]) + XCTAssertEqual(targets.map(\.id), ["0-leading", "1-leading", "2-leading", "2-trailing"]) + XCTAssertEqual(targets.map(\.targetTime), [0, 2, 4, 6]) + XCTAssertEqual(targets[0].x, geometries[0].staffStartX, accuracy: 0.0001) + XCTAssertEqual(targets[1].x, geometries[1].cellStartX, accuracy: 0.0001) + XCTAssertEqual(targets[3].x, geometries[2].staffEndX, accuracy: 0.0001) + } + + func testBarlineHitTargetsKeepAttributedMeasureBoundaryTimes() { + let fullAttributes = MeasureAttributes( + keySignature: KeySignature.normalized(from: "F major"), + timeSignature: .fourFour, + clef: .treble + ) + let changedAttributes = MeasureAttributes( + keySignature: KeySignature.normalized(from: "Bb major"), + timeSignature: TimeSignature(beatsPerBar: 3, beatUnit: 4), + clef: .treble + ) + let firstReserve = NotationMeasureLayout.attributeReserveWidth( + for: fullAttributes, + display: .full + ) + let secondReserve = NotationMeasureLayout.attributeReserveWidth( + for: changedAttributes, + display: .full + ) + let geometries = NotationMeasureLayout.canvasGeometries( + measureCount: 2, + totalWidth: 300 + firstReserve + secondReserve, + attributeReserveWidths: [firstReserve, secondReserve] + ) + let measures = [ + scoreMeasure(number: 1, start: 0, end: 2, attributes: fullAttributes), + scoreMeasure(number: 2, start: 2, end: 3.5, attributes: changedAttributes) + ] + + let targets = NotationMeasureLayout.barlineHitTargets( + for: geometries, + measures: measures + ) + + XCTAssertEqual(targets.map(\.targetTime), [0, 2, 3.5]) + XCTAssertEqual(targets.map(\.id), ["0-leading", "1-leading", "1-trailing"]) + XCTAssertEqual(targets[1].x, geometries[1].cellStartX, accuracy: 0.0001) + XCTAssertNotEqual(targets[1].x, geometries[1].contentStartX, accuracy: 0.0001) + } + + func testBarlineHitTargetIDsStayStableWhenOnlyTimesChange() { + let geometries = NotationMeasureLayout.fallbackCanvasGeometries( + measureCount: 2, + totalWidth: 200 + ) + let originalMeasures = [ + scoreMeasure(number: 1, start: 0, end: 2), + scoreMeasure(number: 2, start: 2, end: 4) + ] + let retimedMeasures = [ + scoreMeasure(number: 1, start: 1, end: 3), + scoreMeasure(number: 2, start: 3, end: 5) + ] + + let originalTargets = NotationMeasureLayout.barlineHitTargets( + for: geometries, + measures: originalMeasures + ) + let retimedTargets = NotationMeasureLayout.barlineHitTargets( + for: geometries, + measures: retimedMeasures + ) + + XCTAssertEqual(retimedTargets.map(\.id), originalTargets.map(\.id)) + XCTAssertNotEqual(retimedTargets.map(\.targetTime), originalTargets.map(\.targetTime)) + } + + private func scoreMeasure( + number: Int, + start: TimeInterval, + end: TimeInterval, + attributes: MeasureAttributes = MeasureAttributes( + keySignature: .cMajor, + timeSignature: .fourFour, + clef: .treble + ) + ) -> ScoreMeasure { + ScoreMeasure( + number: number, + startTime: start, + endTime: end, + attributes: attributes + ) + } + } diff --git a/JammLabTests/ViewModelNotationPasteTests.swift b/JammLabTests/ViewModelNotationPasteTests.swift index 1c46ec7..3ad99d6 100644 --- a/JammLabTests/ViewModelNotationPasteTests.swift +++ b/JammLabTests/ViewModelNotationPasteTests.swift @@ -19,6 +19,7 @@ final class ViewModelNotationPasteTests: XCTestCase { XCTAssertTrue(viewModel.copySelectedNotationMeasure()) viewModel.selectNotationMeasure(targetMeasure) let beforePaste = viewModel.harmonySymbols + viewModel.markProjectClean() XCTAssertTrue(viewModel.pasteNotationMeasureClipboard()) diff --git a/JammLabTests/ViewModelNotationSelectionTests.swift b/JammLabTests/ViewModelNotationSelectionTests.swift index 19c3d21..fcb2284 100644 --- a/JammLabTests/ViewModelNotationSelectionTests.swift +++ b/JammLabTests/ViewModelNotationSelectionTests.swift @@ -3,17 +3,35 @@ import XCTest final class ViewModelNotationSelectionTests: XCTestCase { @MainActor - func testSelectingNotationMeasureDoesNotMarkProjectModified() throws { + func testSelectingNotationMeasureAtCurrentMarkerDoesNotMarkProjectModified() throws { let viewModel = try loadedNotationViewModel(duration: 8) let measure = try notationMeasure(1, in: viewModel) viewModel.selectNotationMeasure(measure) XCTAssertEqual(viewModel.selectedNotationMeasures.map(\.number), [1]) + XCTAssertEqual(viewModel.playbackMarkerTime, measure.startTime, accuracy: 0.0001) + XCTAssertEqual(viewModel.currentTime, measure.startTime, accuracy: 0.0001) XCTAssertTrue(viewModel.canCopySelectedNotationMeasure) XCTAssertFalse(viewModel.isProjectModified) } + @MainActor + func testSelectingNotationMeasureMovesPlaybackMarkerExactlyToMeasureStart() throws { + let engine = MockPlaybackEngine() + let viewModel = try loadedNotationViewModel(duration: 8, playbackEngine: engine) + viewModel.isSnapEnabled = true + let measure = try notationMeasure(2, in: viewModel) + + viewModel.selectNotationMeasure(measure) + + XCTAssertEqual(viewModel.selectedNotationMeasures.map(\.number), [2]) + XCTAssertEqual(viewModel.playbackMarkerTime, measure.startTime, accuracy: 0.0001) + XCTAssertEqual(viewModel.currentTime, measure.startTime, accuracy: 0.0001) + XCTAssertEqual(engine.currentTime, measure.startTime, accuracy: 0.0001) + XCTAssertTrue(viewModel.isProjectModified) + } + @MainActor func testSelectingNotationItemDoesNotMarkProjectModifiedAndClearsMeasureSelection() throws { let viewModel = try loadedNotationViewModel(duration: 8) @@ -89,18 +107,39 @@ final class ViewModelNotationSelectionTests: XCTestCase { @MainActor func testShiftSelectingNotationMeasuresBuildsContiguousRange() throws { + let engine = MockPlaybackEngine() + let viewModel = try loadedNotationViewModel(duration: 10, playbackEngine: engine) + let secondMeasure = try notationMeasure(2, in: viewModel) + let fourthMeasure = try notationMeasure(4, in: viewModel) + + viewModel.selectNotationMeasure(secondMeasure) + viewModel.selectNotationMeasure(fourthMeasure, extendingSelection: true) + + XCTAssertEqual(viewModel.selectedNotationMeasures.map(\.number), [2, 3, 4]) + XCTAssertEqual(viewModel.playbackMarkerTime, secondMeasure.startTime, accuracy: 0.0001) + XCTAssertEqual(viewModel.currentTime, secondMeasure.startTime, accuracy: 0.0001) + XCTAssertEqual(engine.currentTime, secondMeasure.startTime, accuracy: 0.0001) + + viewModel.selectNotationMeasure(secondMeasure, extendingSelection: true) + + XCTAssertEqual(viewModel.selectedNotationMeasures.map(\.number), [2]) + } + + @MainActor + func testReverseShiftSelectingNotationMeasuresMovesMarkerToFirstSelectedMeasure() throws { let viewModel = try loadedNotationViewModel(duration: 8) let firstMeasure = try notationMeasure(1, in: viewModel) let thirdMeasure = try notationMeasure(3, in: viewModel) - viewModel.selectNotationMeasure(firstMeasure) - viewModel.selectNotationMeasure(thirdMeasure, extendingSelection: true) + viewModel.selectNotationMeasure(thirdMeasure) - XCTAssertEqual(viewModel.selectedNotationMeasures.map(\.number), [1, 2, 3]) + XCTAssertEqual(viewModel.playbackMarkerTime, thirdMeasure.startTime, accuracy: 0.0001) viewModel.selectNotationMeasure(firstMeasure, extendingSelection: true) - XCTAssertEqual(viewModel.selectedNotationMeasures.map(\.number), [1]) + XCTAssertEqual(viewModel.selectedNotationMeasures.map(\.number), [1, 2, 3]) + XCTAssertEqual(viewModel.playbackMarkerTime, firstMeasure.startTime, accuracy: 0.0001) + XCTAssertEqual(viewModel.currentTime, firstMeasure.startTime, accuracy: 0.0001) } @MainActor @@ -124,16 +163,35 @@ final class ViewModelNotationSelectionTests: XCTestCase { XCTAssertTrue(viewModel.selectedNotationMeasures.isEmpty) } + @MainActor + func testClearingNotationMeasureSelectionDoesNotMovePlaybackMarker() throws { + let viewModel = try loadedNotationViewModel(duration: 8) + let measure = try notationMeasure(2, in: viewModel) + viewModel.selectNotationMeasure(measure) + viewModel.markProjectClean() + + viewModel.clearNotationMeasureSelection() + + XCTAssertTrue(viewModel.selectedNotationMeasures.isEmpty) + XCTAssertEqual(viewModel.playbackMarkerTime, measure.startTime, accuracy: 0.0001) + XCTAssertEqual(viewModel.currentTime, measure.startTime, accuracy: 0.0001) + XCTAssertFalse(viewModel.isProjectModified) + } + } extension XCTestCase { @MainActor - func loadedNotationViewModel(duration: TimeInterval) throws -> AudioPlayerViewModel { + func loadedNotationViewModel( + duration: TimeInterval, + playbackEngine: MockPlaybackEngine? = nil + ) throws -> AudioPlayerViewModel { let audioURL = try temporaryAudioFile(duration: duration) + let playbackEngine = playbackEngine ?? MockPlaybackEngine() let viewModel = AudioPlayerViewModel( analyzer: MockAnalyzer(), peakformProvider: MockPeakformProvider(), - playbackEngine: MockPlaybackEngine() + playbackEngine: playbackEngine ) let media = ImportedAudioFile(url: audioURL, displayName: "notation.wav", duration: duration) try viewModel.loadImportedAudio(media) diff --git a/JammLabTests/ViewModelPlaybackStateTests.swift b/JammLabTests/ViewModelPlaybackStateTests.swift index 6c8ad8d..87795c4 100644 --- a/JammLabTests/ViewModelPlaybackStateTests.swift +++ b/JammLabTests/ViewModelPlaybackStateTests.swift @@ -95,6 +95,68 @@ final class ViewModelPlaybackStateTests: XCTestCase { XCTAssertTrue(viewModel.isProjectModified) } + @MainActor + func testLocatingPlaybackMarkerExactlyBypassesSnapAndMarksProjectModified() throws { + let audioURL = try temporaryAudioFile(duration: 4) + defer { try? FileManager.default.removeItem(at: audioURL) } + let engine = MockPlaybackEngine() + let videoFollower = MockVideoFollower() + let viewModel = AudioPlayerViewModel( + analyzer: MockAnalyzer(), + peakformProvider: MockPeakformProvider(), + playbackEngine: engine, + videoFollower: videoFollower + ) + let media = ImportedAudioFile(url: audioURL, displayName: "exact-marker.wav", duration: 4) + try viewModel.loadImportedAudio(media) + viewModel.isSnapEnabled = true + viewModel.beatGridSettings = BeatGridSettings(bpm: 120, firstBeatTime: 0, timeSignature: .fourFour) + viewModel.markProjectClean() + + viewModel.locatePlaybackMarkerExactly(to: 0.74) + + XCTAssertEqual(viewModel.playbackMarkerTime, 0.74, accuracy: 0.0001) + XCTAssertEqual(viewModel.currentTime, 0.74, accuracy: 0.0001) + XCTAssertEqual(engine.currentTime, 0.74, accuracy: 0.0001) + XCTAssertEqual(try XCTUnwrap(videoFollower.seekTimes.last), 0.74, accuracy: 0.0001) + XCTAssertTrue(viewModel.isProjectModified) + } + + @MainActor + func testLocatingPlaybackMarkerExactlyClampsToTimelineBounds() throws { + let audioURL = try temporaryAudioFile(duration: 4) + defer { try? FileManager.default.removeItem(at: audioURL) } + let engine = MockPlaybackEngine() + let videoFollower = MockVideoFollower() + let viewModel = AudioPlayerViewModel( + analyzer: MockAnalyzer(), + peakformProvider: MockPeakformProvider(), + playbackEngine: engine, + videoFollower: videoFollower + ) + let media = ImportedAudioFile(url: audioURL, displayName: "exact-marker-bounds.wav", duration: 4) + try viewModel.loadImportedAudio(media) + + viewModel.setPlaybackMarkerExactly(to: 2) + viewModel.markProjectClean() + viewModel.locatePlaybackMarkerExactly(to: -1) + + XCTAssertEqual(viewModel.playbackMarkerTime, 0, accuracy: 0.0001) + XCTAssertEqual(viewModel.currentTime, 0, accuracy: 0.0001) + XCTAssertEqual(engine.currentTime, 0, accuracy: 0.0001) + XCTAssertEqual(try XCTUnwrap(videoFollower.seekTimes.last), 0, accuracy: 0.0001) + XCTAssertTrue(viewModel.isProjectModified) + + viewModel.markProjectClean() + viewModel.locatePlaybackMarkerExactly(to: 12) + + XCTAssertEqual(viewModel.playbackMarkerTime, 4, accuracy: 0.0001) + XCTAssertEqual(viewModel.currentTime, 4, accuracy: 0.0001) + XCTAssertEqual(engine.currentTime, 4, accuracy: 0.0001) + XCTAssertEqual(try XCTUnwrap(videoFollower.seekTimes.last), 4, accuracy: 0.0001) + XCTAssertTrue(viewModel.isProjectModified) + } + @MainActor func testPlaybackClockMovementDoesNotMarkProjectModified() throws { let audioURL = try temporaryAudioFile(duration: 4)