diff --git a/src/engraving/compat/midi/compatmidirender.cpp b/src/engraving/compat/midi/compatmidirender.cpp index 84e6e552c338b..36ac37afd1fc3 100644 --- a/src/engraving/compat/midi/compatmidirender.cpp +++ b/src/engraving/compat/midi/compatmidirender.cpp @@ -597,9 +597,12 @@ void CompatMidiRender::createGraceNotesPlayEvents(const Score* score, const Frac el.push_back(nel); } - if (gc->playEventType() == PlayEventType::Auto) { - gc->setNoteEventLists(el); - } + // Always overwrite grace note events: MuseScore 4 exposes no UI for + // manually editing grace note MIDI timing, so any stored PlayEventType::User + // on a grace chord is a stale artifact from an earlier render. Honouring it + // would leave zero-duration events (saved by old buggy renders) in place and + // make the grace note silent in MIDI export. + gc->setNoteEventLists(el); on += graceDuration; } @@ -634,9 +637,7 @@ void CompatMidiRender::createGraceNotesPlayEvents(const Score* score, const Frac el.push_back(nel); } - if (gc->playEventType() == PlayEventType::Auto) { - gc->setNoteEventLists(el); - } + gc->setNoteEventLists(el); on += graceDuration1; } } diff --git a/src/engraving/compat/midi/compatmidirenderinternal.cpp b/src/engraving/compat/midi/compatmidirenderinternal.cpp index 1943144921e89..9b687d01bc1e4 100644 --- a/src/engraving/compat/midi/compatmidirenderinternal.cpp +++ b/src/engraving/compat/midi/compatmidirenderinternal.cpp @@ -789,7 +789,10 @@ static void collectNote(EventsHolder& events, const Note* note, const CollectNot // calculate additional length due to ties forward // taking NoteEvent length adjustments into account - int tick1 = note->tick().ticks() + noteParams.tickOffset; + // Grace notes are exported relative to their principal chord and then shifted + // earlier via grace offsets. Using the grace note's own tick here double-shifts + // acciaccaturas in the MIDI export path. + int tick1 = chord->tick().ticks() + noteParams.tickOffset; const GuitarBend* bendFor = note->bendFor(); const GuitarBend* bendBack = note->bendBack(); @@ -861,8 +864,15 @@ static void collectNote(EventsHolder& events, const Note* note, const CollectNot int eventChannel = noteChannel; playParams.pitch = p; playParams.velo = std::clamp(velo, 1, 127); - playParams.onTime = std::max(0, on - noteParams.graceOffsetOn); - playParams.offTime = std::max(0, off - noteParams.graceOffsetOff); + int rawOnTime = on - noteParams.graceOffsetOn; + playParams.onTime = std::max(0, rawOnTime); + if (noteParams.graceOffsetOn > 0 && rawOnTime < 0) { + // Grace before beat was clamped (e.g., first beat of piece). + // Preserve the intended duration so the grace remains audible. + playParams.offTime = playParams.onTime + noteParams.graceOffsetOn - 1; + } else { + playParams.offTime = std::max(0, off - noteParams.graceOffsetOff); + } if (eventEffect == MidiInstrumentEffect::NONE) { eventEffect = midiEffectFromEvent(e); diff --git a/src/engraving/rw/read400/tread.cpp b/src/engraving/rw/read400/tread.cpp index 04448e9c98a9b..cbe7bd8662493 100644 --- a/src/engraving/rw/read400/tread.cpp +++ b/src/engraving/rw/read400/tread.cpp @@ -3184,7 +3184,10 @@ bool TRead::readProperties(Note* n, XmlReader& e, ReadContext& ctx) } } n->setPlayEvents(playEvents); - if (n->chord()) { + // Don't mark grace chords as User: their stored events are always stale + // render artifacts (MuseScore 4 has no UI for editing grace note timing), + // and honouring the flag would suppress recomputation on the next render. + if (n->chord() && !n->chord()->isGrace()) { n->chord()->setPlayEventType(PlayEventType::User); } } else if (tag == "offset") { diff --git a/src/engraving/rw/write/twrite.cpp b/src/engraving/rw/write/twrite.cpp index 57e8884f54711..80d92d2085909 100644 --- a/src/engraving/rw/write/twrite.cpp +++ b/src/engraving/rw/write/twrite.cpp @@ -2468,7 +2468,14 @@ void TWrite::write(const Note* item, XmlWriter& xml, WriteContext& ctx) writeSpannerEnd(item->tieBack(), xml, ctx, item, item->track()); } - if ((item->chord() == 0 || item->chord()->playEventType() != PlayEventType::Auto) && !item->playEvents().empty()) { + // Never persist play events for grace note chords: MuseScore 4 exposes no UI + // for editing grace note MIDI timing, so stored events are always stale render + // artifacts. Skipping them here breaks the write→load→User→skip-recompute cycle + // that caused grace notes to be silent after a score was saved and reopened. + const bool isGraceNote = item->chord() && item->chord()->isGrace(); + if (!isGraceNote + && (item->chord() == 0 || item->chord()->playEventType() != PlayEventType::Auto) + && !item->playEvents().empty()) { xml.startElement("Events"); for (const NoteEvent& e : item->playEvents()) { write(&e, xml, ctx); diff --git a/src/importexport/midi/tests/midiexport_data/testGraceAfter.mscx b/src/importexport/midi/tests/midiexport_data/testGraceAfter.mscx new file mode 100644 index 0000000000000..7b162adc7fbc8 --- /dev/null +++ b/src/importexport/midi/tests/midiexport_data/testGraceAfter.mscx @@ -0,0 +1,82 @@ + + + + 480 + + 1 + 1 + 1 + 0 + TestGraceAfter + + + + stdNormal + + + Piano + + Piano + Pno. + Piano + 21 + 108 + 21 + 108 + + 100 + 95 + + + + + + + + + 10 + + + TestGraceAfter + + + + + + 1 + 1 + metNoteQuarterUp = 60 + + + mf + 80 + + + + eighth + + + 62 + 16 + + + + quarter + + 60 + 14 + + + + quarter + + + half + + + + + + diff --git a/src/importexport/midi/tests/midiexport_data/testGraceAppoggiatura.mscx b/src/importexport/midi/tests/midiexport_data/testGraceAppoggiatura.mscx new file mode 100644 index 0000000000000..a1d18cff10e7e --- /dev/null +++ b/src/importexport/midi/tests/midiexport_data/testGraceAppoggiatura.mscx @@ -0,0 +1,92 @@ + + + + 480 + + 1 + 1 + 1 + 0 + TestGraceAppoggiatura + + + + stdNormal + + + Piano + + Piano + Pno. + Piano + 21 + 108 + 21 + 108 + + 100 + 95 + + + + + + + + + 10 + + + TestGraceAppoggiatura + + + + + + 1 + 1 + metNoteQuarterUp = 60 + + + mf + 80 + + + + eighth + + + 71 + 19 + + + + quarter + + 72 + 14 + + + + + eighth + + + 71 + 19 + + + + half + + 72 + 14 + + + + + + + diff --git a/src/importexport/midi/tests/midiexport_data/testGraceStaleEvents.mscx b/src/importexport/midi/tests/midiexport_data/testGraceStaleEvents.mscx new file mode 100644 index 0000000000000..4da2dafd1a79f --- /dev/null +++ b/src/importexport/midi/tests/midiexport_data/testGraceStaleEvents.mscx @@ -0,0 +1,109 @@ + + + + 480 + + 1 + 1 + 1 + 0 + TestGraceStaleEvents + + + + stdNormal + + + Piano + + Piano + Pno. + Piano + 21 + 108 + 21 + 108 + + 100 + 95 + + + + + + + + + 10 + + + TestGraceStaleEvents + + + + + + 1 + 1 + metNoteQuarterUp = 60 + + + mf + 80 + + + + eighth + + + 71 + 19 + + + 0 + + + + + + half + + 72 + 14 + + + + + eighth + + + 71 + 19 + + + 0 + + + + + + quarter + + 72 + 14 + + + + quarter + + + + + + diff --git a/src/importexport/midi/tests/midiexport_tests.cpp b/src/importexport/midi/tests/midiexport_tests.cpp index c3ebb9e9cf863..d2bc6a85847bf 100644 --- a/src/importexport/midi/tests/midiexport_tests.cpp +++ b/src/importexport/midi/tests/midiexport_tests.cpp @@ -26,6 +26,7 @@ #include +#include #include #include "global/io/ifilesystem.h" @@ -42,6 +43,7 @@ #include "engraving/tests/utils/scorerw.h" #include "importexport/midi/internal/midiexport/exportmidi.h" +#include "importexport/midi/internal/midishared/midifile.h" #include "utils/smfyamlserializer.h" @@ -101,6 +103,24 @@ static void exportAndCompareWithRef(const std::string& name) class MidiExportTests : public ::testing::Test { protected: + /** + * Search \p track for a MIDI NoteOn event at exactly \p tick with the given \p pitch and + * \p velocity. Returns a pointer to the matching event, or \c nullptr if none is found. + * Used by grace-note tests to assert that specific on/off events exist at expected positions. + */ + static const MidiEvent* findNoteEvent(const MidiTrack& track, int tick, int pitch, int velocity) + { + auto [begin, end] = track.events().equal_range(tick); + for (auto it = begin; it != end; ++it) { + const MidiEvent& event = it->second; + if (event.type() == ME_NOTEON && event.pitch() == pitch && event.velo() == velocity) { + return &event; + } + } + + return nullptr; + } + static void testTimeStretchFermata(MasterScore* score, const String& file, const String& testName) { const String writeFile = String(u"%1-%2-test-%3.mid").arg(file).arg(testName); @@ -214,6 +234,181 @@ TEST_F(MidiExportTests, midiMutedUnison) { exportAndCompareWithRef("testMutedUnison"); } +/// Verifies acciaccatura (grace-before) MIDI timing: grace notes start before the beat, +/// stealing time from the previous note. Covers tick=0 edge case (no prior note to steal from), +/// single and multiple grace notes, and both 60 bpm and 320 bpm tempos. +TEST_F(MidiExportTests, midiGraceBefore) +{ + std::unique_ptr score(ScoreRW::readScore(MIDI_EXPORT_DATA_DIR + u"/testGraceBefore.mscx")); + ASSERT_TRUE(score); + score->doLayout(); + score->rebuildMidiMapping(); + + ExportMidi exporter(score.get()); + QBuffer buffer; + ASSERT_TRUE(buffer.open(QIODevice::ReadWrite)); + ASSERT_TRUE(exporter.write(&buffer, true, true)); + + buffer.seek(0); + MidiFile midiFile; + ASSERT_TRUE(midiFile.read(&buffer)); + ASSERT_FALSE(midiFile.tracks().empty()); + + const MidiTrack& track = midiFile.tracks().front(); + + // Acciaccatura starts BEFORE the beat, stealing time from the previous note. + // graceTickSum = min(prevChord->ticks()/2, grace->notatedTicks()). + // At 60 bpm: quarter=480, eighth=240. + + // Measure 1 beat 1 (tick 0): grace eighth (240 ticks), prevChord=null → graceTickSum=240. + // No previous note to steal from, so grace is clamped to tick 0 with duration preserved. + EXPECT_NE(findNoteEvent(track, 0, 71, 80), nullptr); // grace on (clamped) + EXPECT_NE(findNoteEvent(track, 239, 71, 0), nullptr); // grace off at 239 (duration preserved) + EXPECT_NE(findNoteEvent(track, 0, 72, 80), nullptr); // principal on at beat + + // Measure 1 beat 3 (tick 480): grace eighth, prevChord=eighth(240) → graceTickSum=min(120,240)=120. + EXPECT_NE(findNoteEvent(track, 360, 71, 80), nullptr); // grace on 120 ticks before beat + EXPECT_NE(findNoteEvent(track, 479, 71, 0), nullptr); // grace off at beat-1 (off=on-1 for len=0) + EXPECT_NE(findNoteEvent(track, 480, 72, 80), nullptr); // principal on at beat + + // Measure 1 beat 4 (tick 960): grace eighth, prevChord=quarter(480) → graceTickSum=min(240,240)=240. + EXPECT_NE(findNoteEvent(track, 720, 71, 80), nullptr); // grace on 240 ticks before beat + EXPECT_NE(findNoteEvent(track, 960, 72, 80), nullptr); // principal on at beat + + // Measure 2 beat 1 (tick 1920): grace eighth, prevChord=half(960) → graceTickSum=min(480,240)=240. + EXPECT_NE(findNoteEvent(track, 1680, 71, 80), nullptr); // grace on 240 ticks before beat + EXPECT_NE(findNoteEvent(track, 1920, 72, 80), nullptr); // principal on at beat + + // Measure 3 beat 1 (tick 3840): 3 grace eighths, prevChord=whole(1920) → graceTickSum=min(960,240)=240, offset=80. + EXPECT_NE(findNoteEvent(track, 3600, 74, 80), nullptr); // grace 1 on at 3840-240 + EXPECT_NE(findNoteEvent(track, 3680, 72, 80), nullptr); // grace 2 on at 3840-160 + EXPECT_NE(findNoteEvent(track, 3760, 71, 80), nullptr); // grace 3 on at 3840-80 + EXPECT_NE(findNoteEvent(track, 3840, 72, 80), nullptr); // principal on at beat + + // Measures 4-6 at 320 bpm. Tick positions identical to 60 bpm (same score ticks, different wall-clock time). + + // Measure 4 beat 1 (tick 5760): grace eighth, prevChord=measure-3 whole(1920) → graceTickSum=240. + EXPECT_NE(findNoteEvent(track, 5520, 71, 80), nullptr); // grace on 240 ticks before beat + EXPECT_NE(findNoteEvent(track, 5760, 72, 80), nullptr); // principal on at beat + + // Measure 4 beat 3 (tick 6240): grace eighth, prevChord=eighth(240) → graceTickSum=120. + EXPECT_NE(findNoteEvent(track, 6120, 71, 80), nullptr); // grace on 120 ticks before beat + EXPECT_NE(findNoteEvent(track, 6240, 72, 80), nullptr); // principal on at beat + + // Measure 4 beat 4 (tick 6720): grace eighth, prevChord=quarter(480) → graceTickSum=240. + EXPECT_NE(findNoteEvent(track, 6480, 71, 80), nullptr); // grace on 240 ticks before beat + EXPECT_NE(findNoteEvent(track, 6720, 72, 80), nullptr); // principal on at beat + + // Measure 5 beat 1 (tick 7680): grace eighth, prevChord=half(960) → graceTickSum=240. + EXPECT_NE(findNoteEvent(track, 7440, 71, 80), nullptr); // grace on 240 ticks before beat + EXPECT_NE(findNoteEvent(track, 7680, 72, 80), nullptr); // principal on at beat + + // Measure 6 beat 1 (tick 9600): 3 grace eighths, prevChord=whole(1920) → graceTickSum=240, offset=80. + EXPECT_NE(findNoteEvent(track, 9360, 74, 80), nullptr); // grace 1 on at 9600-240 + EXPECT_NE(findNoteEvent(track, 9440, 72, 80), nullptr); // grace 2 on at 9600-160 + EXPECT_NE(findNoteEvent(track, 9520, 71, 80), nullptr); // grace 3 on at 9600-80 + EXPECT_NE(findNoteEvent(track, 9600, 72, 80), nullptr); // principal on at beat +} + +/// Verifies grace-after (trailtime) MIDI timing: the grace note starts at the midpoint +/// of the principal note (trailtime=500 per-1000). +TEST_F(MidiExportTests, midiGraceAfter) +{ + std::unique_ptr score(ScoreRW::readScore(MIDI_EXPORT_DATA_DIR + u"/testGraceAfter.mscx")); + ASSERT_TRUE(score); + score->doLayout(); + score->rebuildMidiMapping(); + + ExportMidi exporter(score.get()); + QBuffer buffer; + ASSERT_TRUE(buffer.open(QIODevice::ReadWrite)); + ASSERT_TRUE(exporter.write(&buffer, true, true)); + + buffer.seek(0); + MidiFile midiFile; + ASSERT_TRUE(midiFile.read(&buffer)); + ASSERT_FALSE(midiFile.tracks().empty()); + + const MidiTrack& track = midiFile.tracks().front(); + + // At 60 bpm, quarter = 480 ticks. Grace-after (trailtime=500) starts at 480*500/1000 = 240. + EXPECT_NE(findNoteEvent(track, 0, 60, 80), nullptr); // principal C4 on beat + EXPECT_NE(findNoteEvent(track, 240, 62, 80), nullptr); // grace-after D4 at midpoint +} + +/// Verifies appoggiatura MIDI timing: the grace note plays on the beat and takes a +/// proportional slice of the principal note's duration — min(500, graceNotated/principal * 1000) +/// per-1000. Covers both quarter and half principal note durations. +TEST_F(MidiExportTests, midiGraceAppoggiatura) +{ + std::unique_ptr score(ScoreRW::readScore(MIDI_EXPORT_DATA_DIR + u"/testGraceAppoggiatura.mscx")); + ASSERT_TRUE(score); + score->doLayout(); + score->rebuildMidiMapping(); + + ExportMidi exporter(score.get()); + QBuffer buffer; + ASSERT_TRUE(buffer.open(QIODevice::ReadWrite)); + ASSERT_TRUE(exporter.write(&buffer, true, true)); + + buffer.seek(0); + MidiFile midiFile; + ASSERT_TRUE(midiFile.read(&buffer)); + ASSERT_FALSE(midiFile.tracks().empty()); + + const MidiTrack& track = midiFile.tracks().front(); + + // At 60 bpm, quarter = 480 ticks. Eighth appoggiatura takes 50% of quarter = 240 ticks. + EXPECT_NE(findNoteEvent(track, 0, 71, 80), nullptr); // appoggiatura B4 on beat + EXPECT_NE(findNoteEvent(track, 240, 72, 80), nullptr); // principal C5 at +240 + + // Eighth appoggiatura before half (960 ticks). Grace notated duration = eighth = 240 ticks, + // which is 240/960 = 25% of the principal → ontime = min(500, 250) = 250 per-1000 + // → grace duration = 960 * 250/1000 = 240 ticks. + EXPECT_NE(findNoteEvent(track, 480, 71, 80), nullptr); // appoggiatura B4 on beat + EXPECT_NE(findNoteEvent(track, 720, 72, 80), nullptr); // principal C5 at +240 +} + +/// Regression test for the stale-events bug (issue #22669): grace notes whose NoteEvent +/// was previously saved to the .mscx file with len=0 must still export with correct MIDI +/// timing. Loading such a file used to mark the chord PlayEventType::User, causing +/// createGraceNotesPlayEvents() to skip recomputation and leave the silent len=0 in place. +TEST_F(MidiExportTests, midiGraceStaleEvents) +{ + // Regression test: grace notes whose NoteEvent was saved to the score file with len=0 + // (by an older MuseScore version) must still be exported with correct timing. + // When a note has stored , loading marks the chord PlayEventType::User, which + // previously caused createGraceNotesPlayEvents() to skip recomputing the grace events, + // leaving the stale len=0 in place and making the grace note silent in MIDI export. + std::unique_ptr score(ScoreRW::readScore(MIDI_EXPORT_DATA_DIR + u"/testGraceStaleEvents.mscx")); + ASSERT_TRUE(score); + score->doLayout(); + score->rebuildMidiMapping(); + + ExportMidi exporter(score.get()); + QBuffer buffer; + ASSERT_TRUE(buffer.open(QIODevice::ReadWrite)); + ASSERT_TRUE(exporter.write(&buffer, true, true)); + + buffer.seek(0); + MidiFile midiFile; + ASSERT_TRUE(midiFile.read(&buffer)); + ASSERT_FALSE(midiFile.tracks().empty()); + + const MidiTrack& track = midiFile.tracks().front(); + + // At 60 bpm: quarter=480, half=960 ticks. + // Appoggiatura (eighth) before half at beat 1 (tick 0): + // ontime = min(500, 500ms/2000ms*1000) = 250 per-1000 → grace ends at tick 239, principal at tick 240. + EXPECT_NE(findNoteEvent(track, 0, 71, 80), nullptr); // appoggiatura on at beat (stale len=0 overridden) + EXPECT_NE(findNoteEvent(track, 240, 72, 80), nullptr); // principal on after grace + + // Acciaccatura (eighth) before quarter at beat 3 (tick 960): + // prevChord=half(960), graceTickSum=min(480,240)=240 → grace starts 240 ticks before beat. + EXPECT_NE(findNoteEvent(track, 720, 71, 80), nullptr); // acciaccatura on before beat (stale len=0 overridden) + EXPECT_NE(findNoteEvent(track, 960, 72, 80), nullptr); // principal on at beat +} + TEST_F(MidiExportTests, midiMeasureRepeats) { exportAndCompareWithRef("testMeasureRepeats"); } diff --git a/src/importexport/musicxml/tests/data/testBackupRoundingError_ref.mscx b/src/importexport/musicxml/tests/data/testBackupRoundingError_ref.mscx index 19e8be9328282..24b3b0aa7f409 100644 --- a/src/importexport/musicxml/tests/data/testBackupRoundingError_ref.mscx +++ b/src/importexport/musicxml/tests/data/testBackupRoundingError_ref.mscx @@ -229,11 +229,6 @@ d_d 2 - - - 0 - - 75 11 @@ -1041,11 +1036,6 @@ jC_jC - - - 0 - - 78 20 diff --git a/src/importexport/musicxml/tests/data/testCueGraceNotes_ref.mscx b/src/importexport/musicxml/tests/data/testCueGraceNotes_ref.mscx index 20d731a7fd8e8..088056aad0934 100644 --- a/src/importexport/musicxml/tests/data/testCueGraceNotes_ref.mscx +++ b/src/importexport/musicxml/tests/data/testCueGraceNotes_ref.mscx @@ -161,11 +161,6 @@ N_N - - - 0 - - 65 13 0 @@ -261,11 +256,6 @@ d_d - - - 0 - - 69 17 0 @@ -345,11 +335,6 @@ r_r - - - 0 - - 69 17 0 @@ -454,11 +439,6 @@ +_+ - - - 0 - - 67 15 @@ -537,11 +517,6 @@ NB_NB - - - 0 - - 60 14 @@ -597,11 +572,6 @@ accidentalNatural YB_YB - - - 0 - - 74 16 @@ -788,11 +758,6 @@ accidentalFlat 6B_6B - - - 0 - - 70 12 0 @@ -912,11 +877,6 @@ accidentalSharp PC_PC - - - 0 - - 66 20 0 @@ -994,11 +954,6 @@ cC_cC - - - 0 - - 65 13 0 diff --git a/src/importexport/musicxml/tests/data/testDuplicateFermataOnGraceNoteAndMainNote_ref.mscx b/src/importexport/musicxml/tests/data/testDuplicateFermataOnGraceNoteAndMainNote_ref.mscx index 30a952b1f38fd..ab0cd3141e7a8 100644 --- a/src/importexport/musicxml/tests/data/testDuplicateFermataOnGraceNoteAndMainNote_ref.mscx +++ b/src/importexport/musicxml/tests/data/testDuplicateFermataOnGraceNoteAndMainNote_ref.mscx @@ -258,11 +258,6 @@ T_T - - - 0 - - 83 19 diff --git a/src/importexport/musicxml/tests/data/testDuplicateFermataOnGraceNote_ref.mscx b/src/importexport/musicxml/tests/data/testDuplicateFermataOnGraceNote_ref.mscx index 9b66777b2c7a4..d070d549b2835 100644 --- a/src/importexport/musicxml/tests/data/testDuplicateFermataOnGraceNote_ref.mscx +++ b/src/importexport/musicxml/tests/data/testDuplicateFermataOnGraceNote_ref.mscx @@ -250,11 +250,6 @@ R_R - - - 0 - - 83 19 diff --git a/src/importexport/musicxml/tests/data/testSticking_ref.mscx b/src/importexport/musicxml/tests/data/testSticking_ref.mscx index ebbad1db00da9..051b68d1bc5de 100644 --- a/src/importexport/musicxml/tests/data/testSticking_ref.mscx +++ b/src/importexport/musicxml/tests/data/testSticking_ref.mscx @@ -455,11 +455,6 @@ r_r - - - 0 - - 38 16 @@ -549,11 +544,6 @@ 7_7 - - - 0 - - 38 16 @@ -623,11 +613,6 @@ IB_IB - - - 0 - - 38 16 @@ -638,11 +623,6 @@ KB_KB - - - 0 - - 38 16 @@ -653,11 +633,6 @@ MB_MB - - - 0 - - 38 16 @@ -668,11 +643,6 @@ OB_OB - - - 0 - - 38 16