From 2bbb3568e52f4f46ecf95b9094eeb9a3bb99d569 Mon Sep 17 00:00:00 2001 From: Roman Pudashkin Date: Mon, 16 Sep 2024 13:03:34 +0300 Subject: [PATCH] fix #23168: correctly calculate the tremolo start time and duration for tied notes --- src/engraving/dom/chord.cpp | 22 +-- src/engraving/dom/chord.h | 1 - .../playback/renderers/tremolorenderer.cpp | 79 +++++++- .../playback/renderers/tremolorenderer.h | 6 +- src/engraving/playback/renderingcontext.h | 9 +- .../single_chord_tremolo_tied_notes.mscx | 179 ++++++++++++++++++ .../playbackeventsrendering_tests.cpp | 69 +++++++ 7 files changed, 341 insertions(+), 24 deletions(-) create mode 100755 src/engraving/tests/playback/playbackeventsrenderer_data/single_chord_tremolo_tied_notes/single_chord_tremolo_tied_notes.mscx diff --git a/src/engraving/dom/chord.cpp b/src/engraving/dom/chord.cpp index f635e8b40a33e..26e5692b8b772 100644 --- a/src/engraving/dom/chord.cpp +++ b/src/engraving/dom/chord.cpp @@ -463,33 +463,17 @@ bool Chord::containsEqualArticulations(const Chord* other) const return true; } -bool Chord::containsEqualArpeggio(const Chord* other) const -{ - if (m_arpeggio && other->m_arpeggio) { - if (m_arpeggio->arpeggioType() != other->m_arpeggio->arpeggioType()) { - return false; - } - } - - return !m_arpeggio && !other->m_arpeggio; -} - bool Chord::containsEqualTremolo(const Chord* other) const { if (tremoloSingleChord() && other->tremoloSingleChord()) { - if (tremoloSingleChord()->tremoloType() != other->tremoloSingleChord()->tremoloType()) { - return false; - } + return tremoloSingleChord()->tremoloType() == other->tremoloSingleChord()->tremoloType(); } if (tremoloTwoChord() && other->tremoloTwoChord()) { - if (tremoloTwoChord()->tremoloType() != other->tremoloTwoChord()->tremoloType()) { - return false; - } + return tremoloTwoChord()->tremoloType() == other->tremoloTwoChord()->tremoloType(); } - return !tremoloSingleChord() && !other->tremoloSingleChord() - && !tremoloTwoChord() && !other->tremoloTwoChord(); + return false; } //--------------------------------------------------------- diff --git a/src/engraving/dom/chord.h b/src/engraving/dom/chord.h index 78efa117f0256..917adbb92b486 100644 --- a/src/engraving/dom/chord.h +++ b/src/engraving/dom/chord.h @@ -98,7 +98,6 @@ class Chord final : public ChordRest Chord& operator=(const Chord&) = delete; bool containsEqualArticulations(const Chord* other) const; - bool containsEqualArpeggio(const Chord* other) const; bool containsEqualTremolo(const Chord* other) const; // Score Tree functions diff --git a/src/engraving/playback/renderers/tremolorenderer.cpp b/src/engraving/playback/renderers/tremolorenderer.cpp index 67a6dd576d849..e82e98a2d244f 100644 --- a/src/engraving/playback/renderers/tremolorenderer.cpp +++ b/src/engraving/playback/renderers/tremolorenderer.cpp @@ -32,6 +32,38 @@ using namespace mu::engraving; using namespace muse; using namespace muse::mpe; +static const Note* findFirstTremoloNote(const Note* note) +{ + while (note && note->tieBack()) { + const Note* prevNote = note->tieBack()->startNote(); + const Chord* prevChord = prevNote ? prevNote->chord() : nullptr; + + if (!prevChord || !prevChord->containsEqualTremolo(note->chord())) { + break; + } + + note = prevNote; + } + + return note; +} + +static const Note* findLastTremoloNote(const Note* note) +{ + while (note && note->tieFor()) { + const Note* nextNote = note->tieFor()->endNote(); + const Chord* nextChord = nextNote ? nextNote->chord() : nullptr; + + if (!nextChord || !nextChord->containsEqualTremolo(note->chord())) { + break; + } + + note = nextNote; + } + + return note; +} + const ArticulationTypeSet& TremoloRenderer::supportedTypes() { static const mpe::ArticulationTypeSet types = { @@ -97,6 +129,8 @@ void TremoloRenderer::doRender(const EngravingItem* item, const mpe::Articulatio stepDurationTicks = overallDurationTicks / stepsCount; + TremoloTimeCache tremoloTimeCache; + if (tremolo.two) { const Chord* firstTremoloChord = tremolo.two->chord1(); const Chord* secondTremoloChord = tremolo.two->chord2(); @@ -113,7 +147,7 @@ void TremoloRenderer::doRender(const EngravingItem* item, const mpe::Articulatio } buildAndAppendEvents(currentChord, preferredType, stepDurationTicks, context.nominalPositionStartTick + i * stepDurationTicks, - context, result); + context, tremoloTimeCache, result); } return; @@ -121,7 +155,7 @@ void TremoloRenderer::doRender(const EngravingItem* item, const mpe::Articulatio for (int i = 0; i < stepsCount; ++i) { buildAndAppendEvents(chord, preferredType, stepDurationTicks, context.nominalPositionStartTick + i * stepDurationTicks, - context, result); + context, tremoloTimeCache, result); } } @@ -137,6 +171,7 @@ int TremoloRenderer::stepDurationTicks(const Chord* chord, int tremoloLines) void TremoloRenderer::buildAndAppendEvents(const Chord* chord, const ArticulationType type, const int stepDurationTicks, const int startTick, const RenderingContext& context, + TremoloTimeCache& tremoloCache, mpe::PlaybackEventList& result) { const Score* score = chord->score(); @@ -160,8 +195,48 @@ void TremoloRenderer::buildAndAppendEvents(const Chord* chord, const Articulatio noteCtx.dynamicLevel = context.playbackCtx->appliableDynamicLevel(note->track(), utick); NoteArticulationsParser::buildNoteArticulationMap(note, context, noteCtx.chordCtx.commonArticulations); + + const TimestampAndDuration& tremoloTnD = tremoloTimeAndDuration(note, context, tremoloCache); + muse::mpe::ArticulationAppliedData& articulationData = noteCtx.chordCtx.commonArticulations.at(type); + articulationData.meta.timestamp = tremoloTnD.timestamp; + articulationData.meta.overallDuration = tremoloTnD.duration; + updateArticulationBoundaries(type, noteCtx.timestamp, noteCtx.duration, noteCtx.chordCtx.commonArticulations); result.emplace_back(buildNoteEvent(std::move(noteCtx))); } } + +const TimestampAndDuration& TremoloRenderer::tremoloTimeAndDuration(const Note* note, const RenderingContext& ctx, + TremoloTimeCache& cache) +{ + auto cacheIt = cache.find(note); + if (cacheIt != cache.end()) { + return cacheIt->second; + } + + TimestampAndDuration& tnd = cache[note]; + tnd.timestamp = ctx.nominalTimestamp; + tnd.duration = ctx.nominalDuration; + + const Score* score = note->score(); + + const Note* firstTremoloNote = findFirstTremoloNote(note); + if (firstTremoloNote) { + if (firstTremoloNote != note) { + tnd.timestamp = timestampFromTicks(score, firstTremoloNote->tick().ticks()); + } + } + + const Note* lastTremoloNote = findLastTremoloNote(note); + if (lastTremoloNote) { + const TimestampAndDuration lastNoteTnD = timestampAndDurationFromStartAndDurationTicks(score, + lastTremoloNote->tick().ticks(), + lastTremoloNote->chord()->actualTicks().ticks(), + ctx.positionTickOffset); + + tnd.duration = lastNoteTnD.timestamp + lastNoteTnD.duration - tnd.timestamp; + } + + return tnd; +} diff --git a/src/engraving/playback/renderers/tremolorenderer.h b/src/engraving/playback/renderers/tremolorenderer.h index e4eb66f7f3612..f6335efe58416 100644 --- a/src/engraving/playback/renderers/tremolorenderer.h +++ b/src/engraving/playback/renderers/tremolorenderer.h @@ -35,9 +35,13 @@ class TremoloRenderer : public RenderBase muse::mpe::PlaybackEventList& result); private: + using TremoloTimeCache = std::unordered_map; + static int stepDurationTicks(const Chord* chord, int tremoloLines); static void buildAndAppendEvents(const Chord* chord, const muse::mpe::ArticulationType type, const int stepDurationTicks, - const int startTick, const RenderingContext& ctx, muse::mpe::PlaybackEventList& result); + const int startTick, const RenderingContext& ctx, TremoloTimeCache& tremoloCache, + muse::mpe::PlaybackEventList& result); + static const TimestampAndDuration& tremoloTimeAndDuration(const Note* note, const RenderingContext& ctx, TremoloTimeCache& cache); }; } diff --git a/src/engraving/playback/renderingcontext.h b/src/engraving/playback/renderingcontext.h index a181b6e24e289..2f45650d91fa6 100644 --- a/src/engraving/playback/renderingcontext.h +++ b/src/engraving/playback/renderingcontext.h @@ -156,7 +156,14 @@ inline bool isNotePlayable(const Note* note, const muse::mpe::ArticulationMap& a const Chord* firstChord = tie->startNote()->chord(); const Chord* lastChord = tie->endNote()->chord(); - return !firstChord->containsEqualTremolo(lastChord); + if (firstChord && lastChord) { + if (firstChord->tremoloType() != TremoloType::INVALID_TREMOLO + || lastChord->tremoloType() != TremoloType::INVALID_TREMOLO) { + return true; + } + } + + return false; } return true; diff --git a/src/engraving/tests/playback/playbackeventsrenderer_data/single_chord_tremolo_tied_notes/single_chord_tremolo_tied_notes.mscx b/src/engraving/tests/playback/playbackeventsrenderer_data/single_chord_tremolo_tied_notes/single_chord_tremolo_tied_notes.mscx new file mode 100755 index 0000000000000..d3a8898203ad5 --- /dev/null +++ b/src/engraving/tests/playback/playbackeventsrenderer_data/single_chord_tremolo_tied_notes/single_chord_tremolo_tied_notes.mscx @@ -0,0 +1,179 @@ + + + 4.4.2 + + 1004 + + 480 + 1 + 1 + 1 + 0 + 1 + + + Composer / arranger + + 2024-09-13 + + + + Apple Macintosh + + + Subtitle + + + Untitled score + + Orchestral + + Keyboards + + + Orchestral Strings + +
+ flutes + oboes + clarinets + saxophones + bassoons + +
+
+ horns + trumpets + cornets + flugelhorns + trombones + tubas +
+
+ timpani +
+
+ keyboard-percussion + drums + unpitched-metal-percussion + unpitched-wooden-percussion + other-percussion +
+ keyboards + harps + organs + synths + +
+ voices + voice-groups +
+
+ orchestral-strings +
+ + + + + + stdNormal + + + 1 + + Violin + + Violin + Vln. + Violin + 55 + 103 + 55 + 88 + strings.violin + + + Fluid + + + + Fluid + + + + Fluid + + + + + + + + 1554778161175 + 0 + + + 1546188226585 + 4 + 4 + + + 4299262263322 + whole + + + + + + + 3083786518643 + whole + + 3079491551252 + + + 4260607557663 + + + + 1 + + + + 65 + 13 + + + r32 + 3423088935049 + + + + + + + + 3083786518643 + whole + + 3079491551252 + + + + -1 + + + + 65 + 13 + + + r32 + 3423088935049 + + + + + + + diff --git a/src/engraving/tests/playback/playbackeventsrendering_tests.cpp b/src/engraving/tests/playback/playbackeventsrendering_tests.cpp index 10aae5d9c0e60..0701955b939da 100644 --- a/src/engraving/tests/playback/playbackeventsrendering_tests.cpp +++ b/src/engraving/tests/playback/playbackeventsrendering_tests.cpp @@ -2360,6 +2360,75 @@ TEST_F(Engraving_PlaybackEventsRendererTests, Single_Chord_Tremolo) } } +/** + * @brief PlaybackEventsRendererTests_Single_Chord_Tremolo_TiedNotes + * @details In this case we're gonna render a simple piece of score with 3 single measures. + * The second measure has a whole note tied to a whole note in the third measure. + * Each note has a Tremolo32nd. + * This test checks that all tied notes have the correct tremolo time and duration + * See: https://github.com/musescore/MuseScore/issues/23168 + */ +TEST_F(Engraving_PlaybackEventsRendererTests, Single_Chord_Tremolo_TiedNotes) +{ + // [GIVEN] Simple piece of score (violin, 4/4, 120 bpm, Treble Cleff) + Score* score + = ScoreRW::readScore(PLAYBACK_EVENTS_RENDERING_DIR + "single_chord_tremolo_tied_notes/single_chord_tremolo_tied_notes.mscx"); + + // [GIVEN] Fulfill articulations profile with dummy patterns + m_defaultProfile->setPattern(ArticulationType::Tremolo32nd, m_dummyPattern); + + // [GIVEN] Dummy context + PlaybackContextPtr ctx = std::make_shared(); + + // [WHEN] Render the score + PlaybackEventsMap result; + + for (const Measure* measure = score->firstMeasure(); measure; measure = measure->nextMeasure()) { + for (const Segment* segment = measure->first(SegmentType::ChordRest); segment; segment = segment->next(SegmentType::ChordRest)) { + const mu::engraving::EngravingItem* element = segment->element(0); + if (element && element->isChord()) { + m_renderer.render(toChord(element), 0, m_defaultProfile, ctx, result); + } + } + } + + // [THEN] We expect 2 lists of events (for 2 tied notes) + EXPECT_EQ(result.size(), 2); + + constexpr timestamp_t expectedTremoloTimestamp = 2000000; // timestamp of the 1st tied note + constexpr timestamp_t expectedTremoloDuration = WHOLE_NOTE_DURATION * 2; // total duration of all tied notes + + constexpr pitch_level_t expectedPitchLevel = pitchLevel(PitchClass::F, 4); + constexpr duration_t expectedSubNoteDuration = 62500; + + for (const auto& pair : result) { + // [THEN] Expected number of sub-notes + EXPECT_EQ(pair.second.size(), 32); + + for (const PlaybackEvent& event : pair.second) { + ASSERT_TRUE(std::holds_alternative(event)); + const mpe::NoteEvent& noteEvent = std::get(event); + + // [THEN] We expect that each note event has only one articulation applied + EXPECT_EQ(noteEvent.expressionCtx().articulations.size(), 1); + ASSERT_TRUE(noteEvent.expressionCtx().articulations.contains(ArticulationType::Tremolo32nd)); + + // [THEN] Each note event has the correct articulation time and duration + const ArticulationAppliedData& articulationData = noteEvent.expressionCtx().articulations.at(ArticulationType::Tremolo32nd); + EXPECT_EQ(articulationData.meta.timestamp, expectedTremoloTimestamp); + EXPECT_EQ(articulationData.meta.overallDuration, expectedTremoloDuration); + + // [THEN] Each note event has the correct pitch level + EXPECT_EQ(noteEvent.pitchCtx().nominalPitchLevel, expectedPitchLevel); + + // [THEN] Each note event has the correct duration + EXPECT_EQ(noteEvent.arrangementCtx().actualDuration, expectedSubNoteDuration); + } + } + + delete score; +} + /** * @brief PlaybackEventsRendererTests_Two_Chords_Tremolo * @details In this case we're gonna render a simple piece of score with a single measure,