Fix grace note MIDI export: silent grace notes in saved score files

[jonashogstrom]

Fixes #22669

Background

Issue #22669 reports that acciaccatura grace notes stopped exporting correctly to MIDI in MuseScore 4.1 (regression from 4.0.2). In the exported MIDI they play on the beat rather than before it, or are silent entirely. The workaround discovered by users — manually entering play events via the Properties panel — is itself a clue: setting events manually forces PlayEventType::User, which (as described below) is part of the root cause.

Root causes and fixes

Fix 1 — Before-beat timing regression (the core 4.1 regression)

The graceTickSum mechanism that shifts acciaccatura events before the beat was broken, causing grace notes to play on the beat with near-zero duration instead of before it. Restored the correct before-beat offset computation in collectGraceBeforeChordEvents.

Fix 2 — Stale PlayEventType::User silences grace notes in saved files

MuseScore writes <Events> blocks for every note whenever a score is saved, including grace notes. On load, reading <Events> marks the chord PlayEventType::User. createGraceNotesPlayEvents() was guarded by if (gc->playEventType() == PlayEventType::Auto), so it skipped recomputing grace events — leaving stale len=0 values from earlier renders in place and making those grace notes completely silent.

This affected any score that had been saved at least once, including files freshly imported from MusicXML and saved with the current version. It also explains why the manual-entry workaround in the issue worked: forcing PlayEventType::User with correct values bypassed the broken auto-computation.

Fixed at three levels (defence-in-depth):

• twrite.cpp: never write <Events> for grace note chords. MuseScore 4 exposes no UI for editing grace note MIDI timing, so there is nothing worth persisting. Breaks the write-load-skip cycle at its source; existing saved files are cleaned up on next save.
• tread.cpp: when reading <Events> for a grace chord, do not set PlayEventType::User — the stored data is always a stale render artifact.
• compatmidirender.cpp: always recompute grace note events regardless of PlayEventType, as a defensive backstop.

Fix 3 — Acciaccatura at tick 0 produces zero-duration MIDI note

Acciaccatura before the very first beat of a piece was silent. The before-beat start is tick - graceTickSum; at tick=0 this clamps to 0. With NoteEvent.len=0 (correct for acciaccatura), off = max(0, 0 - 1) = 0, giving a zero-duration MIDI note. Fixed by preserving the intended duration when the start is clamped: offTime = graceOffsetOn - 1.

Tests added

Test	What it covers
midiGraceBefore	Acciaccatura before-beat timing at 60 and 320 bpm, including the tick=0 edge case and multi-grace groups
midiGraceAfter	Grace-after notes start at the midpoint of the principal note
midiGraceAppoggiatura	Appoggiatura takes proportional time from the principal note
midiGraceStaleEvents	Grace notes with stored len=0 Events (as written by MuseScore on every save) are exported with correct timing

Files changed

File	Change
src/engraving/compat/midi/compatmidirender.cpp	Remove PlayEventType::Auto guards — always recompute grace events
src/engraving/compat/midi/compatmidirenderinternal.cpp	Fix tick=0 duration clamping; restore before-beat graceTickSum mechanism
src/engraving/rw/write/twrite.cpp	Skip writing <Events> for grace note chords
src/engraving/rw/read400/tread.cpp	Don't set PlayEventType::User when reading grace chord events
src/importexport/midi/tests/midiexport_tests.cpp	Four new test functions
src/importexport/midi/tests/midiexport_data/testGraceAfter.mscx	New test score
src/importexport/midi/tests/midiexport_data/testGraceAppoggiatura.mscx	New test score
src/importexport/midi/tests/midiexport_data/testGraceStaleEvents.mscx	New test score (grace notes with pre-stored stale events)
src/importexport/musicxml/tests/data/testBackupRoundingError_ref.mscx	Removed stale grace note Events blocks (consequence of twrite.cpp fix)
src/importexport/musicxml/tests/data/testCueGraceNotes_ref.mscx	Same — 9 blocks removed
src/importexport/musicxml/tests/data/testDuplicateFermataOnGraceNote_ref.mscx	Same — 1 block removed
src/importexport/musicxml/tests/data/testDuplicateFermataOnGraceNoteAndMainNote_ref.mscx	Same — 1 block removed
src/importexport/musicxml/tests/data/testSticking_ref.mscx	Same — 6 blocks removed

Note on the MusicXML reference file changes: The five _ref.mscx files are round-trip reference snapshots used by MusicXml_Tests. Because twrite.cpp no longer writes <Events> for grace note chords, the saved output no longer contains those blocks — so the reference files must match. The removed blocks were all stale <len>0</len> render artifacts that had no effect on the score; removing them is correct behaviour.

---

• I signed the CLA
• The title of the PR describes the problem it addresses
• Each commit's message describes its purpose and effects, and references the issue it resolves
• If changes are extensive, there is a sequence of easily reviewable commits
• The code in the PR follows the coding rules
• There are no unnecessary changes
• The code compiles and runs on my machine, preferably after each commit individually
• I created a unit test or vtest to verify the changes I made (if applicable)

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

This PR fixes grace note MIDI export by preventing grace chords from being marked/persisted as user play-events, correcting timing in the render path to use the principal chord tick and properly clamp/adjust on/off times for negative offsets, always regenerating stored grace-note event lists, and adding end-to-end tests (grace-before, grace-after, appoggiatura, and stale zero-length event regression).

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 58.33% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'Fix grace note MIDI export: silent grace notes in saved score files' accurately summarizes the main change—fixing a regression where grace notes exported to MIDI were silent or misplaced.
Linked Issues check	✅ Passed	The PR fully addresses issue #22669 by fixing grace note MIDI export regressions: restores before-beat timing for acciaccaturas, prevents stale Events from silencing grace notes, corrects tick=0 duration clamping, and includes comprehensive MIDI tests.
Out of Scope Changes check	✅ Passed	All changes directly support fixing the grace note MIDI export issue: core logic fixes target grace-note timing and serialization, tests validate the fixes, and reference file updates are expected consequences of the serialization changes.
Description check	✅ Passed	The PR description is comprehensive, well-structured, and addresses all required template sections including issue resolution, technical details, and testing verification.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

Warning

Review ran into problems

🔥 Problems

Linked repositories: Your configuration references 1 linked repositories, but your current plan allows 0. Analyzed ``, skipped musescore/muse_framework.git.

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/importexport/midi/tests/midiexport_tests.cpp`:
- Around line 352-354: The comment in the midiGraceAppoggiatura test is wrong
about the appoggiatura duration: update the explanatory comment next to the
EXPECT_NE checks for findNoteEvent(track, 480, 71, 80) and findNoteEvent(track,
720, 72, 80) to state that the grace starts at 480 and the principal is at 720
(so the grace length is 240 ticks, not 480), and remove the incorrect
“(min(500,500)) = 480” math so the comment correctly reflects the +240 tick
displacement between grace and principal.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 84390cd9-843c-4166-b809-4c61049b76b6

📥 Commits

Reviewing files that changed from the base of the PR and between 04ee563 and eeae620.

📒 Files selected for processing (8)

• src/engraving/compat/midi/compatmidirender.cpp
• src/engraving/compat/midi/compatmidirenderinternal.cpp
• src/engraving/rw/read400/tread.cpp
• src/engraving/rw/write/twrite.cpp
• src/importexport/midi/tests/midiexport_data/testGraceAfter.mscx
• src/importexport/midi/tests/midiexport_data/testGraceAppoggiatura.mscx
• src/importexport/midi/tests/midiexport_data/testGraceStaleEvents.mscx
• src/importexport/midi/tests/midiexport_tests.cpp