-
Notifications
You must be signed in to change notification settings - Fork 2.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
tremolo_tied_notes_fix #24774
base: master
Are you sure you want to change the base?
tremolo_tied_notes_fix #24774
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -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,15 +147,15 @@ void TremoloRenderer::doRender(const EngravingItem* item, const mpe::Articulatio | |||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
buildAndAppendEvents(currentChord, preferredType, stepDurationTicks, context.nominalPositionStartTick + i * stepDurationTicks, | ||||||||||||||||||||||||
context, result); | ||||||||||||||||||||||||
context, tremoloTimeCache, result); | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
return; | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
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))); | ||||||||||||||||||||||||
Comment on lines
+198
to
206
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How does it work exactly, is the entire tremolo rendered for the first tied note? And are subsequent tied notes encountered here too, or are they filtered out already? If we encounter them here, won't we render the tremolo multiple times? I don't fully understand why we need to cache something because I suppose we see each note only once, don't we? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We're rendering all tied notes (not just the 1st one) that have the tremolo articulation. For each tied note we're creating stepCount sub-events (see TremoloRenderer::doRender; there is a for loop there). Each sub-event has ArticulationMeta that stores the global start position and duration of the tremolo, which are then used to trigger a single continuous sound (if the instrument supports it, otherwise we will just play multiple note on/off events). The cache is used to avoid recalculation the start position and duration for sub-events |
||||||||||||||||||||||||
} | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
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) { | ||||||||||||||||||||||||
Comment on lines
+224
to
+226
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Either
Suggested change
or
Suggested change
|
||||||||||||||||||||||||
tnd.timestamp = timestampFromTicks(score, firstTremoloNote->tick().ticks()); | ||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think you need to do something with
Suggested change
|
||||||||||||||||||||||||
} | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
const Note* lastTremoloNote = findLastTremoloNote(note); | ||||||||||||||||||||||||
if (lastTremoloNote) { | ||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need to check |
||||||||||||||||||||||||
const TimestampAndDuration lastNoteTnD = timestampAndDurationFromStartAndDurationTicks(score, | ||||||||||||||||||||||||
lastTremoloNote->tick().ticks(), | ||||||||||||||||||||||||
lastTremoloNote->chord()->actualTicks().ticks(), | ||||||||||||||||||||||||
ctx.positionTickOffset); | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
tnd.duration = lastNoteTnD.timestamp + lastNoteTnD.duration - tnd.timestamp; | ||||||||||||||||||||||||
Comment on lines
+233
to
+238
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Simpler:
Suggested change
I.e. instead of doing the addition in |
||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
return tnd; | ||||||||||||||||||||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,179 @@ | ||
<?xml version="1.0" encoding="UTF-8"?> | ||
<museScore version="4.40"> | ||
<programVersion>4.4.2</programVersion> | ||
<programRevision></programRevision> | ||
<LastEID>1004</LastEID> | ||
<Score> | ||
<Division>480</Division> | ||
<showInvisible>1</showInvisible> | ||
<showUnprintable>1</showUnprintable> | ||
<showFrames>1</showFrames> | ||
<showMargins>0</showMargins> | ||
<open>1</open> | ||
<metaTag name="arranger"></metaTag> | ||
<metaTag name="audioComUrl"></metaTag> | ||
<metaTag name="composer">Composer / arranger</metaTag> | ||
<metaTag name="copyright"></metaTag> | ||
<metaTag name="creationDate">2024-09-13</metaTag> | ||
<metaTag name="lyricist"></metaTag> | ||
<metaTag name="movementNumber"></metaTag> | ||
<metaTag name="movementTitle"></metaTag> | ||
<metaTag name="platform">Apple Macintosh</metaTag> | ||
<metaTag name="source"></metaTag> | ||
<metaTag name="sourceRevisionId"></metaTag> | ||
<metaTag name="subtitle">Subtitle</metaTag> | ||
<metaTag name="translator"></metaTag> | ||
<metaTag name="workNumber"></metaTag> | ||
<metaTag name="workTitle">Untitled score</metaTag> | ||
<Order id="orchestral"> | ||
<name>Orchestral</name> | ||
<instrument id="piano"> | ||
<family id="keyboards">Keyboards</family> | ||
</instrument> | ||
<instrument id="violin"> | ||
<family id="orchestral-strings">Orchestral Strings</family> | ||
</instrument> | ||
<section id="woodwind" brackets="true" barLineSpan="true" thinBrackets="true"> | ||
<family>flutes</family> | ||
<family>oboes</family> | ||
<family>clarinets</family> | ||
<family>saxophones</family> | ||
<family>bassoons</family> | ||
<unsorted group="woodwinds"/> | ||
</section> | ||
<section id="brass" brackets="true" barLineSpan="true" thinBrackets="true"> | ||
<family>horns</family> | ||
<family>trumpets</family> | ||
<family>cornets</family> | ||
<family>flugelhorns</family> | ||
<family>trombones</family> | ||
<family>tubas</family> | ||
</section> | ||
<section id="timpani" brackets="true" barLineSpan="true" thinBrackets="true"> | ||
<family>timpani</family> | ||
</section> | ||
<section id="percussion" brackets="true" barLineSpan="true" thinBrackets="true"> | ||
<family>keyboard-percussion</family> | ||
<family>drums</family> | ||
<family>unpitched-metal-percussion</family> | ||
<family>unpitched-wooden-percussion</family> | ||
<family>other-percussion</family> | ||
</section> | ||
<family>keyboards</family> | ||
<family>harps</family> | ||
<family>organs</family> | ||
<family>synths</family> | ||
<soloists/> | ||
<section id="voices" brackets="true" barLineSpan="false" thinBrackets="true"> | ||
<family>voices</family> | ||
<family>voice-groups</family> | ||
</section> | ||
<section id="strings" brackets="true" barLineSpan="true" thinBrackets="true"> | ||
<family>orchestral-strings</family> | ||
</section> | ||
<unsorted/> | ||
</Order> | ||
<Part id="1"> | ||
<Staff id="1"> | ||
<StaffType group="pitched"> | ||
<name>stdNormal</name> | ||
</StaffType> | ||
<bracket type="1" span="2" col="2" visible="1"/> | ||
<barLineSpan>1</barLineSpan> | ||
</Staff> | ||
<trackName>Violin</trackName> | ||
<Instrument id="violin"> | ||
<longName>Violin</longName> | ||
<shortName>Vln.</shortName> | ||
<trackName>Violin</trackName> | ||
<minPitchP>55</minPitchP> | ||
<maxPitchP>103</maxPitchP> | ||
<minPitchA>55</minPitchA> | ||
<maxPitchA>88</maxPitchA> | ||
<instrumentId>strings.violin</instrumentId> | ||
<Channel name="arco"> | ||
<program value="40"/> | ||
<synti>Fluid</synti> | ||
</Channel> | ||
<Channel name="pizzicato"> | ||
<program value="45"/> | ||
<synti>Fluid</synti> | ||
</Channel> | ||
<Channel name="tremolo"> | ||
<program value="44"/> | ||
<synti>Fluid</synti> | ||
</Channel> | ||
</Instrument> | ||
</Part> | ||
<Staff id="1"> | ||
<Measure> | ||
<voice> | ||
<KeySig> | ||
<eid>1554778161175</eid> | ||
<concertKey>0</concertKey> | ||
</KeySig> | ||
<TimeSig> | ||
<eid>1546188226585</eid> | ||
<sigN>4</sigN> | ||
<sigD>4</sigD> | ||
</TimeSig> | ||
<Rest> | ||
<eid>4299262263322</eid> | ||
<durationType>whole</durationType> | ||
</Rest> | ||
</voice> | ||
</Measure> | ||
<Measure> | ||
<voice> | ||
<Chord> | ||
<eid>3083786518643</eid> | ||
<durationType>whole</durationType> | ||
<Note> | ||
<eid>3079491551252</eid> | ||
<Spanner type="Tie"> | ||
<Tie> | ||
<eid>4260607557663</eid> | ||
</Tie> | ||
<next> | ||
<location> | ||
<measures>1</measures> | ||
</location> | ||
</next> | ||
</Spanner> | ||
<pitch>65</pitch> | ||
<tpc>13</tpc> | ||
</Note> | ||
<TremoloSingleChord> | ||
<subtype>r32</subtype> | ||
<eid>3423088935049</eid> | ||
</TremoloSingleChord> | ||
</Chord> | ||
</voice> | ||
</Measure> | ||
<Measure> | ||
<voice> | ||
<Chord> | ||
<eid>3083786518643</eid> | ||
<durationType>whole</durationType> | ||
<Note> | ||
<eid>3079491551252</eid> | ||
<Spanner type="Tie"> | ||
<prev> | ||
<location> | ||
<measures>-1</measures> | ||
</location> | ||
</prev> | ||
</Spanner> | ||
<pitch>65</pitch> | ||
<tpc>13</tpc> | ||
</Note> | ||
<TremoloSingleChord> | ||
<subtype>r32</subtype> | ||
<eid>3423088935049</eid> | ||
</TremoloSingleChord> | ||
</Chord> | ||
</voice> | ||
</Measure> | ||
</Staff> | ||
</Score> | ||
</museScore> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The methods
Note::first/lastTiedNote
contain some additional complications, one seemingly to prevent infinite loops. It might be good to investigate why these complications are necessary there, and if they are necessary here too.