fix #97106: crash on undo after save with courtesy keysig #3154
Conversation
|
Here too my fix (adding this |
|
Actually, I'm not so sure it wasn't a good fix at the time. It may well be calling update() might have been safe at that point, but changes made later may have made it so doLayout() needs to be called within an undo context to work properly. After all, as per cadiz1's investigation, these steps didn't actually crash until later in the development cycle. Still amazing to me, though, that as common as the situation that tirggers this must be, we haven't heard more reports of this. |
| startCmd(); | ||
| info.refresh(); // update file info | ||
| setLayoutAll(true); | ||
| endCmd(); // force relayout |
There was a problem hiding this comment.
Would endCmd(true) work here? instead of the two next lines.
There was a problem hiding this comment.
Pretty sure I tried that but it still left a null entry on the undo stack. Now I'm doubting myself, so I'll check again.
The crash is actually an assertion failure, removing a keysig from a segment that doesn't contain one. The cause seems to be the layout that is forced at the end of the save (to update $m macro in header/footer), because it happens while not in undo mode. Generated elements get added and removed during layout, and doing this while not in undo mode confuses the undo stack a bit. I can't say I understand exactly what goes wrong in this case, but I am pretty confident this is the problem.
Assuming we want to keep the layout operation at the end of the save, the solution seems to be to do so in the context of startCmd()/endCmd(). This puts an extra macro on the stack, so I remove it afterwards. Because endCmd() does an update() anyhow, I removed that, but I call setLayoutAll() first to make sure all linked scores get updated. Finally, I moved this bit of code to before the undo()->setClean() just to be sure we are in a clean state after the save.
This fixes the crash and doesn't introduce any null undo steps or other artifacts that I can see. It also fixes another small bug in the original code, where $m macros in headers/footers would only get updated on save if the score has parts. That's because there was no setLayoutAll() call before the update(), and unless there were parts, _updateAll would generally not be set at that point.