Regroup Rhythms doesn't handle tuplets correctly

• Feb 23, 2020 - 12:22
Reported version
3.4
Type
Functional
Frequency
Once
Severity
S3 - Major
Reproducibility
Always
Status
PR created
Regression
No
Workaround
No
Project

Attach score shows a simple tuplet with just rests:
before.png

After Regroup Rhythms the score looks like
after.png

The root cause is Score::regroupNotesAndRests() doesn't take any tuplets into account. When Score::regroupNotesAndRests() sees a tuplet it should be handles as a separate case and when the tuplet are just all rests, replace it by a normal rest in which case it can be regrouped with other rests.

Attachment Size
before.png 13.14 KB
regroup.mscz 2.5 KB
after.png 24.93 KB

Comments

Some background: we are working on a project for Delft University with a few master students. We are studying MuseScore's architecture and are also looking to try and fix one or a few bugs, and we came across this one.

So, after some debugging and trying, we have pinned down that the lastRest.ticks() on the 3 eighth triplet returns 1/8 instead of 1/12, which as we understand should be the length of the final rest, to make sure that the restTicks (or the new rest that is created) is of the proper length.

However, although we can implement a workaround to work specifically for tuplets, this is probably not the best way for a fix, as there is most likely more wrong with the ticks implementation of a tuplet. But it is hard to see how much of the system will be affected if we change the tuplet ticks implementation.

I'm speaking off the top of my head a bit here, but I don't think it's necessarily wrong to return 1/8 for an eighth note or rest just because it is in a triplet. We make a distinction between the "duration type" versus the :"global duration" versus the "actual duration", in part for this very sort of purpose. Also to handle "local" time signatures (different time signatures for different staves on same measure). So possibly the issue is just the wrong function being called - ticks() versus globalTicks() versus actualTicks().

Status PR created active

I'm also looking into this and am wandering where you will come up with. In my solution I had to handle tuplets as a special case. But again, I'm curious to your solution.
In the meantime, I set the status back to active since there is no PR created yet. When you created to PR, please set the status to PR and put here a link to the PR.

Title Regroup Rhythms doesn't handle tuplets correct Regroup Rhythms doesn't handle tuplets correctly

Fix typo in issue title.