Copy-paste sextuplets/octuplets and their removal leads to corruption

• May 17, 2017 - 22:21
Reported version
2.2
Type
Functional
Severity
S2 - Critical
Status
closed
Project
MuseScore
Tags
2.2

In 4/4 time, sextuplet eighths can't be changed to a larger note value starting halfway through the measure (beat 3).

Changing a sextuplet eighth to a sextuplet sixteenth works, but also doesn't produce the rest that should be there between the sixteenth and the next note.

Deleting two or more sextuplet eighths also results in the lack of rests.

Comments

• May 18, 2017 - 02:45
Title sextuplet eighths can't be changed to a larger note value ⇒ Sextuplet eighths can't be changed to a larger note value and removal leads to corruption

Steps:
1) Load this file: tuplets.mscz
2) Select the first rest of second sextuplet, image below - or first sextuplet (same with octuplets, second measure) and type 6 (half note value)
sel.jpg
Result: nothing happens
3) Delete the sextuplets and octuplets
Result: corruption
2 beats.jpg
beats2.jpg
NB: the octuplets react in a different way (the corruption is different) after save/reload the score, or after copy-paste (ie two 16th rests, instead of a half rest and a 16th rest)

No see, by now, similar corruption with eg septuplets and quintuplets (and "Other"?)
Further checking probably useful.

Reply

• May 18, 2017 - 14:22

I find no problem creating such sextuplets directly, but if I create it then copy and paste it elsewhere, then I get the problem. Is that how the sample file above was created as well? If not, can you explain how you created those tuplets?

Reply

• May 18, 2017 - 15:30

If it's the first one (ie https://musescore.org/en/node/201746#comment-719146),
I fill the measure with 8th rests.
Like this: sextup1.mscz

So, it's seems the problem comes from copy-paste (and so, always related I guess with the issue root? : #136406: Non-Reduced ratio tuplets of full measure duration not copy-pastable

Because with this file, if I do:
1) Select the measure
2) Notes -> Tuplets -> Sextuplet (for the record, the shortcut Ctrl + 6 doesn't work here, but no matter, due probably to my AZERTY keyboard configuration)
3) Remove the last sextuplet
4) Save-Reload
Result: no corruption observed

BUT:
1) Create a first sextuplet (in the way of previous step #2), see image below.
exemple 1.jpg
2) Then, copy-paste one by one on the other rests (or by "R" key)

3) Remove the last sextuplet ->you see already the corruption: 16th rest instead of a 8th rest (second part of image below)
im set.jpg
4) Save-reload
Result: corruption warning

Reply

• May 18, 2017 - 19:27

OK, thanks. But actually, I now see I can also reproduce the problem not just after copy & paste but also after save / reload. That is:

1) new 4/4 score
2) note input
3) 6 Ctrl+6
4) save
5) reload

Result: measure is now in the same state as the example in #2.

The code used for save/load is basically the same as used for copy/paste, so it often works out that if one fails, so does the other.

The line that was changed in that previous fix is definitely the culprit. Previously it was setting a too-long duration for the tuplet, hence the error about crossing barlines on paste. Now it's setting a too-short duration, preventing you from lengthening notes and creating corruption on delete. BTW, it was probably possible to create corruption before as well. I'd need to think this through more to really understand.

Reply

• May 18, 2017 - 19:52

Seems like the same problem indeed.

Debugging a bit more, I think the original code had it right - we shouldn't be reducing the ratio when calculating the duration for the tuplet. The problem in the original bug report, I think, is that the baseLen was wrong. That is, if you select a full measure reset and set a ratio of 10/8, then the baseLen should be 1/8, not 1/4. I guess we are setting 1/4 note to simplify the display (we actually default to showing five quarters, not ten eighths). But if we're going to do that, then we should reduce the ratio as well.

Which is to say, I think the original problem in #136406: Non-Reduced ratio tuplets of full measure duration not copy-pastable is in the code that created the tuplet from the dialog in the first place, not here in Tuplet::read(). That's my hunch, anyhow. Because I can't really see how else we'd be able to calculate the duration correctly here.

Reply

• May 19, 2017 - 11:30

See also:
https://github.com/musescore/MuseScore/pull/2880
and in particular: https://github.com/musescore/MuseScore/pull/2880#issuecomment-258902959

Commit in master branch: 3b7c0fedd43

