Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix #106146 Crash when pressing LEFT in note entry mode #2546

Merged
merged 1 commit into from Apr 15, 2016

Conversation

heuchi
Copy link
Contributor

@heuchi heuchi commented Apr 14, 2016

@heuchi heuchi force-pushed the 106146_CrashLEFTinNoteEntryMode branch from 48764a7 to f8fb525 Compare April 14, 2016 19:01
@@ -116,7 +116,9 @@ void InputState::moveInputPos(Element* e)
return;

Segment* s;
if (e->isChordRest())
if (e->isSegment())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The else is not useful anymore then.
Also, it's probably a good case to use the new toSegment(), toChord() everywhere in the function https://github.com/musescore/MuseScore/blob/master/mscore3.txt#L70

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated to follow new guide lines and removed the else branch.
I added a Q_ASSERT to make sure s has been assigned a valid pointer.

@heuchi heuchi force-pushed the 106146_CrashLEFTinNoteEntryMode branch from f8fb525 to 3d5fdad Compare April 15, 2016 09:09
@heuchi
Copy link
Contributor Author

heuchi commented Apr 15, 2016

The main reason for the crash is the Q_ASSERT statement in Element.isChordRest() in file libmscore/element.h:616

bool isChordRest() const { Q_ASSERT(type() != Element::Type::SEGMENT);
      return type() == Element::Type::REST || type() == Element::Type::CHORD
      || type() == Element::Type::REPEAT_MEASURE; }

I don't quite understand why we disallow a call to isChordRest() for Segment pointers.

EDIT: Just noticed there is isChordRest1()...
Going to update this PR to use that...

@heuchi heuchi force-pushed the 106146_CrashLEFTinNoteEntryMode branch from 3d5fdad to 3e21eb9 Compare April 15, 2016 09:31
@wschweer
Copy link
Contributor

There was aname conflict for isChordRest(); in old code it was used to check Segments (and the segment type). Current code use isChordRest() to test the element type. To find the old uses i added an ASSERT. For legal uses of isChordRest() on segments you have to use isChordRest1(). This is a hack and isChordRest1() will be replaced by isChordRest() and the assert removed.

@lasconic lasconic merged commit bfc67b4 into musescore:master Apr 15, 2016
@heuchi heuchi deleted the 106146_CrashLEFTinNoteEntryMode branch April 15, 2016 11:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants