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 #116196: Crash applying line via double click (debug build) #2690

Closed
wants to merge 1 commit into from

Conversation

Jojo-Schmitz
Copy link
Contributor

No description provided.

@AntonioBL
Copy link
Contributor

Well, this PR does not completely solve the problem. If you try to add a hairpin with a double click from the palette you still hit the Q_ASSERT at line 737 of libmscore\element.h (referring to line 791 of element.h); the ottava case is solved since with this PR it no more goes inside the if-clause of line 294 of libmscore\cmd.cpp and therefore does not hit the "TextLine* tl = toTextLine(spanner)" instruction.
Hairpins, pedals and voltae are still affected, but only when inserted with double click from the palette.

@Jojo-Schmitz
Copy link
Contributor Author

Jojo-Schmitz commented Jun 24, 2016

Change to HairpinSegment and VoltaSegment seems to fix that. There is no PedalSegment though, not sure what to do about this?

Edit: Ah, found PedalSegment and PEDAL_SEGMENT

@AntonioBL
Copy link
Contributor

But that means it is now just completely skipping the if-clause of line 294 of cmd.cpp. Should it be removed then?
Otherwise, if the spanner is not strictly a textLine, the "toTextLine" instruction will always give the Q_ASSERT.

@Jojo-Schmitz
Copy link
Contributor Author

Hmm, just reverting back to TextLine* tl = static_cast<TextLine*>(spanner); should fix this too then?

@Jojo-Schmitz
Copy link
Contributor Author

OK, let's try another Approach then

@@ -732,6 +734,18 @@ static inline const DurationElement* toDurationElement(const Element* e) {
|| e->type() == Element::Type::REPEAT_MEASURE || e->type() == Element::Type::TUPLET);
return (const DurationElement*)e;
}
static inline TextLine* toTextLine(Element* e) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

maybe toSortOfTextLine() instead?

@Jojo-Schmitz Jojo-Schmitz changed the title fix #116196: Crash using ottava (debug build) fix #116196: Crash Crash applying line via double click (debug build) Jun 24, 2016
@Jojo-Schmitz Jojo-Schmitz changed the title fix #116196: Crash Crash applying line via double click (debug build) fix #116196: Crash applying line via double click (debug build) Jul 1, 2016
@Jojo-Schmitz Jojo-Schmitz force-pushed the ottava-crash branch 5 times, most recently from 5e4c40b to 69a89c2 Compare July 10, 2016 11:38
adding and using the proper CONVERT macros for isNoteLine() and
toNoteLine() as well as methods for isTextLineType() and
toTextLineType() in the due course
@Jojo-Schmitz
Copy link
Contributor Author

Seems this got made obsolete by 0b92b20 ?

@lasconic
Copy link
Contributor

After a weekend of thinking, Werner changed his mind and he believe "that in this special case a dynamic_cast may be faster and its the intended semantic". See 0b92b20
Would you mind doing another PR for the toNoteLine part ?

@lasconic lasconic closed this Jul 11, 2016
@Jojo-Schmitz
Copy link
Contributor Author

Done, see #2735, but it isn't used anywhere

@Jojo-Schmitz Jojo-Schmitz deleted the ottava-crash branch July 11, 2016 11:11
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