In principle, in master branch the bug should not be present any more (I can't directly check at the moment). I think for compatibility with previously saved files it was decided not to include the correction of the tuplet baselen duration set in the tuplet dialog for branch 2.1.

EDIT:
See here: https://github.com/musescore/MuseScore/pull/2881#discussion_r86953943
about the change in master

Reply

• May 20, 2017 - 22:51

Thanks, that explains a lot. I do now remember that discussion but didn't really understand the issue at the time so it didn't sink in.

Anyhow, I do concur the original problem really was with the calculation of baseLen. I believe we should revert the Tuplet::read() fix, as I don't believe it's possible to get that approach to work in this new case, and instead incorporate the tupletDialog() fix. Again, with more testing of course to be sure we haven't missed something.

Reply

• May 22, 2017 - 07:45
Title Sextuplet eighths can't be changed to a larger note value and removal leads to corruption ⇒ Copy-paste sextuplets/octuplets and their removal leads to corruption

More complete title.

Reply

• Aug 30, 2017 - 14:43

There is still the issue of the case that is not fixed by that PR. I don't know if that is reason to abandon that approach to find a new one, or if could at least be considered a step in the right direction.

Reply

• Nov 2, 2017 - 20:48

I'm wondering how bad things actually are right now. I see we get corruption if you delete the sextuplet, or otherwise try to alter durations etc, after a copy/paste operation or a save/reload. But, if you just enter the sextuplet and leave it alone, do bad things happen later? In other words, do I need to avoid sextuplets completely, or just be careful with them? Optimistically I hope being careful is enough...

Reply

• Nov 2, 2017 - 23:48

V2.1 I just did a test on a sextuplet based upon a 1/4 note.

I created it, saved, reloaded and tried to change a 16th rest to an 8th rest and it wouldn't change any of them after the second one. None of them would change to a 1/4 rest and only the first would change to a dotted 16th. I tried to change one to a 32nd rest and it worked. Same results after create, copy and paste. Changing them to notes, then deleting them worked.

I guess the answer it it will work as long as you edit it before reload or paste. The decision is yours as to if it's worth it.

Reply

• Feb 9, 2018 - 12:04

Let's take the bull by the horns and try to do something about this bug.
Here is the situation at the moment in 2.1(2.2-dev).
Example: create a 2/4 sextuplet in two different ways:
(1) By selecting a 2/4 rest and ctrl+6
(2) By selecting a 2/4 rest and Notes->Tuplets->Other...->Relation=6/4->OK, and then changing the quarter rests to eighth rests/notes (note that the fact that the tuplet is filled with 3 quarter rests instead of 1/8th rests is already an indicator of a wrongly set base length).
In (1) the duration is set to 2/4 (correct) and the base length to one eighth note (correct).
In (2) the duration is set to 2/4 (correct) and the base length to one quarter note (wrong).
Following the comment by zhangmei ( https://musescore.org/en/node/202271#comment-817667 - by the way: good catch!) we can try to test these tuplets.
Whichever of the eighth rests (or notes) of the two tuplets we select and press 3, the change is as expected.
However, the tuplet of (2) can give other problems when copy/pasting, because its base length is not correct.
Now save the score, close the tab and load the just-saved file.
Upon reading:
The simplified ratio is 3/2.
In (1) the duration is set to 1/8(saved base length) * 2(denom. of simplified ratio)= 1/4 (wrong), while the base length is set to one eighth note (correct).
In (2) the duration is set to 1/4(saved base length) * 2(denom. of simplified ratio)= 2/4 (correct), while the base length is set to one quarter note (wrong).
As you can see, none of the tuplets is now a "good" object.
Indeed, following the comment by zhangmei we can try to select the rests/notes of these tuplets and press 3 to split each of these eighth rests/notes to two 16th notes. Tuplet (2) behaves correctly as it was behaving before (since its duration is set correctly). But tuplet (1) behaves correctly only for its first 3 notes, the last 3 ones get in a corrupted state.
Note that (1) is imported correctly by MuseScore 2.0.3 (both correct duration and base length), while (2) is imported with both a wrong duration and a wrong base length by 2.0.3.

I believe that we cannot solve the bug and guarantee at the same time that correct files will not exhibit one problem or another one (namely, wrong duration and/or wrong base length) when opened in the released 2.1.
In my opinion, we should make sure that:
-i- when creating the tuplet both duration and base length are set correctly (i.e. patch tupletdialog.cpp and maybe also tst_tuplet.cpp), and
-ii- then assume this when importing (and therefore revert d6869fedc0 )
By doing that, tuplet (2) would be imported with a wrong duration and a wrong base length, thus reopening #136406: Non-Reduced ratio tuplets of full measure duration not copy-pastable. Therefore, I think we should include a "sanitize-tuplet" step for all imported tuplets with a non-reduced ratio, similarly to the function I introduced in master, but extending it to all tuplets with non-reduced ratios (and I think this should be done also in master).
Such a complicated function is needed because, given the data saved in MuseScore files (mscz/mscx) it is not possible to compute the tuplet length a priori if we are not sure whether the base length is correct or not.
There would still be some cases in which such an approach would fail, namely the case of a tuplet with non-reduced ratio created in a voice >1 with the Tuplet Dialog and with some of its duration elements deleted (which is possible only for voice >1).

Something could be written in the "known incompatibilities between 2.1 and 2.2" section if we decide to go this way.

What do you think?

Reply

• Feb 9, 2018 - 13:19

With that PR as it is at the moment:
- tuplets of type (1) would be read correctly, but
- newly created tuplets of type (2) would have a wrong ratio and therefore https://github.com/musescore/MuseScore/pull/3197#issuecomment-309361716 and also a wrong base length (because the ratio was reduced), but these would compensate for each other. However, we are losing the information that the tuplet is a sextuplet of 8th notes since it would transform into a triplet of quarter notes.
Namely:
in the example, newly created sextuplet of type (2) would have a ratio 3/2 (instead of 6/4), a duration of 2/4, a base length of one quarter (instead of one eighth).

In my opinion in tupletdialog the ratio should not be reduced, and then the PR would be equivalent to the first part of what I am saying above.

However, I think a function should be run on import to heal already corrupted tuplets of existing files.

Reply

• Feb 28, 2018 - 10:32

Ok, I'm looking at this bug again and I updated the PR with @ABL suggestions. I'm trying to define if we really need a sanitize tuplet function and if we do, how complete it should be. What would be really helpful is "corrupted files" created in 2.1 or even earlier (1.3).

For 1.3, my go to test for tuplet is https://musescore.com/nicolas/fabric and it seems to work.
"My Instruments" can also be imported ok as far as I can tell.

Reply

• Feb 28, 2018 - 16:06

Note that after the PR issue #136406: Non-Reduced ratio tuplets of full measure duration not copy-pastable will be re-opened for already corrupted tuplets created by the tuplet dialog. That's why I think a sanitize step is needed.
Here are a couple of possible test files. The tuplet in these files were created in MuseScore 2.1 with the keyboard shortcut (first tuplets in both files, crtl+6 and ctrl+8, respectively) and with the tuplet dialog (second tuplets in both files, 6/4 and 8/6 ratio, respectively).
As you can see from the mscx, the base length of the second tuplets (created with the tuplet dialog) is wrong (1/2 instead of 1/4 in the first file, 1/2 instead of 1/16 in the second file).
We must ensure that:
- both first and second measure can be copied to the empty measure: this summarizes issue #136406: Non-Reduced ratio tuplets of full measure duration not copy-pastable and my comment in https://github.com/musescore/MuseScore/pull/2881#discussion_r86953943 ; without a "sanitize" step, the tuplets created with the tuplet dialog will give a "tuplet cannot cross barline" error after the merging of the PR (which however I think is the correct way to proceed).
- the E in both tuplets of test1 (i.e. 4th note in the 6-uplet) can be changed to a small value, similarly to what happens when selecting them and pressing "3", and this does not corrupt the measure (i.e. also the rests are added, so that the total duration adds up to the correct value).

Attachment Size
test_tuplets1.mscx 6.49 KB
test_tuplets2.mscx 7.69 KB

Reply

• Feb 28, 2018 - 19:53

FWIW, if the worst that happens is that already-corrupted tuplets stay corrupted, but we manage to effectively prevent more from being created, I can live with that. People already have to correct corruptions manually, and while it might be "nice" to fix them automatically, that's not a deal-breaker to me if we don't offer that. If these tuplets don't actually report as corrupt but as merely not copyable, I can live with that even better.

Actually, if we do look at automatically fixing corruptions, I'd love a more general-purpose facility that can repair holes and overlaps in the segment list. I know this has been discussed and maybe even worked on to some extent?

I recognize that my grasp of the issue is tenuous as I still don't have a real clear sense of how these tuplets are "supposed" to be represented and what assumptions might exist in various places in the code about this.

Reply

• Mar 2, 2018 - 14:09

Thank you lasconic. Now the behavior is basically the same as in master.
It should work in most cases.
As it is written, it will always sanitize (i.e. sum up the element duration to compute the actual duration) every tuplet with non-reduced ratio imported, since after the import (which is equal to 2.1 import before the sanitize step) tuplet of both kind (1) and (2) [ see comment above https://musescore.org/en/node/202271#comment-819430 ] will go through the sanitize function.
It is probably a little redundant and it could slow down a litlle the loading of files containing a lot of tuplets with non-reduced ratios, but maybe better safe than sorry :-)

I think in both 2.2 and master the "isValid" duration function could be improved by checking as first thing if the denominator of the fraction f is a power of 2.

EDIT:
I am wondering if also in tst_tuplet.cpp we should correct this line:
https://github.com/musescore/MuseScore/blob/2.2/mtest/libmscore/tuplet/…

Reply

• Mar 8, 2018 - 11:06

The problem comes from the copy-paste step.
Here:
https://github.com/musescore/MuseScore/blob/2.2/libmscore/paste.cpp#L158
the tuplet is read by the same function which reads a saved file, but it does not pass through the sanitation step. As you can see from my comment above, tuplets with non-reduced ratio are read back with a wrong duration, which is then corrected by the sanitize step.
In this case, therefore, the duration is set wrongly.

In my opinion we should: correct the read function so that correct tuplets are read correctly, by avoiding the reduction of the tuplet ratio during read step. I think it is then safer to extend the sanitize step to all tuplets with non-reduced ratio, even if in principle not needed for these kind of tuplets (the if clause comparing the two durations would prevent this step for these tuplets), since we do not know a priori if the tuplet is of kind (1) or (2) [see comments above for the difference between the two cases].

Reply