Corruption on deleting initial rest in voice 2 tuplet

• Mar 5, 2018 - 20:34
Priority
P2 - Medium
Type
Functional
Frequency
Few
Severity
S2 - Critical
Reproducibility
Always
Status
closed
Regression
No
Workaround
Yes
Project
MuseScore

To reproduce:

1) new score, treble template, 4/4
2) measure 1, note input
3) voice 2
4) 5 Ctrl+3 0 C C (to enter triplet starting with a rest)
5) exit note input
6) select leading rest
7) delete

At this point, the file is in an awkward position, and while corruption isn't necessarily present yet, it can be induced in several ways. For one, try selecting the measure and pressing "R", for instance) - you'll see the measures gets copied well enough but the one after is rendered corrupt in voice 2, with something other 4 beats of rest. Or try swapping voices 1-2 - corruption results in voice 1 as the rests inserted to fill the beginning of the measure are not part of the tuplet and don't add up.

Comments

• Mar 5, 2018 - 20:41

I still wish we didn't support deleting rests at all, as there are lots of problems internally with this that we keep having to patch. But given that we aren't ready to disallow it in general, we could at least disallow it for rests within tuplets. That will prevent people from creating this type of situation in the future. It won't help with people who already have scores of this form, though - not corrupt yet, but with corruption waiting to happen. That's actually how I discovered this, from a thread where someone was trying to copy content from such a score to a new one. See https://musescore.org/en/node/269973.

Unfortunately, preventing the problem once the hole is created probably means adding checks and handling for this in multiple places. The original problem was an error on copy paste because the duration of the tuplet was calculated incorrect. The copy paste problem I mention above happens elsewhere. The voice exchange problem might be different still, although it's possible both of these problems are in makeGap() or some such.

Reply

• Mar 6, 2018 - 15:33

In makegap, the main issue is that we call it based on the voice gap stored at the beginning of the clipboard. In this case, it's 160 for voice 2 (a 8th note in a triplet). When we call makeGap, makeGap1 and in the end expandVoice, we end up calling setRest() without even a tuplet being set... so in the end, no Rest are created and that's the beginning of the end.

Reply

• Mar 6, 2018 - 15:38

In deleteItem(), replacing the removal of a Rest in voice > 1 by switching visibility off is doable.

--- a/libmscore/edit.cpp
+++ b/libmscore/edit.cpp
@@ -2031,9 +2031,13 @@ void Score::deleteItem(Element* el)
                   if (rest->tuplet() && rest->tuplet()->elements().empty())
                         undoRemoveElement(rest->tuplet());
                   if (el->voice() != 0) {
-                        undoRemoveElement(el);
-                        if (noteEntryMode())
-                              _is.moveToNextInputPos();
+                        if (rest->tuplet())
+                              undoChangeProperty(el, P_ID::VISIBLE, false);
+                        else {
+                              undoRemoveElement(el);
+                              if (noteEntryMode())
+                                    _is.moveToNextInputPos();
+                              }
                         }
                   }
                   break;

Reply

• Dec 7, 2018 - 02:11
Priority P1 - High ⇒ P2 - Medium

This is mostly fixed now. You can still get corruption with some work, but it takes more determined effort and I can't reproduce the steps. So I'm leaving this open, but reducing priority.

Reply

Fix version
3.5.0