fix #166016 slur on grace after wrong #3380
Conversation
mscore/scoreview.cpp
Outdated
| @@ -4487,6 +4487,11 @@ void ScoreView::cmdAddSlur() | |||
| return; | |||
| if (firstNote == lastNote) | |||
| lastNote = 0; | |||
| else if (firstNote->chord()->isGraceAfter()) { | |||
There was a problem hiding this comment.
Do we understand why the notes are reversed, and are we sure it is both a) always true for grace notes after, and b) never not true for grace notes after? Personally, I'd rather see the the basic algorithm fixed rather than patched at the end.
There was a problem hiding this comment.
I agree with your doubts and revised the algorithm. Now it should cover every possible case, including grace notes before and after.
|
Please make sure the issue numbers are mentioned in the commit title. having them only in the PR title don't result in the issues to automagically get marked as fixed once the PR got merged. |
|
I changed the commit titles |
|
I guess we don't really need the two commits here right ? Can they be squashed? |
|
Shure. |
|
In general we don't accept PR on the 2.2 branch. The development happens on the master branch only and fixes that apply to 2.2 are manually cherry picked to the 2.2 branch. If you plan to do other PR, please use the master branch. |
|
Unless the PR applies to 2.2 only and not to master at all, or needs to be very different between the 2 |
|
So I should close this and do another PR on master? Since the original code looks very similar in 2.2. an master. |
|
I believe @lasconic would let this one here pass as is (once squashed) ;-), as far as I understood he's talking about future PRs from you |
…selection ends up with the two notes reversed. This happens only if a gracenote after is part of the selection. So I decided to test for this case and simply commute the two values.
…note in a list of selected notes. It respects notes as well as gracenotes before and after them.
|
BTW: this makes me wonder whether #3381 was really meant for 2.2 only? |
|
#3381 fixes three issues in 2.2, but in master only one of them (#267721) exists |
|
Oh, OK, and for that there is no fix yet, right? I'll reopen that issue then |
|
If find the logic in this PR hard to follow. Here is my take on the same issue: #3387 What do you think? |
The algorithm for finding the first and last note of the selection ends up with the two notes reversed. This happens only if a gracenote after is part of the selection. So I decided to test for this case and simply commute the two values.