Skip to content
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

Allow entering a melisma in voice 2 while a longer note sustains in voice 1 #20232

Merged
merged 1 commit into from
Dec 2, 2023

Conversation

miiizen
Copy link
Contributor

@miiizen miiizen commented Nov 28, 2023

Resolves: #19962

Melismas returned the chord in the first voice found in the segment as their startCR when we really want the chord in the same voice as the melisma.
See below for an example of what couldn't be inputted before.
Screenshot 2023-11-28 at 16 54 15

I opted to add an exception to Spanner::startCR() rather than overriding as I thought that was most appropriate for this single case.

@@ -992,7 +992,8 @@ Chord* Spanner::endChord()
ChordRest* Spanner::startCR()
{
assert(m_anchor == Anchor::SEGMENT || m_anchor == Anchor::CHORD);
if (!m_startElement || m_startElement->score() != score()) {
if (!m_startElement || m_startElement->score() != score()
|| (type() == ElementType::LYRICSLINE && m_startElement->track() != track())) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels a bit hacky. Ideally, startCR() should be const and only return the start element, never compute it. The correct start element should be computed and set earlier, if possible, as soon as the spanner is created. Maybe we can find the function that creates the lyrics line and set the correct start chord from there? Otherwise, Spanner::computeStartElement could also be a good place to put this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. Spanner::computeStartElement is called a few times during layout, so it should probably go there - any startElement we set will get overwritten by this.

@cbjeukendrup cbjeukendrup merged commit f2bd354 into musescore:master Dec 2, 2023
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Voice 2 lyrics melisma diappears
4 participants