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

add clefs, keysigs, and timesigs by double click #2104

Merged

Conversation

MarcSabatella
Copy link
Contributor

This change allows you to add clefs, key signatures, and time signatures by selecting a range and double clicking the palette icon. If the range is exactly one full measure, then only that change is inserted - so the change lasts until the end of the score. If it is longer (or shorter), then the original clef / keysig / timesig is restored at the end of the selection. For clefs, mid-measure changes are supported.

@@ -276,7 +282,7 @@ static void applyDrop(Score* score, ScoreView* viewer, Element* target, Element*
DropData dropData;
dropData.view = viewer;
dropData.pos = pt;
dropData.dragOffset = pt;
dropData.dragOffset = QPointF();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not actually sure about this; maybe I should have left it alone. It's only used for symbols apparently?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It seems that either way, the one case where it seems relevant doesn't work right: adding a symbol to another symbol via double click. The problem is that neither pos nor dragOffset will have meaningful values in that case - they are both set to 0. Seems we really need to set pt the same way here as in the code below for measures - we need the beginning-of-score-relative position of the target, since that is what drop() relies on. But it's not directly relevant to my changes here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Further investigation led me to discover that target->pagePos() is actually good here. I guess it needs to be score-relative only for measures, page-relative for everything else? So I have updated this.

@MarcSabatella
Copy link
Contributor Author

Also added support for MARKER / JUMP (segno, coda, dc al fine, etc), and fixed a small issue with interpretation of position when adding a symbol to another symbol. I guess these two commits will need to be squashed, but I'm leaving them as is for now so the line notes I added will still be valid, in case anyone has any comments.

@MarcSabatella
Copy link
Contributor Author

This PR now supports all the palette elements I can think of that formerly required drag & drop. Most simply use the existing code for barlines (improved to loop through measures). Only clefs, keysigs, and timesigs have or need the special handling to restore the original at the end of the selection.

if (element->type() == Element::Type::CLEF) {
if (sel.startSegment()->segmentType() == Segment::Type::ChordRest && sel.startSegment()->rtick() != 0) {
ChordRest* cr = static_cast<ChordRest*>(sel.startSegment()->element(i * VOICES));
if (cr->isChord())
Copy link
Contributor

Choose a reason for hiding this comment

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

What about if cr && cr->isChord() ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ooops, yes, seems it would crash with different rhythms on different staves and mid-measure clef changes. I should probably find the next cr. I'll make that change and squah/rebase a bit later, also see if you find anything else that needs fixing.

@MarcSabatella MarcSabatella force-pushed the double-click-clef-keysig-timesig branch from e70f017 to 2d041c9 Compare July 3, 2015 19:30
@MarcSabatella
Copy link
Contributor Author

Fixed the crash on cr == 0 when adding clefs to a multi-staff selection, and squashed commits.

lasconic added a commit that referenced this pull request Jul 4, 2015
…timesig

add clefs, keysigs, and timesigs by double click
@lasconic lasconic merged commit ad99577 into musescore:master Jul 4, 2015
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

2 participants