From 8cea54e67b347cddbfc586e3ede32e9ee91bedae Mon Sep 17 00:00:00 2001 From: Roman Pudashkin Date: Wed, 7 Aug 2024 14:52:48 +0300 Subject: [PATCH 1/2] fix #23940: fixed hairpins overlap When a hairpin ends at the same tick at which the next RepeatSegment starts. See PR #20374 --- src/engraving/playback/playbackcontext.cpp | 2 +- .../dynamics/hairpins_and_repeats.mscx | 83 +++++++++++++++++-- .../tests/playback/playbackcontext_tests.cpp | 43 +++++++--- 3 files changed, 110 insertions(+), 18 deletions(-) diff --git a/src/engraving/playback/playbackcontext.cpp b/src/engraving/playback/playbackcontext.cpp index c6b93059411e3..5ee44a0a0baa3 100644 --- a/src/engraving/playback/playbackcontext.cpp +++ b/src/engraving/playback/playbackcontext.cpp @@ -364,7 +364,7 @@ void PlaybackContext::handleSpanners(const ID partId, const Score* score, const return; } - auto intervals = spannerMap.findOverlapping(segmentStartTick, segmentEndTick - 1); + auto intervals = spannerMap.findOverlapping(segmentStartTick + 1, segmentEndTick - 1); for (const auto& interval : intervals) { const Spanner* spanner = interval.value; diff --git a/src/engraving/tests/playback/playbackcontext_data/dynamics/hairpins_and_repeats.mscx b/src/engraving/tests/playback/playbackcontext_data/dynamics/hairpins_and_repeats.mscx index d7dde2fc5c07e..b1b18ff88d643 100644 --- a/src/engraving/tests/playback/playbackcontext_data/dynamics/hairpins_and_repeats.mscx +++ b/src/engraving/tests/playback/playbackcontext_data/dynamics/hairpins_and_repeats.mscx @@ -1,8 +1,8 @@ - - 4.2.0 + + 4.4.0 - 2149 + 3025 480 1 @@ -94,22 +94,92 @@ - - 2 - 47244640280 + 9616431775769 4 4 + + p + 49 + 11879879540772 + + + + 0 + 13052405612638 + + 0 + + + 13056700579905 + + + + + 1 + + + + + 9715216023667 + quarter + + 9710921056276 + 65 + 13 + + + + 9745280794739 + quarter + + 9740985827348 + 65 + 13 + + + + 9783935500403 + quarter + + 9779640533012 + 65 + 13 + + + + 9814000271475 + quarter + + 9809705304084 + 65 + 13 + + + + + + + 2 + f 96 8766028251171 + + + + -1 + + + 0 + down 5549097746526 0 @@ -184,6 +254,7 @@ 0 + down 408021893214 0 diff --git a/src/engraving/tests/playback/playbackcontext_tests.cpp b/src/engraving/tests/playback/playbackcontext_tests.cpp index 479a6512241ae..ea1e12d3e40ad 100644 --- a/src/engraving/tests/playback/playbackcontext_tests.cpp +++ b/src/engraving/tests/playback/playbackcontext_tests.cpp @@ -67,6 +67,9 @@ class Engraving_PlaybackContextTests : public ::testing::Test } }; +//! Checks that hairpins outside/inside repeats don't overlap. See: +//! https://github.com/musescore/MuseScore/issues/16167 +//! https://github.com/musescore/MuseScore/issues/23940 TEST_F(Engraving_PlaybackContextTests, Hairpins_Repeats) { // [GIVEN] Score with hairpins and repeats @@ -87,29 +90,47 @@ TEST_F(Engraving_PlaybackContextTests, Hairpins_Repeats) // [GIVEN] DynamicLevelMap expectedDynamics; - // [GIVEN] 1st hairpin (inside the repeat): f -> fff + // [GIVEN] 1st hairpin (before the repeat): p -> f + constexpr mpe::dynamic_level_t p = dynamicLevelFromType(mpe::DynamicType::p); constexpr mpe::dynamic_level_t f = dynamicLevelFromType(mpe::DynamicType::f); + + const std::map p_to_f_curve = TConv::easingValueCurve(1920, HAIRPIN_STEPS, static_cast(f - p), + ChangeMethod::NORMAL); + + for (const auto& pair : p_to_f_curve) { + mpe::timestamp_t time = timestampFromTicks(score, pair.first); + ASSERT_FALSE(muse::contains(expectedDynamics, time)); + expectedDynamics.emplace(time, p + static_cast(pair.second)); + } + + // [GIVEN] 2nd hairpin (inside the repeat): f -> fff constexpr mpe::dynamic_level_t fff = dynamicLevelFromType(mpe::DynamicType::fff); - constexpr int f_to_fff_startTick = 0; + constexpr int f_to_fff_startTick = 1920; - std::map f_to_fff_curve = TConv::easingValueCurve(1440, HAIRPIN_STEPS, static_cast(fff - f), ChangeMethod::NORMAL); + const std::map f_to_fff_curve = TConv::easingValueCurve(1440, HAIRPIN_STEPS, static_cast(fff - f), + ChangeMethod::NORMAL); for (const mu::engraving::RepeatSegment* repeatSegment : repeats) { int tickPositionOffset = repeatSegment->utick - repeatSegment->tick; for (const auto& pair : f_to_fff_curve) { - mpe::timestamp_t time = timestampFromTicks(score, f_to_fff_startTick + pair.first + tickPositionOffset); - ASSERT_FALSE(muse::contains(expectedDynamics, time)); + int tick = f_to_fff_startTick + pair.first + tickPositionOffset; + mpe::timestamp_t time = timestampFromTicks(score, tick); + + if (tick != f_to_fff_startTick) { // f already added by the previous hairpin + ASSERT_FALSE(muse::contains(expectedDynamics, time)); + } + expectedDynamics.emplace(time, f + static_cast(pair.second)); } } - // [GIVEN] 2nd hairpin (right after the repeat): ppp -> p + // [GIVEN] 3rd hairpin (right after the repeat): ppp -> p constexpr mpe::dynamic_level_t ppp = dynamicLevelFromType(mpe::DynamicType::ppp); - constexpr mpe::dynamic_level_t p = dynamicLevelFromType(mpe::DynamicType::p); - constexpr int ppp_to_p_startTick = 1920 + 1920; // real tick + repeat tick + constexpr int ppp_to_p_startTick = 3840 + 1920; // real tick + repeat tick - std::map ppp_to_p_curve = TConv::easingValueCurve(1440, HAIRPIN_STEPS, static_cast(p - ppp), ChangeMethod::NORMAL); + const std::map ppp_to_p_curve = TConv::easingValueCurve(1440, HAIRPIN_STEPS, static_cast(p - ppp), + ChangeMethod::NORMAL); for (const auto& pair : ppp_to_p_curve) { mpe::timestamp_t time = timestampFromTicks(score, ppp_to_p_startTick + pair.first); @@ -292,8 +313,8 @@ TEST_F(Engraving_PlaybackContextTests, Dynamics_Overlap) constexpr mpe::dynamic_level_t ff = dynamicLevelFromType(mpe::DynamicType::ff); constexpr mpe::dynamic_level_t pp = dynamicLevelFromType(mpe::DynamicType::pp); - std::map ff_to_pp_curve = TConv::easingValueCurve(1920 - Fraction::eps().ticks(), - HAIRPIN_STEPS, static_cast(pp - ff), ChangeMethod::NORMAL); + const std::map ff_to_pp_curve = TConv::easingValueCurve(1920 - Fraction::eps().ticks(), + HAIRPIN_STEPS, static_cast(pp - ff), ChangeMethod::NORMAL); for (const auto& pair : ff_to_pp_curve) { mpe::timestamp_t time = timestampFromTicks(score, pair.first); expectedDynamics.emplace(time, ff + static_cast(pair.second)); From e65f2fcb099b4fc470d4b2621aa3cb7199dbea4e Mon Sep 17 00:00:00 2001 From: Roman Pudashkin Date: Thu, 8 Aug 2024 16:47:36 +0300 Subject: [PATCH 2/2] ignore hairpins on linked staves See: https://github.com/musescore/MuseScore/pull/23948#issuecomment-2273810815 --- src/engraving/playback/playbackcontext.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/engraving/playback/playbackcontext.cpp b/src/engraving/playback/playbackcontext.cpp index 5ee44a0a0baa3..59238870046c0 100644 --- a/src/engraving/playback/playbackcontext.cpp +++ b/src/engraving/playback/playbackcontext.cpp @@ -376,6 +376,11 @@ void PlaybackContext::handleSpanners(const ID partId, const Score* score, const continue; } + const Staff* staff = spanner->staff(); + if (staff && !staff->isPrimaryStaff()) { + continue; // ignore linked staves + } + int spannerFrom = spanner->tick().ticks(); int spannerTo = spannerFrom + std::abs(spanner->ticks().ticks());