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

Fix trill cue note bugs #18438

Merged
merged 4 commits into from
Jul 20, 2023
Merged

Conversation

mike-spa
Copy link
Contributor

@mike-spa mike-spa commented Jul 6, 2023

Resolves: #18331

Bug 1. Solved -> Trill cue note follows staff move of main note

Bug 3. Solved -> The trill-line should behave consistently with the trill-without-line, meaning it should be deleted when the note is deleted.

Bug 2. Automatically solved by solving bug 3 because creating a tuplet on a note actually causes that note to be deleted (along with all articulations, ornaments etc). We may wish to change this in future but, in that case, that's a separate issue.

@mike-spa mike-spa requested a review from oktophonie July 6, 2023 12:33
@mike-spa mike-spa force-pushed the fixTrillCueNoteBugs branch 2 times, most recently from 92df9d4 to c786c8b Compare July 7, 2023 14:07
EngravingItem* startItem = item->trill()->startElement();
Chord* startChord = startItem && startItem->isChord() ? toChord(startItem) : nullptr;
if (startChord) {
item->setStaffIdx(startChord->vStaffIdx());
Copy link
Contributor

Choose a reason for hiding this comment

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

Please explain why you change the index here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because it needs to follow the staffMove of the chord but, unlike chords, spanner items don't have the staffMove property (i.e. the property of belonging to one staff but displaying in another for cross-staff notation), so the only option is to change the staffIdx itself. It's a slight hack, but I think it's the least-bad option (also cause I don't like staffMove in general). I'll put in a comment

@RomanPudashkin RomanPudashkin merged commit dd21992 into musescore:master Jul 20, 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.

Engraving bugs related to trill cue notes
3 participants