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 #26516: crash on repitch #992

Merged
merged 7 commits into from Jun 23, 2014
Merged

Conversation

MarcSabatella
Copy link
Contributor

No description provided.

@MarcSabatella
Copy link
Contributor Author

Closing temporarily while I finish looking at the tie issue

@MarcSabatella
Copy link
Contributor Author

I think ties are working as well as they need to. Only ties between single note chords are handled; one sided ties and ties from or to multi-note chords are removed.

My only concern is with how I am updating the tied-into notes. Currently, I copy pitch, tpc, tpc1, and tpc2 from the repitched note. It seems to work, including for transposing instruments, but the code could stand review.

@MarcSabatella MarcSabatella reopened this Jun 19, 2014
@MarcSabatella
Copy link
Contributor Author

Nope, doesn't work with linked staves/parts - I need a better way of updating the tied-into notes.

@MarcSabatella
Copy link
Contributor Author

undoChangePitch does the same thing I was doing manually, but works with links. I think everything works correctly now.

@MarcSabatella MarcSabatella reopened this Jun 19, 2014
if (replace) {
QList<Note*> notes = chord->notes();
Copy link
Contributor

Choose a reason for hiding this comment

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

There is still a small issue with tie.

  • Create a score with a quarter C tied to a quarter C
  • Select the first one, enter note input, enter repitch
  • move cursor to the second note, press D

--> Result : We see a C "tied" to a D
--> Expected result: not sure, maybe we need to make sure that user can't select the second note of a tie?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch! I think the best thing is to do what is done for up/down - automatically "rewinding" to the start of the tie at the beginning of the whole operation (and I can fix it in both places to not go into an infinite loop if a note is tied to itself).

Another possibility is to assume the user is doing this on purpose, and break the tie back from the initially selected note. But I think I'll try the first solution.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

On second thought, it seems the first solution may be more trouble than it's worth, plus it would raise questions about how to handle mutli-note chords anywhere in the chain. Much simpler and more predictable to simply break any tie into a note.

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 might as well get the ties working for MIDI too, and fix a couple of other issues with MIDI input on repitch (treatment of rests). Should have it ready by end of day.

if (!addFlag) {
QList<Note*> notes = chord->notes();
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add a comment here? Something like // in repitch mode only ties between single note chords are handled; one sided ties and ties from or to multi-note chords are removed.
Idem in the copy of the code, maybe with a reference to each other.

lasconic added a commit that referenced this pull request Jun 23, 2014
@lasconic lasconic merged commit 5f68171 into musescore:master Jun 23, 2014
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