Undo does not restore sound of deleted instrument change, crash on subsequent undo

• Jan 24, 2015 - 02:05
Type
Functional
Severity
S2 - Critical
Status
closed
Project
MuseScore

1. Open piano score.
2. Apply 'Instrument' (from 'Text' palette) to first note of bar 2.
3. 'View'>'Mixer'.
4. Change sound of the entry (e.g. 'Harpsichord').
5. Delete the instrument change text.
6. 'Edit'>'Undo'.
7. Click note, or play score.

Expected result: The notes of the changed instrument sound harpsichord.
Actual result: The notes of the changed instrument sound piano.

Notes:

This can also be reproduced if substituting steps 3-4 with the 'Change Instrument…' process via right-click of the text.

This is reproducible in 1.3 and 2.0 scores.

Using MuseScore 2.0 Nightly Build a78eec8 - Mac 10.7.5.

Comments

• Mar 4, 2015 - 14:55
Title Undo does not restore sound of deleted instrument change ⇒ Undo does not restore sound of deleted instrument change, crash on subsequent undo
Status (old) closed ⇒ active

The recent changes involving this code seem to have broken this again, even after the fix that allows Instrument Change text to be placed on tick 0 again. And now there is is a crash with a particular series of steps (well, for all I know, that might have been there before too).

Here are steps to reproduce - basically the same as the original:

1) my first score
2) add quarter notes C to first two measure measure
3) add instrument change text to first note of bar 2
4) use Mixer to change sound in second entry to harpsichord - verify it plays as harpsichord
5) delete instrument change text - verify it now plays as piano
6) undo

Result: sound remains piano

Frankly, that in itself would be minor. However, now try the following:

7) undo again

Result: crash. Debugger shows us to be in ChangePatch::flip(), and I'm pretty sure the "channel" info we are trying to access from the undo element is garbage.

I'm not sure this is 100% repeatable with these steps - you may need to add another text and try again, etc, as the garbage value may or not result in a crash immediately. But it definitely crashes pretty reliably for me.

Reply

• Mar 4, 2015 - 20:03

Reproduced on Ubuntu 13.04, MuseScore 2 commit ba34515.

The same happens if you try to undo path change after changing the staff instrument:
1. Create a piano new score, add a few notes.
2. Change sound to celestia in Mixer. Verify it plays as celestia.
3. Right click on the staff -> Staff Properties...->Change Instrument...
4. Select "Voice". Press Ok. Verify it plays as voice.
5. Undo. Patch changed to celestia.
6. Undo -> Crash.

"Result: crash. Debugger shows us to be in ChangePatch::flip(), and I'm pretty sure the "channel" info we are trying to access from the undo element is garbage."
Yes, as I understand, after removing an InstrumentChange or Instrument "channel" pointer becomes wrong. When we hit "Redo" a new Instrument is created, so "channel" stays wrong.

An easy fix:
Every time we should undo patch change, check the pointer if it is valid (if it is in score()->midiMapping()). If yes, change the patch, otherwise just do nothing.
In this case undo/redo will work unless we remove some instrument. After removal we won't be able to apply redo.

Medium fix: <-preferred option
Give more freedom in changing the undo stack. If we should undo/redo the path change, we will search for newly-created Instrument and use it's channel.

Advanced fix:
Review/rethink undoing patch change, abandon using pointer to channel. Use something different, that could be re-applied to a newly-created instrument.

What do you think?

Reply