fix #151521: restore selection for all linked scores on undo/redo #3160
Conversation
| @@ -140,7 +141,8 @@ void Score::endCmd(bool rollback) | |||
|
|
|||
| if (MScore::debugMode) | |||
| qDebug("===endCmd() %d", undo()->current()->childCount()); | |||
| bool noUndo = (undo()->current()->childCount() <= 1); // nothing to undo? | |||
| // nothing to undo if current undo macro has only the SaveState commands for each score | |||
| bool noUndo = (undo()->current()->childCount() <= scoreList().size()); | |||
There was a problem hiding this comment.
This sounds dangerous. Are we 100% sure that these commands are SaveState ones?
There was a problem hiding this comment.
Not 100%, no, although it "should" be so. After all, we did put those SaveState commands there, and it's hard to imagine what might have removed them. And in the current source we check for a childCount() of 1, but we don't check that this is the SaveState for the current score, either.
But sure, better, I suppose, would be to check the actual commands. But then, if there is something else there, that would be pretty unfortunate, as it isn't clear how that should happen or what we should do if it does - what would have happened to our SaveState commands?
Anyhow, I'm certainly open to other solutions.
|
@MarcSabatella The code in master is significantly different... Could you please check the case whether it is still actual? |
|
No crash following those same steps. I think that's because we no longer even attempt to save/restore selection on undo/redo - it was commented out a year ago in SaveState:: ac41fa3 My guess is that this was to work around some other problem - or maybe just a different manifestation of this same problem. And to be sure, selection potentially gets messed up right now between linked scores, on the undo as well as the original operation. So I don't for a minute believe all is well. I still think there should be an effort to manage selection better. But I don't have the stomach for it myself... |
|
There is probably not much chance this PR has any value any more - the crash is gone, things have changed, etc. |
Currently, we begin each undoable command with a "SaveState" - basically remembering the input state and selection, so we can restore it on undo. Howeve,r we only do this for the score actually being edited, not for any linked scores. As a result, if you perform an operation while viewing the score but undo while viewing a part (or vice versa), strange things can happen. In particular, if you had anything selected when you pressed Ctrl+Z, that may or may not still be a valid selection when the operation completes, and we have no way of detecting problems here. As result, you can get a crash if you try doing an operation on the selection after an undo of an operation originally performed in another linked score.
I see two basic ways of dealing with this. One is to do the same thing we do on every regular command: clear the selection in all linked scores (except the one we are actually editing). However, due to the way things are structured, this wasn't actually as simple as it seemed it would be at first. It turned out to be easier to do what I considered the better solution: saving/restoring state for all linked scores on every command.
Seems to work just fine; I haven't been able to break it. Since it affects pretty much all commands on scores with linked parts, though, it really should get more testing.