Repository navigation
fix(importer): pair and apply MusicXML spans per part, staff and number - #2938
Merged
Merged
Conversation
…nown MusicXML notes are attached to their beat before <technical><string> is read, so the beat's string lookup stayed empty. Lookups by string failed for every MusicXML note: hammer-on/pull-off destinations were never found (the effect was dropped) and let ring could not end on a note struck on the same string. Only the start of a hammer-on/pull-off is the origin now. The stop note was marked as origin too, which was hidden while the lookup was empty.
Let ring and palm mute written as <words> + <dashes> were only set as the derived beat flags. Playback and the palm mute rendering read the note flags, and the beat flags are lost when the score is serialized (worker rendering). The span state was a global flag applied in document order: a <dashes type="stop"/> without repeated words never ended the span, and spans leaked into other staves and parts. Direction spans are now tracked per part (MusicXmlSpans): starts and stops are paired by element family and number, preferring the same staff and kind, stops before starts within a direction, and stops appearing in the document before their start are parked. Positions are musical positions including <offset>, the stop is the exclusive end. After parsing, the spans set the note flags on all covered notes of their staff. Fixes #2867
Snapshot before restructuring the code design. Direction spans (dashes, bracket, wedge, octave-shift, pedal), wavy lines, measure styles (slash, measure-repeat), endings and note links (tied, slur, glissando, slide) are paired and applied through MusicXmlSpans instead of importer-global state in document order: - spans stay on their part, staff and voice (wedges and octave shifts no longer spill onto other staves and parts) - numbers pair starts and stops, stops before starts are parked - MuseScore let ring / palm mute spellings and brackets are supported, unknown line labels are kept as text - octave shift stops follow the exporter convention (Finale, Sibelius) - pedal releases in later bars, pedal changes, resume and discontinue - trill lines cover all notes in their voice, trill steps and frets fixed - measure repeats cover their range, endings are combined across parts - unended ties are tied to the next note, glissando and slide numbers no longer collide, slurs written along hammer-ons are not duplicated - offsets are converted from divisions (tempo, metronome, pedal) Pedal markers pressed after a lift in a held bar are no longer turned into holds, and pedal changes are drawn side by side.
The span tracker handled time ranges, note links and bar ranges in one place and branched on specific elements throughout. Split it up: - MusicXmlSpans: span records, the pairing of starts and stops per part and the readers of the span elements. - MusicXmlSpanApplier: applies the time ranges of a part to beats, pedal markers and simile bars (coverage, offsets, exporter conventions). - MusicXmlNoteLinks: pairs and applies note links (ties, slurs, slides, glissandos, trill and vibrato lines). - Endings are tracked per part in the importer. The placement of a span element is passed to the tracker instead of patched into the events, and the voice a wedge is written at comes from the following beat instead of a queue of pending events. This fixes: - directions separated by a <forward> lost their order - a direction with a voice changed the voice of earlier directions - a stop read before its start closed a span of another staff with the same number - pedal markers in overfull bars were placed after the bar end
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
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.
Note
AI-authored disclosure (
alphatab-ai-authored-v1)Portions of this content were authored by an AI agent. The agent has read
AGENTS.md and the human submitter accepts responsibility for
compliance with the rules in that document.
Issues
Fixes #2867
Proposed changes
#2867: a let ring written as
<words>LetRing</words>+<dashes>was drawn but not heard. There were two causes:Beat.isLetRing, but playback and the palm-mute rendering readNote.isLetRing/Note.isPalmMute. The beat flag is not serialized either, so the line was also missing in the web worker.<dashes type="stop"/>without repeated words never ended the span;numberand<offset>were ignored.Wedges, octave shifts, slash notation and endings had the same global, document-order handling. So the fix gives all MusicXML spans one shared foundation instead of special-casing let ring.
Design
MusicXmlSpanspairs starts and stops per part:<backup>) are parked until the start appears.MusicXmlSpanApplierapplies the paired time ranges to beats, pedal markers and simile bars.<offset>into the neighbouring note.MusicXmlNoteLinkspairs ties, slurs, slides, glissandos and trill/vibrato lines within a staff.Behaviour changes
<bracket>and MuseScore's spellings ("let ring", "P.M.", "palm mute"), and from a label in a separate preceding direction (music21);<tied type="let-ring"/>is supported, including TuxGuitar's placement outside<notations>.<software>, as in test suite 33da/33db).change,resumeanddiscontinueare supported. A change is drawn as "✱ Ped." side by side.trill-stephalf/unison is supported;<ending>no longer erases them.<offset sound="yes">.<string>is read. Before, the string lookup of MusicXML beats was always empty, so hammer-ons and pull-offs were never imported. Only the start of a hammer-on/pull-off marks the origin.Bar.finish): a pedal pressed again after a lift, in a bar after a held bar, was turned into a hold.Reference renders
The changed renders were checked one by one:
Not covered
The model has nothing to hold these:
accidental-markother-notation,principal-voice,string-mutePedals are not played back (MIDI does not read the pedal markers). This is unchanged.
Checklist
New tests:
MusicXmlImporter.test.ts:hammer-pullanddescribe('spans'), covering lines, wedges, octave shifts, pedals, wavy lines, measure styles, endings, note links and tempo offsetsBar.test.ts: the pedal retakeTS, C# and Kotlin test suites pass.
AI authorship disclosure
(
alphatab-ai-authored-v1) is present at the top of this body, and I havepersonally reviewed every change and can explain each one
Further details