MIDI export: keep lyrics with their notes under swing - #34714
Conversation
Swing shifts the offbeat notes later, but the lyric meta events kept the unadjusted chord tick. The syllables ended up on ticks where no note starts, and MIDI import matches a lyric to a note by exact onset, so half of them were dropped on a round trip. createPlayEvents() applies swing through Swing::swingAdjustParams(), which raises the chord's ontime, and the note-on tick becomes tick1 + ticks * ontime / 1000. The lyric tick was built from the chord tick alone. Both pass through the same pause mapping afterwards, so the gap survived into the file. swingTickOffset() recomputes that one term using the same shared helper and the same conditions the renderer applies, so a lyric cannot drift from its note. It returns 0 for rests, for staves with no swing setting, for tuplets, and for chords whose play events are user-defined, because createPlayEvents() discards the swung list in that case and the notes do not move either. Reading the rendered ontime back off the note would be shorter, but it also carries the grace note and arpeggio offsets, and moving those lyrics is a separate question. Resolves: musescore#32288
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughThe MIDI exporter now applies staff swing timing to lyric events for eligible automatic, non-tuplet chords. Three MuseScore fixtures cover standard swing lyrics, user events, and repeated measures. Tests parse exported MIDI files and verify that lyric events occur at the expected note-on ticks. Merge Risk: ⚪ Minimal · up to The change keeps lyric events aligned with swung note onsets and adds focused regression coverage for affected cases; no actionable merge-blocking risk remains beyond normal checks and review. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 2 files. (3 skipped: 3 unsupported.)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Linked repositories: Public OSS repositories can only analyze public repositories installed in this organization. No linked repositories were analyzed; skipped Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
2 participants
Resolves: #32288
Swing moves the offbeat notes later, but the lyric meta events kept the unadjusted chord tick, so the syllables landed on ticks where no note starts. MIDI import matches a lyric to a note by exact onset, so those syllables were dropped on a round trip. The lyric tick now takes the same swing offset the renderer applies to the notes, computed with the same shared helper and the same conditions, so staves without swing, tuplets, and chords with user-defined play events are unaffected.
Not addressed here: a swung chord that also carries grace notes still will not have its lyric exactly on the onset, because the grace offset is a separate term. That gap is unchanged in size by this PR.
Built in CI; I tested the resulting Windows build against a swung score with lyrics, and byte-compared its exports against a build differing only by this change.