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 #280817 staff implode delete orphaned spanners #4522

Merged

Conversation

ericfont
Copy link
Contributor

Previously, imploding a staff would leave orphaned spanners after the notes they connected to were deleted when moving those notes to the first voice. These orphaned spanners would cause a crash during layout when the Implode command finished.

This is a simple fix to avoid a crash by deleting these orphaned spanners.

An ideal solution would be to somehow reconnect these orphaned spanners to the new version of their original notes after they move to the first 1st voice. But that solution would require a lot more thought to get correct, and would also require special consideration for various corner cases, such as what happens with spanners who have one end connected to notes outside the imploded range but the other end connected to a note inside the range...

Previously, imploding a staff would leave orphaned spanners after the notes they connected to were deleted when moving those notes to the first voice.  These orphaned spanners would cause a crash during layout when the Implode command finished.

This is a simple fix to avoid a crash by deleting these orphaned spanners.

An ideal solution would be to somehow reconnect these orphaned spanners to the new version of their original notes after they move to the first 1st voice.  But that solution would require a lot more thought to get correct, and would also require special consideration for various corner cases, such as what happens with spanners who have one end connected to notes outside the imploded range but the other end connected to a note inside the range...
@ericfont
Copy link
Contributor Author

Original report at https://musescore.org/en/node/280817 (but note my comments there are stream of thought & debugging, so don't read my comments).

@ericfont
Copy link
Contributor Author

This version of slur hell at least avoids crashing:

implode-slur-hell

I am observing that the final half measure for some reason doesn't implode...but I don't think that is my code to blame...let me look into that

@ericfont
Copy link
Contributor Author

After some quick investigation to explain what is going on at the final half measure not imploding, I see this is deliberate behavior from #4309 to for the time being not implode onto a rest in the first voice...but I see there is a TODO in the code to maybe add a feature to still implode in that case. So I don't need to worry about that for this PR.

@ericfont
Copy link
Contributor Author

I think this PR is ready for review...very basic fix to avoid crashes.

@Jojo-Schmitz
Copy link
Contributor

We may want to combine it with the sanity checks from #4534

@anatoly-os anatoly-os merged commit 7eb067c into musescore:master Jan 8, 2019
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