7-Tuplet does not copy correctly

• Mar 12, 2015 - 17:47
Type
Functional
Severity
S3 - Major
Status
closed
Project

(Evidenced on OSX 10.2.2, 2.0 Beta 2 although Marc Sabatella confirms exists on latest builds)

Where a 7-tuplet in a single crotchet beat (7 semiquavers) is copied and pasted, it is written as 6 semiquavers and a 7th demisemiquaver, yet still occupying one beat and notated as -7-. This is incorrect.

Example exists in attached score - see bar 10 and try to copy the 7-tuplet to a blank bar.

Attachment Size
Trial_By_Jury - No.1.mscz 35.96 KB

Comments

After the bad copy, the score is corrupt - save it and you get an errors on load.

Realistically, it's not the worst corruption I've ever seen. Ignore the error on laod and you're back where you were, and things do basically work. So I'm marking this "major" rather than "critical".

The bad copy dates back to at least the beta. But it did work in 1.3 whereas lots of other things involving copying of non-simple tuplets didn't.

The problem happens here (although the fix could perhaps be elsewhere):

https://github.com/musescore/MuseScore/blob/master/libmscore/paste.cpp#…

The segments making up the septuplet are 69 ticks apart, and 69 * 7 is 483. By the time we get to the last note of the tuplet, we are at 483 ticks. So we decide here to shorten the last note, thus turning it into a 32nd and creating what will later be detected as a gap.

While we could consider exempting notes within tuplets from this shortening - we should already be enforcing a no-partial-tuplets rule anyhow - I wonder if maybe we would be better off forcing tuplets tick counts to always round down? The fact that the septuplet adds up to too mch is, I think, why it's hard to select the septuplet - its tick count actually overlaps the following segment. As far as I can tell, you pretty much have to select it backwards (click last note, shift+click first) to select it without the following note or rest. Maybe if the segments were only 68 apart, we wouldn't have this problem. And as far as I know, no true gap would be detected, since the notes would at least still all be 16ths. It's the 32nd note, not the tick discrepancy, that is the real problem here I assume.

Still looking...

Funny, looks like I "fixed" that code in the last hours before beta 1 - see #30661: Copy-paste or repeat Triplet and others (quintuplet...) displays wrong results. Well, I *did* fix the case at hand for that issue, and actually, I don't think I made this case any *worse* - I think it would otherwise have shortened the last note even more. Anyhow, I'm considering exempting tuplets here, or adding a fuge factor (much as I hate those). As well as perhaps rounding down the tick counts in tuplets with ratios that don't divide evenly into 480.

Status (old) active patch (code needs review)

https://github.com/musescore/MuseScore/pull/1879

This implements the simplest of the possible fixes - I exempt tuplets from the shortening code (that exists because of #11889: [Trunk] Pasting multi-voice notes corrupts timing). I rely on the fact that we are already preventing partial tuplets from being copied, so there should be reason to ever shorten the last note of a tuplet on paste. We could certainly also investigate more complex fixes, but this does the job for now as far as I can tell.

Status (old) patch (code needs review) active

lasconic correctly points out that my proposed fix crashes when copying the full measure in the original example. Anyhow, the analysis part is still valid, we just need a better fix.

I think we may be able to do this. The original change - not shortening the last note of the tuplet even though it is a few ticks too long - solved the original problem when copying just the tuplet, but it crashed when copying the whole measure. I now see that is only because by not shortening the last note, and by copying the full measure, we are now thinking that this note extends past the end of the measure. As a result, we are trying to break it up with a tie, and that's just not going to work.

If we take my original fix and add a check for cr->tuplet() here:

https://github.com/musescore/MuseScore/blob/master/libmscore/paste.cpp#…

so that we don't try to break a member of a tuplet up over the barline (which isn't going to work anyhow), this results in the pasted tuplet being an exact copy of the original. All durations, duration types, and tick counts match.

So I think this is the correct least invasive fix. I do think we could also consider always rounding tick counts down when creating tuplets as it is likely to be less problematic. And of course ultimately, the more we rely on fractions rather than tick counts, the better. But implementing the fix as I am describing it results in the paste tuplet being no worse than the original, and it works both when copying the tuplet along and when copying the full measure.

I think I will update and reopen my PR, or else submit a new one, so we can consider this. It would be really nice to not release with this known cause of corruption if we can help it.

If I select (by hitting Shift key) the last (seventh) C , then the first, the selection includes the 7-Tuplet correctly.
select last note .jpg
If I begin by selecting the first C, then the last, the selection engloble the next beat , in this case, the quarter rest.
select first note.jpg

Or, other example (all produced with fa0f99b)
7 tuplet.jpg
Is it the expected behavior? Probably not?