Time signature replacement crashes if double-clicking first on it

• Dec 29, 2018 - 01:29
Reported version
3.0
Priority
P0 - Critical
Type
Functional
Frequency
Once
Severity
S2 - Critical
Reproducibility
Always
Status
closed
Regression
Yes
Workaround
No
Project

Take the default score
Double click on the 4/4 time signature using mouse
Go to the Time Signature pallet
Double click on a replacement signature eg 3/4
Crashes.

Note. You have to double click on the default TS when selecting it. Single click select works fine.


Comments

Title Time signature replacement crash Beta 3.0 Time signature replacement crashes if double-clicking first on it

The normal (documented) procedure for changing time signature is either:
1) Select, ie one click, the time signature/measure/rest on the score + double-click in the palette
2) From the palette, drag and drop the new time signature onto measure/rest/existing time sig.

If by misunderstanding or clumsiness, the first step is: double-click on the time signature, that leads to crash (if second step is also by double-clicking)

  • Note that with the key signature, and same initial step (double-click on it), the key signature is cleared after double-clicking in the palette. But no crash, and Undo/Redo (?) leads to the expected display. Is it the expected behaviour? No sure.

Here is call stack:

MuseScore3.exe!QList::operator[](int i) Line 548    C++
MuseScore3.exe!Ms::System::staff(int staffIdx) Line 125 C++
MuseScore3.exe!Ms::StaffLines::pagePos() Line 53    C++
MuseScore3.exe!Ms::Element::pageBoundingRect() Line 242 C++
MuseScore3.exe!Ms::BspTree::items(const QRectF & rec) Line 144  C++
MuseScore3.exe!Ms::Page::items(const QRectF & r) Line 59    C++
MuseScore3.exe!Ms::ScoreView::paint(const QRect & r, QPainter & p) Line 1123    C++
MuseScore3.exe!Ms::ScoreView::paintEvent(QPaintEvent * ev) Line 903 C++

assert failure with:

inline const T &QList::operator[](int i) const
{ Q_ASSERT_X(i >= 0 && i < p.size(), "QList::operator[]", "index out of range");

although i is 0 there. So what it seems is that SysStaff* staff is a bogus object...since the indexing of _staves[0] is failing.

As that earlier mentioned commit says it is called "fix #270381 Crash after using F2 to insert ü", I've just checked to make sure that I can still insert various symbols by opening master palette and double-clicking. So at least I believe I haven't broken what that commit tried to fix.

I was chatting with Johan Temmerman. I was asking about if it makes sense to disable the palette entirely while in edit mode. He made a counter suggestion:

[quote]I mean, if after editing an element I double click something on the palette, isn't that a clear enough intent of action that should in itself rather take us out of edit mode of that element and then apply the palette action?

As a user I'd get frustrated in having to click "somewhere else" first (for newbies) or hit Esc every time[/quote]

So that seems to maybe be a better way to deal with this crash: simply exit edit mode if use double-clicks palette.

I was also noting some funny behavior if double click a Measure Rest (entering edit mode) and then double-click system text in palette, the system text would appear to not be applied. But upon clicking the measure rest (exiting edit mode) and double-clicking system text in palette, it would notice two systems texts have been applied...which suggests that the earlier system text was actually applied, although maybe that measure wasn't updated so we didn't see it:

edit-mode-staff-text.gif

Anyway this leads me to think that would be better to simply exit edit mode when double-click on palette.

Status active PR created

Good. Submitted PR to exit edit mode on double-click palette: https://github.com/musescore/MuseScore/pull/4524

Seems to be working well (i.e. no crash or glitches). And I've verified though debugger that when I call viewer->changeState(Ms::ViewState::NORMAL); coming from edit mode that changeState does test for:

case ViewState::NORMAL:
                  if (state == ViewState::EDIT)
                        endEdit();

and runs endsEdit() as I think is desired.

Although mattmcclinch says in https://github.com/musescore/MuseScore/pull/4524#issuecomment-450603804 that "I really do not think that this is the solution. Remember #3899 and all the problems it caused?"

Regression No Yes

Thanks guys. I just replaced my beta with the 3.0 stable. It crashed again. Has the fix been included or an expected update later? Here's what I got on the screen when I went through the same process earlier

Attachment Size
20190107_134801.jpg 2.44 MB
Status PR created fixed

Fixed in branch master, commit 7f9f1df8eb

fix #280830 exit edit mode if double-click palette

Previously, if user was in edit mode (by double-clicking an element in score), then applying palette elements by double-clicking would be problematic. This would be because Palette::applyPaletteElement() doesn't perform score->startCmd() before applying the element nor a score->endCmd() after applying the element, because it assumes that the score is already in a startCmd.

Exiting edit mode fixes #280830 which was a crash when applying a time signature from palette while in editmode. And fixes another glitch where applying system text from palette while in edit mode.

Turns out that ScoreView::editMode() encompasses more than just plane element edit, so I'm also making sure not inside states for LYRICS_EDIT, HARMONY_FIGBASS_EDIT, and TEXT_EDIT, where it might make sense for user to apply a symbol from a palette by doubleclicking.

To be clear though: the crash it. happens because you are double-clicking the time signature in the score - you aren't supposed to do that. If you just single click, it works as expected already.

Fix version
3.0.1