diff --git a/src.csharp/AlphaTab/Core/EcmaScript/Set.cs b/src.csharp/AlphaTab/Core/EcmaScript/Set.cs index 09af796b8..fa03971be 100644 --- a/src.csharp/AlphaTab/Core/EcmaScript/Set.cs +++ b/src.csharp/AlphaTab/Core/EcmaScript/Set.cs @@ -49,5 +49,10 @@ IEnumerator IEnumerable.GetEnumerator() { return GetEnumerator(); } + + public void Delete(T item) + { + _data.Remove(item); + } } } diff --git a/src.csharp/AlphaTab/Core/TypeHelper.cs b/src.csharp/AlphaTab/Core/TypeHelper.cs index e161067b5..63bb4a5cd 100644 --- a/src.csharp/AlphaTab/Core/TypeHelper.cs +++ b/src.csharp/AlphaTab/Core/TypeHelper.cs @@ -142,7 +142,7 @@ public static void InsertRange(this IList data, int index, IEnumerable } } - public static void Sort(this IList data, Func func) + public static IList Sort(this IList data, Func func) { switch (data) { @@ -156,6 +156,8 @@ public static void Sort(this IList data, Func func) throw new NotSupportedException("Cannot sort list of type " + data.GetType().FullName); } + + return data; } public static void Sort(this IList data) { @@ -314,6 +316,11 @@ public static bool IsTruthy(object? s) { return s != null; } + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public static bool IsTruthy(bool? b) + { + return b.GetValueOrDefault(); + } [MethodImpl(MethodImplOptions.AggressiveInlining)] public static bool IsTruthy(double s) @@ -328,6 +335,13 @@ public static IList Map(this IList source, return source.Select(func).ToList(); } + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public static IList Map(this IList source, + Func func) + { + return source.Select(i => (double)func(i)).ToList(); + } + [MethodImpl(MethodImplOptions.AggressiveInlining)] public static string SubstringIndex(this string s, double startIndex) { diff --git a/src.kotlin/alphaTab/alphaTab/src/commonMain/kotlin/alphaTab/collections/List.kt b/src.kotlin/alphaTab/alphaTab/src/commonMain/kotlin/alphaTab/collections/List.kt index 220880f6f..1e02326fa 100644 --- a/src.kotlin/alphaTab/alphaTab/src/commonMain/kotlin/alphaTab/collections/List.kt +++ b/src.kotlin/alphaTab/alphaTab/src/commonMain/kotlin/alphaTab/collections/List.kt @@ -59,8 +59,9 @@ public class List : Iterable { return _data.removeLast() } - public fun sort(comparison: (a: T, b: T) -> Double) { + public fun sort(comparison: (a: T, b: T) -> Double) : List { _data.sortWith { a, b -> comparison(a, b).toInt() } + return this } public fun map(transform: (v: T) -> TOut): List { @@ -75,6 +76,15 @@ public class List : Iterable { return mapped } + @kotlin.jvm.JvmName("mapIntToDouble") + public fun map(transform: (v: T) -> Int): DoubleList { + val mapped = DoubleList(_data.size) + _data.forEachIndexed { index, item -> + mapped[index] = transform(item).toDouble() + } + return mapped + } + public fun reverse(): List { _data.reverse() return this diff --git a/src.kotlin/alphaTab/alphaTab/src/commonMain/kotlin/alphaTab/core/ecmaScript/Set.kt b/src.kotlin/alphaTab/alphaTab/src/commonMain/kotlin/alphaTab/core/ecmaScript/Set.kt index 4efd54436..4b4d273ef 100644 --- a/src.kotlin/alphaTab/alphaTab/src/commonMain/kotlin/alphaTab/core/ecmaScript/Set.kt +++ b/src.kotlin/alphaTab/alphaTab/src/commonMain/kotlin/alphaTab/core/ecmaScript/Set.kt @@ -19,6 +19,10 @@ internal class Set : Iterable { return _set.contains(item) } + public fun delete(item: T) { + _set.remove(item) + } + public fun forEach(action: (item: T) -> Unit) { for (i in _set) { action(i) diff --git a/src/importer/MusicXmlImporter.ts b/src/importer/MusicXmlImporter.ts index 39c5a97a8..79ada6988 100644 --- a/src/importer/MusicXmlImporter.ts +++ b/src/importer/MusicXmlImporter.ts @@ -69,7 +69,6 @@ export class MusicXmlImporter extends ScoreImporter { this.mergePartGroups(); } this._score.finish(this.settings); - // the structure of MusicXML does not allow live creation of the groups, this._score.rebuildRepeatGroups(); return this._score; } @@ -848,6 +847,13 @@ export class MusicXmlImporter extends ScoreImporter { if (!slurNumber) { slurNumber = '1'; } + + // slur numbers are unique in the way that they have the same ID across + // staffs/tracks etc. as long they represent the logically same slur. + // but in our case it must be globally unique to link the correct notes. + // adding the staff ID should be enough to achieve this + slurNumber = beat.voice.bar.staff.index + '_' + slurNumber; + switch (c.getAttribute('type')) { case 'start': this._slurStarts.set(slurNumber, note); @@ -857,7 +863,7 @@ export class MusicXmlImporter extends ScoreImporter { note.isSlurDestination = true; let slurStart: Note = this._slurStarts.get(slurNumber)!; slurStart.slurDestination = note; - note.slurOrigin = note; + note.slurOrigin = slurStart; } break; } diff --git a/src/midi/MidiPlaybackController.ts b/src/midi/MidiPlaybackController.ts index 3f111921d..05e4e4c1d 100644 --- a/src/midi/MidiPlaybackController.ts +++ b/src/midi/MidiPlaybackController.ts @@ -1,12 +1,31 @@ +import { RepeatGroup } from '@src/model'; import { MasterBar } from '@src/model/MasterBar'; import { Score } from '@src/model/Score'; +/** + * Helper container to handle repeats correctly + */ +class Repeat { + public group: RepeatGroup; + public opening: MasterBar; + public iterations: number[]; + public closingIndex: number = 0; + + public constructor(group: RepeatGroup, opening: MasterBar) { + this.group = group; + this.opening = opening; + // sort ascending according to index + group.closings = group.closings.sort((a, b) => a.index - b.index); + this.iterations = group.closings.map(_ => 0); + } +} + export class MidiPlaybackController { private _score: Score; - private _repeatStartIndex: number = 0; - private _repeatNumber: number = 0; - private _repeatOpen: boolean = false; + private _repeatStack: Repeat[] = []; + private _groupsOnStack: Set = new Set(); + private _previousAlternateEndings: number = 0; public shouldPlay: boolean = true; public index: number = 0; public currentTick: number = 0; @@ -21,31 +40,40 @@ export class MidiPlaybackController { public processCurrent(): void { const masterBar: MasterBar = this._score.masterBars[this.index]; - const masterBarAlternateEndings: number = masterBar.alternateEndings; - // if the repeat group wasn't closed we reset the repeating - // on the last group opening - if ( - !masterBar.repeatGroup.isClosed && - masterBar.repeatGroup.openings[masterBar.repeatGroup.openings.length - 1] === masterBar - ) { - this._repeatNumber = 0; - this._repeatOpen = false; + let masterBarAlternateEndings: number = masterBar.alternateEndings; + // if there are no alternate endings set on this bar. take the ones + // from the previously played bar which had alternate endings + if(masterBarAlternateEndings === 0) { + masterBarAlternateEndings = this._previousAlternateEndings; } - if ((masterBar.isRepeatStart || masterBar.index === 0) && this._repeatNumber === 0) { - this._repeatStartIndex = this.index; - this._repeatOpen = true; - } else if (masterBar.isRepeatStart) { - this.shouldPlay = true; + + // Repeat start (only properly closed ones) + if (masterBar === masterBar.repeatGroup.opening && masterBar.repeatGroup.isClosed) { + // first encounter of the repeat group? -> initialize repeats accordingly + if (!this._groupsOnStack.has(masterBar.repeatGroup)) { + const repeat = new Repeat(masterBar.repeatGroup, masterBar); + this._repeatStack.push(repeat); + this._groupsOnStack.add(masterBar.repeatGroup); + this._previousAlternateEndings = 0; + } } - // if we encounter an alternate ending - if (this._repeatOpen && masterBarAlternateEndings > 0) { + + // if we're not within repeats or not alternative endings set -> simply play + if (this._repeatStack.length === 0 || masterBarAlternateEndings === 0) { + this.shouldPlay = true; + } else { + const repeat = this._repeatStack[this._repeatStack.length - 1]; + const iteration = repeat.iterations[repeat.closingIndex]; + this._previousAlternateEndings = masterBarAlternateEndings; + // do we need to skip this section? - if ((masterBarAlternateEndings & (1 << this._repeatNumber)) === 0) { + if ((masterBarAlternateEndings & (1 << iteration)) === 0) { this.shouldPlay = false; } else { this.shouldPlay = true; } } + if (this.shouldPlay) { this.currentTick += masterBar.calculateDuration(); } @@ -54,21 +82,42 @@ export class MidiPlaybackController { public moveNext(): void { const masterBar: MasterBar = this._score.masterBars[this.index]; const masterBarRepeatCount: number = masterBar.repeatCount - 1; - // if we encounter a repeat end - if (this._repeatOpen && masterBarRepeatCount > 0) { - // more repeats required? - if (this._repeatNumber < masterBarRepeatCount) { + // if we encounter a repeat end... + if (this._repeatStack.length > 0 && masterBarRepeatCount > 0) { + // ...more repeats required? + const repeat = this._repeatStack[this._repeatStack.length - 1]; + const iteration = repeat.iterations[repeat.closingIndex]; + + // -> if yes, increase the iteration and jump back to start + if (iteration < masterBarRepeatCount) { // jump to start - this.index = this._repeatStartIndex; - this._repeatNumber++; + this.index = repeat.opening.index; + repeat.iterations[repeat.closingIndex]++; + + // clear iterations for previous closings and start over all repeats + // this ensures on scenarios like "open, bar, close, bar, close" + // that the second close will repeat again the first repeat. + for (let i = 0; i < repeat.closingIndex; i++) { + repeat.iterations[i] = 0; + } + repeat.closingIndex = 0; + this._previousAlternateEndings = 0; } else { - // no repeats anymore, jump after repeat end - this._repeatNumber = 0; - this._repeatOpen = false; - this.shouldPlay = true; - this.index++; + // if we don't have further iterations left but we have additional closings in this group + // proceed heading to the next close but keep the repeat group active + if (repeat.closingIndex < repeat.group.closings.length - 1) { + repeat.closingIndex++; + this.index++; // go to next bar after current close + } else { + // if there are no further closings in the current group, we consider the current repeat done and handled + this._repeatStack.pop(); + this._groupsOnStack.delete(repeat.group); + + this.index++; // go to next bar after current close + } } } else { + // we have no started repeat, just proceed to next bar this.index++; } } diff --git a/src/model/MasterBar.ts b/src/model/MasterBar.ts index cdaa23491..62ec83baf 100644 --- a/src/model/MasterBar.ts +++ b/src/model/MasterBar.ts @@ -140,9 +140,8 @@ export class MasterBar { let duration: number = 0; for (let track of this.score.tracks) { for (let staff of track.staves) { - let barDuration: number = this.index < staff.bars.length - ? staff.bars[this.index].calculateDuration() - : 0; + let barDuration: number = + this.index < staff.bars.length ? staff.bars[this.index].calculateDuration() : 0; if (barDuration > duration) { duration = barDuration; } diff --git a/src/model/RepeatGroup.ts b/src/model/RepeatGroup.ts index 8e1c51766..81fade806 100644 --- a/src/model/RepeatGroup.ts +++ b/src/model/RepeatGroup.ts @@ -9,10 +9,19 @@ export class RepeatGroup { */ public masterBars: MasterBar[] = []; + /** + * the masterbars which opens the group. + */ + public opening: MasterBar|null = null; + /** * a list of masterbars which open the group. + * @deprecated There can only be one opening, use the opening property instead */ - public openings: MasterBar[] = []; + public get openings(): MasterBar[] { + const opening = this.opening; + return opening ? [opening] : []; + } /** * a list of masterbars which close the group. @@ -20,9 +29,9 @@ export class RepeatGroup { public closings: MasterBar[] = []; /** - * true if the repeat group was opened well + * Gets whether this repeat group is really opened as a repeat. */ - public isOpened: boolean = false; + public get isOpened():boolean { return this.opening?.isRepeatStart === true; } /** * true if the repeat group was closed well @@ -30,21 +39,14 @@ export class RepeatGroup { public isClosed: boolean = false; public addMasterBar(masterBar: MasterBar): void { - if (this.openings.length === 0) { - this.openings.push(masterBar); + if (this.opening === null) { + this.opening = masterBar; } this.masterBars.push(masterBar); masterBar.repeatGroup = this; if (masterBar.isRepeatEnd) { this.closings.push(masterBar); this.isClosed = true; - if (!this.isOpened) { - this.masterBars[0].isRepeatStart = true; - this.isOpened = true; - } - } else if (this.isClosed) { - this.isClosed = false; - this.openings.push(masterBar); } } } diff --git a/src/model/Score.ts b/src/model/Score.ts index 4b727b67b..503645198 100644 --- a/src/model/Score.ts +++ b/src/model/Score.ts @@ -12,7 +12,9 @@ import { Settings } from '@src/Settings'; * @json_strict */ export class Score { - private _currentRepeatGroup: RepeatGroup = new RepeatGroup(); + private _currentRepeatGroup: RepeatGroup | null = null; + private _openedRepeatGroups: RepeatGroup[] = []; + private _properlyOpenedRepeatGroups:number = 0; /** * The album of this song. @@ -92,15 +94,11 @@ export class Score { public stylesheet: RenderStylesheet = new RenderStylesheet(); public rebuildRepeatGroups(): void { - let currentGroup: RepeatGroup = new RepeatGroup(); - for (let bar of this.masterBars) { - // if the group is closed only the next upcoming header can - // reopen the group in case of a repeat alternative, so we - // remove the current group - if (bar.isRepeatStart || (this._currentRepeatGroup.isClosed && bar.alternateEndings <= 0)) { - currentGroup = new RepeatGroup(); - } - currentGroup.addMasterBar(bar); + this._currentRepeatGroup = null; + this._openedRepeatGroups = []; + this._properlyOpenedRepeatGroups = 0; + for (const bar of this.masterBars) { + this.addMasterBarToRepeatGroups(bar); } } @@ -117,14 +115,59 @@ export class Score { bar.previousMasterBar.start + (bar.previousMasterBar.isAnacrusis ? 0 : bar.previousMasterBar.calculateDuration()); } - // if the group is closed only the next upcoming header can - // reopen the group in case of a repeat alternative, so we - // remove the current group - if (bar.isRepeatStart || (this._currentRepeatGroup.isClosed && bar.alternateEndings <= 0)) { + + this.addMasterBarToRepeatGroups(bar); + + this.masterBars.push(bar); + } + + /** + * Adds the given bar correctly into the current repeat group setup. + * @param bar + */ + private addMasterBarToRepeatGroups(bar: MasterBar) { + // handling the repeats is quite tricky due to many invalid combinations a user might define + // there are also some complexities due to nested repeats and repeats with multiple endings but only one opening. + // all scenarios are handled below. + + // NOTE: In all paths we need to ensure that the bar is added to some repeat group + + // start a new repeat group if really a repeat is started + // or we don't have a group. + if (bar.isRepeatStart) { + // if the current group was already closed (this opening doesn't cause nesting) + // we consider the group as completed + if(this._currentRepeatGroup?.isClosed) { + this._openedRepeatGroups.pop(); + this._properlyOpenedRepeatGroups--; + } + this._currentRepeatGroup = new RepeatGroup(); + this._openedRepeatGroups.push(this._currentRepeatGroup); + this._properlyOpenedRepeatGroups++; + } else if(!this._currentRepeatGroup) { this._currentRepeatGroup = new RepeatGroup(); + this._openedRepeatGroups.push(this._currentRepeatGroup); } + + // close current group if there was one started this._currentRepeatGroup.addMasterBar(bar); - this.masterBars.push(bar); + + // handle repeat ends + if (bar.isRepeatEnd) { + // if we have nested repeat groups a repeat end + // will treat the group as completed + if (this._properlyOpenedRepeatGroups > 1) { + this._openedRepeatGroups.pop(); + this._properlyOpenedRepeatGroups--; + // restore outer group in cases like "open open close close" + this._currentRepeatGroup = + this._openedRepeatGroups.length > 0 + ? this._openedRepeatGroups[this._openedRepeatGroups.length - 1] + : null; + } + // else: if only one group is opened, this group stays active for + // scenarios like open close bar close + } } public addTrack(track: Track): void { diff --git a/test-data/visual-tests/general/alternate-endings.png b/test-data/visual-tests/general/alternate-endings.png index b4761acee..6349d3cef 100644 Binary files a/test-data/visual-tests/general/alternate-endings.png and b/test-data/visual-tests/general/alternate-endings.png differ diff --git a/test/audio/MidiPlaybackController.test.ts b/test/audio/MidiPlaybackController.test.ts index 19a73f950..539eadd9f 100644 --- a/test/audio/MidiPlaybackController.test.ts +++ b/test/audio/MidiPlaybackController.test.ts @@ -9,6 +9,8 @@ describe('MidiPlaybackControllerTest', () => { const testRepeat: ((score: Score, expectedIndexes: number[], maxBars: number) => void) = (score: Score, expectedIndexes: number[], maxBars: number): void => { let controller: MidiPlaybackController = new MidiPlaybackController(score); let i: number = 0; + let errors:number[] = []; + let actual:number[] = []; while (!controller.finished) { let index: number = controller.index; controller.processCurrent(); @@ -17,11 +19,18 @@ describe('MidiPlaybackControllerTest', () => { fail('Too many bars generated'); } Logger.debug('Test', `Checking index ${i}, expected[${expectedIndexes[i]}]`, i, expectedIndexes[i]); + if(index !== expectedIndexes[i]) { + errors.push(i); + } + actual.push(index); expect(index).toEqual(expectedIndexes[i]); i++; } controller.moveNext(); } + if(errors.length > 0) { + fail(`Sequence errors: ${errors.join(', ')}, Expected: [${expectedIndexes.join(', ')}], Actual: [${actual.join(', ')}]`) + } expect(i).toEqual(expectedIndexes.length); expect(controller.finished).toBe(true); }; @@ -42,25 +51,25 @@ describe('MidiPlaybackControllerTest', () => { it('repeat-close', async () => { let file = 'audio/repeat-close.gp5'; let expectedIndexes = [0, 1, 0, 1, 2]; - testGuitarProRepeat(file, expectedIndexes, 20); + await testGuitarProRepeat(file, expectedIndexes, 20); }); it('repeat-close-multi', async () => { let file = 'audio/repeat-close-multi.gp5'; let expectedIndexes = [0, 1, 0, 1, 0, 1, 0, 1, 2]; - testGuitarProRepeat(file, expectedIndexes, 20); + await testGuitarProRepeat(file, expectedIndexes, 20); }); it('repeat-close-without-start-at-beginning', async () => { let file = 'audio/repeat-close-without-start-at-beginning.gp5'; let expectedIndexes = [0, 1, 0, 1]; - testGuitarProRepeat(file, expectedIndexes, 20); + await testGuitarProRepeat(file, expectedIndexes, 20); }); it('repeat-close-alternate-endings', async () => { let file = 'audio/repeat-close-alternate-endings.gp5'; let expectedIndexes = [0, 1, 0, 2, 3, 0, 1, 0, 4]; - testGuitarProRepeat(file, expectedIndexes, 20); + await testGuitarProRepeat(file, expectedIndexes, 20); }); it('repeat-with-alphaTex', () => { @@ -87,4 +96,47 @@ describe('MidiPlaybackControllerTest', () => { ]; testAlphaTexRepeat(tex, expectedBars, 50); }); + + it('multiple-closes', () => { + let tex: string = ` + . + 4.3.4*4 | \\ro 4.3.4*4 | \\rc 2 4.3.4*4 | 4.3.4*4 | 4.3.4*4 | \\rc 2 4.3.4*4 | 4.3.4*4 | + \\ro 4.3.4*4 | \\rc 2 4.3.4*4 | 4.3.4*4 | 4.3.4*4 + `; + let expectedBars: number[] = [ + 0, // no repeat + // First round of outer repeat + 1, 2, // First round of inner repeat + 1, 2, // Second round of inner repeat + 3, 4, 5, + // Second round of outer repeat + 1, 2, // First round of inner repeat + 1, 2, // Second round of inner repeat + 3, 4, 5, + 6, + // next repeat + 7, 8, + 7, 8, + // Second repeat done + 9, 10 // last two bars + ]; + testAlphaTexRepeat(tex, expectedBars, 50); + }); + + it('nested-repeats', () => { + let tex: string = ` + . + \\ro 4.3.4*4 | \\ro 4.3.4*4 | \\rc 2 4.3.4*4 | 4.3.4*4 | \\rc 2 4.3.4*4 | + 3.3.4*4 | + \\ro 4.3.4*4 | \\ro 4.3.4*4 | \\rc 2 4.3.4*4 | 4.3.4*4 | \\rc 2 4.3.4*4 + `; + let expectedBars: number[] = [ + 0, 1, 2, 1, 2, 3, 4, + 0, 1, 2, 1, 2, 3, 4, + 5, + 6, 7, 8, 7, 8, 9, 10, + 6, 7, 8, 7, 8, 9, 10 + ]; + testAlphaTexRepeat(tex, expectedBars, 50); + }); }); diff --git a/test/importer/MusicXmlImporterTestHelper.ts b/test/importer/MusicXmlImporterTestHelper.ts index a700b1526..d38a0aa44 100644 --- a/test/importer/MusicXmlImporterTestHelper.ts +++ b/test/importer/MusicXmlImporterTestHelper.ts @@ -16,6 +16,8 @@ import { Track } from '@src/model/Track'; import { Voice } from '@src/model/Voice'; import { Settings } from '@src/Settings'; import { TestPlatform } from '@test/TestPlatform'; +import { JsonConverter } from '@src/model/JsonConverter'; +import { ComparisonHelpers } from '@test/model/ComparisonHelpers'; export class MusicXmlImporterTestHelper { public static prepareImporterWithBytes(buffer: Uint8Array): MusicXmlImporter { @@ -30,16 +32,30 @@ export class MusicXmlImporterTestHelper { renderAllTracks: boolean = false ): Promise { const fileData = await TestPlatform.loadFile(file); + let score: Score; try { let importer: MusicXmlImporter = MusicXmlImporterTestHelper.prepareImporterWithBytes(fileData); - let score: Score = importer.readScore(); - return score; + score = importer.readScore(); } catch (e) { if (e instanceof UnsupportedFormatError) { fail(`Failed to load file ${file}: ${e}`); } throw e; } + + // send it to serializer once and check equality + try { + const expectedJson = JsonConverter.scoreToJsObject(score); + + const deserialized = JsonConverter.jsObjectToScore(expectedJson); + const actualJson = JsonConverter.scoreToJsObject(deserialized); + + ComparisonHelpers.expectJsonEqual(expectedJson, actualJson, '<' + file + '>', null); + } catch(e) { + fail(e); + } + + return score; } protected static getHierarchy(node: unknown): string {