From 5c5090bff08334614645236d751edfba1dcd555e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20LUSTREMENT?= Date: Wed, 13 May 2026 12:28:39 +0200 Subject: [PATCH] fix(musicxml): finalize percussion articulation after note context resolution (#2700) --- .../alphatab/src/importer/MusicXmlImporter.ts | 56 +++++++++++++++---- .../musicxml4/percussion-articulation.xml | 51 +++++++++++++++++ .../test/importer/MusicXmlImporter.test.ts | 14 +++++ 3 files changed, 111 insertions(+), 10 deletions(-) create mode 100644 packages/alphatab/test-data/musicxml4/percussion-articulation.xml diff --git a/packages/alphatab/src/importer/MusicXmlImporter.ts b/packages/alphatab/src/importer/MusicXmlImporter.ts index 41ae4f533..af233ca19 100644 --- a/packages/alphatab/src/importer/MusicXmlImporter.ts +++ b/packages/alphatab/src/importer/MusicXmlImporter.ts @@ -2582,16 +2582,6 @@ export class MusicXmlImporter extends ScoreImporter { this._insertBeatToVoice(newBeat, voice); - if (note !== null) { - note!.isVisible = noteIsVisible; - const trackInfo = this._indexToTrackInfo.get(track.index)!; - if (instrumentId !== null) { - note!.percussionArticulation = trackInfo.getOrCreateArticulation(instrumentId!, note!); - } else if (!isPitched) { - note!.percussionArticulation = trackInfo.getOrCreateArticulation('', note!); - } - } - // duration only after we added it into the tree if (graceType !== GraceType.None) { newBeat.graceType = graceType; @@ -2759,6 +2749,52 @@ export class MusicXmlImporter extends ScoreImporter { // if not yet created do it befor we exit to ensure we created the beat/note ensureBeat(); + + if (note !== null) { + // Final note post-processing depends on the note already being attached to the + // beat/voice/bar/staff tree (e.g. percussion clef context on the resolved staff). + // Therefore this must run after ensureBeat() and after transposition has been applied. + this._finalizeImportedNote(note, track, instrumentId, isPitched, noteIsVisible); + } + } + + /** + * Applies note-level post-processing that requires the fully resolved parse context. + * + * Purpose: + * - Set final visibility. + * - Resolve percussion articulation consistently in one place. + * + * Why this is called at the end of _parseNote: + * - The logic relies on final note context (attached beat/voice/bar/staff), especially + * staff percussion state, and on the final display value after transposition. + * - Running this earlier could use incomplete or wrong context and produce wrong + * articulation mapping. + */ + private _finalizeImportedNote( + note: Note, + track: Track, + instrumentId: string | null, + isPitched: boolean, + noteIsVisible: boolean + ) { + note.isVisible = noteIsVisible; + + if (note.percussionArticulation >= 0) { + return; + } + + const trackInfo = this._indexToTrackInfo.get(track.index)!; + if (instrumentId !== null) { + note.percussionArticulation = trackInfo.getOrCreateArticulation(instrumentId, note); + } else if (note.beat.voice.bar.staff.isPercussion && isPitched) { + const knownArticulation = PercussionMapper.getArticulationById(note.displayValue); + if (knownArticulation) { + note.percussionArticulation = knownArticulation.id; + } + } else if (!isPitched) { + note.percussionArticulation = trackInfo.getOrCreateArticulation('', note); + } } private _parsePlay(element: XmlNode, note: Note | null) { diff --git a/packages/alphatab/test-data/musicxml4/percussion-articulation.xml b/packages/alphatab/test-data/musicxml4/percussion-articulation.xml new file mode 100644 index 000000000..ef7a56897 --- /dev/null +++ b/packages/alphatab/test-data/musicxml4/percussion-articulation.xml @@ -0,0 +1,51 @@ + + + + + Drums + + Drumset + + + 10 + 1 + + + + + + + 1 + + 0 + + + + percussion + + + + + D + 2 + + 1 + 1 + quarter + + + + C + 1 + 3 + + 1 + 1 + quarter + + + + diff --git a/packages/alphatab/test/importer/MusicXmlImporter.test.ts b/packages/alphatab/test/importer/MusicXmlImporter.test.ts index d405dee12..2ad1577c6 100644 --- a/packages/alphatab/test/importer/MusicXmlImporter.test.ts +++ b/packages/alphatab/test/importer/MusicXmlImporter.test.ts @@ -275,6 +275,20 @@ describe('MusicXmlImporterTests', () => { expect(score).toMatchSnapshot(); }); + it('percussion-articulation', async () => { + const score = await MusicXmlImporterTestHelper.loadFile('test-data/musicxml4/percussion-articulation.xml'); + const notes = score.tracks[0].staves[0].bars[0].voices[0].beats.flatMap(b => b.notes); + + expect(notes).toHaveLength(2); + expect(notes[0].displayValue).toBe(38); + expect(notes[0].isPercussion).toBe(true); + expect(notes[0].percussionArticulation).toBe(38); + + expect(notes[1].displayValue).toBe(49); + expect(notes[1].isPercussion).toBe(true); + expect(notes[1].percussionArticulation).toBe(49); + }); + describe('barnumberdisplay', async () => { async function testPartwise(filename: string, display: BarNumberDisplay) { const score = await MusicXmlImporterTestHelper.loadFile(`test-data/musicxml4/${filename}`);